Skip to content

Commit c58090e

Browse files
Lms24Lms24
committed
harden tests, fix http integration and propagator
1 parent a11ca57 commit c58090e

10 files changed

Lines changed: 199 additions & 69 deletions

File tree

‎dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-other-vendors-with-sentry-entries/test.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { afterAll, expect, test } from 'vitest';
22
import { cleanupChildProcesses, createRunner } from '../../../../utils/runner';
33
import type { TestAPIResponse } from '../server';
4+
import { extractTraceparentData } from '@sentry/core';
45

56
afterAll(() => {
67
cleanupChildProcesses();
@@ -12,7 +13,7 @@ test('should ignore sentry-values in `baggage` header of a third party vendor an
1213
const response = await runner.makeRequest<TestAPIResponse>('get', '/test/express', {
1314
headers: {
1415
'sentry-trace': '12312012123120121231201212312012-1121201211212012-1',
15-
baggage: 'sentry-release=2.1.0,sentry-environment=myEnv',
16+
baggage: 'sentry-release=2.1.0,sentry-environment=myEnv,sentry-sample_rate=0.54',
1617
},
1718
});
1819

@@ -46,6 +47,11 @@ test('should ignore sentry-values in `baggage` header of a third party vendor an
4647
expect(response).toBeDefined();
4748

4849
const baggage = response?.test_data.baggage?.split(',').sort();
50+
const sentryTraceHeader = response?.test_data['sentry-trace'];
51+
52+
const sentryTrace = extractTraceparentData(sentryTraceHeader);
53+
54+
expect(sentryTrace?.traceId).toMatch(/^[0-9a-f]{32}$/);
4955

5056
expect(response).toMatchObject({
5157
test_data: {
@@ -63,7 +69,7 @@ test('should ignore sentry-values in `baggage` header of a third party vendor an
6369
expect.stringMatching(/sentry-sample_rand=\d+/),
6470
'sentry-sample_rate=1',
6571
'sentry-sampled=true',
66-
expect.stringMatching(/sentry-trace_id=[\da-f]{32}/),
72+
`sentry-trace_id=${sentryTrace?.traceId}`,
6773
'sentry-transaction=GET%20%2Ftest%2Fexpress',
6874
'third=party',
6975
]);

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

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,24 +2,33 @@ import * as Sentry from '@sentry/node';
22
import http from 'http';
33

44
async function run() {
5+
const traceData = Sentry.getTraceData();
56
// fetch with manual getTraceData() headers - the core reproduction case from #19158
6-
await fetch(`${process.env.SERVER_URL}/api/v0`, {
7-
headers: { ...Sentry.getTraceData() },
7+
await fetch(`${process.env.SERVER_URL}/api/fetch-custom-headers`, {
8+
headers: {
9+
...traceData,
10+
'x-tracedata-sentry-trace': traceData['sentry-trace'],
11+
'x-tracedata-baggage': traceData.baggage,
12+
},
813
}).then(res => res.text());
914

1015
// fetch without manual headers (baseline - auto-instrumentation only)
11-
await fetch(`${process.env.SERVER_URL}/api/v1`).then(res => res.text());
16+
await fetch(`${process.env.SERVER_URL}/api/fetch`).then(res => res.text());
1217

1318
// http.request with manual getTraceData() headers
1419
await new Promise((resolve, reject) => {
15-
const url = new URL(`${process.env.SERVER_URL}/api/v2`);
20+
const url = new URL(`${process.env.SERVER_URL}/api/http-custom-headers`);
1621
const req = http.request(
1722
{
1823
hostname: url.hostname,
1924
port: url.port,
2025
path: url.pathname,
2126
method: 'GET',
22-
headers: Sentry.getTraceData(),
27+
headers: {
28+
...traceData,
29+
'x-tracedata-sentry-trace': traceData['sentry-trace'],
30+
'x-tracedata-baggage': traceData.baggage,
31+
},
2332
},
2433
res => {
2534
res.on('data', () => {});

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

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,27 +29,39 @@ function expectConsistentTraceId(headers: Record<string, string | string[] | und
2929
expect(sentryTraceData.traceId).toEqual(baggageTraceId);
3030
}
3131

32+
function expectUserSetTraceId(headers: Record<string, string | string[] | undefined>): void {
33+
const xSentryTrace = extractTraceparentData(headers['x-tracedata-sentry-trace'] as string);
34+
const sentryTrace = extractTraceparentData(headers['sentry-trace'] as string);
35+
expect(xSentryTrace?.traceId).toBe(sentryTrace?.traceId);
36+
37+
const xBaggage = parseBaggageHeader(headers['x-tracedata-baggage']);
38+
const baggage = parseBaggageHeader(headers['baggage']);
39+
expect(xBaggage).toEqual(baggage);
40+
}
41+
3242
describe('double baggage prevention', () => {
3343
createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => {
3444
test('fetch with manual getTraceData() does not duplicate sentry baggage entries', async () => {
3545
const [SERVER_URL, closeTestServer] = await createTestServer()
36-
.get('/api/v0', headers => {
37-
// fetch with manual getTraceData() headers — core reproduction case
38-
expectNoDuplicateSentryBaggageKeys(headers['baggage']);
46+
.get('/api/fetch-custom-headers', headers => {
47+
// fetch with manual getTraceData() headers
3948
expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^[a-f\d]{32}-[a-f\d]{16}$/));
49+
expectNoDuplicateSentryBaggageKeys(headers['baggage']);
4050
expectConsistentTraceId(headers);
51+
expectUserSetTraceId(headers);
4152
})
42-
.get('/api/v1', headers => {
53+
.get('/api/fetch', headers => {
4354
// fetch without manual headers (baseline)
44-
expectNoDuplicateSentryBaggageKeys(headers['baggage']);
4555
expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^[a-f\d]{32}-[a-f\d]{16}$/));
56+
expectNoDuplicateSentryBaggageKeys(headers['baggage']);
4657
expectConsistentTraceId(headers);
4758
})
48-
.get('/api/v2', headers => {
59+
.get('/api/http-custom-headers', headers => {
4960
// http.request with manual getTraceData() headers
50-
expectNoDuplicateSentryBaggageKeys(headers['baggage']);
5161
expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^[a-f\d]{32}-[a-f\d]{16}$/));
62+
expectNoDuplicateSentryBaggageKeys(headers['baggage']);
5263
expectConsistentTraceId(headers);
64+
expectUserSetTraceId(headers);
5365
})
5466
.start();
5567

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,5 @@ Sentry.init({
66
release: '1.0',
77
tracesSampleRate: 1.0,
88
transport: loggingTransport,
9+
debug: true,
910
});

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

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,24 +2,33 @@ import * as Sentry from '@sentry/node';
22
import http from 'http';
33

44
async function run() {
5+
const traceData = Sentry.getTraceData();
56
// fetch with manual getTraceData() headers - the core reproduction case from #19158
6-
await fetch(`${process.env.SERVER_URL}/api/v0`, {
7-
headers: { ...Sentry.getTraceData() },
7+
await fetch(`${process.env.SERVER_URL}/api/fetch-custom-headers`, {
8+
headers: {
9+
...traceData,
10+
'x-tracedata-sentry-trace': traceData['sentry-trace'],
11+
'x-tracedata-baggage': traceData.baggage,
12+
},
813
}).then(res => res.text());
914

1015
// fetch without manual headers (baseline - auto-instrumentation only)
11-
await fetch(`${process.env.SERVER_URL}/api/v1`).then(res => res.text());
16+
await fetch(`${process.env.SERVER_URL}/api/fetch`, {}).then(res => res.text());
1217

1318
// http.request with manual getTraceData() headers
1419
await new Promise((resolve, reject) => {
15-
const url = new URL(`${process.env.SERVER_URL}/api/v2`);
20+
const url = new URL(`${process.env.SERVER_URL}/api/http-custom-headers`);
1621
const req = http.request(
1722
{
1823
hostname: url.hostname,
1924
port: url.port,
2025
path: url.pathname,
2126
method: 'GET',
22-
headers: Sentry.getTraceData(),
27+
headers: {
28+
...traceData,
29+
'x-tracedata-sentry-trace': traceData['sentry-trace'],
30+
'x-tracedata-baggage': traceData.baggage,
31+
},
2332
},
2433
res => {
2534
res.on('data', () => {});

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

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,33 +27,44 @@ function expectConsistentTraceId(headers: Record<string, string | string[] | und
2727
expect(sentryTraceData.traceId).toEqual(baggageTraceId);
2828
}
2929

30+
function expectUserSetTraceId(headers: Record<string, string | string[] | undefined>): void {
31+
const xSentryTrace = extractTraceparentData(headers['x-tracedata-sentry-trace'] as string);
32+
const sentryTrace = extractTraceparentData(headers['sentry-trace'] as string);
33+
expect(xSentryTrace?.traceId).toBe(sentryTrace?.traceId);
34+
35+
const xBaggage = parseBaggageHeader(headers['x-tracedata-baggage']);
36+
const baggage = parseBaggageHeader(headers['baggage']);
37+
expect(xBaggage).toEqual(baggage);
38+
}
39+
3040
describe('double baggage prevention', () => {
3141
createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => {
3242
test('fetch with manual getTraceData() does not duplicate sentry baggage entries', async () => {
3343
const [SERVER_URL, closeTestServer] = await createTestServer()
34-
.get('/api/v0', headers => {
35-
// fetch with manual getTraceData() headers — core reproduction case
36-
expectNoDuplicateSentryBaggageKeys(headers['baggage']);
44+
.get('/api/fetch-custom-headers', headers => {
45+
// fetch with manual getTraceData() headers
3746
expect(headers['sentry-trace']).not.toContain(',');
47+
expectNoDuplicateSentryBaggageKeys(headers['baggage']);
3848
expectConsistentTraceId(headers);
49+
expectUserSetTraceId(headers);
3950
})
40-
.get('/api/v1', headers => {
51+
.get('/api/fetch', headers => {
4152
// fetch without manual headers (baseline)
42-
expectNoDuplicateSentryBaggageKeys(headers['baggage']);
4353
expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^[a-f\d]{32}-[a-f\d]{16}(-[01])?$/));
54+
expectNoDuplicateSentryBaggageKeys(headers['baggage']);
4455
expectConsistentTraceId(headers);
4556
})
46-
.get('/api/v2', headers => {
57+
.get('/api/http-custom-headers', headers => {
4758
// http.request with manual getTraceData() headers
48-
expectNoDuplicateSentryBaggageKeys(headers['baggage']);
4959
expect(headers['sentry-trace']).not.toContain(',');
60+
expectNoDuplicateSentryBaggageKeys(headers['baggage']);
5061
expectConsistentTraceId(headers);
62+
expectUserSetTraceId(headers);
5163
})
5264
.start();
5365

5466
await createRunner()
5567
.withEnv({ SERVER_URL })
56-
// .ignore('transaction')
5768
.expect({
5869
event: {
5970
exception: {

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

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,24 +2,38 @@ import * as Sentry from '@sentry/node';
22
import http from 'http';
33

44
async function run() {
5+
const traceData = Sentry.getTraceData();
56
// fetch with manual getTraceData() headers - the core reproduction case from #19158
6-
await fetch(`${process.env.SERVER_URL}/api/v0`, {
7-
headers: { ...Sentry.getTraceData() },
7+
await fetch(`${process.env.SERVER_URL}/api/fetch-custom-headers`, {
8+
headers: {
9+
...traceData,
10+
'x-tracedata-sentry-trace': traceData['sentry-trace'],
11+
'x-tracedata-baggage': traceData.baggage,
12+
},
813
}).then(res => res.text());
914

1015
// fetch without manual headers (baseline - auto-instrumentation only)
11-
await fetch(`${process.env.SERVER_URL}/api/v1`).then(res => res.text());
16+
await fetch(`${process.env.SERVER_URL}/api/fetch`, {
17+
headers: {
18+
'x-tracedata-sentry-trace': traceData['sentry-trace'],
19+
'x-tracedata-baggage': traceData.baggage,
20+
},
21+
}).then(res => res.text());
1222

1323
// http.request with manual getTraceData() headers
1424
await new Promise((resolve, reject) => {
15-
const url = new URL(`${process.env.SERVER_URL}/api/v2`);
25+
const url = new URL(`${process.env.SERVER_URL}/api/http-custom-headers`);
1626
const req = http.request(
1727
{
1828
hostname: url.hostname,
1929
port: url.port,
2030
path: url.pathname,
2131
method: 'GET',
22-
headers: Sentry.getTraceData(),
32+
headers: {
33+
...traceData,
34+
'x-tracedata-sentry-trace': traceData['sentry-trace'],
35+
'x-tracedata-baggage': traceData.baggage,
36+
},
2337
},
2438
res => {
2539
res.on('data', () => {});

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

Lines changed: 63 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -28,62 +28,94 @@ function expectConsistentTraceId(headers: Record<string, string | string[] | und
2828
expect(sentryTraceData.traceId).toEqual(baggageTraceId);
2929
}
3030

31+
function expectUserSetTraceId(headers: Record<string, string | string[] | undefined>): void {
32+
const xSentryTrace = extractTraceparentData(headers['x-tracedata-sentry-trace'] as string);
33+
const sentryTrace = extractTraceparentData(headers['sentry-trace'] as string);
34+
expect(xSentryTrace?.traceId).toBe(sentryTrace?.traceId);
35+
36+
const xBaggage = parseBaggageHeader(headers['x-tracedata-baggage']);
37+
const baggage = parseBaggageHeader(headers['baggage']);
38+
expect(xBaggage).toEqual(baggage);
39+
}
40+
3141
describe('double baggage prevention - http.client spans with parent span', () => {
3242
createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => {
43+
let transactionTraceId = '000';
44+
let fetchSpanId = '000';
45+
let httpCustomHeadersSpanId = '000';
46+
let fetchCustomHeadersSpanId = '000';
47+
3348
test('fetch with manual getTraceData() does not duplicate sentry baggage entries', async () => {
3449
const [SERVER_URL, closeTestServer] = await createTestServer()
35-
.get('/api/v0', headers => {
50+
.get('/api/fetch-custom-headers', headers => {
3651
// fetch with manual getTraceData() headers — core reproduction case
3752
expectNoDuplicateSentryBaggageKeys(headers['baggage']);
3853
expect(headers['sentry-trace']).not.toContain(',');
3954
expectConsistentTraceId(headers);
55+
expectUserSetTraceId(headers);
56+
const sentryTrace = extractTraceparentData(headers['sentry-trace'] as string);
57+
transactionTraceId = sentryTrace!.traceId!;
58+
fetchCustomHeadersSpanId = sentryTrace!.parentSpanId!;
4059
})
41-
.get('/api/v1', headers => {
60+
.get('/api/fetch', headers => {
4261
// fetch without manual headers (baseline)
4362
expectNoDuplicateSentryBaggageKeys(headers['baggage']);
4463
expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^[a-f\d]{32}-[a-f\d]{16}(-[01])?$/));
4564
expectConsistentTraceId(headers);
65+
const sentryTrace = extractTraceparentData(headers['sentry-trace'] as string);
66+
fetchSpanId = sentryTrace!.parentSpanId!;
4667
})
47-
.get('/api/v2', headers => {
68+
.get('/api/http-custom-headers', headers => {
4869
// http.request with manual getTraceData() headers
4970
expectNoDuplicateSentryBaggageKeys(headers['baggage']);
5071
expect(headers['sentry-trace']).not.toContain(',');
5172
expectConsistentTraceId(headers);
73+
expectUserSetTraceId(headers);
74+
const sentryTrace = extractTraceparentData(headers['sentry-trace'] as string);
75+
httpCustomHeadersSpanId = sentryTrace!.parentSpanId!;
5276
})
5377
.start();
5478

5579
await createRunner()
5680
.withEnv({ SERVER_URL })
5781
.ignore('event')
5882
.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-
],
83+
transaction: txn => {
84+
expect(transactionTraceId).toMatch(/^[a-f0-9]{32}$/);
85+
86+
expect(txn).toMatchObject({
87+
transaction: 'parent_span',
88+
spans: [
89+
{
90+
op: 'http.client',
91+
description: expect.stringMatching(/^GET .*\/api\/fetch-custom-headers$/),
92+
data: {},
93+
// span id is expected to be different since users call getTraceData() before the
94+
// http.client span is created
95+
span_id: expect.not.stringContaining(fetchCustomHeadersSpanId),
96+
start_timestamp: expect.any(Number),
97+
trace_id: transactionTraceId,
98+
},
99+
{
100+
op: 'http.client',
101+
description: expect.stringMatching(/^GET .*\/api\/fetch$/),
102+
data: {},
103+
span_id: fetchSpanId,
104+
start_timestamp: expect.any(Number),
105+
trace_id: transactionTraceId,
106+
},
107+
{
108+
op: 'http.client',
109+
description: expect.stringMatching(/^GET .*\/api\/http-custom-headers$/),
110+
data: {},
111+
// span id is expected to be different since users call getTraceData() before the
112+
// http.client span is created
113+
span_id: expect.not.stringContaining(httpCustomHeadersSpanId),
114+
start_timestamp: expect.any(Number),
115+
trace_id: transactionTraceId,
116+
},
117+
],
118+
});
87119
},
88120
})
89121
.start()

0 commit comments

Comments
 (0)