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 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.

Author: Bslabe123

e2e/tests/test_sglang_tgi_metric_names.py

@@ -51,7 +51,7 @@
 committed fixture is by construction what these checks would have parsed.
 """
 
-from typing import Dict
+from typing import Any, Dict
 from unittest.mock import MagicMock, patch
 
 import pytest
@@ -74,6 +74,7 @@
 )
 
 from inference_perf.config import APIConfig, APIType
+from inference_perf.client.modelserver.metrics.base import Metric
 
 SERVER_IDS = sorted(SERVERS)
 
@@ -101,8 +102,11 @@
 }
 
 
-def declared_for(spec: ServerSpec) -> Dict[str, str]:
-    """Metric base names -> type, as the server's client subclass declares them.
+def declared_for(spec: ServerSpec) -> Dict[str, Metric[Any]]:
+    """Declared metric name -> the metric object, as the server's client subclass declares them.
+
+    The metric is carried rather than its name and type because only the metric knows
+    which series its queries select (``candidate_names``).
 
     Only the declarations are read, never the tokenizer, so CustomTokenizer is
     patched out to keep construction offline (same approach as
@@ -118,7 +122,7 @@ def declared_for(spec: ServerSpec) -> Dict[str, str]:
             max_tcp_connections=1,
             additional_filters=[],
         )
-    declared = declared_metrics(client.get_prometheus_metric_metadata(), spec.prefix)
+    declared = declared_metrics(client.get_prometheus_metric_metadata())
     assert declared, f"{spec.name} client declared no metric names"
     return declared
 
@@ -153,9 +157,7 @@ def test_declared_names_resolve_against_fixture(server: str) -> None:
     allowed = KNOWN_UNRESOLVED[server]
 
     missing = sorted(
-        name
-        for name, metric_type in declared.items()
-        if name not in allowed and not resolves(name, metric_type, fixture.families)
+        name for name, metric in declared.items() if name not in allowed and not resolves(metric, fixture.families)
     )
     assert not missing, (
         f"{len(missing)}/{len(declared)} names declared by the {server} client do not resolve against "
@@ -177,7 +179,7 @@ def test_known_unresolved_are_still_unresolved(server: str) -> None:
     undeclared = sorted(name for name in allowed if name not in declared)
     assert not undeclared, f"{server} no longer declares {undeclared}; drop the KNOWN_UNRESOLVED entries"
 
-    now_resolving = sorted(name for name in allowed if resolves(name, declared[name], fixture.families))
+    now_resolving = sorted(name for name in allowed if resolves(declared[name], fixture.families))
     assert not now_resolving, (
         f"{server} declarations {now_resolving} now resolve against {fixture_path(server).name}; "
         f"drop their KNOWN_UNRESOLVED entries so the strict check covers them again"
@@ -215,9 +217,7 @@ def test_declared_metric_names_exist(server: str) -> None:
     declared = declared_for(spec)
     allowed = KNOWN_UNRESOLVED[server]
 
-    missing = sorted(
-        name for name, metric_type in declared.items() if name not in allowed and not is_exposed(name, metric_type, names)
-    )
+    missing = sorted(name for name, metric in declared.items() if name not in allowed and not is_exposed(metric, names))
     assert not missing, (
         f"{len(missing)}/{len(declared)} names declared by the {server} client are absent from a real "
         f"/metrics exposition (stale names produce silently empty report fields): {missing}"

e2e/tests/test_vllm_cpu_metric_names.py

@@ -45,6 +45,8 @@
 """
 
 import os
+from pathlib import Path
+from typing import Any
 
 import aiohttp
 import pytest
@@ -64,6 +66,7 @@
 from utils.testdata import extract_tarball
 from utils.vllm_server import DEFAULT_MODEL, VLLMServerRunner
 
+from inference_perf.client.modelserver.metrics.base import Metric
 from inference_perf.client.modelserver.vllm_client import vLLMModelServerClient
 from inference_perf.config import APIConfig, APIType, CustomTokenizerConfig
 from inference_perf.metrics.request_collector.local import LocalRequestMetricCollector
@@ -94,11 +97,27 @@
     "vllm:prompt_tokens_recomputed",
 }
 
+# Declarations whose queries select nothing, with a fix already in flight. Kept
+# apart from CONDITIONALLY_EXPOSED because the reason is different: those names
+# are gated off on a stock server, these are simply queried under a name the
+# server does not use. Found by this check once it started asking what the
+# queries select rather than whether the declared name is findable (#669).
+#
+# The list cannot rot: test_known_unresolved_still_do_not_resolve fails the
+# moment an entry starts resolving, which forces it to be deleted.
+# Empty: #568 landed and made counters span both name forms, so vllm:prompt_tokens and
+# vllm:generation_tokens resolve again and the strict check covers them directly. The
+# mechanism stays for the next declaration whose fix is still in flight.
+KNOWN_UNRESOLVED: dict[str, str] = {}
+
+# Names excluded from the strict checks for either reason.
+SKIPPED = CONDITIONALLY_EXPOSED | set(KNOWN_UNRESOLVED)
+
 GOLDEN_FILES = sorted(GOLDEN_DIR.glob("*.txt"))
 
 
-def _declared(base_url: str, model_name: str) -> dict[str, str]:
-    """Metric base names -> metric type, as declared by the vLLM client."""
+def _declared(base_url: str, model_name: str) -> dict[str, Metric[Any]]:
+    """Declared metric name -> the metric object, as declared by the vLLM client."""
     client = vLLMModelServerClient(
         metrics_collector=LocalRequestMetricCollector(),
         api_config=APIConfig(type=APIType.Completion),
@@ -128,26 +147,41 @@ def test_goldens_are_committed() -> None:
 
 
 @pytest.mark.parametrize("golden_file", GOLDEN_FILES, ids=lambda p: p.stem)
-def test_declared_names_resolve_against_goldens(golden_file) -> None:
+def test_declared_names_resolve_against_goldens(golden_file: Path) -> None:
     golden = load_golden(golden_file)
     assert golden, f"golden {golden_file} is empty"
     declared = _declared("http://127.0.0.1:1", DEFAULT_MODEL)
     assert declared, "vLLM client declared no metric names"
 
-    missing = sorted(
-        name
-        for name, metric_type in declared.items()
-        if name not in CONDITIONALLY_EXPOSED and not in_golden(name, metric_type, golden)
-    )
+    missing = sorted(name for name, metric in declared.items() if name not in SKIPPED and not in_golden(metric, golden))
     assert not missing, (
         f"{len(missing)}/{len(declared)} declared metric names do not resolve against {golden_file.name} "
         f"(stale names produce silently empty report fields): {missing}"
     )
 
 
+@pytest.mark.parametrize("golden_file", GOLDEN_FILES, ids=lambda p: p.stem)
+def test_known_unresolved_still_do_not_resolve(golden_file: Path) -> None:
+    # Guards the allowlist above. Each KNOWN_UNRESOLVED name must still be declared
+    # and must still fail to resolve; the moment #568 lands and vllm:prompt_tokens
+    # starts selecting vllm:prompt_tokens_total, this goes red and the entry has to
+    # be deleted. An allowlist that quietly stops applying is how a gate rots.
+    golden = load_golden(golden_file)
+    declared = _declared("http://127.0.0.1:1", DEFAULT_MODEL)
+
+    undeclared = sorted(name for name in KNOWN_UNRESOLVED if name not in declared)
+    assert not undeclared, f"no longer declared, drop the KNOWN_UNRESOLVED entries: {undeclared}"
+
+    now_resolving = sorted(name for name in KNOWN_UNRESOLVED if in_golden(declared[name], golden))
+    assert not now_resolving, (
+        f"{now_resolving} now resolve against {golden_file.name}; drop their KNOWN_UNRESOLVED "
+        f"entries so the strict check covers them again"
+    )
+
+
 @pytest.mark.asyncio
 @pytest.mark.skipif(not VLLMServerRunner.is_available(), reason="no vLLM server or executable available")
-async def test_exposed_families_match_golden():
+async def test_exposed_families_match_golden() -> None:
     release_tag = os.environ.get(ENV_VLLM_VERSION)
     if not release_tag:
         pytest.skip(f"{ENV_VLLM_VERSION} not set; cannot tell which release the server runs")
@@ -176,17 +210,13 @@ async def test_exposed_families_match_golden():
 
 @pytest.mark.asyncio
 @pytest.mark.skipif(not VLLMServerRunner.is_available(), reason="no vLLM server or executable available")
-async def test_declared_metric_names_exist():
+async def test_declared_metric_names_exist() -> None:
     async with VLLMServerRunner(port=get_free_port()) as server:
         names = exposed_names(await _warmed_up_exposition(server))
         declared = _declared(server.base_url, server.model)
 
     assert declared, "vLLM client declared no metric names"
-    missing = sorted(
-        name
-        for name, metric_type in declared.items()
-        if name not in CONDITIONALLY_EXPOSED and not is_exposed(name, metric_type, names)
-    )
+    missing = sorted(name for name, metric in declared.items() if name not in SKIPPED and not is_exposed(metric, names))
     assert not missing, (
         f"{len(missing)}/{len(declared)} declared metric names absent from a real vLLM /metrics exposition "
         f"(stale names produce silently empty report fields): {missing}"

e2e/utils/metric_families.py

@@ -33,29 +33,38 @@
 ``PROMETHEUS_DISABLE_CREATED_SERIES`` without any semantic change.
 """
 
-import re
 from pathlib import Path
-from typing import Dict, Set
+from typing import Any, Dict, Set
 
 from inference_perf.client.modelserver.metrics import CounterMetric, GaugeMetric, HistogramMetric
+from inference_perf.client.modelserver.metrics.base import Metric
 from inference_perf.client.modelserver.openai_client import OpenAIMetrics
 
 GOLDEN_DIR = Path(__file__).resolve().parents[1] / "testdata" / "vllm_metric_families"
 
-_BASE_NAME = re.compile(r"vllm:[A-Za-z0-9_]+")
 
+def declared_metrics(metadata: OpenAIMetrics) -> Dict[str, Metric[Any]]:
+    """Declared metric name -> the metric object, as declared by a client's metadata.
 
-def declared_metrics(metadata: OpenAIMetrics) -> Dict[str, str]:
-    """Metric base names -> prometheus type, as declared by a client's metadata."""
-    types = ((CounterMetric, "counter"), (GaugeMetric, "gauge"), (HistogramMetric, "histogram"))
-    declared: Dict[str, str] = {}
+    The metric itself is carried, not just its name and type, because it is the
+    only thing that knows which series its queries select (``candidate_names``).
+    The key stays the declared name so callers can key allowlists and failure
+    messages off exactly what appears in the client source.
+    """
+    declared: Dict[str, Metric[Any]] = {}
     for _field, metric in metadata:
-        metric_type = next(t for cls, t in types if isinstance(metric, cls))
-        for name in _BASE_NAME.findall(metric.metric_name):
-            declared[name] = metric_type
+        declared[metric.metric_name] = metric
     return declared
 
 
+def prometheus_type(metric: Metric[Any]) -> str:
+    """The exposition type a declared metric expects its family to carry."""
+    for cls, metric_type in ((CounterMetric, "counter"), (GaugeMetric, "gauge"), (HistogramMetric, "histogram")):
+        if isinstance(metric, cls):
+            return metric_type
+    raise TypeError(f"no prometheus type known for {type(metric).__name__}")
+
+
 def exposed_names(metrics_text: str) -> Set[str]:
     """All family and sample names present in a /metrics exposition."""
     names = set()
@@ -79,25 +88,39 @@ def exposed_vllm_families(metrics_text: str) -> Dict[str, str]:
     return families
 
 
-def is_exposed(name: str, metric_type: str, names: Set[str]) -> bool:
-    """Whether a declared (name, type) is present in a live exposition's names.
+def provided_by_families(series: str, metric_type: str, families: Dict[str, str]) -> bool:
+    """Whether a family -> type map provides one series a query selects.
+
+    A family map records what ``# TYPE`` declares, so counter and gauge series
+    are families in their own right, while ``_bucket``/``_count``/``_sum`` series
+    are produced by a histogram or summary family with the suffix stripped. That
+    second case also covers a counter declared straight onto a histogram's
+    ``_count`` series, which is valid PromQL and which SGLang's request count uses.
+    """
+    if families.get(series) == metric_type:
+        return True
+    for suffix in ("_bucket", "_count", "_sum"):
+        if series.endswith(suffix) and families.get(series[: -len(suffix)]) in ("histogram", "summary"):
+            return True
+    return False
+
+
+def is_exposed(metric: Metric[Any], names: Set[str]) -> bool:
+    """Whether every series this metric's queries select is in a live exposition.
 
-    Presence is type-aware, mirroring what a Prometheus scrape stores: gauges
-    by bare name, counters by bare or ``_total``-suffixed name, histograms by
-    their ``_bucket``/``_count``/``_sum`` series.
+    An exposition lists real series names, so this is a plain subset test over the
+    metric's own candidate groups; nothing here needs to know how a counter or a
+    histogram is spelled.
     """
-    if metric_type == "histogram":
-        return all(f"{name}{suffix}" in names for suffix in ("_bucket", "_count", "_sum"))
-    if metric_type == "counter":
-        return name in names or f"{name}_total" in names
-    return name in names
-
-
-def in_golden(name: str, metric_type: str, golden: Dict[str, str]) -> bool:
-    """Whether a declared (name, type) resolves against a golden family map."""
-    if metric_type == "counter":
-        return golden.get(name) == "counter" or golden.get(f"{name}_total") == "counter"
-    return golden.get(name) == metric_type
+    return any(group <= names for group in metric.candidate_names())
+
+
+def in_golden(metric: Metric[Any], golden: Dict[str, str]) -> bool:
+    """Whether every series this metric's queries select resolves against a golden."""
+    metric_type = prometheus_type(metric)
+    return any(
+        all(provided_by_families(series, metric_type, golden) for series in group) for group in metric.candidate_names()
+    )
 
 
 def golden_path(release_tag: str) -> Path: