feat(observability): persist forward pass metrics traces

[jasonqinzhou]

Overview:

Add opt-in, best-effort producer-side persistence for forward-pass metrics (FPM). Supported vLLM/SGLang relay paths and direct TensorRT-LLM/mocker publishers can write bounded rotating gzip JSONL traces without putting disk I/O on the inference path.

Details:

• Capture finalized FPM payloads before event-plane publication, with sampled and full modes, bounded nonblocking queues, producer-scoped files, structured drop reporting, and graceful shutdown draining.
• Keep one trace worker per runtime producer, share it safely across relays/components, isolate different producers, and preserve per-component source attribution and sampled state.
• Extend the shared gzip JSONL writer with awaited close, restart-safe and collision-safe segment allocation, exact-prefix retention, once-per-segment pruning, and symlink-safe file creation. Audit and request tracing retain unlimited segments by default.
• Expose --fpm-trace/--no-fpm-trace through the shared runtime argument system (backed by DYN_FPM_TRACE), and auto-enable backend FPM only for vLLM/SGLang topologies that actually create a Dynamo relay. Legacy explicit-port activation keeps precedence; unified, snapshot, embedding, headless, and other unsupported paths warn instead of silently producing no trace.
• Validate all DYN_FPM_* settings and document configuration, topology support, record shape, sizing, retention, shutdown, shared-volume I/O, and Kubernetes storage.

Where should the reviewer start?

1. lib/llm/src/fpm_trace/mod.rs and lib/llm/src/fpm_trace/sink.rs for ownership, queueing, sampling, and shutdown.
2. lib/llm/src/telemetry/jsonl_gz.rs for segment allocation, retention, and awaited close.
3. components/src/dynamo/common/configuration/groups/runtime_args.py, components/src/dynamo/vllm/args.py, and components/src/dynamo/sglang/args.py for CLI/env resolution and topology-aware activation.
4. docs/observability/forward-pass-metrics-tracing.md for the operator contract and limitations.

Related Issues

🚫 This PR is NOT linked to an issue:

• Confirmed — no related issue

Validation

• cargo test --offline -p dynamo-llm --lib --no-default-features: 1,325 passed, 0 failed, 3 ignored.
• cargo clippy --offline -p dynamo-llm --lib --tests --no-default-features -- -D warnings.
• Focused FPM trace tests: 25 passed.
• Focused gzip JSONL tests: 14 passed.
• Focused FPM publisher tests: 9 passed.
• Common runtime argument, configuration utility, and environment tests: 42 passed.
• Black 23.1.0, isort 5.12.0, Ruff 0.5.2, Python compile checks, codespell, docs YAML parsing, cargo fmt --all -- --check, and git diff --check.

The backend-specific vLLM/SGLang suites were not collected locally because those optional backend packages are not installed on this host. A production GPU acceptance soak was not run; PR CI remains the authority for those environments.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[tedzhouhk]

One additional lifecycle issue found while reviewing the persistence path.

[devin-ai-integration]

Devin Review found 2 potential issues.

[coderabbitai]

Walkthrough

Adds opt-in forward-pass-metrics (FPM) trace capture: a Python env parser (DYN_FPM_TRACE), a Rust fpm_trace module (config, telemetry bus, sink with sampled/full write modes), tap points in publisher paths, gzip segment retention across sinks, backend wiring in vLLM/sglang, and new documentation.

Changes

Forward Pass Metrics Tracing

Layer / File(s)	Summary
Env parsing components/src/dynamo/common/utils/env.py, .../tests/test_env.py	Adds fpm_trace_enabled() parsing DYN_FPM_TRACE with warn-once invalid-value handling; tests cover truthy/falsy/invalid cases.
sglang activation components/src/dynamo/sglang/args.py, .../tests/test_sglang_unit.py	parse_args enables forward-pass metrics via port env var or fpm_trace_enabled(), logging the source; tests cover both paths and invalid values.
vLLM activation components/src/dynamo/vllm/envs.py, args.py, main.py, tests/test_vllm_unit.py	Adds forward_pass_metrics_enabled() and DEFAULT_FORWARDPASS_METRIC_PORT; scheduler injection and relay setup now gate on this helper; tests cover injection, port defaults, invalid values, and relay creation.
Rust config module lib/runtime/src/config/environment_names.rs, lib/llm/src/fpm_trace/config.rs	Adds env var name constants and FpmTracePolicy/FpmTraceMode with lazy env-driven loading and validation, with tests.
Telemetry bus and module wiring lib/llm/src/fpm_trace/mod.rs, lib/llm/src/lib.rs	Adds FpmTraceEvent, bounded telemetry bus, init_from_env_with_shutdown, publish_payload, is_active, subscribe; registers new crate module.
Sink worker (full/sampled) lib/llm/src/fpm_trace/sink.rs	Decodes/validates msgpack payloads, sanitizes producer paths, preflights writability, spawns single-shot worker with full and sampled emission modes, with extensive tests.
Publisher tap points lib/llm/src/fpm_publisher.rs	Initializes tracing on relay/direct publisher construction and taps payloads before publishing via publish_bytes_ref, with tests.
Gzip segment retention lib/llm/src/telemetry/jsonl_gz.rs, lib/llm/src/audit/sink.rs, lib/llm/src/request_trace/sink.rs	Adds max_segments option, refactors writer for shutdown-aware sending/close, rewrites segment index discovery/pruning, updates dependent sinks and tests.
Documentation docs/index.yml, docs/observability/README.md, docs/observability/forward-pass-metrics-tracing.md	Adds navigation entry, README row, and a new detailed guide on configuration, capture modes, trace format, and operational guidance.

Estimated code review effort: 4 (Complex) | ~60 minutes

Related PRs: None specified.

Suggested labels: observability, feature, rust, python, documentation

Suggested reviewers: None specified.

🐇 A trace of hops, gzip-tucked and light,
Sampled or full, we log through the night,
Ports and flags decide what we see,
Segments prune old, keeping paths free,
Metrics now whisper where workers take flight.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title is concise and accurately summarizes the main change: persisting forward pass metrics traces.
Description check	✅ Passed	The description follows the template well, with overview, details, reviewer guidance, and a completed related-issues section.

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (2)

components/src/dynamo/sglang/tests/test_sglang_unit.py (1)

469-496: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a precedence test for explicit port + trace both set.

The vLLM test suite covers the "explicit port wins when trace is enabled" case (test_explicit_port_wins_when_trace_is_enabled); the sglang suite lacks a matching test even though parse_args implements the same if explicit_fpm_port ... elif fpm_trace_enabled() precedence.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@components/src/dynamo/sglang/tests/test_sglang_unit.py` around lines 469 -
496, The sglang tests are missing coverage for the precedence path where both an
explicit forward-pass metrics port and DYN_FPM_TRACE are set. Add a new async
test alongside the existing parse_args tests in test_sglang_unit.py that sets
DYN_FORWARDPASS_METRIC_PORT and DYN_FPM_TRACE together, invokes mock_sglang_cli
and parse_args, and asserts server_args.enable_forward_pass_metrics remains
enabled because the explicit port should win. Use the existing parse_args,
mock_sglang_cli, and server_args.enable_forward_pass_metrics symbols to keep the
test aligned with the current precedence logic.

lib/llm/src/fpm_trace/config.rs (1)

60-86: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicate numeric-parsing helpers.

positive_u64_from_env and positive_usize_from_env are identical apart from the numeric type. Consider collapsing into one generic helper.

♻️ Proposed generic helper

-fn positive_u64_from_env(name: &str, default: u64) -> anyhow::Result<u64> {
-    let Some(value) = std::env::var(name).ok() else {
-        return Ok(default);
-    };
-    let parsed = value
-        .trim()
-        .parse::<u64>()
-        .map_err(|_| anyhow::anyhow!("{name} must be a positive integer"))?;
-    if parsed == 0 {
-        anyhow::bail!("{name} must be greater than zero");
-    }
-    Ok(parsed)
-}
-
-fn positive_usize_from_env(name: &str, default: usize) -> anyhow::Result<usize> {
-    let Some(value) = std::env::var(name).ok() else {
-        return Ok(default);
-    };
-    let parsed = value
-        .trim()
-        .parse::<usize>()
-        .map_err(|_| anyhow::anyhow!("{name} must be a positive integer"))?;
-    if parsed == 0 {
-        anyhow::bail!("{name} must be greater than zero");
-    }
-    Ok(parsed)
-}
+fn positive_from_env<T>(name: &str, default: T) -> anyhow::Result<T>
+where
+    T: std::str::FromStr + Copy + PartialEq + Default,
+{
+    let Some(value) = std::env::var(name).ok() else {
+        return Ok(default);
+    };
+    let parsed = value
+        .trim()
+        .parse::<T>()
+        .map_err(|_| anyhow::anyhow!("{name} must be a positive integer"))?;
+    if parsed == T::default() {
+        anyhow::bail!("{name} must be greater than zero");
+    }
+    Ok(parsed)
+}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/llm/src/fpm_trace/config.rs` around lines 60 - 86, The numeric env
parsing logic is duplicated in positive_u64_from_env and
positive_usize_from_env, differing only by the target type. Refactor them into a
single generic helper in config.rs that parses any positive integer type and
reuse it from the existing callers, keeping the same default handling,
trim/parse flow, and zero-check behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@components/src/dynamo/sglang/tests/test_sglang_unit.py`:
- Around line 499-515: Move the dynamo.common.utils.env import out of
test_invalid_fpm_trace_warns_and_does_not_enable_metrics and place it with the
other top-level imports in test_sglang_unit.py; then update the test to use the
top-level common_env symbol for monkeypatching
_fpm_trace_invalid_warning_emitted and for
caplog.at_level(logger=common_env.__name__) without any in-function imports.

In `@components/src/dynamo/vllm/tests/test_vllm_unit.py`:
- Around line 1233-1262: The test `test_trace_only_starts_relay_on_default_port`
is doing in-function imports of `dynamo.llm` and `dynamo.vllm.main`, which
violates the import guideline. Move those imports to the top of the test file
and keep the test body focused on patching `FpmEventRelay`,
`get_dp_range_for_worker`, and `setup_fpm_relay` behavior using the
already-imported modules.
- Around line 1196-1216: The two vLLM tests still import
dynamo.common.utils.env, dynamo.vllm.main, and dynamo.llm inside the test
bodies, but this file expects imports at module scope. Hoist those imports to
the top of the test module near the other imports, and only keep them local if
there is a deliberate import-side-effect isolation reason; update
test_invalid_trace_warns_once_and_does_not_enable_fpm and
test_trace_only_starts_relay_on_default_port to use the module-level symbols.

---

Nitpick comments:
In `@components/src/dynamo/sglang/tests/test_sglang_unit.py`:
- Around line 469-496: The sglang tests are missing coverage for the precedence
path where both an explicit forward-pass metrics port and DYN_FPM_TRACE are set.
Add a new async test alongside the existing parse_args tests in
test_sglang_unit.py that sets DYN_FORWARDPASS_METRIC_PORT and DYN_FPM_TRACE
together, invokes mock_sglang_cli and parse_args, and asserts
server_args.enable_forward_pass_metrics remains enabled because the explicit
port should win. Use the existing parse_args, mock_sglang_cli, and
server_args.enable_forward_pass_metrics symbols to keep the test aligned with
the current precedence logic.

In `@lib/llm/src/fpm_trace/config.rs`:
- Around line 60-86: The numeric env parsing logic is duplicated in
positive_u64_from_env and positive_usize_from_env, differing only by the target
type. Refactor them into a single generic helper in config.rs that parses any
positive integer type and reuse it from the existing callers, keeping the same
default handling, trim/parse flow, and zero-check behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: bf455de6-1239-49a2-9479-5598aa8eda2d

📥 Commits

Reviewing files that changed from the base of the PR and between ec45adf and 0a021ed.

📒 Files selected for processing (20)

• components/src/dynamo/common/utils/env.py
• components/src/dynamo/common/utils/tests/test_env.py
• components/src/dynamo/sglang/args.py
• components/src/dynamo/sglang/tests/test_sglang_unit.py
• components/src/dynamo/vllm/args.py
• components/src/dynamo/vllm/envs.py
• components/src/dynamo/vllm/main.py
• components/src/dynamo/vllm/tests/test_vllm_unit.py
• docs/index.yml
• docs/observability/README.md
• docs/observability/forward-pass-metrics-tracing.md
• lib/llm/src/audit/sink.rs
• lib/llm/src/fpm_publisher.rs
• lib/llm/src/fpm_trace/config.rs
• lib/llm/src/fpm_trace/mod.rs
• lib/llm/src/fpm_trace/sink.rs
• lib/llm/src/lib.rs
• lib/llm/src/request_trace/sink.rs
• lib/llm/src/telemetry/jsonl_gz.rs
• lib/runtime/src/config/environment_names.rs

[jasonqinzhou]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[jasonqinzhou]

@coderabbitai resolve

[coderabbitai]

✅ Action performed

Comments resolved. Approval is disabled; enable reviews.request_changes_workflow to allow explicit top-level @coderabbitai resolve or @coderabbitai approve commands.

[sachalmalick]

/ok to test 7065d41

[copy-pr-bot]

/ok to test 7065d41

@sachalmalick, there was an error processing your request: E2

See the following link for more information: https://docs.gha-runners.nvidia.com/cpr/e/2/

[sachalmalick]

/ok to test 6683334

[github-actions]

🌿 Fern Docs Preview: https://nvidia-preview-019f2983-4333-70c1-93bd-1de4e02ba299.docs.buildwithfern.com/dynamo/dev