Skip to content

Commit e8d7bd4

Browse files
Lms24cursoragent
Lms24
and
cursoragent
authored
fix(core): Preserve propagation across ignored spans (#22277)
This PR preserves trace propagation for ignored spans in span streaming mode: - When segment spans are ignored, incoming DSC (via baggage) is kept as-is, but is mutated to carry a negative sampling decision, in line with the `sentry-trace` header, which already emitted a negative sampling decision - ignored child and HTTP client spans propagate from the nearest emitted parent across fetch, `node:http` - For Orchestrion/tracing-channel-based instrumentation, we only set a child span as the active span if it is not ignored. This was already implemented in the OTel context manager previously but probably didn't get ported correctly to the new approach. Turns out, the DSC changes are also a requirement for #22246, so I threw it on top of this PR. ref #22262 --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com>
1 parent bd69bd9 commit e8d7bd4

17 files changed

Lines changed: 482 additions & 25 deletions

File tree

‎dev-packages/node-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-child/instrument.mjs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import * as Sentry from '@sentry/node';
2+
import { loggingTransport } from '@sentry-internal/node-integration-tests';
3+
4+
Sentry.init({
5+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
6+
tracesSampleRate: 0,
7+
transport: loggingTransport,
8+
traceLifecycle: 'stream',
9+
ignoreSpans: ['ignoredMiddleware'],
10+
tracePropagationTargets: [process.env.SERVER_URL],
11+
clientReportFlushInterval: 1_000,
12+
});

‎dev-packages/node-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-child/server.mjs‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import * as Sentry from '@sentry/node';
2+
import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests';
3+
import express from 'express';
4+
import * as http from 'http';
5+
6+
const app = express();
7+
8+
app.use(async function ignoredMiddleware(req, _res, next) {
9+
if (req.path === '/ignored-child-http') {
10+
await makeHttpRequest(`${process.env.SERVER_URL}/outgoing`);
11+
} else {
12+
await fetch(`${process.env.SERVER_URL}/outgoing`);
13+
}
14+
next();
15+
});
16+
17+
app.get(['/ignored-child', '/ignored-child-http'], (_req, res) => {
18+
res.send({ status: 'ok' });
19+
setTimeout(() => {
20+
Sentry.flush();
21+
});
22+
});
23+
24+
startExpressServerAndSendPortToRunner(app);
25+
26+
function makeHttpRequest(url) {
27+
return new Promise((resolve, reject) => {
28+
const request = http.get(url, httpRes => {
29+
httpRes.resume();
30+
httpRes.on('end', resolve);
31+
});
32+
request.on('error', reject);
33+
});
34+
}

‎dev-packages/node-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-child/test.ts‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
import { createTestServer } from '@sentry-internal/test-utils';
2+
import { afterAll, describe, expect } from 'vitest';
3+
import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../../utils/runner';
4+
import { SENTRY_OP } from '@sentry/conventions/attributes';
5+
6+
describe('ignoring a child of a continued server segment (streaming)', () => {
7+
afterAll(() => {
8+
cleanupChildProcesses();
9+
});
10+
11+
createEsmAndCjsTests(__dirname, 'server.mjs', 'instrument.mjs', (createRunner, test) => {
12+
const testPropagation = async (path: string): Promise<void> => {
13+
const [SERVER_URL, closeTestServer] = await createTestServer()
14+
.get('/outgoing', headers => {
15+
expect(headers['sentry-trace']).toMatch(/^12345678901234567890123456789012-[\da-f]{16}-1$/);
16+
17+
expect(headers['baggage']).toBe(
18+
'sentry-trace_id=12345678901234567890123456789012,sentry-sample_rate=1,sentry-sampled=true,sentry-public_key=public,sentry-sample_rand=0.5',
19+
);
20+
})
21+
.start();
22+
const runner = createRunner()
23+
.withEnv({ SERVER_URL })
24+
.unignore('client_report')
25+
.expect({
26+
client_report: {
27+
discarded_events: [{ category: 'span', quantity: 1, reason: 'ignored' }],
28+
},
29+
})
30+
.expect({
31+
span: container => {
32+
const httpServerSpan = container.items.find(item => item.attributes[SENTRY_OP]?.value === 'http.server');
33+
34+
expect(httpServerSpan?.is_segment).toBe(true);
35+
expect(httpServerSpan?.trace_id).toBe('12345678901234567890123456789012');
36+
expect(container.items.some(item => item.name === 'ignoredMiddleware')).toBe(false);
37+
},
38+
})
39+
.start();
40+
41+
try {
42+
const response = await runner.makeRequest<{ status: string }>('get', path, {
43+
headers: {
44+
'sentry-trace': '12345678901234567890123456789012-1234567890123456-1',
45+
baggage:
46+
'sentry-trace_id=12345678901234567890123456789012,sentry-sample_rate=1,sentry-sampled=true,sentry-public_key=public,sentry-sample_rand=0.5',
47+
},
48+
});
49+
50+
expect(response?.status).toBe('ok');
51+
await runner.completed();
52+
} finally {
53+
closeTestServer();
54+
}
55+
};
56+
57+
test('preserves the positive sampling decision on outgoing fetch requests', () =>
58+
testPropagation('/ignored-child'));
59+
60+
test('preserves the positive sampling decision on outgoing node:http requests', () =>
61+
testPropagation('/ignored-child-http'));
62+
});
63+
});

‎dev-packages/node-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-http-client/instrument.mjs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import * as Sentry from '@sentry/node';
2+
import { loggingTransport } from '@sentry-internal/node-integration-tests';
3+
4+
Sentry.init({
5+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
6+
tracesSampleRate: 0,
7+
transport: loggingTransport,
8+
traceLifecycle: 'stream',
9+
ignoreSpans: [{ attributes: { 'url.path': '/outgoing' } }],
10+
tracePropagationTargets: [process.env.SERVER_URL],
11+
clientReportFlushInterval: 1_000,
12+
});

‎dev-packages/node-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-http-client/server.mjs‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import * as Sentry from '@sentry/node';
2+
import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests';
3+
import express from 'express';
4+
import * as http from 'http';
5+
6+
const app = express();
7+
8+
app.get('/ignored-http-client', async (_req, res) => {
9+
await fetch(`${process.env.SERVER_URL}/outgoing`);
10+
res.send({ status: 'ok' });
11+
setTimeout(() => {
12+
Sentry.flush();
13+
});
14+
});
15+
16+
app.get('/ignored-node-http-client', async (_req, res) => {
17+
await makeHttpRequest(`${process.env.SERVER_URL}/outgoing`);
18+
res.send({ status: 'ok' });
19+
setTimeout(() => {
20+
Sentry.flush();
21+
});
22+
});
23+
24+
startExpressServerAndSendPortToRunner(app);
25+
26+
function makeHttpRequest(url) {
27+
return new Promise((resolve, reject) => {
28+
const request = http.get(url, httpRes => {
29+
httpRes.resume();
30+
httpRes.on('end', resolve);
31+
});
32+
request.on('error', reject);
33+
});
34+
}

‎dev-packages/node-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-http-client/test.ts‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
import { createTestServer } from '@sentry-internal/test-utils';
2+
import { afterAll, describe, expect } from 'vitest';
3+
import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../../utils/runner';
4+
import { SENTRY_OP } from '@sentry/conventions/attributes';
5+
6+
describe('ignoring an HTTP client child of a continued server segment (streaming)', () => {
7+
afterAll(() => {
8+
cleanupChildProcesses();
9+
});
10+
11+
createEsmAndCjsTests(__dirname, 'server.mjs', 'instrument.mjs', (createRunner, test) => {
12+
const testPropagation = async (path: string): Promise<void> => {
13+
const [SERVER_URL, closeTestServer] = await createTestServer()
14+
.get('/outgoing', headers => {
15+
expect
16+
.soft(headers['sentry-trace'])
17+
.toEqual(expect.stringMatching(/^12345678901234567890123456789012-[\da-f]{16}-1$/));
18+
19+
expect(headers['baggage']).toBe(
20+
'sentry-trace_id=12345678901234567890123456789012,sentry-sample_rate=1,sentry-sampled=true,sentry-public_key=public,sentry-sample_rand=0.5',
21+
);
22+
})
23+
.start();
24+
25+
const runner = createRunner()
26+
.withEnv({ SERVER_URL })
27+
.unignore('client_report')
28+
.expect({
29+
client_report: {
30+
discarded_events: [{ category: 'span', quantity: 1, reason: 'ignored' }],
31+
},
32+
})
33+
.expect({
34+
span: container => {
35+
const httpServerSpan = container.items.find(item => item.attributes[SENTRY_OP]?.value === 'http.server');
36+
37+
expect(httpServerSpan?.is_segment).toBe(true);
38+
expect(httpServerSpan?.trace_id).toBe('12345678901234567890123456789012');
39+
40+
expect(container.items.some(item => item.attributes[SENTRY_OP]?.value === 'http.client')).toBe(false);
41+
},
42+
})
43+
.start();
44+
45+
try {
46+
const response = await runner.makeRequest<{ status: string }>('get', path, {
47+
headers: {
48+
'sentry-trace': '12345678901234567890123456789012-1234567890123456-1',
49+
baggage:
50+
'sentry-trace_id=12345678901234567890123456789012,sentry-sample_rate=1,sentry-sampled=true,sentry-public_key=public,sentry-sample_rand=0.5',
51+
},
52+
});
53+
54+
expect(response?.status).toBe('ok');
55+
await runner.completed();
56+
} finally {
57+
closeTestServer();
58+
}
59+
};
60+
61+
test('preserves the positive sampling decision on the outgoing fetch request', () =>
62+
testPropagation('/ignored-http-client'));
63+
64+
test('preserves the positive sampling decision on the outgoing node:http request', () =>
65+
testPropagation('/ignored-node-http-client'));
66+
});
67+
});

‎dev-packages/node-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-segment/instrument.mjs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
import * as Sentry from '@sentry/node';
2+
import { loggingTransport } from '@sentry-internal/node-integration-tests';
3+
4+
Sentry.init({
5+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
6+
tracesSampleRate: 0,
7+
transport: loggingTransport,
8+
traceLifecycle: 'stream',
9+
ignoreSpans: [{ op: 'http.server' }],
10+
tracePropagationTargets: [process.env.SERVER_URL],
11+
});

‎dev-packages/node-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-segment/server.mjs‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests';
2+
import express from 'express';
3+
import * as http from 'http';
4+
5+
const app = express();
6+
7+
app.get('/ignored', async (_req, res) => {
8+
await fetch(`${process.env.SERVER_URL}/outgoing`);
9+
res.send({ status: 'ok' });
10+
});
11+
12+
app.get('/ignored-http', async (_req, res) => {
13+
await makeHttpRequest(`${process.env.SERVER_URL}/outgoing`);
14+
res.send({ status: 'ok' });
15+
});
16+
17+
startExpressServerAndSendPortToRunner(app);
18+
19+
function makeHttpRequest(url) {
20+
return new Promise((resolve, reject) => {
21+
const request = http.get(url, httpRes => {
22+
httpRes.resume();
23+
httpRes.on('end', resolve);
24+
});
25+
request.on('error', reject);
26+
});
27+
}

‎dev-packages/node-integration-tests/suites/tracing/ignoreSpans-streamed/continued-trace-segment/test.ts‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
import { createTestServer } from '@sentry-internal/test-utils';
2+
import { afterAll, describe, expect } from 'vitest';
3+
import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../../utils/runner';
4+
5+
describe('ignoring a continued server segment (streaming)', () => {
6+
afterAll(() => {
7+
cleanupChildProcesses();
8+
});
9+
10+
createEsmAndCjsTests(__dirname, 'server.mjs', 'instrument.mjs', (createRunner, test) => {
11+
const testPropagation = async (path: string): Promise<void> => {
12+
expect.assertions(3);
13+
14+
const [SERVER_URL, closeTestServer] = await createTestServer()
15+
.get('/outgoing', headers => {
16+
const sentryTrace = headers['sentry-trace'];
17+
const baggage = headers['baggage'];
18+
19+
expect(sentryTrace).toMatch(/12345678901234567890123456789012-[\da-f]{16}-0/);
20+
expect(baggage).toBe(
21+
'sentry-trace_id=12345678901234567890123456789012,sentry-sample_rate=1,sentry-sampled=false,sentry-public_key=public,sentry-sample_rand=0.5',
22+
);
23+
})
24+
.start();
25+
const runner = createRunner().withEnv({ SERVER_URL }).start();
26+
27+
try {
28+
const response = await runner.makeRequest<{ status: string }>('get', path, {
29+
headers: {
30+
'sentry-trace': '12345678901234567890123456789012-1234567890123456-1',
31+
baggage:
32+
'sentry-trace_id=12345678901234567890123456789012,sentry-sample_rate=1,sentry-sampled=true,sentry-public_key=public,sentry-sample_rand=0.5',
33+
},
34+
});
35+
36+
expect(response?.status).toBe('ok');
37+
} finally {
38+
closeTestServer();
39+
}
40+
};
41+
42+
test('propagates a negative sampling decision to outgoing fetch requests', () => testPropagation('/ignored'));
43+
test('propagates a negative sampling decision to outgoing node:http requests', () =>
44+
testPropagation('/ignored-http'));
45+
});
46+
});

‎packages/core/src/tracing/dynamicSamplingContext.ts‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -107,20 +107,26 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly<Partial<
107107
return applyLocalSampleRateToDsc(frozenDsc);
108108
}
109109

110-
// For a non-recording placeholder in Tracing without Performance (TwP) mode, the DSC is not
111-
// carried on the span; the scope is the source of truth. Resolve it from the span's captured
112-
// scope: continued traces keep the incoming DSC, new traces derive it from the client.
110+
// For a non-recording placeholder in Tracing without Performance (TwP) mode or an ignored segment,
111+
// the DSC is not carried on the span; the scope is the source of truth. Resolve it from the span's
112+
// captured scope so continued traces keep their incoming DSC.
113113
//
114114
// We gate this on `!hasSpansEnabled()` so it mirrors the `sentry-trace` source in `getTraceData`:
115-
// with tracing enabled, a non-recording span (e.g. an `onlyIfParent` placeholder) keeps deriving
116-
// its DSC from the span/client so the baggage agrees with the `-0` decision that `spanToTraceHeader`
117-
// encodes for `sentry-trace`. Without this guard the two headers can disagree.
115+
// with tracing enabled, other non-recording spans (e.g. an `onlyIfParent` placeholder) keep deriving
116+
// their DSC from the span/client. Ignored segments are an explicit exception: they preserve the
117+
// incoming DSC but override its sampling decision to agree with the propagated `sentry-trace`.
118118
//
119119
// We spread into a new object so applying the local sample rate can't mutate the scope's DSC.
120-
if (spanIsNonRecordingSpan(rootSpan) && !hasSpansEnabled(client.getOptions())) {
120+
const isNonRecordingRoot = spanIsNonRecordingSpan(rootSpan);
121+
const isIgnoredRoot = isNonRecordingRoot && rootSpan.dropReason === 'ignored';
122+
if (isNonRecordingRoot && (!hasSpansEnabled(client.getOptions()) || isIgnoredRoot)) {
121123
const capturedScope = getCapturedScopesOnSpan(rootSpan).scope;
122124
if (capturedScope) {
123-
return applyLocalSampleRateToDsc({ ...getDynamicSamplingContextFromScope(client, capturedScope) });
125+
const dsc = { ...getDynamicSamplingContextFromScope(client, capturedScope) };
126+
if (isIgnoredRoot) {
127+
dsc.sampled = 'false';
128+
}
129+
return applyLocalSampleRateToDsc(dsc);
124130
}
125131
}
126132

0 commit comments

Comments
 (0)