Skip to content

Commit 1ced37c

Browse files
committed
fix(opentelemetry): Ensure Sentry spans don't leak when tracing is disabled
Currently, when using Sentry alongside a custom OpenTelemetry setup, any spans started via our Sentry.startSpanX apis leak into the OpenTelemetry setup even if tracing is disabled. This fix suppresses tracing for span creation via our startSpanX apis but ensures tracing is not suppressed within the callback so that for example custom OTel spans created within Sentry.startSpanX calls are not suppresed. Closes: #17826
1 parent ebb8eed commit 1ced37c

2 files changed

Lines changed: 111 additions & 3 deletions

File tree

packages/opentelemetry/src/trace.ts

Lines changed: 62 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { Context, Span, SpanContext, SpanOptions, Tracer } from '@opentelemetry/api';
22
import { context, SpanStatusCode, trace, TraceFlags } from '@opentelemetry/api';
3-
import { suppressTracing } from '@opentelemetry/core';
3+
import { isTracingSuppressed, suppressTracing } from '@opentelemetry/core';
44
import type {
55
Client,
66
continueTrace as baseContinueTrace,
@@ -17,6 +17,7 @@ import {
1717
getRootSpan,
1818
getTraceContextFromScope,
1919
handleCallbackErrors,
20+
hasSpansEnabled,
2021
SDK_VERSION,
2122
SEMANTIC_ATTRIBUTE_SENTRY_OP,
2223
spanToJSON,
@@ -53,6 +54,34 @@ export function startSpan<T>(options: OpenTelemetrySpanContext, callback: (span:
5354

5455
const spanOptions = getSpanOptions(options);
5556

57+
// If spans are not enabled, ensure we suppress tracing for the span creation
58+
// but preserve the original context for the callback execution
59+
// This ensures that we don't create spans when tracing is disabled which
60+
// would otherwise be a problem for users that don't enable tracing but use
61+
// custom OpenTelemetry setups.
62+
if (!hasSpansEnabled()) {
63+
const suppressedCtx = isTracingSuppressed(ctx) ? ctx : suppressTracing(ctx);
64+
65+
return context.with(suppressedCtx, () => {
66+
return tracer.startActiveSpan(name, spanOptions, suppressedCtx, span => {
67+
// Restore the original context for the callback execution
68+
// so that custom OpenTelemetry spans maintain the correct context
69+
return context.with(ctx, () => {
70+
return handleCallbackErrors(
71+
() => callback(span),
72+
() => {
73+
// Only set the span status to ERROR when there wasn't any status set before, in order to avoid stomping useful span statuses
74+
if (spanToJSON(span).status === undefined) {
75+
span.setStatus({ code: SpanStatusCode.ERROR });
76+
}
77+
},
78+
() => span.end(),
79+
);
80+
});
81+
});
82+
});
83+
}
84+
5685
return tracer.startActiveSpan(name, spanOptions, ctx, span => {
5786
return handleCallbackErrors(
5887
() => callback(span),
@@ -96,6 +125,33 @@ export function startSpanManual<T>(
96125

97126
const spanOptions = getSpanOptions(options);
98127

128+
if (!hasSpansEnabled()) {
129+
// If spans are not enabled, ensure we suppress tracing for the span creation
130+
// but preserve the original context for the callback execution
131+
// This ensures that we don't create spans when tracing is disabled which
132+
// would otherwise be a problem for users that don't enable tracing but use
133+
// custom OpenTelemetry setups.
134+
const suppressedCtx = isTracingSuppressed(ctx) ? ctx : suppressTracing(ctx);
135+
136+
return context.with(suppressedCtx, () => {
137+
return tracer.startActiveSpan(name, spanOptions, suppressedCtx, span => {
138+
// Restore the original context for the callback execution
139+
// so that custom OpenTelemetry spans maintain the correct context
140+
return context.with(ctx, () => {
141+
return handleCallbackErrors(
142+
() => callback(span, () => span.end()),
143+
() => {
144+
// Only set the span status to ERROR when there wasn't any status set before, in order to avoid stomping useful span statuses
145+
if (spanToJSON(span).status === undefined) {
146+
span.setStatus({ code: SpanStatusCode.ERROR });
147+
}
148+
},
149+
);
150+
});
151+
});
152+
});
153+
}
154+
99155
return tracer.startActiveSpan(name, spanOptions, ctx, span => {
100156
return handleCallbackErrors(
101157
() => callback(span, () => span.end()),
@@ -134,9 +190,12 @@ export function startInactiveSpan(options: OpenTelemetrySpanContext): Span {
134190

135191
const spanOptions = getSpanOptions(options);
136192

137-
const span = tracer.startSpan(name, spanOptions, ctx);
193+
if (!hasSpansEnabled()) {
194+
const suppressedCtx = isTracingSuppressed(ctx) ? ctx : suppressTracing(ctx);
195+
return tracer.startSpan(name, spanOptions, suppressedCtx);
196+
}
138197

139-
return span;
198+
return tracer.startSpan(name, spanOptions, ctx);
140199
});
141200
}
142201

packages/opentelemetry/test/trace.test.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1388,6 +1388,55 @@ describe('trace (tracing disabled)', () => {
13881388
});
13891389
});
13901390

1391+
describe('trace (spans disabled)', () => {
1392+
beforeEach(() => {
1393+
// Initialize SDK without any tracing configuration (no tracesSampleRate or tracesSampler)
1394+
mockSdkInit({ tracesSampleRate: undefined, tracesSampler: undefined });
1395+
});
1396+
1397+
afterEach(async () => {
1398+
await cleanupOtel();
1399+
});
1400+
1401+
it('startSpan creates non-recording spans when hasSpansEnabled() === false', () => {
1402+
const val = startSpan({ name: 'outer' }, outerSpan => {
1403+
expect(outerSpan).toBeDefined();
1404+
expect(outerSpan.isRecording()).toBe(false);
1405+
1406+
// Nested spans should also be non-recording
1407+
return startSpan({ name: 'inner' }, innerSpan => {
1408+
expect(innerSpan).toBeDefined();
1409+
expect(innerSpan.isRecording()).toBe(false);
1410+
return 'test value';
1411+
});
1412+
});
1413+
1414+
expect(val).toEqual('test value');
1415+
});
1416+
1417+
it('startSpanManual creates non-recording spans when hasSpansEnabled() === false', () => {
1418+
const val = startSpanManual({ name: 'outer' }, outerSpan => {
1419+
expect(outerSpan).toBeDefined();
1420+
expect(outerSpan.isRecording()).toBe(false);
1421+
1422+
return startSpanManual({ name: 'inner' }, innerSpan => {
1423+
expect(innerSpan).toBeDefined();
1424+
expect(innerSpan.isRecording()).toBe(false);
1425+
return 'test value';
1426+
});
1427+
});
1428+
1429+
expect(val).toEqual('test value');
1430+
});
1431+
1432+
it('startInactiveSpan returns non-recording spans when hasSpansEnabled() === false', () => {
1433+
const span = startInactiveSpan({ name: 'test' });
1434+
1435+
expect(span).toBeDefined();
1436+
expect(span.isRecording()).toBe(false);
1437+
});
1438+
});
1439+
13911440
describe('trace (sampling)', () => {
13921441
afterEach(async () => {
13931442
await cleanupOtel();

0 commit comments

Comments
 (0)