feat(chart): alert when an engine is up but not serving

[Tanguille]

InferenceServiceDown only catches a dead process. An engine can hold its port open, pass /health and report Ready while admitting almost no work: requests queue, KV cache sits partly free, prompt throughput reads zero. Nothing in the chart caught that.

Observed on a single-GPU vLLM deployment: three hours of 1/1 Running, /health 200, no chart alert, while the engine sat at Running: 1, Waiting: 5 with 36% KV free and 0.0 tok/s prompt throughput.

New llmkube-serving group

alert	expr basis	runtimes	default
InferenceAdmissionStalled	queued requests with none running	vllm + sglang	on
InferenceQueueTimeHigh	mean queue wait > threshold	vllm + sglang	on
KVOffloadLoadSlow	mean async KV-offload load > threshold	vllm	off

KVOffloadLoadSlow names the likely cause (a slow, non-preemptible async KV load gates admission for everything behind it); InferenceQueueTimeHigh only reports the symptom. It's off by default since OffloadingConnector is upstream-experimental and its series are absent without a KV connector configured, same pattern as the existing gpu.memoryPressure toggle.

Also fixes a recording rule that only covered SGLang

vLLM exports vllm:request_queue_time_seconds, which charts/llmkube/dashboards/vllm-dashboard.json already queries. Both queue-wait recording rules now use the same or alternation as their e2e_request_latency and ttft_seconds neighbours.

Thresholds

Values-driven, set well above the healthy band. Measured on a healthy single-GPU vLLM deployment:

	healthy	default threshold	headroom
mean queue wait	1.4s	30s	~21x
mean async offload load	0.9s	10s	~11x

Validation

• helm unittest: 83/83 passing, including 6 new cases.
• Rendered expressions run against a live VictoriaMetrics instance: all parse and return the expected series with service / namespace / runtime labels. Confirmed they aren't silent no-ops by lowering thresholds to 0 and checking they then match, the #1223 failure mode.

Caveat

Thresholds are backtested against one incident on one deployment. Metric names are stock and rule shapes are runtime-agnostic, but treat the numbers as reasonable defaults rather than fleet-validated. Happy to adjust, or gate serving.enabled off by default for a first release.

Assisted-by: OpenCode (helped draft this PR; I reviewed the final change and stand behind it.)

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[Defilan]

Thanks for this, and for the writeup. The gap is real: InferenceServiceDown only catches a dead process, and an engine that holds its port open while admitting nothing is exactly the case the chart has been blind to. The recording-rule half of this PR is a clean fix and I want that regardless of what happens to the rest.

I reviewed the alert expressions closely, ran helm unittest (83/83 pass), rendered and parsed the templates, and checked every metric name against internal/metrics/testdata/*-metrics.txt. The blocking problem is that I do not think the two on-by-default alerts can fire for the incident described above.

Blocking

1. InferenceAdmissionStalled cannot match the motivating incident.

The PR body states the observed state was Running: 1, Waiting: 5. The expression requires vllm:num_requests_running == 0. With one request running that operand is an empty vector, the and yields nothing, and the alert never enters pending. A partial stall, which is what three hours at Running: 1 is, needs a ratio or a "waiting high while running pinned at or below a small constant for N minutes" form.

2. InferenceQueueTimeHigh goes silent in exactly the total-stall case.

vllm:request_queue_time_seconds and sglang:queue_time_seconds are observed at request completion. An engine that admits nothing completes nothing, so increase(..._count[15m]) stays 0, increase(..._sum[15m]) stays 0, and the expression evaluates 0/0 = NaN. NaN > 30 is false, no series exists, nothing fires. A three hour hard stall produces zero pages from this rule. That is the same silent-no-op class as #1223 that GPUMetricsMissing and ControllerMetricsMissing were added to close.

Taken together, neither new on-by-default alert detects the failure the PR was written for. Worth validating the corrected expressions against the recorded incident data if you still have it.

3. The serving group defaults on, but depends on labels only a disabled-by-default PodMonitor injects.

values.yaml has prometheus.inferencePodMonitor.enabled: false (line 257) while the new rules.serving.enabled is true. The three new rules group by service and runtime, which are applied only by inference-podmonitor.yaml's relabelings. Under any foreign scrape config, sum by (service, namespace, runtime) collapses every vLLM pod in a namespace into one empty-label series (so the mean queue wait becomes a fleet average that hides the single stalled engine), and the descriptions render <no value> in <namespace>.

The chart already handles this for ControllerMetricsMissing, gated with {{- if .Values.prometheus.serviceMonitor.enabled }} and the comment "Gated on the chart owning the scrape target" (line 167). The serving group should follow that convention.

4. No absent() companion for the new group.

All three rules are presence-dependent: X > 0 and Y == 0 and increase(sum)/increase(count) > T all produce an empty vector when the series vanish. If the PodMonitor selector stops matching after a pod label rename, or the job label drifts, or scraping just breaks, the operator sees no alert and it is indistinguishable from a healthy fleet. The chart established the counter-pattern twice for this reason (GPUMetricsMissing line 74, ControllerMetricsMissing line 171, both citing #1223/#1356). Something like absent(vllm:num_requests_running) and absent(sglang:num_running_reqs) is missing.

Should fix before merge

5. No minimum-sample guard, so one slow request pages critical.

A freshly rolled vLLM pod queues its first request 45s behind weight load and CUDA graph capture, then serves nothing else. Over the next 15m, increase(_sum[15m]) is about 45 and increase(_count[15m]) is about 1, so the mean reads 45 > 30 continuously, satisfies for: 15m, and fires a critical alert on an idle healthy service. The usual guard (and sum by (...) (increase(..._count[15m])) > N) is absent from both this rule and KVOffloadLoadSlow, which has the same shape over 30m.

6. InferenceQueueTimeHigh is service-aggregated while the alert above it is per-pod.

With replicas: 4, one pod stalled at a 200s mean queue wait and three healthy at 1.4s, the ratio weights by completed-request count. The healthy pods complete nearly all requests, so the service-level mean lands around 2 to 3s, far under threshold, and the stalled replica is invisible. Adding pod to the by clause keeps per-replica resolution and matches InferenceAdmissionStalled directly above it.

7. for: 10m over an instantaneous gauge resets on any single scrape.

An engine that drains one queued request every few minutes but is otherwise wedged shows num_requests_running == 1 at roughly one evaluation in six. Each one empties the vector and resets the pending timer, so the alert never reaches 10 continuous minutes. avg_over_time(vllm:num_requests_running[10m]) < 1 or a max_over_time form survives the flap.

8. severity: critical is inconsistent with the rest of the chart.

Existing criticals are hard-down or damage conditions: up == 0 for InferenceServiceDown and ControllerDown, and GPU over-temperature. A 30s mean queue wait is a saturation signal a busy but healthy batch deployment can hit legitimately. KVOffloadLoadSlow in the same new group is correctly warning. Combined with finding 5, this pages out of hours on an idle service.

9. One test passes vacuously.

tests/prometheusrule_test.yaml:250, "should omit the KV-offload alert by default". I ran that assertion with prometheus.prometheusRule.rules.serving.enabled: false so the JSONPath resolves to nothing, and it still passes. helm-unittest treats notMatchRegex over an empty path as satisfied, so if a future edit drops or renames the serving group the test stays green. The same file already solved this for the GPU group with lengthEqual (line ~137); the serving tests want count: 2 by default and count: 3 with kvOffload enabled.

Worth addressing, non-blocking

10. Alert expressions are the one PromQL surface in the repo with no name guard. internal/metrics/dashboard_sync_test.go validates dashboards and marked doc regions against the metric fixtures precisely to stop the #786/#1223/#1226 class, but it only reads - record: lines from the chart, never - alert:. vllm:kv_offload_lookup_async_delay_seconds is real upstream (kv_connector/v1/offloading/metrics.py) but is in no fixture, so a typo or rename in any of the six names here ships a rule that silently never fires. The durable fix is extending that extractor to alert exprs; happy to take that as a follow-up rather than block you on it.

11. Partial runtime coverage, unguarded. TestChartRestartRuleCoversEveryBackend enforces that the restart recording rule matches every backend's ContainerName(). The new alerts enumerate vllm/sglang/llamacpp only; PersonaPlexBackend, GenericBackend and LlamaCppRouterBackend get no coverage and no comment explaining the omission (only TGI's is explained). A seventh runtime will silently go unmonitored.

12. The queue-metric alternation is now written in two places. The PR widens the p95/p50 recording rules to rate(vllm:...) or rate(sglang:...), and twelve lines earlier hand-writes the same two-runtime alternation over _sum/_count. A third runtime means editing four expressions instead of two. Alerting on the existing llmkube:inference:queue_time_seconds:p95_5m recording rule collapses this to one runtime list and gives a percentile, which is also more robust to the single-slow-request case in finding 5.

13. charts/llmkube/README.md values table is not updated. Lines 145 to 152 document every prometheus.prometheusRule.rules.* parameter. The four new keys, including an on-by-default group and two tunable thresholds, are undocumented.

14. The vLLM dashboard now duplicates the recording rule. vllm-dashboard.json:415 computes histogram_quantile(0.95, ...rate(vllm:request_queue_time_seconds_bucket...)) at query time, while sglang-dashboard.json:461 reads llmkube:inference:queue_time_seconds:p95_5m. Now that the recording rule covers vLLM, switching that panel closes the gap and drops the raw-bucket cost per refresh.

15. The p50 recording rule has no test. Both p95 and p50 changed, but "should cover both runtimes in the queue-wait recording rule" asserts only against p95. A later revert or typo in p50 leaves the suite green and the vLLM p50 panel empty.

On the caveat

You offered to gate serving.enabled off by default for a first release. I would rather fix findings 1 and 2 and ship it on, since an alert group that ships off tends to stay off. The thresholds themselves I am not worried about; they are values-driven and the headroom is documented.

Findings 1, 2 and 9 are the ones I would want addressed before merge. Happy to pair on the expression rewrite if that is useful, and if you would rather split the recording-rule fix into its own PR I will merge that side quickly.