feat(observability): ctx.trace spans + ctx.metrics — the two missing OTel pillars - #149
Merged
Conversation
✅ Deploy Preview for lunorash ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
|
Thank you for following the naming conventions! 🙏 |
Contributor
|
Thank you for confirming the Contributor License Agreement! 🙏 |
|
Warning Review limit reached
Next review available in: 10 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (11)
📒 Files selected for processing (14)
WalkthroughAdds ChangesObservability instrumentation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant ShardDO
participant TelemetrySink
participant Studio
Client->>ShardDO: RPC with traceparent
ShardDO->>TelemetrySink: emit logs, metrics, and spans
ShardDO->>ShardDO: buffer and fold telemetry
Studio->>ShardDO: getTraces or getMetricSeries
ShardDO-->>Studio: trace summaries or metric series
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Contributor
|
React Doctor found no new issues. 🎉 Reviewed by React Doctor for commit |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/do/src/shard-do.ts (1)
4432-4483: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
ctx.logtrace correlation re-reads a live, mutable field — missing the same anchor-threading fixctx.trace/ctx.metricsgot.
recordUserLogreadsconst trace = this.currentRequestTrace;(Line 4446) at the moment each individuallog.*()call executes, andmakeLogger(Line 4492) has no anchor parameter at all to thread one through. Contrast withmakeTracer(Line 4523), which explicitly acceptsanchor?: TraceAnchorand is invoked from the generatedbuildCtxwithoptions.identity ? undefined : this.getCurrentTrace()— captured once, or explicitly re-minted for a deferred context.Because Durable Object
async/awaitcan interleave concurrent dispatches on the same instance,this.currentRequestTracecan belong to a different, concurrently in-flight/rpccall (or beundefined, already cleared by that call'sfinally) by the time a handler'sctx.log.info(...)actually runs — especially inside a subscription re-run (executeSubscription), which deliberately threadsidentityby value for exactly this reason but still buildsctx.logviathis.makeLogger(logFunctionPath, observability)with no anchor. The same live-field dependency also affectshandleRunAs, lifecycle hooks (dispatchLifecycle), and stream dispatch (executeStream), none of which populatecurrentRequestTracethemselves before callingbuildCtx.Net effect: a log line can be correlated to the wrong trace/span, or lose correlation entirely — directly undermining this PR's stated goal of "correct trace attribution across concurrent dispatches, subscription reruns."
Recommend threading an explicit
anchor?: TraceAnchorthroughmakeLogger/recordUserLog, mirroringmakeTracer, and having the generatedbuildCtx(inpackages/codegen/src/emit.ts) pass the sameoptions.identity ? undefined : this.getCurrentTrace()value it already computes fortrace.Also applies to: 4492-4492
🤖 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/shard-do.ts` around lines 4432 - 4483, Thread an explicit optional TraceAnchor through recordUserLog and makeLogger, and use that anchor for event traceId/spanId instead of reading the mutable currentRequestTrace at log-call time. Update generated buildCtx in emit.ts to pass the same captured options.identity ? undefined : this.getCurrentTrace() anchor used for trace creation, preserving explicit re-minting for deferred contexts and consistent correlation across concurrent dispatches.
🤖 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/studio/src/features/logs/logs-panel.tsx`:
- Around line 45-57: Update the LEVEL_VARIANT mapping to include the missing
warn LogLevel key, assigning it the appropriate BadgeVariant while preserving
all existing severity mappings and the Record<LogLevel, BadgeVariant> type.
---
Outside diff comments:
In `@packages/do/src/shard-do.ts`:
- Around line 4432-4483: Thread an explicit optional TraceAnchor through
recordUserLog and makeLogger, and use that anchor for event traceId/spanId
instead of reading the mutable currentRequestTrace at log-call time. Update
generated buildCtx in emit.ts to pass the same captured options.identity ?
undefined : this.getCurrentTrace() anchor used for trace creation, preserving
explicit re-minting for deferred contexts and consistent correlation across
concurrent dispatches.
🪄 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: b5b15cb2-e3fc-48ef-8d7c-81cdb631cefa
⛔ Files ignored due to path filters (26)
api-snapshots/do.api.mdis excluded by none and included by noneapi-snapshots/lunora.api.mdis excluded by none and included by noneapi-snapshots/runtime.api.mdis excluded by none and included by noneapi-snapshots/server.api.mdis excluded by none and included by noneapi-snapshots/studio.api.mdis excluded by none and included by noneexamples/auth-playground/lunora/_generated/shard.tsis excluded by!**/_generated/**and included by noneexamples/blog/lunora/_generated/shard.tsis excluded by!**/_generated/**and included by noneexamples/expo/lunora/_generated/shard.tsis excluded by!**/_generated/**and included by noneexamples/offline-rejections/lunora/_generated/shard.tsis excluded by!**/_generated/**and included by noneexamples/payment-demo/lunora/_generated/shard.tsis excluded by!**/_generated/**and included by noneexamples/realtime-cursors/lunora/_generated/shard.tsis excluded by!**/_generated/**and included by noneexamples/todo-app/lunora/_generated/shard.tsis excluded by!**/_generated/**and included by nonepackages/codegen/__tests__/fixtures/simple/expected/_generated/shard.tsis excluded by!**/_generated/**,!**/__tests__/**and included bypackages/**packages/do/__tests__/shard-do.admin.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/do/__tests__/tracing.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/runtime/__tests__/observability-sinks.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/server/__tests__/context.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/server/__tests__/functions-v-from.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/server/__tests__/functions.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/studio/__tests__/app/studio.test.tsxis excluded by!**/__tests__/**and included bypackages/**packages/studio/__tests__/features/traces/traces-panel.test.tsxis excluded by!**/__tests__/**and included bypackages/**packages/testing/__tests__/ctx-telemetry-contract.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**shared/log-event.tsis excluded by none and included by noneshared/metric-event.tsis excluded by none and included by noneshared/otlp.tsis excluded by none and included by noneshared/span-event.tsis excluded by none and included by none
📒 Files selected for processing (21)
apps/docs/src/content/docs/concepts/observability.mdxpackages/codegen/src/emit.tspackages/do/src/ctx-telemetry.tspackages/do/src/index.tspackages/do/src/introspect.tspackages/do/src/log-buffer.tspackages/do/src/shard-do.tspackages/do/src/span-buffer.tspackages/do/src/trace-context.tspackages/runtime/src/index.tspackages/runtime/src/observability-sinks.tspackages/runtime/src/observability.tspackages/server/src/index.tspackages/server/src/types.tspackages/studio/src/app/studio.tsxpackages/studio/src/features/logs/logs-panel.tsxpackages/studio/src/features/traces/trace-geometry.tspackages/studio/src/features/traces/traces-panel.tsxpackages/studio/src/lib/admin.tspackages/studio/src/locales/en.tspackages/testing/src/harness.ts
Merging this PR will not alter performance
Comparing 
Footnotes
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/do/src/metric-buffer.ts (1)
84-86: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueGuard against fractional capacities truncating to 0.
If a fractional capacity between
0and1(e.g.0.5) is supplied,capacity > 0evaluates to true butMath.trunc(capacity)evaluates to0, resulting in a0capacity buffer (which effectively behaves as a size1cache). Changing the check tocapacity >= 1prevents this edge case.💡 Proposed refactor
public constructor(capacity: number = DEFAULT_CAPACITY) { - this.capacity = capacity > 0 ? Math.trunc(capacity) : DEFAULT_CAPACITY; + this.capacity = capacity >= 1 ? Math.trunc(capacity) : DEFAULT_CAPACITY; }🤖 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/metric-buffer.ts` around lines 84 - 86, Update the constructor’s capacity validation in MetricBuffer so fractional values below 1 fall back to DEFAULT_CAPACITY before truncation; use a threshold that only accepts capacities of at least 1, while preserving truncation for valid fractional values at or above that threshold.
🤖 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.
Nitpick comments:
In `@packages/do/src/metric-buffer.ts`:
- Around line 84-86: Update the constructor’s capacity validation in
MetricBuffer so fractional values below 1 fall back to DEFAULT_CAPACITY before
truncation; use a threshold that only accepts capacities of at least 1, while
preserving truncation for valid fractional values at or above that threshold.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: de93e88f-1fe8-4cd0-98bf-538fdb23a3bb
⛔ Files ignored due to path filters (6)
packages/do/__tests__/metric-buffer.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/do/__tests__/tracing.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/studio/__tests__/app/studio.test.tsxis excluded by!**/__tests__/**and included bypackages/**packages/studio/__tests__/features/reports/instruments-table.test.tsxis excluded by!**/__tests__/**and included bypackages/**packages/studio/__tests__/features/traces/traces-panel.test.tsxis excluded by!**/__tests__/**and included bypackages/**
📒 Files selected for processing (9)
packages/do/src/introspect.tspackages/do/src/metric-buffer.tspackages/do/src/shard-do.tspackages/do/src/span-buffer.tspackages/studio/src/features/reports/instruments-table.tsxpackages/studio/src/features/reports/metrics-panel.tsxpackages/studio/src/features/traces/traces-panel.tsxpackages/studio/src/lib/admin.tspackages/studio/src/locales/en.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/studio/src/lib/admin.ts
- packages/studio/src/features/traces/traces-panel.tsx
- packages/do/src/shard-do.ts
The in-memory LogBuffer folded the seven ctx.log severities onto four tiers (trace→debug, log/info→info, fatal→error), so a line logged at trace or fatal was indistinguishable from debug or error in the Studio Logs panel — the two tiers #147 added were unreadable at the only place they surface locally. Buffer the level the caller actually logged at, widen the studio's mirrored LogLevel union to match, and source the panel's chip list + grouped summary from a new LOG_LEVEL_ORDER in the shared contract so the ordering has one home. Also drops a stale comment on otlpSink claiming trace correlation is "a later phase" — it shipped in #143 and the sink already reuses event.traceId/spanId. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0197KjhmBDB3PXAc6TXiPqZ9
…dispatch PR #143 propagated W3C trace context worker→shard→container and #147 stamped traceId/spanId onto every ctx.log line, but a handler still had no way to instrument a sub-operation: one SERVER span per dispatch was the entire worker trace surface. A slow request was one opaque bar. Adds ctx.trace(name, fn, attributes?) on Query/Mutation/ActionCtx. It returns the body's value unchanged and re-throws a failure after recording it as an error span — instrumentation, never flow control. Nesting is lexical, so the shape of the code is the shape of the waterfall. Design notes: - The span stack lives in a closure scoped to the ctx, NOT on `this` like the surrounding currentRequest* fields. Those are set-at-entry/cleared-at-exit and so are only sound across code that doesn't span an interleaving point, whereas a span stack is by definition held across the awaited body. Scoping it to the ctx makes concurrent dispatches structurally unable to corrupt each other's nesting. - The trace anchor IS resolved once per dispatch on `this`, so ctx.trace and the synthetic root span agree on the ids even with no inbound traceparent. - The root span is recorded only when the dispatch actually produced spans: minting one per request would fill the bounded ring with single-bar traces from uninstrumented handlers and evict the instrumented ones the panel exists for. It is not sent to onSpan — the runtime already emits the dispatch to onRpc, and a collector would otherwise show it twice. - foldTraces orders rows by (offset, depth), not arrival. Spans are recorded on completion, so a child is buffered before its parent, and at millisecond resolution the two routinely share a startTs — ordering has to come from the structure. The anchor is likewise picked structurally (the span whose parent is absent), since a fast parent/child pair is indistinguishable by timing. Caught by a test; both cases are now regression-covered. - The fold is total: the ring routinely holds partial traces (eviction can drop a parent, and a trace can be read before its root settles), so an orphan is re-parented onto the anchor rather than dropped. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0197KjhmBDB3PXAc6TXiPqZ9
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0197KjhmBDB3PXAc6TXiPqZ9
The getTraces admin RPC has shipped recent ctx.trace waterfalls since f512b5a8, but nothing in the studio rendered them: a handler could instrument its sub-operations and still had no way to see the result. Adds the Traces page under Observability, right after Logs — a trace is the drill-down from a log line, so it sits where you'd reach for it. The list shows one row per recent trace (root name, function path, total duration, span count, ok/error badge); selecting one expands the waterfall. A debounced search narrows the list over trace name and function path, and ShardInput scopes the read, both mirroring the Logs panel. Design notes: - No client-side tree math. foldTraces already stamps every span with a depth and an offsetMs, so a row is a pure function of its record: indent by depth, place the bar from offsetMs, size it by durationMs. - The geometry and the filter live in trace-geometry.ts rather than in the panel, so both are unit-testable without mounting React — the same split as metrics-aggregate.ts and slo-aggregate.ts. - A trace duration of 0 is routine, not a bug: a DO clock only advances on I/O, so a whole dispatch can settle inside one wall-clock millisecond. spanBar treats a non-positive denominator as "lay every span out full-width" instead of emitting NaN%, which would render as no bar at all. Sub-millisecond spans get a minimum bar width for the same reason — an instant span should read as instant, not as missing. - Bars are clipped to the remaining track width, so a partial trace (evicted parent, anchor is a survivor rather than the true root) cannot overflow its row. - TraceSpan/TraceSummary are hand-mirrored into lib/admin.ts like every other @lunora/do wire shape: the studio ships browser components and must not pull the DO runtime into its bundle. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0197KjhmBDB3PXAc6TXiPqZ9
Lunora had logs and (as of the previous commit) traces, but no metrics pillar at
all: no ctx.metrics, no counters/histograms, and otlpSink posted only /v1/traces
and /v1/logs — shared/otlp.ts had no metrics envelope. A trace tells you what one
request did; there was no way to ask what a million requests did.
Adds ctx.metrics.{count,gauge,record} on Query/Mutation/ActionCtx, an onMetric
sink hook, wrapResourceMetrics in the shared OTLP encoders, and a /v1/metrics
export from otlpSink (monotonic Sum / Gauge / Histogram).
Design notes:
- Stateless by design: one call is one measurement, with DELTA temporality for
the collector to aggregate. Pre-aggregating in the runtime would mean picking a
flush point and a merge rule per instrument kind — and a histogram cannot be
merged at all without losing the distribution, which is the only reason to use
one. The runtime stays a transport; the collector, built for this, aggregates.
Documented, with the hot-loop guidance that follows from it.
- A histogram sample is exported as a single observation in one implicit bucket
(explicitBounds: []), so the runtime never has to pick bucket boundaries on the
user's behalf.
- Non-finite values are dropped rather than exported: NaN/Infinity has no
meaningful encoding and would poison an aggregate downstream.
- No in-memory buffer and no Studio panel, unlike logs and spans. A metric's
value is its aggregate over time, which a bounded ring on a hibernating
instance cannot represent — buffering would imply a fidelity that isn't there.
consoleSink prints them for dev; otlpSink is the real destination.
The lunoraTest harness gets a passthrough ctx.trace (runs the body, records
nothing) and a no-op ctx.metrics, so instrumenting a handler never changes what a
test observes.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0197KjhmBDB3PXAc6TXiPqZ9
Picks up the ctx.trace / ctx.metrics wiring added to the buildCtx emitter. Kept as its own commit per the regeneration convention, since a regen can sweep in unrelated emitter drift — this one did not: the diff is exactly the two new ctx members across the six examples that had drifted. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0197KjhmBDB3PXAc6TXiPqZ9
Fixes from a two-pass adversarial review of the branch. Three were real
correctness bugs, all in the same place: reading a mutable per-request field
outside the window where it is valid.
**Subscription re-runs inherited the writing mutation's trace.** flushChangedTables
runs inside the dispatch try, so a re-run built its ctx while currentRequestTrace
still held the mutation's anchor — N re-run queries merged into one bogus trace
rooted at an unrelated mutation, and shipped that to the collector. The anchor is
now threaded into buildCtx explicitly, discriminated on options.identity exactly
as the identity itself already is, so a deferred caller mints its own.
**recordDispatchRootSpan read the anchor after the handler's awaits**, so with two
interleaved dispatches A and B, A's root span was filed under B's trace and B got
none. Captured into a local at entry and passed in.
**Concurrent siblings were mis-parented.** The ambient span stack recorded
`Promise.all([trace("a"), trace("b")])` with b as a CHILD of a, because b starts
while a is on the stack. A stack cannot express this, and telling "inside a" from
"concurrent with a" needs AsyncLocalStorage — which this package deliberately
avoids (dependency-tracker.ts documents why: shard DOs run under a slimmer compat
profile than nodejs_compat). So nesting is now explicit: a span's body receives a
tracer bound to that span. Correct in every case, visible at the call site, and
consistent with how identity and the dependency tracker are already threaded.
Calling ctx.trace inside a body still works — that span is parented to the
dispatch instead, flatter but never wrong. Regression-tested with a real
Promise.all.
Also:
- ctx.log now correlates via the resolved anchor rather than re-parsing the
inbound traceparent, so a dispatch that MINTED its ids no longer silently
splits its logs from its spans — which is what the docs already promised.
- Dropped startTimeUnixNano from delta Sum/Histogram points: equal start and end
declares a zero-width aggregation window that collectors may discard.
- otlpSpanBody always emits INTERNAL; the SERVER branch was unreachable.
- Removed emitSpanEvent/emitMetricEvent — uncalled, untested, and structurally
uncallable (spans originate in @lunora/do, which cannot import the runtime).
- foldTraces slices to the limit BEFORE folding and memoizes depth across the
group; it ran on every write flush for a live subscriber, folding 500 spans to
return 50. Split into groupByTrace/selectAnchor/depthResolver.
- One shared encodeSignalAttributes: the caller-overrides-reserved-key precedence
is a wire contract that was re-implemented in all three signal encoders.
- combineSinks: four identical fan-out loops folded into one policy.
- Drift guards for the studio's TraceSpan/TraceSummary mirrors, per the existing
STUDIO_FEATURE_KEYS precedent — the mirror had already drifted inside this
branch, still documenting the ordering that 96ba4d9 corrected.
- Moved the pure resolveTraceAnchor/toErrorType out of the 8.5k-line shard-do.ts
into trace-context.ts, the principle that moved parseLogArgs out.
- LogSink -> TelemetrySink (it carries spans and metrics too), with a deprecated
alias; SpanEvent.root -> dispatch (it is not the collector-side root).
- Studio: record the browsed shard in history like the nine other shard-scoped
panels, and stop hiding attributes on failed spans — the one span where you
most want to see them.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0197KjhmBDB3PXAc6TXiPqZ9
Addresses the review's headline structural finding: shard-do.ts was 8.5k lines and this branch had added ~200 more to it, regressing the principle that moved parseLogArgs/isLogFields out in the first place — module-level logic that does not need the shard instance. createTracer/createMetrics/dispatchRootSpan now live in ctx-telemetry.ts with an explicit dependency set (anchor, functionPath, shardKey, userId, and a `record` callback), so they decide only WHAT a span or measurement is, never where it goes. shard-do keeps ~15 lines of wiring per signal instead of ~200, and is down to 8.3k lines. The payoff is testability: the span and measurement semantics are now tested directly against the factories, and the TracingShard subclass — which existed only to reach `protected` methods — shrank to the one getTraces integration test that genuinely needs a Durable Object. The extraction surfaced a real weakness. The "recording must never break the handler" guard lived only in ShardDO.recordSpan/recordMetric, so a caller injecting a raw `record` would silently lose it — and a throw there would replace the body's own error with a telemetry one, after the body had already succeeded. The invariant now lives in the factory that promises it; the shard's guards remain as belt-and-braces for the sink specifically. Also adds a cross-package drift guard for CtxTracer/CtxMetrics vs the server's LunoraTracer/LunoraMetrics, in @lunora/testing — the only package depending on both, so it costs no new dependency edge. It asserts mutual assignability rather than key equality, so a changed parameter or return type fails the build, not just an added/removed member. Verified to fire on real drift; the one blind spot (a trailing optional parameter stays assignable) is documented rather than implied away. Moving the shared ctx types into `shared/` instead was considered and rejected: @lunora/server currently imports nothing from there, and per CLAUDE.md becoming a consumer means dropping outDir/rootDir from its tsconfig — a real structural cost to the most central package for a type-only dedup that a guard already covers. API snapshots regenerated: they had drifted across this whole branch, not just this commit — `api:update` had not been run since the first observability commit. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0197KjhmBDB3PXAc6TXiPqZ9
The automated insert put `trace` before `now` and left a stray blank line. Cosmetic, but the repo sorts object keys and the ESLint rule that would normally catch it can't run (eslint-plugin-sonarjs@4.0.3 crashes on ESLint 10). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0197KjhmBDB3PXAc6TXiPqZ9
…fer/panel
Traces: getTraces now reports `total` (distinct traces in the ring) alongside
the capped `traces`, so the Studio panel flags "showing N of M" when older
traces exist but weren't returned. foldTraces returns `{ total, traces }` — the
distinct-trace count is free, it already computes the by-trace map.
Metrics: add a shard-level MetricBuffer that folds ctx.metrics.* measurements
into one running aggregate per series (name+kind+dimensions → count/sum/min/max/
last), bounded by distinct-series count with least-recently-updated eviction. A
bounded ring of raw samples can't represent a metric's value (it evicts the
oldest samples a running total needs) — a bounded map of aggregates can, and
resets on hibernation like the log/span readouts. recordMetric folds into it and
still fans out to the sink; a new getMetricSeries admin RPC serves it, surfaced
as an Instruments section on the Studio Metrics page.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019M6G6CAoLVrQMxDYg7BWq2
The mock dev-client (pnpm --filter @lunora/studio dev:mock) had no getTraces or getMetricSeries fixtures, so the Traces panel and the new Instruments section rendered empty during UI iteration. Seed both: - getTraces: four representative folded waterfalls — deep nesting, structured span attributes, a fast single-child trace, and an errored child — with `total` above the returned count so the "showing N of M" notice renders. - getMetricSeries: seven aggregated series across all three instrument kinds, with and without dimensions, exercising the counter-total / gauge-reading / histogram-mean projections. Verified both panels against the seed in the mock harness. One refinement it surfaced: the Instruments RANGE column showed "1–1" for a +1 counter — collapse a degenerate min===max range to a dash so only a real spread (gauge/histogram) carries a range. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019M6G6CAoLVrQMxDYg7BWq2
- Extract metricHeadline/formatMetricValue from instruments-table into a pure instrument-format module, so the component file exports only its component (React Doctor only-export-components) — mirrors traces-panel/trace-geometry. - Give the trace waterfall bar gridcell an aria-label with the span's offset and duration; it carried no text, so a screen reader announced an empty cell (React Doctor control-has-associated-label). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019M6G6CAoLVrQMxDYg7BWq2
…onto alpha Completes the rebase of the ctx.trace/ctx.metrics work onto alpha (which now carries the TS6 lint toolchain from #150). Regenerates the do/studio api snapshots to match the merged code and applies prettier to the hand-resolved conflict files. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0197KjhmBDB3PXAc6TXiPqZ9
prisis
force-pushed
the
feat/observability-traces-metrics
branch
from
July 21, 2026 06:36
4cca022 to
374523e
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/docs/src/content/docs/concepts/observability.mdx (1)
302-306: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUpdate the JSON payload documentation to include metrics.
Since metrics support was added to the wire contract table here, consider also updating the downstream text (around line 321) to include
ExportMetricsServiceRequestalongsideExportTraceServiceRequestandExportLogsServiceRequestto keep the encoding documentation complete.🤖 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 `@apps/docs/src/content/docs/concepts/observability.mdx` around lines 302 - 306, Update the JSON payload documentation near the observability wire contract table to include ExportMetricsServiceRequest alongside ExportTraceServiceRequest and ExportLogsServiceRequest, ensuring the encoding description covers metrics consistently with the documented metrics endpoint.packages/studio/src/features/reports/instruments-table.tsx (1)
96-105: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueEnsure unique
data-testidvalues for robust testing.If multiple series share the same instrument name but differ in dimensions (such as the
http.requestsmetrics in the mock data), thedata-testidattributes on the row and the value cell will be duplicated. Consider using the already computedkeyto ensure uniqueness.💡 Proposed refactor
- <TableRow data-testid={`mt-instrument-${s.name}`} key={key}> + <TableRow data-testid={`mt-instrument-${key}`} key={key}> <TableCell className="font-mono text-xs" title={s.functionPath}> {s.name} </TableCell> <TableCell> <Badge variant={KIND_VARIANT[s.kind]}>{kindLabel(s.kind)}</Badge> </TableCell> - <TableCell className="text-right font-mono tabular-nums" data-testid={`mt-instrument-value-${s.name}`}> + <TableCell className="text-right font-mono tabular-nums" data-testid={`mt-instrument-value-${key}`}> {formatMetricValue(metricHeadline(s))} </TableCell>🤖 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/features/reports/instruments-table.tsx` around lines 96 - 105, Update the `data-testid` values on the `TableRow` and metric value `TableCell` in the instruments table to incorporate the already computed `key`, ensuring rows with identical `s.name` values but different dimensions remain uniquely addressable. Preserve the existing test ID prefixes and displayed content.
🤖 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.
Nitpick comments:
In `@apps/docs/src/content/docs/concepts/observability.mdx`:
- Around line 302-306: Update the JSON payload documentation near the
observability wire contract table to include ExportMetricsServiceRequest
alongside ExportTraceServiceRequest and ExportLogsServiceRequest, ensuring the
encoding description covers metrics consistently with the documented metrics
endpoint.
In `@packages/studio/src/features/reports/instruments-table.tsx`:
- Around line 96-105: Update the `data-testid` values on the `TableRow` and
metric value `TableCell` in the instruments table to incorporate the already
computed `key`, ensuring rows with identical `s.name` values but different
dimensions remain uniquely addressable. Preserve the existing test ID prefixes
and displayed content.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ef86fd8f-6343-4643-993e-342b33455668
⛔ Files ignored due to path filters (28)
api-snapshots/do.api.mdis excluded by none and included by noneapi-snapshots/lunora.api.mdis excluded by none and included by noneapi-snapshots/runtime.api.mdis excluded by none and included by noneapi-snapshots/server.api.mdis excluded by none and included by noneapi-snapshots/studio.api.mdis excluded by none and included by noneexamples/auth-playground/lunora/_generated/shard.tsis excluded by!**/_generated/**and included by noneexamples/blog/lunora/_generated/shard.tsis excluded by!**/_generated/**and included by noneexamples/expo/lunora/_generated/shard.tsis excluded by!**/_generated/**and included by noneexamples/offline-rejections/lunora/_generated/shard.tsis excluded by!**/_generated/**and included by noneexamples/payment-demo/lunora/_generated/shard.tsis excluded by!**/_generated/**and included by noneexamples/realtime-cursors/lunora/_generated/shard.tsis excluded by!**/_generated/**and included by noneexamples/todo-app/lunora/_generated/shard.tsis excluded by!**/_generated/**and included by nonepackages/codegen/__tests__/fixtures/simple/expected/_generated/shard.tsis excluded by!**/_generated/**,!**/__tests__/**and included bypackages/**packages/do/__tests__/metric-buffer.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/do/__tests__/tracing.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/runtime/__tests__/observability-sinks.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/server/__tests__/context.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/server/__tests__/functions-v-from.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/server/__tests__/functions.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**packages/studio/__tests__/app/studio.test.tsxis excluded by!**/__tests__/**and included bypackages/**packages/studio/__tests__/features/reports/instruments-table.test.tsxis excluded by!**/__tests__/**and included bypackages/**packages/studio/__tests__/features/traces/traces-panel.test.tsxis excluded by!**/__tests__/**and included bypackages/**packages/testing/__tests__/ctx-telemetry-contract.test.tsis excluded by!**/__tests__/**,!**/*.test.tsand included bypackages/**shared/log-event.tsis excluded by none and included by noneshared/metric-event.tsis excluded by none and included by noneshared/otlp.tsis excluded by none and included by noneshared/span-event.tsis excluded by none and included by none
📒 Files selected for processing (26)
apps/docs/src/content/docs/concepts/observability.mdxapps/studio/src/mock/dev-client.tspackages/codegen/src/emit.tspackages/do/src/ctx-telemetry.tspackages/do/src/index.tspackages/do/src/introspect.tspackages/do/src/log-buffer.tspackages/do/src/metric-buffer.tspackages/do/src/shard-do.tspackages/do/src/span-buffer.tspackages/do/src/trace-context.tspackages/runtime/src/index.tspackages/runtime/src/observability-sinks.tspackages/runtime/src/observability.tspackages/server/src/index.tspackages/server/src/types.tspackages/studio/src/app/studio.tsxpackages/studio/src/features/logs/logs-panel.tsxpackages/studio/src/features/reports/instrument-format.tspackages/studio/src/features/reports/instruments-table.tsxpackages/studio/src/features/reports/metrics-panel.tsxpackages/studio/src/features/traces/trace-geometry.tspackages/studio/src/features/traces/traces-panel.tsxpackages/studio/src/lib/admin.tspackages/studio/src/locales/en.tspackages/testing/src/harness.ts
🚧 Files skipped from review as they are similar to previous changes (22)
- packages/server/src/index.ts
- packages/runtime/src/index.ts
- packages/do/src/trace-context.ts
- packages/do/src/log-buffer.ts
- packages/studio/src/features/traces/trace-geometry.ts
- packages/studio/src/features/logs/logs-panel.tsx
- packages/do/src/introspect.ts
- packages/studio/src/locales/en.ts
- packages/runtime/src/observability.ts
- packages/studio/src/lib/admin.ts
- packages/do/src/ctx-telemetry.ts
- packages/studio/src/app/studio.tsx
- packages/studio/src/features/traces/traces-panel.tsx
- packages/codegen/src/emit.ts
- packages/do/src/index.ts
- packages/server/src/types.ts
- packages/do/src/span-buffer.ts
- packages/do/src/metric-buffer.ts
- packages/testing/src/harness.ts
- packages/runtime/src/observability-sinks.ts
- packages/do/src/shard-do.ts
- packages/studio/src/features/reports/metrics-panel.tsx
… alpha Rebasing the ctx.trace/ctx.metrics work onto alpha put its new code under the now-working lint toolchain (from #150) for the first time, surfacing ~140 errors. All fixed to zero (the CI eslint gate); the ~188 remaining warnings are the repo-wide advisory test-hygiene debt, non-blocking and shared with alpha. Notable, beyond the mechanical autofixes (import sorting, ReadonlyArray, jsdoc): - prevent-abbreviations: renamed CtxTracer/CtxMetrics -> Context* and the file ctx-telemetry.ts -> context-telemetry.ts, matching alpha's convention (it had already renamed CtxLogger -> ContextLogger via #150). - span-buffer: a real find — the depth-walk's `while (current !== undefined)` was statically always-true (the loop breaks before it would assign an absent parent), so it became `for (;;)` driven by its internal breaks. Every sort()/reverse() there is on a fresh/local array, so toSorted()/toReversed() are behavior-identical. - prefer-single-call on `buffer.push(a); buffer.push(b)` in the metric/span tests is a FALSE POSITIVE — SpanBuffer/MetricBuffer.push are single-arg custom methods, not Array#push, so combining the calls would silently drop all but the first. File-level disables with that rationale. - prefer-strict-boolean-matchers auto-converted `getByTestId(...).toBeTruthy()` to `.toBe(true)` in the traces panel test — but a DOM element is truthy, not `=== true`, so it broke the test. Reverted with a file-level disable so the unsafe autofix can't re-apply. - two cognitive-complexity thresholds (the DO's central `fetch` router and the flat admin-RPC dispatch chain, each 1-2 over after the trace/metrics branches) and one member-ordering: justified disables rather than risky refactors of hot-path code. Verified: 0 eslint errors, lint:types 59/59, full test suite 48/48, api snapshots regenerated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0197KjhmBDB3PXAc6TXiPqZ9
…t files The observability PR's new/modified test files had 4 advisory warnings: three require-mock-type-parameters on sentrySink's capture mock (typed as (event: ObservabilityEvent) => void) and one no-conditional-in-test (the defensive if was redundant — the series exists right after the push). The remaining ~184 warnings are pre-existing repo-wide test-hygiene debt in packages this branch does not touch (errors, mcp, agent, browser, queue, …); they belong in a dedicated alpha sweep, not bundled into a feature PR. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0197KjhmBDB3PXAc6TXiPqZ9
prisis
added a commit
that referenced
this pull request
Jul 21, 2026
…exemplars Squashed: durable ctx.metrics rollups + trend charts + exemplars, with the thermo-review fixes folded in (text-safe series key, raw SQL handle, canonical join). Rebased onto alpha after #149 (the ctx.trace/ctx.metrics base) merged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019M6G6CAoLVrQMxDYg7BWq2
This was referenced Jul 21, 2026
prisis
added a commit
that referenced
this pull request
Jul 21, 2026
…159) The metric series key used a raw NUL byte as its separator, which made git classify packages/do/src/metric-buffer.ts as binary — invisible in diff, blame, and PR review (it shipped that way via #149). Switch to the unicode escape sequence: byte-identical at runtime, valid UTF-8 text on disk. Guard against recurrence: a new scripts/no-nul-bytes.mjs runs in vis.config.ts's staged pre-commit chain (before Prettier, which doesn't catch it) and rejects any staged source file containing a raw NUL byte. Claude-Session: https://claude.ai/code/session_019M6G6CAoLVrQMxDYg7BWq2 Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
prisis
added a commit
that referenced
this pull request
Jul 21, 2026
…exemplars Squashed: durable ctx.metrics rollups + trend charts + exemplars, with the thermo-review fixes folded in (text-safe series key, raw SQL handle, canonical join). Rebased onto alpha after #149 (the ctx.trace/ctx.metrics base) merged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019M6G6CAoLVrQMxDYg7BWq2
prisis
added a commit
that referenced
this pull request
Jul 21, 2026
…exemplars Squashed: durable ctx.metrics rollups + trend charts + exemplars, with the thermo-review fixes folded in (text-safe series key, raw SQL handle, canonical join). Rebased onto alpha after #149 (the ctx.trace/ctx.metrics base) merged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019M6G6CAoLVrQMxDYg7BWq2
prisis
added a commit
that referenced
this pull request
Jul 22, 2026
…exemplars Squashed: durable ctx.metrics rollups + trend charts + exemplars, with the thermo-review fixes folded in (text-safe series key, raw SQL handle, canonical join). Rebased onto alpha after #149 (the ctx.trace/ctx.metrics base) merged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019M6G6CAoLVrQMxDYg7BWq2
prisis
added a commit
that referenced
this pull request
Jul 22, 2026
…exemplars Squashed: durable ctx.metrics rollups + trend charts + exemplars, with the thermo-review fixes folded in (text-safe series key, raw SQL handle, canonical join). Rebased onto alpha after #149 (the ctx.trace/ctx.metrics base) merged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019M6G6CAoLVrQMxDYg7BWq2
13 tasks
This was referenced Aug 16, 2026
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.
What & why
Closes the two missing OpenTelemetry pillars. The framework already had structured logging (
ctx.log) and W3C trace-context propagation worker→shard→container — but a handler had no way to instrument a sub-operation, and there were no metrics at all: noctx.metrics, andotlpSinkposted only/v1/tracesand/v1/logs.Concretely, before this PR the
traceId/spanIdthat #143 threads onto every log line were dead data — nothing rendered them, and one SERVER span per dispatch was the entire worker trace surface. A slow request was a single opaque bar.What's new
ctx.trace(name, fn, attributes?)— user-created spans nested under the dispatch. Returns the body's value unchanged; a throw is recorded as an error span and re-thrown (instrumentation, never flow control).ctx.metrics.{count,gauge,record}— counters, gauges, histograms → OTLP/v1/metricsas monotonicSum/Gauge/Histogram.ctx.loglevels onto 4, sotraceandfatal(added in feat(observability): structured ctx.log fields, trace correlation, durable log sink #147) were unreadable in the only place they show up locally.The one API call worth reviewing
Nesting is explicit — a span's body receives a tracer bound to that span:
I built it the other way first (an ambient "currently open span" stack, so bare
ctx.tracenested lexically). It reads better and it is unfixably wrong under concurrency: inPromise.all([trace("a"), trace("b")]),bstarts whileais on the stack and gets recorded as a child ofarather than its sibling. A reviewer reproduced it — a 5ms span rendered nested inside an unrelated 30ms one. Parallel fan-out is a mainline reason to reach for a tracer, so silent mis-parenting was not acceptable.Telling "called inside
a" from "called concurrently witha" needsAsyncLocalStorage, which this package deliberately avoids —dependency-tracker.tsdocuments why (shard DOs run under a slimmer compat profile thannodejs_compat). So the parent is threaded, exactly like the dependency tracker and the subscription identity. Callingctx.traceinside a body still works; that span parents to the dispatch instead — flatter, never wrong.Bugs found by review (all fixed, all regression-tested)
Two adversarial review passes over the branch found three real correctness bugs, all the same root cause — reading a mutable per-request field outside the window where it is valid:
flushChangedTablesruns inside the dispatchtry, so a re-run built its ctx whilecurrentRequestTracestill held the mutation's anchor — N re-run queries merged into one bogus trace rooted at an unrelated mutation, andotlpSinkshipped that to the collector. The anchor is now threaded intobuildCtx, discriminated onoptions.identityexactly as the identity already is.recordDispatchRootSpanread the anchor after the handler's awaits — with interleaved dispatches A and B, A's root span was filed under B's trace and B got none.Promise.allmis-parenting above.Plus:
ctx.lognow correlates via the resolved anchor instead of re-parsing the inboundtraceparent, so a dispatch that minted its ids no longer silently splits its logs from its spans — which the docs already promised it didn't.Breaking changes
QueryCtx/MutationCtx/ActionCtxgain requiredtraceandmetricsmembers. No runtime break in-repo (codegen has exactly one ctx construction site), but it is a compile-time break for externally hand-rolled ctx doubles.@lunora/do's publicLogLevelwidens 4 → 7 members, breaking exhaustive switches andRecord<LogLevel, X>downstream.LogSink→TelemetrySink(it carries spans and metrics now), with a deprecated alias so existing imports keep working.Verification
origin/alpha(36 release commits) and re-verified there._generatedregenerated.Known gaps (deliberate)
eslint-plugin-sonarjs@4.0.3crashes at plugin load on ESLint 10.7.0; verified on an untouched package with this branch stashed. The pre-commit hook aborts, so these commits have Prettier applied but ESLint never run. Needs a dependency fix independent of this PR.🤖 Generated with Claude Code
Summary by CodeRabbit
ctx.tracenested sub-operation tracing andctx.metricscounters/gauges/records with outcome/error capture.