Skip to content

Commit aabe208

Browse files
committed
dont record sampling client outcome for suppressed spans
1 parent 92a7714 commit aabe208

File tree

3 files changed

+51
-12
lines changed

3 files changed

+51
-12
lines changed

dev-packages/node-integration-tests/suites/tracing/ignoreSpans-streamed/children/server.mjs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,13 @@ app.get('/test/express', (_req, res) => {
3131
},
3232
);
3333

34-
Sentry.startSpan({ name: 'name-passes-but-op-not-span-1', op: 'ignored-op' }, () => {}),
35-
Sentry.startSpan(
36-
// sentry.op attribute has precedence over top op argument
37-
{ name: 'name-passes-but-op-not-span-2', /*op: 'keep',*/ attributes: { 'sentry.op': 'ignored-op' } },
38-
() => {},
39-
),
40-
res.send({ response: 'response 1' });
34+
(Sentry.startSpan({ name: 'name-passes-but-op-not-span-1', op: 'ignored-op' }, () => {}),
35+
Sentry.startSpan(
36+
// sentry.op attribute has precedence over top op argument
37+
{ name: 'name-passes-but-op-not-span-2', /*op: 'keep',*/ attributes: { 'sentry.op': 'ignored-op' } },
38+
() => {},
39+
),
40+
res.send({ response: 'response 1' }));
4141
});
4242

4343
Sentry.setupExpressErrorHandler(app);

packages/core/src/tracing/trace.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -489,7 +489,7 @@ function _startRootSpan(spanArguments: SentrySpanArguments, scope: Scope, parent
489489
sampled,
490490
});
491491

492-
if (!sampled && client) {
492+
if (!sampled && client && !scope.getScopeData().sdkProcessingMetadata[SUPPRESS_TRACING_KEY]) {
493493
DEBUG_BUILD && debug.log('[Tracing] Discarding root span because its trace was not chosen to be sampled.');
494494
client.recordDroppedEvent('sample_rate', hasSpanStreamingEnabled(client) ? 'span' : 'transaction');
495495
}
@@ -533,7 +533,7 @@ function _startChildSpan(parentSpan: Span, scope: Scope, spanArguments: SentrySp
533533
// record a client outcome for the child.
534534
childSpan.dropReason = parentSpan.dropReason;
535535
client.recordDroppedEvent(parentSpan.dropReason, 'span');
536-
} else {
536+
} else if (!scope.getScopeData().sdkProcessingMetadata[SUPPRESS_TRACING_KEY]) {
537537
// Otherwise, the child is not sampled due to sampling of the parent span,
538538
// hence we record a sample_rate client outcome for the child.
539539
childSpan.dropReason = 'sample_rate';

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

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -573,12 +573,15 @@ describe('startSpan', () => {
573573

574574
describe('onlyIfParent', () => {
575575
it('starts a non recording span if there is no parent', () => {
576+
const spyOnDroppedEvent = vi.spyOn(client, 'recordDroppedEvent');
577+
576578
const span = startSpan({ name: 'test span', onlyIfParent: true }, span => {
577579
return span;
578580
});
579581

580582
expect(span).toBeDefined();
581583
expect(span).toBeInstanceOf(SentryNonRecordingSpan);
584+
expect(spyOnDroppedEvent).not.toHaveBeenCalled();
582585
});
583586

584587
it('creates a span if there is a parent', () => {
@@ -2305,7 +2308,7 @@ describe('suppressTracing', () => {
23052308
vi.clearAllMocks();
23062309
});
23072310

2308-
it('works for a root span', () => {
2311+
it('suppresses a root span', () => {
23092312
const span = suppressTracing(() => {
23102313
return startInactiveSpan({ name: 'span' });
23112314
});
@@ -2314,7 +2317,7 @@ describe('suppressTracing', () => {
23142317
expect(spanIsSampled(span)).toBe(false);
23152318
});
23162319

2317-
it('works for a child span', () => {
2320+
it('suppresses a child span', () => {
23182321
startSpan({ name: 'outer' }, span => {
23192322
expect(span.isRecording()).toBe(true);
23202323
expect(spanIsSampled(span)).toBe(true);
@@ -2333,7 +2336,7 @@ describe('suppressTracing', () => {
23332336
});
23342337
});
23352338

2336-
it('works for a child span with forceTransaction=true', () => {
2339+
it('suppresses a child span with forceTransaction=true', () => {
23372340
startSpan({ name: 'outer' }, span => {
23382341
expect(span.isRecording()).toBe(true);
23392342
expect(spanIsSampled(span)).toBe(true);
@@ -2347,6 +2350,42 @@ describe('suppressTracing', () => {
23472350
});
23482351
});
23492352

2353+
it("doesn't record a client outcome for suppressed spans", () => {
2354+
getCurrentScope().clear();
2355+
getIsolationScope().clear();
2356+
getGlobalScope().clear();
2357+
2358+
setAsyncContextStrategy(undefined);
2359+
2360+
const options = getDefaultTestClientOptions({ tracesSampleRate: 1, traceLifecycle: 'stream' });
2361+
client = new TestClient(options);
2362+
setCurrentClient(client);
2363+
client.init();
2364+
2365+
const spyOnDroppedEvent = vi.spyOn(client, 'recordDroppedEvent');
2366+
2367+
const span1 = suppressTracing(() => {
2368+
return startInactiveSpan({ name: 'span' });
2369+
});
2370+
2371+
expect(span1.isRecording()).toBe(false);
2372+
expect(spanIsSampled(span1)).toBe(false);
2373+
2374+
startSpan({ name: 'outer' }, span => {
2375+
expect(span.isRecording()).toBe(true);
2376+
expect(spanIsSampled(span)).toBe(true);
2377+
2378+
const child = suppressTracing(() => {
2379+
return startInactiveSpan({ name: 'span' });
2380+
});
2381+
2382+
expect(child.isRecording()).toBe(false);
2383+
expect(spanIsSampled(child)).toBe(false);
2384+
});
2385+
2386+
expect(spyOnDroppedEvent).not.toHaveBeenCalled();
2387+
});
2388+
23502389
it('works with parallel processes', async () => {
23512390
const span = suppressTracing(() => {
23522391
return startInactiveSpan({ name: 'span' });

0 commit comments

Comments
 (0)