feat(opentelemetry)!: Simplify propagation and remove custom OTel tracing utils - #22702
Open
mydea wants to merge 11 commits into
Open
feat(opentelemetry)!: Simplify propagation and remove custom OTel tracing utils#22702mydea wants to merge 11 commits into
mydea wants to merge 11 commits into
Conversation
Contributor
size-limit report 📦
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
mydea
force-pushed
the
fn/propagator
branch
2 times, most recently
from
July 29, 2026 11:15
c4a4b13 to
93d6542
Compare
mydea
force-pushed
the
fn/propagator
branch
2 times, most recently
from
July 29, 2026 14:13
afdbe2c to
7aa618c
Compare
mydea
commented
Jul 30, 2026
| scope.setPropagationContext({ | ||
| traceId: generateTraceId(), | ||
| sampleRand: safeMathRandom(), | ||
| return withActiveSpan(null, () => { |
Member
Author
There was a problem hiding this comment.
just switching this up, it should not make concrete difference but seems more resilient, as it makes us more independent of what withActiveSpan does under the hood!
mydea
commented
Jul 30, 2026
| } | ||
|
|
||
| /** Custom implementation for OTEL, so we can handle scope-span linking. */ | ||
| protected _getTraceInfoFromScope( |
Member
Author
There was a problem hiding this comment.
it seems this was not used anywhere anymore!
mydea
changed the title
SentryPropagator
mydea
marked this pull request as ready for review
July 30, 2026 07:54
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit dff4c15. Configure here.
s1gr1d
reviewed
Jul 30, 2026
| consoleSandbox(() => { | ||
| // oxlint-disable-next-line no-console | ||
| console.warn( | ||
| 'SentryPropagator: Injecting with a different context than the active one - this is not supported. Skipping injection.', |
Member
There was a problem hiding this comment.
We could make this more descriptive. A suggestion:
Suggested change
| 'SentryPropagator: Injecting with a different context than the active one - this is not supported. Skipping injection.', | |
| 'SentryPropagator: Injecting trace data of a different context than the active one is not supported. Skipping injection.', |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Bun does not emit the `http.server.request.start` diagnostics channel that the Node SDK relies on to isolate each incoming request. As a result, servers built on `node:http` (such as Next.js running via `bun --bun`) do not get a fresh isolation scope and trace per request, so unrelated requests can share one trace. `bunHttpIntegration` closes this gap by patching `http.Server.prototype.emit` and, on each server's first `request` event, handing the server to the same core instrumentation the Node SDK uses (`getHttpServerSubscriptions` → `instrumentServer`). We patch the prototype rather than `createServer` because the server is typically created and `listen()`ed before Sentry initializes (e.g. Next.js creates its server before running the `instrumentation.ts` `register()` hook). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds an assertion that an `http.server` span is created for incoming requests when spans are enabled, alongside the existing isolation and trace-continuation coverage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: isaacs <i@izs.me>
mydea
changed the base branch from
fn/bun-http-integration
to
fn/streamline-acs-isolation-scope
July 31, 2026 10:20
remove getTraceData & fixes fix traceparent propagation
mydea
force-pushed
the
fn/propagator
branch
2 times, most recently
from
July 31, 2026 11:26
cf12656 to
5d24bc4
Compare
Comment on lines
72
to
73
| } | ||
|
|
Contributor
There was a problem hiding this comment.
Bug: The check for a continuing trace in SentryPropagator.extract is unreliable, potentially causing new requests to inherit incorrect trace IDs from a pre-existing active context.
Severity: LOW
Suggested Fix
The check for a continuing trace should be more specific. Instead of just checking for the existence of a span context, verify that the context was actually modified by getContextWithRemoteActiveSpan. A better condition would be: const isContinuingTrace = ctxWithRemoteSpan !== ctx && trace.getSpanContext(ctxWithRemoteSpan) !== undefined;.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/opentelemetry/src/propagator.ts#L129-L130
Potential issue: In the `SentryPropagator.extract` method, the logic to determine if a
trace is being continued is flawed. The check `trace.getSpanContext(ctxWithRemoteSpan)
!== undefined` cannot distinguish between a new remote span being added versus the input
context already containing a span. If `extract` is called with a context that already
has an active span but no incoming trace headers are present, the check will incorrectly
evaluate to true. This prevents the creation of a new trace ID, causing the new request
to erroneously inherit the trace ID from the pre-existing, unrelated context. While
`AsyncLocalStorage` in Node.js mitigates this in most cases by providing context
isolation, the implementation remains technically incorrect and could lead to trace
context bleeding between requests under specific interleaving scenarios.
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
2 participants
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.
Simplifies the OpenTelemetry
SentryPropagatordown to a thinTextMapPropagatorthat delegates all trace-data serialization to core'sgetTraceData(), and removes the custom OTel-specific tracing/propagation overrides (continueTrace,startNewTrace,getTraceData,getTraceContextForScope) in favor of core's implementations. The net effect is that OTel-powered SDKs (Node, Next.js, SvelteKit, etc.) now share the same propagation and trace-continuation code paths as the rest of the SDK instead of maintaining a parallel OTel-only variant.What changed
SentryPropagatorno longer extendsW3CBaggagePropagator. It now implementsTextMapPropagatordirectly.inject()reads headers fromgetTraceData()and only operates on the active context — if called with a non-active context it warns and skips.tracePropagationTargetsfiltering andpropagateTraceparentare no longer the propagator's concern — outgoing-request filtering already lives in the Node HTTP layer (inject-trace-propagation-headers.ts), so the duplicated logic, the_urlMatchesTargetsMapLRU cache, and thesentry.urltrace-state (SENTRY_TRACE_STATE_URL) are removed.getTraceData,continueTrace, andstartNewTraceoverrides from the async context strategy. Core's implementations are now used directly. To make this work,continueTrace/startNewTracein core were reworked to route throughwithActiveSpan(null, …), and OTel's non-recording (TwP) spans fall back to the scope's propagation-context trace id.getTraceContextForScope(andNodeClient._getTraceInfoFromScope); scope→trace-context resolution now goes through the shared core path._startSpannow runs the callback with the started span set active on the unsuppressed context, fixing event trace-context attaching to a stale ancestor span acrossstartNewTrace/continueTraceboundaries.Aligning Node and Browser behavior
Previously, the OTel/Node path and the browser (core) path had subtly diverging propagation behavior because they ran through different implementations of
getTraceData,continueTrace, andstartNewTrace. The OTel variants derived trace data from the OTel context/span graph, while the browser used core's scope- and span-based logic — so edge cases (TwP sampling, DSC freezing, which trace id a fresh root span lands on) could resolve differently between the two.By deleting the OTel-specific overrides and routing everything through core, Node and Browser now produce trace headers and continue/start traces via a single common code path. There is one source of truth for how a
sentry-trace/baggagepair is derived from the current scope and span, which removes a class of Node-vs-Browser discrepancies and makes future changes apply uniformly to both.Test adjustments
Because this aligns Node's propagation/trace-continuation behavior with the common core path, a number of Node integration tests had to be updated to match the new (now shared) behavior — primarily expectations around trace ids for parallel/root spans and outgoing-request propagation (
parallel-root-spans,parallel-spans-in-scope,fetch-sampled-no-active-span, etc.). These changes reflect the corrected/unified behavior, not regressions.Breaking changes
getTraceContextForScopeis no longer exported from@sentry/opentelemetry.sentry.urltrace-state and thegetInjectionData/ OTel-specificgetTraceDatainternals of the propagator are removed.SentryPropagatorno longer injects when called with a non-active context.