feat(effect): Add opt-in for external span parents and isolate root spans - #23900
Conversation
07eefa2 to
61b80c5
Compare
size-limit report 📦
|
Lms24
left a comment
Lms24
left a comment
There was a problem hiding this comment.
Sorry for blocking but IIUC this allows a Sentry-instrumented Effect app to continue a trace based on any incoming traceparent header, correct?
If so, let's not merge this. We explicitly don't continue traceparent-initiated traces. We only propagate our traces via traceparent (if propagateTraceparent: true) to allow downstream services to continue a sentry trace in OTel-instrumented services.
8a32bfb to
c900a24
Compare
|
@Lms24 it is now changed to be opt-in. So users must explicitly add an extra layer to add this. const SentryLive = Layer.mergeAll(
Sentry.effectLayer({ dsn: '__DSN__', tracesSampleRate: 1.0 }),
Layer.setTracer(Sentry.SentryEffectTracer),
Sentry.SentryEffectExternalSpanLayer, // this must be added now
);To clarify, we talked offline about it. It has nothing to do with external traces coming in with a |
Lms24
left a comment
Lms24
left a comment
There was a problem hiding this comment.
Thanks for making the change! As discussed, we can go with this approach first, and if necessary revisit default behaviour.
For some more public context why I think opt-in behaviour is the way to go: If the Sentry SDK is used in a "sub app" and the surrounding SDK is instrumented with pure OTel, we still have the issue that it's not us making a sampling decision, but the OTel SDK (or it inheriting a trace and sampling decision from some other service via traceparent headers). So our span metrics extrapolation is potentially compromised, our own tracesSampleRate would always be overridden and it can also lead to cross-org/customer traces being continued. For these reasons, we so far stayed away from continuing traceparent-initiated traces. This is just a special case of that, where we have no service boundary crossing.
|
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you apply the label |
`@sentry/effect` ignored `Tracer.ExternalSpan` parents and fell back to the active Sentry span. An incoming `traceparent` header on an Effect HTTP server or a persisted trace continued with `Tracer.externalSpan` therefore started a disconnected trace, and a `root: true` span or a span leaked from another fiber through the async context could become the parent of an unrelated span. On the server, every parentless span also shared the process-wide propagation context, so a long-lived process put all of its work into one trace. - An external parent now continues its trace as a new root span with the external span as `parent_span_id`. No dynamic sampling context is frozen, so the SDK builds one from the client. - A parentless span only nests under a foreign active Sentry span (an `http.server` span from the Node SDK, a pageload), never under a span this tracer created. - The server tracer starts a new trace for every parentless span, unless the user set up the current scope (`continueTrace`, `withScope`, an isolation scope). The client tracer keeps parentless spans in the page trace. These fixes apply to both trace lifecycles. One related limitation stays and is specific to `traceLifecycle: 'static'`: a child span that ends after its root span is dropped with the transaction. Long-running Effect fibers, such as a background agent that outlives the request that started it, lose those children unless `traceLifecycle: 'stream'` is used, which sends every span on its own end and is the default since v11. The effect-3-node and effect-4-node e2e apps cover an incoming traceparent header, a `Tracer.externalSpan` parent, and a `root: true` span inside a request. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
c900a24 to
4f9aa86
Compare
JPeer264
changed the title
2 participants
@sentry/effectparented every Effect span on the active Sentry span. ATracer.ExternalSpanparent was dropped, so a persisted trace continued withTracer.externalSpanstarted a disconnected trace, and aroot: truespan or a span leaked from another fiber through the async context could become the parent of an unrelated span. On the server, every parentless span also shared the process-wide propagation context, so a long-lived process put all of its work into one trace.http.serverspan from the Node SDK, a pageload), never under a span this tracer created.continueTrace,withScope, an isolation scope). The client tracer keeps parentless spans in the page trace.parentoption: under the fiber's current span, or parentless.SentryEffectExternalSpanLayeropts into continuing external parents: the span becomes a new root span with the external span asparent_span_id. No dynamic sampling context is frozen, so the SDK builds one from the client. Next to the tracer layer it applies to the whole runtime, withEffect.provideto a single effect only there.Decisions
External parents are opt-in. A
Tracer.externalSpanparent bridges a trace the SDK did not start, for example an OpenTelemetry span of another app in the same process, or trace state persisted with a queue message. Joining such a trace silently would re-parent spans into a trace the SDK cannot vouch for, so the user adds the layer to ask for it. Incoming trace headers on the Effect HTTP server are out of scope here and get their own PR.The opt-in is a layer, not a second tracer. Two layers that set the tracer race inside
Layer.mergeAll, and a wrapping tracer only sees the Sentry one when nested withLayer.provide. A flag in the fiber context composes inLayer.mergeAlllikeLayer.setTracerdoes, and also works per effect. It is a plain service in both Effect versions, because v4 has noFiberRefand v3 has noContext.Reference.How the tracer reads the flag. Effect's
spanhook gets no fiber in either version, only the span's own context or annotations. Thecontexthook does get the fiber and wraps every operation the fiber evaluates, andspanruns synchronously inside it, so the hook keeps the fiber in a module variable for that extent, the same way it already sets the active Sentry span there. The flag is read only when an external parent shows up.These fixes apply to both trace lifecycles. One related limitation stays and is specific to
traceLifecycle: 'static': a child span that ends after its root span is dropped with the transaction. Long-running Effect fibers, such as a background agent that outlives the request that started it, lose those children unlesstraceLifecycle: 'stream'is used, which sends every span on its own end and is the default since v11.The effect-3-node and effect-4-node e2e apps cover a
Tracer.externalSpanparent continued through the layer and aroot: truespan inside a request.🤖 Generated with Claude Code