Skip to content

Commit 8da4703

Browse files
Lms24Lms24
committed
fix(node): Avoid duplicated sentry-trace and baggage headers on fetch requests
1 parent da07971 commit 8da4703

File tree

13 files changed

+735
-45
lines changed

13 files changed

+735
-45
lines changed

‎dev-packages/browser-integration-tests/suites/tracing/request/fetch-trace-header-merging/test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ async function assertRequests({
3939

4040
// No merged sentry trace headers
4141
expect(headers['sentry-trace']).not.toContain(',');
42+
expect(headers['sentry-trace']).toBe('12312012123120121231201212312012-1121201211212012-1');
4243

4344
// No multiple baggage entries
4445
expect(headers['baggage'].match(/sentry-release/g) ?? []).toHaveLength(1);

‎dev-packages/node-integration-tests/suites/tracing/double-baggage-no-spans/instrument.mjs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
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+
release: '1.0',
7+
// explicitly not setting tracesSampleRate,
8+
transport: loggingTransport,
9+
});

‎dev-packages/node-integration-tests/suites/tracing/double-baggage-no-spans/scenario.mjs‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import * as Sentry from '@sentry/node';
2+
import http from 'http';
3+
4+
async function run() {
5+
// fetch with manual getTraceData() headers - the core reproduction case from #19158
6+
await fetch(`${process.env.SERVER_URL}/api/v0`, {
7+
headers: { ...Sentry.getTraceData() },
8+
}).then(res => res.text());
9+
10+
// fetch without manual headers (baseline - auto-instrumentation only)
11+
await fetch(`${process.env.SERVER_URL}/api/v1`).then(res => res.text());
12+
13+
// http.request with manual getTraceData() headers
14+
await new Promise((resolve, reject) => {
15+
const url = new URL(`${process.env.SERVER_URL}/api/v2`);
16+
const req = http.request(
17+
{
18+
hostname: url.hostname,
19+
port: url.port,
20+
path: url.pathname,
21+
method: 'GET',
22+
headers: Sentry.getTraceData(),
23+
},
24+
res => {
25+
res.on('data', () => {});
26+
res.on('end', () => resolve());
27+
},
28+
);
29+
req.on('error', reject);
30+
req.end();
31+
});
32+
33+
Sentry.captureException(new Error('done'));
34+
}
35+
36+
run();

‎dev-packages/node-integration-tests/suites/tracing/double-baggage-no-spans/test.ts‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
import { createTestServer } from '@sentry-internal/test-utils';
2+
import { describe, expect } from 'vitest';
3+
import { createEsmAndCjsTests } from '../../../utils/runner';
4+
import { extractTraceparentData, parseBaggageHeader, TRACEPARENT_REGEXP } from '@sentry/core';
5+
6+
function expectNoDuplicateSentryBaggageKeys(baggage: string | string[] | undefined): void {
7+
expect(baggage).toBeDefined();
8+
const baggageStr = Array.isArray(baggage) ? baggage.join(',') : (baggage as string);
9+
const sentryEntries = baggageStr.split(',').filter(entry => entry.trim().startsWith('sentry-'));
10+
const sentryKeyNames = sentryEntries.map(entry => entry.trim().split('=')[0]);
11+
const uniqueKeyNames = [...new Set(sentryKeyNames)];
12+
expect(sentryKeyNames).toEqual(uniqueKeyNames);
13+
}
14+
15+
function expectConsistentTraceId(headers: Record<string, string | string[] | undefined>): void {
16+
const sentryTrace = headers['sentry-trace'];
17+
expect(sentryTrace).toMatch(TRACEPARENT_REGEXP);
18+
19+
const sentryTraceData = extractTraceparentData(sentryTrace as string)!;
20+
expect(sentryTraceData.traceId).toMatch(/^[a-f\d]{32}$/);
21+
22+
const baggage = parseBaggageHeader(headers['baggage']);
23+
24+
const baggageTraceId = baggage!['sentry-trace_id'];
25+
expect(baggageTraceId).toBeDefined();
26+
expect(baggageTraceId).toMatch(/^[a-f\d]{32}$/);
27+
28+
// both headers must have the same trace_id
29+
expect(sentryTraceData.traceId).toEqual(baggageTraceId);
30+
}
31+
32+
describe('double baggage prevention', () => {
33+
createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => {
34+
test('fetch with manual getTraceData() does not duplicate sentry baggage entries', async () => {
35+
const [SERVER_URL, closeTestServer] = await createTestServer()
36+
.get('/api/v0', headers => {
37+
// fetch with manual getTraceData() headers — core reproduction case
38+
expectNoDuplicateSentryBaggageKeys(headers['baggage']);
39+
console.log('xx sentry-trace', headers['sentry-trace']);
40+
expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^[a-f\d]{32}-[a-f\d]{16}$/));
41+
expectConsistentTraceId(headers);
42+
})
43+
.get('/api/v1', headers => {
44+
// fetch without manual headers (baseline)
45+
expectNoDuplicateSentryBaggageKeys(headers['baggage']);
46+
expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^[a-f\d]{32}-[a-f\d]{16}$/));
47+
expectConsistentTraceId(headers);
48+
})
49+
.get('/api/v2', headers => {
50+
// http.request with manual getTraceData() headers
51+
expectNoDuplicateSentryBaggageKeys(headers['baggage']);
52+
expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^[a-f\d]{32}-[a-f\d]{16}$/));
53+
expectConsistentTraceId(headers);
54+
})
55+
.start();
56+
57+
await createRunner()
58+
.withEnv({ SERVER_URL })
59+
.ignore('transaction')
60+
.expect({
61+
event: {
62+
exception: {
63+
values: [{ type: 'Error', value: 'done' }],
64+
},
65+
},
66+
})
67+
.start()
68+
.completed();
69+
closeTestServer();
70+
});
71+
});
72+
});

‎dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-no-parent/instrument.mjs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
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+
release: '1.0',
7+
tracesSampleRate: 1.0,
8+
transport: loggingTransport,
9+
});

‎dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-no-parent/scenario.mjs‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import * as Sentry from '@sentry/node';
2+
import http from 'http';
3+
4+
async function run() {
5+
// fetch with manual getTraceData() headers - the core reproduction case from #19158
6+
await fetch(`${process.env.SERVER_URL}/api/v0`, {
7+
headers: { ...Sentry.getTraceData() },
8+
}).then(res => res.text());
9+
10+
// fetch without manual headers (baseline - auto-instrumentation only)
11+
await fetch(`${process.env.SERVER_URL}/api/v1`).then(res => res.text());
12+
13+
// http.request with manual getTraceData() headers
14+
await new Promise((resolve, reject) => {
15+
const url = new URL(`${process.env.SERVER_URL}/api/v2`);
16+
const req = http.request(
17+
{
18+
hostname: url.hostname,
19+
port: url.port,
20+
path: url.pathname,
21+
method: 'GET',
22+
headers: Sentry.getTraceData(),
23+
},
24+
res => {
25+
res.on('data', () => {});
26+
res.on('end', () => resolve());
27+
},
28+
);
29+
req.on('error', reject);
30+
req.end();
31+
});
32+
33+
Sentry.captureException(new Error('done'));
34+
}
35+
36+
run();

‎dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-no-parent/test.ts‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
import { createTestServer } from '@sentry-internal/test-utils';
2+
import { describe, expect } from 'vitest';
3+
import { createEsmAndCjsTests } from '../../../utils/runner';
4+
import { extractTraceparentData, parseBaggageHeader, TRACEPARENT_REGEXP } from '@sentry/core';
5+
6+
function expectNoDuplicateSentryBaggageKeys(baggage: string | string[] | undefined): void {
7+
expect(baggage).toBeDefined();
8+
const baggageStr = Array.isArray(baggage) ? baggage.join(',') : (baggage as string);
9+
const sentryKeyNames = Object.keys(parseBaggageHeader(baggageStr) || {});
10+
const uniqueKeyNames = [...new Set(sentryKeyNames)];
11+
expect(sentryKeyNames).toEqual(uniqueKeyNames);
12+
}
13+
14+
function expectConsistentTraceId(headers: Record<string, string | string[] | undefined>): void {
15+
const sentryTrace = headers['sentry-trace'];
16+
expect(sentryTrace).toMatch(TRACEPARENT_REGEXP);
17+
18+
const sentryTraceData = extractTraceparentData(sentryTrace as string)!;
19+
expect(sentryTraceData.traceId).toMatch(/^[a-f\d]{32}$/);
20+
21+
const baggage = parseBaggageHeader(headers['baggage']);
22+
23+
const baggageTraceId = baggage!['sentry-trace_id'];
24+
expect(baggageTraceId).toBeDefined();
25+
expect(baggageTraceId).toMatch(/^[a-f\d]{32}$/);
26+
27+
expect(sentryTraceData.traceId).toEqual(baggageTraceId);
28+
}
29+
30+
describe('double baggage prevention', () => {
31+
createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => {
32+
test('fetch with manual getTraceData() does not duplicate sentry baggage entries', async () => {
33+
const [SERVER_URL, closeTestServer] = await createTestServer()
34+
.get('/api/v0', headers => {
35+
// fetch with manual getTraceData() headers — core reproduction case
36+
expectNoDuplicateSentryBaggageKeys(headers['baggage']);
37+
expect(headers['sentry-trace']).not.toContain(',');
38+
expectConsistentTraceId(headers);
39+
})
40+
.get('/api/v1', headers => {
41+
// fetch without manual headers (baseline)
42+
expectNoDuplicateSentryBaggageKeys(headers['baggage']);
43+
expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^[a-f\d]{32}-[a-f\d]{16}(-[01])?$/));
44+
expectConsistentTraceId(headers);
45+
})
46+
.get('/api/v2', headers => {
47+
// http.request with manual getTraceData() headers
48+
expectNoDuplicateSentryBaggageKeys(headers['baggage']);
49+
expect(headers['sentry-trace']).not.toContain(',');
50+
expectConsistentTraceId(headers);
51+
})
52+
.start();
53+
54+
await createRunner()
55+
.withEnv({ SERVER_URL })
56+
// .ignore('transaction')
57+
.expect({
58+
event: {
59+
exception: {
60+
values: [{ type: 'Error', value: 'done' }],
61+
},
62+
},
63+
})
64+
.start()
65+
.completed();
66+
closeTestServer();
67+
});
68+
});
69+
});

‎dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-parent/instrument.mjs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
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+
release: '1.0',
7+
tracesSampleRate: 1.0,
8+
transport: loggingTransport,
9+
});

‎dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-parent/scenario.mjs‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import * as Sentry from '@sentry/node';
2+
import http from 'http';
3+
4+
async function run() {
5+
// fetch with manual getTraceData() headers - the core reproduction case from #19158
6+
await fetch(`${process.env.SERVER_URL}/api/v0`, {
7+
headers: { ...Sentry.getTraceData() },
8+
}).then(res => res.text());
9+
10+
// fetch without manual headers (baseline - auto-instrumentation only)
11+
await fetch(`${process.env.SERVER_URL}/api/v1`).then(res => res.text());
12+
13+
// http.request with manual getTraceData() headers
14+
await new Promise((resolve, reject) => {
15+
const url = new URL(`${process.env.SERVER_URL}/api/v2`);
16+
const req = http.request(
17+
{
18+
hostname: url.hostname,
19+
port: url.port,
20+
path: url.pathname,
21+
method: 'GET',
22+
headers: Sentry.getTraceData(),
23+
},
24+
res => {
25+
res.on('data', () => {});
26+
res.on('end', () => resolve());
27+
},
28+
);
29+
req.on('error', reject);
30+
req.end();
31+
});
32+
33+
Sentry.captureException(new Error('done'));
34+
}
35+
36+
Sentry.startSpan({ name: 'parent_span' }, () => run());

‎dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-parent/test.ts‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
import { createTestServer } from '@sentry-internal/test-utils';
2+
import { describe, expect } from 'vitest';
3+
import { createEsmAndCjsTests } from '../../../utils/runner';
4+
import { extractTraceparentData, parseBaggageHeader, TRACEPARENT_REGEXP } from '@sentry/core';
5+
6+
function expectNoDuplicateSentryBaggageKeys(baggage: string | string[] | undefined): void {
7+
expect(baggage).toBeDefined();
8+
const baggageStr = Array.isArray(baggage) ? baggage.join(',') : (baggage as string);
9+
const sentryEntries = baggageStr.split(',').filter(entry => entry.trim().startsWith('sentry-'));
10+
const sentryKeyNames = sentryEntries.map(entry => entry.trim().split('=')[0]);
11+
const uniqueKeyNames = [...new Set(sentryKeyNames)];
12+
expect(sentryKeyNames).toEqual(uniqueKeyNames);
13+
}
14+
15+
function expectConsistentTraceId(headers: Record<string, string | string[] | undefined>): void {
16+
const sentryTrace = headers['sentry-trace'];
17+
expect(sentryTrace).toMatch(TRACEPARENT_REGEXP);
18+
19+
const sentryTraceData = extractTraceparentData(sentryTrace as string)!;
20+
expect(sentryTraceData.traceId).toMatch(/^[a-f\d]{32}$/);
21+
22+
const baggage = parseBaggageHeader(headers['baggage']);
23+
24+
const baggageTraceId = baggage!['sentry-trace_id'];
25+
expect(baggageTraceId).toBeDefined();
26+
expect(baggageTraceId).toMatch(/^[a-f\d]{32}$/);
27+
28+
expect(sentryTraceData.traceId).toEqual(baggageTraceId);
29+
}
30+
31+
describe('double baggage prevention - http.client spans with parent span', () => {
32+
createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => {
33+
test('fetch with manual getTraceData() does not duplicate sentry baggage entries', async () => {
34+
const [SERVER_URL, closeTestServer] = await createTestServer()
35+
.get('/api/v0', headers => {
36+
// fetch with manual getTraceData() headers — core reproduction case
37+
expectNoDuplicateSentryBaggageKeys(headers['baggage']);
38+
expect(headers['sentry-trace']).not.toContain(',');
39+
expectConsistentTraceId(headers);
40+
})
41+
.get('/api/v1', headers => {
42+
// fetch without manual headers (baseline)
43+
expectNoDuplicateSentryBaggageKeys(headers['baggage']);
44+
expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^[a-f\d]{32}-[a-f\d]{16}(-[01])?$/));
45+
expectConsistentTraceId(headers);
46+
})
47+
.get('/api/v2', headers => {
48+
// http.request with manual getTraceData() headers
49+
expectNoDuplicateSentryBaggageKeys(headers['baggage']);
50+
expect(headers['sentry-trace']).not.toContain(',');
51+
expectConsistentTraceId(headers);
52+
})
53+
.start();
54+
55+
await createRunner()
56+
.withEnv({ SERVER_URL })
57+
.ignore('event')
58+
.expect({
59+
transaction: {
60+
transaction: 'parent_span',
61+
spans: [
62+
{
63+
op: 'http.client',
64+
description: expect.stringMatching(/^GET .*\/api\/v0$/),
65+
data: {},
66+
span_id: expect.stringMatching(/[a-f0-9]{16}/),
67+
start_timestamp: expect.any(Number),
68+
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
69+
},
70+
{
71+
op: 'http.client',
72+
description: expect.stringMatching(/^GET .*\/api\/v1$/),
73+
data: {},
74+
span_id: expect.stringMatching(/[a-f0-9]{16}/),
75+
start_timestamp: expect.any(Number),
76+
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
77+
},
78+
{
79+
op: 'http.client',
80+
description: expect.stringMatching(/^GET .*\/api\/v2$/),
81+
data: {},
82+
span_id: expect.stringMatching(/[a-f0-9]{16}/),
83+
start_timestamp: expect.any(Number),
84+
trace_id: expect.stringMatching(/[a-f0-9]{32}/),
85+
},
86+
],
87+
},
88+
})
89+
.start()
90+
.completed();
91+
closeTestServer();
92+
});
93+
});
94+
});

0 commit comments

Comments
 (0)