Skip to content

feat(trtllm): publish Dynamo engine metrics by default - #421

Merged
nv-yna merged 5 commits into
NVIDIA:mainfrom
nv-yna:yna/dynamo-publish-metrics-default
Sep 12, 2026
Merged

nv-yna merged 5 commits into
NVIDIA:mainfrom
nv-yna:yna/dynamo-publish-metrics-default

Conversation

@nv-yna

@nv-yna nv-yna commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

Summary

Enable TRT-LLM engine metric publication by default for workers launched through python -m dynamo.trtllm, independently of observability.enabled.

  • Add backend.publish_metrics: true by default, passing the dedicated --publish-metrics flag for prefill, decode, and aggregated workers.
  • Respect explicit backend.publish_events_and_metrics: false as a master opt-out of both publication flags, regardless of publish_metrics or observability.
  • Distinguish omitted/null from explicit false, including schema and YAML round-trips. When the combined setting is omitted/null, preserve observability.enabled as the existing metrics-and-events superset. With both settings enabled, both positive flags are passed.
  • Recognize metrics-only publication when advertising worker metrics URLs to built-in AIPerf and custom benchmarks. Preserve KVBM endpoints and explicit URL overrides.
  • Show the selected flags in dry-run output and correct the warning that previously treated the combined flag as the only way to publish metrics.
  • Leave native trtllm_serve, sidecar commands, and existing sidecar URL behavior unchanged.

Default behavior

Recipe Worker publication flags
Combined setting omitted/null; observability absent or disabled --publish-metrics
Combined setting omitted/null; observability.enabled: true --publish-metrics --publish-events-and-metrics
Explicit backend.publish_events_and_metrics: false; any observability setting none
Explicit backend.publish_events_and_metrics: true; any observability setting --publish-metrics --publish-events-and-metrics

The standalone flag does not enable KV-cache events. Explicit environment settings remain under the user's control.

Compatibility

Requires a Dynamo build containing ai-dynamo/dynamo#12162 or equivalent standalone-flag support. Older builds, including v1.4.2, reject --publish-metrics.

For older workers, backend.publish_metrics: false omits only the new flag; it does not emit the also-unsupported negative flag or silently fall back to enabling KV events. Under observability, the existing combined flag can still be enabled. Explicit backend.publish_events_and_metrics: false omits both flags and suppresses ordinary Dynamo/TRT-LLM worker scrape URLs. This does not override user-supplied publication environment variables.

Metrics-only publication performs engine telemetry work; this PR does not claim zero overhead or alter the engine's profiling configuration.

Validation

  • uv run pytest tests/ -q: 2109 passed, 2 skipped, 6 deselected.
  • Focused worker/schema/observability/benchmark tests: 294 passed; dry-run tests: 50 passed. Includes explicit-false precedence, all worker modes, both YAML loaders, and omitted-to-null serialization followed by observability expansion.
  • uv run ruff check src/srtctl/: passed.
  • git diff --check: passed.
  • Configured pre-commit formatting passed for the changed source and dry-run/worker/observability tests. The pinned Ruff hook reports an existing UP038 at the unchanged sidecar validation in schema.py, also present on upstream main. test_benchmarks.py retains its pre-existing 19 SIM117 findings and two formatting differences; no unrelated cleanup included.

Draft pending live validation with a compatible Dynamo runtime. No GPU jobs were launched and no existing benchmark runs or shared run artifacts were modified.

Signed-off-by: Yuewei Na <nv-yna@users.noreply.github.com>
@codecov-commenter

codecov-commenter commented Sep 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@3be8c79). Learn more about missing BASE report.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #421   +/-   ##
=======================================
  Coverage        ?   75.14%           
=======================================
  Files           ?      109           
  Lines           ?    16042           
  Branches        ?        0           
=======================================
  Hits            ?    12055           
  Misses          ?     3987           
  Partials        ?        0           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

nv-yna and others added 3 commits September 11, 2026 00:45
Signed-off-by: Yuewei Na <nv-yna@users.noreply.github.com>
Signed-off-by: Yuewei Na <nv-yna@users.noreply.github.com>
@nv-yna
nv-yna marked this pull request as ready for review September 11, 2026 23:58
Signed-off-by: Yuewei Na <nv-yna@users.noreply.github.com>
@nv-yna
nv-yna merged commit be5ab92 into NVIDIA:main Sep 12, 2026
6 checks passed
ishandhanani added a commit that referenced this pull request Sep 13, 2026
Nine conflicting files, resolved by hand:

- The process exporter (#413) and the wider node-exporter collector set (#415)
  arrive as tachometer-stage exporter launches; on this branch the exporters
  are services. process-exporter is now a service kind (`type:
  process-exporter`, implied while tachometer runs, `placement.node: all`),
  host-native from the `configs/process-exporter` binary `make setup`
  installs (no container, no mounts), skipped with a warning when the binary
  is missing, container launch when a recipe declares a `container`. The
  group file is written by the kind's `prepare` hook; `ServiceKind` gains
  `host_native`, `prepare`, and `skip_reason`. node-exporter's built command
  carries the stat/vmstat/pressure/meminfo_numa/processes collectors and the
  widened vmstat field filter. The templates, the group YAML, and the host
  binary resolver move to `srtctl.services.exporters`; their tests follow.
- Load-window tachometer (#359): start/stop inside run_benchmark is kept;
  `stop_tachometer` now terminates through `ManagedProcess.terminate`, which
  signals the Slurm step (SIGTERM to the srun client would abort the step and
  SIGKILL the scraper). The scraper's `terminate_timeout` is the recipe's
  `shutdown_grace_secs`; the 90 s module constant is gone.
- Ingest timestamp fallback (#414) is taken from main wholesale (it also
  offers `--start-ns`); this branch's own fallback is dropped, its dedup
  module and docstring edits re-applied.
- CPU power telemetry (#410, #422): taken as is (power is frozen); the
  energy report runs before the S3 upload, which returns the URL only.
- Makefile: the `examples`/`golden-check` targets plus the cpu-power targets;
  the `recipes/`-based runner targets stay deleted. SUMMARY: both new pages,
  `analyzing.md` stays deleted. submit.py: the direct-host renderer import
  stays deleted; the arch helpers the cpu-power preflight uses are kept.
  telemetry.py: IPv6-safe host and the SGLang gateway metrics port together.

2355 tests on Python 3.10 and 3.13, lint, schema docs, 21 examples validated,
golden 574 identical / 0 mismatched.
nv-yna added a commit that referenced this pull request Sep 14, 2026
…ine config (#446)

srtctl passes --publish-metrics to Dynamo TRT-LLM workers by default (#421),
and dynamo.trtllm turns that flag into enable_iter_perf_stats: true in the
engine arguments, so every worker paid TensorRT-LLM's per-iteration
statistics cost (KV-cache stats and CUDA-event step timing on every executor
loop) for gauges no srt-slurm benchmark reads. The engine YAML is merged over
the derived arguments and wins on conflicts, so an explicit false is what
turns the statistics off while keeping the request-level trtllm_* series,
which only need the per-request perf metrics.

- New expand_trtllm_engine_defaults() bakes enable_iter_perf_stats: false
  into every trtllm_config section a recipe uses (prefill/decode or
  aggregated, created when absent), for both frontend.type: dynamo and
  trtllm_serve. setdefault semantics: an explicit true in the recipe wins.
- expand_observability() now creates the sections for the modes in use
  instead of only touching existing ones, so its true still wins for a
  recipe with no engine yaml; it warns when it meets an explicit false,
  which is also what a saved or locked recipe carries.
- expand_engine_config_defaults() runs the three engine-config expansions
  in order; load_config, SrtConfig.from_yaml and the override submit path
  all use it, so srtctl dry-run and the in-job loader agree.
- expand_trtllm_serve_defaults() shares the new _setdefault_trtllm_engine_keys
  helper; a non-mapping trtllm_config is left for schema validation instead
  of being replaced.
- srtctl dry-run shows a "TRT-LLM Engine Statistics" panel with the per-role
  enable_iter_perf_stats / return_perf_metrics values for every TRT-LLM
  backend.
- Docs: config-reference "Iteration statistics default" and the trtllm_serve
  worker-metrics note; regenerated schema-reference row for publish_metrics.

Signed-off-by: Yuewei Na <nv-yna@users.noreply.github.com>
Co-authored-by: Yuewei Na <nv-yna@users.noreply.github.com>
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants