Skip to content

Commit 953a1bf

Browse files
committed
test: source drift-check name resolution from the metrics themselves
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 #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 #669.
1 parent 52327f4 commit 953a1bf

11 files changed

Lines changed: 256 additions & 103 deletions

File tree

e2e/tests/test_sglang_tgi_metric_names.py

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ def declared_for(spec: ServerSpec) -> Dict[str, str]:
118118
max_tcp_connections=1,
119119
additional_filters=[],
120120
)
121-
declared = declared_metrics(client.get_prometheus_metric_metadata(), spec.prefix)
121+
declared = declared_metrics(client.get_prometheus_metric_metadata())
122122
assert declared, f"{spec.name} client declared no metric names"
123123
return declared
124124

@@ -153,9 +153,7 @@ def test_declared_names_resolve_against_fixture(server: str) -> None:
153153
allowed = KNOWN_UNRESOLVED[server]
154154

155155
missing = sorted(
156-
name
157-
for name, metric_type in declared.items()
158-
if name not in allowed and not resolves(name, metric_type, fixture.families)
156+
name for name, metric in declared.items() if name not in allowed and not resolves(metric, fixture.families)
159157
)
160158
assert not missing, (
161159
f"{len(missing)}/{len(declared)} names declared by the {server} client do not resolve against "
@@ -177,7 +175,7 @@ def test_known_unresolved_are_still_unresolved(server: str) -> None:
177175
undeclared = sorted(name for name in allowed if name not in declared)
178176
assert not undeclared, f"{server} no longer declares {undeclared}; drop the KNOWN_UNRESOLVED entries"
179177

180-
now_resolving = sorted(name for name in allowed if resolves(name, declared[name], fixture.families))
178+
now_resolving = sorted(name for name in allowed if resolves(declared[name], fixture.families))
181179
assert not now_resolving, (
182180
f"{server} declarations {now_resolving} now resolve against {fixture_path(server).name}; "
183181
f"drop their KNOWN_UNRESOLVED entries so the strict check covers them again"
@@ -215,9 +213,7 @@ def test_declared_metric_names_exist(server: str) -> None:
215213
declared = declared_for(spec)
216214
allowed = KNOWN_UNRESOLVED[server]
217215

218-
missing = sorted(
219-
name for name, metric_type in declared.items() if name not in allowed and not is_exposed(name, metric_type, names)
220-
)
216+
missing = sorted(name for name, metric in declared.items() if name not in allowed and not is_exposed(metric, names))
221217
assert not missing, (
222218
f"{len(missing)}/{len(declared)} names declared by the {server} client are absent from a real "
223219
f"/metrics exposition (stale names produce silently empty report fields): {missing}"

e2e/tests/test_vllm_cpu_metric_names.py

Lines changed: 41 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
"""
4646

4747
import os
48+
from typing import Any
4849

4950
import aiohttp
5051
import pytest
@@ -64,6 +65,7 @@
6465
from utils.testdata import extract_tarball
6566
from utils.vllm_server import DEFAULT_MODEL, VLLMServerRunner
6667

68+
from inference_perf.client.modelserver.metrics.base import Metric
6769
from inference_perf.client.modelserver.vllm_client import vLLMModelServerClient
6870
from inference_perf.config import APIConfig, APIType, CustomTokenizerConfig
6971
from inference_perf.metrics.request_collector.local import LocalRequestMetricCollector
@@ -94,11 +96,27 @@
9496
"vllm:prompt_tokens_recomputed",
9597
}
9698

99+
# Declarations whose queries select nothing, with a fix already in flight. Kept
100+
# apart from CONDITIONALLY_EXPOSED because the reason is different: those names
101+
# are gated off on a stock server, these are simply queried under a name the
102+
# server does not use. Found by this check once it started asking what the
103+
# queries select rather than whether the declared name is findable (#669).
104+
#
105+
# The list cannot rot: test_known_unresolved_still_do_not_resolve fails the
106+
# moment an entry starts resolving, which forces it to be deleted.
107+
KNOWN_UNRESOLVED = {
108+
"vllm:prompt_tokens": "queried bare; v0.26.0 exposes vllm:prompt_tokens_total. Fixed by #568",
109+
"vllm:generation_tokens": "queried bare; v0.26.0 exposes vllm:generation_tokens_total. Fixed by #568",
110+
}
111+
112+
# Names excluded from the strict checks for either reason.
113+
SKIPPED = CONDITIONALLY_EXPOSED | set(KNOWN_UNRESOLVED)
114+
97115
GOLDEN_FILES = sorted(GOLDEN_DIR.glob("*.txt"))
98116

99117

100-
def _declared(base_url: str, model_name: str) -> dict[str, str]:
101-
"""Metric base names -> metric type, as declared by the vLLM client."""
118+
def _declared(base_url: str, model_name: str) -> dict[str, Metric[Any]]:
119+
"""Declared metric name -> the metric object, as declared by the vLLM client."""
102120
client = vLLMModelServerClient(
103121
metrics_collector=LocalRequestMetricCollector(),
104122
api_config=APIConfig(type=APIType.Completion),
@@ -134,17 +152,32 @@ def test_declared_names_resolve_against_goldens(golden_file) -> None:
134152
declared = _declared("http://127.0.0.1:1", DEFAULT_MODEL)
135153
assert declared, "vLLM client declared no metric names"
136154

137-
missing = sorted(
138-
name
139-
for name, metric_type in declared.items()
140-
if name not in CONDITIONALLY_EXPOSED and not in_golden(name, metric_type, golden)
141-
)
155+
missing = sorted(name for name, metric in declared.items() if name not in SKIPPED and not in_golden(metric, golden))
142156
assert not missing, (
143157
f"{len(missing)}/{len(declared)} declared metric names do not resolve against {golden_file.name} "
144158
f"(stale names produce silently empty report fields): {missing}"
145159
)
146160

147161

162+
@pytest.mark.parametrize("golden_file", GOLDEN_FILES, ids=lambda p: p.stem)
163+
def test_known_unresolved_still_do_not_resolve(golden_file) -> None:
164+
# Guards the allowlist above. Each KNOWN_UNRESOLVED name must still be declared
165+
# and must still fail to resolve; the moment #568 lands and vllm:prompt_tokens
166+
# starts selecting vllm:prompt_tokens_total, this goes red and the entry has to
167+
# be deleted. An allowlist that quietly stops applying is how a gate rots.
168+
golden = load_golden(golden_file)
169+
declared = _declared("http://127.0.0.1:1", DEFAULT_MODEL)
170+
171+
undeclared = sorted(name for name in KNOWN_UNRESOLVED if name not in declared)
172+
assert not undeclared, f"no longer declared, drop the KNOWN_UNRESOLVED entries: {undeclared}"
173+
174+
now_resolving = sorted(name for name in KNOWN_UNRESOLVED if in_golden(declared[name], golden))
175+
assert not now_resolving, (
176+
f"{now_resolving} now resolve against {golden_file.name}; drop their KNOWN_UNRESOLVED "
177+
f"entries so the strict check covers them again"
178+
)
179+
180+
148181
@pytest.mark.asyncio
149182
@pytest.mark.skipif(not VLLMServerRunner.is_available(), reason="no vLLM server or executable available")
150183
async def test_exposed_families_match_golden():
@@ -182,11 +215,7 @@ async def test_declared_metric_names_exist():
182215
declared = _declared(server.base_url, server.model)
183216

184217
assert declared, "vLLM client declared no metric names"
185-
missing = sorted(
186-
name
187-
for name, metric_type in declared.items()
188-
if name not in CONDITIONALLY_EXPOSED and not is_exposed(name, metric_type, names)
189-
)
218+
missing = sorted(name for name, metric in declared.items() if name not in SKIPPED and not is_exposed(metric, names))
190219
assert not missing, (
191220
f"{len(missing)}/{len(declared)} declared metric names absent from a real vLLM /metrics exposition "
192221
f"(stale names produce silently empty report fields): {missing}"

e2e/utils/metric_families.py

Lines changed: 50 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -33,29 +33,38 @@
3333
``PROMETHEUS_DISABLE_CREATED_SERIES`` without any semantic change.
3434
"""
3535

36-
import re
3736
from pathlib import Path
38-
from typing import Dict, Set
37+
from typing import Any, Dict, Set
3938

4039
from inference_perf.client.modelserver.metrics import CounterMetric, GaugeMetric, HistogramMetric
40+
from inference_perf.client.modelserver.metrics.base import Metric
4141
from inference_perf.client.modelserver.openai_client import OpenAIMetrics
4242

4343
GOLDEN_DIR = Path(__file__).resolve().parents[1] / "testdata" / "vllm_metric_families"
4444

45-
_BASE_NAME = re.compile(r"vllm:[A-Za-z0-9_]+")
4645

46+
def declared_metrics(metadata: OpenAIMetrics) -> Dict[str, Metric[Any]]:
47+
"""Declared metric name -> the metric object, as declared by a client's metadata.
4748
48-
def declared_metrics(metadata: OpenAIMetrics) -> Dict[str, str]:
49-
"""Metric base names -> prometheus type, as declared by a client's metadata."""
50-
types = ((CounterMetric, "counter"), (GaugeMetric, "gauge"), (HistogramMetric, "histogram"))
51-
declared: Dict[str, str] = {}
49+
The metric itself is carried, not just its name and type, because it is the
50+
only thing that knows which series its queries select (``candidate_names``).
51+
The key stays the declared name so callers can key allowlists and failure
52+
messages off exactly what appears in the client source.
53+
"""
54+
declared: Dict[str, Metric[Any]] = {}
5255
for _field, metric in metadata:
53-
metric_type = next(t for cls, t in types if isinstance(metric, cls))
54-
for name in _BASE_NAME.findall(metric.metric_name):
55-
declared[name] = metric_type
56+
declared[metric.metric_name] = metric
5657
return declared
5758

5859

60+
def prometheus_type(metric: Metric[Any]) -> str:
61+
"""The exposition type a declared metric expects its family to carry."""
62+
for cls, metric_type in ((CounterMetric, "counter"), (GaugeMetric, "gauge"), (HistogramMetric, "histogram")):
63+
if isinstance(metric, cls):
64+
return metric_type
65+
raise TypeError(f"no prometheus type known for {type(metric).__name__}")
66+
67+
5968
def exposed_names(metrics_text: str) -> Set[str]:
6069
"""All family and sample names present in a /metrics exposition."""
6170
names = set()
@@ -79,25 +88,39 @@ def exposed_vllm_families(metrics_text: str) -> Dict[str, str]:
7988
return families
8089

8190

82-
def is_exposed(name: str, metric_type: str, names: Set[str]) -> bool:
83-
"""Whether a declared (name, type) is present in a live exposition's names.
91+
def provided_by_families(series: str, metric_type: str, families: Dict[str, str]) -> bool:
92+
"""Whether a family -> type map provides one series a query selects.
93+
94+
A family map records what ``# TYPE`` declares, so counter and gauge series
95+
are families in their own right, while ``_bucket``/``_count``/``_sum`` series
96+
are produced by a histogram or summary family with the suffix stripped. That
97+
second case also covers a counter declared straight onto a histogram's
98+
``_count`` series, which is valid PromQL and which SGLang's request count uses.
99+
"""
100+
if families.get(series) == metric_type:
101+
return True
102+
for suffix in ("_bucket", "_count", "_sum"):
103+
if series.endswith(suffix) and families.get(series[: -len(suffix)]) in ("histogram", "summary"):
104+
return True
105+
return False
106+
107+
108+
def is_exposed(metric: Metric[Any], names: Set[str]) -> bool:
109+
"""Whether every series this metric's queries select is in a live exposition.
84110
85-
Presence is type-aware, mirroring what a Prometheus scrape stores: gauges
86-
by bare name, counters by bare or ``_total``-suffixed name, histograms by
87-
their ``_bucket``/``_count``/``_sum`` series.
111+
An exposition lists real series names, so this is a plain subset test over the
112+
metric's own candidate groups; nothing here needs to know how a counter or a
113+
histogram is spelled.
88114
"""
89-
if metric_type == "histogram":
90-
return all(f"{name}{suffix}" in names for suffix in ("_bucket", "_count", "_sum"))
91-
if metric_type == "counter":
92-
return name in names or f"{name}_total" in names
93-
return name in names
94-
95-
96-
def in_golden(name: str, metric_type: str, golden: Dict[str, str]) -> bool:
97-
"""Whether a declared (name, type) resolves against a golden family map."""
98-
if metric_type == "counter":
99-
return golden.get(name) == "counter" or golden.get(f"{name}_total") == "counter"
100-
return golden.get(name) == metric_type
115+
return any(group <= names for group in metric.candidate_names())
116+
117+
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."""
120+
metric_type = prometheus_type(metric)
121+
return any(
122+
all(provided_by_families(series, metric_type, golden) for series in group) for group in metric.candidate_names()
123+
)
101124

102125

103126
def golden_path(release_tag: str) -> Path:

e2e/utils/server_metric_names.py

Lines changed: 51 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -47,14 +47,13 @@
4747

4848
import json
4949
import os
50-
import re
5150
import urllib.request
5251
from dataclasses import dataclass
5352
from pathlib import Path
54-
from typing import Any, Dict, Optional, Set, Tuple, Type
53+
from typing import Any, Dict, Optional, Set, Type
5554

5655
from inference_perf.client.modelserver.metrics import CounterMetric, GaugeMetric, HistogramMetric
57-
from inference_perf.client.modelserver.metrics.base import BaseMetrics
56+
from inference_perf.client.modelserver.metrics.base import BaseMetrics, Metric
5857
from inference_perf.client.modelserver.openai_client import openAIModelServerClient
5958
from inference_perf.client.modelserver.sglang_client import SGlangModelServerClient
6059
from inference_perf.client.modelserver.tgi_client import TGImodelServerClient
@@ -70,11 +69,11 @@
7069
# without saying where its contents came from.
7170
REQUIRED_HEADER_KEYS = ("provenance", "server", "version", "source", "captured")
7271

73-
# Series suffixes a histogram or summary family produces. A client may declare
74-
# one of these directly as a counter (SGLang counts requests off the latency
75-
# histogram's _count series), which is valid PromQL, so name resolution has to
76-
# recognise it.
77-
_AGGREGATE_SUFFIXES = ("_count", "_sum")
72+
# Series suffixes produced by a histogram or summary family rather than being
73+
# families in their own right. A client may declare one directly as a counter
74+
# (SGLang counts requests off the latency histogram's _count series), which is
75+
# valid PromQL, so resolving a series against a family map has to recognise it.
76+
_AGGREGATE_SUFFIXES = ("_bucket", "_count", "_sum")
7877

7978

8079
@dataclass(frozen=True)
@@ -159,30 +158,28 @@ def fetch_json(url: str, timeout: float = 30.0) -> Any:
159158
return json.loads(fetch_text(url, timeout))
160159

161160

162-
def declared_metrics(metadata: BaseMetrics, prefix: str) -> Dict[str, str]:
163-
"""Metric base names -> prometheus type, as declared by a client's metadata.
161+
def declared_metrics(metadata: BaseMetrics) -> Dict[str, Metric[Any]]:
162+
"""Declared metric name -> the metric object, as declared by a client's metadata.
164163
165-
A declared name is normally bare (``sglang:num_queue_reqs``) but the
166-
counter type also accepts a version-spanning PromQL selector
167-
(``{__name__=~"tgi_request_success(_total)?"}``), so base names are pulled
168-
out by prefix rather than taken whole. A name that yields no base name at
169-
all is kept verbatim: it cannot resolve against any fixture, and a
170-
declaration this code cannot even parse is itself drift worth failing on.
164+
The metric is carried rather than its name and type because only the metric
165+
knows which series its queries select (``candidate_names``). The server's
166+
family prefix used to be needed here to pull base names out of a declaration;
167+
the metric takes itself apart now, so nothing here has to know the prefix.
171168
"""
172-
types: Tuple[Tuple[type, str], ...] = (
173-
(CounterMetric, "counter"),
174-
(GaugeMetric, "gauge"),
175-
(HistogramMetric, "histogram"),
176-
)
177-
base_name = re.compile(re.escape(prefix) + r"[A-Za-z0-9_]+")
178-
declared: Dict[str, str] = {}
169+
declared: Dict[str, Metric[Any]] = {}
179170
for _field, metric in metadata:
180-
metric_type = next(t for cls, t in types if isinstance(metric, cls))
181-
for name in base_name.findall(metric.metric_name) or [metric.metric_name]:
182-
declared[name] = metric_type
171+
declared[metric.metric_name] = metric
183172
return declared
184173

185174

175+
def prometheus_type(metric: Metric[Any]) -> str:
176+
"""The exposition type a declared metric expects its family to carry."""
177+
for cls, metric_type in ((CounterMetric, "counter"), (GaugeMetric, "gauge"), (HistogramMetric, "histogram")):
178+
if isinstance(metric, cls):
179+
return metric_type
180+
raise TypeError(f"no prometheus type known for {type(metric).__name__}")
181+
182+
186183
def parse_exposition(metrics_text: str, prefix: str) -> Dict[str, str]:
187184
"""The exposition's ``<prefix>*`` family -> type map, ``*_created`` dropped."""
188185
families: Dict[str, str] = {}
@@ -207,43 +204,47 @@ def exposed_names(metrics_text: str) -> Set[str]:
207204

208205

209206
def _aggregate_of(name: str) -> str:
210-
"""The family a ``_count``/``_sum`` series belongs to, or "" if not one."""
207+
"""The family a ``_bucket``/``_count``/``_sum`` series belongs to, or "" if not one."""
211208
for suffix in _AGGREGATE_SUFFIXES:
212209
if name.endswith(suffix):
213210
return name[: -len(suffix)]
214211
return ""
215212

216213

217-
def resolves(name: str, metric_type: str, families: Dict[str, str]) -> bool:
218-
"""Whether a declared (name, type) resolves against a family -> type map.
214+
def provided_by_families(series: str, metric_type: str, families: Dict[str, str]) -> bool:
215+
"""Whether a family -> type map provides one series a query selects.
219216
220-
Type aware, mirroring what a Prometheus scrape stores:
217+
A fixture records what ``# TYPE`` declares, so counter and gauge series are
218+
families in their own right, while ``_bucket``/``_count``/``_sum`` series come
219+
from a histogram or summary family with the suffix stripped.
220+
"""
221+
if families.get(series) == metric_type:
222+
return True
223+
base = _aggregate_of(series)
224+
return bool(base) and families.get(base) in ("histogram", "summary")
221225

222-
- gauges and histograms by exact family name and type;
223-
- counters by exact name, by the ``_total``-suffixed name that
224-
prometheus_client emits for a counter registered without it, or as the
225-
``_count``/``_sum`` series of a histogram or summary family.
226+
227+
def resolves(metric: Metric[Any], families: Dict[str, str]) -> bool:
228+
"""Whether every series this metric's queries select resolves against a fixture.
229+
230+
The naming conventions (a counter spanning the optional ``_total`` suffix, a
231+
histogram needing all three of its series) are not restated here: they come
232+
from the metric's own ``candidate_names``, so this check cannot drift away
233+
from what the client actually queries the way it did in #669.
226234
"""
227-
if metric_type == "counter":
228-
if families.get(name) == "counter" or families.get(f"{name}_total") == "counter":
229-
return True
230-
base = _aggregate_of(name)
231-
return bool(base) and families.get(base) in ("histogram", "summary")
232-
return families.get(name) == metric_type
235+
metric_type = prometheus_type(metric)
236+
return any(
237+
all(provided_by_families(series, metric_type, families) for series in group) for group in metric.candidate_names()
238+
)
233239

234240

235-
def is_exposed(name: str, metric_type: str, names: Set[str]) -> bool:
236-
"""Whether a declared (name, type) is present in a live exposition's names.
241+
def is_exposed(metric: Metric[Any], names: Set[str]) -> bool:
242+
"""Whether every series this metric's queries select is in a live exposition.
237243
238-
Presence is type aware for the same reason as ``resolves``: gauges by bare
239-
name, counters by bare or ``_total``-suffixed name, histograms by their
240-
``_bucket``/``_count``/``_sum`` series.
244+
An exposition lists real series names, so this is a plain subset test over the
245+
metric's candidate groups.
241246
"""
242-
if metric_type == "histogram":
243-
return all(f"{name}{suffix}" in names for suffix in ("_bucket", "_count", "_sum"))
244-
if metric_type == "counter":
245-
return name in names or f"{name}_total" in names
246-
return name in names
247+
return any(group <= names for group in metric.candidate_names())
247248

248249

249250
def parse_fixture(text: str) -> Fixture:

0 commit comments

Comments
 (0)