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.tsdashboard/src/collections/data-sources.tsdashboard/src/routes/_protected/cloud/$organizationSlug/_project/$projectSlug/$environmentSlug/-components/canvas/StoreServiceNode.tsxdashboard/src/routes/_protected/cloud/$organizationSlug/_project/$projectSlug/$environmentSlug/-components/canvas/node-status.test.tsdashboard/src/routes/_protected/cloud/$organizationSlug/_project/$projectSlug/$environmentSlug/-components/canvas/node-status.ts
| 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.tsdashboard/src/collections/data-sources.tsdashboard/src/modules/metrics/metrics.queries.tsdashboard/src/modules/metrics/metrics.server.tsdashboard/src/modules/metrics/metrics.tsdashboard/src/routes/_protected/cloud/$organizationSlug/_project/$projectSlug/$environmentSlug/-components/canvas/StoreServiceNode.tsxdashboard/src/routes/_protected/cloud/$organizationSlug/_project/$projectSlug/$environmentSlug/-components/canvas/node-status-view.test.tsdashboard/src/routes/_protected/cloud/$organizationSlug/_project/$projectSlug/$environmentSlug/-components/canvas/node-status-view.tsxdashboard/src/routes/_protected/cloud/$organizationSlug/_project/$projectSlug/$environmentSlug/-components/canvas/node-status.test.tsdashboard/src/routes/_protected/cloud/$organizationSlug/_project/$projectSlug/$environmentSlug/-components/canvas/node-status.tsdashboard/src/routes/_protected/cloud/$organizationSlug/_project/$projectSlug/$environmentSlug/-components/canvas/use-container-trouble.test.tsxdashboard/src/routes/_protected/cloud/$organizationSlug/_project/$projectSlug/$environmentSlug/-components/canvas/use-container-trouble.tsdocs/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.tsThe 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.tsxRecent 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.mdTrouble 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)
mermaid1 sequenceDiagram 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 - New Features
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 name Status Explanation Description check Passed The description directly explains the canvas warning changes, marker-based metrics read, expiration behavior, tests, and known limitations. Title check Passed The title clearly and concisely describes the main change: naming recent OOM kills or restarts on canvas card warnings. Docstring Coverage Passed Docstring 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 check Passed Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Passed Check 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.