Skip to content

Commit 8055a2b

Browse files
committed
test: resolve declared metric names by type, not by name alone
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 #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.
1 parent 0e09c3e commit 8055a2b

4 files changed

Lines changed: 94 additions & 23 deletions

File tree

e2e/tests/test_sglang_tgi_metric_names.py

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -212,13 +212,26 @@ def test_exposed_families_match_fixture(server: str) -> None:
212212

213213
@pytest.mark.parametrize("server", SERVER_IDS)
214214
def test_declared_metric_names_exist(server: str) -> None:
215+
# Two oracles over one exposition, both driven by the metric's own
216+
# candidate_names(): every series a query selects must be present
217+
# (is_exposed, over sample and family names), and the family behind it must
218+
# carry the type the query assumes (resolves, over the `# TYPE` map).
219+
# Presence alone is not enough. Given `# TYPE sglang:prefix_cache_hit gauge`
220+
# and a matching sample line, a declared CounterMetric("sglang:prefix_cache_hit")
221+
# is_exposed -> True on the bare-name candidate group, while the increase()
222+
# it emits is nonsense over a gauge and reports no error. resolves -> False.
215223
spec = SERVERS[server]
216-
names = exposed_names(live_metrics_text(spec))
224+
exposition = live_metrics_text(spec)
225+
names = exposed_names(exposition)
226+
families = parse_exposition(exposition, spec.prefix)
217227
declared = declared_for(spec)
218228
allowed = KNOWN_UNRESOLVED[server]
219-
220-
missing = sorted(name for name, metric in declared.items() if name not in allowed and not is_exposed(metric, names))
221-
assert not missing, (
222-
f"{len(missing)}/{len(declared)} names declared by the {server} client are absent from a real "
223-
f"/metrics exposition (stale names produce silently empty report fields): {missing}"
229+
checked = {name: metric for name, metric in declared.items() if name not in allowed}
230+
231+
absent = {name for name, metric in checked.items() if not is_exposed(metric, names)}
232+
mistyped = sorted(name for name, metric in checked.items() if name not in absent and not resolves(metric, families))
233+
assert not (absent or mistyped), (
234+
f"{len(absent) + len(mistyped)}/{len(checked)} names declared by the {server} client are unusable "
235+
f"against a real /metrics exposition (a stale name reports an empty field, a retyped family reports "
236+
f"a nonsense one, and neither raises): absent={sorted(absent)}, wrong_type={mistyped}"
224237
)

e2e/tests/test_vllm_cpu_metric_names.py

Lines changed: 57 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@
3131
release, which is the deliberate state of ``latest``: a floating tag
3232
would guarantee golden rot, so it never gets one.
3333
3. ``test_declared_metric_names_exist`` (live, every release in the table):
34-
declared names present in the real exposition, the end-invariant itself.
34+
declared names present in the real exposition under the type their query
35+
assumes, the end-invariant itself.
3536
On ``latest`` this is the early warning that an upstream rename is
3637
coming, red on live PRs before, not at, the next pin bump.
3738
@@ -58,8 +59,8 @@
5859
exposed_vllm_families,
5960
format_golden,
6061
golden_path,
61-
in_golden,
6262
is_exposed,
63+
resolves,
6364
load_golden,
6465
)
6566
from utils.net import get_free_port
@@ -82,19 +83,22 @@
8283
# name declarations are read from it, never the tokenizer itself.
8384
GEMMA_TARBALL = "e2e/testdata/models/google_gemma-3-270m.tar.gz"
8485

85-
# Declared names that a STOCK vLLM does not expose. All five arrived in #348
86+
# Declared names that a STOCK vLLM does not expose. All four arrived in #348
8687
# ("vLLM latest (0.15.0) production metrics") and are absent from a default
87-
# v0.26.0 server, seemingly gated on optional features (KV offloading and
88-
# similar) whose components never register their metric families on a stock
89-
# configuration. Kept out of the strict checks rather than deleted so the
88+
# v0.26.0 server, gated on optional features (VLLM_COMPUTE_NANS_IN_LOGITS and
89+
# --kv-cache-metrics) whose components never register their metric families on
90+
# a stock configuration. Kept out of the strict checks rather than deleted so the
9091
# declarations can be triaged: each is either config-gated (then this list
9192
# documents the gate) or stale (then it should be removed from vllm_client).
93+
#
94+
# Guarded by test_conditionally_exposed_still_apply. A fifth entry,
95+
# vllm:prompt_tokens_recomputed, outlived its declaration and sat here shrinking
96+
# the strict checks for nothing; it is registered nowhere in vLLM v0.26.0-v0.28.0.
9297
CONDITIONALLY_EXPOSED = {
9398
"vllm:corrupted_requests",
9499
"vllm:kv_block_idle_before_evict_seconds",
95100
"vllm:kv_block_lifetime_seconds",
96101
"vllm:kv_block_reuse_gap_seconds",
97-
"vllm:prompt_tokens_recomputed",
98102
}
99103

100104
# Declarations whose queries select nothing, with a fix already in flight. Kept
@@ -153,7 +157,7 @@ def test_declared_names_resolve_against_goldens(golden_file: Path) -> None:
153157
declared = _declared("http://127.0.0.1:1", DEFAULT_MODEL)
154158
assert declared, "vLLM client declared no metric names"
155159

156-
missing = sorted(name for name, metric in declared.items() if name not in SKIPPED and not in_golden(metric, golden))
160+
missing = sorted(name for name, metric in declared.items() if name not in SKIPPED and not resolves(metric, golden))
157161
assert not missing, (
158162
f"{len(missing)}/{len(declared)} declared metric names do not resolve against {golden_file.name} "
159163
f"(stale names produce silently empty report fields): {missing}"
@@ -172,13 +176,36 @@ def test_known_unresolved_still_do_not_resolve(golden_file: Path) -> None:
172176
undeclared = sorted(name for name in KNOWN_UNRESOLVED if name not in declared)
173177
assert not undeclared, f"no longer declared, drop the KNOWN_UNRESOLVED entries: {undeclared}"
174178

175-
now_resolving = sorted(name for name in KNOWN_UNRESOLVED if in_golden(declared[name], golden))
179+
now_resolving = sorted(name for name in KNOWN_UNRESOLVED if resolves(declared[name], golden))
176180
assert not now_resolving, (
177181
f"{now_resolving} now resolve against {golden_file.name}; drop their KNOWN_UNRESOLVED "
178182
f"entries so the strict check covers them again"
179183
)
180184

181185

186+
@pytest.mark.parametrize("golden_file", GOLDEN_FILES, ids=lambda p: p.stem)
187+
def test_conditionally_exposed_still_apply(golden_file: Path) -> None:
188+
# Guards the other allowlist the same way. CONDITIONALLY_EXPOSED shrinks the
189+
# strict checks, so each entry has to still be earning that: still declared by
190+
# vllm_client, and still absent from a stock server's golden. Feeding it
191+
# {"vllm:corrupted_requests"} against v0.26.0.txt passes, because the client
192+
# declares it and the golden (captured from a stock server, which does not run
193+
# with VLLM_COMPUTE_NANS_IN_LOGITS) does not list it. Dropping the declaration
194+
# or a release starting to expose it both fail here, which is what stops the
195+
# list quietly widening the hole it opens.
196+
golden = load_golden(golden_file)
197+
declared = _declared("http://127.0.0.1:1", DEFAULT_MODEL)
198+
199+
undeclared = sorted(name for name in CONDITIONALLY_EXPOSED if name not in declared)
200+
assert not undeclared, f"no longer declared, drop the CONDITIONALLY_EXPOSED entries: {undeclared}"
201+
202+
now_resolving = sorted(name for name in CONDITIONALLY_EXPOSED if resolves(declared[name], golden))
203+
assert not now_resolving, (
204+
f"{now_resolving} resolve against {golden_file.name}, so they are not gated off on a stock "
205+
f"server; drop their CONDITIONALLY_EXPOSED entries so the strict check covers them again"
206+
)
207+
208+
182209
@pytest.mark.asyncio
183210
@pytest.mark.skipif(not VLLMServerRunner.is_available(), reason="no vLLM server or executable available")
184211
async def test_exposed_families_match_golden() -> None:
@@ -211,13 +238,29 @@ async def test_exposed_families_match_golden() -> None:
211238
@pytest.mark.asyncio
212239
@pytest.mark.skipif(not VLLMServerRunner.is_available(), reason="no vLLM server or executable available")
213240
async def test_declared_metric_names_exist() -> None:
241+
# Two oracles over one exposition, both driven by the metric's own
242+
# candidate_names(): every series a query selects must be present
243+
# (is_exposed, over sample and family names), and the family behind it must
244+
# carry the type the query assumes (resolves, over the `# TYPE` map).
245+
# Presence alone is not enough, and that gap is the same shape as the bug
246+
# this module exists for. Given `# TYPE vllm:prefix_cache_hits gauge` and a
247+
# sample line `vllm:prefix_cache_hits{...} 3`, a declared
248+
# CounterMetric("vllm:prefix_cache_hits") is_exposed -> True on the bare-name
249+
# candidate group, while the query it emits, increase(...[60s]), is nonsense
250+
# over a gauge and reports no error. resolves -> False catches it.
214251
async with VLLMServerRunner(port=get_free_port()) as server:
215-
names = exposed_names(await _warmed_up_exposition(server))
252+
exposition = await _warmed_up_exposition(server)
216253
declared = _declared(server.base_url, server.model)
217254

218255
assert declared, "vLLM client declared no metric names"
219-
missing = sorted(name for name, metric in declared.items() if name not in SKIPPED and not is_exposed(metric, names))
220-
assert not missing, (
221-
f"{len(missing)}/{len(declared)} declared metric names absent from a real vLLM /metrics exposition "
222-
f"(stale names produce silently empty report fields): {missing}"
256+
names = exposed_names(exposition)
257+
families = exposed_vllm_families(exposition)
258+
checked = {name: metric for name, metric in declared.items() if name not in SKIPPED}
259+
260+
absent = {name for name, metric in checked.items() if not is_exposed(metric, names)}
261+
mistyped = sorted(name for name, metric in checked.items() if name not in absent and not resolves(metric, families))
262+
assert not (absent or mistyped), (
263+
f"{len(absent) + len(mistyped)}/{len(checked)} declared metric names unusable against a real vLLM "
264+
f"/metrics exposition (a stale name reports an empty field, a retyped family reports a nonsense one, "
265+
f"and neither raises): absent={sorted(absent)}, wrong_type={mistyped}"
223266
)

e2e/utils/metric_families.py

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,15 +111,26 @@ def is_exposed(metric: Metric[Any], names: Set[str]) -> bool:
111111
An exposition lists real series names, so this is a plain subset test over the
112112
metric's own candidate groups; nothing here needs to know how a counter or a
113113
histogram is spelled.
114+
115+
Presence only. It cannot see a family that kept its name and changed type, so
116+
the live check pairs it with ``resolves`` over the same exposition's family
117+
map rather than using it alone (#669).
114118
"""
115119
return any(group <= names for group in metric.candidate_names())
116120

117121

118-
def in_golden(metric: Metric[Any], golden: Dict[str, str]) -> bool:
119-
"""Whether every series this metric's queries select resolves against a golden."""
122+
def resolves(metric: Metric[Any], families: Dict[str, str]) -> bool:
123+
"""Whether every series this metric's queries select resolves against a family map.
124+
125+
Takes a family -> type map, so it serves both a committed golden and a map
126+
parsed off a live exposition. Unlike ``is_exposed`` this is type-aware: a
127+
family carrying the right name under the wrong type does not resolve, which
128+
is what a name-only check cannot see (a counter turned gauge still answers
129+
to its bare name while ``increase()`` over it returns nonsense).
130+
"""
120131
metric_type = prometheus_type(metric)
121132
return any(
122-
all(provided_by_families(series, metric_type, golden) for series in group) for group in metric.candidate_names()
133+
all(provided_by_families(series, metric_type, families) for series in group) for group in metric.candidate_names()
123134
)
124135

125136

e2e/utils/server_metric_names.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,10 @@ def is_exposed(metric: Metric[Any], names: Set[str]) -> bool:
243243
244244
An exposition lists real series names, so this is a plain subset test over the
245245
metric's candidate groups.
246+
247+
Presence only. It cannot see a family that kept its name and changed type, so
248+
the live check pairs it with ``resolves`` over the same exposition's family
249+
map rather than using it alone (#669).
246250
"""
247251
return any(group <= names for group in metric.candidate_names())
248252

0 commit comments

Comments
 (0)