Repository navigation
fix: select the OTLP log exporter by protocol like spans and metrics - #80
Merged
Merged
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
7 tasks done
iguehring-nvidia
added a commit
to NVIDIA-NeMo/Gym
that referenced
this pull request
Oct 6, 2026
…RPC port The driver environment pointed OTEL_EXPORTER_OTLP_LOGS_ENDPOINT at the collector's gRPC port because the nemo-lens of #3619 built its log exporter over gRPC whatever the protocol said. Lens picks that exporter by import order, not by protocol, so which one runs depends on the environment; in the Slurm jobs it is now the OTLP/HTTP exporter, which the gRPC port answers with a connection reset. Every Gym log batch fails with "Transient error ('Connection aborted.', ConnectionResetError(104, 'Connection reset by peer'))" while traces and metrics flow (seen on oci-hsg job 7755363). Logs now follow the shared endpoint and protocol like the other two signals. NVIDIA-NeMo/Lens#80 makes Lens honour the protocol for logs, so the choice is deterministic once that pin lands; until then this matches what the jobs actually run. The two prefill/decode fixtures lose the override line. Validation: pytest tests/unit_tests/test_orchestration_otel.py tests/unit_tests/test_slurm_script.py tests/unit_tests/test_slurm_executor.py tests/unit_tests/test_orchestration_api.py -- 367 passed, 1 failed (test_build_vllm_ray_serve_command_single_node_model_with_space_survives_quoting shells out and needs pip on the login-shell PATH; fails identically on main here). pre-commit on the changed files passes. Cluster check in progress: oci-hsg job 7755613 runs the same scicode config with driver.env routing logs to the HTTP port. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Ingo Guehring <iguehring@nvidia.com>
iguehring-nvidia
added a commit
to NVIDIA-NeMo/Gym
that referenced
this pull request
Oct 6, 2026
…RPC port The driver environment pointed OTEL_EXPORTER_OTLP_LOGS_ENDPOINT at the collector's gRPC port because the nemo-lens of #3619 built its log exporter over gRPC whatever the protocol said. Lens picks that exporter by import order, not by protocol, and in one Slurm job the processes disagree: the driver gets the gRPC log exporter, the Gym servers get the OTLP/HTTP one. With the gRPC port configured, the servers' log batches fail with "Transient error ('Connection aborted.', ConnectionResetError(104, 'Connection reset by peer'))", no server log reaches the collector, and the driver spends its shutdown in the HTTP exporter's retry loop until the job hits its wall time (oci-hsg job 7755363: rollouts done at 10:11, killed at 11:00 after 477 retries). Logs now follow the shared endpoint and protocol like the other two signals, which is correct once nemo-lens selects the log exporter by protocol (NVIDIA-NeMo/Lens#80). This change must land together with that pin bump: on the current pin the driver's gRPC log exporter would aim at the HTTP port instead (oci-hsg job 7755613 showed exactly that when the port was overridden by hand). The two prefill/decode fixtures lose the override line. Validation: pytest tests/unit_tests/test_orchestration_otel.py tests/unit_tests/test_slurm_script.py tests/unit_tests/test_slurm_executor.py tests/unit_tests/test_orchestration_api.py -- 367 passed, 1 failed (test_build_vllm_ray_serve_command_single_node_model_with_space_survives_quoting shells out and needs pip on the login-shell PATH; fails identically on main here). pre-commit on the changed files passes. Cluster validation of the combined fix pending the Lens pin. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Ingo Guehring <iguehring@nvidia.com>
`_build_log_exporter` picked the gRPC log exporter whenever that package imported and fell back to HTTP otherwise, ignoring OTEL_EXPORTER_OTLP_PROTOCOL and OTEL_EXPORTER_OTLP_LOGS_PROTOCOL. Spans and metrics already resolve the protocol first and fall back on ImportError only. A process configured for http/protobuf could therefore end up with a gRPC log exporter aimed at an HTTP endpoint, or, once a caller worked around that with a per-signal gRPC endpoint, with an HTTP exporter aimed at the gRPC port. Either way every log record is dropped after a connection reset, while traces and metrics flow. Seen in NeMo Gym's Slurm jobs: the driver sets http/protobuf with the collector's HTTP endpoint plus OTEL_EXPORTER_OTLP_LOGS_ENDPOINT on the gRPC port (the workaround from NVIDIA-NeMo/Gym#3619); the HTTP log exporter then logs "Transient error ... Connection reset by peer" for each batch. Logs now follow the same rule as the other two signals: the signal-specific protocol wins, then the general one, default grpc, with the existing import fallback. Three tests mirror the span-exporter protocol tests. Signed-off-by: Ingo Guehring <iguehring@nvidia.com>
iguehring-nvidia
force-pushed
the
iguehring/log-exporter-protocol
branch
from
October 8, 2026 00:41
6adfd6e to
e0f4b7f
Compare
iguehring-nvidia
marked this pull request as ready for review
October 8, 2026 01:01
ahmadki
previously approved these changes
Oct 11, 2026
Member
|
/ok to test e0f4b7f |
Exercise the HTTP-to-gRPC and gRPC-to-HTTP fallbacks and the error when neither exporter package is installed. Signed-off-by: Ingo Guehring <iguehring@nvidia.com>
Member
|
/ok to test b813584 |
ahmadki
approved these changes
Oct 11, 2026
Closed
3 tasks done
iguehring-nvidia
added a commit
to NVIDIA-NeMo/Gym
that referenced
this pull request
Oct 12, 2026
… the OTLP protocol Bump the nemo-lens pin from b0f977d4 to 4d045c03 (Lens main, includes NVIDIA-NeMo/Lens#80). Until #80, nemo-lens picked its OTLP log exporter by import order instead of OTEL_EXPORTER_OTLP_PROTOCOL, unlike spans and metrics. The driver environment that `gym eval submit` renders sets one endpoint and one protocol for every signal (#4167), which is only right when the log exporter honours them: in an environment that carries the gRPC exporter package the driver built a gRPC log exporter, every batch to the collector's HTTP port failed, and at shutdown the driver sat in the exporter's retry loop until the job hit its wall time. With the new pin the log exporter is chosen from the protocol like the other two, so the shared endpoint holds whichever exporter packages the host environment installs and no per-process port probing is needed. Adapt to the rest of the pin move: mirror the new `span_attributes` no-op in Gym's fallback shim (Lens#71), and point the span test fixtures at Lens's cached default-tracer lookup (Lens#73). Signed-off-by: Ingo Guehring <iguehring@nvidia.com>
iguehring-nvidia
added a commit
to NVIDIA-NeMo/Gym
that referenced
this pull request
Oct 12, 2026
… the OTLP protocol Bump the nemo-lens pin from b0f977d4 to 4d045c03 (Lens main, includes NVIDIA-NeMo/Lens#80). Until #80, nemo-lens picked its OTLP log exporter by import order instead of OTEL_EXPORTER_OTLP_PROTOCOL, unlike spans and metrics. The driver environment that `gym eval submit` renders sets one endpoint and one protocol for every signal (#4167), which is only right when the log exporter honours them: in an environment that carries the gRPC exporter package the driver built a gRPC log exporter, every batch to the collector's HTTP port failed, and at shutdown the driver sat in the exporter's retry loop until the job hit its wall time. With the new pin the log exporter is chosen from the protocol like the other two, so the shared endpoint holds whichever exporter packages the host environment installs and no per-process port probing is needed. Adapt to the rest of the pin move: mirror the new `span_attributes` no-op in Gym's fallback shim (Lens#71), and point the span test fixtures at Lens's cached default-tracer lookup (Lens#73). Signed-off-by: Ingo Guehring <iguehring@nvidia.com>
khazic
pushed a commit
to khazic/Gym-lao
that referenced
this pull request
Oct 13, 2026
… the OTLP protocol (NVIDIA-NeMo#4117) ## What does this PR do? Bumps the nemo-lens pin from `b0f977d4` to `4d045c03` (Lens main, includes NVIDIA-NeMo/Lens#80) and adapts Gym to the pin move. Until Lens#80, nemo-lens chose its OTLP **log** exporter by import order instead of by `OTEL_EXPORTER_OTLP_PROTOCOL`, unlike the span and metric exporters. The driver environment rendered by `gym eval submit` sets one endpoint and one protocol for every signal (NVIDIA-NeMo#4167 removed the last per-signal override). That is only right when the log exporter honours them. In an environment that carries the gRPC exporter package, the driver built a gRPC log exporter: every log batch to the collector's HTTP port failed with `StatusCode.UNAVAILABLE`, and at shutdown the driver sat in the exporter's retry loop until the job hit its wall time. With the new pin the log exporter follows the protocol like the other two, so the shared endpoint holds in every environment and no per-process port probing is needed. Changes beyond the pin: - `nemo_gym/telemetry/_fallbacks.py`: mirrors the new `span_attributes` no-op (Lens#71); `test_fallbacks.py` enforces parity with `nemo.lens.fallbacks`. - `tests/unit_tests/telemetry/test_endpoints.py`, `test_call_sites.py`: the span-capture fixtures patch `nemo.lens.helpers.get_tracer`, since Lens#73 resolves default tracers through its own cache rather than `opentelemetry.trace.get_tracer`. - `nemo_gym/orchestration/executors/otel.py`: drops the comment that said Gym never installs the gRPC exporter; `nemo-lens[sdk]` requires it, and the exporter choice is now made by protocol rather than by what is installed. No code change; `test_script_ships_gym_logs_by_default_and_can_switch_them_off` asserts the shared endpoint and protocol and no per-signal override. Supersedes the probing approach in NVIDIA-NeMo#4226 if this lands first: with the pin, `http/protobuf` selects the HTTP log exporter whether or not the gRPC package is installed. ## Validation - `pytest tests/unit_tests/test_orchestration_otel.py tests/unit_tests/test_slurm_script.py tests/unit_tests/telemetry`: 531 passed, 4 failed. The 4 (`test_orchestrator_is_the_only_writer_of_the_active_servers_gauge`, `test_multi_destination_waits_are_attributed_to_the_binding_limit`, two `test_memory` cases) need Linux `/proc` or a second loopback address and fail identically on main at the old pin on this macOS machine. - `pre-commit run --files ...` on the changed files: pass. - `uv lock --upgrade-package nemo-lens`: only the nemo-lens entries move in `uv.lock`; no other dependency changes. - Cluster run at Lens#80's pre-merge head (`e0f4b7f`) on 2026-10-08, oci-hsg job 7792824, scicode with 4 tasks: exit 0, job ended 36 s after the last rollout (main held its node 49 min), zero log-export errors, driver and server spans and the driver's log records all arrived at the collector over HTTP. - Cluster run at the merged pin (`4d045c03`, this PR's head) on 2026-10-12, oci-hsg job 7888521 (run `gym-job-20261012T072113Z-8f5c94`), same shape: COMPLETED with exit 0 in 9 min 7 s; the job ended 29 s after the last rollout was written; zero occurrences of `StatusCode.UNAVAILABLE`, `ConnectionResetError`, `Failed to export logs` or `Transient error` in the driver log; collector healthy with no non-debug errors; spans from every process reached the collector (orchestrator 22, policy model 422, agent 460, environment server 41, resources server 39); the driver's log records arrived over HTTP (the servers log nothing at WARNING or above in a clean run, so none were expected from them). A first attempt (job 7888331) failed the eval itself on an empty inference key and still exited 30 s after the error, with the policy model's four WARN records delivered. ## Rollouts N/A: dependency pin and telemetry test fixtures only; no agent, model or verifier behaviour changes. ## Compatibility and benchmark impact Jobs ship Gym logs from every process to the collector's HTTP port and exit promptly after the driver finishes. An explicit `driver.env.OTEL_EXPORTER_OTLP_LOGS_ENDPOINT` still wins. nemo-lens between the pins also adds observable metric kinds to its instrument registry (Lens#84) and generated semconv constants (Lens#81); Gym's uses of both pass the existing tests. No benchmark-result impact. ## Checklist - [x] I have read the [contributing guidelines](https://github.com/NVIDIA-NeMo/Gym/blob/main/CONTRIBUTING.md). - [x] The change is focused; unrelated "drive-by" edits are tracked as separate issues/PRs. - [x] Tests added or updated and pass locally. - [x] Documentation added or updated, or N/A with justification (N/A: no user-facing behaviour change; the docs already describe the shared endpoint). - [x] Pre-commit checks pass locally on the changed files. - [x] New source files include the required Apache-2.0 SPDX header (no new files). - [x] All commits have DCO sign-off (`git commit -s`). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Ingo Guehring <iguehring@nvidia.com>
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Makes
_build_log_exporterchoose gRPC or HTTP fromOTEL_EXPORTER_OTLP_LOGS_PROTOCOL/OTEL_EXPORTER_OTLP_PROTOCOL, exactly as the span and metric builders already do, instead of by which exporter package happens to import first.Today a process configured for
http/protobufstill gets a gRPC log exporter wheneveropentelemetry-exporter-otlp-proto-grpcis installed, so its logs go to the wrong port and are dropped while traces and metrics flow. NeMo Gym works around this by pointingOTEL_EXPORTER_OTLP_LOGS_ENDPOINTat the collector's gRPC port (NVIDIA-NeMo/Gym#3619), which breaks the other way as soon as the HTTP exporter is the one chosen: in Gym's Slurm jobs the HTTP log exporter now reportsTransient error ('Connection aborted.', ConnectionResetError(104, 'Connection reset by peer'))per batch. With the protocol deciding, the workaround can go (follow-up in Gym).Behaviour: signal-specific protocol wins, then the general one, default
grpc, with the existingImportErrorfallback. No change for callers that set no protocol.Issues
None filed; found while validating Gym's job collector on 2026-10-06.
Usage
OTEL_EXPORTER_OTLP_PROTOCOL=http/protobuf OTEL_EXPORTER_OTLP_ENDPOINT=http://localhost:4318 python app.py # logs, traces and metrics now all go to the HTTP endpoint; before, logs went over gRPC to it and were lostBefore your PR is "Ready for review"
Pre checks:
git commit -s) per the DCO.tests/for any behavior change.pytestandpre-commit run --all-fileslocally; both pass (465 passed; ruff and ruff-format pass on the changed files).docs/andREADME.mdwhere behavior, configuration, or public API changed (N/A: no public API or documented configuration changes; the protocol variables were already documented for the other signals)._fallbacks.pyfiles in Megatron-LM / RL / Gym (N/A).Additional information
Testing
OTEL_EXPORTER_OTLP_PROTOCOL=http/protobufselects the HTTP log exporter;OTEL_EXPORTER_OTLP_LOGS_PROTOCOLoverrides the general setting.http/protobufconfigured, a process logging a WARNING and an ERROR to a local OTLP/HTTP receiver: before this change Lens builds the gRPC log exporter and both records are lost (Failed to export logs to localhost:4318 … UNAVAILABLE); with it, the HTTP exporter delivers both.