feat(observability): grouped error Issues (local, OSS) — Phase 1 - #138
Conversation
✅ Deploy Preview for lunorash ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Thank you for following the naming conventions! 🙏 |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a cross-runtime fingerprint package, persists and groups errors into Issues, exposes a new admin RPC, and adds Studio navigation and panels for grouped issues and per-instance container lifecycle data. ChangesIssues observability
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Studio
participant AdminRPC
participant RequestLog
participant Fingerprint
Studio->>AdminRPC: request getIssues
AdminRPC->>RequestLog: read error entries
RequestLog->>Fingerprint: fingerprint functionPath and message
Fingerprint-->>RequestLog: grouping hash
RequestLog-->>AdminRPC: grouped issues
AdminRPC-->>Studio: issues payload
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Thank you for confirming the Contributor License Agreement! 🙏 |
|
React Doctor found no new issues. 🎉 Reviewed by React Doctor for commit |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/do/src/shard-do.ts`:
- Around line 4966-4990: After the successful appendRequestLogEntry call in
handleRecordContainerEvent, mark the request-log table as changed and flush the
pending changes so live getIssues/getLogs subscriptions refresh immediately.
Keep this notification inside the try block, ensuring failed best-effort
persistence does not trigger a flush.
In `@packages/studio/src/locales/en.ts`:
- Around line 1104-1105: Update the two locale strings and their matching call
sites in containers-panel.tsx and studio.tsx to describe lifecycle state and
observations per instance rather than per container, while preserving the
existing message IDs and meaning.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 606607cb-c036-4fa8-88f5-0c90b8287634
⛔ Files ignored due to path filters (8)
labeler-config.ymlis excluded by none and included by nonepackages/do/__tests__/request-log.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/do/__tests__/shard-do.admin.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/fingerprint/__assets__/package-og.svgis excluded by!**/*.svgand included bypackages/**packages/fingerprint/__tests__/fingerprint.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/studio/__tests__/features/containers/fold-container-instances.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yamland included by nonepnpm-workspace.yamlis excluded by none and included by none
📒 Files selected for processing (26)
packages/do/package.jsonpackages/do/src/introspect.tspackages/do/src/log-buffer.tspackages/do/src/request-log.tspackages/do/src/shard-do.tspackages/fingerprint/.releaserc.jsonpackages/fingerprint/LICENSE.mdpackages/fingerprint/NOTICEpackages/fingerprint/README.mdpackages/fingerprint/eslint.config.jspackages/fingerprint/package.jsonpackages/fingerprint/packem.config.tspackages/fingerprint/prettier.config.jspackages/fingerprint/project.jsonpackages/fingerprint/src/index.tspackages/fingerprint/src/lunora.tspackages/fingerprint/src/sha256.tspackages/fingerprint/src/superlog.tspackages/fingerprint/tsconfig.jsonpackages/fingerprint/vitest.config.tspackages/studio/src/app/studio.tsxpackages/studio/src/features/containers/containers-panel.tsxpackages/studio/src/features/containers/fold-container-instances.tspackages/studio/src/features/issues/issues-panel.tsxpackages/studio/src/lib/admin.tspackages/studio/src/locales/en.ts
| "Live Cloudflare Containers — current lifecycle state per container from the log stream.", | ||
| "Cloudflare Containers are observed from their lifecycle log stream. This shows the current state per container — ports and health checks aren't carried in that stream.", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Describe the view as per-instance.
The new panel renders separate rows for concurrent instances, but these strings still say “per container,” obscuring the feature this PR adds. Update the matching call sites in containers-panel.tsx and studio.tsx too, since they use these message IDs.
Proposed fix
- "Live Cloudflare Containers — current lifecycle state per container from the log stream.",
- "Cloudflare Containers are observed from their lifecycle log stream. This shows the current state per container — ports and health checks aren't carried in that stream.",
+ "Live Cloudflare Containers — current lifecycle state per instance from the log stream.",
+ "Cloudflare Containers are observed from their lifecycle log stream. This shows the current state per instance — ports and health checks aren't carried in that stream.",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "Live Cloudflare Containers — current lifecycle state per container from the log stream.", | |
| "Cloudflare Containers are observed from their lifecycle log stream. This shows the current state per container — ports and health checks aren't carried in that stream.", | |
| "Live Cloudflare Containers — current lifecycle state per instance from the log stream.", | |
| "Cloudflare Containers are observed from their lifecycle log stream. This shows the current state per instance — ports and health checks aren't carried in that stream.", |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/studio/src/locales/en.ts` around lines 1104 - 1105, Update the two
locale strings and their matching call sites in containers-panel.tsx and
studio.tsx to describe lifecycle state and observations per instance rather than
per container, while preserving the existing message IDs and meaning.
Merging this PR will improve performance by 10.14%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | baseline (Object.keys + toInternal + path spread per field) |
71.5 µs | 64.9 µs | +10.14% |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing feat/observability-issues (9a2d879) with alpha (32ee8a6)1
Footnotes
- fingerprint: replace regex stack-frame parser with linear index scan and clamp bucketer input length, removing the polynomial-ReDoS sinks (CodeQL #25); inline the firstNonEmpty helper - do: treat a container stop with a non-zero exit code as a crash so it folds into Issues; extract a shared persistRequestLog seam so container crashes also reach Logpush; flush the reqlog table on the RPC error and container paths so live getIssues/getLogs refresh in real time; project only the 3 needed columns in readErrorIssues instead of the full 14-column hydrate - studio: drop the NUL-delimiter container key for a JSON-encoded pair (file was a binary blob), remove the redundant useMemo, format Issue timestamps via formatTimestamp, reword container copy to per-instance - docs: add the @lunora/fingerprint package row Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Addressed the review + bot findings in a284d9d (plus a Bots
From the audit
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/do/src/request-log.ts (1)
574-598: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFilter/limit-building logic duplicates
readRequestLog.The
functionPathPrefix/userId/shardKeyconjunct-building (Lines 582-595) and thelimitclamp (Line 577,Math.max(1, Math.min(options.limit ?? REQUEST_LOG_RETENTION, 10_000))) are copy-pasted fromreadRequestLog(Lines 468, 473-486). A shared helper (e.g.buildCommonLogFilters(options)/clampLimit(limit)) would remove the duplication and prevent the two readers from silently drifting apart if a filter is added to one but not the other.♻️ Extract shared filter/limit helpers
+const clampLimit = (limit: number | undefined): number => Math.max(1, Math.min(limit ?? REQUEST_LOG_RETENTION, 10_000)); + +const buildCommonLogFilters = (options: { + functionPathPrefix?: string; + shardKey?: string; + userId?: string; +}): { conjuncts: string[]; parameters: unknown[] } => { + const conjuncts: string[] = []; + const parameters: unknown[] = []; + + if (options.functionPathPrefix !== undefined && options.functionPathPrefix !== "") { + conjuncts.push(String.raw`function_path LIKE ? ESCAPE '\'`); + parameters.push(`${escapeLike(options.functionPathPrefix)}%`); + } + + if (options.userId !== undefined && options.userId !== "") { + conjuncts.push("user_id = ?"); + parameters.push(options.userId); + } + + if (options.shardKey !== undefined && options.shardKey !== "") { + conjuncts.push("shard_key = ?"); + parameters.push(options.shardKey); + } + + return { conjuncts, parameters }; +};Then both
readRequestLogandreadErrorIssuescallbuildCommonLogFilters(options)and append their own extra conjuncts (seq > ?,outcome = 'error',tableTouched, etc.) plusclampLimit(options.limit).As per path instructions, "Follow DRY principles" for
packages/**/*.ts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/do/src/request-log.ts` around lines 574 - 598, Extract the duplicated limit clamping and functionPathPrefix/userId/shardKey conjunct construction from readRequestLog and readErrorIssues into shared helpers such as clampLimit and buildCommonLogFilters. Update both readers to reuse these helpers while preserving their reader-specific predicates and parameter ordering, including readErrorIssues’ outcome filter and limit parameter.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/do/src/request-log.ts`:
- Around line 564-573: Update the issue-folding logic around the newest-first
row iteration to select representative title, sampleMessage, and culprit fields
by maximum ts rather than first seq order. Track the timestamp of the current
representative, refresh those fields whenever a row has a newer ts, and keep
lastSeen/count/firstSeen aggregation behavior unchanged.
---
Nitpick comments:
In `@packages/do/src/request-log.ts`:
- Around line 574-598: Extract the duplicated limit clamping and
functionPathPrefix/userId/shardKey conjunct construction from readRequestLog and
readErrorIssues into shared helpers such as clampLimit and
buildCommonLogFilters. Update both readers to reuse these helpers while
preserving their reader-specific predicates and parameter ordering, including
readErrorIssues’ outcome filter and limit parameter.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0a6020c7-1661-4eb2-af44-9403039f1fa3
⛔ Files ignored due to path filters (2)
AGENTS.mdis excluded by none and included by nonepackages/do/__tests__/shard-do.admin.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**
📒 Files selected for processing (9)
packages/do/src/request-log.tspackages/do/src/shard-do.tspackages/fingerprint/src/lunora.tspackages/fingerprint/src/superlog.tspackages/studio/src/app/studio.tsxpackages/studio/src/features/containers/containers-panel.tsxpackages/studio/src/features/containers/fold-container-instances.tspackages/studio/src/features/issues/issues-panel.tsxpackages/studio/src/locales/en.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- packages/studio/src/features/containers/fold-container-instances.ts
- packages/studio/src/features/issues/issues-panel.tsx
- packages/fingerprint/src/lunora.ts
- packages/studio/src/features/containers/containers-panel.tsx
- packages/studio/src/app/studio.tsx
- packages/studio/src/locales/en.ts
- packages/do/src/shard-do.ts
- packages/fingerprint/src/superlog.ts
Add @lunora/fingerprint, a zero-dependency error-grouping core vendored
from superloglabs/superlog (Apache-2.0). It exposes `fingerprintError({
functionPath, message, code? }) -> { hash, title, culprit }`, whose
canonical hash is computed over `functionPath :: bucket(message)` only —
code is returned as metadata, never hashed — so a live-sink error event
and a persisted request-log row for the same error collapse onto one
stable hash. The message bucketer strips per-request noise (URLs, UUIDs,
IPs, timestamps, hex, long ids, paths) so a route-scanner sweep folds
into a single Issue. Ships a portable SHA-256 backend (workerd/browser
safe) proven byte-for-byte against upstream's node:crypto vectors.
superlog's copyright and a NOTICE are retained per Apache-2.0 §4; the
package ships under the repo's FSL-1.1-Apache-2.0 convention.
Registered in pnpm-workspace overrides and the labeler config.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018sRFb1136YE8KDmDbFMYmm
Add a `__lunora_admin__:getIssues` admin RPC that folds the recent
`error`-outcome rows of the bounded request-log readout into grouped
Issues by fingerprint (`@lunora/fingerprint`). `readErrorIssues` is a
pure read-side aggregation — no new storage, no transport — returning
`{ hash, title, culprit, count, firstSeen, lastSeen, sampleMessage }`
per group, most-recently-active first. It is wired through `readAdminOp`
with an `ADMIN_WILDCARD` table dep so it serves both the one-shot POST
and live WebSocket subscriptions, gated by the existing admin auth.
Carry the container instance id and process exit code through to
`LogEntry`: `parseRecordContainerEventArgs` now reads `envelope.instance`
and parses `(exit <n>)` out of the lifecycle message instead of
collapsing to `container:<name>` only. Error-level container events also
append an `error`-outcome request-log row, so a container crash and a
Worker throw fold into the same Issues list under one fingerprint.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018sRFb1136YE8KDmDbFMYmm
Add an Issues observability panel that reads the new `__lunora_admin__:getIssues` RPC live over the admin WebSocket, showing grouped error triage — each row a fingerprinted Issue (title, culprit, event count, first/last-seen) folding Worker throws and container crashes together. Threaded through the compile-checked route table (tab union, nav group, lazy import, icon, label, description, panel). Relocate Containers from the Functions nav group into Observability, and key the container fold per `(name, instance)` now that the instance id survives — adding Instance and Exit columns so crash-looping instances are distinguishable. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018sRFb1136YE8KDmDbFMYmm
- fingerprint: replace regex stack-frame parser with linear index scan and clamp bucketer input length, removing the polynomial-ReDoS sinks (CodeQL #25); inline the firstNonEmpty helper - do: treat a container stop with a non-zero exit code as a crash so it folds into Issues; extract a shared persistRequestLog seam so container crashes also reach Logpush; flush the reqlog table on the RPC error and container paths so live getIssues/getLogs refresh in real time; project only the 3 needed columns in readErrorIssues instead of the full 14-column hydrate - studio: drop the NUL-delimiter container key for a JSON-encoded pair (file was a binary blob), remove the redundant useMemo, format Issue timestamps via formatTimestamp, reword container copy to per-instance - docs: add the @lunora/fingerprint package row Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The @lunora/fingerprint row broke prettier's markdown table alignment, failing the Lint (prettier) check. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A container lifecycle row carries the caller's envelope `ts`, not `Date.now()`, so an out-of-order or clock-skewed push can write an older-`ts` row at a higher `seq`. readErrorIssues seeded title/sampleMessage from the first row in `seq` order while lastSeen took `max(ts)` — so the sample could describe a different occurrence than the one lastSeen pointed at. Track the representative by max ts. culprit needs no such tracking: it is `functionPath`, an input to the hash, so it is invariant across a group. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
3d29a07 to
51a2c77
Compare
Phase 1 — Fingerprint + local Issues + container-crash grouping
First slice of the observability plan: grouped error triage in local dev and any self-hosted worker, shipping in the framework (pure OSS, no cloud dependency). Draws the error-grouping model from superloglabs/superlog (Apache-2.0). Phases 2–4 (OTLP transport, cloud ingest/store/hosted views, alerts+AI) stack later on the cloud branch (PR #85).
What lands
@lunora/fingerprint(new package) — zero-dependency error-grouping core vendored from superlog.fingerprintError({ functionPath, message, code? }) → { hash, title, culprit }. The canonical hash is overfunctionPath :: bucket(message)only (code is metadata, never hashed), so a live-sink error event and a persisted request-log row for the same error collapse onto one stable hash — the same key a future cloud Incident groups on. The message bucketer strips per-request noise (URLs, UUIDs, IPs, timestamps, hex, long ids, paths) so a route-scanner sweep folds into a single Issue. Portable SHA-256 backend (workerd/browser-safe), proven byte-for-byte against upstream'snode:cryptovectors. superlog copyright +NOTICEretained per Apache-2.0 §4; ships under the repo'sFSL-1.1-Apache-2.0.@lunora/do— new__lunora_admin__:getIssuesadmin RPC.readErrorIssuesis a pure read-side aggregation over the bounded__lunora_reqlog__readout (no new storage, no transport): it groups recenterror-outcome rows by fingerprint into{ hash, title, culprit, count, firstSeen, lastSeen, sampleMessage }, most-recently-active first. Wired throughreadAdminOpwith anADMIN_WILDCARDtable dep so it serves both the one-shot POST and live WS subscriptions under the existing admin auth. Container lifecycle events now carry the instance id and parsed exit code ontoLogEntry, and an error-level container event also appends anerrorrequest-log row — so a container crash and a Worker throw fold into the same Issues list under one fingerprint.@lunora/studio— new Issues observability panel (live over the admin WS), threaded through the compile-checked route table. Containers moved from the Functions nav group into Observability, and the container fold is now keyed per(name, instance)with Instance + Exit columns so crash-looping instances are distinguishable.Verification
@lunora/fingerprint: 19 golden-vector tests (bot-sweep collapse, live-sink↔reqlog hash equality, container-crash grouping) ✓ · tsc ✓ · eslint ✓@lunora/do: full suite 1042 passed + 1 todo (incl. newreadErrorIssues+ container-crash-fold tests) ✓ · tsc ✓ · eslint ✓@lunora/studio: tsc ✓ · eslint ✓ · fold-container-instances unit suite 12 passed ✓🤖 Generated with Claude Code
Summary by CodeRabbit
exitCode(including correlationinstance).