Skip to content

feat(runtime,do): head + error-biased trace sampling - #166

Merged
prisis merged 6 commits into
alphafrom
feat/trace-sampling
Jul 22, 2026
Merged

feat(runtime,do): head + error-biased trace sampling#166
prisis merged 6 commits into
alphafrom
feat/trace-sampling

Conversation

@prisis

@prisis prisis commented Jul 22, 2026

Copy link
Copy Markdown
Member

Tier 0 — trace sampling (mirrors Cloudflare head_sampling_rate)

Configurable head sampling with an always-keep-errors tail bias, so high-traffic apps cut trace volume without losing error traces.

  • Config CreateWorkerOptions.sampling = { headRate?: 0..1 (default 1), alwaysSampleErrors?: boolean (default true) }.
  • Deterministic per-trace decision from traceId (shared/sampling.ts, zero-dep) → a trace is wholly kept or dropped (no half traces).
  • SERVER span gated in emitRpcEvent; ctx.trace INTERNAL spans gated in the DO — the head decision propagates via the W3C traceparent sampled flag + an x-lunora-sample-errors header (no DO codegen config needed). Spans always buffer locally (Studio panel unaffected); the export decision is sampled || (alwaysSampleErrors && traceErrored), made at the flush choke point so the error-keep works even when the error is only known at the end.
  • Backward-compatible: unset sampling ⇒ everything exported as before.

Verify: runtime 585 / do 1158 tests pass; tsc + eslint clean; container otel consumers still pass.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added configurable trace sampling for RPC dispatch, including deterministic per-trace head sampling.
    • Added tail-bias behavior to optionally keep entire traces when errors occur.
    • Updated runtime observability to respect sampling decisions and expose trace-sampling configuration.
  • Bug Fixes
    • Improved trace export so sampled-out traces can be retained for failures when configured.
    • Ensured per-trace sampling state is cleared after each RPC dispatch.

Add a configurable trace-sampling policy to the observability pipeline,
mirroring Cloudflare Workers' head_sampling_rate. A deterministic per-trace
head decision (derived from the traceId) keeps or drops a whole trace, biased
to always keep traces that produced an error span (tail bias). Only trace spans
are governed; metrics and logs are untouched.

- shared/sampling.ts: zero-dep deterministic helpers (traceIdToUnitInterval,
  isTraceHeadSampled, resolveTraceSampling, shouldExportTrace).
- shared/otlp.ts: buildTraceparent encodes the sampled flag; parseTraceparent
  returns it, so the runtime propagates its head decision to shards.
- runtime: CreateWorkerOptions.sampling; emitRpcEvent gates the SERVER span;
  dispatchSingleShard sets the traceparent flag + x-lunora-sample-errors header.
- do: ShardDO reads the propagated flags, holds sampled-out ctx.trace spans out
  of the live export, and re-decides at the dispatch finally (export on error).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0197KjhmBDB3PXAc6TXiPqZ9
@netlify

netlify Bot commented Jul 22, 2026

Copy link
Copy Markdown

Deploy Preview for lunorash ready!

Name Link
🔨 Latest commit f10e116
🔍 Latest deploy log https://app.netlify.com/projects/lunorash/deploys/6a60ed2aa3353e0008688b28
😎 Deploy Preview https://deploy-preview-166--lunorash.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 798d9c83-9633-4678-95df-4b26ba87b978

📥 Commits

Reviewing files that changed from the base of the PR and between 7f4f1fe and f10e116.

⛔ Files ignored due to path filters (3)
  • api-snapshots/lunora.api.md is excluded by none and included by none
  • api-snapshots/runtime.api.md is excluded by none and included by none
  • packages/do/__tests__/shard-do.sampling.test.ts is excluded by !**/__tests__/**, !**/*.test.ts and included by packages/**
📒 Files selected for processing (1)
  • packages/do/src/shard-do.ts

Walkthrough

WorkerOptions now supports deterministic trace sampling and error tail bias. Sampling decisions propagate through traceparent and request headers, RPC events are filtered accordingly, and ShardDO defers sampled-out span export until dispatch completion.

Changes

Trace sampling and tail-biased export

Layer / File(s) Summary
Sampling-aware observability contract
packages/runtime/src/observability.ts, packages/runtime/src/index.ts
emitRpcEvent accepts sampling configuration, suppresses non-exportable trace events, and re-exports TraceSamplingConfig.
Worker sampling propagation
packages/runtime/src/create-worker.ts
WorkerOptions.sampling configures per-trace decisions; dispatchSingleShard propagates sampled and error-retention flags and passes sampling to success and failure observability events.
ShardDO tail-biased span export
packages/do/src/shard-do.ts
ShardDO parses inbound sampling headers, buffers sampled-out spans, flushes them for retained error traces, and resets request-scoped sampling state afterward.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Worker
  participant ShardDO
  participant ObservabilitySink
  Worker->>Worker: Resolve trace sampling decision
  Worker->>ShardDO: Dispatch with traceparent and error-retention header
  ShardDO->>ShardDO: Buffer spans and track dispatch outcome
  Worker->>ObservabilitySink: Emit sampled RPC event
  ShardDO->>ObservabilitySink: Flush retained error-trace spans
Loading

Possibly related PRs

  • anolilab/lunora#143: Updates worker-to-ShardDO trace context propagation used by this sampling flow.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description has a useful summary, but it omits required sections like linked issues, test plan checkboxes, checklist, notes, and the CLA text. Add the missing template sections, especially linked issues, a concrete test plan, checklist items, reviewer notes, and the required CLA statement.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise, conventional, and accurately summarizes the main change: trace sampling with error bias in runtime/DO.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/trace-sampling

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.

❤️ Share

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

@github-actions

Copy link
Copy Markdown
Contributor

Thank you for following the naming conventions! 🙏

@github-actions

Copy link
Copy Markdown
Contributor

Thank you for confirming the Contributor License Agreement! 🙏

@codspeed-hq

codspeed-hq Bot commented Jul 22, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 160 untouched benchmarks
⏩ 1 skipped benchmark1


Comparing feat/trace-sampling (f10e116) with alpha (fc26783)2

Open in CodSpeed

Footnotes

  1. 1 benchmark was skipped, so the baseline result was used instead. If it was deleted from the codebase, click here and archive it to remove it from the performance reports.

  2. No successful run was found on alpha (d4f746c) during the generation of this report, so fc26783 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 4847-4909: The sampled-out trace state in recordSpan and
flushSampledOutTrace is stored in shared per-instance fields, allowing
interleaved dispatches to overwrite sampling and sink decisions. Snapshot the
sampling decision, trace identity, and sink within each dispatch’s tracer/anchor
context (leveraging makeTracer’s per-call sink closure), or key deferred
held-trace state by traceId, so recordSpan and flushSampledOutTrace use the
originating dispatch’s state and preserve error flushing under 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: 650385eb-7039-4b03-8b4d-c06a98f348bb

📥 Commits

Reviewing files that changed from the base of the PR and between d71f8f1 and 7f4f1fe.

⛔ Files ignored due to path filters (4)
  • packages/do/__tests__/shard-do.sampling.test.ts is excluded by !**/__tests__/**, !**/*.test.ts and included by packages/**
  • packages/runtime/__tests__/sampling.test.ts is excluded by !**/__tests__/**, !**/*.test.ts and included by packages/**
  • shared/otlp.ts is excluded by none and included by none
  • shared/sampling.ts is excluded by none and included by none
📒 Files selected for processing (4)
  • packages/do/src/shard-do.ts
  • packages/runtime/src/create-worker.ts
  • packages/runtime/src/index.ts
  • packages/runtime/src/observability.ts

Comment thread packages/do/src/shard-do.ts
Records the new TraceSamplingConfig interface, the `sampling` worker option,
and the emitRpcEvent sampling arg in the runtime/lunora api-snapshots.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0197KjhmBDB3PXAc6TXiPqZ9
…ispatches

CodeRabbit (#166, Major): the sampled-out / keep-errors / held-sink state was
stored in flat per-instance fields (`currentTraceSampled`,
`currentTraceKeepErrors`, `currentDispatchSink`) and read in recordSpan /
flushSampledOutTrace, which run after a span body or the whole dispatch settles.
A Durable Object interleaves dispatches across await points, so a sibling
dispatch's `finally` could reset those fields first — leaking a sampled-out
trace's spans live, or (worse) dropping a sampled-out+errored trace's held spans
and defeating the always-sample-errors tail bias. The `currentRequestTrace`
identity check in recordSpan had the same staleness.

Replace the three fields with a single `traceSampling` Map keyed by traceId,
registered at dispatch entry and deleted in the `finally`; recordSpan looks up
by the span's own `traceId` and flushSampledOutTrace by the dispatch's trace, so
two concurrent dispatches can't clobber each other's verdict or sink. Adds an
interleaved-dispatch regression test (a slow sampled-out error trace whose flush
runs after a sibling sampled-in dispatch's finally).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0197KjhmBDB3PXAc6TXiPqZ9
@prisis
prisis merged commit 7f16ac1 into alpha Jul 22, 2026
11 of 13 checks passed
@prisis
prisis deleted the feat/trace-sampling branch July 22, 2026 16:17
prisis added a commit that referenced this pull request Jul 28, 2026
From better-auth-ui #195, #193, #169, #166, #74 and #337.

#195 — the organization limits are now published by uiConfig(). better-auth
enforces organizationLimit / allowUserToCreateOrganization server-side and
exposes no endpoint that reports them, so a UI can only find out by being
refused. The create form is disabled with the reason instead. A function-form
allowUserToCreateOrganization is a per-request decision the endpoint cannot
evaluate, so it reports allowed and lets the server refuse — the alternative
hides the button from everyone because one user might be denied.

#166 — the 2FA card no longer vanishes for an OAuth-only account; it explains
that a password is needed first. A setting that is simply absent reads as
'this app doesn't support 2FA'. Only a successful accounts read that found no
credential row counts: while loading, or on error, the card behaves as
before — the flow gate's rule, don't hide what you cannot reason about.

#74 — the TOTP key is shown beside the otpauth:// URI. Most authenticators
reject a pasted URI, so a desktop app or a broken camera had no way in.

#193 — createActiveMemberController resolves the signed-in user's role in the
active organization, which otherwise means fetching the org and finding
yourself in members at every call site. A rendering hint, not authorization.

#337 — organization.showSlug hides the slug field; create already derives one
from the name. #169 was already supported by that same fallback.

#165 (organization custom fields) is not done: it needs an additionalFields
design that spans every card, not a flag.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant