Skip to content

feat(telemetry): name the rollout in X-Request-Id on model-server backend calls - #4750

Merged
lafrancef merged 1 commit into
mainfrom
iguehring/engine-request-id
Oct 20, 2026
Merged

lafrancef merged 1 commit into
mainfrom
iguehring/engine-request-id

Conversation

@iguehring-nvidia

@iguehring-nvidia iguehring-nvidia commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

What does this PR do?

Inside a rollout, every call a Gym model server makes to its inference backend now carries X-Request-Id: <rollout id>-<8 hex chars>, set in the shared OpenAI client wrapper (NeMoGymAsyncOpenAI._request) so every model server built on it gets it.

Why: vLLM takes its request id from X-Request-Id and records it as gen_ai.request.id on the request's span and in its request log. With the rollout id in front, an engine span or an engine log line can be matched to its rollout directly, by attribute, instead of by walking the trace from the engine span up through the Gym client span and the model-call span to the rollout span. That also keeps the match intact when a parent span is dropped. The rollout id is the same correlation key Gym already uses in the /ng-rollout/<id>/ URL prefix and on its own spans.

Behaviour:

  • Only added inside a rollout context (current_rollout_id() set). Calls made outside a rollout are unchanged.
  • A caller's own X-Request-Id header, in any case, is kept.
  • The suffix is 8 hex characters from a UUID, so calls stay distinct across server workers that cannot share a counter.
  • Backends that do not read the header ignore it.

No issue; a small follow-on to the rollout correlation work (#2647).

Validation

  • pytest tests/unit_tests/test_openai_utils.py: 299 passed, including two new tests: the header names the rollout and differs per call; it is absent outside a rollout and never overrides a caller's header.
  • pytest tests/unit_tests/telemetry tests/unit_tests/test_rollout_correlation.py: 357 passed; the 4 failures are the pre-existing macOS-only ones (test_memory, test_connection_pool, test_call_sites active-servers gauge) and fail identically on main.
  • pre-commit run --files <changed files>: passed.
  • Checked against vLLM 0.29 sources: _base_request_id reads X-Request-Id and the id reaches the llm_request span as gen_ai.request.id.

Rollout evidence

N/A for behaviour: the change adds one request header; request bodies, retries and responses are unchanged. The end-to-end effect (rollout id visible on engine spans) will be confirmed in the next collector validation run together with #4749.

Combined end-to-end run (added after opening)

Run on a Slurm cluster from a temporary branch merging #4749, #4750 and #4751 on top of main (7719379), with otel.vllm_traces: true and no hand-set flags: Nano 3.5 on vLLM 0.26.0, 2 GPUs (TP2), 16 scicode rollouts, 8 in parallel. Completed in 8 min 31 s, 16 of 16 scored, collector log clean, no token in the job directory.

Sandboxed end-to-end run (added after opening)

Terminal-bench 4 miniswe on OpenSandbox sandboxes, 8 tasks, agent running inside the sandbox and calling the policy model through the rollout-prefixed URL. vLLM's request log shows a rollout-prefixed id for all 321 requests, and gen_ai.request.id on all 321 engine spans carries it. Those engine spans are roots of their own traces, because the in-sandbox harness sends no traceparent; the request-id prefix is what ties them to the rollout, which is the case this header exists for.

…kend calls

Inside a rollout, every call a Gym model server makes to its inference
backend now carries `X-Request-Id: <rollout id>-<8 hex chars>`. vLLM takes
its request id from that header and records it as `gen_ai.request.id` on
the request's span and in its request log, so an engine span or log line
can be matched to a rollout directly, without walking the trace tree.

The header is set in the shared OpenAI client wrapper, so every model
server that uses it gets it. It is only added inside a rollout context,
and a caller's own `X-Request-Id` is kept. The random suffix keeps calls
distinct across server workers that cannot share a counter.

Signed-off-by: Ingo Guehring <iguehring@nvidia.com>
@github-actions

Copy link
Copy Markdown
Contributor

@iguehring-nvidia iguehring-nvidia added feature New capabilities, enhancements, or enablement work area:model Model servers, inference providers, and model adapters needs-review PR is ready for code review and waiting on a reviewer labels Oct 19, 2026
@iguehring-nvidia iguehring-nvidia self-assigned this Oct 19, 2026
@iguehring-nvidia

Copy link
Copy Markdown
Contributor Author

/review

@svcnvidia-nemo-ci svcnvidia-nemo-ci left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SHIP

LGTM — no reliability concerns. Reviewed all three changed files and traced rollout-context propagation and the shared HTTP transport. Header generation preserves caller-provided request IDs case-insensitively, leaves out-of-rollout calls unchanged, and does not mutate client defaults.

Validation was limited to source and test inspection; tests were not executed.

Note: formal approval is not available for this request, so this review is posted as a comment only.

Review generated by codex

@yaoyu-33 yaoyu-33 added the complexity:low Localized change in one scope with a small, straightforward review surface label Oct 19, 2026
@lafrancef
lafrancef added this pull request to the merge queue Oct 20, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA-NeMo/Gym/actions/runs/39427799824

This branch was previously deployed

1 inactive deployment
public — 79caf559 Deployed Oct 19, 2026 by copy-pr-bot[bot] via release / finalize / notify #5777
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:model Model servers, inference providers, and model adapters complexity:low Localized change in one scope with a small, straightforward review surface feature New capabilities, enhancements, or enablement work needs-review PR is ready for code review and waiting on a reviewer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants