fix(api): /metrics/facet 500 on null-kind rollups; log every 5xx; label by node (JEF-494)#123
Merged
thejefflarson merged 1 commit intoJul 23, 2026
Conversation
…el by node (JEF-494) The meta query in `metric_facet` decoded `metric_series_rollups.kind` as a non-nullable `String`, but that column is nullable. For metrics whose latest meta row is a rollup with `kind IS NULL` (e.g. k8s.container.restarts, system.cpu.load_average.5m), sqlx failed with "unexpected null" → 500, fast, with no Postgres error — breaking the /metrics/:name chart page. Decode `kind` as `Option<String>` (a null kind just means "unknown"), matching how the response already models it. `internal()` mapped errors to 500 but never logged, so the decode failure was invisible in watcher's own logs. It now logs every internal error at ERROR; the active per-handler tracing span carries the route. facetLabels now prefers node/host/container identity keys so node/host metrics are labeled by the node/container they describe rather than by whichever varying attribute (e.g. the reporting collector agent) happened to be discovered first. Tests: regression that a null-kind gauge rollup facets with 200; a unit test that `internal()` both returns 500 and logs the error; UI tests for the label ordering. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JbrmfzHsTMMzPaSrUkZgWo
thejefflarson
deleted the
thejefflarson/jef-494-apimetricsfacet-500s-on-certain-metrics-breaks-the-metric
branch
July 23, 2026 02:29
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes JEF-494.
The 500 (root cause)
GET /api/metrics/facet?name=<m>500'd fork8s.container.restartsandsystem.cpu.load_average.5m. The metric-meta query decodedmetric_series_rollups.kindas a non-nullable RustString, but that column is nullable (unlikemetrics.kind). For a metric whose most-recent meta row is a rollup withkind IS NULL, sqlx failed witherror occurred while decoding column 0: unexpected null→internal()→ 500. Fast, and with no Postgres error, exactly matching the reported symptom. It's the only non-Optiondecode in the whole handler; non-finite floats already serialize tonull(ruled out NaN/Infinity), and jsonb re-serialization can't fail — so the metakinddecode was the sole possible failure.Fix: decode
kindasOption<String>(a null kind just means "unknown"), matching howFacetResponse.kindalready models it. The SQL is unchanged and stays parameterized.Every 5xx is now logged
internal()returned(500, msg)but never logged — which is why this 500 produced zero server-side error logs on an observability tool. It now emitstracing::error!for every internal error; each handler's#[tracing::instrument]span carries the route.Node-vs-agent series labels (UI-only)
facetLabelstook the first ≤3 varying keys in discovery order, which could put a collector/agent key ahead of the node. It now prefers node/host/container identity keys (k8s.node.name,host.name,k8s.container.name, …) so node/host metrics are labeled by the node/container they describe. The node identity is present in these metrics' attrs (k8s.node.name/k8s.container.name), so this is a UI-only fix — no../clustercollector-config follow-up needed.Tests
facet_gauge_with_null_kind_rollup_returns_200(smoke) — seeds the exact failing shape (null-kind gauge rollup, no raw row); fails with 500 before the fix, 200 after.internal_logs_the_error_and_returns_500(unit) — captures the tracing output and assertsinternal()both returns 500 and logs the error at ERROR.metricLabels.test.ts— node key leads ahead of a discovered-first agent key; uid/id keys stay suppressed.Checks
cargo fmt,cargo check,cargo clippy --all-targets(clean),cargo test --locked— 54 passed.npm run build,npm run lint,npm test— 18 passed.🤖 Generated with Claude Code
https://claude.ai/code/session_01JbrmfzHsTMMzPaSrUkZgWo