Skip to content

Commit db32055

Browse files
authored
fix(opentelemetry): Ensure Sentry spans don't leak when tracing is disabled (#18337)
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. I update the `node-otel-without-tracing` e2e tests to reflect that no Sentry spans leak into the OTLP endpoint, as well as trying this out locally with an express app and Jaeger. **Before** the fix, Sentry span leaks: <img width="2608" height="1578" alt="Screenshot 2025-11-24 at 20 35 21" src="https://github.com/user-attachments/assets/45284aac-b9c1-423d-b199-c1c9273d1980" /> <img width="2608" height="1578" alt="Screenshot 2025-11-24 at 20 34 53" src="https://github.com/user-attachments/assets/ff1929e6-a195-4e4e-a45b-5bddf49a1c25" /> <hr> **After** the fix, no Sentry span leakage <img width="2608" height="1578" alt="Screenshot 2025-11-24 at 20 28 22" src="https://github.com/user-attachments/assets/e6584c01-30ef-40f2-87bf-33ac1d81e302" /> <img width="2608" height="1578" alt="Screenshot 2025-11-24 at 20 28 32" src="https://github.com/user-attachments/assets/4388bb01-9f7b-48ab-8bb4-c7129075c89c" /> Closes: #17826
1 parent 2f8a039 commit db32055

File tree

10 files changed

+248
-82
lines changed

10 files changed

+248
-82
lines changed

dev-packages/e2e-tests/test-applications/node-otel-without-tracing/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
"test:assert": "pnpm test"
1212
},
1313
"dependencies": {
14+
"@opentelemetry/api": "1.9.0",
1415
"@opentelemetry/sdk-trace-node": "2.2.0",
1516
"@opentelemetry/exporter-trace-otlp-http": "0.208.0",
1617
"@opentelemetry/instrumentation-undici": "0.13.2",

dev-packages/e2e-tests/test-applications/node-otel-without-tracing/src/app.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import './instrument';
22

33
// Other imports below
44
import * as Sentry from '@sentry/node';
5+
import { trace, type Span } from '@opentelemetry/api';
56
import express from 'express';
67

78
const app = express();
@@ -29,6 +30,23 @@ app.get('/test-transaction', function (req, res) {
2930
});
3031
});
3132

33+
app.get('/test-only-if-parent', function (req, res) {
34+
// Remove the HTTP span from the context to simulate no parent span
35+
Sentry.withActiveSpan(null, () => {
36+
// This should NOT create a span because onlyIfParent is true and there's no parent
37+
Sentry.startSpan({ name: 'test-only-if-parent', onlyIfParent: true }, () => {
38+
// This custom OTel span SHOULD be created and exported
39+
// This tests that custom OTel spans aren't suppressed when onlyIfParent triggers
40+
const customTracer = trace.getTracer('custom-tracer');
41+
customTracer.startActiveSpan('custom-span-with-only-if-parent', (span: Span) => {
42+
span.end();
43+
});
44+
});
45+
46+
res.send({});
47+
});
48+
});
49+
3250
app.get('/test-error', async function (req, res) {
3351
const exceptionId = Sentry.captureException(new Error('This is an error'));
3452

@@ -44,6 +62,16 @@ app.get('/test-exception/:id', function (req, _res) {
4462
throw new Error(`This is an exception with id ${id}`);
4563
});
4664

65+
app.get('/test-logs/:id', function (req, res) {
66+
const id = req.params.id;
67+
68+
Sentry.startSpan({ name: `log-operation-${id}` }, () => {
69+
Sentry.logger.info(`test-log-${id}`, { requestId: id });
70+
});
71+
72+
res.send({ ok: true, id });
73+
});
74+
4775
Sentry.setupExpressErrorHandler(app);
4876

4977
app.use(function onError(err: unknown, req: any, res: any, next: any) {

dev-packages/e2e-tests/test-applications/node-otel-without-tracing/src/instrument.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ Sentry.init({
1717
// Tracing is completely disabled
1818
// Custom OTEL setup
1919
skipOpenTelemetrySetup: true,
20+
enableLogs: true,
2021
});
2122

2223
// Create and configure NodeTracerProvider
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
import { expect, test } from '@playwright/test';
2+
import { waitForEnvelopeItem } from '@sentry-internal/test-utils';
3+
4+
test('Logs from different requests have different trace IDs', async ({ baseURL }) => {
5+
const logEnvelopePromise1 = waitForEnvelopeItem('node-otel-without-tracing', async envelopeItem => {
6+
const [itemHeader, itemPayload] = envelopeItem;
7+
if (itemHeader.type === 'log') {
8+
const logItems = itemPayload as { items: Array<{ body: string; trace_id?: string }> };
9+
return logItems.items.some(item => item.body === 'test-log-1');
10+
}
11+
return false;
12+
});
13+
14+
const logEnvelopePromise2 = waitForEnvelopeItem('node-otel-without-tracing', async envelopeItem => {
15+
const [itemHeader, itemPayload] = envelopeItem;
16+
if (itemHeader.type === 'log') {
17+
const logItems = itemPayload as { items: Array<{ body: string; trace_id?: string }> };
18+
return logItems.items.some(item => item.body === 'test-log-2');
19+
}
20+
return false;
21+
});
22+
23+
// Make two requests to different routes (each Express route is an isolation scope)
24+
await fetch(`${baseURL}/test-logs/1`);
25+
await fetch(`${baseURL}/test-logs/2`);
26+
27+
const logEnvelope1 = await logEnvelopePromise1;
28+
const logEnvelope2 = await logEnvelopePromise2;
29+
30+
const logPayload1 = logEnvelope1[1] as { items: Array<{ body: string; trace_id?: string }> };
31+
const logPayload2 = logEnvelope2[1] as { items: Array<{ body: string; trace_id?: string }> };
32+
33+
const log1 = logPayload1.items.find(item => item.body === 'test-log-1');
34+
const log2 = logPayload2.items.find(item => item.body === 'test-log-2');
35+
36+
const traceId1 = log1?.trace_id;
37+
const traceId2 = log2?.trace_id;
38+
39+
expect(traceId1).toMatch(/[a-f0-9]{32}/);
40+
expect(traceId2).toMatch(/[a-f0-9]{32}/);
41+
expect(traceId1).not.toBe(traceId2);
42+
});

dev-packages/e2e-tests/test-applications/node-otel-without-tracing/tests/transactions.test.ts

Lines changed: 61 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,10 @@ test('Sends an API route transaction to OTLP', async ({ baseURL }) => {
3434
const scopeSpans = json.resourceSpans?.[0]?.scopeSpans;
3535
expect(scopeSpans).toBeDefined();
3636

37-
// Http server span & undici client spans are emitted,
38-
// as well as the spans emitted via `Sentry.startSpan()`
39-
// But our default node-fetch spans are not emitted
40-
expect(scopeSpans.length).toEqual(3);
37+
// Http server span & undici client spans are emitted.
38+
// Sentry.startSpan() spans are NOT emitted (non-recording when tracing is disabled).
39+
// Our default node-fetch spans are also not emitted.
40+
expect(scopeSpans.length).toEqual(2);
4141

4242
const httpScopes = scopeSpans?.filter(scopeSpan => scopeSpan.scope.name === '@opentelemetry/instrumentation-http');
4343
const undiciScopes = scopeSpans?.filter(
@@ -47,43 +47,8 @@ test('Sends an API route transaction to OTLP', async ({ baseURL }) => {
4747

4848
expect(httpScopes.length).toBe(1);
4949

50-
expect(startSpanScopes.length).toBe(1);
51-
expect(startSpanScopes[0].spans.length).toBe(2);
52-
expect(startSpanScopes[0].spans).toEqual([
53-
{
54-
traceId: expect.any(String),
55-
spanId: expect.any(String),
56-
parentSpanId: expect.any(String),
57-
name: 'test-span',
58-
kind: 1,
59-
startTimeUnixNano: expect.any(String),
60-
endTimeUnixNano: expect.any(String),
61-
attributes: [],
62-
droppedAttributesCount: 0,
63-
events: [],
64-
droppedEventsCount: 0,
65-
status: { code: 0 },
66-
links: [],
67-
droppedLinksCount: 0,
68-
flags: expect.any(Number),
69-
},
70-
{
71-
traceId: expect.any(String),
72-
spanId: expect.any(String),
73-
name: 'test-transaction',
74-
kind: 1,
75-
startTimeUnixNano: expect.any(String),
76-
endTimeUnixNano: expect.any(String),
77-
attributes: [{ key: 'sentry.op', value: { stringValue: 'e2e-test' } }],
78-
droppedAttributesCount: 0,
79-
events: [],
80-
droppedEventsCount: 0,
81-
status: { code: 0 },
82-
links: [],
83-
droppedLinksCount: 0,
84-
flags: expect.any(Number),
85-
},
86-
]);
50+
// Sentry spans should not be exported when tracing is disabled
51+
expect(startSpanScopes.length).toBe(0);
8752

8853
// Undici spans are emitted correctly
8954
expect(undiciScopes.length).toBe(1);
@@ -164,3 +129,58 @@ test('Sends an API route transaction to OTLP', async ({ baseURL }) => {
164129
},
165130
]);
166131
});
132+
133+
test('Custom OTel spans work with onlyIfParent when no parent exists', async ({ baseURL }) => {
134+
waitForTransaction('node-otel-without-tracing', transactionEvent => {
135+
throw new Error('THIS SHOULD NEVER HAPPEN!');
136+
});
137+
138+
// Ensure we send data to the OTLP endpoint
139+
const otelPromise = waitForPlainRequest('node-otel-without-tracing-otel', data => {
140+
const json = JSON.parse(data) as any;
141+
142+
const scopeSpans = json.resourceSpans?.[0]?.scopeSpans;
143+
144+
// Look for the custom span from our custom-tracer
145+
const customScope = scopeSpans?.find(scopeSpan => scopeSpan.scope.name === 'custom-tracer');
146+
147+
return customScope && customScope.spans.some(span => span.name === 'custom-span-with-only-if-parent');
148+
});
149+
150+
fetch(`${baseURL}/test-only-if-parent`);
151+
152+
const otelData = await otelPromise;
153+
154+
expect(otelData).toBeDefined();
155+
156+
const json = JSON.parse(otelData);
157+
expect(json.resourceSpans.length).toBe(1);
158+
159+
const scopeSpans = json.resourceSpans?.[0]?.scopeSpans;
160+
expect(scopeSpans).toBeDefined();
161+
162+
// Should have HTTP instrumentation span but NO Sentry span
163+
const httpScopes = scopeSpans?.filter(scopeSpan => scopeSpan.scope.name === '@opentelemetry/instrumentation-http');
164+
const sentryScopes = scopeSpans?.filter(scopeSpan => scopeSpan.scope.name === '@sentry/node');
165+
const customScopes = scopeSpans?.filter(scopeSpan => scopeSpan.scope.name === 'custom-tracer');
166+
167+
// HTTP span exists (from the incoming request)
168+
expect(httpScopes.length).toBe(1);
169+
170+
// Sentry span should NOT exist (onlyIfParent + no parent = suppressed)
171+
expect(sentryScopes.length).toBe(0);
172+
173+
// Custom OTel span SHOULD exist (this is what we're testing - the fix ensures this works)
174+
expect(customScopes.length).toBe(1);
175+
expect(customScopes[0].spans.length).toBe(1);
176+
expect(customScopes[0].spans[0]).toMatchObject({
177+
name: 'custom-span-with-only-if-parent',
178+
kind: 1,
179+
status: { code: 0 },
180+
});
181+
182+
// Verify the custom span is recording (not suppressed)
183+
const customSpan = customScopes[0].spans[0];
184+
expect(customSpan.spanId).not.toBe('0000000000000000');
185+
expect(customSpan.traceId).not.toBe('00000000000000000000000000000000');
186+
});

dev-packages/node-integration-tests/suites/pino/instrument-auto-off.mjs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import * as Sentry from '@sentry/node';
33
Sentry.init({
44
dsn: process.env.SENTRY_DSN,
55
release: '1.0',
6+
tracesSampleRate: 1.0,
67
enableLogs: true,
78
integrations: [Sentry.pinoIntegration({ autoInstrument: false })],
89
});

dev-packages/node-integration-tests/suites/pino/instrument.mjs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import * as Sentry from '@sentry/node';
33
Sentry.init({
44
dsn: process.env.SENTRY_DSN,
55
release: '1.0',
6+
tracesSampleRate: 1.0,
67
enableLogs: true,
78
integrations: [Sentry.pinoIntegration({ error: { levels: ['error', 'fatal'] } })],
89
});

dev-packages/node-integration-tests/suites/pino/test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ conditionalTest({ min: 20 })('Pino integration', () => {
1111
.withMockSentryServer()
1212
.withInstrument(instrumentPath)
1313
.ignore('event')
14+
.ignore('transaction')
1415
.expect({
1516
log: log => {
1617
const traceId1 = log.items?.[0]?.trace_id;
@@ -28,6 +29,7 @@ conditionalTest({ min: 20 })('Pino integration', () => {
2829
await createRunner(__dirname, 'scenario.mjs')
2930
.withMockSentryServer()
3031
.withInstrument(instrumentPath)
32+
.ignore('transaction')
3133
.expect({
3234
event: {
3335
exception: {
@@ -105,6 +107,7 @@ conditionalTest({ min: 20 })('Pino integration', () => {
105107
await createRunner(__dirname, 'scenario-next.mjs')
106108
.withMockSentryServer()
107109
.withInstrument(instrumentPath)
110+
.ignore('transaction')
108111
.expect({
109112
event: {
110113
exception: {
@@ -182,6 +185,7 @@ conditionalTest({ min: 20 })('Pino integration', () => {
182185
await createRunner(__dirname, 'scenario-track.mjs')
183186
.withMockSentryServer()
184187
.withInstrument(instrumentPath)
188+
.ignore('transaction')
185189
.expect({
186190
log: {
187191
items: [

0 commit comments

Comments
 (0)