Skip to content

fix(webapp): stop api inheriting inbound sampled traceparents so trace sampling applies - #4532

Merged
ericallam merged 1 commit into
mainfrom
feature/tri-13031-api-trace-sampling-is-inert-996-of-its-spans-bypass-the
Aug 7, 2026
Merged

fix(webapp): stop api inheriting inbound sampled traceparents so trace sampling applies#4532
ericallam merged 1 commit into
mainfrom
feature/tri-13031-api-trace-sampling-is-inert-996-of-its-spans-bypass-the

Conversation

@ericallam

Copy link
Copy Markdown
Member

What

The internal tracing ParentBasedSampler in tracer.server.ts left remoteParentSampled at its default of AlwaysOn. Any request arriving with a traceparent whose sampled flag was set got recorded in full, bypassing INTERNAL_OTEL_TRACE_SAMPLING_RATE entirely. Because the SDK propagates its (always-sampled) trace context on calls back to the platform from inside running tasks, the large majority of API server spans inherited a sampled parent and ignored the divisor. The sampling knob was effectively inert on the busiest service.

This registers a custom propagator (NonInheritingTraceContextPropagator) that stops adopting the inbound trace as the parent:

  • inject still delegates to the standard W3C trace + baggage propagators, so outbound propagation is unchanged.
  • extract drops the parent span (trace.deleteSpan) while preserving baggage, so every incoming request roots its own trace and the ratio sampler applies uniformly.

remoteParentSampled is also set to the ratio sampler as a belt-and-suspenders fallback, in case an inbound sampled parent ever reaches the sampler another way.

Two effects: the divisor becomes effective on the API server, and the API no longer stitches onto (and inflates) the propagated task-run traces, which is where the very large, un-thinnable trace chains came from. Rooting each request removes those chains rather than only diluting them.

Only the internal APM trace pipeline (INTERNAL_OTEL_TRACE_EXPORTER_URL) is affected. The user-facing run-trace pipeline (otel.v1.traces -> ClickHouse) is a separate path and is untouched. The only consumer of the global propagator's extract is the OTel HTTP/Express auto-instrumentation, so the blast radius is inbound-request trace shape.

Evidence (local full-stack red/green, divisor 10)

A local OTLP/JSON sink counting spans; a driver fires N requests at a real endpoint, each carrying a distinct sampled traceparent, then counts how many spans/traces carry that run's marker.

runcodesentkept traceskept fraction
beforeunmodified5005001.00
afterthis PR500670.134
afterthis PR20002130.1065

Before: 100% of inherited-sampled requests kept, divisor ignored. After: ~10% kept (the divisor), converging on it at larger N. In every after-run each kept request is a single self-rooted trace (kept spans == kept distinct traces), confirming the inherited chains are gone, not just thinned. typecheck passes.

Rollout / rollback

No flag. Behavior stays governed by the existing INTERNAL_OTEL_TRACE_SAMPLING_RATE. Rollback is a straight revert with no data migration.

Notes

Internal dashboards that count raw span or request volume from this pipeline will read lower once this ships. That is expected: those counts were inflated by the bypass, not a real drop in traffic. Latency/percentile monitors retain plenty of samples at the current divisor.

refs TRI-13031

…e sampling applies
The internal APM tracer's ParentBasedSampler left remoteParentSampled at its
AlwaysOn default, so any request arriving with a sampled traceparent (SDK task-run
traces propagated in from running tasks) was recorded in full, ignoring
INTERNAL_OTEL_TRACE_SAMPLING_RATE. On api that is ~99.6% of spans, so the divisor
was effectively inert.
Register a non-inheriting propagator: inject still delegates to W3C trace+baggage
so outbound propagation is unchanged, but extract drops the parent span, so every
inbound request roots its own trace and the ratio sampler applies uniformly. This
also stops api stitching onto (and inflating) the SDK's task-run traces, which is
where the untrimmable multi-thousand-span chains came from. Also set
remoteParentSampled to the ratio sampler as a fallback.
@changeset-bot

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: b9f887d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai

coderabbitaiBot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

The webapp tracer now uses typed composite W3C trace-context and baggage propagators. Its non-inheriting propagator removes extracted span contexts while preserving propagation behavior. Provider registration installs this propagator with the existing context manager. Sampling now reuses one ratio sampler for custom root sampling and remote sampled-parent handling. A change note documents reduced internal API overhead under high load.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Docstring Coverage✅ PassedNo functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
Title check✅ PassedThe title clearly describes the main change: preventing inbound sampled traceparents from bypassing API trace sampling.
Description check✅ PassedThe description explains the change, testing evidence, affected pipelines, rollout, rollback, and expected impact in sufficient detail.
✨ 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 feature/tri-13031-api-trace-sampling-is-inert-996-of-its-spans-bypass-the

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.

coderabbitai[bot]

This comment was marked as resolved.

@ericallam
ericallam marked this pull request as ready for review August 7, 2026 14:05

@devin-ai-integrationdevin-ai-integrationBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

Open in Devin Review

Comment threadapps/webapp/app/v3/tracer.server.ts
@ericallam
ericallam merged commit 63176a6 into mainAug 7, 2026
45 checks passed
@ericallam
ericallam deleted the feature/tri-13031-api-trace-sampling-is-inert-996-of-its-spans-bypass-the branch August 7, 2026 14:13
@github-actionsgithub-actionsBot mentioned this pull request Aug 10, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@ericallam@nicktrn