test: catch SGLang and TGI metric-name drift - #737
Open
Bslabe123 wants to merge 3 commits into
Open
Conversation
kubernetes-prow
Bot
added
the
do-not-merge/work-in-progress
|
[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 DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
github-actions
Bot
added
the
do-not-merge
kubernetes-prow
Bot
added
cncf-cla: yes
github-actions
Bot
removed
the
do-not-merge
Bslabe123
changed the title
Bslabe123
marked this pull request as ready for review
August 17, 2026 21:18
kubernetes-prow
Bot
removed
the
do-not-merge/work-in-progress
Bslabe123
force-pushed
the
test/metric-drift-sglang-tgi-669
branch
from
August 18, 2026 18:19
243b97f to
9d95a15
Compare
github-actions
Bot
added
do-not-merge
This was referenced Aug 18, 2026
Closed
Bslabe123
force-pushed
the
test/metric-drift-sglang-tgi-669
branch
from
August 19, 2026 20:16
9d95a15 to
2c1c308
Compare
github-actions
Bot
added
do-not-merge
Bslabe123
added
the
priority/important-longterm
This was referenced Aug 20, 2026
Bslabe123
changed the title
kubernetes-prow
Bot
added
the
do-not-merge/work-in-progress
github-actions
Bot
added
the
do-not-merge
Bslabe123
marked this pull request as draft
August 20, 2026 21:07
Bslabe123
marked this pull request as ready for review
August 20, 2026 21:16
Bslabe123
changed the title
kubernetes-prow
Bot
removed
the
do-not-merge/work-in-progress
github-actions
Bot
removed
the
do-not-merge
Bslabe123
force-pushed
the
test/metric-drift-sglang-tgi-669
branch
from
August 26, 2026 15:50
de64d90 to
f925c30
Compare
github-actions
Bot
added
do-not-merge
Bslabe123
force-pushed
the
test/metric-drift-sglang-tgi-669
branch
from
August 26, 2026 17:48
f925c30 to
953a1bf
Compare
github-actions
Bot
added
do-not-merge
Bslabe123
force-pushed
the
test/metric-drift-sglang-tgi-669
branch
from
August 27, 2026 20:35
953a1bf to
1c9fcf0
Compare
github-actions
Bot
added
the
do-not-merge
Bslabe123
force-pushed
the
test/metric-drift-sglang-tgi-669
branch
from
August 27, 2026 20:46
1c9fcf0 to
7c0b2f8
Compare
github-actions
Bot
added
do-not-merge
Bslabe123
added
priority/important-soon
The drift checks decided whether a declared name resolves by restating the
exposition's naming conventions ("a counter matches X or X_total") in the test
utils, a second copy of what Metric.get_queries already implements. The copies
drifted: CounterMetric("vllm:prompt_tokens") queries the bare name only, while
the check accepted the declaration because v0.26.0 exposes
vllm:prompt_tokens_total. The check passed on a metric whose query selects
nothing, which is the exact failure mode it exists to catch.
Metric.candidate_names() now reports the series a metric's queries select, as
groups that are OR'd with the names within a group AND'd, and both drift checks
ask the metric rather than restating the rules. The test utils keep only the
series to family/type mapping, which is genuinely fixture-format knowledge.
Against the v0.26.0 golden this reds exactly vllm:prompt_tokens and
vllm:generation_tokens, both fixed by kubernetes-sigs#568. They go on a KNOWN_UNRESOLVED list,
kept separate from CONDITIONALLY_EXPOSED (metrics gated off on a stock server)
because the reason differs, and guarded by
test_known_unresolved_still_do_not_resolve so the entries cannot outlive the fix.
Part of kubernetes-sigs#669.
The live checks decided a declared name was usable by testing series presence alone. A family that keeps its name and changes type passes that test while the query built for it returns nonsense: increase() over a gauge does not raise, it reports a wrong number. That is the same failure shape as kubernetes-sigs#669 itself, where the check verified the server's naming rather than the query's selection. Both live checks now run two oracles over one exposition, each driven by the metric's own candidate_names(): is_exposed for series presence, resolves for the type the query assumes. Failures report absent and wrong_type separately because they are different bugs with different fixes. in_golden becomes resolves, since it now takes any family -> type map rather than a golden specifically. Also drops vllm:prompt_tokens_recomputed from CONDITIONALLY_EXPOSED: vllm_client no longer declares it and vLLM registers it nowhere in v0.26.0 through v0.28.0, so it was shrinking the strict checks for nothing. test_conditionally_exposed_still_apply guards that list the way test_known_unresolved_still_do_not_resolve already guards the other one. Checked against latest vLLM while here: all 33 declared names resolve against v0.28.0, whose metric registrations are byte-identical to v0.26.0.
Bslabe123
force-pushed
the
test/metric-drift-sglang-tgi-669
branch
from
September 3, 2026 20:30
5f110df to
dfeb91f
Compare
github-actions
Bot
added
do-not-merge
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
2 participants
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.
Part of #669 (SGLang/TGI half; the vLLM half is #697). Row in #606: "Metric-name drift, SGLang/TGI (#669)", e2e table, type
live.Why
A stale metric name never errors: the query matches nothing and the field in
summary_prometheus_metrics.jsoncomes back absent or zero, indistinguishable from a server that did not report it. #382 carries one instance (sglang:cache_hit_raterenamed tosglang:token_usage), caught by hand.This PR found three more.
sglang:time_per_output_token_seconds, declared bysglang_client.py, is registered nowhere in SGLang v0.5.17, so every SGLang run emits an emptytime_per_output_tokensection.sglang:inter_token_latency_secondsis the surviving histogram, so the fix is small; it is deliberately not in this test-only PR, theKNOWN_UNRESOLVEDentry being its repro.vllm:prompt_tokensandvllm:generation_tokensare queried under their bare names while v0.26.0 exposes only the_totalforms, soprompt_lenandoutput_lenhave read 0 against a stock server on any Prometheus. #568 fixes the query side.Name resolution now comes from the metrics
Both checks decided whether a declared name resolves by restating the exposition's naming conventions (a counter matches
XorX_total) in the test util, a second copy of whatMetric.get_queriesimplements. The copies drifted, which is how the vLLM pair passed #697's check while selecting nothing.Metric.candidate_names()now reports the series a metric's queries select, as groups OR'd with the names in a group AND'd, and both checks ask the metric instead of restating the rules. The utils keep only the series to family/type mapping, which is genuinely fixture-format knowledge, andis_exposedcollapses to a subset test.This adds scope:
inference_perf/client/modelserver/metrics/(one abstract method, three implementations) and #697'se2e/utils/metric_families.py. Two test stubs implement the new method.The two vLLM names go on
KNOWN_UNRESOLVED, kept apart fromCONDITIONALLY_EXPOSED(gated off on a stock server) because the reason differs, and guarded bytest_known_unresolved_still_do_not_resolve, which fails once they resolve. Verified by simulating #568'sCounterMetric: the guard fires. So this need not stack on #568, and #568 cannot leave a stale entry behind.What the tests do
e2e/tests/test_sglang_tgi_metric_names.py, five checks over both servers. Serverless: every declared name resolves againste2e/testdata/server_metric_families/<server>.txt; eachKNOWN_UNRESOLVEDentry is still declared and unresolved; each fixture states provenance, server, version, source, capture date. Live: the server's families equal the fixture, and declared names exist in a real exposition.The oracle is the names the server publishes, so provenance lives in the fixture. Both carry
provenance: upstream-source, derived by reading pinned upstream registration code (SGLang v0.5.17, 144 families; TGI v3.3.7, 27 families), not verified against a live server. A source-derived set is a superset of any live exposition, so the family-equality check skips until a live scrape lands.Supporting:
scripts/capture_server_metric_names.py, and.github/workflows/metric_name_drift.yml(weekly plus dispatch, nopull_requesttrigger, gated on aMETRIC_DRIFT_RUNNERvariable, so it never gates a PR).Status
pdm run validateclean.tests/requiredplus both drift modules: 666 passed, 13 skipped.Not verified: fixture contents have never been compared to a live SGLang or TGI (both need an accelerator); the scheduled workflow has never run; live checks need
E2E_SGLANG_BASE_URL/E2E_TGI_BASE_URLand skip without them.