feat(reliability-dashboard): add control-plane reconciliation metrics (Hypershell 286 )

[kdoberst]

Summary

• Problem: The reliability dashboard exposes API health but does not show whether control-plane reconciliation is failing, retrying, lagging, or leaving resource status stale.
• Fix: Adds Prometheus-backed control-plane reconciliation metrics, a protected BFF endpoint with 24-hour hourly trends, and dashboard summary/trend widgets for reconciliation health. It also aligns API rate/error units and precision with the dashboard contract.
• Alternatives considered: None

Breaking change? No

Tracking

Fixes HYPERSHELL-286

Specs and other PRs

Related specs and dependent PRs

• specs/platform/control-plane-reconciliation-metrics.spec.md => defines reconciliation metric types, bounded labels, PromQL, BFF response behavior, and verification requirements.
• specs/web-console/reliability-dashboard.spec.md => extends the reliability dashboard contract with reconciliation sources, widgets, layouts, and failure handling.
• specs/platform/api-reliability-metrics.spec.md => aligns API request-rate units and display precision used by the expanded dashboard.
• packages/operational-dashboard-ui/DATA_SOURCES.md => documents the new BFF source and metric mappings.
• No dependent PRs identified.

Components changed

• components/control-plane
• components/web-console
• packages/operational-dashboard-ui
• specs/platform
• specs/web-console

Changes with impact

• ⚠️ MEDIUM: Adds four control-plane Prometheus metrics and records reconciliation failures, retries, processing lag, and stale-resource status without exporting unbounded resource identifiers as metric labels.
• ⚠️ MEDIUM: Adds the authenticated GET /api/metrics/control-plane-reconciliation BFF contract; required current-value query failures return 502, while optional lag and historical-series failures remain partial.
• ⚠️ MEDIUM: Expands the reliability dashboard layout and summary from three API metrics to API plus reconciliation metrics, including responsive two-column and mobile layouts.
• ✅ LOW: Updates API request-rate and error-rate display units/precision and adds localized labels, fixtures, and coverage for the new dashboard data flow.

Verification

• Unit/integration tests created or updated
• Error paths considered and addressed
• Code changes match spec or acceptance criteria
• Code changes match Jira

Screenshots / video

Questions for discussion

• None

---

Details

Technical details (for humans and bots)

• The control plane registers failure and retry counters, a seconds-based reconciliation-lag histogram with explicit buckets, and a stale-resource gauge. Resource IDs are retained only in process memory to maintain a bounded metric label set.
• Reconciliation lag is recorded when the existing reconciliation span closes; retry counts are recorded when the watcher schedules a retry; stale-resource state is updated during gateway reconciliation.
• The BFF aggregates Prometheus data using increase for 24-hour and hourly counter values, histogram_quantile(0.50, ...) for lag, and a summed gauge for stale resources. Historical queries use the rolling 24-hour range with hourly samples and omit invalid/unavailable series instead of converting them to zero.
• The browser continues to access metrics only through same-origin BFF routes. Dashboard adapters merge the API and reconciliation sources, preserve partial failures, and omit unavailable values rather than rendering zero fallbacks.
• The default reliability layout includes reconciliation failure, retry, lag, and stale-resource widgets, with localized labels and responsive stacking behavior.
• Added or updated tests cover metric source mapping, PromQL and range-query behavior, required versus optional failures, adapter behavior, widget rendering, layouts, and unavailable metrics.

[coderabbitai]

Important

Review skipped

Auto reviews are limited based on label configuration.

🚫 Excluded labels (none allowed) (2)

• do-not-merge/work-in-progress
• do-not-merge/hold

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Advanced

Run ID: cab5d52d-9f54-4ddc-9844-bc9be31d2144

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

_{Comment @coderabbitai help to get the list of available commands.}

[amber-review-bot]

Amber review

Status: Complete

View the submitted review.

[amber-review-bot]

Verdict

This revision carries all fixes from the prior review into the current head: the stale gauge now serializes store/delete/count/record under staleResourcesMu, carries the hypershell.cluster_id label with max-by-cluster BFF dedup, the lag P50 is optional so an idle/fresh histogram no longer 502s the whole batch, and Go + BFF route/unit coverage now satisfies CRM-005. No new in-code defects; the only open items are cross-PR design decisions that need maintainer coordination.

Findings

No new findings this revision. The committer addressed the previous concerns (see below).

Cross-PR coordination

Two independent efforts change contracts this PR now depends on; each needs a maintainer decision plus a defined merge order.

• Control-plane cluster identity source (#362). This PR threads cfg.ClusterID into NewGatewayReconciler (cmd/hypershell-controller/main.go, reconciler.go:361,416) and uses it as the hypershell.cluster_id label on the stale gauge (metrics.go:317-319). #362 deletes the ClusterID config field and HYPERSHELL_CLUSTER_ID env entirely (internal/config/config.go) and re-sources identity from the registration-resolved cluster_id (RegisteredClusterIDForSubject), already wiring a registered clusterID into the reconcilers. If #362 lands first, this PR stops compiling on cfg.ClusterID and the stale-gauge label must be rewired to the registered identity. Maintainers should pick the canonical identity source and sequence the two merges so the label reads it correctly.
• Canonical convergence/staleness signal (#151, #200). This PR adds SetResourceStatusStale(...) into GatewayReconciler.Handle and defines staleness as GetReleaseId() != GetObservedReleaseId() (reconciler.go:445-446). #151 modifies the same Handle to re-key convergence on a new generation/observed_generation primitive (converged = observed_generation == generation, with the control plane writing observed_generation), and #200 defines the canonical control-plane reconciliation contract those generations implement. Two divergent definitions of "converged/stale" would ship in the same reconciler. A maintainer should choose the canonical convergence signal, decide whether this stale metric should key on the generation model rather than release IDs, and order the merges accordingly.

Previous concerns

• [Major] Reconciliation-lag P50 could 502 the whole endpoint on idle/fresh control planes (r4098852159) - addressed. The lag P50 is the only query run with optional = true (metrics-control-plane-reconciliation.ts:105); queryInstant returns undefined for a non-finite/invalid optional sample instead of throwing (:59-62); the lag field is omitted via ...(lag === undefined ? {} : {...}) (:123); and bff/test/metrics-control-plane-reconciliation.test.ts asserts the required counts survive an idle NaN/empty lag histogram.
• [Major] Feature lacks the tests required by CRM-005 (r4098852165) - addressed. internal/otel/metrics_test.go:75-124 (TestReconciliationMetrics) covers the failures/retries counter types, the hypershell.reconciliation.lag explicit bucket bounds, and the stale gauge's hypershell.cluster_id label plus the SetResourceStatusStale store/store/delete dedup count; dashboard-control-plane.test.ts asserts the reconciliation mapping; and the route-level 502 path is now covered by bff/test/metrics-control-plane-reconciliation-route.test.ts:65-76, which drives a failing upstream and asserts 502 + {error: "Metrics unavailable", statusCode: 502}.
• [Minor] Redundant map() per metric in mapControlPlaneReconciliationResponse (r4098852172) - addressed. Each trend is computed once into a local (failuresTrend/retriesTrend/lagTrend/staleTrend) and reused in both the guard and the value in dashboard-control-plane.ts.
• [Minor] Stale gauge has no cluster label / dedup (r4105649788) - addressed. The Go gauge records attribute.String("hypershell.cluster_id", AttentionClusterID(clusterID)) (metrics.go:317-319), and the BFF query is sum(max by (hypershell_cluster_id) (hypershell_stale_resource_status_count)) (metrics-control-plane-reconciliation.ts:19-20), matching the sandbox attention-gauge dedup pattern.
• [Minor] PR silently changes the just-merged HYPERSHELL-285 API-reliability display contract (r4105649796) - addressed. The req/s->requests/sec and toFixed(2)->toFixed(3) change is retained but now documented as an intentional compatibility change in specs/platform/control-plane-reconciliation-metrics.spec.md (Assumptions), instead of the spec asserting the contract is unchanged.
• [Minor] Stale-gauge count+record is not atomic with the map mutation (r4105894585) - addressed. SetResourceStatusStale now takes staleResourcesMu.Lock()/defer Unlock() before the store/delete, the Range count, and the Record, so concurrent reconciles can no longer export a snapshot that disagrees with the map (metrics.go:307-319).

The committer addressed the previous concerns.

Findings Summary (ordered by severity, highest first)

No open findings in this PR's code. Remaining items are cross-PR design decisions (canonical cluster-identity source; canonical convergence/staleness signal) that require maintainer coordination and a merge order.

Convention Checklist

Convention	Result
No panic() in production code	Pass
Errors wrapped / propagated with context	Pass
No secrets in logs or responses	Pass
Input validated (static PromQL, no injection)	Pass
Prometheus labels bounded (resource IDs stay in process memory)	Pass
Authenticated BFF endpoint	Pass
Replica-safe gauge aggregation (max-by-cluster dedup)	Pass
Concurrency-safe gauge count+record	Pass
Go coverage for new counters/histogram/gauge/dedup	Pass
Route-level failure (502) test coverage	Pass
Spec self-consistency (API-reliability change documented)	Pass

[markturansky]

Amber review

Status: Complete

View the submitted review.

[markturansky]

Verdict

This revision is clean: all six prior Amber findings are fixed on this head (the lag P50 no longer 502s an idle/fresh control plane, the stale gauge carries a deduped hypershell.cluster_id label recorded atomically under staleResourcesMu, and Go + BFF unit/route tests now satisfy CRM-005). No new in-code defects; the only open items are cross-PR design decisions that need maintainer coordination and a defined merge order.

Findings

No new findings this revision. The committer addressed the previous concerns (see below).

Cross-PR coordination

Three open efforts change contracts this PR now depends on; each needs a maintainer decision plus a defined merge order.

• Control-plane cluster identity source (#362). This PR threads cfg.ClusterID into NewGatewayReconciler (cmd/hypershell-controller/main.go, reconciler.go) and uses it as the hypershell.cluster_id label on the stale gauge (metrics.go:317-319). #362 deletes the ClusterID config field and the HYPERSHELL_CLUSTER_ID env entirely from internal/config/config.go and re-sources identity from a registration-resolved clusterID at startup. If #362 lands first, this PR stops compiling on cfg.ClusterID and the stale-gauge label must be rewired to the registered identity. Maintainers should pick the canonical identity source and sequence the two merges so the label reads it correctly.

• Canonical convergence/staleness signal (#151, #200). This PR adds SetResourceStatusStale(...) into GatewayReconciler.Handle and defines staleness as gw.GetReleaseId() != gw.GetObservedReleaseId() (reconciler.go:445-446). #151 modifies the same Handle to re-key convergence on a new generation/observed_generation primitive (converged = observed_generation == generation), and #200 defines the canonical control-plane reconciliation contract those generations implement. Two divergent definitions of "converged/stale" would ship in the same reconciler. A maintainer should choose the canonical convergence signal, decide whether this stale metric should key on the generation model rather than release IDs, and order the merges accordingly.

• Reconciliation widget help text (#365). This PR introduces the reconciliation dashboard widgets and their metric IDs (reconciliation-failures, reconciliation-retries, reconciliation-lag, stale-resource-status-count) plus their message catalog entries in packages/operational-dashboard-ui/src/dashboard/reliability-dashboard-widgets.tsx, messages.ts, and components/web-console/locales/en.json. #365 adds help text keyed on those same reconciliation metric IDs (dashboardHelpReconciliationFailures/Retries/Lag) and edits the same messages.ts, en.json, and dashboard-widget.css. #365's help-text mapping presupposes this PR's widgets and metric IDs exist, and it covers only three of the four IDs (no stale-resource-status-count). Maintainers should merge this PR first, keep the metric-ID naming contract identical across both, and decide whether the stale widget also needs help text.

Previous concerns

• [Major] Reconciliation-lag P50 could 502 the whole endpoint on idle/fresh control planes (r4098852159) - addressed. The lag P50 is the only query run with optional = true (metrics-control-plane-reconciliation.ts:105); queryInstant returns undefined for a non-finite/invalid optional sample instead of throwing (:59-62); the lag field is omitted via ...(lag === undefined ? {} : {...}) (:123); and bff/test/metrics-control-plane-reconciliation.test.ts asserts the required counts survive an idle NaN/empty lag histogram.
• [Major] Feature lacks the tests required by CRM-005 (r4098852165) - addressed. internal/otel/metrics_test.go (TestReconciliationMetrics, from L75) covers the failures/retries counter types, the hypershell.reconciliation.lag explicit bucket bounds, and the stale gauge's hypershell.cluster_id label plus the SetResourceStatusStale store/store/delete dedup count; dashboard-control-plane.test.ts asserts the reconciliation mapping; and the required-query 502 path is covered by bff/test/metrics-control-plane-reconciliation-route.test.ts:65-74, which drives a failing upstream and asserts 502 + {error: "Metrics unavailable", statusCode: 502}.
• [Minor] Redundant map() per metric in mapControlPlaneReconciliationResponse (r4098852172) - addressed. Each trend is computed once into a local and reused in both the guard and the value in dashboard-control-plane.ts.
• [Minor] Stale gauge has no cluster label / dedup (r4105649788) - addressed. The Go gauge records attribute.String("hypershell.cluster_id", AttentionClusterID(clusterID)) (metrics.go:317-319), and the BFF query is sum(max by (hypershell_cluster_id) (hypershell_stale_resource_status_count)) (metrics-control-plane-reconciliation.ts:19-20), matching the sandbox attention-gauge dedup pattern.
• [Minor] PR silently changes the just-merged HYPERSHELL-285 API-reliability display contract (r4105649796) - addressed. The req/s->requests/sec and toFixed(2)->toFixed(3) change is retained but now documented as an intentional compatibility change in specs/platform/control-plane-reconciliation-metrics.spec.md (Assumptions), instead of the spec asserting the contract is unchanged.
• [Minor] Stale-gauge count+record is not atomic with the map mutation (r4105894585) - addressed. SetResourceStatusStale now takes staleResourcesMu.Lock()/defer Unlock() before the store/delete, the Range count, and the Record, so concurrent reconciles can no longer export a snapshot that disagrees with the map (metrics.go:307-319).

The committer addressed the previous concerns.

Findings Summary (ordered by severity, highest first)

No open findings in this PR's code. Remaining items are cross-PR design decisions (canonical cluster-identity source; canonical convergence/staleness signal; reconciliation widget help-text ordering) that require maintainer coordination and a merge order.

Convention Checklist

Convention	Result
No panic() in production code	Pass
Errors wrapped / propagated with context	Pass
No secrets in logs or responses	Pass
Input validated (static PromQL, no injection)	Pass
Prometheus labels bounded (resource IDs stay in process memory)	Pass
Authenticated BFF endpoint	Pass
Replica-safe gauge aggregation (max-by-cluster dedup)	Pass
Concurrency-safe gauge count+record	Pass
Go coverage for new counters/histogram/gauge/dedup	Pass
Route-level failure (502) test coverage	Pass
Spec self-consistency (API-reliability change documented)	Pass

[kdoberst]

Hey Amber, the other PRs aren't a concern: #362 merged, #151/#200 are older PRs and most likely will never be merged at this point, and this PR will go in before #365. #365 will be adjusted once this PR is merged.