feat(core): Add deferred segment-span transaction capture#21839
Draft
andreiborza wants to merge 3 commits into
Draft
feat(core): Add deferred segment-span transaction capture#21839andreiborza wants to merge 3 commits into
andreiborza wants to merge 3 commits into
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
size-limit report 📦
|
andreiborza
force-pushed
the
ab/sentry-trace-provider-core-capture
branch
from
920659d to
1397906
Compare
andreiborza
changed the title
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
andreiborza
force-pushed
the
ab/sentry-trace-provider-otel
branch
from
a1613c2 to
6bd79ac
Compare
andreiborza
requested review from
JPeer264 and
mydea
and removed request for
a team
andreiborza
force-pushed
the
ab/sentry-trace-provider-core-capture
branch
from
29ce501 to
c741940
Compare
andreiborza
force-pushed
the
ab/sentry-trace-provider-otel
branch
2 times, most recently
from
e005612 to
924ce11
Compare
andreiborza
force-pushed
the
ab/sentry-trace-provider-core-capture
branch
from
c741940 to
4b3fc03
Compare
andreiborza
force-pushed
the
ab/sentry-trace-provider-otel
branch
from
924ce11 to
630bb67
Compare
andreiborza
force-pushed
the
ab/sentry-trace-provider-core-capture
branch
from
4b3fc03 to
14cb421
Compare
andreiborza
force-pushed
the
ab/sentry-trace-provider-otel
branch
from
630bb67 to
15154ed
Compare
andreiborza
force-pushed
the
ab/sentry-trace-provider-core-capture
branch
from
14cb421 to
6b8db6e
Compare
nicohrubec
reviewed
Jun 30, 2026
Comment on lines
+57
to
+62
| // Clients whose segment-span transaction capture should be deferred (rather than run synchronously on | ||
| // span end), mapped to the function that queues a deferred capture. Tracked per client rather than as | ||
| // a process-wide flag so pending captures and their timer cannot leak across `Sentry.init()` calls — | ||
| // a client that never opts in is simply absent. Enabled per client by SDKs that assemble transactions | ||
| // from the live span tree on root-span end (e.g. the Node SDK), which would otherwise drop children | ||
| // that close after it. Every other setup keeps its synchronous capture. |
Member
There was a problem hiding this comment.
Suggested change
| // Clients whose segment-span transaction capture should be deferred (rather than run synchronously on | |
| // span end), mapped to the function that queues a deferred capture. Tracked per client rather than as | |
| // a process-wide flag so pending captures and their timer cannot leak across `Sentry.init()` calls — | |
| // a client that never opts in is simply absent. Enabled per client by SDKs that assemble transactions | |
| // from the live span tree on root-span end (e.g. the Node SDK), which would otherwise drop children | |
| // that close after it. Every other setup keeps its synchronous capture. | |
| // Clients that opted into deferred segment-span capture (see `_INTERNAL_se | |
| tDeferSegmentSpanCapture`), | |
| // mapped to the function that queues a deferred capture. Keyed by client r | |
| ather than a process-wide | |
| // flag so pending captures and their timer cannot leak across `Sentry.init | |
| ()` calls. |
just a suggestion but I am wondering if we could cut this comment down a bit, reads quite lengthy 😅
Comment on lines
+65
to
+67
| // Spans already included in a captured transaction. Used so a child that ends after its root segment | ||
| // was captured can be emitted as its own orphan transaction (see `_onSpanEnded`) without any span ever | ||
| // being sent in more than one transaction. |
Member
There was a problem hiding this comment.
Suggested change
| // Spans already included in a captured transaction. Used so a child that ends after its root segment | |
| // was captured can be emitted as its own orphan transaction (see `_onSpanEnded`) without any span ever | |
| // being sent in more than one transaction. | |
| // Track spans already sent in a transaction, so we can emit spans ending after the root in a separate transaction |
| const CAPTURED_SPANS = new WeakSet<Span>(); | ||
|
|
||
| /** | ||
| * Defer a client's segment-span transaction capture. Set once by the SDK during setup (e.g. the Node |
Member
There was a problem hiding this comment.
same here, maybe we can cut this down a bit
| // fires on a later tick must reach the client active at span end and never whatever client | ||
| // is current when the timer fires (e.g. a different client after re-init), and the scope's | ||
| // client reference can be reassigned. Only the snapshot is deferred, so late children land. | ||
| client.captureEvent(transactionEvent, undefined, scope); |
Member
There was a problem hiding this comment.
q: the comment says do not resolve from the scope but then you pass the scope here, how does that work?
| debouncedDrain.flush(); | ||
| }); | ||
|
|
||
| DEFERRED_SEGMENT_SPAN_CAPTURES.set(client, capture => { |
Member
There was a problem hiding this comment.
as discussed offline: maybe we can think about simplifying this by not doing this per client or using an explicit queue class or something like that. if not also fine just think this is quite hard to understand at first pass
Add per-client deferral of the segment-span transaction capture. The transaction is otherwise assembled synchronously from the live span tree when the root span ends, dropping child spans whose instrumentation closes them after it - in the same tick (diagnostics-channel `asyncEnd`) or on a later tick (e.g. prisma engine spans). When a client opts in via `_INTERNAL_setDeferSegmentSpanCapture`, a debounced timer (the one the OpenTelemetry span exporter uses) delays the snapshot so those children land first, and drains on the client `flush` hook so `Sentry.flush()` / `close()` stays safe. The browser keeps its synchronous capture. The opt-in call is wired separately (the Node SDK enables it on the SentryTracerProvider path).
andreiborza
force-pushed
the
ab/sentry-trace-provider-otel
branch
from
15154ed to
7837eb8
Compare
andreiborza
force-pushed
the
ab/sentry-trace-provider-core-capture
branch
from
6b8db6e to
2df53ad
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 2df53ad. Configure here.
|
|
||
| expect(onSpy.mock.calls.filter(([hook]) => hook === 'flush')).toHaveLength(1); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Missing defer behavior tests
Low Severity
This feat change adds deferred transaction capture and orphan emission, but the diff only adds a unit test that the flush hook registers once. There is no integration or E2E test here exercising deferred assembly, flush draining, or orphan transactions.
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit 2df53ad. Configure here.
| !!DEFERRED_SEGMENT_SPAN_CAPTURES.get(client) && | ||
| !isBrowser() && | ||
| !CAPTURED_SPANS.has(this) && | ||
| CAPTURED_SPANS.has(rootSpan); |
Contributor
There was a problem hiding this comment.
Bug: If a root span is a SentryNonRecordingSpan, the check CAPTURED_SPANS.has(rootSpan) will always be false, silently disabling orphan segment handling for its children.
Severity: MEDIUM
Suggested Fix
The check for orphan segments should not depend on the root span being a SentrySpan that has been converted to a transaction. Re-evaluate the CAPTURED_SPANS.has(rootSpan) condition. The logic could be adjusted to correctly identify orphans even when the root span is non-recording, perhaps by tracking sent transactions differently.
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/core/src/tracing/sentrySpan.ts#L417
Potential issue: The logic to detect orphan segments relies on checking if the root span
is present in the `CAPTURED_SPANS` set. However, a span is only added to this set when
`_convertSpanToTransaction` is called, a method exclusive to `SentrySpan`. If the root
span of a trace is a `SentryNonRecordingSpan`, it will never be added to
`CAPTURED_SPANS`. Consequently, any child `SentrySpan` that finishes after the root will
fail the `isOrphanSegment` check at `sentrySpan.ts:417`, and its data will be lost
instead of being sent as an orphan segment.
Member
Author
|
I'm going to rework this slightly so it has no impact on browser SDKs. |
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.
What
Adds the ability to defer the assembly of transactions to avoid dropping spans from transactions that end shortly after the segment span itself. Additionally, it also handles children that end after the debounce fired and transactions have already been sent. Spans that don't quite make it will end up as their own transaction in the same trace instead of being dropped .
This mimics what is already done today in the span exporter (a buffer + debounced flush).
Why
SentrySpan assembles a transaction synchronously from the span tree the instant the segment span ends. But some child spans are closed by their instrumentation after the root ends.
For example:
asyncEnd/response callback that runs after the root handler returns.@prisma/instrumentationemits its engine spans on a later tick once it receives the engine trace data.Without deferral those children aren't in the tree yet at root-end, so they're silently dropped from the transaction. With the OTel SDK, this never happened because the
SentrySpanExporteralready buffers finished spans and flushes on a debounced timer. The SentryTracerProvider has no exporter, so defer reinstates that buffering window so late-ending children land before the snapshot.This is used and tested in #21680's integration/e2e tests.