fix(observability): repair admin Command Center (summary/trends/top/rollups) - #655
Merged
Conversation
The `telemetry_trend_rollups` materialized view backing the admin Command Center's 7d/30d trend charts could never be refreshed. Its unique index was built on an expression (`COALESCE(route, '')`), and PostgreSQL only accepts a plain-column unique index as the row-matching index for `REFRESH ... CONCURRENTLY`. So the hourly refresh failed every cycle with "cannot refresh materialized view concurrently", the view stayed empty since introduction, and 7d/30d charts showed no data. Live (1h/6h/24h) ranges were unaffected — they read `telemetry_metric_samples` directly. - Migration recreates the view with `route` as a non-null column (`COALESCE(labels->>'http.route', '')`) and a plain-column unique index on `(day, metric_name, scope, route)`, which CONCURRENTLY accepts. - The refresh job now falls back to a blocking `REFRESH` if the concurrent refresh can't proceed, so rollups self-heal instead of silently going stale. Verified against the production schema/data in a rolled-back transaction: the recreated view populates (222 rows) and the index is plain-column. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JDEzVG39gGZFozPCndbSid
The Command Center showed no data once telemetry accumulated — the endpoints errored rather than returning empty: - summary (500) and top-routes/top-errors: their SUM(value_count) over the bigint column returns NUMERIC, which failed to decode into i64. Cast the aggregates back to ::bigint. (Invisible on an empty DB: SUM over zero rows is NULL, which decodes fine — so it only broke in production with data.) - trends (400): the endpoint used axum's Query (serde_urlencoded), which can't parse repeated `metric=` params into a Vec. Switch to axum_extra's Query (serde_html_form); enable the axum-extra `query` feature. - 7d/30d rollups: the materialized view's SUM columns are also cast to ::bigint so the rollup trend query decodes. Adds an integration regression test that seeds samples and asserts the summary/top-routes/top-errors/trends queries decode with data present — the coverage gap that let this ship (previous tests ran against an empty DB). Verified against production data (types now decode as bigint; migration populates the matview). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JDEzVG39gGZFozPCndbSid
Detair
changed the title
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.
Problem
The admin Command Center showed no data at all — reported live as empty vital signs and charts. The browser console showed the endpoints erroring, not just returning empty:
GET /api/admin/observability/summary→ 500 "Database error"GET /api/admin/observability/trends?...→ 400 (body "Failed to …", not JSON)Root causes (three distinct bugs)
SUM(bigint)→NUMERICdecode failure (summary 500, top-routes/top-errors 500).value_countisbigint;SUM(value_count)returnsNUMERIC, which can't decode into the Rusti64the code expects. This was invisible on an empty database —SUMover zero rows isNULL, which decodes fine — so it only broke in production once telemetry accumulated. Fixed by casting the aggregates back to::bigint(insummary_error_and_request_counts,query_top_routes,query_top_errors, and the rollup materialized view'stotal_count/error_count).Repeated
metric=params couldn't be parsed (trends 400). The trends endpoint'smetric: Vec<String>usedaxum::extract::Query(serde_urlencoded), which doesn't support repeated keys → 400Failed to deserialize query string. The client always sends 6metric=params, so trends never worked. Switched toaxum_extra::extract::Query(serde_html_form) and enabled the axum-extraqueryfeature.7d/30d rollups additionally empty. The
telemetry_trend_rollupsmaterialized view could neverREFRESH … CONCURRENTLYbecause its unique index was on an expression (COALESCE(route,'')), which PostgreSQL rejects. Recreated the view withrouteas a plain non-null column + a plain-column unique index; the refresh job now falls back to a blocking refresh if a concurrent one can't proceed.Why it wasn't caught
These observability query functions had zero test coverage, and the one failure mode (SUM decode) only appears with data. Added
server/tests/integration/observability_queries.rs, which seeds samples and asserts summary/top-routes/top-errors/trends all decode.Verification
SUM(...)::bigintaggregates now report typebigint; the recreated matview populates (222 rows). Applied an immediate one-off refresh on beta so 7d/30d has data now.cargo build, libclippy -D warnings,fmt --checkclean; integration test compiles. (Fullsqlx::testrun happens in CI — local container networking is broken in this env.)🤖 Generated with Claude Code
https://claude.ai/code/session_01JDEzVG39gGZFozPCndbSid