Repository navigation
[Rust Frontend] Add per-request timing metrics to chat and completions - #59314
Merged
njhill merged 6 commits intoOct 12, 2026
Merged
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Member
|
[AUTOMATED] @codex review |
xinnywinne
force-pushed
the
feat/rust-per-request-metrics
branch
from
October 5, 2026 22:51
615309a to
a3f8502
Compare
njhill
reviewed
Oct 7, 2026
njhill
left a comment
njhill
left a comment
Member
There was a problem hiding this comment.
Formulas and the --disable-log-stats validation match the Python implementation. A few structural comments inline.
Separately: could you add e2e coverage that runs against the Rust frontend in .buildkite/test_areas/rust_frontend.yaml (as #54883 did with test_include_reasoning.py), checking metrics on a non-streaming response and on the final usage chunk? That keeps the two frontends' wire output from drifting.
xinnywinne
requested review from
AndreasKaratzas,
DarkLight1337,
NickLucche,
aarnphm and
robertgshaw2-redhat
as code owners
October 8, 2026 18:54
Contributor
Author
|
Addressed the review in 50e8eca7. I also added API-level coverage to |
BugenZhao
approved these changes
Oct 9, 2026
BugenZhao
left a comment
BugenZhao
left a comment
Member
There was a problem hiding this comment.
Rest LGTM. Thanks @xinnywinne! I'll push a few minor simplifications.
|
✅ @xinnywinne, CI is now available for this PR.
|
Member
|
/ci run |
|
✅ Triggered Buildkite CI #93851 for commit |
BugenZhao
enabled auto-merge (squash)
October 9, 2026 13:28
Contributor
|
This pull request has merge conflicts that must be resolved before it can be |
njhill
approved these changes
Oct 9, 2026
njhill
left a comment
njhill
left a comment
Member
There was a problem hiding this comment.
Thanks @xinnywinne, please rebase again!
auto-merge was automatically disabled
October 9, 2026 21:49
Head branch was pushed to by a user without write access
xinnywinne
force-pushed
the
feat/rust-per-request-metrics
branch
from
October 9, 2026 21:49
60b6ba8 to
40b65b1
Compare
Contributor
Author
|
/ci run |
1 similar comment
Member
|
/ci run |
|
✅ Triggered Buildkite CI #93926 for commit |
Member
|
There are some test failures which look probably related. |
Contributor
|
This pull request has merge conflicts that must be resolved before it can be |
Collaborator
CI selector (shadow): no narrower answer, every step runs: 195 test steps (271 jobs) instead of 9 (9 jobs)Shadow mode: this changes nothing about what CI runs. It shows what the evidence-based selector would pick for this PR, next to today's rules. How it works. Feedback welcome: reply here if it would skip a step this change needs, or runs something unrelated.
Why: rust: rust/src/chat/src/event.rs: root crate bucket; 16 gate-env steps + 12 hardware-image steps; + the PyO3 bridge claim (vllm/tool_parsers/rust_tool_parser.py) Selector would run (195)
Would skip (today's rules run them) (0)none Would add (today's rules do not run them) (186)
AMD mirrors: would skip (0)none AMD mirrors: would add (156)
28 changed files · base |
Assisted-by: OpenAI Codex Signed-off-by: wxin <wxin@nvidia.com>
Assisted-by: OpenAI Codex <noreply@openai.com> Signed-off-by: wxin <wxin@nvidia.com>
Assisted-by: OpenAI Codex <noreply@openai.com> Signed-off-by: wxin <wxin@nvidia.com>
Replace the hand-written stream metrics serializer with a `StreamPerRequestMetrics` type using `skip_serializing_none`, converted from `PerRequestMetrics` by exhaustive destructuring so new fields cannot be missed. Pass metrics into `usage_chunk` directly, and inline the `per_request_metrics` helper as `enabled.then(|| ...)` at call sites. Also tidy import grouping and method order. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: wxin <wxin@nvidia.com>
Frontend text decoding can terminate a request after matching a stop string before the engine emits a terminal output. Carry the current request timestamp snapshot on every generated output so that locally terminated requests can report valid metrics without panicking. Assisted-by: OpenAI Codex <noreply@openai.com> Signed-off-by: wxin <wxin@nvidia.com>
xinnywinne
force-pushed
the
feat/rust-per-request-metrics
branch
from
October 12, 2026 21:19
40b65b1 to
9b53d5d
Compare
Contributor
Author
|
The reported CI failures were related. The Rust decoder can complete a request locally when it matches a stop string, before the engine sends a terminal output; the terminal-only timestamp invariant therefore caused the panic. I rebased onto current |
Contributor
Author
|
/ci run |
|
✅ Triggered Buildkite CI #94320 for commit |
github-project-automation
Bot
moved this from Ready
to Done
in gpt-oss Issues & Enhancements
6 tasks done
AndreasKaratzas
pushed a commit
to AndreasKaratzas/vllm
that referenced
this pull request
Oct 16, 2026
vllm-project#59314) Signed-off-by: wxin <wxin@nvidia.com> Signed-off-by: Bugen Zhao <i@bugenzhao.com> Co-authored-by: Bugen Zhao <i@bugenzhao.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Andreas Karatzas <AndreasKaratzas@users.noreply.github.com>
michael-gpu
pushed a commit
to practical-inference/vllm-ampere-extended
that referenced
this pull request
Oct 20, 2026
vllm-project#59314) Signed-off-by: wxin <wxin@nvidia.com> Signed-off-by: Bugen Zhao <i@bugenzhao.com> Co-authored-by: Bugen Zhao <i@bugenzhao.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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.
Implements the per-request
metrics(--enable-per-request-metrics) roadmap item in #44280. Speculative-decoding acceptance stats remain a separate checklist item.Summary
--enable-per-request-metricsin the Rust frontend and reject it alongside--disable-log-stats./v1/chat/completionsand/v1/completionsresponses. For streaming, attach them to the final usage chunk whenstream_options.include_usageis enabled.nullin non-streaming responses and omit them from streaming usage chunks, matching Python's recursiveexclude_none=Truebehavior.Implementation detail
Each engine output carries the current request-timestamp snapshot. This is required because the Rust text decoder can terminate a request locally after matching a stop string, before the engine emits a terminal output. Keeping the snapshot with the current output lets that path report valid metrics; limiting timestamps to engine-terminal outputs caused the Rust frontend to panic in Buildkite CI. The snapshot is four
f64values and does not add GPU synchronization.Scope
Speculative-decoding acceptance metrics are intentionally not included in this PR.
GenerateOutputalready carries terminalspec_decode_metrics, but mapping those values into the OpenAIPerRequestMetrics.speculative_decodingresponse field is separate follow-up work.Duplicate-work check
Tests
cargo fmt --all— passed.cargo test -p vllm-llm -p vllm-text -p vllm-chat -p vllm-server -p vllm-cmd --quiet -- --test-threads=1— passed (1,019 tests passed, 1 ignored). An earlier parallel rerun intermittently missed an unrelatedpreprocess_audio_secssnapshot key invllm-chat; that test passed in isolation and in the serial suite.cargo check --workspace --tests— passed after the review updates.main:cargo test -p vllm-textpassed (105 passed, 1 network test ignored),cargo test -p vllm-llm --test generatepassed (12 passed), andcargo test -p vllm-chat --libpassed (355 passed).vllm-servermetric, completion, and streaming tests — passed.cargo clippy -p vllm-llm -p vllm-text -p vllm-server --all-targets -- -D warnings— passed after the rebase and stop-string fix.test_chat_completion.py— passed. The existing Rust frontend Buildkite step runs this file and now covers non-streaming metrics and the final streaming usage chunk.HuggingFaceTB/SmolLM2-135M-Instructand--enable-per-request-metrics:finish_reason: "stop", valid metrics, and left the server healthy;This validates response behavior and the CI reproducer, not model accuracy or performance.
AI assistance
AI assistance was used to implement and test this change. The human submitter reviewed and tested the original implementation and will review the latest reviewer-driven update before merge.