Name an OOM kill or restart on the canvas card's warning#1698

merged6
main
t3/metrics-3-canvas
openedOct 13, 2026byNNick
View on GitHubGitHub import enabledPR last imported 21d ago

The canvas card's ⚠ now names an OOM kill or a restart from the last hour. postgres shows an amber ⚠ OOM, and the other cards don't change. The canvas still shows no numbers.

What changes

  • The ⚠ says why. An OOM kill in the last hour adds one amber issue named OOM. One replica restarting twice or more in the hour, without an OOM kill, is named Restarting; a single restart per replica, as a requested restart or rollback causes, adds nothing. Exits alone add nothing. The badge shows the word, or OOM +1 when it shares the card with other issues. Clicking it opens Monitoring, the same as an open uptime incident.
  • The hour runs on the clock. A marker counts until an hour after it happened, measured against the current time, not against the last read. A timer wakes the card when the next marker ages out, so a warning clears on time even while reads fail and the last answer stays cached.
  • One model, not two. The marker feeds nodeIssues, the function behind the ⚠ count and tone. NodeIssues.uptime becomes monitoring, which means "this ⚠ opens Monitoring", and a trouble field holds the word. A Service that's down stays red.
  • Nothing extra without an answer. Loading, a failed read, an offline Organization, and Servers that need an upgrade or didn't answer give no markers, so the card looks as it did before. redis needs an upgrade in the fixture and still shows only its uptime ⚠ 1:

The read

Each Service read asks every Server in the Cluster. One read per card at the Monitoring range would cost Services × Servers RPCs a minute, each returning about 300 buckets.

So the card asks for markers only. markersOnly: true on ReadServiceMetricsInput sets step_nanos to the whole range, and every Server answers one bucket. Markers are filtered by time, not buckets, so none are lost. The query has its own key, a 60s stale time and a 5-minute refetch.

A project of 20 Services on 3 Servers costs 20 GETs and 60 one-bucket RPCs every 5 minutes. The alternative was 60 RPCs every minute. A batch endpoint wouldn't cut RPCs, because core has no subject for a whole namespace. The trade-off is that this read no longer shares a cache entry with the Monitoring tab. serviceMarkersQueryOptions is listed as an on-demand read, since no loader waits on every Server.

Checked

  • pnpm typecheck, oxlint --type-aware on the touched trees, and vitest on src/modules/metrics, node-status.test.ts and the data-boundaries test (53 tests). use-container-trouble.test.tsx renders the hook against a cached read with every refetch failing: an OOM 2.5 hours old reads nothing, and an OOM 30 minutes old reads OOM until 31 minutes later, then nothing. Both cases fail against the previous hook.
  • The verify dashboard with the PR 1 fixture and no Servers: the screenshots above. The badges' aria labels read OOM, 1 issue for postgres and 1 issue for redis and api, whose ⚠ comes from the seeded uptime incidents.

Not checked

  • The fixture ignores step_nanos, so no real daemon has served the one-bucket read here. Core clamps the step to the range width.
  • Before the Monitoring metrics section ships outside dev, the ⚠ opens a Monitoring tab without charts.

Checks

10 successful · 6 skipped
All checks passed
5a1eea4
Synced from GitHub

Review in Change Stack →

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 14cfd97d-7e66-423d-b2fc-e25a454dbcf6
📥 Commits

Reviewing files that changed from the base of the PR and between f140254c643fea0d883cc6a1d3956f84ed4f51a1 and c8b7dd6c046f6ed7140cc612a8ac90b0628a224e.

📒 Files selected for processing (5)
  • dashboard/src/collections/data-boundaries.static.test.ts
  • dashboard/src/collections/data-sources.ts
  • dashboard/src/routes/_protected/cloud/$organizationSlug/_project/$projectSlug/$environmentSlug/-components/canvas/StoreServiceNode.tsx
  • dashboard/src/routes/_protected/cloud/$organizationSlug/_project/$projectSlug/$environmentSlug/-components/canvas/node-status.test.ts
  • dashboard/src/routes/_protected/cloud/$organizationSlug/_project/$projectSlug/$environmentSlug/-components/canvas/node-status.ts
ascii
1 __________________________________________________________________________________________________________________________________________
2< Test early. Test often. Test automatically. Tests that run with every build are much more effective than test plans that sit on a shelf. >
3 ------------------------------------------------------------------------------------------------------------------------------------------
4 \
5 \ \
6 \ /\
7 ( )
8 .( o ).

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: fb1ebe5d-9020-4fb9-bad1-59a04fe5f486
📥 Commits

Reviewing files that changed from the base of the PR and between ca80990e4a35ef16efc08206099deb2e8a78dfd8 and 8e798a6ed535032a0bb384c2c1c74bc0996d9ed5.

📒 Files selected for processing (13)
  • dashboard/src/collections/data-boundaries.static.test.ts
  • dashboard/src/collections/data-sources.ts
  • dashboard/src/modules/metrics/metrics.queries.ts
  • dashboard/src/modules/metrics/metrics.server.ts
  • dashboard/src/modules/metrics/metrics.ts
  • dashboard/src/routes/_protected/cloud/$organizationSlug/_project/$projectSlug/$environmentSlug/-components/canvas/StoreServiceNode.tsx
  • dashboard/src/routes/_protected/cloud/$organizationSlug/_project/$projectSlug/$environmentSlug/-components/canvas/node-status-view.test.ts
  • dashboard/src/routes/_protected/cloud/$organizationSlug/_project/$projectSlug/$environmentSlug/-components/canvas/node-status-view.tsx
  • dashboard/src/routes/_protected/cloud/$organizationSlug/_project/$projectSlug/$environmentSlug/-components/canvas/node-status.test.ts
  • dashboard/src/routes/_protected/cloud/$organizationSlug/_project/$projectSlug/$environmentSlug/-components/canvas/node-status.ts
  • dashboard/src/routes/_protected/cloud/$organizationSlug/_project/$projectSlug/$environmentSlug/-components/canvas/use-container-trouble.test.tsx
  • dashboard/src/routes/_protected/cloud/$organizationSlug/_project/$projectSlug/$environmentSlug/-components/canvas/use-container-trouble.ts
  • docs/user/observe/metrics.md

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.

📝 Summary

Summary by CodeRabbit

  • New Features
    • Service cards now show an OOM warning when a replica was OOM-killed in the past hour, or Restarting when a replica restarted at least twice without an OOM kill. A single restart does not trigger a warning.
  • Improvements
    • Issue badges include the number of additional issues and use a red warning when the service is down.
    • Monitoring links are available for monitoring-related issues, and warning badges open the service’s Monitoring tab.

Service cards now query service markers and show recent OOM kills or repeated restarts as issues. The marker query requests a whole-range bucket. Trouble expires from the card after one hour, and the badge can open the service’s Monitoring tab.

Changes: Service card container trouble

Layer / File(s)Summary
Marker read query and history range
dashboard/src/modules/metrics/metrics.ts, dashboard/src/modules/metrics/metrics.queries.ts, dashboard/src/modules/metrics/metrics.server.ts, dashboard/src/collections/data-boundaries.static.test.ts, dashboard/src/collections/data-sources.ts
The marker-only query uses a whole-range history bucket. The query is included in the on-demand read allowlist, and its freshness details are documented.
Recent trouble detection and expiry
dashboard/src/routes/_protected/cloud/$organizationSlug/_project/$projectSlug/$environmentSlug/-components/canvas/node-status.ts, dashboard/src/routes/_protected/cloud/$organizationSlug/_project/$projectSlug/$environmentSlug/-components/canvas/use-container-trouble.ts, dashboard/src/routes/_protected/cloud/$organizationSlug/_project/$projectSlug/$environmentSlug/-components/canvas/node-status.test.ts, dashboard/src/routes/_protected/cloud/$organizationSlug/_project/$projectSlug/$environmentSlug/-components/canvas/use-container-trouble.test.tsx
Recent OOM markers take precedence over restart markers. Two or more restarts for the same server and replica indicate trouble. The hook expires trouble based on marker age, including when a replacement read fails.
Canvas issue aggregation and badge
dashboard/src/routes/_protected/cloud/$organizationSlug/_project/$projectSlug/$environmentSlug/-components/canvas/node-status.ts, dashboard/src/routes/_protected/cloud/$organizationSlug/_project/$projectSlug/$environmentSlug/-components/canvas/StoreServiceNode.tsx, dashboard/src/routes/_protected/cloud/$organizationSlug/_project/$projectSlug/$environmentSlug/-components/canvas/node-status-view.tsx, dashboard/src/routes/_protected/cloud/$organizationSlug/_project/$projectSlug/$environmentSlug/-components/canvas/node-status.test.ts, dashboard/src/routes/_protected/cloud/$organizationSlug/_project/$projectSlug/$environmentSlug/-components/canvas/node-status-view.test.ts, docs/user/observe/metrics.md
Trouble adds to the card’s issue count and enables Monitoring navigation. The badge displays the trouble label and other issue count. The documentation describes the badge conditions and appearance.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature

Sequence Diagram(s)

mermaid
1sequenceDiagram
2 participant StoreServiceCard
3 participant useContainerTrouble
4 participant serviceMarkersQueryOptions
5 participant readServiceMetrics
6 participant readHistory
7 participant nodeIssues
8 StoreServiceCard->>useContainerTrouble: Read trouble for the service
9 useContainerTrouble->>serviceMarkersQueryOptions: Request service markers
10 serviceMarkersQueryOptions->>readServiceMetrics: Read marker-only metrics
11 readServiceMetrics->>readHistory: Request a whole-range bucket
12 readHistory-->>useContainerTrouble: Return marker data
13 useContainerTrouble->>nodeIssues: Provide derived trouble
14 nodeIssues-->>StoreServiceCard: Return issue count and Monitoring status
Merge Risk: ⚪ Minimal · up to 8e798

No merge-blocking issue is established for the new service-card warnings. A real-daemon check of the one-bucket marker read remains worthwhile before broader rollout.

Security architecture risk: 🔵 Low · up to 8e798

The new warnings use the existing authenticated, organization-scoped metrics read and do not grant additional server privileges. One client-cache isolation assumption remains unverified: cached results are identified without the organization name.

Retained concerns

  • Low · security · inferred: The new marker-query cache key omits organizationSlug even though the request and service lookup are organization-scoped. If environment IDs can collide across organizations, a shared browser cache could reuse another organization's previously loaded result. Global uniqueness was not established. Existing metrics keys follow the same convention, and server authorization remains intact; this is an unresolved isolation invariant, not a verified cross-tenant vulnerability.
Security review details

Security Blast Radius

  • inferred — The server-side read remains bounded to the authorized organization's runtime and its service subject. The unresolved cache concern concerns reuse of previously loaded results within a shared browser cache, not demonstrated access to arbitrary organizations or host credentials.

Security Findings and Attack Paths

  • inferred — A conditional cache-confusion path would require matching environment and service identifiers across organizations, previously cached data, and reuse of the same query client. Whether the identity collision precondition is possible remains unresolved; no server authorization bypass was established.

Trust Boundaries and Controls

  • observed — Client-supplied organization, environment, service, range, and sampling mode cross the existing validated server-function boundary. Actor middleware and organization access checks precede organization-owned service resolution and runtime invocation.

Resilience and Maintainability Implications

  • observed — The new trouble state is derived presentation state. Its timer and classification do not perform deployment, persistence, credential, or authorization transitions, limiting the consequences of stale warning state.

Hardening Proposals

  • proposed — Include organizationSlug in the marker query key to make organization ownership explicit and remove dependence on an undocumented global environment-ID uniqueness guarantee.
Pre-merge checks · Passed 5
Passed Passed checks (5 passed)
Check nameStatusExplanation
Description checkPassedThe description directly explains the canvas warning changes, marker-based metrics read, expiration behavior, tests, and known limitations.
Title checkPassedThe title clearly and concisely describes the main change: naming recent OOM kills or restarts on canvas card warnings.
Docstring CoveragePassedDocstring coverage is 90.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 12 files. (1 skipped: 1…
Linked Issues checkPassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes checkPassedCheck skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

Synced from GitHub

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

ReviewStatusCommitReview trigger
📝 Code Review✅ Completed 2026-10-13T06:23:26.205166Z5a1eea4New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

Cchatgpt-codex-connectorBotreviewed · 1 inline threadOct 13, 2026
Synced from GitHub

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8e798a6ed5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

……@@ -51,7 +51,10 @@ function StatusMark({ tone }: { tone: Tone }) {
5353
54-/** What runs now, in one word (grey with its age when it isn't current), ending in the node's ⚠ N. */
54+/** "OOM" alone, "OOM +1" beside other issues, else the count. */
55+export const issuesText = ({ count, trouble }: NodeIssues) => trouble === null ? String(count) : count > 1 ? `${trouble} +${count - 1}` : trouble;

P1 Badge Amend the canvas badge design contract

Amend dashboard/DESIGN.md alongside this change: its canvas-node contract explicitly says the status line ends in ⚠ N, while issuesText now renders OOM or Restarting instead of that count whenever container trouble is present. Leaving the governing design document unchanged makes the implementation contradict the project's stated UI contract and future work may restore the numeric form; either document this intentional exception or retain ⚠ N.

AGENTS.md reference: AGENTS.md:L5-L5

Useful? React with 👍 / 👎.

Cchatgpt-codex-connectorBotreviewed · 1 inline threadOct 13, 2026
Synced from GitHub

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f140254c64

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

……@@ -0,0 +1,29 @@
7+
8+const NO_NAMES: ReadonlyMap<string, string> = new Map();
9+const NO_MARKERS: readonly Marker[] = [];
10+const markersOf = (read: MetricRead) => serviceMetrics(read, NO_NAMES).markers;

P2 Badge Preserve cached markers when a server stops answering

When a warning is cached and the next poll reports its machine under history.unanswered (for example, because that server is temporarily unreachable or needs an upgrade), markersOf returns only markers from answering machines. TanStack Query treats that aggregate response as successful and replaces the cached markers, so an OOM or restart warning can disappear before its one-hour expiry; the rejection test does not cover these per-machine failures. Preserve unexpired markers from unanswered machines or prevent an incomplete read from overwriting them.

AGENTS.md reference: dashboard/AGENTS.md:L19-L21

Useful? React with 👍 / 👎.

Added 6 commits

  • Document the canvas warning for OOM kills and restarts321a956
  • Read a Service's metrics as one bucket when the caller wants only its markers57ad857
  • Expire the canvas OOM and Restarting warnings an hour after they happened, even when reads fail5a1eea4
  • Name an OOM kill or restart in the last hour on the canvas card's warning99a825b
  • Show Restarting only when a replica restarts twice in the hourc027bb4
  • Read the canvas card's markers through a registered on-demand queryc552e0c
Cchatgpt-codex-connectorBotreviewed · 1 inline threadOct 13, 2026
Synced from GitHub

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5a1eea4fba

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

……@@ -42,8 +43,10 @@ export function StoreServiceCard({ service, domains, changeCount, runtimeIdentit
46- const issues = nodeIssues(status, domains, unreachable);
46+ const scope = { organizationSlug: params.organizationSlug, environmentId };
47+ const unreachable = useUnreachableHost(scope, service.id);
48+ const trouble = useContainerTrouble(scope, service.name);

P2 Badge Keep markers through private-address deployments

When a deployment applies a Private address change, this hook continues querying by the Service's display name, but findService now resolves that name to the newly applied runtime name, while core/crates/ployzd/src/observe/metrics/segment.rs::shows selects historical markers by their recorded runtime name. The next successful poll therefore replaces the cache with no markers from the old address and clears a recent OOM/Restarting badge before its documented one-hour lifetime. Preserve unexpired cached markers across that transition or query metrics using the stable Service identity.

AGENTS.md reference: AGENTS.md:L4-L4

Useful? React with 👍 / 👎.