From 12c3f516e7dd550640a8a6ddc332b26a033d37e9 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 20 Mar 2026 17:16:13 +0100 Subject: [PATCH 01/19] wip --- packages/node-core/src/utils/baggage.ts | 4 +- .../src/utils/outgoingFetchRequest.ts | 77 +++++++++++++++---- .../src/utils/outgoingHttpRequest.ts | 4 +- packages/node-core/test/utils/baggage.test.ts | 29 ++++++- 4 files changed, 96 insertions(+), 18 deletions(-) diff --git a/packages/node-core/src/utils/baggage.ts b/packages/node-core/src/utils/baggage.ts index d236851559db..8828737e9bbb 100644 --- a/packages/node-core/src/utils/baggage.ts +++ b/packages/node-core/src/utils/baggage.ts @@ -1,4 +1,4 @@ -import { objectToBaggageHeader, parseBaggageHeader } from '@sentry/core'; +import { objectToBaggageHeader, parseBaggageHeader, SENTRY_BAGGAGE_KEY_PREFIX } from '@sentry/core'; /** * Merge two baggage headers into one. @@ -29,7 +29,7 @@ export function mergeBaggageHeaders { // Sentry-specific keys always take precedence from new baggage // Non-Sentry keys only added if not already present - if (key.startsWith('sentry-') || !mergedBaggageEntries[key]) { + if (key.startsWith(SENTRY_BAGGAGE_KEY_PREFIX) || !mergedBaggageEntries[key]) { mergedBaggageEntries[key] = value; } }); diff --git a/packages/node-core/src/utils/outgoingFetchRequest.ts b/packages/node-core/src/utils/outgoingFetchRequest.ts index ce04a26db1e0..d256bfa28188 100644 --- a/packages/node-core/src/utils/outgoingFetchRequest.ts +++ b/packages/node-core/src/utils/outgoingFetchRequest.ts @@ -14,8 +14,8 @@ import { mergeBaggageHeaders } from './baggage'; const SENTRY_TRACE_HEADER = 'sentry-trace'; const SENTRY_BAGGAGE_HEADER = 'baggage'; -// For baggage, we make sure to merge this into a possibly existing header -const BAGGAGE_HEADER_REGEX = /baggage: (.*)\r\n/; +const BAGGAGE_HEADER_REGEX_GLOBAL = /baggage: (.*)\r\n/g; +const SENTRY_TRACE_HEADER_REGEX_GLOBAL = /sentry-trace: .*\r\n/g; /** * Add trace propagation headers to an outgoing fetch/undici request. @@ -60,15 +60,44 @@ export function addTracePropagationHeadersToFetchRequest( requestHeaders.push('traceparent', traceparent); } - // For baggage, we make sure to merge this into a possibly existing header - const existingBaggagePos = requestHeaders.findIndex(header => header === SENTRY_BAGGAGE_HEADER); - if (baggage && existingBaggagePos === -1) { + // Consolidate all duplicate baggage entries into one, then merge with our new baggage. + // OTel's UndiciInstrumentation may append a second baggage header via propagation.inject(), + // so we need to handle multiple entries — not just the first one. + const baggagePositions: number[] = []; + for (let i = 0; i < requestHeaders.length; i++) { + if (requestHeaders[i] === SENTRY_BAGGAGE_HEADER) { + baggagePositions.push(i); + } + } + + if (baggage && !baggagePositions.length) { requestHeaders.push(SENTRY_BAGGAGE_HEADER, baggage); } else if (baggage) { - const existingBaggage = requestHeaders[existingBaggagePos + 1]; - const merged = mergeBaggageHeaders(existingBaggage, baggage); + // First, consolidate all existing baggage values into one + let consolidatedBaggage = requestHeaders[baggagePositions[0]! + 1] as string; + for (let i = baggagePositions.length - 1; i >= 1; i--) { + const pos = baggagePositions[i]!; + const val = requestHeaders[pos + 1] as string; + consolidatedBaggage = mergeBaggageHeaders(consolidatedBaggage, val) || consolidatedBaggage; + requestHeaders.splice(pos, 2); + } + + // Then merge with the new baggage we want to add + const merged = mergeBaggageHeaders(consolidatedBaggage, baggage); if (merged) { - requestHeaders[existingBaggagePos + 1] = merged; + requestHeaders[baggagePositions[0]! + 1] = merged; + } + } + + // Also deduplicate sentry-trace headers — keep only the first occurrence. + // OTel's UndiciInstrumentation may have appended a second one via propagation.inject(). + let firstSentryTraceFound = false; + for (let i = requestHeaders.length - 2; i >= 0; i--) { + if (requestHeaders[i] === SENTRY_TRACE_HEADER) { + if (firstSentryTraceFound) { + requestHeaders.splice(i, 2); + } + firstSentryTraceFound = true; } } } else { @@ -82,15 +111,37 @@ export function addTracePropagationHeadersToFetchRequest( request.headers += `traceparent: ${traceparent}\r\n`; } - const existingBaggage = request.headers.match(BAGGAGE_HEADER_REGEX)?.[1]; - if (baggage && !existingBaggage) { + // Consolidate all duplicate baggage entries into one, then merge with our new baggage. + // OTel's UndiciInstrumentation may append a second baggage header via propagation.inject(), + // so we need to handle multiple entries — not just the first one. + const allBaggageMatches = request.headers.matchAll(BAGGAGE_HEADER_REGEX_GLOBAL); + let consolidatedBaggage: string | undefined; + for (const match of allBaggageMatches) { + if (match[1]) { + consolidatedBaggage = consolidatedBaggage + ? mergeBaggageHeaders(consolidatedBaggage, match[1]) || consolidatedBaggage + : match[1]; + } + } + + // Remove all existing baggage entries + request.headers = request.headers.replace(BAGGAGE_HEADER_REGEX_GLOBAL, ''); + + if (baggage && !consolidatedBaggage) { request.headers += `${SENTRY_BAGGAGE_HEADER}: ${baggage}\r\n`; - } else if (baggage) { - const merged = mergeBaggageHeaders(existingBaggage, baggage); + } else if (baggage && consolidatedBaggage) { + const merged = mergeBaggageHeaders(consolidatedBaggage, baggage); if (merged) { - request.headers = request.headers.replace(BAGGAGE_HEADER_REGEX, `baggage: ${merged}\r\n`); + request.headers += `${SENTRY_BAGGAGE_HEADER}: ${merged}\r\n`; } } + + // Deduplicate sentry-trace headers — keep only the first occurrence. + let sentryTraceCount = 0; + request.headers = request.headers.replace(SENTRY_TRACE_HEADER_REGEX_GLOBAL, match => { + sentryTraceCount++; + return sentryTraceCount === 1 ? match : ''; + }); } } diff --git a/packages/node-core/src/utils/outgoingHttpRequest.ts b/packages/node-core/src/utils/outgoingHttpRequest.ts index 7eafa941286a..bd3ba9b04e5f 100644 --- a/packages/node-core/src/utils/outgoingHttpRequest.ts +++ b/packages/node-core/src/utils/outgoingHttpRequest.ts @@ -92,7 +92,9 @@ export function addTracePropagationHeadersToOutgoingRequest( } if (baggage) { - const newBaggage = mergeBaggageHeaders(request.getHeader('baggage'), baggage); + const existingBaggage = request.getHeader('baggage'); + + const newBaggage = mergeBaggageHeaders(existingBaggage, baggage); if (newBaggage) { try { request.setHeader('baggage', newBaggage); diff --git a/packages/node-core/test/utils/baggage.test.ts b/packages/node-core/test/utils/baggage.test.ts index aae5c48d6068..98ced72c1985 100644 --- a/packages/node-core/test/utils/baggage.test.ts +++ b/packages/node-core/test/utils/baggage.test.ts @@ -87,7 +87,7 @@ describe('mergeBaggageHeaders', () => { it('handles array-type existing baggage', () => { const result = mergeBaggageHeaders(['foo=bar', 'other=vendor'], 'sentry-release=1.0.0'); - const entries = result?.split(','); + const entries = (result as string)?.split(','); expect(entries).toContain('foo=bar'); expect(entries).toContain('other=vendor'); expect(entries).toContain('sentry-release=1.0.0'); @@ -115,7 +115,7 @@ describe('mergeBaggageHeaders', () => { expect(entries).not.toContain('sentry-environment=old'); }); - it('matches OTEL propagation.inject() behavior for Sentry keys', () => { + it('overwrites existing Sentry entries with new SDK values', () => { const result = mergeBaggageHeaders( 'sentry-trace_id=abc123,sentry-sampled=false,non-sentry=keep', 'sentry-trace_id=xyz789,sentry-sampled=true', @@ -128,4 +128,29 @@ describe('mergeBaggageHeaders', () => { expect(entries).not.toContain('sentry-trace_id=abc123'); expect(entries).not.toContain('sentry-sampled=false'); }); + + it('merges non-conflicting baggage entries', () => { + const existing = 'custom-key=value'; + const newBaggage = 'sentry-environment=production'; + const result = mergeBaggageHeaders(existing, newBaggage); + expect(result).toContain('custom-key=value'); + expect(result).toContain('sentry-environment=production'); + }); + + it('overwrites existing Sentry entries when keys conflict', () => { + const existing = 'sentry-environment=staging'; + const newBaggage = 'sentry-environment=production'; + const result = mergeBaggageHeaders(existing, newBaggage); + expect(result).toBe('sentry-environment=production'); + }); + + it('handles multiple entries with Sentry conflicts', () => { + const existing = 'custom-key=value1,sentry-environment=staging'; + const newBaggage = 'sentry-environment=production,sentry-trace_id=123'; + const result = mergeBaggageHeaders(existing, newBaggage); + expect(result).toContain('custom-key=value1'); + expect(result).toContain('sentry-environment=production'); + expect(result).toContain('sentry-trace_id=123'); + expect(result).not.toContain('sentry-environment=staging'); + }); }); From 63b766656cf369bcba856bcb4debeedb27ecf449 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 23 Mar 2026 18:19:09 +0100 Subject: [PATCH 02/19] use dedupe approach --- .../src/utils/outgoingFetchRequest.ts | 156 +++++++++++------- 1 file changed, 93 insertions(+), 63 deletions(-) diff --git a/packages/node-core/src/utils/outgoingFetchRequest.ts b/packages/node-core/src/utils/outgoingFetchRequest.ts index d256bfa28188..817ed76d1d9e 100644 --- a/packages/node-core/src/utils/outgoingFetchRequest.ts +++ b/packages/node-core/src/utils/outgoingFetchRequest.ts @@ -14,8 +14,8 @@ import { mergeBaggageHeaders } from './baggage'; const SENTRY_TRACE_HEADER = 'sentry-trace'; const SENTRY_BAGGAGE_HEADER = 'baggage'; -const BAGGAGE_HEADER_REGEX_GLOBAL = /baggage: (.*)\r\n/g; -const SENTRY_TRACE_HEADER_REGEX_GLOBAL = /sentry-trace: .*\r\n/g; +// For baggage, we make sure to merge this into a possibly existing header +const BAGGAGE_HEADER_REGEX = /baggage: (.*)\r\n/; /** * Add trace propagation headers to an outgoing fetch/undici request. @@ -45,6 +45,12 @@ export function addTracePropagationHeadersToFetchRequest( const { 'sentry-trace': sentryTrace, baggage, traceparent } = addedHeaders; + // OTel's UndiciInstrumentation calls propagation.inject() which unconditionally + // appends headers to the request. When the user also sets headers via getTraceData(), + // this results in duplicate sentry-trace and baggage entries. + // We clean these up before applying our own logic. + _deduplicateHeaders(request); + // We do not want to overwrite existing headers here // If the core UndiciInstrumentation is registered, it will already have set the headers // We do not want to add any then @@ -60,89 +66,113 @@ export function addTracePropagationHeadersToFetchRequest( requestHeaders.push('traceparent', traceparent); } - // Consolidate all duplicate baggage entries into one, then merge with our new baggage. - // OTel's UndiciInstrumentation may append a second baggage header via propagation.inject(), - // so we need to handle multiple entries — not just the first one. - const baggagePositions: number[] = []; - for (let i = 0; i < requestHeaders.length; i++) { - if (requestHeaders[i] === SENTRY_BAGGAGE_HEADER) { - baggagePositions.push(i); - } - } - - if (baggage && !baggagePositions.length) { + // For baggage, we make sure to merge this into a possibly existing header + const existingBaggagePos = requestHeaders.findIndex(header => header === SENTRY_BAGGAGE_HEADER); + if (baggage && existingBaggagePos === -1) { requestHeaders.push(SENTRY_BAGGAGE_HEADER, baggage); } else if (baggage) { - // First, consolidate all existing baggage values into one - let consolidatedBaggage = requestHeaders[baggagePositions[0]! + 1] as string; - for (let i = baggagePositions.length - 1; i >= 1; i--) { - const pos = baggagePositions[i]!; - const val = requestHeaders[pos + 1] as string; - consolidatedBaggage = mergeBaggageHeaders(consolidatedBaggage, val) || consolidatedBaggage; - requestHeaders.splice(pos, 2); - } - - // Then merge with the new baggage we want to add - const merged = mergeBaggageHeaders(consolidatedBaggage, baggage); + // headers in format [key_0, value_0, key_1, value_1, ...], hence the +1 here + const existingBaggage = requestHeaders[existingBaggagePos + 1]; + const merged = mergeBaggageHeaders(existingBaggage, baggage); if (merged) { - requestHeaders[baggagePositions[0]! + 1] = merged; - } - } - - // Also deduplicate sentry-trace headers — keep only the first occurrence. - // OTel's UndiciInstrumentation may have appended a second one via propagation.inject(). - let firstSentryTraceFound = false; - for (let i = requestHeaders.length - 2; i >= 0; i--) { - if (requestHeaders[i] === SENTRY_TRACE_HEADER) { - if (firstSentryTraceFound) { - requestHeaders.splice(i, 2); - } - firstSentryTraceFound = true; + requestHeaders[existingBaggagePos + 1] = merged; } } } else { - const requestHeaders = request.headers; // We do not want to overwrite existing header here, if it was already set - if (sentryTrace && !requestHeaders.includes(`${SENTRY_TRACE_HEADER}:`)) { + if (sentryTrace && !request.headers.includes(`${SENTRY_TRACE_HEADER}:`)) { request.headers += `${SENTRY_TRACE_HEADER}: ${sentryTrace}\r\n`; } - if (traceparent && !requestHeaders.includes('traceparent:')) { + if (traceparent && !request.headers.includes('traceparent:')) { request.headers += `traceparent: ${traceparent}\r\n`; } - // Consolidate all duplicate baggage entries into one, then merge with our new baggage. - // OTel's UndiciInstrumentation may append a second baggage header via propagation.inject(), - // so we need to handle multiple entries — not just the first one. - const allBaggageMatches = request.headers.matchAll(BAGGAGE_HEADER_REGEX_GLOBAL); - let consolidatedBaggage: string | undefined; - for (const match of allBaggageMatches) { - if (match[1]) { - consolidatedBaggage = consolidatedBaggage - ? mergeBaggageHeaders(consolidatedBaggage, match[1]) || consolidatedBaggage - : match[1]; + const existingBaggage = request.headers.match(BAGGAGE_HEADER_REGEX)?.[1]; + if (baggage && !existingBaggage) { + request.headers += `${SENTRY_BAGGAGE_HEADER}: ${baggage}\r\n`; + } else if (baggage) { + const merged = mergeBaggageHeaders(existingBaggage, baggage); + if (merged) { + request.headers = request.headers.replace(BAGGAGE_HEADER_REGEX, `baggage: ${merged}\r\n`); } } + } +} - // Remove all existing baggage entries - request.headers = request.headers.replace(BAGGAGE_HEADER_REGEX_GLOBAL, ''); +/** + * Remove duplicate sentry-trace and baggage headers from the request. + * + * OTel's UndiciInstrumentation unconditionally appends headers via propagation.inject(), + * which can create duplicates when the user has already set these headers (e.g. via getTraceData()). + * For sentry-trace, we keep the first occurrence (user-set). + * For baggage, we merge all occurrences into one to preserve both sentry and non-sentry entries. + */ +function _deduplicateHeaders(request: UndiciRequest): void { + if (Array.isArray(request.headers)) { + _deduplicateArrayHeaders(request.headers); + } else if (typeof request.headers === 'string') { + request.headers = _deduplicateStringHeaders(request.headers); + } +} - if (baggage && !consolidatedBaggage) { - request.headers += `${SENTRY_BAGGAGE_HEADER}: ${baggage}\r\n`; - } else if (baggage && consolidatedBaggage) { - const merged = mergeBaggageHeaders(consolidatedBaggage, baggage); +function _deduplicateArrayHeaders(headers: (string | string[])[]): void { + _deduplicateArrayHeader(headers, SENTRY_TRACE_HEADER); + _deduplicateArrayHeader(headers, SENTRY_BAGGAGE_HEADER); +} + +/** + * For a given header name, if there are multiple entries in the [key, value, key, value, ...] array, + * keep the first entry and remove the rest. + * For baggage, values are merged to preserve all entries. For other headers, the first value wins. + */ +function _deduplicateArrayHeader(headers: (string | string[])[], name: string): void { + let firstPos = -1; + for (let i = 0; i < headers.length; i += 2) { + if (headers[i] !== name) { + continue; + } + + if (firstPos === -1) { + firstPos = i; + continue; + } + + // Duplicate found after firstPos. Merge into firstPos and remove. + if (name === SENTRY_BAGGAGE_HEADER) { + const merged = mergeBaggageHeaders(headers[firstPos + 1] as string, headers[i + 1] as string); if (merged) { - request.headers += `${SENTRY_BAGGAGE_HEADER}: ${merged}\r\n`; + headers[firstPos + 1] = merged; } } + headers.splice(i, 2); + i -= 2; + } +} + +function _deduplicateStringHeaders(input: string): string { + // Deduplicate sentry-trace — keep only the first occurrence + let sentryTraceCount = 0; + let result = input.replace(/sentry-trace: .*\r\n/g, match => { + return ++sentryTraceCount === 1 ? match : ''; + }); + + // Deduplicate baggage — merge all occurrences into one + let mergedBaggage: string | undefined; + result = result.replace(/baggage: (.*)\r\n/g, (_match, value: string) => { + if (!mergedBaggage) { + mergedBaggage = value; + } else { + mergedBaggage = mergeBaggageHeaders(mergedBaggage, value) || mergedBaggage; + } + return ''; + }); - // Deduplicate sentry-trace headers — keep only the first occurrence. - let sentryTraceCount = 0; - request.headers = request.headers.replace(SENTRY_TRACE_HEADER_REGEX_GLOBAL, match => { - sentryTraceCount++; - return sentryTraceCount === 1 ? match : ''; - }); + if (mergedBaggage) { + result += `${SENTRY_BAGGAGE_HEADER}: ${mergedBaggage}\r\n`; } + + return result; } /** Add a breadcrumb for an outgoing fetch/undici request. */ From 8e7b3e8f0a553b440bd21143fa2efec80cc2abd7 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 23 Mar 2026 18:45:41 +0100 Subject: [PATCH 03/19] fix bug in baggage merging --- packages/node-core/src/utils/baggage.ts | 19 +++++++++++++++++-- packages/node-core/test/utils/baggage.test.ts | 11 ++++++++--- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/packages/node-core/src/utils/baggage.ts b/packages/node-core/src/utils/baggage.ts index 8828737e9bbb..30798ef6729c 100644 --- a/packages/node-core/src/utils/baggage.ts +++ b/packages/node-core/src/utils/baggage.ts @@ -24,8 +24,23 @@ export function mergeBaggageHeaders + key.startsWith(SENTRY_BAGGAGE_KEY_PREFIX), + ); + + const oldBaggageEntriesWithoutSentry = existingBaggageEntries + ? Object.entries(existingBaggageEntries).filter(([key]) => !key.startsWith(SENTRY_BAGGAGE_KEY_PREFIX)) + : []; + + // If new baggage contains at least one sentry- value, we remove all old sentry- values + // otherwise, we keep old sentry- values. If we don't remove old sentry- values, we end + // up with an inconsistent dynamic sampling context propagation. + const mergedBaggageEntries = newSentryBaggageEntries.length + ? Object.fromEntries(oldBaggageEntriesWithoutSentry) + : existingBaggageEntries + ? Object.fromEntries(Object.entries(existingBaggageEntries)) + : {}; + Object.entries(newBaggageEntries).forEach(([key, value]) => { // Sentry-specific keys always take precedence from new baggage // Non-Sentry keys only added if not already present diff --git a/packages/node-core/test/utils/baggage.test.ts b/packages/node-core/test/utils/baggage.test.ts index 98ced72c1985..16ace2039962 100644 --- a/packages/node-core/test/utils/baggage.test.ts +++ b/packages/node-core/test/utils/baggage.test.ts @@ -69,7 +69,6 @@ describe('mergeBaggageHeaders', () => { expect(entries).toContain('third=party'); expect(entries).toContain('sentry-environment=myEnv'); expect(entries).toContain('sentry-release=2.1.0'); - expect(entries).toContain('sentry-sample_rate=0.54'); expect(entries).not.toContain('sentry-environment=staging'); expect(entries).not.toContain('sentry-release=9.9.9'); }); @@ -133,8 +132,7 @@ describe('mergeBaggageHeaders', () => { const existing = 'custom-key=value'; const newBaggage = 'sentry-environment=production'; const result = mergeBaggageHeaders(existing, newBaggage); - expect(result).toContain('custom-key=value'); - expect(result).toContain('sentry-environment=production'); + expect(result).toBe('custom-key=value,sentry-environment=production'); }); it('overwrites existing Sentry entries when keys conflict', () => { @@ -153,4 +151,11 @@ describe('mergeBaggageHeaders', () => { expect(result).toContain('sentry-trace_id=123'); expect(result).not.toContain('sentry-environment=staging'); }); + + it('removes all sentry- values from old baggage and only adds new ones (if at least one new sentry- value is present)', () => { + const existing = 'sentry-trace_id=old,sentry-sampled=false,non-sentry=keep'; + const newBaggage = 'sentry-trace_id=new,sentry-environment=new'; + const result = mergeBaggageHeaders(existing, newBaggage); + expect(result).toBe('non-sentry=keep,sentry-trace_id=new,sentry-environment=new'); + }); }); From 32d3822eded30f45d16f4aa58f3e7cb5183fd7ab Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 24 Mar 2026 18:58:15 +0100 Subject: [PATCH 04/19] fix(node): Avoid duplicated `sentry-trace` and `baggage` headers on fetch requests --- .../fetch-trace-header-merging/test.ts | 1 + .../double-baggage-no-spans/instrument.mjs | 9 + .../double-baggage-no-spans/scenario.mjs | 36 ++ .../tracing/double-baggage-no-spans/test.ts | 72 ++++ .../instrument.mjs | 9 + .../scenario.mjs | 36 ++ .../double-baggage-spans-no-parent/test.ts | 69 ++++ .../instrument.mjs | 9 + .../double-baggage-spans-parent/scenario.mjs | 36 ++ .../double-baggage-spans-parent/test.ts | 94 ++++++ .../src/utils/outgoingFetchRequest.ts | 99 +++--- .../test/utils/outgoingFetchRequest.test.ts | 308 ++++++++++++++++++ packages/node/src/integrations/node-fetch.ts | 2 +- 13 files changed, 735 insertions(+), 45 deletions(-) create mode 100644 dev-packages/node-integration-tests/suites/tracing/double-baggage-no-spans/instrument.mjs create mode 100644 dev-packages/node-integration-tests/suites/tracing/double-baggage-no-spans/scenario.mjs create mode 100644 dev-packages/node-integration-tests/suites/tracing/double-baggage-no-spans/test.ts create mode 100644 dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-no-parent/instrument.mjs create mode 100644 dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-no-parent/scenario.mjs create mode 100644 dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-no-parent/test.ts create mode 100644 dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-parent/instrument.mjs create mode 100644 dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-parent/scenario.mjs create mode 100644 dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-parent/test.ts create mode 100644 packages/node-core/test/utils/outgoingFetchRequest.test.ts diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-trace-header-merging/test.ts b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-trace-header-merging/test.ts index 7c0f6db3483b..1385f6882dc7 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-trace-header-merging/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-trace-header-merging/test.ts @@ -39,6 +39,7 @@ async function assertRequests({ // No merged sentry trace headers expect(headers['sentry-trace']).not.toContain(','); + expect(headers['sentry-trace']).toBe('12312012123120121231201212312012-1121201211212012-1'); // No multiple baggage entries expect(headers['baggage'].match(/sentry-release/g) ?? []).toHaveLength(1); diff --git a/dev-packages/node-integration-tests/suites/tracing/double-baggage-no-spans/instrument.mjs b/dev-packages/node-integration-tests/suites/tracing/double-baggage-no-spans/instrument.mjs new file mode 100644 index 000000000000..7acd36926f22 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/double-baggage-no-spans/instrument.mjs @@ -0,0 +1,9 @@ +import * as Sentry from '@sentry/node'; +import { loggingTransport } from '@sentry-internal/node-integration-tests'; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + // explicitly not setting tracesSampleRate, + transport: loggingTransport, +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/double-baggage-no-spans/scenario.mjs b/dev-packages/node-integration-tests/suites/tracing/double-baggage-no-spans/scenario.mjs new file mode 100644 index 000000000000..fd3bd04efd9e --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/double-baggage-no-spans/scenario.mjs @@ -0,0 +1,36 @@ +import * as Sentry from '@sentry/node'; +import http from 'http'; + +async function run() { + // fetch with manual getTraceData() headers - the core reproduction case from #19158 + await fetch(`${process.env.SERVER_URL}/api/v0`, { + headers: { ...Sentry.getTraceData() }, + }).then(res => res.text()); + + // fetch without manual headers (baseline - auto-instrumentation only) + await fetch(`${process.env.SERVER_URL}/api/v1`).then(res => res.text()); + + // http.request with manual getTraceData() headers + await new Promise((resolve, reject) => { + const url = new URL(`${process.env.SERVER_URL}/api/v2`); + const req = http.request( + { + hostname: url.hostname, + port: url.port, + path: url.pathname, + method: 'GET', + headers: Sentry.getTraceData(), + }, + res => { + res.on('data', () => {}); + res.on('end', () => resolve()); + }, + ); + req.on('error', reject); + req.end(); + }); + + Sentry.captureException(new Error('done')); +} + +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/double-baggage-no-spans/test.ts b/dev-packages/node-integration-tests/suites/tracing/double-baggage-no-spans/test.ts new file mode 100644 index 000000000000..6ca5e474e342 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/double-baggage-no-spans/test.ts @@ -0,0 +1,72 @@ +import { createTestServer } from '@sentry-internal/test-utils'; +import { describe, expect } from 'vitest'; +import { createEsmAndCjsTests } from '../../../utils/runner'; +import { extractTraceparentData, parseBaggageHeader, TRACEPARENT_REGEXP } from '@sentry/core'; + +function expectNoDuplicateSentryBaggageKeys(baggage: string | string[] | undefined): void { + expect(baggage).toBeDefined(); + const baggageStr = Array.isArray(baggage) ? baggage.join(',') : (baggage as string); + const sentryEntries = baggageStr.split(',').filter(entry => entry.trim().startsWith('sentry-')); + const sentryKeyNames = sentryEntries.map(entry => entry.trim().split('=')[0]); + const uniqueKeyNames = [...new Set(sentryKeyNames)]; + expect(sentryKeyNames).toEqual(uniqueKeyNames); +} + +function expectConsistentTraceId(headers: Record): void { + const sentryTrace = headers['sentry-trace']; + expect(sentryTrace).toMatch(TRACEPARENT_REGEXP); + + const sentryTraceData = extractTraceparentData(sentryTrace as string)!; + expect(sentryTraceData.traceId).toMatch(/^[a-f\d]{32}$/); + + const baggage = parseBaggageHeader(headers['baggage']); + + const baggageTraceId = baggage!['sentry-trace_id']; + expect(baggageTraceId).toBeDefined(); + expect(baggageTraceId).toMatch(/^[a-f\d]{32}$/); + + // both headers must have the same trace_id + expect(sentryTraceData.traceId).toEqual(baggageTraceId); +} + +describe('double baggage prevention', () => { + createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { + test('fetch with manual getTraceData() does not duplicate sentry baggage entries', async () => { + const [SERVER_URL, closeTestServer] = await createTestServer() + .get('/api/v0', headers => { + // fetch with manual getTraceData() headers — core reproduction case + expectNoDuplicateSentryBaggageKeys(headers['baggage']); + console.log('xx sentry-trace', headers['sentry-trace']); + expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^[a-f\d]{32}-[a-f\d]{16}$/)); + expectConsistentTraceId(headers); + }) + .get('/api/v1', headers => { + // fetch without manual headers (baseline) + expectNoDuplicateSentryBaggageKeys(headers['baggage']); + expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^[a-f\d]{32}-[a-f\d]{16}$/)); + expectConsistentTraceId(headers); + }) + .get('/api/v2', headers => { + // http.request with manual getTraceData() headers + expectNoDuplicateSentryBaggageKeys(headers['baggage']); + expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^[a-f\d]{32}-[a-f\d]{16}$/)); + expectConsistentTraceId(headers); + }) + .start(); + + await createRunner() + .withEnv({ SERVER_URL }) + .ignore('transaction') + .expect({ + event: { + exception: { + values: [{ type: 'Error', value: 'done' }], + }, + }, + }) + .start() + .completed(); + closeTestServer(); + }); + }); +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-no-parent/instrument.mjs b/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-no-parent/instrument.mjs new file mode 100644 index 000000000000..46a27dd03b74 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-no-parent/instrument.mjs @@ -0,0 +1,9 @@ +import * as Sentry from '@sentry/node'; +import { loggingTransport } from '@sentry-internal/node-integration-tests'; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracesSampleRate: 1.0, + transport: loggingTransport, +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-no-parent/scenario.mjs b/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-no-parent/scenario.mjs new file mode 100644 index 000000000000..fd3bd04efd9e --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-no-parent/scenario.mjs @@ -0,0 +1,36 @@ +import * as Sentry from '@sentry/node'; +import http from 'http'; + +async function run() { + // fetch with manual getTraceData() headers - the core reproduction case from #19158 + await fetch(`${process.env.SERVER_URL}/api/v0`, { + headers: { ...Sentry.getTraceData() }, + }).then(res => res.text()); + + // fetch without manual headers (baseline - auto-instrumentation only) + await fetch(`${process.env.SERVER_URL}/api/v1`).then(res => res.text()); + + // http.request with manual getTraceData() headers + await new Promise((resolve, reject) => { + const url = new URL(`${process.env.SERVER_URL}/api/v2`); + const req = http.request( + { + hostname: url.hostname, + port: url.port, + path: url.pathname, + method: 'GET', + headers: Sentry.getTraceData(), + }, + res => { + res.on('data', () => {}); + res.on('end', () => resolve()); + }, + ); + req.on('error', reject); + req.end(); + }); + + Sentry.captureException(new Error('done')); +} + +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-no-parent/test.ts b/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-no-parent/test.ts new file mode 100644 index 000000000000..90f175963fa4 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-no-parent/test.ts @@ -0,0 +1,69 @@ +import { createTestServer } from '@sentry-internal/test-utils'; +import { describe, expect } from 'vitest'; +import { createEsmAndCjsTests } from '../../../utils/runner'; +import { extractTraceparentData, parseBaggageHeader, TRACEPARENT_REGEXP } from '@sentry/core'; + +function expectNoDuplicateSentryBaggageKeys(baggage: string | string[] | undefined): void { + expect(baggage).toBeDefined(); + const baggageStr = Array.isArray(baggage) ? baggage.join(',') : (baggage as string); + const sentryKeyNames = Object.keys(parseBaggageHeader(baggageStr) || {}); + const uniqueKeyNames = [...new Set(sentryKeyNames)]; + expect(sentryKeyNames).toEqual(uniqueKeyNames); +} + +function expectConsistentTraceId(headers: Record): void { + const sentryTrace = headers['sentry-trace']; + expect(sentryTrace).toMatch(TRACEPARENT_REGEXP); + + const sentryTraceData = extractTraceparentData(sentryTrace as string)!; + expect(sentryTraceData.traceId).toMatch(/^[a-f\d]{32}$/); + + const baggage = parseBaggageHeader(headers['baggage']); + + const baggageTraceId = baggage!['sentry-trace_id']; + expect(baggageTraceId).toBeDefined(); + expect(baggageTraceId).toMatch(/^[a-f\d]{32}$/); + + expect(sentryTraceData.traceId).toEqual(baggageTraceId); +} + +describe('double baggage prevention', () => { + createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { + test('fetch with manual getTraceData() does not duplicate sentry baggage entries', async () => { + const [SERVER_URL, closeTestServer] = await createTestServer() + .get('/api/v0', headers => { + // fetch with manual getTraceData() headers — core reproduction case + expectNoDuplicateSentryBaggageKeys(headers['baggage']); + expect(headers['sentry-trace']).not.toContain(','); + expectConsistentTraceId(headers); + }) + .get('/api/v1', headers => { + // fetch without manual headers (baseline) + expectNoDuplicateSentryBaggageKeys(headers['baggage']); + expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^[a-f\d]{32}-[a-f\d]{16}(-[01])?$/)); + expectConsistentTraceId(headers); + }) + .get('/api/v2', headers => { + // http.request with manual getTraceData() headers + expectNoDuplicateSentryBaggageKeys(headers['baggage']); + expect(headers['sentry-trace']).not.toContain(','); + expectConsistentTraceId(headers); + }) + .start(); + + await createRunner() + .withEnv({ SERVER_URL }) + // .ignore('transaction') + .expect({ + event: { + exception: { + values: [{ type: 'Error', value: 'done' }], + }, + }, + }) + .start() + .completed(); + closeTestServer(); + }); + }); +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-parent/instrument.mjs b/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-parent/instrument.mjs new file mode 100644 index 000000000000..46a27dd03b74 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-parent/instrument.mjs @@ -0,0 +1,9 @@ +import * as Sentry from '@sentry/node'; +import { loggingTransport } from '@sentry-internal/node-integration-tests'; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracesSampleRate: 1.0, + transport: loggingTransport, +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-parent/scenario.mjs b/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-parent/scenario.mjs new file mode 100644 index 000000000000..a9cb526cae3d --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-parent/scenario.mjs @@ -0,0 +1,36 @@ +import * as Sentry from '@sentry/node'; +import http from 'http'; + +async function run() { + // fetch with manual getTraceData() headers - the core reproduction case from #19158 + await fetch(`${process.env.SERVER_URL}/api/v0`, { + headers: { ...Sentry.getTraceData() }, + }).then(res => res.text()); + + // fetch without manual headers (baseline - auto-instrumentation only) + await fetch(`${process.env.SERVER_URL}/api/v1`).then(res => res.text()); + + // http.request with manual getTraceData() headers + await new Promise((resolve, reject) => { + const url = new URL(`${process.env.SERVER_URL}/api/v2`); + const req = http.request( + { + hostname: url.hostname, + port: url.port, + path: url.pathname, + method: 'GET', + headers: Sentry.getTraceData(), + }, + res => { + res.on('data', () => {}); + res.on('end', () => resolve()); + }, + ); + req.on('error', reject); + req.end(); + }); + + Sentry.captureException(new Error('done')); +} + +Sentry.startSpan({ name: 'parent_span' }, () => run()); diff --git a/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-parent/test.ts b/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-parent/test.ts new file mode 100644 index 000000000000..cc077e8c3ffe --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-parent/test.ts @@ -0,0 +1,94 @@ +import { createTestServer } from '@sentry-internal/test-utils'; +import { describe, expect } from 'vitest'; +import { createEsmAndCjsTests } from '../../../utils/runner'; +import { extractTraceparentData, parseBaggageHeader, TRACEPARENT_REGEXP } from '@sentry/core'; + +function expectNoDuplicateSentryBaggageKeys(baggage: string | string[] | undefined): void { + expect(baggage).toBeDefined(); + const baggageStr = Array.isArray(baggage) ? baggage.join(',') : (baggage as string); + const sentryEntries = baggageStr.split(',').filter(entry => entry.trim().startsWith('sentry-')); + const sentryKeyNames = sentryEntries.map(entry => entry.trim().split('=')[0]); + const uniqueKeyNames = [...new Set(sentryKeyNames)]; + expect(sentryKeyNames).toEqual(uniqueKeyNames); +} + +function expectConsistentTraceId(headers: Record): void { + const sentryTrace = headers['sentry-trace']; + expect(sentryTrace).toMatch(TRACEPARENT_REGEXP); + + const sentryTraceData = extractTraceparentData(sentryTrace as string)!; + expect(sentryTraceData.traceId).toMatch(/^[a-f\d]{32}$/); + + const baggage = parseBaggageHeader(headers['baggage']); + + const baggageTraceId = baggage!['sentry-trace_id']; + expect(baggageTraceId).toBeDefined(); + expect(baggageTraceId).toMatch(/^[a-f\d]{32}$/); + + expect(sentryTraceData.traceId).toEqual(baggageTraceId); +} + +describe('double baggage prevention - http.client spans with parent span', () => { + createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { + test('fetch with manual getTraceData() does not duplicate sentry baggage entries', async () => { + const [SERVER_URL, closeTestServer] = await createTestServer() + .get('/api/v0', headers => { + // fetch with manual getTraceData() headers — core reproduction case + expectNoDuplicateSentryBaggageKeys(headers['baggage']); + expect(headers['sentry-trace']).not.toContain(','); + expectConsistentTraceId(headers); + }) + .get('/api/v1', headers => { + // fetch without manual headers (baseline) + expectNoDuplicateSentryBaggageKeys(headers['baggage']); + expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^[a-f\d]{32}-[a-f\d]{16}(-[01])?$/)); + expectConsistentTraceId(headers); + }) + .get('/api/v2', headers => { + // http.request with manual getTraceData() headers + expectNoDuplicateSentryBaggageKeys(headers['baggage']); + expect(headers['sentry-trace']).not.toContain(','); + expectConsistentTraceId(headers); + }) + .start(); + + await createRunner() + .withEnv({ SERVER_URL }) + .ignore('event') + .expect({ + transaction: { + transaction: 'parent_span', + spans: [ + { + op: 'http.client', + description: expect.stringMatching(/^GET .*\/api\/v0$/), + data: {}, + span_id: expect.stringMatching(/[a-f0-9]{16}/), + start_timestamp: expect.any(Number), + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + }, + { + op: 'http.client', + description: expect.stringMatching(/^GET .*\/api\/v1$/), + data: {}, + span_id: expect.stringMatching(/[a-f0-9]{16}/), + start_timestamp: expect.any(Number), + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + }, + { + op: 'http.client', + description: expect.stringMatching(/^GET .*\/api\/v2$/), + data: {}, + span_id: expect.stringMatching(/[a-f0-9]{16}/), + start_timestamp: expect.any(Number), + trace_id: expect.stringMatching(/[a-f0-9]{32}/), + }, + ], + }, + }) + .start() + .completed(); + closeTestServer(); + }); + }); +}); diff --git a/packages/node-core/src/utils/outgoingFetchRequest.ts b/packages/node-core/src/utils/outgoingFetchRequest.ts index 817ed76d1d9e..9c1de10759c9 100644 --- a/packages/node-core/src/utils/outgoingFetchRequest.ts +++ b/packages/node-core/src/utils/outgoingFetchRequest.ts @@ -57,44 +57,53 @@ export function addTracePropagationHeadersToFetchRequest( if (Array.isArray(request.headers)) { const requestHeaders = request.headers; - // We do not want to overwrite existing header here, if it was already set - if (sentryTrace && !requestHeaders.includes(SENTRY_TRACE_HEADER)) { - requestHeaders.push(SENTRY_TRACE_HEADER, sentryTrace); - } + const hasExistingSentryTraceHeader = requestHeaders.includes(SENTRY_TRACE_HEADER); - if (traceparent && !requestHeaders.includes('traceparent')) { - requestHeaders.push('traceparent', traceparent); - } + // We do not want to set any headers if we already have an existing sentry-trace header. + // This is still the source of truth, otherwise we risk mixing up baggage and sentry-trace values. + if (!hasExistingSentryTraceHeader) { + if (sentryTrace) { + requestHeaders.push(SENTRY_TRACE_HEADER, sentryTrace); + } - // For baggage, we make sure to merge this into a possibly existing header - const existingBaggagePos = requestHeaders.findIndex(header => header === SENTRY_BAGGAGE_HEADER); - if (baggage && existingBaggagePos === -1) { - requestHeaders.push(SENTRY_BAGGAGE_HEADER, baggage); - } else if (baggage) { - // headers in format [key_0, value_0, key_1, value_1, ...], hence the +1 here - const existingBaggage = requestHeaders[existingBaggagePos + 1]; - const merged = mergeBaggageHeaders(existingBaggage, baggage); - if (merged) { - requestHeaders[existingBaggagePos + 1] = merged; + if (traceparent && !requestHeaders.includes('traceparent')) { + requestHeaders.push('traceparent', traceparent); + } + + // For baggage, we make sure to merge this into a possibly existing header + const existingBaggagePos = requestHeaders.findIndex(header => header === SENTRY_BAGGAGE_HEADER); + if (baggage && existingBaggagePos === -1) { + requestHeaders.push(SENTRY_BAGGAGE_HEADER, baggage); + } else if (baggage) { + // headers in format [key_0, value_0, key_1, value_1, ...], hence the +1 here + const existingBaggage = requestHeaders[existingBaggagePos + 1]; + const merged = mergeBaggageHeaders(existingBaggage, baggage); + if (merged) { + requestHeaders[existingBaggagePos + 1] = merged; + } } } } else { // We do not want to overwrite existing header here, if it was already set - if (sentryTrace && !request.headers.includes(`${SENTRY_TRACE_HEADER}:`)) { - request.headers += `${SENTRY_TRACE_HEADER}: ${sentryTrace}\r\n`; - } + const hasExistingSentryTraceHeader = request.headers.includes(`${SENTRY_TRACE_HEADER}:`); - if (traceparent && !request.headers.includes('traceparent:')) { - request.headers += `traceparent: ${traceparent}\r\n`; - } + if (!hasExistingSentryTraceHeader) { + if (sentryTrace) { + request.headers += `${SENTRY_TRACE_HEADER}: ${sentryTrace}\r\n`; + } - const existingBaggage = request.headers.match(BAGGAGE_HEADER_REGEX)?.[1]; - if (baggage && !existingBaggage) { - request.headers += `${SENTRY_BAGGAGE_HEADER}: ${baggage}\r\n`; - } else if (baggage) { - const merged = mergeBaggageHeaders(existingBaggage, baggage); - if (merged) { - request.headers = request.headers.replace(BAGGAGE_HEADER_REGEX, `baggage: ${merged}\r\n`); + if (traceparent && !request.headers.includes('traceparent:')) { + request.headers += `traceparent: ${traceparent}\r\n`; + } + + const existingBaggage = request.headers.match(BAGGAGE_HEADER_REGEX)?.[1]; + if (baggage && !existingBaggage) { + request.headers += `${SENTRY_BAGGAGE_HEADER}: ${baggage}\r\n`; + } else if (baggage) { + const merged = mergeBaggageHeaders(existingBaggage, baggage); + if (merged) { + request.headers = request.headers.replace(BAGGAGE_HEADER_REGEX, `baggage: ${merged}\r\n`); + } } } } @@ -111,12 +120,12 @@ export function addTracePropagationHeadersToFetchRequest( function _deduplicateHeaders(request: UndiciRequest): void { if (Array.isArray(request.headers)) { _deduplicateArrayHeaders(request.headers); - } else if (typeof request.headers === 'string') { + } else { request.headers = _deduplicateStringHeaders(request.headers); } } -function _deduplicateArrayHeaders(headers: (string | string[])[]): void { +function _deduplicateArrayHeaders(headers: string[]): void { _deduplicateArrayHeader(headers, SENTRY_TRACE_HEADER); _deduplicateArrayHeader(headers, SENTRY_BAGGAGE_HEADER); } @@ -124,25 +133,27 @@ function _deduplicateArrayHeaders(headers: (string | string[])[]): void { /** * For a given header name, if there are multiple entries in the [key, value, key, value, ...] array, * keep the first entry and remove the rest. - * For baggage, values are merged to preserve all entries. For other headers, the first value wins. + * For baggage, values are merged to preserve all entries but to dedupe sentry- values, and always + * keept the first occurance of them */ -function _deduplicateArrayHeader(headers: (string | string[])[], name: string): void { - let firstPos = -1; +function _deduplicateArrayHeader(headers: string[], headerName: string): void { + let firstIndex = -1; for (let i = 0; i < headers.length; i += 2) { - if (headers[i] !== name) { + if (headers[i] !== headerName) { continue; } - if (firstPos === -1) { - firstPos = i; + if (firstIndex === -1) { + firstIndex = i; continue; } - // Duplicate found after firstPos. Merge into firstPos and remove. - if (name === SENTRY_BAGGAGE_HEADER) { - const merged = mergeBaggageHeaders(headers[firstPos + 1] as string, headers[i + 1] as string); + if (headerName === SENTRY_BAGGAGE_HEADER) { + // merge the initial entry into the later occurance so that we keep the initial sentry- values around. + // all other non-sentry values are merged + const merged = mergeBaggageHeaders(headers[i + 1] as string, headers[firstIndex + 1] as string); if (merged) { - headers[firstPos + 1] = merged; + headers[firstIndex + 1] = merged; } } headers.splice(i, 2); @@ -157,13 +168,13 @@ function _deduplicateStringHeaders(input: string): string { return ++sentryTraceCount === 1 ? match : ''; }); - // Deduplicate baggage — merge all occurrences into one + // Deduplicate baggage — merge all occurrences into one but preserve initial sentry- values let mergedBaggage: string | undefined; result = result.replace(/baggage: (.*)\r\n/g, (_match, value: string) => { if (!mergedBaggage) { mergedBaggage = value; } else { - mergedBaggage = mergeBaggageHeaders(mergedBaggage, value) || mergedBaggage; + mergedBaggage = mergeBaggageHeaders(value, mergedBaggage) || mergedBaggage; } return ''; }); diff --git a/packages/node-core/test/utils/outgoingFetchRequest.test.ts b/packages/node-core/test/utils/outgoingFetchRequest.test.ts new file mode 100644 index 000000000000..d596dd5fda1d --- /dev/null +++ b/packages/node-core/test/utils/outgoingFetchRequest.test.ts @@ -0,0 +1,308 @@ +import { describe, beforeEach, vi, expect, it } from 'vitest'; +import type { UndiciRequest } from '../../src/integrations/node-fetch/types'; +import { addTracePropagationHeadersToFetchRequest } from '../../src/utils/outgoingFetchRequest'; +import { LRUMap } from '@sentry/core'; +import * as SentryCore from '@sentry/core'; + +const mockedGetTraceData = vi.hoisted(() => + vi.fn(() => ({ + 'sentry-trace': 'trace_id_1-span_id_1-1', + baggage: 'sentry-trace_id=trace_id_1,sentry-sampled=true,sentry-environment=staging', + })), +); + +vi.mock('@sentry/core', async () => { + const actual = await vi.importActual('@sentry/core'); + return { + ...actual, + getClient: vi.fn(() => ({ + getOptions: vi.fn(() => ({ + tracePropagationTargets: ['https://example.com'], + })), + })), + shouldPropagateTraceForUrl: () => true, + getTraceData: mockedGetTraceData, + }; +}); + +describe('addTracePropagationHeadersToFetchRequest', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("doesn't add headers if shouldPropagateTraceForUrl returns false", () => { + vi.spyOn(SentryCore, 'shouldPropagateTraceForUrl').mockReturnValueOnce(false); + + const request = { + headers: [] as string[], + origin: 'https://some-service.com', + path: '/api/test', + } as UndiciRequest; + + addTracePropagationHeadersToFetchRequest(request, new LRUMap(100)); + + expect(request.headers).toEqual([]); + }); + + describe('when headers are an array', () => { + it('adds sentry-trace and baggage headers to request', () => { + const request = { + headers: [] as string[], + origin: 'https://some-service.com', + path: '/api/test', + } as UndiciRequest; + + addTracePropagationHeadersToFetchRequest(request, new LRUMap(100)); + + expect(request.headers).toEqual([ + 'sentry-trace', + 'trace_id_1-span_id_1-1', + 'baggage', + 'sentry-trace_id=trace_id_1,sentry-sampled=true,sentry-environment=staging', + ]); + }); + + it('preserves non-sentry entries in existing baggage header', () => { + const request = { + headers: ['baggage', 'other=entry,not=sentry'], + origin: 'https://some-service.com', + path: '/api/test', + } as UndiciRequest; + + addTracePropagationHeadersToFetchRequest(request, new LRUMap(100)); + + expect(request.headers).toEqual([ + 'baggage', + 'other=entry,not=sentry,sentry-trace_id=trace_id_1,sentry-sampled=true,sentry-environment=staging', + 'sentry-trace', + 'trace_id_1-span_id_1-1', + ]); + }); + + describe('when sentry-trace is already set', () => { + it("preserves original sentry-trace header doesn't add baggage", () => { + const request = { + headers: ['sentry-trace', 'trace_id_2-span_id_2-1'], + origin: 'https://some-service.com', + path: '/api/test', + } as UndiciRequest; + + addTracePropagationHeadersToFetchRequest(request, new LRUMap(100)); + + expect(request.headers).toEqual(['sentry-trace', 'trace_id_2-span_id_2-1']); + }); + + it('preserves original baggage header', () => { + const request = { + headers: [ + 'sentry-trace', + 'trace_id_2-span_id_2-1', + 'baggage', + 'sentry-trace_id=trace_id_2,sentry-sampled=true,sentry-environment=staging', + ], + origin: 'https://some-service.com', + path: '/api/test', + } as UndiciRequest; + + addTracePropagationHeadersToFetchRequest(request, new LRUMap(100)); + + expect(request.headers).toEqual([ + 'sentry-trace', + 'trace_id_2-span_id_2-1', + 'baggage', + 'sentry-trace_id=trace_id_2,sentry-sampled=true,sentry-environment=staging', + ]); + }); + }); + + describe('pre-existing header deduplication', () => { + it('deduplicates sentry-trace and baggage headers', () => { + const request = { + headers: [ + 'sentry-trace', + 'user-trace_id-xyz-1', + 'baggage', + 'sentry-trace_id=user-trace_id-xyz-1,sentry-sampled=true,sentry-environment=user', + 'sentry-trace', + 'undici-trace_id-abc-1', + 'baggage', + 'sentry-trace_id=undici-trace_id-abc-1,sentry-sampled=true,sentry-environment=undici', + ], + origin: 'https://some-service.com', + path: '/api/test', + } as UndiciRequest; + + addTracePropagationHeadersToFetchRequest(request, new LRUMap(100)); + + expect(request.headers).toEqual([ + 'sentry-trace', + 'user-trace_id-xyz-1', + 'baggage', + 'sentry-trace_id=user-trace_id-xyz-1,sentry-sampled=true,sentry-environment=user', + ]); + }); + + // admittedly an unrealistic edge case but doesn't hurt to test it + it("doesn't crash with incomplete headers array", () => { + const request = { + headers: [ + 'sentry-trace', + 'user-trace_id-xyz-1', + 'baggage', + 'sentry-trace_id=user-trace_id-xyz-1,sentry-sampled=true,sentry-environment=user', + 'sentry-trace', + 'undici-trace_id-abc-1', + 'baggage', + 'sentry-trace_id=undici-trace_id-abc-1,sentry-sampled=true,sentry-environment=undici', + 'baggage', // only the key, no value + ], + origin: 'https://some-service.com', + path: '/api/test', + } as UndiciRequest; + + addTracePropagationHeadersToFetchRequest(request, new LRUMap(100)); + + expect(request.headers).toEqual([ + 'sentry-trace', + 'user-trace_id-xyz-1', + 'baggage', + 'sentry-trace_id=user-trace_id-xyz-1,sentry-sampled=true,sentry-environment=user', + ]); + }); + + it('dedupes multiple baggage headers with sentry- values keeps non-sentry values around', () => { + const request = { + headers: [ + 'sentry-trace', + 'user-trace_id-xyz-1', + 'baggage', + 'user-added=value,another=one', + 'baggage', + 'yet-another=value,another=two', + 'sentry-trace', + 'undici-trace_id-abc-1', + 'baggage', + 'sentry-trace_id=undici-trace_id-abc-1,sentry-sampled=true,sentry-environment=undici,', + ], + origin: 'https://some-service.com', + path: '/api/test', + } as UndiciRequest; + + addTracePropagationHeadersToFetchRequest(request, new LRUMap(100)); + + expect(request.headers).toEqual([ + 'sentry-trace', + 'user-trace_id-xyz-1', + 'baggage', + 'sentry-trace_id=undici-trace_id-abc-1,sentry-sampled=true,sentry-environment=undici,yet-another=value,another=two,user-added=value', + ]); + }); + + it('dedupes multiple baggage headers keeps non-sentry values around', () => { + const request = { + headers: ['baggage', 'user-added=value,another=one', 'baggage', 'yet-another=value,another=two'], + origin: 'https://some-service.com', + path: '/api/test', + } as UndiciRequest; + + addTracePropagationHeadersToFetchRequest(request, new LRUMap(100)); + + expect(request.headers).toEqual([ + 'baggage', + 'yet-another=value,another=two,user-added=value,sentry-trace_id=trace_id_1,sentry-sampled=true,sentry-environment=staging', + 'sentry-trace', + 'trace_id_1-span_id_1-1', + ]); + }); + }); + }); + + describe('when headers are a string', () => { + it('adds sentry-trace and baggage headers to request', () => { + const request = { + headers: '', + origin: 'https://some-service.com', + path: '/api/test', + } as UndiciRequest; + + addTracePropagationHeadersToFetchRequest(request, new LRUMap(100)); + + expect(request.headers).toBe( + 'sentry-trace: trace_id_1-span_id_1-1\r\n' + + 'baggage: sentry-trace_id=trace_id_1,sentry-sampled=true,sentry-environment=staging\r\n', + ); + }); + + describe('when sentry-trace is already set', () => { + it("preserves original sentry-trace header doesn't add baggage", () => { + const request = { + headers: 'sentry-trace: trace_id_2-span_id_2-1\r\n', + origin: 'https://some-service.com', + path: '/api/test', + } as UndiciRequest; + + addTracePropagationHeadersToFetchRequest(request, new LRUMap(100)); + + expect(request.headers).toBe('sentry-trace: trace_id_2-span_id_2-1\r\n'); + }); + + it('preserves the original baggage header', () => { + const request = { + headers: + 'sentry-trace: trace_id_2-span_id_2-1\r\n' + + 'baggage: sentry-trace_id=trace_id_2,sentry-sampled=true,sentry-environment=staging\r\n', + origin: 'https://some-service.com', + path: '/api/test', + } as UndiciRequest; + + addTracePropagationHeadersToFetchRequest(request, new LRUMap(100)); + + expect(request.headers).toBe( + 'sentry-trace: trace_id_2-span_id_2-1\r\n' + + 'baggage: sentry-trace_id=trace_id_2,sentry-sampled=true,sentry-environment=staging\r\n', + ); + }); + }); + + describe('pre-existing header deduplication', () => { + it('deduplicates sentry-trace and baggage headers', () => { + const request = { + headers: + 'sentry-trace: user-trace_id-xyz-1\r\n' + + 'baggage: sentry-trace_id=user-trace_id,sentry-sampled=true,sentry-environment=user\r\n' + + 'sentry-trace: undici-trace_id-abc-1\r\n' + + 'baggage: sentry-trace_id=undici-trace_id-abc-1,sentry-sampled=true,sentry-environment=undici\r\n', + origin: 'https://some-service.com', + path: '/api/test', + } as UndiciRequest; + + addTracePropagationHeadersToFetchRequest(request, new LRUMap(100)); + + expect(request.headers).toBe( + 'sentry-trace: user-trace_id-xyz-1\r\n' + + 'baggage: sentry-trace_id=user-trace_id,sentry-sampled=true,sentry-environment=user\r\n', + ); + }); + + it("doesn't crash with incomplete headers string", () => { + const request = { + headers: + 'sentry-trace: user-trace_id-xyz-1\r\n' + + 'baggage: sentry-trace_id=user-trace_id,sentry-sampled=true,sentry-environment=user\r\n' + + 'sentry-trace: undici-trace_id-abc-1\r\n' + + 'baggage: sentry-trace_id=undici-trace_id-abc-1,sentry-sampled=true,sentry-environment=undici\r\n' + + 'baggage: \r\n', + origin: 'https://some-service.com', + path: '/api/test', + } as UndiciRequest; + + addTracePropagationHeadersToFetchRequest(request, new LRUMap(100)); + + expect(request.headers).toBe( + 'sentry-trace: user-trace_id-xyz-1\r\n' + + 'baggage: sentry-trace_id=user-trace_id,sentry-sampled=true,sentry-environment=user\r\n', + ); + }); + }); + }); +}); diff --git a/packages/node/src/integrations/node-fetch.ts b/packages/node/src/integrations/node-fetch.ts index 74bfff2dab47..fe96eea3450e 100644 --- a/packages/node/src/integrations/node-fetch.ts +++ b/packages/node/src/integrations/node-fetch.ts @@ -83,7 +83,7 @@ const _nativeNodeFetchIntegration = ((options: NodeFetchOptions = {}) => { // This is the Sentry-specific instrumentation that creates breadcrumbs & propagates traces // This must be registered after the OTEL one, to ensure that the core trace propagation logic takes presedence // Otherwise, the sentry-trace header may be set multiple times - instrumentSentryNodeFetch(options); + instrumentSentryNodeFetch({ ...options }); }, }; }) satisfies IntegrationFn; From a0b37a48be360dc0eee5ba06f24324bf9f76b4a1 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 24 Mar 2026 19:14:00 +0100 Subject: [PATCH 05/19] lint --- .../suites/tracing/double-baggage-no-spans/test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/dev-packages/node-integration-tests/suites/tracing/double-baggage-no-spans/test.ts b/dev-packages/node-integration-tests/suites/tracing/double-baggage-no-spans/test.ts index 6ca5e474e342..131b155866fa 100644 --- a/dev-packages/node-integration-tests/suites/tracing/double-baggage-no-spans/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/double-baggage-no-spans/test.ts @@ -36,7 +36,6 @@ describe('double baggage prevention', () => { .get('/api/v0', headers => { // fetch with manual getTraceData() headers — core reproduction case expectNoDuplicateSentryBaggageKeys(headers['baggage']); - console.log('xx sentry-trace', headers['sentry-trace']); expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^[a-f\d]{32}-[a-f\d]{16}$/)); expectConsistentTraceId(headers); }) From e7eb93320af63c69b0aceb95d8460e8bdbdf8fa2 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 25 Mar 2026 23:21:25 +0100 Subject: [PATCH 06/19] harden tests, fix http integration and propagator --- .../test.ts | 10 +- .../double-baggage-no-spans/scenario.mjs | 19 +++- .../tracing/double-baggage-no-spans/test.ts | 26 +++-- .../instrument.mjs | 1 + .../scenario.mjs | 19 +++- .../double-baggage-spans-no-parent/test.ts | 27 ++++-- .../double-baggage-spans-parent/scenario.mjs | 24 ++++- .../double-baggage-spans-parent/test.ts | 94 +++++++++++++------ .../src/utils/outgoingHttpRequest.ts | 28 +++++- packages/opentelemetry/src/propagator.ts | 20 +++- 10 files changed, 199 insertions(+), 69 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-other-vendors-with-sentry-entries/test.ts b/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-other-vendors-with-sentry-entries/test.ts index dbdd69ffb45b..6dbbc3d694fc 100644 --- a/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-other-vendors-with-sentry-entries/test.ts +++ b/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-other-vendors-with-sentry-entries/test.ts @@ -1,6 +1,7 @@ import { afterAll, expect, test } from 'vitest'; import { cleanupChildProcesses, createRunner } from '../../../../utils/runner'; import type { TestAPIResponse } from '../server'; +import { extractTraceparentData } from '@sentry/core'; afterAll(() => { cleanupChildProcesses(); @@ -12,7 +13,7 @@ test('should ignore sentry-values in `baggage` header of a third party vendor an const response = await runner.makeRequest('get', '/test/express', { headers: { 'sentry-trace': '12312012123120121231201212312012-1121201211212012-1', - baggage: 'sentry-release=2.1.0,sentry-environment=myEnv', + baggage: 'sentry-release=2.1.0,sentry-environment=myEnv,sentry-sample_rate=0.54', }, }); @@ -46,6 +47,11 @@ test('should ignore sentry-values in `baggage` header of a third party vendor an expect(response).toBeDefined(); const baggage = response?.test_data.baggage?.split(',').sort(); + const sentryTraceHeader = response?.test_data['sentry-trace']; + + const sentryTrace = extractTraceparentData(sentryTraceHeader); + + expect(sentryTrace?.traceId).toMatch(/^[0-9a-f]{32}$/); expect(response).toMatchObject({ test_data: { @@ -63,7 +69,7 @@ test('should ignore sentry-values in `baggage` header of a third party vendor an expect.stringMatching(/sentry-sample_rand=\d+/), 'sentry-sample_rate=1', 'sentry-sampled=true', - expect.stringMatching(/sentry-trace_id=[\da-f]{32}/), + `sentry-trace_id=${sentryTrace?.traceId}`, 'sentry-transaction=GET%20%2Ftest%2Fexpress', 'third=party', ]); diff --git a/dev-packages/node-integration-tests/suites/tracing/double-baggage-no-spans/scenario.mjs b/dev-packages/node-integration-tests/suites/tracing/double-baggage-no-spans/scenario.mjs index fd3bd04efd9e..046e980c5fe2 100644 --- a/dev-packages/node-integration-tests/suites/tracing/double-baggage-no-spans/scenario.mjs +++ b/dev-packages/node-integration-tests/suites/tracing/double-baggage-no-spans/scenario.mjs @@ -2,24 +2,33 @@ import * as Sentry from '@sentry/node'; import http from 'http'; async function run() { + const traceData = Sentry.getTraceData(); // fetch with manual getTraceData() headers - the core reproduction case from #19158 - await fetch(`${process.env.SERVER_URL}/api/v0`, { - headers: { ...Sentry.getTraceData() }, + await fetch(`${process.env.SERVER_URL}/api/fetch-custom-headers`, { + headers: { + ...traceData, + 'x-tracedata-sentry-trace': traceData['sentry-trace'], + 'x-tracedata-baggage': traceData.baggage, + }, }).then(res => res.text()); // fetch without manual headers (baseline - auto-instrumentation only) - await fetch(`${process.env.SERVER_URL}/api/v1`).then(res => res.text()); + await fetch(`${process.env.SERVER_URL}/api/fetch`).then(res => res.text()); // http.request with manual getTraceData() headers await new Promise((resolve, reject) => { - const url = new URL(`${process.env.SERVER_URL}/api/v2`); + const url = new URL(`${process.env.SERVER_URL}/api/http-custom-headers`); const req = http.request( { hostname: url.hostname, port: url.port, path: url.pathname, method: 'GET', - headers: Sentry.getTraceData(), + headers: { + ...traceData, + 'x-tracedata-sentry-trace': traceData['sentry-trace'], + 'x-tracedata-baggage': traceData.baggage, + }, }, res => { res.on('data', () => {}); diff --git a/dev-packages/node-integration-tests/suites/tracing/double-baggage-no-spans/test.ts b/dev-packages/node-integration-tests/suites/tracing/double-baggage-no-spans/test.ts index 131b155866fa..278f2ef9c135 100644 --- a/dev-packages/node-integration-tests/suites/tracing/double-baggage-no-spans/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/double-baggage-no-spans/test.ts @@ -29,27 +29,39 @@ function expectConsistentTraceId(headers: Record): void { + const xSentryTrace = extractTraceparentData(headers['x-tracedata-sentry-trace'] as string); + const sentryTrace = extractTraceparentData(headers['sentry-trace'] as string); + expect(xSentryTrace?.traceId).toBe(sentryTrace?.traceId); + + const xBaggage = parseBaggageHeader(headers['x-tracedata-baggage']); + const baggage = parseBaggageHeader(headers['baggage']); + expect(xBaggage).toEqual(baggage); +} + describe('double baggage prevention', () => { createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { test('fetch with manual getTraceData() does not duplicate sentry baggage entries', async () => { const [SERVER_URL, closeTestServer] = await createTestServer() - .get('/api/v0', headers => { - // fetch with manual getTraceData() headers — core reproduction case - expectNoDuplicateSentryBaggageKeys(headers['baggage']); + .get('/api/fetch-custom-headers', headers => { + // fetch with manual getTraceData() headers expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^[a-f\d]{32}-[a-f\d]{16}$/)); + expectNoDuplicateSentryBaggageKeys(headers['baggage']); expectConsistentTraceId(headers); + expectUserSetTraceId(headers); }) - .get('/api/v1', headers => { + .get('/api/fetch', headers => { // fetch without manual headers (baseline) - expectNoDuplicateSentryBaggageKeys(headers['baggage']); expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^[a-f\d]{32}-[a-f\d]{16}$/)); + expectNoDuplicateSentryBaggageKeys(headers['baggage']); expectConsistentTraceId(headers); }) - .get('/api/v2', headers => { + .get('/api/http-custom-headers', headers => { // http.request with manual getTraceData() headers - expectNoDuplicateSentryBaggageKeys(headers['baggage']); expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^[a-f\d]{32}-[a-f\d]{16}$/)); + expectNoDuplicateSentryBaggageKeys(headers['baggage']); expectConsistentTraceId(headers); + expectUserSetTraceId(headers); }) .start(); diff --git a/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-no-parent/instrument.mjs b/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-no-parent/instrument.mjs index 46a27dd03b74..e21f0a36751d 100644 --- a/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-no-parent/instrument.mjs +++ b/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-no-parent/instrument.mjs @@ -6,4 +6,5 @@ Sentry.init({ release: '1.0', tracesSampleRate: 1.0, transport: loggingTransport, + debug: true, }); diff --git a/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-no-parent/scenario.mjs b/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-no-parent/scenario.mjs index fd3bd04efd9e..dd5841685463 100644 --- a/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-no-parent/scenario.mjs +++ b/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-no-parent/scenario.mjs @@ -2,24 +2,33 @@ import * as Sentry from '@sentry/node'; import http from 'http'; async function run() { + const traceData = Sentry.getTraceData(); // fetch with manual getTraceData() headers - the core reproduction case from #19158 - await fetch(`${process.env.SERVER_URL}/api/v0`, { - headers: { ...Sentry.getTraceData() }, + await fetch(`${process.env.SERVER_URL}/api/fetch-custom-headers`, { + headers: { + ...traceData, + 'x-tracedata-sentry-trace': traceData['sentry-trace'], + 'x-tracedata-baggage': traceData.baggage, + }, }).then(res => res.text()); // fetch without manual headers (baseline - auto-instrumentation only) - await fetch(`${process.env.SERVER_URL}/api/v1`).then(res => res.text()); + await fetch(`${process.env.SERVER_URL}/api/fetch`, {}).then(res => res.text()); // http.request with manual getTraceData() headers await new Promise((resolve, reject) => { - const url = new URL(`${process.env.SERVER_URL}/api/v2`); + const url = new URL(`${process.env.SERVER_URL}/api/http-custom-headers`); const req = http.request( { hostname: url.hostname, port: url.port, path: url.pathname, method: 'GET', - headers: Sentry.getTraceData(), + headers: { + ...traceData, + 'x-tracedata-sentry-trace': traceData['sentry-trace'], + 'x-tracedata-baggage': traceData.baggage, + }, }, res => { res.on('data', () => {}); diff --git a/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-no-parent/test.ts b/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-no-parent/test.ts index 90f175963fa4..3e771221729f 100644 --- a/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-no-parent/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-no-parent/test.ts @@ -27,33 +27,44 @@ function expectConsistentTraceId(headers: Record): void { + const xSentryTrace = extractTraceparentData(headers['x-tracedata-sentry-trace'] as string); + const sentryTrace = extractTraceparentData(headers['sentry-trace'] as string); + expect(xSentryTrace?.traceId).toBe(sentryTrace?.traceId); + + const xBaggage = parseBaggageHeader(headers['x-tracedata-baggage']); + const baggage = parseBaggageHeader(headers['baggage']); + expect(xBaggage).toEqual(baggage); +} + describe('double baggage prevention', () => { createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { test('fetch with manual getTraceData() does not duplicate sentry baggage entries', async () => { const [SERVER_URL, closeTestServer] = await createTestServer() - .get('/api/v0', headers => { - // fetch with manual getTraceData() headers — core reproduction case - expectNoDuplicateSentryBaggageKeys(headers['baggage']); + .get('/api/fetch-custom-headers', headers => { + // fetch with manual getTraceData() headers expect(headers['sentry-trace']).not.toContain(','); + expectNoDuplicateSentryBaggageKeys(headers['baggage']); expectConsistentTraceId(headers); + expectUserSetTraceId(headers); }) - .get('/api/v1', headers => { + .get('/api/fetch', headers => { // fetch without manual headers (baseline) - expectNoDuplicateSentryBaggageKeys(headers['baggage']); expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^[a-f\d]{32}-[a-f\d]{16}(-[01])?$/)); + expectNoDuplicateSentryBaggageKeys(headers['baggage']); expectConsistentTraceId(headers); }) - .get('/api/v2', headers => { + .get('/api/http-custom-headers', headers => { // http.request with manual getTraceData() headers - expectNoDuplicateSentryBaggageKeys(headers['baggage']); expect(headers['sentry-trace']).not.toContain(','); + expectNoDuplicateSentryBaggageKeys(headers['baggage']); expectConsistentTraceId(headers); + expectUserSetTraceId(headers); }) .start(); await createRunner() .withEnv({ SERVER_URL }) - // .ignore('transaction') .expect({ event: { exception: { diff --git a/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-parent/scenario.mjs b/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-parent/scenario.mjs index a9cb526cae3d..a577bad62333 100644 --- a/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-parent/scenario.mjs +++ b/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-parent/scenario.mjs @@ -2,24 +2,38 @@ import * as Sentry from '@sentry/node'; import http from 'http'; async function run() { + const traceData = Sentry.getTraceData(); // fetch with manual getTraceData() headers - the core reproduction case from #19158 - await fetch(`${process.env.SERVER_URL}/api/v0`, { - headers: { ...Sentry.getTraceData() }, + await fetch(`${process.env.SERVER_URL}/api/fetch-custom-headers`, { + headers: { + ...traceData, + 'x-tracedata-sentry-trace': traceData['sentry-trace'], + 'x-tracedata-baggage': traceData.baggage, + }, }).then(res => res.text()); // fetch without manual headers (baseline - auto-instrumentation only) - await fetch(`${process.env.SERVER_URL}/api/v1`).then(res => res.text()); + await fetch(`${process.env.SERVER_URL}/api/fetch`, { + headers: { + 'x-tracedata-sentry-trace': traceData['sentry-trace'], + 'x-tracedata-baggage': traceData.baggage, + }, + }).then(res => res.text()); // http.request with manual getTraceData() headers await new Promise((resolve, reject) => { - const url = new URL(`${process.env.SERVER_URL}/api/v2`); + const url = new URL(`${process.env.SERVER_URL}/api/http-custom-headers`); const req = http.request( { hostname: url.hostname, port: url.port, path: url.pathname, method: 'GET', - headers: Sentry.getTraceData(), + headers: { + ...traceData, + 'x-tracedata-sentry-trace': traceData['sentry-trace'], + 'x-tracedata-baggage': traceData.baggage, + }, }, res => { res.on('data', () => {}); diff --git a/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-parent/test.ts b/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-parent/test.ts index cc077e8c3ffe..19692cbea0a0 100644 --- a/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-parent/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-parent/test.ts @@ -28,27 +28,51 @@ function expectConsistentTraceId(headers: Record): void { + const xSentryTrace = extractTraceparentData(headers['x-tracedata-sentry-trace'] as string); + const sentryTrace = extractTraceparentData(headers['sentry-trace'] as string); + expect(xSentryTrace?.traceId).toBe(sentryTrace?.traceId); + + const xBaggage = parseBaggageHeader(headers['x-tracedata-baggage']); + const baggage = parseBaggageHeader(headers['baggage']); + expect(xBaggage).toEqual(baggage); +} + describe('double baggage prevention - http.client spans with parent span', () => { createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { + let transactionTraceId = '000'; + let fetchSpanId = '000'; + let httpCustomHeadersSpanId = '000'; + let fetchCustomHeadersSpanId = '000'; + test('fetch with manual getTraceData() does not duplicate sentry baggage entries', async () => { const [SERVER_URL, closeTestServer] = await createTestServer() - .get('/api/v0', headers => { + .get('/api/fetch-custom-headers', headers => { // fetch with manual getTraceData() headers — core reproduction case expectNoDuplicateSentryBaggageKeys(headers['baggage']); expect(headers['sentry-trace']).not.toContain(','); expectConsistentTraceId(headers); + expectUserSetTraceId(headers); + const sentryTrace = extractTraceparentData(headers['sentry-trace'] as string); + transactionTraceId = sentryTrace!.traceId!; + fetchCustomHeadersSpanId = sentryTrace!.parentSpanId!; }) - .get('/api/v1', headers => { + .get('/api/fetch', headers => { // fetch without manual headers (baseline) expectNoDuplicateSentryBaggageKeys(headers['baggage']); expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^[a-f\d]{32}-[a-f\d]{16}(-[01])?$/)); expectConsistentTraceId(headers); + const sentryTrace = extractTraceparentData(headers['sentry-trace'] as string); + fetchSpanId = sentryTrace!.parentSpanId!; }) - .get('/api/v2', headers => { + .get('/api/http-custom-headers', headers => { // http.request with manual getTraceData() headers expectNoDuplicateSentryBaggageKeys(headers['baggage']); expect(headers['sentry-trace']).not.toContain(','); expectConsistentTraceId(headers); + expectUserSetTraceId(headers); + const sentryTrace = extractTraceparentData(headers['sentry-trace'] as string); + httpCustomHeadersSpanId = sentryTrace!.parentSpanId!; }) .start(); @@ -56,34 +80,42 @@ describe('double baggage prevention - http.client spans with parent span', () => .withEnv({ SERVER_URL }) .ignore('event') .expect({ - transaction: { - transaction: 'parent_span', - spans: [ - { - op: 'http.client', - description: expect.stringMatching(/^GET .*\/api\/v0$/), - data: {}, - span_id: expect.stringMatching(/[a-f0-9]{16}/), - start_timestamp: expect.any(Number), - trace_id: expect.stringMatching(/[a-f0-9]{32}/), - }, - { - op: 'http.client', - description: expect.stringMatching(/^GET .*\/api\/v1$/), - data: {}, - span_id: expect.stringMatching(/[a-f0-9]{16}/), - start_timestamp: expect.any(Number), - trace_id: expect.stringMatching(/[a-f0-9]{32}/), - }, - { - op: 'http.client', - description: expect.stringMatching(/^GET .*\/api\/v2$/), - data: {}, - span_id: expect.stringMatching(/[a-f0-9]{16}/), - start_timestamp: expect.any(Number), - trace_id: expect.stringMatching(/[a-f0-9]{32}/), - }, - ], + transaction: txn => { + expect(transactionTraceId).toMatch(/^[a-f0-9]{32}$/); + + expect(txn).toMatchObject({ + transaction: 'parent_span', + spans: [ + { + op: 'http.client', + description: expect.stringMatching(/^GET .*\/api\/fetch-custom-headers$/), + data: {}, + // span id is expected to be different since users call getTraceData() before the + // http.client span is created + span_id: expect.not.stringContaining(fetchCustomHeadersSpanId), + start_timestamp: expect.any(Number), + trace_id: transactionTraceId, + }, + { + op: 'http.client', + description: expect.stringMatching(/^GET .*\/api\/fetch$/), + data: {}, + span_id: fetchSpanId, + start_timestamp: expect.any(Number), + trace_id: transactionTraceId, + }, + { + op: 'http.client', + description: expect.stringMatching(/^GET .*\/api\/http-custom-headers$/), + data: {}, + // span id is expected to be different since users call getTraceData() before the + // http.client span is created + span_id: expect.not.stringContaining(httpCustomHeadersSpanId), + start_timestamp: expect.any(Number), + trace_id: transactionTraceId, + }, + ], + }); }, }) .start() diff --git a/packages/node-core/src/utils/outgoingHttpRequest.ts b/packages/node-core/src/utils/outgoingHttpRequest.ts index bd3ba9b04e5f..f70f4bb23473 100644 --- a/packages/node-core/src/utils/outgoingHttpRequest.ts +++ b/packages/node-core/src/utils/outgoingHttpRequest.ts @@ -2,12 +2,16 @@ import type { LRUMap, SanitizedRequestData } from '@sentry/core'; import { addBreadcrumb, debug, + dynamicSamplingContextToSentryBaggageHeader, getBreadcrumbLogLevelFromHttpStatusCode, getClient, getSanitizedUrlString, getTraceData, isError, + objectToBaggageHeader, + parseBaggageHeader, parseUrl, + SENTRY_BAGGAGE_KEY_PREFIX, shouldPropagateTraceForUrl, } from '@sentry/core'; import type { ClientRequest, IncomingMessage, RequestOptions } from 'http'; @@ -63,7 +67,9 @@ export function addTracePropagationHeadersToOutgoingRequest( const { 'sentry-trace': sentryTrace, baggage, traceparent } = headersToAdd; - if (sentryTrace && !request.getHeader('sentry-trace')) { + const hasExistingSentryTraceHeader = request.getHeader('sentry-trace'); + + if (sentryTrace && !hasExistingSentryTraceHeader) { try { request.setHeader('sentry-trace', sentryTrace); DEBUG_BUILD && debug.log(LOG_PREFIX, 'Added sentry-trace header to outgoing request'); @@ -77,7 +83,7 @@ export function addTracePropagationHeadersToOutgoingRequest( } } - if (traceparent && !request.getHeader('traceparent')) { + if (traceparent && !hasExistingSentryTraceHeader && !request.getHeader('traceparent')) { try { request.setHeader('traceparent', traceparent); DEBUG_BUILD && debug.log(LOG_PREFIX, 'Added traceparent header to outgoing request'); @@ -91,10 +97,24 @@ export function addTracePropagationHeadersToOutgoingRequest( } } - if (baggage) { + if (baggage && !hasExistingSentryTraceHeader) { const existingBaggage = request.getHeader('baggage'); - const newBaggage = mergeBaggageHeaders(existingBaggage, baggage); + let cleanedExistingBaggage = existingBaggage; + + // In the edge case that a baggage header with sentry- keys was added + // BUT NO sentry-trace header, we overwrite the sentry- keys in the header we attach. + // Therefore, we clean the existing baggage header of all sentry- keys. + if (existingBaggage) { + const tmpBaggage = parseBaggageHeader(existingBaggage); + const baggageWithoutSentry = tmpBaggage + ? Object.fromEntries(Object.entries(tmpBaggage).filter(([key]) => !key.startsWith(SENTRY_BAGGAGE_KEY_PREFIX))) + : {}; + cleanedExistingBaggage = objectToBaggageHeader(baggageWithoutSentry); + } + + // If a sentry-trace header was already added, we don't add our baggage at all. + const newBaggage = mergeBaggageHeaders(cleanedExistingBaggage, baggage); if (newBaggage) { try { request.setHeader('baggage', newBaggage); diff --git a/packages/opentelemetry/src/propagator.ts b/packages/opentelemetry/src/propagator.ts index 2b5873658706..34392b38a64c 100644 --- a/packages/opentelemetry/src/propagator.ts +++ b/packages/opentelemetry/src/propagator.ts @@ -63,6 +63,11 @@ export class SentryPropagator extends W3CBaggagePropagator { } const existingBaggageHeader = getExistingBaggage(carrier); + const existingSentryTraceHeader = getExistingSentryTrace(carrier); + + console.log('xx existingSentryTraceHeader', existingSentryTraceHeader); + console.log('xx existingBaggageHeader', existingBaggageHeader); + let baggage = propagation.getBaggage(context) || propagation.createBaggage({}); const { dynamicSamplingContext, traceId, spanId, sampled } = getInjectionData(context); @@ -72,12 +77,15 @@ export class SentryPropagator extends W3CBaggagePropagator { if (baggageEntries) { Object.entries(baggageEntries).forEach(([key, value]) => { + if (!existingSentryTraceHeader && key.startsWith(SENTRY_BAGGAGE_KEY_PREFIX)) { + return; + } baggage = baggage.setEntry(key, { value }); }); } } - if (dynamicSamplingContext) { + if (!existingSentryTraceHeader && dynamicSamplingContext) { baggage = Object.entries(dynamicSamplingContext).reduce((b, [dscKey, dscValue]) => { if (dscValue) { return b.setEntry(`${SENTRY_BAGGAGE_KEY_PREFIX}${dscKey}`, { value: dscValue }); @@ -87,7 +95,7 @@ export class SentryPropagator extends W3CBaggagePropagator { } // We also want to avoid setting the default OTEL trace ID, if we get that for whatever reason - if (traceId && traceId !== INVALID_TRACEID) { + if (!existingSentryTraceHeader && traceId && traceId !== INVALID_TRACEID) { setter.set(carrier, SENTRY_TRACE_HEADER, generateSentryTraceHeader(traceId, spanId, sampled)); if (propagateTraceparent) { @@ -248,6 +256,14 @@ function getExistingBaggage(carrier: unknown): string | undefined { } } +function getExistingSentryTrace(carrier: unknown): string | string[] | undefined { + try { + return (carrier as Record)[SENTRY_TRACE_HEADER]; + } catch { + return undefined; + } +} + /** * It is pretty tricky to get access to the outgoing request URL of a request in the propagator. * As we only have access to the context of the span to be sent and the carrier (=headers), From 7fb7814054b7be0dadb8d2fb63bd73f7b5688a98 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 25 Mar 2026 23:26:13 +0100 Subject: [PATCH 07/19] remove console logs --- packages/opentelemetry/src/propagator.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/opentelemetry/src/propagator.ts b/packages/opentelemetry/src/propagator.ts index 34392b38a64c..18db1e2404c9 100644 --- a/packages/opentelemetry/src/propagator.ts +++ b/packages/opentelemetry/src/propagator.ts @@ -65,9 +65,6 @@ export class SentryPropagator extends W3CBaggagePropagator { const existingBaggageHeader = getExistingBaggage(carrier); const existingSentryTraceHeader = getExistingSentryTrace(carrier); - console.log('xx existingSentryTraceHeader', existingSentryTraceHeader); - console.log('xx existingBaggageHeader', existingBaggageHeader); - let baggage = propagation.getBaggage(context) || propagation.createBaggage({}); const { dynamicSamplingContext, traceId, spanId, sampled } = getInjectionData(context); From 429dad110f52c0dc6726cad5b3e2c99cd7720109 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 26 Mar 2026 14:58:41 +0100 Subject: [PATCH 08/19] adjust propagator tests --- .../opentelemetry/test/propagator.test.ts | 90 ++++++++++++++++++- 1 file changed, 87 insertions(+), 3 deletions(-) diff --git a/packages/opentelemetry/test/propagator.test.ts b/packages/opentelemetry/test/propagator.test.ts index ef8223b5974a..7610f040a72d 100644 --- a/packages/opentelemetry/test/propagator.test.ts +++ b/packages/opentelemetry/test/propagator.test.ts @@ -500,14 +500,17 @@ describe('SentryPropagator', () => { ); }); - it('should overwrite existing sentry baggage header', () => { + it('overwrites existing sentry baggage values and add sentry-trace header if sentry-trace is not set yet', () => { + // This is an edeg case where someone set a baggage header with existing sentry- values but no sentry-trace header. + // There's no evidence this occurs in real-life but if it does, we can assume that this must be some kind of error + // Hence, we overwrite the existing sentry- values with our new ones but keep all other non-sentry values. const spanContext = { traceId: 'd4cda95b652f4a1592b449d5929fda1b', spanId: '6e0c63257de34c92', traceFlags: TraceFlags.SAMPLED, }; - const carrier = { + const carrier: Record = { baggage: 'foo=bar,other=yes,sentry-release=9.9.9,sentry-other=yes', }; const context = trace.setSpanContext(ROOT_CONTEXT, spanContext); @@ -520,11 +523,11 @@ describe('SentryPropagator', () => { 'sentry-trace_id=d4cda95b652f4a1592b449d5929fda1b', 'sentry-public_key=abc', 'sentry-environment=production', - 'sentry-other=yes', 'sentry-release=1.0.0', 'sentry-sampled=true', ].sort(), ); + expect(carrier[SENTRY_TRACE_HEADER]).toBe('d4cda95b652f4a1592b449d5929fda1b-6e0c63257de34c92-1'); }); it('should create baggage without propagation context', () => { @@ -537,6 +540,7 @@ describe('SentryPropagator', () => { expect(carrier[SENTRY_BAGGAGE_HEADER]).toBe( `foo=bar,sentry-environment=production,sentry-release=1.0.0,sentry-public_key=abc,sentry-trace_id=${traceId}`, ); + expect(carrier[SENTRY_TRACE_HEADER]).toBeDefined(); // whenever we set baggage, we must also set sentry-trace }); it('should NOT set baggage and sentry-trace header if instrumentation is suppressed', () => { @@ -551,6 +555,86 @@ describe('SentryPropagator', () => { expect(carrier[SENTRY_TRACE_HEADER]).toBe(undefined); expect(carrier[SENTRY_BAGGAGE_HEADER]).toBe(undefined); }); + + it("doesn't set baggage header if sentry-trace header is already set", () => { + const carrier: Record = { + [SENTRY_TRACE_HEADER]: 'abcdef-xyz-1', + }; + propagator.inject(ROOT_CONTEXT, carrier, defaultTextMapSetter); + + expect(carrier[SENTRY_BAGGAGE_HEADER]).toBe(undefined); + expect(carrier[SENTRY_TRACE_HEADER]).toBe('abcdef-xyz-1'); + }); + + describe('traceparent header', () => { + it("doesn't change baggage header if sentry-trace header is already set", () => { + const carrier: Record = { + [SENTRY_TRACE_HEADER]: 'abcdef-xyz-1', + [SENTRY_BAGGAGE_HEADER]: 'foo=bar,other=yes,sentry-release=9.9.9', + }; + propagator.inject(ROOT_CONTEXT, carrier, defaultTextMapSetter); + + expect(carrier[SENTRY_BAGGAGE_HEADER]).toBe('foo=bar,other=yes,sentry-release=9.9.9'); + expect(carrier[SENTRY_TRACE_HEADER]).toBe('abcdef-xyz-1'); + }); + + it("doesn't set traceparent header if sentry-trace header is already set", () => { + mockSdkInit({ propagateTraceparent: true }); + const carrier: Record = { + [SENTRY_TRACE_HEADER]: 'abcdef-xyz-1', + }; + propagator.inject(ROOT_CONTEXT, carrier, defaultTextMapSetter); + + expect(carrier['traceparent']).toBe(undefined); + expect(carrier[SENTRY_TRACE_HEADER]).toBe('abcdef-xyz-1'); + }); + + it('sets traceparent header if propagateTraceparent is true', () => { + mockSdkInit({ + environment: 'production', + release: '1.0.0', + tracesSampleRate: 1, + dsn: 'https://abc@domain/123', + propagateTraceparent: true, + }); + + const spanContext = { + traceId: 'd4cda95b652f4a1592b449d5929fda1b', + spanId: '6e0c63257de34c92', + traceFlags: TraceFlags.SAMPLED, + }; + const context = trace.setSpanContext(ROOT_CONTEXT, spanContext); + const baggage = propagation.createBaggage({ foo: { value: 'bar' } }); + propagator.inject(propagation.setBaggage(context, baggage), carrier, defaultTextMapSetter); + + expect(baggageToArray(carrier[SENTRY_BAGGAGE_HEADER])).toEqual( + [ + 'foo=bar', + 'sentry-trace_id=d4cda95b652f4a1592b449d5929fda1b', + 'sentry-public_key=abc', + 'sentry-environment=production', + 'sentry-release=1.0.0', + 'sentry-sampled=true', + ].sort(), + ); + expect(carrier['traceparent']).toBe('00-d4cda95b652f4a1592b449d5929fda1b-6e0c63257de34c92-01'); + }); + + it("doesn't set traceparent header if propagateTraceparent is false", () => { + mockSdkInit({ + environment: 'production', + release: '1.0.0', + tracesSampleRate: 1, + dsn: 'https://abc@domain/123', + propagateTraceparent: false, + }); + const carrier: Record = {}; + propagator.inject(ROOT_CONTEXT, carrier, defaultTextMapSetter); + + expect(carrier['traceparent']).toBe(undefined); + expect(carrier[SENTRY_TRACE_HEADER]).toBeDefined(); + }); + }); }); describe('extract', () => { From 9b0b7f9390bc543e20a5a941538568bbbba19d88 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 26 Mar 2026 15:56:07 +0100 Subject: [PATCH 09/19] convert undici node 18 string headers to array to dedupe and then back to string --- .../tracing/double-baggage-no-spans/test.ts | 1 + .../double-baggage-spans-parent/test.ts | 6 +-- .../src/utils/outgoingFetchRequest.ts | 49 +++++++++---------- .../test/utils/outgoingFetchRequest.test.ts | 21 ++++++++ 4 files changed, 48 insertions(+), 29 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/tracing/double-baggage-no-spans/test.ts b/dev-packages/node-integration-tests/suites/tracing/double-baggage-no-spans/test.ts index 278f2ef9c135..8c51f1213185 100644 --- a/dev-packages/node-integration-tests/suites/tracing/double-baggage-no-spans/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/double-baggage-no-spans/test.ts @@ -45,6 +45,7 @@ describe('double baggage prevention', () => { const [SERVER_URL, closeTestServer] = await createTestServer() .get('/api/fetch-custom-headers', headers => { // fetch with manual getTraceData() headers + // console.log('xx sentry-tace', headers['sentry-trace']); expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^[a-f\d]{32}-[a-f\d]{16}$/)); expectNoDuplicateSentryBaggageKeys(headers['baggage']); expectConsistentTraceId(headers); diff --git a/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-parent/test.ts b/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-parent/test.ts index 19692cbea0a0..d11cee283cd7 100644 --- a/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-parent/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-parent/test.ts @@ -49,13 +49,13 @@ describe('double baggage prevention - http.client spans with parent span', () => const [SERVER_URL, closeTestServer] = await createTestServer() .get('/api/fetch-custom-headers', headers => { // fetch with manual getTraceData() headers — core reproduction case + const sentryTrace = extractTraceparentData(headers['sentry-trace'] as string); + transactionTraceId = sentryTrace!.traceId!; + fetchCustomHeadersSpanId = sentryTrace!.parentSpanId!; expectNoDuplicateSentryBaggageKeys(headers['baggage']); expect(headers['sentry-trace']).not.toContain(','); expectConsistentTraceId(headers); expectUserSetTraceId(headers); - const sentryTrace = extractTraceparentData(headers['sentry-trace'] as string); - transactionTraceId = sentryTrace!.traceId!; - fetchCustomHeadersSpanId = sentryTrace!.parentSpanId!; }) .get('/api/fetch', headers => { // fetch without manual headers (baseline) diff --git a/packages/node-core/src/utils/outgoingFetchRequest.ts b/packages/node-core/src/utils/outgoingFetchRequest.ts index 9c1de10759c9..e4caca1c6cd6 100644 --- a/packages/node-core/src/utils/outgoingFetchRequest.ts +++ b/packages/node-core/src/utils/outgoingFetchRequest.ts @@ -121,7 +121,29 @@ function _deduplicateHeaders(request: UndiciRequest): void { if (Array.isArray(request.headers)) { _deduplicateArrayHeaders(request.headers); } else { - request.headers = _deduplicateStringHeaders(request.headers); + const headersArray = request.headers.split('\r\n'); + const headers: string[] = []; + for (const header of headersArray) { + try { + const [key, value] = header.split(':').map(part => part.trim()); + if (key != null && value != null) { + headers.push(key, value); + } + } catch { + continue; + } + } + + _deduplicateArrayHeaders(headers); + + const headerPairs: string[] = []; + for (let i = 0; i < headers.length; i += 2) { + headerPairs.push(`${headers[i]}: ${headers[i + 1]}`); + } + const concatenated = headerPairs.join('\r\n'); + if (concatenated) { + request.headers = concatenated.concat('\r\n'); + } } } @@ -161,31 +183,6 @@ function _deduplicateArrayHeader(headers: string[], headerName: string): void { } } -function _deduplicateStringHeaders(input: string): string { - // Deduplicate sentry-trace — keep only the first occurrence - let sentryTraceCount = 0; - let result = input.replace(/sentry-trace: .*\r\n/g, match => { - return ++sentryTraceCount === 1 ? match : ''; - }); - - // Deduplicate baggage — merge all occurrences into one but preserve initial sentry- values - let mergedBaggage: string | undefined; - result = result.replace(/baggage: (.*)\r\n/g, (_match, value: string) => { - if (!mergedBaggage) { - mergedBaggage = value; - } else { - mergedBaggage = mergeBaggageHeaders(value, mergedBaggage) || mergedBaggage; - } - return ''; - }); - - if (mergedBaggage) { - result += `${SENTRY_BAGGAGE_HEADER}: ${mergedBaggage}\r\n`; - } - - return result; -} - /** Add a breadcrumb for an outgoing fetch/undici request. */ export function addFetchRequestBreadcrumb(request: UndiciRequest, response: UndiciResponse): void { const data = getBreadcrumbData(request); diff --git a/packages/node-core/test/utils/outgoingFetchRequest.test.ts b/packages/node-core/test/utils/outgoingFetchRequest.test.ts index d596dd5fda1d..32f6584d8300 100644 --- a/packages/node-core/test/utils/outgoingFetchRequest.test.ts +++ b/packages/node-core/test/utils/outgoingFetchRequest.test.ts @@ -304,5 +304,26 @@ describe('addTracePropagationHeadersToFetchRequest', () => { ); }); }); + + it("doesn't dedupe nearly-sentry-tracing headers", () => { + const request = { + headers: + 'sentry-trace: user-trace_id-xyz-1\r\n' + + 'baggage: sentry-trace_id=user-trace_id,sentry-sampled=true,sentry-environment=user\r\n' + + 'x-sentry-trace: custom-trace_id-abc-1\r\n' + + 'x-baggage: sentry-trace_id=undici-trace_id-abc-1,sentry-sampled=true,sentry-environment=undici\r\n', + origin: 'https://some-service.com', + path: '/api/test', + } as UndiciRequest; + + addTracePropagationHeadersToFetchRequest(request, new LRUMap(100)); + + expect(request.headers).toBe( + 'sentry-trace: user-trace_id-xyz-1\r\n' + + 'baggage: sentry-trace_id=user-trace_id,sentry-sampled=true,sentry-environment=user\r\n' + + 'x-sentry-trace: custom-trace_id-abc-1\r\n' + + 'x-baggage: sentry-trace_id=undici-trace_id-abc-1,sentry-sampled=true,sentry-environment=undici\r\n', + ); + }); }); }); From 784e9dfd867bc88ecac20f3f0bd4a27c44b3e827 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 26 Mar 2026 17:23:22 +0100 Subject: [PATCH 10/19] always convert string headers to array headers (and back) --- .../src/utils/outgoingFetchRequest.ts | 126 ++++++++---------- 1 file changed, 52 insertions(+), 74 deletions(-) diff --git a/packages/node-core/src/utils/outgoingFetchRequest.ts b/packages/node-core/src/utils/outgoingFetchRequest.ts index e4caca1c6cd6..15871f528fe8 100644 --- a/packages/node-core/src/utils/outgoingFetchRequest.ts +++ b/packages/node-core/src/utils/outgoingFetchRequest.ts @@ -45,68 +45,74 @@ export function addTracePropagationHeadersToFetchRequest( const { 'sentry-trace': sentryTrace, baggage, traceparent } = addedHeaders; + const requestHeaders = Array.isArray(request.headers) ? request.headers : stringToArrayHeaders(request.headers); + // OTel's UndiciInstrumentation calls propagation.inject() which unconditionally // appends headers to the request. When the user also sets headers via getTraceData(), // this results in duplicate sentry-trace and baggage entries. // We clean these up before applying our own logic. - _deduplicateHeaders(request); + _deduplicateArrayHeaders(requestHeaders); // We do not want to overwrite existing headers here // If the core UndiciInstrumentation is registered, it will already have set the headers // We do not want to add any then - if (Array.isArray(request.headers)) { - const requestHeaders = request.headers; - - const hasExistingSentryTraceHeader = requestHeaders.includes(SENTRY_TRACE_HEADER); + const hasExistingSentryTraceHeader = requestHeaders.includes(SENTRY_TRACE_HEADER); - // We do not want to set any headers if we already have an existing sentry-trace header. - // This is still the source of truth, otherwise we risk mixing up baggage and sentry-trace values. - if (!hasExistingSentryTraceHeader) { - if (sentryTrace) { - requestHeaders.push(SENTRY_TRACE_HEADER, sentryTrace); - } + // We do not want to set any headers if we already have an existing sentry-trace header. + // sentry-trace is still the source of truth, otherwise we risk mixing up baggage and sentry-trace values. + if (!hasExistingSentryTraceHeader) { + if (sentryTrace) { + requestHeaders.push(SENTRY_TRACE_HEADER, sentryTrace); + } - if (traceparent && !requestHeaders.includes('traceparent')) { - requestHeaders.push('traceparent', traceparent); - } + if (traceparent && !requestHeaders.includes('traceparent')) { + requestHeaders.push('traceparent', traceparent); + } - // For baggage, we make sure to merge this into a possibly existing header - const existingBaggagePos = requestHeaders.findIndex(header => header === SENTRY_BAGGAGE_HEADER); - if (baggage && existingBaggagePos === -1) { - requestHeaders.push(SENTRY_BAGGAGE_HEADER, baggage); - } else if (baggage) { - // headers in format [key_0, value_0, key_1, value_1, ...], hence the +1 here - const existingBaggage = requestHeaders[existingBaggagePos + 1]; - const merged = mergeBaggageHeaders(existingBaggage, baggage); - if (merged) { - requestHeaders[existingBaggagePos + 1] = merged; - } + // For baggage, we make sure to merge this into a possibly existing header + const existingBaggagePos = requestHeaders.findIndex(header => header === SENTRY_BAGGAGE_HEADER); + if (baggage && existingBaggagePos === -1) { + requestHeaders.push(SENTRY_BAGGAGE_HEADER, baggage); + } else if (baggage) { + // headers in format [key_0, value_0, key_1, value_1, ...], hence the +1 here + const existingBaggage = requestHeaders[existingBaggagePos + 1]; + const merged = mergeBaggageHeaders(existingBaggage, baggage); + if (merged) { + requestHeaders[existingBaggagePos + 1] = merged; } } - } else { - // We do not want to overwrite existing header here, if it was already set - const hasExistingSentryTraceHeader = request.headers.includes(`${SENTRY_TRACE_HEADER}:`); + } - if (!hasExistingSentryTraceHeader) { - if (sentryTrace) { - request.headers += `${SENTRY_TRACE_HEADER}: ${sentryTrace}\r\n`; - } + if (!Array.isArray(request.headers)) { + // For orginal string request headers, we need to wrote them back to the request + request.headers = arrayToStringHeaders(requestHeaders); + } +} - if (traceparent && !request.headers.includes('traceparent:')) { - request.headers += `traceparent: ${traceparent}\r\n`; +function stringToArrayHeaders(requestHeaders: string): string[] { + const headersArray = requestHeaders.split('\r\n'); + const headers: string[] = []; + for (const header of headersArray) { + try { + const [key, value] = header.split(':').map(part => part.trim()); + if (key != null && value != null) { + headers.push(key, value); } + } catch {} + } + return headers; +} - const existingBaggage = request.headers.match(BAGGAGE_HEADER_REGEX)?.[1]; - if (baggage && !existingBaggage) { - request.headers += `${SENTRY_BAGGAGE_HEADER}: ${baggage}\r\n`; - } else if (baggage) { - const merged = mergeBaggageHeaders(existingBaggage, baggage); - if (merged) { - request.headers = request.headers.replace(BAGGAGE_HEADER_REGEX, `baggage: ${merged}\r\n`); - } - } - } +function arrayToStringHeaders(headers: string[]): string { + const headerPairs: string[] = []; + for (let i = 0; i < headers.length; i += 2) { + headerPairs.push(`${headers[i]}: ${headers[i + 1]}`); + } + if (!headerPairs.length) { + return ''; } + + return headerPairs.join('\r\n').concat('\r\n'); } /** @@ -115,37 +121,9 @@ export function addTracePropagationHeadersToFetchRequest( * OTel's UndiciInstrumentation unconditionally appends headers via propagation.inject(), * which can create duplicates when the user has already set these headers (e.g. via getTraceData()). * For sentry-trace, we keep the first occurrence (user-set). - * For baggage, we merge all occurrences into one to preserve both sentry and non-sentry entries. + * For baggage, we merge all occurrences into one header to preserve non-sentry entries. For Sentry + * entries, we keep the first occurance. */ -function _deduplicateHeaders(request: UndiciRequest): void { - if (Array.isArray(request.headers)) { - _deduplicateArrayHeaders(request.headers); - } else { - const headersArray = request.headers.split('\r\n'); - const headers: string[] = []; - for (const header of headersArray) { - try { - const [key, value] = header.split(':').map(part => part.trim()); - if (key != null && value != null) { - headers.push(key, value); - } - } catch { - continue; - } - } - - _deduplicateArrayHeaders(headers); - - const headerPairs: string[] = []; - for (let i = 0; i < headers.length; i += 2) { - headerPairs.push(`${headers[i]}: ${headers[i + 1]}`); - } - const concatenated = headerPairs.join('\r\n'); - if (concatenated) { - request.headers = concatenated.concat('\r\n'); - } - } -} function _deduplicateArrayHeaders(headers: string[]): void { _deduplicateArrayHeader(headers, SENTRY_TRACE_HEADER); From 978b88d0bf3ef157017252d7d6b89c8f25059b9f Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 26 Mar 2026 17:51:38 +0100 Subject: [PATCH 11/19] harden browser integration test --- .../suites/tracing/request/fetch-trace-header-merging/test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-trace-header-merging/test.ts b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-trace-header-merging/test.ts index 1385f6882dc7..dd57b0f6ad84 100644 --- a/dev-packages/browser-integration-tests/suites/tracing/request/fetch-trace-header-merging/test.ts +++ b/dev-packages/browser-integration-tests/suites/tracing/request/fetch-trace-header-merging/test.ts @@ -42,7 +42,7 @@ async function assertRequests({ expect(headers['sentry-trace']).toBe('12312012123120121231201212312012-1121201211212012-1'); // No multiple baggage entries - expect(headers['baggage'].match(/sentry-release/g) ?? []).toHaveLength(1); + expect(headers['baggage']).toBe('sentry-release=4.2.0'); }); } From a9ff1bc66b09942e74bff3c4bb0eb3cae27d76d3 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 26 Mar 2026 18:52:29 +0100 Subject: [PATCH 12/19] cleanup, fix test, fix colon bug in string->array header conversion --- .../test.ts | 3 +- .../tracing/double-baggage-no-spans/test.ts | 84 ------------------- .../suites/tracing/double-baggage/expects.ts | 37 ++++++++ .../no-spans}/instrument.mjs | 0 .../no-spans}/scenario.mjs | 0 .../tracing/double-baggage/no-spans/test.ts | 47 +++++++++++ .../spans-no-parent}/instrument.mjs | 0 .../spans-no-parent}/scenario.mjs | 0 .../spans-no-parent}/test.ts | 38 +-------- .../spans-parent}/instrument.mjs | 1 - .../spans-parent}/scenario.mjs | 0 .../spans-parent}/test.ts | 40 +-------- packages/node-core/src/utils/baggage.ts | 8 +- .../src/utils/outgoingFetchRequest.ts | 18 ++-- .../src/utils/outgoingHttpRequest.ts | 1 - packages/node/src/integrations/node-fetch.ts | 2 +- 16 files changed, 104 insertions(+), 175 deletions(-) delete mode 100644 dev-packages/node-integration-tests/suites/tracing/double-baggage-no-spans/test.ts create mode 100644 dev-packages/node-integration-tests/suites/tracing/double-baggage/expects.ts rename dev-packages/node-integration-tests/suites/tracing/{double-baggage-no-spans => double-baggage/no-spans}/instrument.mjs (100%) rename dev-packages/node-integration-tests/suites/tracing/{double-baggage-no-spans => double-baggage/no-spans}/scenario.mjs (100%) create mode 100644 dev-packages/node-integration-tests/suites/tracing/double-baggage/no-spans/test.ts rename dev-packages/node-integration-tests/suites/tracing/{double-baggage-spans-parent => double-baggage/spans-no-parent}/instrument.mjs (100%) rename dev-packages/node-integration-tests/suites/tracing/{double-baggage-spans-no-parent => double-baggage/spans-no-parent}/scenario.mjs (100%) rename dev-packages/node-integration-tests/suites/tracing/{double-baggage-spans-no-parent => double-baggage/spans-no-parent}/test.ts (51%) rename dev-packages/node-integration-tests/suites/tracing/{double-baggage-spans-no-parent => double-baggage/spans-parent}/instrument.mjs (94%) rename dev-packages/node-integration-tests/suites/tracing/{double-baggage-spans-parent => double-baggage/spans-parent}/scenario.mjs (100%) rename dev-packages/node-integration-tests/suites/tracing/{double-baggage-spans-parent => double-baggage/spans-parent}/test.ts (69%) diff --git a/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-other-vendors-with-sentry-entries/test.ts b/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-other-vendors-with-sentry-entries/test.ts index 6dbbc3d694fc..eebafa06bfd1 100644 --- a/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-other-vendors-with-sentry-entries/test.ts +++ b/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-other-vendors-with-sentry-entries/test.ts @@ -13,7 +13,7 @@ test('should ignore sentry-values in `baggage` header of a third party vendor an const response = await runner.makeRequest('get', '/test/express', { headers: { 'sentry-trace': '12312012123120121231201212312012-1121201211212012-1', - baggage: 'sentry-release=2.1.0,sentry-environment=myEnv,sentry-sample_rate=0.54', + baggage: 'sentry-release=2.1.0,sentry-environment=myEnv', }, }); @@ -34,7 +34,6 @@ test('should ignore sentry-values in `baggage` header of a third party vendor an 'sentry-environment=myEnv', 'sentry-release=2.1.0', expect.stringMatching(/sentry-sample_rand=\d+/), - 'sentry-sample_rate=0.54', 'third=party', ]); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/double-baggage-no-spans/test.ts b/dev-packages/node-integration-tests/suites/tracing/double-baggage-no-spans/test.ts deleted file mode 100644 index 8c51f1213185..000000000000 --- a/dev-packages/node-integration-tests/suites/tracing/double-baggage-no-spans/test.ts +++ /dev/null @@ -1,84 +0,0 @@ -import { createTestServer } from '@sentry-internal/test-utils'; -import { describe, expect } from 'vitest'; -import { createEsmAndCjsTests } from '../../../utils/runner'; -import { extractTraceparentData, parseBaggageHeader, TRACEPARENT_REGEXP } from '@sentry/core'; - -function expectNoDuplicateSentryBaggageKeys(baggage: string | string[] | undefined): void { - expect(baggage).toBeDefined(); - const baggageStr = Array.isArray(baggage) ? baggage.join(',') : (baggage as string); - const sentryEntries = baggageStr.split(',').filter(entry => entry.trim().startsWith('sentry-')); - const sentryKeyNames = sentryEntries.map(entry => entry.trim().split('=')[0]); - const uniqueKeyNames = [...new Set(sentryKeyNames)]; - expect(sentryKeyNames).toEqual(uniqueKeyNames); -} - -function expectConsistentTraceId(headers: Record): void { - const sentryTrace = headers['sentry-trace']; - expect(sentryTrace).toMatch(TRACEPARENT_REGEXP); - - const sentryTraceData = extractTraceparentData(sentryTrace as string)!; - expect(sentryTraceData.traceId).toMatch(/^[a-f\d]{32}$/); - - const baggage = parseBaggageHeader(headers['baggage']); - - const baggageTraceId = baggage!['sentry-trace_id']; - expect(baggageTraceId).toBeDefined(); - expect(baggageTraceId).toMatch(/^[a-f\d]{32}$/); - - // both headers must have the same trace_id - expect(sentryTraceData.traceId).toEqual(baggageTraceId); -} - -function expectUserSetTraceId(headers: Record): void { - const xSentryTrace = extractTraceparentData(headers['x-tracedata-sentry-trace'] as string); - const sentryTrace = extractTraceparentData(headers['sentry-trace'] as string); - expect(xSentryTrace?.traceId).toBe(sentryTrace?.traceId); - - const xBaggage = parseBaggageHeader(headers['x-tracedata-baggage']); - const baggage = parseBaggageHeader(headers['baggage']); - expect(xBaggage).toEqual(baggage); -} - -describe('double baggage prevention', () => { - createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { - test('fetch with manual getTraceData() does not duplicate sentry baggage entries', async () => { - const [SERVER_URL, closeTestServer] = await createTestServer() - .get('/api/fetch-custom-headers', headers => { - // fetch with manual getTraceData() headers - // console.log('xx sentry-tace', headers['sentry-trace']); - expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^[a-f\d]{32}-[a-f\d]{16}$/)); - expectNoDuplicateSentryBaggageKeys(headers['baggage']); - expectConsistentTraceId(headers); - expectUserSetTraceId(headers); - }) - .get('/api/fetch', headers => { - // fetch without manual headers (baseline) - expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^[a-f\d]{32}-[a-f\d]{16}$/)); - expectNoDuplicateSentryBaggageKeys(headers['baggage']); - expectConsistentTraceId(headers); - }) - .get('/api/http-custom-headers', headers => { - // http.request with manual getTraceData() headers - expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^[a-f\d]{32}-[a-f\d]{16}$/)); - expectNoDuplicateSentryBaggageKeys(headers['baggage']); - expectConsistentTraceId(headers); - expectUserSetTraceId(headers); - }) - .start(); - - await createRunner() - .withEnv({ SERVER_URL }) - .ignore('transaction') - .expect({ - event: { - exception: { - values: [{ type: 'Error', value: 'done' }], - }, - }, - }) - .start() - .completed(); - closeTestServer(); - }); - }); -}); diff --git a/dev-packages/node-integration-tests/suites/tracing/double-baggage/expects.ts b/dev-packages/node-integration-tests/suites/tracing/double-baggage/expects.ts new file mode 100644 index 000000000000..e092f29c1e65 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/double-baggage/expects.ts @@ -0,0 +1,37 @@ +import { expect } from 'vitest'; +import { extractTraceparentData, parseBaggageHeader, TRACEPARENT_REGEXP } from '@sentry/core'; + +export function expectNoDuplicateSentryBaggageKeys(baggage: string | string[] | undefined): void { + expect(baggage).toBeDefined(); + const baggageStr = Array.isArray(baggage) ? baggage.join(',') : (baggage as string); + const sentryEntries = baggageStr.split(',').filter(entry => entry.trim().startsWith('sentry-')); + const sentryKeyNames = sentryEntries.map(entry => entry.trim().split('=')[0]); + const uniqueKeyNames = [...new Set(sentryKeyNames)]; + expect(sentryKeyNames).toEqual(uniqueKeyNames); +} + +export function expectConsistentTraceId(headers: Record): void { + const sentryTrace = headers['sentry-trace']; + expect(sentryTrace).toMatch(TRACEPARENT_REGEXP); + + const sentryTraceData = extractTraceparentData(sentryTrace as string)!; + expect(sentryTraceData.traceId).toMatch(/^[a-f\d]{32}$/); + + const baggage = parseBaggageHeader(headers['baggage']); + + const baggageTraceId = baggage!['sentry-trace_id']; + expect(baggageTraceId).toBeDefined(); + expect(baggageTraceId).toMatch(/^[a-f\d]{32}$/); + + expect(sentryTraceData.traceId).toEqual(baggageTraceId); +} + +export function expectUserSetTraceId(headers: Record): void { + const xSentryTrace = extractTraceparentData(headers['x-tracedata-sentry-trace'] as string); + const sentryTrace = extractTraceparentData(headers['sentry-trace'] as string); + expect(xSentryTrace?.traceId).toBe(sentryTrace?.traceId); + + const xBaggage = parseBaggageHeader(headers['x-tracedata-baggage']); + const baggage = parseBaggageHeader(headers['baggage']); + expect(xBaggage).toEqual(baggage); +} diff --git a/dev-packages/node-integration-tests/suites/tracing/double-baggage-no-spans/instrument.mjs b/dev-packages/node-integration-tests/suites/tracing/double-baggage/no-spans/instrument.mjs similarity index 100% rename from dev-packages/node-integration-tests/suites/tracing/double-baggage-no-spans/instrument.mjs rename to dev-packages/node-integration-tests/suites/tracing/double-baggage/no-spans/instrument.mjs diff --git a/dev-packages/node-integration-tests/suites/tracing/double-baggage-no-spans/scenario.mjs b/dev-packages/node-integration-tests/suites/tracing/double-baggage/no-spans/scenario.mjs similarity index 100% rename from dev-packages/node-integration-tests/suites/tracing/double-baggage-no-spans/scenario.mjs rename to dev-packages/node-integration-tests/suites/tracing/double-baggage/no-spans/scenario.mjs diff --git a/dev-packages/node-integration-tests/suites/tracing/double-baggage/no-spans/test.ts b/dev-packages/node-integration-tests/suites/tracing/double-baggage/no-spans/test.ts new file mode 100644 index 000000000000..c517bca0d25b --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/double-baggage/no-spans/test.ts @@ -0,0 +1,47 @@ +import { createTestServer } from '@sentry-internal/test-utils'; +import { describe, expect } from 'vitest'; +import { createEsmAndCjsTests } from '../../../../utils/runner'; +import { expectConsistentTraceId, expectNoDuplicateSentryBaggageKeys, expectUserSetTraceId } from '../expects'; + +describe('double baggage prevention', () => { + createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { + test('fetch with manual getTraceData() does not duplicate sentry baggage entries', async () => { + const [SERVER_URL, closeTestServer] = await createTestServer() + .get('/api/fetch-custom-headers', headers => { + // fetch with manual getTraceData() headers + expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^[a-f\d]{32}-[a-f\d]{16}$/)); + expectNoDuplicateSentryBaggageKeys(headers['baggage']); + expectConsistentTraceId(headers); + expectUserSetTraceId(headers); + }) + .get('/api/fetch', headers => { + // fetch without manual headers (baseline) + expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^[a-f\d]{32}-[a-f\d]{16}$/)); + expectNoDuplicateSentryBaggageKeys(headers['baggage']); + expectConsistentTraceId(headers); + }) + .get('/api/http-custom-headers', headers => { + // http.request with manual getTraceData() headers + expect(headers['sentry-trace']).toEqual(expect.stringMatching(/^[a-f\d]{32}-[a-f\d]{16}$/)); + expectNoDuplicateSentryBaggageKeys(headers['baggage']); + expectConsistentTraceId(headers); + expectUserSetTraceId(headers); + }) + .start(); + + await createRunner() + .withEnv({ SERVER_URL }) + .ignore('transaction') + .expect({ + event: { + exception: { + values: [{ type: 'Error', value: 'done' }], + }, + }, + }) + .start() + .completed(); + closeTestServer(); + }); + }); +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-parent/instrument.mjs b/dev-packages/node-integration-tests/suites/tracing/double-baggage/spans-no-parent/instrument.mjs similarity index 100% rename from dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-parent/instrument.mjs rename to dev-packages/node-integration-tests/suites/tracing/double-baggage/spans-no-parent/instrument.mjs diff --git a/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-no-parent/scenario.mjs b/dev-packages/node-integration-tests/suites/tracing/double-baggage/spans-no-parent/scenario.mjs similarity index 100% rename from dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-no-parent/scenario.mjs rename to dev-packages/node-integration-tests/suites/tracing/double-baggage/spans-no-parent/scenario.mjs diff --git a/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-no-parent/test.ts b/dev-packages/node-integration-tests/suites/tracing/double-baggage/spans-no-parent/test.ts similarity index 51% rename from dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-no-parent/test.ts rename to dev-packages/node-integration-tests/suites/tracing/double-baggage/spans-no-parent/test.ts index 3e771221729f..6b5d20cdb5f7 100644 --- a/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-no-parent/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/double-baggage/spans-no-parent/test.ts @@ -1,41 +1,7 @@ import { createTestServer } from '@sentry-internal/test-utils'; import { describe, expect } from 'vitest'; -import { createEsmAndCjsTests } from '../../../utils/runner'; -import { extractTraceparentData, parseBaggageHeader, TRACEPARENT_REGEXP } from '@sentry/core'; - -function expectNoDuplicateSentryBaggageKeys(baggage: string | string[] | undefined): void { - expect(baggage).toBeDefined(); - const baggageStr = Array.isArray(baggage) ? baggage.join(',') : (baggage as string); - const sentryKeyNames = Object.keys(parseBaggageHeader(baggageStr) || {}); - const uniqueKeyNames = [...new Set(sentryKeyNames)]; - expect(sentryKeyNames).toEqual(uniqueKeyNames); -} - -function expectConsistentTraceId(headers: Record): void { - const sentryTrace = headers['sentry-trace']; - expect(sentryTrace).toMatch(TRACEPARENT_REGEXP); - - const sentryTraceData = extractTraceparentData(sentryTrace as string)!; - expect(sentryTraceData.traceId).toMatch(/^[a-f\d]{32}$/); - - const baggage = parseBaggageHeader(headers['baggage']); - - const baggageTraceId = baggage!['sentry-trace_id']; - expect(baggageTraceId).toBeDefined(); - expect(baggageTraceId).toMatch(/^[a-f\d]{32}$/); - - expect(sentryTraceData.traceId).toEqual(baggageTraceId); -} - -function expectUserSetTraceId(headers: Record): void { - const xSentryTrace = extractTraceparentData(headers['x-tracedata-sentry-trace'] as string); - const sentryTrace = extractTraceparentData(headers['sentry-trace'] as string); - expect(xSentryTrace?.traceId).toBe(sentryTrace?.traceId); - - const xBaggage = parseBaggageHeader(headers['x-tracedata-baggage']); - const baggage = parseBaggageHeader(headers['baggage']); - expect(xBaggage).toEqual(baggage); -} +import { createEsmAndCjsTests } from '../../../../utils/runner'; +import { expectConsistentTraceId, expectNoDuplicateSentryBaggageKeys, expectUserSetTraceId } from '../expects'; describe('double baggage prevention', () => { createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { diff --git a/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-no-parent/instrument.mjs b/dev-packages/node-integration-tests/suites/tracing/double-baggage/spans-parent/instrument.mjs similarity index 94% rename from dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-no-parent/instrument.mjs rename to dev-packages/node-integration-tests/suites/tracing/double-baggage/spans-parent/instrument.mjs index e21f0a36751d..46a27dd03b74 100644 --- a/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-no-parent/instrument.mjs +++ b/dev-packages/node-integration-tests/suites/tracing/double-baggage/spans-parent/instrument.mjs @@ -6,5 +6,4 @@ Sentry.init({ release: '1.0', tracesSampleRate: 1.0, transport: loggingTransport, - debug: true, }); diff --git a/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-parent/scenario.mjs b/dev-packages/node-integration-tests/suites/tracing/double-baggage/spans-parent/scenario.mjs similarity index 100% rename from dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-parent/scenario.mjs rename to dev-packages/node-integration-tests/suites/tracing/double-baggage/spans-parent/scenario.mjs diff --git a/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-parent/test.ts b/dev-packages/node-integration-tests/suites/tracing/double-baggage/spans-parent/test.ts similarity index 69% rename from dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-parent/test.ts rename to dev-packages/node-integration-tests/suites/tracing/double-baggage/spans-parent/test.ts index d11cee283cd7..22de5cb285b3 100644 --- a/dev-packages/node-integration-tests/suites/tracing/double-baggage-spans-parent/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/double-baggage/spans-parent/test.ts @@ -1,42 +1,8 @@ import { createTestServer } from '@sentry-internal/test-utils'; import { describe, expect } from 'vitest'; -import { createEsmAndCjsTests } from '../../../utils/runner'; -import { extractTraceparentData, parseBaggageHeader, TRACEPARENT_REGEXP } from '@sentry/core'; - -function expectNoDuplicateSentryBaggageKeys(baggage: string | string[] | undefined): void { - expect(baggage).toBeDefined(); - const baggageStr = Array.isArray(baggage) ? baggage.join(',') : (baggage as string); - const sentryEntries = baggageStr.split(',').filter(entry => entry.trim().startsWith('sentry-')); - const sentryKeyNames = sentryEntries.map(entry => entry.trim().split('=')[0]); - const uniqueKeyNames = [...new Set(sentryKeyNames)]; - expect(sentryKeyNames).toEqual(uniqueKeyNames); -} - -function expectConsistentTraceId(headers: Record): void { - const sentryTrace = headers['sentry-trace']; - expect(sentryTrace).toMatch(TRACEPARENT_REGEXP); - - const sentryTraceData = extractTraceparentData(sentryTrace as string)!; - expect(sentryTraceData.traceId).toMatch(/^[a-f\d]{32}$/); - - const baggage = parseBaggageHeader(headers['baggage']); - - const baggageTraceId = baggage!['sentry-trace_id']; - expect(baggageTraceId).toBeDefined(); - expect(baggageTraceId).toMatch(/^[a-f\d]{32}$/); - - expect(sentryTraceData.traceId).toEqual(baggageTraceId); -} - -function expectUserSetTraceId(headers: Record): void { - const xSentryTrace = extractTraceparentData(headers['x-tracedata-sentry-trace'] as string); - const sentryTrace = extractTraceparentData(headers['sentry-trace'] as string); - expect(xSentryTrace?.traceId).toBe(sentryTrace?.traceId); - - const xBaggage = parseBaggageHeader(headers['x-tracedata-baggage']); - const baggage = parseBaggageHeader(headers['baggage']); - expect(xBaggage).toEqual(baggage); -} +import { createEsmAndCjsTests } from '../../../../utils/runner'; +import { extractTraceparentData } from '@sentry/core'; +import { expectConsistentTraceId, expectNoDuplicateSentryBaggageKeys, expectUserSetTraceId } from '../expects'; describe('double baggage prevention - http.client spans with parent span', () => { createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { diff --git a/packages/node-core/src/utils/baggage.ts b/packages/node-core/src/utils/baggage.ts index 30798ef6729c..07b3fecb324a 100644 --- a/packages/node-core/src/utils/baggage.ts +++ b/packages/node-core/src/utils/baggage.ts @@ -28,7 +28,7 @@ export function mergeBaggageHeaders !key.startsWith(SENTRY_BAGGAGE_KEY_PREFIX)) : []; @@ -36,10 +36,8 @@ export function mergeBaggageHeaders { // Sentry-specific keys always take precedence from new baggage diff --git a/packages/node-core/src/utils/outgoingFetchRequest.ts b/packages/node-core/src/utils/outgoingFetchRequest.ts index 15871f528fe8..157eb90998f7 100644 --- a/packages/node-core/src/utils/outgoingFetchRequest.ts +++ b/packages/node-core/src/utils/outgoingFetchRequest.ts @@ -14,9 +14,6 @@ import { mergeBaggageHeaders } from './baggage'; const SENTRY_TRACE_HEADER = 'sentry-trace'; const SENTRY_BAGGAGE_HEADER = 'baggage'; -// For baggage, we make sure to merge this into a possibly existing header -const BAGGAGE_HEADER_REGEX = /baggage: (.*)\r\n/; - /** * Add trace propagation headers to an outgoing fetch/undici request. * @@ -84,7 +81,7 @@ export function addTracePropagationHeadersToFetchRequest( } if (!Array.isArray(request.headers)) { - // For orginal string request headers, we need to wrote them back to the request + // For original string request headers, we need to write them back to the request request.headers = arrayToStringHeaders(requestHeaders); } } @@ -94,7 +91,12 @@ function stringToArrayHeaders(requestHeaders: string): string[] { const headers: string[] = []; for (const header of headersArray) { try { - const [key, value] = header.split(':').map(part => part.trim()); + const colonIndex = header.indexOf(':'); + if (colonIndex === -1) { + continue; + } + const key = header.slice(0, colonIndex).trim(); + const value = header.slice(colonIndex + 1).trim(); if (key != null && value != null) { headers.push(key, value); } @@ -122,7 +124,7 @@ function arrayToStringHeaders(headers: string[]): string { * which can create duplicates when the user has already set these headers (e.g. via getTraceData()). * For sentry-trace, we keep the first occurrence (user-set). * For baggage, we merge all occurrences into one header to preserve non-sentry entries. For Sentry - * entries, we keep the first occurance. + * entries, we keep the first occurrence. */ function _deduplicateArrayHeaders(headers: string[]): void { @@ -134,7 +136,7 @@ function _deduplicateArrayHeaders(headers: string[]): void { * For a given header name, if there are multiple entries in the [key, value, key, value, ...] array, * keep the first entry and remove the rest. * For baggage, values are merged to preserve all entries but to dedupe sentry- values, and always - * keept the first occurance of them + * keep the first occurrence of them */ function _deduplicateArrayHeader(headers: string[], headerName: string): void { let firstIndex = -1; @@ -149,7 +151,7 @@ function _deduplicateArrayHeader(headers: string[], headerName: string): void { } if (headerName === SENTRY_BAGGAGE_HEADER) { - // merge the initial entry into the later occurance so that we keep the initial sentry- values around. + // merge the initial entry into the later occurrence so that we keep the initial sentry- values around. // all other non-sentry values are merged const merged = mergeBaggageHeaders(headers[i + 1] as string, headers[firstIndex + 1] as string); if (merged) { diff --git a/packages/node-core/src/utils/outgoingHttpRequest.ts b/packages/node-core/src/utils/outgoingHttpRequest.ts index f70f4bb23473..abc8a7317003 100644 --- a/packages/node-core/src/utils/outgoingHttpRequest.ts +++ b/packages/node-core/src/utils/outgoingHttpRequest.ts @@ -2,7 +2,6 @@ import type { LRUMap, SanitizedRequestData } from '@sentry/core'; import { addBreadcrumb, debug, - dynamicSamplingContextToSentryBaggageHeader, getBreadcrumbLogLevelFromHttpStatusCode, getClient, getSanitizedUrlString, diff --git a/packages/node/src/integrations/node-fetch.ts b/packages/node/src/integrations/node-fetch.ts index fe96eea3450e..74bfff2dab47 100644 --- a/packages/node/src/integrations/node-fetch.ts +++ b/packages/node/src/integrations/node-fetch.ts @@ -83,7 +83,7 @@ const _nativeNodeFetchIntegration = ((options: NodeFetchOptions = {}) => { // This is the Sentry-specific instrumentation that creates breadcrumbs & propagates traces // This must be registered after the OTEL one, to ensure that the core trace propagation logic takes presedence // Otherwise, the sentry-trace header may be set multiple times - instrumentSentryNodeFetch({ ...options }); + instrumentSentryNodeFetch(options); }, }; }) satisfies IntegrationFn; From 933a42ffc6a1baf3952f352ca196d48e3cd70512 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 26 Mar 2026 19:02:03 +0100 Subject: [PATCH 13/19] more cleanup --- packages/node-core/src/utils/outgoingFetchRequest.ts | 2 +- packages/node-core/src/utils/outgoingHttpRequest.ts | 1 - packages/opentelemetry/src/propagator.ts | 3 +++ 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/node-core/src/utils/outgoingFetchRequest.ts b/packages/node-core/src/utils/outgoingFetchRequest.ts index 157eb90998f7..218de4931c45 100644 --- a/packages/node-core/src/utils/outgoingFetchRequest.ts +++ b/packages/node-core/src/utils/outgoingFetchRequest.ts @@ -97,7 +97,7 @@ function stringToArrayHeaders(requestHeaders: string): string[] { } const key = header.slice(0, colonIndex).trim(); const value = header.slice(colonIndex + 1).trim(); - if (key != null && value != null) { + if (key && value) { headers.push(key, value); } } catch {} diff --git a/packages/node-core/src/utils/outgoingHttpRequest.ts b/packages/node-core/src/utils/outgoingHttpRequest.ts index abc8a7317003..276f7fbd7603 100644 --- a/packages/node-core/src/utils/outgoingHttpRequest.ts +++ b/packages/node-core/src/utils/outgoingHttpRequest.ts @@ -112,7 +112,6 @@ export function addTracePropagationHeadersToOutgoingRequest( cleanedExistingBaggage = objectToBaggageHeader(baggageWithoutSentry); } - // If a sentry-trace header was already added, we don't add our baggage at all. const newBaggage = mergeBaggageHeaders(cleanedExistingBaggage, baggage); if (newBaggage) { try { diff --git a/packages/opentelemetry/src/propagator.ts b/packages/opentelemetry/src/propagator.ts index 18db1e2404c9..b8582b37d964 100644 --- a/packages/opentelemetry/src/propagator.ts +++ b/packages/opentelemetry/src/propagator.ts @@ -75,6 +75,9 @@ export class SentryPropagator extends W3CBaggagePropagator { if (baggageEntries) { Object.entries(baggageEntries).forEach(([key, value]) => { if (!existingSentryTraceHeader && key.startsWith(SENTRY_BAGGAGE_KEY_PREFIX)) { + // Edge case: A baggage header with sentry- keys was added previously but no + // sentry-trace header. In this case we remove the old sentry-keys and add new + // ones below. return; } baggage = baggage.setEntry(key, { value }); From e300e335f77e94f33535e56550b6e55f91ab9c87 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 26 Mar 2026 19:07:41 +0100 Subject: [PATCH 14/19] even more cleanup --- packages/node-core/src/utils/outgoingFetchRequest.ts | 12 ++++++++---- packages/node-core/src/utils/outgoingHttpRequest.ts | 2 +- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/packages/node-core/src/utils/outgoingFetchRequest.ts b/packages/node-core/src/utils/outgoingFetchRequest.ts index 218de4931c45..9f4df0abe5f1 100644 --- a/packages/node-core/src/utils/outgoingFetchRequest.ts +++ b/packages/node-core/src/utils/outgoingFetchRequest.ts @@ -97,7 +97,7 @@ function stringToArrayHeaders(requestHeaders: string): string[] { } const key = header.slice(0, colonIndex).trim(); const value = header.slice(colonIndex + 1).trim(); - if (key && value) { + if (key) { headers.push(key, value); } } catch {} @@ -107,9 +107,13 @@ function stringToArrayHeaders(requestHeaders: string): string[] { function arrayToStringHeaders(headers: string[]): string { const headerPairs: string[] = []; - for (let i = 0; i < headers.length; i += 2) { - headerPairs.push(`${headers[i]}: ${headers[i + 1]}`); - } + + try { + for (let i = 0; i < headers.length; i += 2) { + headerPairs.push(`${headers[i]}: ${headers[i + 1]}`); + } + } catch {} + if (!headerPairs.length) { return ''; } diff --git a/packages/node-core/src/utils/outgoingHttpRequest.ts b/packages/node-core/src/utils/outgoingHttpRequest.ts index 276f7fbd7603..8c618b42f7f5 100644 --- a/packages/node-core/src/utils/outgoingHttpRequest.ts +++ b/packages/node-core/src/utils/outgoingHttpRequest.ts @@ -66,7 +66,7 @@ export function addTracePropagationHeadersToOutgoingRequest( const { 'sentry-trace': sentryTrace, baggage, traceparent } = headersToAdd; - const hasExistingSentryTraceHeader = request.getHeader('sentry-trace'); + const hasExistingSentryTraceHeader = !!request.getHeader('sentry-trace'); if (sentryTrace && !hasExistingSentryTraceHeader) { try { From 4caafd88d693a6e0c4e27a114f6f2d23f2b7b6d4 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 27 Mar 2026 12:51:32 +0100 Subject: [PATCH 15/19] more cleanup, fix edge case with odd headers, dedupe traceparent --- .../src/utils/outgoingFetchRequest.ts | 60 +++---- .../test/utils/outgoingFetchRequest.test.ts | 157 +++++++++++++++++- 2 files changed, 184 insertions(+), 33 deletions(-) diff --git a/packages/node-core/src/utils/outgoingFetchRequest.ts b/packages/node-core/src/utils/outgoingFetchRequest.ts index 9f4df0abe5f1..af705411d012 100644 --- a/packages/node-core/src/utils/outgoingFetchRequest.ts +++ b/packages/node-core/src/utils/outgoingFetchRequest.ts @@ -13,7 +13,7 @@ import { mergeBaggageHeaders } from './baggage'; const SENTRY_TRACE_HEADER = 'sentry-trace'; const SENTRY_BAGGAGE_HEADER = 'baggage'; - +const W3C_TRACEPARENT_HEADER = 'traceparent'; /** * Add trace propagation headers to an outgoing fetch/undici request. * @@ -46,14 +46,18 @@ export function addTracePropagationHeadersToFetchRequest( // OTel's UndiciInstrumentation calls propagation.inject() which unconditionally // appends headers to the request. When the user also sets headers via getTraceData(), - // this results in duplicate sentry-trace and baggage entries. + // this results in duplicate sentry-trace and baggage (and optionally traceparent) entries. // We clean these up before applying our own logic. - _deduplicateArrayHeaders(requestHeaders); + _deduplicateArrayHeader(requestHeaders, SENTRY_TRACE_HEADER); + _deduplicateArrayHeader(requestHeaders, SENTRY_BAGGAGE_HEADER); + if (propagateTraceparent) { + _deduplicateArrayHeader(requestHeaders, W3C_TRACEPARENT_HEADER); + } // We do not want to overwrite existing headers here // If the core UndiciInstrumentation is registered, it will already have set the headers // We do not want to add any then - const hasExistingSentryTraceHeader = requestHeaders.includes(SENTRY_TRACE_HEADER); + const hasExistingSentryTraceHeader = _findExistingHeaderIndex(requestHeaders, SENTRY_TRACE_HEADER) !== -1; // We do not want to set any headers if we already have an existing sentry-trace header. // sentry-trace is still the source of truth, otherwise we risk mixing up baggage and sentry-trace values. @@ -62,20 +66,20 @@ export function addTracePropagationHeadersToFetchRequest( requestHeaders.push(SENTRY_TRACE_HEADER, sentryTrace); } - if (traceparent && !requestHeaders.includes('traceparent')) { + if (traceparent && _findExistingHeaderIndex(requestHeaders, 'traceparent') === -1) { requestHeaders.push('traceparent', traceparent); } // For baggage, we make sure to merge this into a possibly existing header - const existingBaggagePos = requestHeaders.findIndex(header => header === SENTRY_BAGGAGE_HEADER); - if (baggage && existingBaggagePos === -1) { + const existingBaggageIndex = _findExistingHeaderIndex(requestHeaders, SENTRY_BAGGAGE_HEADER); + if (baggage && existingBaggageIndex === -1) { requestHeaders.push(SENTRY_BAGGAGE_HEADER, baggage); } else if (baggage) { // headers in format [key_0, value_0, key_1, value_1, ...], hence the +1 here - const existingBaggage = requestHeaders[existingBaggagePos + 1]; - const merged = mergeBaggageHeaders(existingBaggage, baggage); + const existingBaggageValue = requestHeaders[existingBaggageIndex + 1]; + const merged = mergeBaggageHeaders(existingBaggageValue, baggage); if (merged) { - requestHeaders[existingBaggagePos + 1] = merged; + requestHeaders[existingBaggageIndex + 1] = merged; } } } @@ -108,11 +112,15 @@ function stringToArrayHeaders(requestHeaders: string): string[] { function arrayToStringHeaders(headers: string[]): string { const headerPairs: string[] = []; - try { - for (let i = 0; i < headers.length; i += 2) { - headerPairs.push(`${headers[i]}: ${headers[i + 1]}`); + for (let i = 0; i < headers.length; i += 2) { + const key = headers[i]; + const value = headers[i + 1]; + if (!key || value == null) { + // skip falsy keys but only null/undefined values + continue; } - } catch {} + headerPairs.push(`${key}: ${value}`); + } if (!headerPairs.length) { return ''; @@ -121,21 +129,6 @@ function arrayToStringHeaders(headers: string[]): string { return headerPairs.join('\r\n').concat('\r\n'); } -/** - * Remove duplicate sentry-trace and baggage headers from the request. - * - * OTel's UndiciInstrumentation unconditionally appends headers via propagation.inject(), - * which can create duplicates when the user has already set these headers (e.g. via getTraceData()). - * For sentry-trace, we keep the first occurrence (user-set). - * For baggage, we merge all occurrences into one header to preserve non-sentry entries. For Sentry - * entries, we keep the first occurrence. - */ - -function _deduplicateArrayHeaders(headers: string[]): void { - _deduplicateArrayHeader(headers, SENTRY_TRACE_HEADER); - _deduplicateArrayHeader(headers, SENTRY_BAGGAGE_HEADER); -} - /** * For a given header name, if there are multiple entries in the [key, value, key, value, ...] array, * keep the first entry and remove the rest. @@ -167,6 +160,15 @@ function _deduplicateArrayHeader(headers: string[], headerName: string): void { } } +/** + * Find the index of an existing header in an array of headers. + * Only take even indices, because headers are in format [key_0, value_0, key_1, value_1, ...] + * otherwise we could match a header _value_ with @param name + */ +function _findExistingHeaderIndex(headers: string[], name: string): number { + return headers.findIndex((header, i) => i % 2 === 0 && header === name); +} + /** Add a breadcrumb for an outgoing fetch/undici request. */ export function addFetchRequestBreadcrumb(request: UndiciRequest, response: UndiciResponse): void { const data = getBreadcrumbData(request); diff --git a/packages/node-core/test/utils/outgoingFetchRequest.test.ts b/packages/node-core/test/utils/outgoingFetchRequest.test.ts index 32f6584d8300..f023c2cab268 100644 --- a/packages/node-core/test/utils/outgoingFetchRequest.test.ts +++ b/packages/node-core/test/utils/outgoingFetchRequest.test.ts @@ -1,24 +1,30 @@ +import type { MockedFunction } from 'vitest'; import { describe, beforeEach, vi, expect, it } from 'vitest'; import type { UndiciRequest } from '../../src/integrations/node-fetch/types'; import { addTracePropagationHeadersToFetchRequest } from '../../src/utils/outgoingFetchRequest'; import { LRUMap } from '@sentry/core'; import * as SentryCore from '@sentry/core'; -const mockedGetTraceData = vi.hoisted(() => +const mockedGetTraceData: MockedFunction<() => ReturnType> = vi.hoisted(() => vi.fn(() => ({ 'sentry-trace': 'trace_id_1-span_id_1-1', baggage: 'sentry-trace_id=trace_id_1,sentry-sampled=true,sentry-environment=staging', })), ); +const mockedClientGetOptions: MockedFunction<() => Partial> = vi.hoisted(() => + vi.fn(() => ({ + tracePropagationTargets: ['https://example.com'], + propagateTraceparent: true, + })), +); + vi.mock('@sentry/core', async () => { const actual = await vi.importActual('@sentry/core'); return { ...actual, getClient: vi.fn(() => ({ - getOptions: vi.fn(() => ({ - tracePropagationTargets: ['https://example.com'], - })), + getOptions: mockedClientGetOptions, })), shouldPropagateTraceForUrl: () => true, getTraceData: mockedGetTraceData, @@ -62,6 +68,31 @@ describe('addTracePropagationHeadersToFetchRequest', () => { ]); }); + it('adds sentry-trace, baggage and traceparent headers to request', () => { + mockedGetTraceData.mockReturnValueOnce({ + 'sentry-trace': 'trace_id_1-span_id_1-1', + baggage: 'sentry-trace_id=trace_id_1,sentry-sampled=true,sentry-environment=staging', + traceparent: '00-trace_id_1-span_id_1-01', + }); + + const request = { + headers: [] as string[], + origin: 'https://some-service.com', + path: '/api/test', + } as UndiciRequest; + + addTracePropagationHeadersToFetchRequest(request, new LRUMap(100)); + + expect(request.headers).toEqual([ + 'sentry-trace', + 'trace_id_1-span_id_1-1', + 'traceparent', + '00-trace_id_1-span_id_1-01', + 'baggage', + 'sentry-trace_id=trace_id_1,sentry-sampled=true,sentry-environment=staging', + ]); + }); + it('preserves non-sentry entries in existing baggage header', () => { const request = { headers: ['baggage', 'other=entry,not=sentry'], @@ -79,6 +110,31 @@ describe('addTracePropagationHeadersToFetchRequest', () => { ]); }); + it('preserves pre-existing traceparent header', () => { + mockedGetTraceData.mockReturnValueOnce({ + 'sentry-trace': 'trace_id_1-span_id_1-1', + baggage: 'sentry-trace_id=trace_id_1,sentry-sampled=true,sentry-environment=staging', + traceparent: '00-trace_id_1-span_id_1-01', + }); + + const request = { + headers: ['traceparent', '00-some-other-trace_id-span_id_x-01'], + origin: 'https://some-service.com', + path: '/api/test', + } as UndiciRequest; + + addTracePropagationHeadersToFetchRequest(request, new LRUMap(100)); + + expect(request.headers).toEqual([ + 'traceparent', + '00-some-other-trace_id-span_id_x-01', + 'sentry-trace', + 'trace_id_1-span_id_1-1', + 'baggage', + 'sentry-trace_id=trace_id_1,sentry-sampled=true,sentry-environment=staging', + ]); + }); + describe('when sentry-trace is already set', () => { it("preserves original sentry-trace header doesn't add baggage", () => { const request = { @@ -113,6 +169,24 @@ describe('addTracePropagationHeadersToFetchRequest', () => { 'sentry-trace_id=trace_id_2,sentry-sampled=true,sentry-environment=staging', ]); }); + + it("doesn't add traceparent header even if propagateTraceparent is true", () => { + mockedGetTraceData.mockReturnValueOnce({ + 'sentry-trace': 'trace_id_2-span_id_2-1', + baggage: 'sentry-trace_id=trace_id_2,sentry-sampled=true,sentry-environment=staging', + traceparent: '00-trace_id_2-span_id_2-01', + }); + + const request = { + headers: ['sentry-trace', 'trace_id_2-span_id_2-1'], + origin: 'https://some-service.com', + path: '/api/test', + } as UndiciRequest; + + addTracePropagationHeadersToFetchRequest(request, new LRUMap(100)); + + expect(request.headers).toEqual(['sentry-trace', 'trace_id_2-span_id_2-1']); + }); }); describe('pre-existing header deduplication', () => { @@ -142,6 +216,43 @@ describe('addTracePropagationHeadersToFetchRequest', () => { ]); }); + it('deduplicates traceparent headers if propagateTraceparent is true', () => { + mockedClientGetOptions.mockReturnValueOnce({ + tracePropagationTargets: ['https://example.com'], + propagateTraceparent: true, + }); + + const request = { + headers: [ + 'sentry-trace', + 'user-trace_id-xyz-1', + 'baggage', + 'sentry-trace_id=user-trace_id-xyz-1,sentry-sampled=true,sentry-environment=user', + 'traceparent', + '00-user-trace_id-xyz-1-01', + 'sentry-trace', + 'undici-trace_id-abc-1', + 'baggage', + 'sentry-trace_id=undici-trace_id-abc-1,sentry-sampled=true,sentry-environment=undici', + 'traceparent', + '00-undici-trace_id-abc-1-01', + ], + origin: 'https://some-service.com', + path: '/api/test', + } as UndiciRequest; + + addTracePropagationHeadersToFetchRequest(request, new LRUMap(100)); + + expect(request.headers).toEqual([ + 'sentry-trace', + 'user-trace_id-xyz-1', + 'baggage', + 'sentry-trace_id=user-trace_id-xyz-1,sentry-sampled=true,sentry-environment=user', + 'traceparent', + '00-user-trace_id-xyz-1-01', + ]); + }); + // admittedly an unrealistic edge case but doesn't hurt to test it it("doesn't crash with incomplete headers array", () => { const request = { @@ -215,6 +326,44 @@ describe('addTracePropagationHeadersToFetchRequest', () => { ]); }); }); + + it('doesn\'t mistake a header value with "sentry-trace" for a sentry-trace header', () => { + const request = { + headers: ['x-allow-header', 'sentry-trace'], + origin: 'https://some-service.com', + path: '/api/test', + } as UndiciRequest; + + addTracePropagationHeadersToFetchRequest(request, new LRUMap(100)); + + expect(request.headers).toEqual([ + 'x-allow-header', + 'sentry-trace', + 'sentry-trace', + 'trace_id_1-span_id_1-1', + 'baggage', + 'sentry-trace_id=trace_id_1,sentry-sampled=true,sentry-environment=staging', + ]); + }); + + it('doesn\'t mistake a header value with "baggage" for a sentry-trace header', () => { + const request = { + headers: ['x-allow-header', 'baggage'], + origin: 'https://some-service.com', + path: '/api/test', + } as UndiciRequest; + + addTracePropagationHeadersToFetchRequest(request, new LRUMap(100)); + + expect(request.headers).toEqual([ + 'x-allow-header', + 'baggage', + 'sentry-trace', + 'trace_id_1-span_id_1-1', + 'baggage', + 'sentry-trace_id=trace_id_1,sentry-sampled=true,sentry-environment=staging', + ]); + }); }); describe('when headers are a string', () => { From ff6b4360e34b2a54722d7a10db526f1e684d0b4f Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 27 Mar 2026 14:07:31 +0100 Subject: [PATCH 16/19] last cleanup, one more test --- .../src/utils/outgoingFetchRequest.ts | 10 ++++--- .../src/utils/outgoingHttpRequest.ts | 26 ++++++------------- packages/node-core/test/utils/baggage.test.ts | 6 +++++ 3 files changed, 20 insertions(+), 22 deletions(-) diff --git a/packages/node-core/src/utils/outgoingFetchRequest.ts b/packages/node-core/src/utils/outgoingFetchRequest.ts index af705411d012..778d759fde73 100644 --- a/packages/node-core/src/utils/outgoingFetchRequest.ts +++ b/packages/node-core/src/utils/outgoingFetchRequest.ts @@ -147,10 +147,12 @@ function _deduplicateArrayHeader(headers: string[], headerName: string): void { continue; } - if (headerName === SENTRY_BAGGAGE_HEADER) { - // merge the initial entry into the later occurrence so that we keep the initial sentry- values around. - // all other non-sentry values are merged - const merged = mergeBaggageHeaders(headers[i + 1] as string, headers[firstIndex + 1] as string); + const firstHeaderValue = headers[firstIndex + 1]; + if (headerName === SENTRY_BAGGAGE_HEADER && firstHeaderValue) { + // mergeBaggageHeaders always takes sentry- values from the new baggage (2nd param) and merges + // it with the existing one (1st param). Here, we want to keep the first header's existing + // sentry- values in favor of the new ones. Hence we swap the parameters. + const merged = mergeBaggageHeaders(headers[i + 1], firstHeaderValue); if (merged) { headers[firstIndex + 1] = merged; } diff --git a/packages/node-core/src/utils/outgoingHttpRequest.ts b/packages/node-core/src/utils/outgoingHttpRequest.ts index 8c618b42f7f5..728290e44221 100644 --- a/packages/node-core/src/utils/outgoingHttpRequest.ts +++ b/packages/node-core/src/utils/outgoingHttpRequest.ts @@ -68,7 +68,11 @@ export function addTracePropagationHeadersToOutgoingRequest( const hasExistingSentryTraceHeader = !!request.getHeader('sentry-trace'); - if (sentryTrace && !hasExistingSentryTraceHeader) { + if (hasExistingSentryTraceHeader) { + return; + } + + if (sentryTrace) { try { request.setHeader('sentry-trace', sentryTrace); DEBUG_BUILD && debug.log(LOG_PREFIX, 'Added sentry-trace header to outgoing request'); @@ -82,7 +86,7 @@ export function addTracePropagationHeadersToOutgoingRequest( } } - if (traceparent && !hasExistingSentryTraceHeader && !request.getHeader('traceparent')) { + if (traceparent && !request.getHeader('traceparent')) { try { request.setHeader('traceparent', traceparent); DEBUG_BUILD && debug.log(LOG_PREFIX, 'Added traceparent header to outgoing request'); @@ -96,23 +100,9 @@ export function addTracePropagationHeadersToOutgoingRequest( } } - if (baggage && !hasExistingSentryTraceHeader) { + if (baggage) { const existingBaggage = request.getHeader('baggage'); - - let cleanedExistingBaggage = existingBaggage; - - // In the edge case that a baggage header with sentry- keys was added - // BUT NO sentry-trace header, we overwrite the sentry- keys in the header we attach. - // Therefore, we clean the existing baggage header of all sentry- keys. - if (existingBaggage) { - const tmpBaggage = parseBaggageHeader(existingBaggage); - const baggageWithoutSentry = tmpBaggage - ? Object.fromEntries(Object.entries(tmpBaggage).filter(([key]) => !key.startsWith(SENTRY_BAGGAGE_KEY_PREFIX))) - : {}; - cleanedExistingBaggage = objectToBaggageHeader(baggageWithoutSentry); - } - - const newBaggage = mergeBaggageHeaders(cleanedExistingBaggage, baggage); + const newBaggage = mergeBaggageHeaders(existingBaggage, baggage); if (newBaggage) { try { request.setHeader('baggage', newBaggage); diff --git a/packages/node-core/test/utils/baggage.test.ts b/packages/node-core/test/utils/baggage.test.ts index 16ace2039962..0d7ff5f757d5 100644 --- a/packages/node-core/test/utils/baggage.test.ts +++ b/packages/node-core/test/utils/baggage.test.ts @@ -158,4 +158,10 @@ describe('mergeBaggageHeaders', () => { const result = mergeBaggageHeaders(existing, newBaggage); expect(result).toBe('non-sentry=keep,sentry-trace_id=new,sentry-environment=new'); }); + + it('preserves existing sentry entries when new baggage has no sentry entries', () => { + const result = mergeBaggageHeaders('sentry-release=1.0.0,foo=bar', 'baz=qux'); + + expect(result).toBe('sentry-release=1.0.0,foo=bar,baz=qux'); + }); }); From e3e008e072e192e50f5c20f06f05b9924c546f31 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 27 Mar 2026 14:12:44 +0100 Subject: [PATCH 17/19] improve mergeBaggageHeaders performance --- packages/node-core/src/utils/baggage.ts | 41 ++++++++++++++++--------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/packages/node-core/src/utils/baggage.ts b/packages/node-core/src/utils/baggage.ts index 07b3fecb324a..496c834d5c23 100644 --- a/packages/node-core/src/utils/baggage.ts +++ b/packages/node-core/src/utils/baggage.ts @@ -24,28 +24,39 @@ export function mergeBaggageHeaders - key.startsWith(SENTRY_BAGGAGE_KEY_PREFIX), - ); + // Single pass over new entries to partition sentry vs non-sentry + const newSentryEntries: Record = {}; + const newNonSentryEntries: Record = {}; + for (const [key, value] of Object.entries(newBaggageEntries)) { + if (key.startsWith(SENTRY_BAGGAGE_KEY_PREFIX)) { + newSentryEntries[key] = value; + } else { + newNonSentryEntries[key] = value; + } + } - const existingBaggageEntriesWithoutSentry = existingBaggageEntries - ? Object.entries(existingBaggageEntries).filter(([key]) => !key.startsWith(SENTRY_BAGGAGE_KEY_PREFIX)) - : []; + const hasNewSentryEntries = Object.keys(newSentryEntries).length > 0; // If new baggage contains at least one sentry- value, we remove all old sentry- values // otherwise, we keep old sentry- values. If we don't remove old sentry- values, we end // up with an inconsistent dynamic sampling context propagation. - const mergedBaggageEntries = newSentryBaggageEntries.length - ? Object.fromEntries(existingBaggageEntriesWithoutSentry) - : (existingBaggageEntries ?? {}); - - Object.entries(newBaggageEntries).forEach(([key, value]) => { - // Sentry-specific keys always take precedence from new baggage - // Non-Sentry keys only added if not already present - if (key.startsWith(SENTRY_BAGGAGE_KEY_PREFIX) || !mergedBaggageEntries[key]) { + const mergedBaggageEntries: Record = {}; + if (existingBaggageEntries) { + for (const [key, value] of Object.entries(existingBaggageEntries)) { + if (hasNewSentryEntries && key.startsWith(SENTRY_BAGGAGE_KEY_PREFIX)) { + continue; + } mergedBaggageEntries[key] = value; } - }); + } + + // Sentry entries from new baggage always overwrite; non-sentry only if not already present + Object.assign(mergedBaggageEntries, newSentryEntries); + for (const [key, value] of Object.entries(newNonSentryEntries)) { + if (!mergedBaggageEntries[key]) { + mergedBaggageEntries[key] = value; + } + } return objectToBaggageHeader(mergedBaggageEntries); } From c9627af3094a41779e9962e1ba6efc93c8e7c3cf Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 27 Mar 2026 14:34:37 +0100 Subject: [PATCH 18/19] remove unused imports --- packages/node-core/src/utils/outgoingHttpRequest.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/node-core/src/utils/outgoingHttpRequest.ts b/packages/node-core/src/utils/outgoingHttpRequest.ts index 728290e44221..34624900b472 100644 --- a/packages/node-core/src/utils/outgoingHttpRequest.ts +++ b/packages/node-core/src/utils/outgoingHttpRequest.ts @@ -7,10 +7,7 @@ import { getSanitizedUrlString, getTraceData, isError, - objectToBaggageHeader, - parseBaggageHeader, parseUrl, - SENTRY_BAGGAGE_KEY_PREFIX, shouldPropagateTraceForUrl, } from '@sentry/core'; import type { ClientRequest, IncomingMessage, RequestOptions } from 'http'; From 6b77c61db64655cc6bf7930ceaa1d88c8d24922b Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 27 Mar 2026 16:48:10 +0100 Subject: [PATCH 19/19] log when failing to convert string to array header --- packages/node-core/src/utils/outgoingFetchRequest.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/node-core/src/utils/outgoingFetchRequest.ts b/packages/node-core/src/utils/outgoingFetchRequest.ts index 778d759fde73..cad20496e478 100644 --- a/packages/node-core/src/utils/outgoingFetchRequest.ts +++ b/packages/node-core/src/utils/outgoingFetchRequest.ts @@ -10,7 +10,7 @@ import { } from '@sentry/core'; import type { UndiciRequest, UndiciResponse } from '../integrations/node-fetch/types'; import { mergeBaggageHeaders } from './baggage'; - +import { debug } from '@sentry/core'; const SENTRY_TRACE_HEADER = 'sentry-trace'; const SENTRY_BAGGAGE_HEADER = 'baggage'; const W3C_TRACEPARENT_HEADER = 'traceparent'; @@ -104,7 +104,9 @@ function stringToArrayHeaders(requestHeaders: string): string[] { if (key) { headers.push(key, value); } - } catch {} + } catch { + debug.warn(`Failed to convert string request header to array header: ${header}`); + } } return headers; }