Skip to content

Fix wrong and empty built-in engine metrics - #546

Merged
dennis-upbound merged 10 commits into
modelplaneai:mainfrom
dennis-upbound:dennis/metrics-v06
Oct 21, 2026
Merged

dennis-upbound merged 10 commits into
modelplaneai:mainfrom
dennis-upbound:dennis/metrics-v06

Conversation

@dennis-upbound

@dennis-upbound dennis-upbound commented Oct 20, 2026 •

Copy link
Copy Markdown
Collaborator

Description of your changes

Fixes #523.
Fixes #524.
Fixes #525.
Towards #547.
Towards #476.

The built-in engine metrics gave wrong or missing numbers for configurations Modelplane ships, and nothing reported it. vLLM's and SGLang's token histograms shared modelplane_request_input_tokens and modelplane_request_output_tokens with different buckets, so every quantile over them was wrong. Three recipes ran engines whose metrics the mappings can't read. A prefill/decode deployment counted requests twice.

The token histograms can't share a bucket layout, even within vLLM, which sizes its buckets from --max-model-len. They now leave the cluster only as their sums and counts, modelplane_engine_tokens_total{direction} and modelplane_engine_requests_total. Prefill and decode engines' series carry a phase label, and since a decode engine sees every request with its whole prompt, a query counts requests and tokens on phase!="Prefill". The design gave these series a label called role, which already holds the member's role, so the new label is phase.

Nothing checked the mappings against what an engine emits, and that gap is how these shipped. A test now checks every mapping against fixtures of what nine component versions publish, read from pods serving traffic: the four vLLM versions the recipes pin, v0.9.2, v0.23.0, v0.25.1 and v0.27.1, SGLang v0.5.10 and v0.5.12, the gateway, the picker and GKE's DCGM exporter. It fails on main's mappings with #523. The docs manifest validator holds each recipe's engine to the vLLM version and SGLang flags the mappings need, and the telemetry docs name the versions the built-in mappings are tested against.

The built-in mappings cover fleet health from metrics Modelplane is confident in, because every engine mapping ties a release to the engine versions it was written against. So modelplane_responses_total{reason} and DCGM_FI_DEV_FB_FREE are added. The tuning metrics in #547, vLLM's KV offload and speculative decoding counters and DCGM's PCIe and SM fields, are published only while a feature runs. They get a documented MetricMapping instead, and the speculative decoding variant of the Qwen3-8B recipe ships its own. The gpu-operator's metrics CSV types DCGM's PCIe fields as counters and GKE's managed exporter as gauges, so the documented mapping says a Prometheus backend adds _total on one and not the other.

Two things change for existing users. The buckets of modelplane_request_input_tokens and modelplane_request_output_tokens are gone, so a dashboard reading them moves to the new counters. The phase label is a new pod label, so a prefill/decode deployment's engine pods roll when this is installed.

Served on GKE (L4) from a control plane built from this branch: the llama recipe on vLLM v0.23.0, engine counters matching each engine's own totals, and a vLLM prefill/decode pair summing 15 requests to 30 without the phase filter and to 15 with it.

Every metric the built-in mappings export after this change

The collector exports these and nothing else: it drops any series no mapping renamed.

Every series carries cluster, and job and instance for what it was scraped from. Engine series also carry deployment, replica, namespace, engine and role, plus phase on a prefill/decode deployment. Picker series carry the same apart from engine, with role="picker". Gateway series carry the model instead of a deployment, as gen_ai_original_model and gen_ai_request_model. GPU series carry node.

Metric Type Source
modelplane_frontend_request_duration_seconds histogram Gateway: gen_ai_server_request_duration_seconds
modelplane_frontend_ttft_seconds histogram Gateway: gen_ai_server_time_to_first_token_seconds
modelplane_frontend_tpot_seconds histogram Gateway: gen_ai_server_time_per_output_token_seconds
modelplane_request_ttft_seconds histogram vLLM: vllm:time_to_first_token_seconds
modelplane_request_duration_seconds histogram vLLM: vllm:e2e_request_latency_seconds
modelplane_request_queue_seconds histogram vLLM: vllm:request_queue_time_seconds
modelplane_request_prefill_seconds histogram vLLM: vllm:request_prefill_time_seconds
modelplane_request_decode_seconds histogram vLLM: vllm:request_decode_time_seconds
modelplane_requests_running gauge vLLM: vllm:num_requests_running
SGLang: sglang:num_running_reqs
modelplane_requests_waiting gauge vLLM: vllm:num_requests_waiting
SGLang: sglang:num_queue_reqs
modelplane_kv_cache_utilization_ratio gauge vLLM: vllm:kv_cache_usage_perc
SGLang: sglang:token_usage
modelplane_requests_preempted_total counter vLLM: vllm:num_preemptions_total
SGLang: sglang:num_retracted_requests_total
modelplane_prefix_cache_hits_total counter vLLM: vllm:prefix_cache_hits_total
SGLang: sglang:cached_tokens_total
modelplane_prefix_cache_lookups_total counter vLLM: vllm:prefix_cache_queries_total
SGLang: sglang:prompt_tokens_total
modelplane_engine_tokens_total (new) counter vLLM: sum of vllm:request_prompt_tokens, direction="input"
vLLM: sum of vllm:request_generation_tokens, direction="output"
SGLang: sum of sglang:prompt_tokens_histogram, direction="input"
SGLang: sum of sglang:generation_tokens_histogram, direction="output"
modelplane_engine_requests_total (new) counter vLLM: count of vllm:request_prompt_tokens
SGLang: count of sglang:prompt_tokens_histogram
modelplane_responses_total (new) counter vLLM: vllm:request_success_total, finished_reason as reason
modelplane_route_decision_seconds histogram Picker: llm_d_epp_scheduler_e2e_duration_seconds
modelplane_gpu_memory_used_bytes gauge DCGM: DCGM_FI_DEV_FB_USED, converted from mebibytes
modelplane_gpu_memory_free_bytes (new) gauge DCGM: DCGM_FI_DEV_FB_FREE, converted from mebibytes
modelplane_gpu_compute_active_ratio gauge DCGM: DCGM_FI_PROF_GR_ENGINE_ACTIVE
modelplane_gpu_tensor_active_ratio gauge DCGM: DCGM_FI_PROF_PIPE_TENSOR_ACTIVE
modelplane_gpu_memory_bandwidth_ratio gauge DCGM: DCGM_FI_PROF_DRAM_ACTIVE
modelplane_gpu_temperature_celsius gauge DCGM: DCGM_FI_DEV_GPU_TEMP
modelplane_gpu_power_watts gauge DCGM: DCGM_FI_DEV_POWER_USAGE
modelplane_energy_joules_total counter DCGM: DCGM_FI_DEV_TOTAL_ENERGY_CONSUMPTION, converted from millijoules

I have:

  • Read and followed Modelplane's contribution process.
  • Run nix flake check (or ./nix.sh flake check) and made sure it passes.
  • Added or updated tests covering any composition function changes.
  • Signed off every commit with git commit -s.

🤖 Generated with Claude Code

@github-actions

Copy link
Copy Markdown

Docs preview: https://modelplane-docs-pr-546.vercel.app (ready once the site's Content workflow finishes)

@dennis-upbound dennis-upbound changed the title Fix built-in metrics merging bucket layouts, going empty, and double counting Fix wrong and empty built-in engine metrics Oct 20, 2026
@dennis-upbound
dennis-upbound marked this pull request as ready for review October 20, 2026 22:32
@dennis-upbound
dennis-upbound force-pushed the dennis/metrics-v06 branch 2 times, most recently from 4566782 to a85f1b4 Compare October 20, 2026 22:52

@negz negz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stepping back from the line comments, this PR has me wondering which metrics belong in the built-in mappings at all.

We want good defaults, so a deployment's metrics show up without anyone writing a MetricMapping. But every engine mapping ties a Modelplane release to the engine versions we wrote it against, and this PR shows what that costs: vLLM v0.7.3 published different names, SGLang publishes nothing without --enable-metrics, and vLLM sizes its token buckets from --max-model-len. Over time that means chasing engine releases and maintaining a matrix of which Modelplane versions support which engine versions. Outside that matrix the metrics are empty or misleading, and nobody is likely to notice. It's also the per-engine knowledge design.md says Modelplane otherwise avoids.

A few options, which aren't mutually exclusive:

  • Document the engine versions we test each release's built-in mappings against, which today are the recipe pins.
  • Only map metrics upstream treats as an interface. vLLM's metrics design asks for deprecation before removal (metrics.md), and v0.25.1 still publishes the offload metrics' old names beside their new ones.
  • Only map the fleet signals Modelplane and platform teams need from every engine, like load, queueing, KV usage, tokens and latency. Leave metrics for features an ML team turns on, like speculative decoding and KV offload, to a MetricMapping in the recipe that enables them.
  • Take fleet signals from components we pin, the gateway and the picker, where they can supply them.
  • Make it visible when an engine publishes none of the names its mapping reads. Today an empty series is silent.

My lean is to map by default only metrics that are critical to fleet health and very stable. By stable I mean we pin the component that publishes the metric, or we're confident every engine publishes it and its meaning won't change. It's easier to add a metric later than to take one back if we're wrong. Applied to this PR, I think that leaves out most of what #547 adds, since those are tuning metrics rather than fleet health. But it's your call which of these, if any, we pursue, and whether any of it should change v0.6.

Comment thread docs/content/platform/telemetry.md Outdated
Comment on lines +65 to +72
A prefill/decode deployment runs each request on two engines, and both count it.
Count requests and generated tokens on the decode engines, and prompt processing on
the prefill engines:

```promql
sum by (deployment) (rate(modelplane_engine_requests_total{phase!="Prefill"}[5m]))
sum by (deployment) (rate(modelplane_engine_tokens_total{direction="input", phase!="Decode"}[5m]))
```

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the decode engine see every request here? I think the decider only sends a request to prefill when it has at least 16 uncached tokens (routing.py#L135, prefix_based_pd_decider.go#L113-L145), that the sidecar otherwise skips prefill (chat_completions.go#L102-L106), and that decode still gets the whole request when prefill runs (connector_nixlv2.go#L378-L400).

If that's right, prefill never counts short prompts or ones the picker expects decode to have cached, so the input token query here would undercount, most on cache-heavy traffic. How about reading every per-request count on phase!="Prefill"?

Suggested change
A prefill/decode deployment runs each request on two engines, and both count it.
Count requests and generated tokens on the decode engines, and prompt processing on
the prefill engines:
```promql
sum by (deployment) (rate(modelplane_engine_requests_total{phase!="Prefill"}[5m]))
sum by (deployment) (rate(modelplane_engine_tokens_total{direction="input", phase!="Decode"}[5m]))
```
A prefill/decode deployment runs some requests on both engines, and both count them.
The decode engines see every request with its whole prompt, so count requests and
tokens there:
```promql
sum by (deployment) (rate(modelplane_engine_requests_total{phase!="Prefill"}[5m]))
sum by (deployment) (rate(modelplane_engine_tokens_total{direction="input", phase!="Prefill"}[5m]))
```

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. I took the suggestion, so requests and tokens are both counted on phase!="Prefill", and the commit message says why.

Comment thread docs/content/platform/telemetry.md Outdated
Comment on lines +313 to +314
| `vllm:external_prefix_cache_hits_total` | `modelplane_external_prefix_cache_hits_total` | a prefix cache outside the GPU, such as a KV offload tier, from v0.23.0 |
| `vllm:external_prefix_cache_queries_total` | `modelplane_external_prefix_cache_lookups_total` | the same |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do these count NIXL transfers on a prefill/decode deployment? Reading vLLM, I think any KV connector's tokens count as external hits (scheduler.py#L615-L636), and decode's NixlConnector claims the whole prompt once prefill has run (nixl/scheduler.py#L382-L388). If so, on kimi-k2 they'd count KV pulled from prefill, where on an offload deployment they count offload-tier hits.

If that's right, could we leave them out of the built-ins for v0.6, or have the docs say they count any KV connector?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. The scheduler counts any connector's tokens as external hits, so on kimi-k2 they'd count KV pulled from prefill. They're out of the built-ins now. The documented offload mapping says they also count connector KV on a prefill/decode deployment.

name="VLLMOlder",
reason="vLLM v0.11.0 publishes every name the vLLM mapping reads that it has.",
mapping="modelplane-vllm",
file="vllm-v0.11.0.json",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these the versions we pin? I see v0.9.2 (qwen2.5-7b), v0.23.0, v0.25.1 (laguna) and v0.27.1 (nemotron-3.5-lightning) in the recipes, and v0.11.0 only in the design docs. If so, could we scrape the pinned ones instead, or say which versions the test covers?

Could the qwen3-8b-speculative-decoding recipe cover the spec-decode names? It runs ngram drafting on v0.23.0, and I think vLLM publishes those counters for any drafting method (spec_decode/metrics.py#L150-L192).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rescraped: v0.9.2, v0.25.1 and v0.27.1 are in, v0.11.0 is out, and each case names the recipe that pins it. v0.9.2 publishes every name the vLLM mapping reads, including kv_cache_usage_perc, which is also why v0.9.2 is the floor the manifest validator enforces. I scraped the speculative decoding recipe's ngram config on v0.23.0 too, and it publishes the three counters its new MetricMapping reads.

Comment on lines +332 to +336
# Rates a GPU exporter samples, which the gpu-operator's metrics CSV types a
# counter and GKE's managed exporter a gauge. A counter reaches a Prometheus
# backend with _total on its name, so the one metric would arrive under two
# names, by cloud. Made a gauge on every cluster before it's renamed.
_SAMPLED_RATES = ("DCGM_FI_PROF_PCIE_TX_BYTES", "DCGM_FI_PROF_PCIE_RX_BYTES")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This landed inside _UNIT_FACTOR's comment, which now stops mid-sentence at "because" and finishes below the new constant. Please move it out.

It's also the only metric name in this file. Every other DCGM field name is in stacks/metrics.py, and fn.py hands the built-in mappings to the collector. I'd rather this lived beside _GPU and came in the same way. Then the collector doesn't need to know any exporter's metric names.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed it. With PCIe out of the built-ins, the collector doesn't need any exporter's field names. The documented PCIe mapping says that the GPU Operator's metrics configuration types those fields as counters, so a Prometheus backend adds _total to their names on clusters running it.

# Names this scrape can't show: ones the component publishes only from the
# first event it counts, or only when it's started with a feature the
# scraped pod didn't run. Each is checked against the component's source.
unscraped: tuple[str, ...]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Every case's want is []. The expectation is in unscraped instead. Could we make want the names we expect the scrape to lack, and drop unscraped? Then the test is got = sorted(sources - names), and each case reads like our other tables. The comments above the cases already say why each name is missing.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. want is now the names a scrape lacks, and unscraped is gone. The test is sorted(sources - names). Only the two SGLang cases expect anything (sglang:num_retracted_requests_total), and the comment above them says why.

Comment on lines +146 to +169
def engine_metrics_problems(obj: pydantic.BaseModel) -> list[str]:
"""Why an engine in a ModelDeployment or ModelReplica wouldn't publish what the built-in mappings read."""
problems: list[str] = []
for engine in _engines(obj):
for member in engine.members:
for c in member.template.spec.containers:
if c.name != "engine":
continue
argv = " ".join((c.command or []) + (c.args or []))
version = _version(c.image)
if "lmsysorg/sglang:" in c.image:
if "--enable-metrics" not in argv:
problems.append(f"engine {engine.name}: SGLang serves no /metrics without --enable-metrics")
if version and version < (0, 5, 12) and "--collect-tokens-histogram" not in argv:
problems.append(
f"engine {engine.name}: SGLang before v0.5.12 publishes no token histograms "
"without --collect-tokens-histogram"
)
if "vllm/vllm-openai:" in c.image and version and version < (0, 9, 2):
problems.append(
f"engine {engine.name}: vLLM before v0.9.2 runs the V0 engine, whose metric names "
"and latency buckets the built-in mappings don't read"
)
return problems

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: four levels of loop with the SGLang and vLLM rules inline is harder to follow than the rest of this file. Could the body become a helper that takes one engine container and returns early for anything that isn't vLLM or SGLang?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. _container_problems takes one engine container, returns early for anything that isn't vLLM or SGLang, and returns the problem text. engine_metrics_problems just walks the engines and prefixes each problem with the engine's name.

@dennis-upbound
dennis-upbound force-pushed the dennis/metrics-v06 branch 2 times, most recently from 39cdd58 to 0ed2743 Compare October 21, 2026 04:03
@dennis-upbound

Copy link
Copy Markdown
Collaborator Author

Agreed, and I've applied it. The built-in mappings now cover only fleet health from metrics we're confident in:

  • the token counters
  • phase
  • modelplane_responses_total{reason}
  • DCGM_FI_DEV_FB_FREE
  • everything that was already built in

KV offload, speculative decoding, the external prefix cache counters, and DCGM's PCIe and SM fields are out. The docs carry a MetricMapping for each, and the speculative decoding variant of the Qwen3-8B recipe ships one beside the deployment that drafts. #547 is now "Towards".

On versions: the test now scrapes exactly the vLLM versions the recipes pin (v0.9.2, v0.23.0, v0.25.1, v0.27.1), plus SGLang v0.5.10 and v0.5.12. The telemetry docs say those are the versions the built-in mappings are tested against. v0.11.0 is gone.

@haarchri haarchri left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

over all looks good:

histograms become counters, every modelplane_* histogram has one source, and the scrape fixtures guard regressions. One issue is the prefill/decode effect on responses_total{reason="length"}. The question about using histogram _sum instead of the plain counters is a design point worth an answer ...

tokens there:

```promql
sum by (deployment) (rate(modelplane_engine_requests_total{phase!="Prefill"}[5m]))

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On a prefill/decode deployment, the prefill engine usually runs with max_tokens=1. vLLM then finishes that request with finished_reason="length". As a result, modelplane_responses_total{reason="length"} goes up for every request, and the truncation signal described at line 306 becomes useless on prefill/decode deployments.

Please add modelplane_responses_total to this paragraph, with the same phase!="Prefill" filter.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. The paragraph now counts modelplane_responses_total on phase!="Prefill" too, with a query for it. llm-d's NIXL and Mooncake sidecars set max_tokens to 1 on the prefill request, so prefill reports length for every request it runs.

exports only their sums and counts, as `modelplane_engine_tokens_total` and
`modelplane_engine_requests_total`, which add up correctly across any mix of engines.

vLLM's `vllm:request_success_total` becomes `modelplane_responses_total`, the

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same point here. Please add one sentence saying that on prefill/decode deployments you should read reason with phase!="Prefill". Otherwise length counts every request.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. The reason paragraph says to read it on phase!="Prefill" on a prefill/decode deployment, and why.

# SGLang v0.5.10 publishes these histograms only with
# --collect-tokens-histogram as well as --enable-metrics; from v0.5.12 it
# always does.
_VLLM_TOKENS = ("vllm:request_prompt_tokens", "vllm:request_generation_tokens")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why take tokens from the histogram _sum and not from the plain counters vllm:prompt_tokens_total and vllm:generation_tokens_total

Those counters exist in every pinned version and need no flag. For SGLang, the histogram path is the only reason recipes on versions before v0.5.12 need --collect-tokens-histogram right?
If the reason is “count only finished requests” (the histograms are observed when a request finishes), please say that in the comment.. The two totals will differ while requests are running or aborted.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's so tokens and requests count the same requests: both engines observe these histograms when a request finishes, so the sums and the count divide into tokens per request. The plain counters count tokens as they're processed, including requests that never finish, and sglang:prompt_tokens_total already feeds modelplane_prefix_cache_lookups_total. The comment above _VLLM_TOKENS says this now. You're right that SGLang before v0.5.12 needs --collect-tokens-histogram only for this path.

"to": "modelplane_engine_tokens_total",
"labels": [{"name": "direction", "value": "output"}],
},
{"from": prompt, "part": "Count", "to": "modelplane_engine_requests_total"},

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Request count comes from the prompt histograms count. For vLLM, vllm:request_success_total summed over reason is the same number and is already mapped. That gives two sources for the same thing, and they can drift, for example on aborted requests. Is that intended? If yes, a short note on which one dashboards should use would help.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intended. They count the same finished requests through different hooks, and can differ slightly, as on aborted requests. The comment says dashboards count requests with modelplane_engine_requests_total, which every engine has, and use modelplane_responses_total only for how vLLM's requests ended.

Comment thread design/telemetry.md Outdated
of them describes neither. GPU series carry `gpu_uuid` and the node, which is all DCGM
carries a `phase` of `Prefill` or `Decode`, because the two do different work and an average
of them describes neither, and because both count every request they share. `role`
is the member's role in its engine, `Standalone`, `Leader` or `Worker`. GPU series carry `gpu_uuid` and the node, which is all DCGM

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: This line is not wrapped like the rest of the paragraph

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

if "lmsysorg/sglang:" not in image:
return []
problems = []
if "--enable-metrics" not in argv:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--enable-metrics not in argv is a substring check, so --enable-metrics-for-all-schedulers alone passes, even though that flag does not turn on /metrics by itself. Please match the flag as a whole token. For example, use re.search(r"(?<!\S)--enable-metrics(?:=\S+)?(?!\S)", argv), which still works for the qwen3-coder recipe that passes args as one shell string.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, fixed. _has_flag matches the flag as a whole token, bare or as flag=value, so --enable-metrics-for-all-schedulers alone no longer passes, and the qwen3-coder shell string still does.

image = container.image
argv = " ".join((container.command or []) + (container.args or []))
version = _version(image)
if "vllm/vllm-openai:" in image:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An image pinned by digest vllm/vllm-openai@sha256:... or with no v tag (:latest) skips the check silently ? is this okay?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not okay, fixed. A vLLM or SGLang image the validator can't read a version from, such as a digest or :latest, now fails with a message to pin a vX.Y.Z tag. The image match also covers @sha256 now. Every recipe already pins a v tag.

@negz negz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for turning this around so quickly. LGTM once you've addressed Chris's prefill/decode point on modelplane_responses_total.

I left some nits inline where the docs and comments explain more than they need to, or describe this change rather than the code. None of them block.

Comment thread docs/content/platform/telemetry.md Outdated
Comment on lines +44 to +46
- `sum by (le)` inside `histogram_quantile`, for a histogram. Each `modelplane_*`
histogram comes from one component with one set of buckets, so a quantile across
the whole fleet is sound.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I'd stop at "for a histogram". The rest reads as a response to the old mappings, and keeping it true is our job rather than something a reader needs to know.

Suggested change
- `sum by (le)` inside `histogram_quantile`, for a histogram. Each `modelplane_*`
histogram comes from one component with one set of buckets, so a quantile across
the whole fleet is sound.
- `sum by (le)` inside `histogram_quantile`, for a histogram.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comment thread docs/content/platform/telemetry.md Outdated
Comment on lines +301 to +304
Both engines' token counts are histograms, but their buckets differ between the
engines, and vLLM's also follow each deployment's `--max-model-len`, so Modelplane
exports only their sums and counts, as `modelplane_engine_tokens_total` and
`modelplane_engine_requests_total`, which add up correctly across any mix of engines.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this explains why the token counts aren't histograms, which only makes sense to someone who saw the previous version. The table already says what the two counters are, so I'd delete this paragraph and leave the change to the release note.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed.

Comment thread docs/content/platform/telemetry.md Outdated
Comment on lines +323 to +351
```yaml
apiVersion: modelplane.ai/v1alpha1
kind: MetricMapping
metadata:
name: vllm-kv-offload
spec:
metrics:
- from: vllm:kv_offload_store_bytes_total
to: modelplane_kv_offload_store_bytes_total
- from: vllm:kv_offload_load_bytes_total
to: modelplane_kv_offload_load_bytes_total
- from: vllm:kv_offload_store_time_total
to: modelplane_kv_offload_store_seconds_total
- from: vllm:kv_offload_load_time_total
to: modelplane_kv_offload_load_seconds_total
- from: vllm:kv_offload_cpu_cache_usage_perc
to: modelplane_kv_offload_in_transfer_ratio
- from: vllm:external_prefix_cache_hits_total
to: modelplane_external_prefix_cache_hits_total
- from: vllm:external_prefix_cache_queries_total
to: modelplane_external_prefix_cache_lookups_total
```

A store is the GPU writing KV to the tier and a load is the GPU reading it back, so
bytes over seconds is the transfer rate. `vllm:kv_offload_cpu_cache_usage_perc` is the
share of the tier's chunks being moved, not how full the tier is, and vLLM publishes
nothing that says how full it is. vLLM counts the tokens any KV connector supplies as
external prefix cache hits, so on a prefill/decode deployment those two counters also
count the KV a decode engine pulls from prefill.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this is now the largest new section, for metrics we decided not to map by default. The paragraph below it mostly explains the two we had doubts about. Could we trim the example to the four byte and second counters and drop that paragraph?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trimmed to the four byte and second counters, and dropped the paragraph.

Comment thread docs/content/platform/telemetry.md Outdated
than once per engine under `--enable-priority-scheduling`, once in total and once per
priority, and once per tensor-parallel rank under
`--enable-metrics-for-all-schedulers`. With either flag, a sum over them counts each
request more than once. Keeping only one of those series is tracked in #549.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: #549 won't render as a link on the docs site. Could we make it a full link to the issue?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, it's a full link now.

Comment thread docs/content/platform/telemetry.md Outdated
Comment on lines +493 to +517
For a question about tensor parallelism over PCIe, map DCGM's PCIe and SM fields
too:

```yaml
apiVersion: modelplane.ai/v1alpha1
kind: MetricMapping
metadata:
name: dcgm-interconnect
spec:
metrics:
- from: DCGM_FI_PROF_PCIE_TX_BYTES
to: modelplane_gpu_pcie_transmit_bytes_per_second
- from: DCGM_FI_PROF_PCIE_RX_BYTES
to: modelplane_gpu_pcie_receive_bytes_per_second
- from: DCGM_FI_PROF_SM_ACTIVE
to: modelplane_gpu_sm_active_ratio
- from: DCGM_FI_PROF_SM_OCCUPANCY
to: modelplane_gpu_sm_occupancy_ratio
```

DCGM samples PCIe traffic as a rate, in bytes per second, so read the two PCIe series
as they are and don't take `rate()` of them. The GPU Operator's metrics configuration
types them as counters, so on a cluster running it a Prometheus backend adds `_total`
to their names. GKE's managed exporter types them as gauges, and doesn't publish SM
occupancy.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: same as the offload section. I'd keep the mapping and the line about not taking rate(), and cut the rest, or link DCGM's docs.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kept the mapping and the line about rate(), and cut the rest.

Comment on lines +53 to +55
# The engines, which explain what the gateway measured. vLLM publishes these
# names from v0.9.2. Before it, vllm:gpu_cache_usage_perc stood for the KV cache,
# and there were no prefix cache counters.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: keep the floor, since the validator enforces it. The history of what came before reads like this PR's llama fix.

Suggested change
# The engines, which explain what the gateway measured. vLLM publishes these
# names from v0.9.2. Before it, vllm:gpu_cache_usage_perc stood for the KV cache,
# and there were no prefix cache counters.
# The engines, which explain what the gateway measured. vLLM publishes these
# names from v0.9.2.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

# Each engine's prompt and generation token histograms, taken as counters: the
# prompt and generated tokens of the requests it finished, and from the prompt
# histogram's count, the requests. A prompt's tokens include any the prefix
# cache served. The histograms themselves are dropped (see the module docstring).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: the sentence about dropping the histograms only means something to a reader who knew we used to export them. I'd cut it and keep the rest.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cut.

corrupts the sum for everyone. So each modelplane_* histogram has
exactly one source. The engines' token histograms can't meet that even alone:
vLLM sizes their buckets from --max-model-len, so two vLLM deployments
disagree. They leave as counters instead, a sum and a count having no buckets.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: "They leave as counters instead" reads as a reaction to the old mappings. Could this say what's true now?

Suggested change
disagree. They leave as counters instead, a sum and a count having no buckets.
disagree. So they're exported only as their sums and counts, which have no buckets.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comment on lines +7020 to +7021
# A disaggregated request runs on both engines, and each counts it, so a
# query needs the phase to count it once.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this repeats the case's reason and the comment in base.py. I'd drop it.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dropped.

Comment on lines +123 to +128
# The built-in MetricMappings in compose-serving-stack rename what an engine
# publishes, so an engine that publishes nothing, or something else, leaves its
# modelplane_* series empty with no error. Recipes are what users copy,
# so each one's engines are held to what the mappings read. The versions and
# flags below are the ones function/stacks/metrics.py in compose-serving-stack
# says its engine mappings need, so a change to either changes both.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: the module docstring already says this in brief. Could this be shorter? Something like:

Suggested change
# The built-in MetricMappings in compose-serving-stack rename what an engine
# publishes, so an engine that publishes nothing, or something else, leaves its
# modelplane_* series empty with no error. Recipes are what users copy,
# so each one's engines are held to what the mappings read. The versions and
# flags below are the ones function/stacks/metrics.py in compose-serving-stack
# says its engine mappings need, so a change to either changes both.
# Each recipe's engines must publish what the built-in mappings in
# compose-serving-stack's stacks/metrics.py read, so keep the two in step.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

dennis-upbound and others added 9 commits October 21, 2026 11:09
The built-in mappings renamed vLLM's and SGLang's prompt and generation
token histograms onto modelplane_request_input_tokens and
modelplane_request_output_tokens. The two engines bucket them
differently, and vLLM sizes its buckets from --max-model-len, so even
two vLLM deployments disagree. A quantile sums buckets by boundary, so
any mix of sources under one name gave a wrong answer that looked right:
a p90 of 460 tokens where every prompt was 150.

No single layout exists to rename them onto, so the histograms no longer
leave the cluster. Their sums and counts do, which have no buckets:
modelplane_engine_tokens_total{direction} and
modelplane_engine_requests_total. Those add up correctly across any mix
of engines and context lengths. The token-length distribution is the
cost: it isn't in the normalized engine metrics any more.

The engine_ prefix keeps these apart from the gateway's
modelplane_tokens_total in the design, so the two can't be summed
together by accident.

Fixes modelplaneai#523.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Dennis Ramdass <dennis@upbound.io>
Three recipes shipped engines whose metrics the built-in mappings can't
read, and nothing reported it: the collector drops what it doesn't
rename, so a modelplane_* series just never appears.

The llama-3.1-8b recipe pinned vLLM v0.7.3, whose V0 engine publishes
vllm:gpu_cache_usage_perc and prefix-cache hit-rate gauges instead of
the names the mappings read, and latency histograms with fewer buckets
than every later version. The laguna and qwen3-coder recipes ran SGLang
without --enable-metrics, so they served no /metrics at all, and
qwen3-coder's v0.5.10 also needs --collect-tokens-histogram for its
token histograms.

This moves llama-3.1-8b to vLLM v0.23.0, the version most recipes pin,
and adds the flags to both SGLang recipes. The llama recipe was served
on v0.23.0 on GKE with one L4. The docs manifest validator now fails a
recipe whose SGLang engine lacks those flags or whose vLLM image is
older than v0.9.2, so the next one is caught before it ships.

Fixes modelplaneai#524.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Dennis Ramdass <dennis@upbound.io>
A request the picker disaggregates runs on both a prefill and a decode
engine, and each counts it: the prefill engine gets the whole request
with its token limit set to one. Summed over a deployment, as the
telemetry docs say to sum anything counted, requests and input tokens
came to twice what callers sent. Nothing on a series told the two
engines apart except the engine's name, which the user picks, because
role holds the member's role and is Standalone on both.

compose-model-replica now copies an engine's phase onto a
modelplane.ai/phase pod label, and the collector carries it as phase,
part of the identity beside role. It's absent on a unified engine. The
decode engine sees every request with its whole prompt, because the
picker sends one to prefill only when it has enough uncached tokens, so
a query counts requests and tokens on phase!="Prefill". The telemetry
docs now say so. Served live with vLLM v0.23.0 on two L4s, 15 requests
summed to 30 without the filter and to 15 with it.

The design already gave disaggregated engine series a label for this,
calling it role. It's phase here because role was taken.

SGLang counts a running or waiting request more than once under
--enable-priority-scheduling and --enable-metrics-for-all-schedulers. No
recipe sets either, and a mapping can't keep one series out of several,
so the docs say so. modelplaneai#549 tracks letting it.

Fixes modelplaneai#525.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Dennis Ramdass <dennis@upbound.io>
The built-in mappings are written against upstream source, and nothing
checked them against a running component. modelplaneai#523, modelplaneai#524 and modelplaneai#525 all came
from that gap: a name an engine version doesn't publish leaves a
modelplane_* series empty with no error, and histograms with different
buckets under one name give wrong quantiles.

This records what nine component versions publish, read from pods
serving traffic on GKE: the gateway, the picker, GKE's DCGM exporter,
the four vLLM versions the recipes pin, v0.9.2, v0.23.0, v0.25.1 and
v0.27.1, and SGLang v0.5.10 and v0.5.12. hack/scrape_fixture.py turns a
pod's /metrics into a fixture of every name it publishes and each
histogram's bucket boundaries. A test checks that every name each
mapping reads is in its scrapes, and that each modelplane_* histogram
has one source with the same buckets in every version. Against the
mappings before modelplaneai#523 it fails on modelplane_request_input_tokens and
modelplane_request_output_tokens.

SGLang publishes its retraction counter only after the first retraction,
which a lightly loaded pod never does, so that one name is checked
against SGLang's source and listed on its cases. A version bump needs a
new scrape and a case.

Towards modelplaneai#476.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Dennis Ramdass <dennis@upbound.io>
vLLM's KV offload tier and speculative decoding, and DCGM's PCIe and SM
fields, had no mappings, so every operator running these features wrote
the same MetricMapping by hand.

The built-in mappings cover what every deployment needs, from engine
versions Modelplane tests. These are tuning metrics for features a team
turns on, published only while the feature runs, so they stay out of the
built-ins and the docs carry a MetricMapping for each: KV offload and
the external prefix cache counters, and DCGM's PCIe and SM fields, in
the telemetry docs, and speculative decoding in the Qwen3-8B recipe that
drafts. vLLM counts any KV connector's tokens as external prefix cache
hits, so on a prefill/decode deployment those also count KV pulled from
prefill, which the docs say. DCGM_FI_DEV_FB_FREE is the other half of
GPU memory, which every exporter publishes, so it's built in beside
FB_USED.

DCGM samples PCIe traffic as a rate, but the gpu-operator's metrics CSV
types it a counter and GKE's managed exporter a gauge, so a Prometheus
backend adds _total to its name on one cloud and not the other. The
documented mapping says so, rather than the collector naming an
exporter's fields.

Towards modelplaneai#547.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Dennis Ramdass <dennis@upbound.io>
vLLM counts the requests it finishes by finished_reason, and nothing
mapped it. Without it there was no engine-side view of truncation, and
no way to see that a client's disconnect never reached the engine: a
request whose client left should end as abort, and when abort stays at 0
the engine is generating output nobody reads.

vllm:request_success_total becomes modelplane_responses_total, the name
the design gives it, with finished_reason renamed to reason. vLLM
publishes stop, length and abort at v0.9.2, and error and repetition as
well from v0.23.0. SGLang has no such counter. The design said reason
held tool_calls, which no engine reports; it now lists what vLLM does.

Towards modelplaneai#476.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Dennis Ramdass <dennis@upbound.io>
llm-d's sidecar caps the prefill request at one token, so a prefill
engine reports finished_reason="length" for every request it runs, and
modelplane_responses_total{reason="length"} counted every request on a
prefill/decode deployment. The telemetry docs now read responses, like
requests and tokens, on phase!="Prefill".

The validator matches SGLang's flags as whole tokens, so
--enable-metrics-for-all-schedulers no longer passes for
--enable-metrics, and it rejects a vLLM or SGLang image it can't read a
version from, such as a digest or :latest, instead of skipping it.

metrics.py says why token counts come from the histograms' sums rather
than the engines' plain token counters, and which request count a
dashboard reads. The docs drop the explanation of why token counts aren't
histograms, and trim the offload and DCGM mappings to what a reader needs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Dennis Ramdass <dennis@upbound.io>
A canary run that compares an SGLang recipe with a vLLM one can't see three
things on SGLang. Its first-token time has no modelplane_* name, so a first
token slower than the gateway's 60 s top bucket can't be read at all. Its
errors don't show by status, because SGLang publishes no counter of how a
request ended. And nothing shows the requests SGLang has open, which is how an
abandoned request that keeps decoding gets noticed.

SGLang publishes all three at v0.5.10 and v0.5.12. sglang:time_to_first_token_seconds
becomes modelplane_sglang_ttft_seconds, a name no other source shares, because its
buckets don't match vLLM's and one name per histogram source is what keeps a
quantile sound. sglang:http_responses_total becomes
modelplane_engine_responses_total, with its status_code folded into the gateway's
ok, client_error and server_error for the codes an OpenAI-compatible server
returns, so the label stays bounded. sglang:http_requests_active becomes
modelplane_engine_requests_active. Both keep SGLang's endpoint label, since SGLang
counts /health and /metrics too.

SGLang's spec_accept_rate and spec_accept_length stay unmapped: spec_accept_rate
changed what it measures between the two versions.

The fixture test checks the new names against both SGLang scrapes, and the
histogram test confirms the first-token buckets match across them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Dennis Ramdass <dennis@upbound.io>
After rebasing onto modelplaneai#548, test_mappings_scraped failed: the new
modelplane-gateway-proxy mapping, which reads envoy_cluster_upstream_rq_active,
had no scrape to check it against.

envoy-v1.38.4.json is the /stats/prometheus of the proxy Envoy Gateway v1.8.4,
the version the stack pins, runs for a fleet gateway that had served requests.
Envoy names some stats after the route they count, so those carry a deployment's
name and are left out of the fixture. envoy_cluster_upstream_rq_active is not one
of them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Dennis Ramdass <dennis@upbound.io>
Rebasing onto main put main's lws_name, grove_pcs_name and dns_1035_label tables
between TELEMETRY_LABELS_CASES and test_telemetry_labels, which the function test
style check rejects (MPT101, MPT103). Move the telemetry table and its helpers
down to its test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Dennis Ramdass <dennis@upbound.io>
@dennis-upbound
dennis-upbound merged commit 9022577 into modelplaneai:main Oct 21, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants