Skip to content

refactor(chart): apply three dashboard conventions across every board - #1368

Open
Tanguille wants to merge 2 commits into
defilantech:mainfrom
Tanguille:refactor/dashboard-conventions
Open

refactor(chart): apply three dashboard conventions across every board#1368
Tanguille wants to merge 2 commits into
defilantech:mainfrom
Tanguille:refactor/dashboard-conventions

Conversation

@Tanguille

@Tanguille Tanguille commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

First of four. The three layers stacked on this branch (operator-health row, SGLang panels, llama.cpp board) follow one at a time as this merges, so each arrives as its own reviewable diff rather than one large change.

Adds no panels and no features: it exists so those layers inherit one convention instead of each inventing their own.

What

Convention Applied to
min: 0 / max: 1 on percentunit ratios against a hard limit llmkube-quota ids 1, 5
noValue: "0" where the outer PromQL is sum()/count() model-router 1/2/4, sglang 1/6/8/14/15, vllm 1/6
palette-classic-by-name where series vary with a template variable 30 timeseries panels across sglang, vllm, llmkube-slo

Plus TestDashboardConventions in internal/metrics/dashboard_sync_test.go, so a future dashboard cannot silently violate any of the three. It walks fieldConfig.defaults per panel, pairing each panel with its own target exprs, and scopes the rules structurally rather than with an exemption list: error budgets (1 - (...)) and burn-rate thresholds (14 * (...)) fall out of rule 1 by shape, and sum(a)/sum(b) ratio-of-sums falls out of rule 2 because the sum() match does not close at the end of the string.

Why

palette-classic assigns colour by series order, so filtering $service repaints the survivors and a reader who learned "service X is green" is misled. Ratio panels without a max autoscale, so a 2% blip renders identically to saturation. count() over an empty vector reads "No data" on a fresh cluster instead of 0.

Not in scope

Legend calcs (["mean","max"] is the existing convention and stays), fillOpacity (a drawStyle: bars panel legitimately needs a high fill), colours on fixed series sets, and config/grafanas two boards, which predate this pass.

Evidence

  • Audited all 7 boards, not only the 5 changed: amd-gpu-observability and llmkube-inference have no qualifying panels.
  • Diffing both trees with min/max/noValue/color stripped is byte-identical on all 7 files, so no legend, fill, panel-id or formatting churn.
  • helm unittest charts/llmkube 60/60, go test ./internal/metrics/... ok, all dashboard JSON valid.
  • The test was falsified rather than assumed: removing one noValue makes it fail naming the exact panel, restoring makes it pass. It has already caught six real violations in the layers stacked on top, two of them in a commit that claimed to have fixed them.

Happy to split or reorder if you would rather see the test separately from the JSON changes.

Live validation

Chart rendered from all four branches merged, applied to a production cluster (AMD R9700, SGLang v0.5.16 + three llama.cpp services) behind a suspended HelmRelease, then reverted and proved clean.

33 of 37 panels return live data. The four empties: two are histogram_quantile over an idle operator, one was the queue-wait panel awaiting its recording rules (which materialised on apply at p95 18.0s / p50 1.0s), and one was a real defect in the llama.cpp branch, now fixed there.

Revert proof: recording rules back to the 5 the release ships, dashboards back to the 2 Flux owns, zero suspended Flux objects.

…eries color

Three conventions made consistent across every dashboard, no panels or
features added:

- percentunit ratio-against-a-hard-limit panels get min:0/max:1 so they
  read against the limit instead of autoscaling (llmkube-quota GPU/VRAM
  utilization).
- sum()/count() queries that can return an empty vector on a fresh
  cluster get noValue:"0" instead of "No data" (model-router request and
  fail-closed rates; sglang/vllm throughput, queue depth, and pool
  panels).
- Panels whose series vary with a template variable (service/slo) use
  color.mode palette-classic-by-name so filtering doesn't repaint
  survivors (every timeseries panel in sglang-dashboard, vllm-dashboard,
  and the llmkube-slo over-time/burn-rate panels).

amd-gpu-observability and llmkube-inference have no qualifying panels
and are untouched. model-router's own $router variable isn't wired into
any query, so its series aren't template-driven and its colors are
left alone.

Signed-off-by: Tanguille <91473554+Tanguille@users.noreply.github.com>
…Conventions

Walks fieldConfig.defaults per panel (dashboardPanels), not the flat
expr list dashboardQueries returns, so a rule can see a panel's unit,
color and target queries together. Table-driven over three rules,
each scoped to timeseries panels (stat/gauge color by threshold,
tables have no per-series color or axis, so none of the three read on
them the way they do on a graph):

- percentunit + a bare metric/metric division -> min:0/max:1. Error
  budgets ("1 - (...)") and burn-rate reference lines
  ("14 * (1 - $objective/100)") are excluded structurally: more than
  one '/' or any '*'/'+' in the expr disqualifies it.
- sum()/count() as the outer operator -> noValue:"0". A ratio-of-sums
  like sum(a)/sum(b) starts with "sum(" too, but its outer op is the
  division; the sum() match must close at the very end of the trimmed
  expr to count, which excludes it without an explicit list.
- a template variable inside a {...} label matcher ->
  color.mode:"palette-classic-by-name". model-router's $router isn't
  referenced in any query, so it correctly stays unmatched.

Scoped to charts/llmkube/dashboards only: config/grafana's standalone
pair predates the convention pass and was never touched by it.

Passes clean on the current tree, and a targeted sanity check
(stripping min/max from one panel) confirmed it fails when a
convention is actually violated.

Signed-off-by: Tanguille <91473554+Tanguille@users.noreply.github.com>

@Defilan Defilan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Tanguille, this is one of the cleaner refactors I've seen, thank you. Zero PromQL touched (I diffed it: 0 changed expr lines), the key-change tally matches your description exactly, and the guard test is genuinely falsifiable, it already caught 41 real inconsistencies in the pre-PR dashboards. One thing before I approve: please add the AI-assistance disclosure line to the PR body per CONTRIBUTING (Assisted-by: <tool> (what it did; what you verified)). Add that and I'll approve and merge this one first, since #1369/#1370/#1371 all stack on it and will rebase onto it.

@codecov

codecov Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants