Skip to content

Commit 152d6ed

Browse files
andreiborzaclaude
andauthored
fix(core): Bound child span tracking on long-lived spans (#22916)
## What Bounds the child spans a span keeps a reference to. A child is no longer tracked when it could never show up in what that span sends: - the parent is unsampled, so its tree is never read - the parent has stopped recording and so has its segment span, so the tree has been read for the last time Late children still resolve their root span, so they keep being sent on their own, and a late child that is itself still recording keeps collecting the subtree it is re-emitted with. Both conditions are read off the parent, so this holds for every capture path, including span streaming and deferred capture. ## Why A span that outlives its children retains every one of them. In NestJS the `Create Nest App` span stays the active parent of anything a `setInterval` from a provider constructor starts, so those spans are never released and the process leaks around 80 MB/h. The cutoff covers the whole tree, not only the segment span, so any span pinned in a long-lived async context is bounded the same way. Reproduction: https://github.com/andreiborza/sentry-nestjs-span-retention-repro (60 s at 20 rps, 12000 spans, post-GC heap snapshots). Children retained on the boot span and heap node growth over the run, both measured on this branch with and without the fix: | | retained | node growth | | --- | --- | --- | | before, span streaming (the default) | 12000 | +115.7% | | before, transactions | 12000 | +116.2% | | after, span streaming | 0 | -2.5% | | after, transactions | 0 | -1.7% | Neither trace lifecycle avoids it, and published `@sentry/nestjs@10.69.0` leaks the same way on both, so this is not new in v11. Closes: #22860 Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 1297420 commit 152d6ed

3 files changed

Lines changed: 137 additions & 3 deletions

File tree

packages/core/src/utils/spanUtils.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,23 @@ export function addChildSpanToSpan(span: SpanWithPotentialChildren, childSpan: S
378378
const rootSpan = span[ROOT_SPAN_FIELD] || span;
379379
addNonEnumerableProperty(childSpan, ROOT_SPAN_FIELD, rootSpan);
380380

381+
// `_sentryChildSpans` exists only so `getSpanDescendants()` can walk the tree when the segment span
382+
// is sent, and that walk stops at an unsampled span without ever visiting its children. So a child
383+
// tracked here would be held for the parent's lifetime and never read.
384+
if (!spanIsSampled(span)) {
385+
return;
386+
}
387+
388+
// Once the segment span stopped recording, the tree has been read for the last time, and a child
389+
// starting now belongs to whatever segment comes next: it is re-emitted on its own instead. Tracking
390+
// it here would pin it for as long as the parent lives, which for a span left active in an async
391+
// context (e.g. a framework boot span captured by a queue consumer) is the rest of the process. Only
392+
// a parent that is itself still recording keeps tracking, so a late child that outlives its segment
393+
// still collects the subtree it is re-emitted with.
394+
if (!span.isRecording() && !rootSpan.isRecording()) {
395+
return;
396+
}
397+
381398
// We store a list of child spans on the parent span
382399
// We need this for `getSpanDescendants()` to work
383400
if (span[CHILD_SPANS_FIELD]) {

packages/core/test/lib/tracing/sentrySpan.test.ts

Lines changed: 52 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,19 @@ import {
88
} from '../../../src/semanticAttributes';
99
import { SentrySpan } from '../../../src/tracing/sentrySpan';
1010
import { SPAN_STATUS_ERROR } from '../../../src/tracing/spanstatus';
11-
import { startInactiveSpan, startSpan } from '../../../src/tracing/trace';
11+
import { startInactiveSpan, startSpan, withActiveSpan } from '../../../src/tracing/trace';
1212
import { markSpanAsTracerProviderSpan } from '../../../src/tracing/utils';
1313
import { withStaticSpan } from '../../../src/tracing/spans/beforeSendSpan';
1414
import type { Envelope } from '../../../src/types/envelope';
15-
import type { SpanJSON } from '../../../src/types/span';
16-
import { spanToStaticSpanJSON, TRACE_FLAG_NONE, TRACE_FLAG_SAMPLED } from '../../../src/utils/spanUtils';
15+
import type { Span, SpanJSON } from '../../../src/types/span';
16+
import { getRootSpan, spanToStaticSpanJSON, TRACE_FLAG_NONE, TRACE_FLAG_SAMPLED } from '../../../src/utils/spanUtils';
1717
import { timestampInSeconds } from '../../../src/utils/time';
1818
import { getDefaultTestClientOptions, TestClient } from '../../mocks/client';
1919

20+
function childSpansOf(span: Span): Set<Span> {
21+
return (span as unknown as { _sentryChildSpans?: Set<Span> })._sentryChildSpans ?? new Set();
22+
}
23+
2024
describe('SentrySpan', () => {
2125
describe('name', () => {
2226
it('works with name', () => {
@@ -168,6 +172,51 @@ describe('SentrySpan', () => {
168172
});
169173
});
170174

175+
describe('child span retention', () => {
176+
it('stops tracking children on a segment span once it has been captured', () => {
177+
const client = new TestClient(getDefaultTestClientOptions({ tracesSampleRate: 1 }));
178+
setCurrentClient(client);
179+
const captureEvent = vi.spyOn(client, 'captureEvent');
180+
181+
let rootSpan: Span | undefined;
182+
startSpan({ name: 'root' }, span => {
183+
rootSpan = span;
184+
startSpan({ name: 'child' }, () => {});
185+
});
186+
187+
expect(captureEvent).toHaveBeenCalledTimes(1);
188+
expect(captureEvent).toHaveBeenCalledWith(
189+
expect.objectContaining({ spans: [expect.objectContaining({ description: 'child' })] }),
190+
expect.any(Object),
191+
expect.any(Object),
192+
);
193+
expect(childSpansOf(rootSpan!).size).toBe(1);
194+
195+
// A child that starts after the tree was read is not tracked, but can still find its root span,
196+
// which is all that re-emitting it as its own transaction needs.
197+
const lateChild = withActiveSpan(rootSpan!, () => startInactiveSpan({ name: 'late child' }));
198+
expect(childSpansOf(rootSpan!).size).toBe(1);
199+
expect(getRootSpan(lateChild)).toBe(rootSpan);
200+
});
201+
202+
it('stops tracking children on a segment span that has streamed', () => {
203+
const client = new TestClient(getDefaultTestClientOptions({ tracesSampleRate: 1, traceLifecycle: 'stream' }));
204+
setCurrentClient(client);
205+
206+
let rootSpan: Span | undefined;
207+
startSpan({ name: 'root' }, span => {
208+
rootSpan = span;
209+
startSpan({ name: 'child' }, () => {});
210+
});
211+
212+
expect(childSpansOf(rootSpan!).size).toBe(1);
213+
214+
const lateChild = withActiveSpan(rootSpan!, () => startInactiveSpan({ name: 'late child' }));
215+
expect(childSpansOf(rootSpan!).size).toBe(1);
216+
expect(getRootSpan(lateChild)).toBe(rootSpan);
217+
});
218+
});
219+
171220
describe('end', () => {
172221
test('simple', () => {
173222
const span = new SentrySpan({});

packages/core/test/lib/utils/spanUtils.test.ts

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,10 @@ import type { SpanStatus } from '../../../src/types/spanStatus';
2727
import { _setSpanForScope } from '../../../src/utils/spanOnScope';
2828
import type { OpenTelemetrySdkTraceBaseSpan } from '../../../src/utils/spanUtils';
2929
import {
30+
addChildSpanToSpan,
3031
getActiveSpan,
3132
getRootSpan,
33+
getSpanDescendants,
3234
spanIsSampled,
3335
spanTimeInputToSeconds,
3436
spanToStaticSpanJSON,
@@ -872,6 +874,72 @@ describe('getActiveSpan', () => {
872874
});
873875
});
874876

877+
describe('addChildSpanToSpan', () => {
878+
it('does not track children on an unsampled span', () => {
879+
const parent = new SentrySpan({ name: 'parent', sampled: false });
880+
const child = new SentrySpan({ name: 'child', sampled: false });
881+
882+
addChildSpanToSpan(parent, child);
883+
884+
expect(getRootSpan(child)).toBe(parent);
885+
expect((parent as unknown as { _sentryChildSpans?: Set<Span> })._sentryChildSpans).toBeUndefined();
886+
});
887+
888+
it('does not track children on a segment span that stopped recording', () => {
889+
const parent = new SentrySpan({ name: 'parent', sampled: true });
890+
parent.end();
891+
892+
const child = new SentrySpan({ name: 'child', sampled: true });
893+
addChildSpanToSpan(parent, child);
894+
895+
// the child that was not tracked can still find its root span
896+
expect(getRootSpan(child)).toBe(parent);
897+
expect(getSpanDescendants(parent)).toEqual([parent]);
898+
});
899+
900+
it('keeps tracking children on an ended span while its segment span is still recording', () => {
901+
const segment = new SentrySpan({ name: 'segment', sampled: true });
902+
const parent = new SentrySpan({ name: 'parent', sampled: true });
903+
addChildSpanToSpan(segment, parent);
904+
parent.end();
905+
906+
const child = new SentrySpan({ name: 'child', sampled: true });
907+
addChildSpanToSpan(parent, child);
908+
909+
// the segment span is still open, so its transaction has not been assembled yet
910+
expect(getSpanDescendants(segment)).toEqual([segment, parent, child]);
911+
});
912+
913+
it('stops tracking children on an ended span once its segment span has ended', () => {
914+
const segment = new SentrySpan({ name: 'segment', sampled: true });
915+
const parent = new SentrySpan({ name: 'parent', sampled: true });
916+
addChildSpanToSpan(segment, parent);
917+
parent.end();
918+
segment.end();
919+
920+
const child = new SentrySpan({ name: 'child', sampled: true });
921+
addChildSpanToSpan(parent, child);
922+
923+
// the child that was not tracked can still find its root span
924+
expect(getRootSpan(child)).toBe(segment);
925+
expect(getSpanDescendants(segment)).toEqual([segment, parent]);
926+
});
927+
928+
it('keeps tracking children on a still-recording span after its segment span ended', () => {
929+
const segment = new SentrySpan({ name: 'segment', sampled: true });
930+
const lateChild = new SentrySpan({ name: 'late child', sampled: true });
931+
addChildSpanToSpan(segment, lateChild);
932+
segment.end();
933+
934+
const grandChild = new SentrySpan({ name: 'grandchild', sampled: true });
935+
addChildSpanToSpan(lateChild, grandChild);
936+
937+
// a late child that outlives its segment is re-emitted as its own orphan transaction with its
938+
// subtree, so the subtree must keep collecting
939+
expect(getSpanDescendants(lateChild)).toEqual([lateChild, grandChild]);
940+
});
941+
});
942+
875943
describe('updateSpanName', () => {
876944
it('updates the span name and source', () => {
877945
const span = new SentrySpan({ name: 'old-name', attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url' } });

0 commit comments

Comments
 (0)