Skip to content

[PENDING: #694] test: pin reported latency metrics to injected values - #734

Open
Bslabe123 wants to merge 2 commits into
kubernetes-sigs:mainfrom
Bslabe123:test/metric-accuracy-latency-726
Open

[PENDING: #694] test: pin reported latency metrics to injected values#734
Bslabe123 wants to merge 2 commits into
kubernetes-sigs:mainfrom
Bslabe123:test/metric-accuracy-latency-726

Conversation

@Bslabe123

@Bslabe123 Bslabe123 commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Part of #726. Row in #606: Integration / "Metric accuracy gate, latency (#726)".

The gap

inference-perf reports four latency metrics per request: time to first token (TTFT), inter-token latency (ITL, the gap between streamed chunks), time per output token (TPOT) and request latency. The only test that checks these numbers against a known input is e2e/tests/test_metrics_fidelity.py (#614), which runs in the e2e tier against llm-d-inference-sim with fixed TTFT and inter-token latency, so it needs the sim binary and is not in the per-PR unit suite. Everything in tests/required either hands the latency math pre-made timestamps or only checks that a metric is present and plausible.

That is how #564 happened: #410 changed how streamed chunks were tokenized, ITL was under-reported, and nothing caught it for about two months.

What this adds

One new file, tests/required/integration/test_latency_metric_accuracy.py, 7 tests. Each:

  1. Starts a fake OpenAI-compatible server (FakeOpenAIServer from fix: anchor TTFT to the first reasoning-channel token #694) that streams five word-chunks over real HTTP with scripted pauses: 250 ms to first token, then gaps of 400, 100, 400, 100 ms.
  2. Runs a normal inference-perf request against it through the real client, streaming parser, metric collector and report generator. Only the model server is fake.
  3. Reads TTFT, ITL, TPOT and request latency from the generated report and checks each against the injected pauses.

The reference values are the fake server's own per-chunk send timestamps. Server and client share one process and clock, so each reported number is compared to when the bytes actually left, not to how long asyncio.sleep was asked for. Tolerances (50 ms delivery, 100 ms scheduling) are documented in the file; 100 vs 400 ms gaps stay separable.

The seven checks:

  • the fake server really pauses (so nothing below passes by accident)
  • TTFT matches the 250 ms first-token delay
  • changing the first-token delay moves reported TTFT by the same amount
  • ITL min, max, mean and median match the 400/100 ms gaps
  • TPOT excludes the first-token wait and is below normalized TPOT
  • request latency is bounded both ways by the injected total
  • the re-tokenization branch where [bug] Incorrect output length for neuralmagic/Meta-Llama-3.1-8B-Instruct-FP8 #564 lived does not halve ITL or emit zero-length intervals

Compared with #614: runs in tests/required on every PR with no external binary; the reference is the send timestamp, not the configured sleep, so each value is bounded on both sides; the gaps are uneven (400/100 ms) so ITL min, max, mean and median are each checked, where #614 uses a constant gap and asserts only the median; and the report-time re-tokenization branch is exercised directly rather than sidestepped with use_server_output_tokens. The tokenizer here maps one word to one token, so the multi-token-per-chunk case #614 notes (zero-length gaps dragging the ITL mean down) is covered by neither test.

Breaking summarize_requests locally four ways (the #564 re-tokenization, TTFT anchored to end of stream, first-token wait folded into TPOT, flat average as ITL) fails the matching test each time. Product code is unchanged.

Status

Stacked on #694 (base 3b17a21), which supplies fake_openai_server.py. Once #694 merges the diff shrinks to this file and the [PENDING] prefix comes off.

Verified locally on #694: 7 new tests pass, pdm run validate clean, unit suite 892 passed / 15 skipped. About 10 seconds, almost all injected sleep.

Notes: the #606 row says this "requires giving MockModelServerClient a controllable response profile"; that mock is unary-only with no chunk timeline and would bypass the HTTP client and parser, so the row should read "uses the FakeOpenAIServer fake from #694". Coordinated-omission correction (adjusting latency by schedule_delay) is out of scope: a metric-definition question, open on #726 after v0.7.0.

@kubernetes-prow kubernetes-prow Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 17, 2026
@kubernetes-prow

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Bslabe123

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubernetes-prow kubernetes-prow Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 17, 2026
@github-actions github-actions Bot added the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label Aug 17, 2026
@kubernetes-prow kubernetes-prow Bot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 17, 2026
@kubernetes-prow kubernetes-prow Bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Aug 17, 2026
@github-actions github-actions Bot removed the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label Aug 17, 2026
@Bslabe123 Bslabe123 mentioned this pull request Aug 17, 2026
64 tasks
@Bslabe123 Bslabe123 changed the title [WIP] [PENDING: #694] test: pin reported latency metrics to injected values [PENDING: #694] test: pin reported latency metrics to injected values Aug 18, 2026
@Bslabe123
Bslabe123 force-pushed the test/metric-accuracy-latency-726 branch from 96c55ad to 6fda061 Compare August 18, 2026 18:19
@github-actions github-actions Bot added do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. and removed do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. labels Aug 18, 2026
@Bslabe123
Bslabe123 marked this pull request as ready for review August 18, 2026 20:12
@kubernetes-prow kubernetes-prow Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 18, 2026
@kubernetes-prow
kubernetes-prow Bot requested a review from Jeffwan August 18, 2026 20:12
@Bslabe123
Bslabe123 force-pushed the test/metric-accuracy-latency-726 branch from 6fda061 to 132771f Compare August 19, 2026 20:15
@github-actions github-actions Bot added do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. and removed do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. labels Aug 19, 2026
@Bslabe123 Bslabe123 added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Aug 20, 2026
@Bslabe123
Bslabe123 force-pushed the test/metric-accuracy-latency-726 branch from 132771f to 1f983e2 Compare August 26, 2026 15:50
@github-actions github-actions Bot added do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. and removed do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. labels Aug 26, 2026
@Bslabe123
Bslabe123 force-pushed the test/metric-accuracy-latency-726 branch from 1f983e2 to 735cc72 Compare August 26, 2026 17:48
@github-actions github-actions Bot added do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. and removed do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. labels Aug 26, 2026
@kubernetes-prow kubernetes-prow Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 27, 2026
@Bslabe123
Bslabe123 force-pushed the test/metric-accuracy-latency-726 branch from 735cc72 to a0cf90a Compare August 27, 2026 20:14
@kubernetes-prow kubernetes-prow Bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 27, 2026
@github-actions github-actions Bot added do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. and removed do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. labels Aug 27, 2026
@Bslabe123 Bslabe123 added priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. and removed priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels Sep 3, 2026
…s#559)

Reasoning models stream delta.reasoning_content (or delta.reasoning)
before, and when the output budget is exhausted instead of, any
delta.content chunk. Only content chunks were timestamped, so
time_to_first_token measured time-to-first-content (prefill plus the
entire reasoning-decode phase) and reported null when the stream ended
while still in the reasoning channel.

parse_sse_stream now takes an optional extract_reasoning extractor and
returns a named ParsedSSEStream instead of a positional 5-tuple;
reasoning-bearing chunks are recorded in new reasoning_chunks and
reasoning_chunk_times fields on StreamedResponseMetrics. TTFT anchors
to the first generated token of either channel, matching server-side
vllm:time_to_first_token. output_len, TPOT, and ITL stay content-based:
reasoning is thinking, not user-facing output. The token-count mismatch
accounting now includes reasoning tokens, since the server's
completion_tokens counts them.

Adds the first kubernetes-sigs#606 Integration-tier tests: an in-process fake OpenAI
server (tests/required/integration/fake_openai_server.py) streams
scripted reasoning/content sequences over real HTTP with recorded send
timestamps, and the reported TTFT is asserted against the server's own
timeline. Unit tests pin the TTFT arithmetic with synthetic timestamps
and the content-only output_len semantics end to end.
@Bslabe123
Bslabe123 force-pushed the test/metric-accuracy-latency-726 branch from a0cf90a to 054bf3f Compare September 3, 2026 20:30
@github-actions github-actions Bot added do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. and removed do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. labels Sep 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant