diff --git a/dev-packages/node-integration-tests/suites/tracing/double-baggage-prevention/instrument.mjs b/dev-packages/node-integration-tests/suites/tracing/double-baggage-prevention/instrument.mjs new file mode 100644 index 000000000000..9ffde125d498 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/double-baggage-prevention/instrument.mjs @@ -0,0 +1,8 @@ +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', + transport: loggingTransport, +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/double-baggage-prevention/scenario.mjs b/dev-packages/node-integration-tests/suites/tracing/double-baggage-prevention/scenario.mjs new file mode 100644 index 000000000000..c68c507a7cb9 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/double-baggage-prevention/scenario.mjs @@ -0,0 +1,96 @@ +import * as Sentry from '@sentry/node'; +import http from 'http'; + +let capturedHeaders = {}; +const targetServer = http.createServer((req, res) => { + capturedHeaders = { + 'sentry-trace': req.headers['sentry-trace'], + baggage: req.headers['baggage'], + }; + res.writeHead(200); + res.end('ok'); +}); + +targetServer.listen(0, async () => { + const targetPort = targetServer.address().port; + const targetUrl = `http://localhost:${targetPort}/target`; + + try { + // Step 1: fetch with manual getTraceData() headers + capturedHeaders = {}; + await fetch(targetUrl, { headers: { ...Sentry.getTraceData() } }); + const fetchHeaders1 = { ...capturedHeaders }; + + // Step 2: fetch without manual headers + capturedHeaders = {}; + await fetch(targetUrl); + const fetchHeaders2 = { ...capturedHeaders }; + + // Step 3: http.request with manual getTraceData() headers + capturedHeaders = {}; + await new Promise((resolve, reject) => { + const traceData = Sentry.getTraceData(); + const req = http.request( + { + hostname: 'localhost', + port: targetPort, + path: '/target', + method: 'GET', + headers: traceData, + }, + res => { + res.on('data', () => {}); + res.on('end', () => resolve()); + }, + ); + req.on('error', reject); + req.end(); + }); + const httpHeaders = { ...capturedHeaders }; + + // Step 4: fetch with custom + manual sentry baggage + capturedHeaders = {}; + const traceData = Sentry.getTraceData(); + await fetch(targetUrl, { + headers: { + ...traceData, + baggage: `custom-key=value,${traceData.baggage}`, + }, + }); + const fetchHeaders4 = { ...capturedHeaders }; + + const results = { + test1: { + sentryTrace: fetchHeaders1['sentry-trace'], + baggage: fetchHeaders1.baggage, + hasDuplicateSentryTrace: fetchHeaders1['sentry-trace']?.includes(','), + sentryBaggageCount: (fetchHeaders1.baggage?.match(/sentry-/g) || []).length, + }, + test2: { + sentryTrace: fetchHeaders2['sentry-trace'], + baggage: fetchHeaders2.baggage, + hasDuplicateSentryTrace: fetchHeaders2['sentry-trace']?.includes(','), + sentryBaggageCount: (fetchHeaders2.baggage?.match(/sentry-/g) || []).length, + }, + test3: { + sentryTrace: httpHeaders['sentry-trace'], + baggage: httpHeaders.baggage, + hasDuplicateSentryTrace: httpHeaders['sentry-trace']?.includes(','), + sentryBaggageCount: (httpHeaders.baggage?.match(/sentry-/g) || []).length, + }, + test4: { + sentryTrace: fetchHeaders4['sentry-trace'], + baggage: fetchHeaders4.baggage, + hasDuplicateSentryTrace: fetchHeaders4['sentry-trace']?.includes(','), + hasCustomBaggage: fetchHeaders4.baggage?.includes('custom-key=value'), + sentryBaggageCount: (fetchHeaders4.baggage?.match(/sentry-/g) || []).length, + }, + }; + + process.stdout.write(`RESULTS: ${JSON.stringify(results)}\n`); + + Sentry.captureMessage('double-baggage-test-complete'); + } finally { + targetServer.close(); + } +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/double-baggage-prevention/test.ts b/dev-packages/node-integration-tests/suites/tracing/double-baggage-prevention/test.ts new file mode 100644 index 000000000000..ccdcc82ac3f3 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/double-baggage-prevention/test.ts @@ -0,0 +1,45 @@ +import { describe, expect } from 'vitest'; +import { createEsmAndCjsTests } from '../../../utils/runner'; + +describe('double baggage prevention', () => { + createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => { + test('prevents duplicate headers when using manual getTraceData() with auto-instrumentation', async () => { + const runner = createRunner() + .ignore('transaction') + .expect({ event: { message: 'double-baggage-test-complete' } }) + .start(); + + await runner.completed(); + + const logs = runner.getLogs(); + const resultsLine = logs.find(line => line.startsWith('RESULTS: ')); + expect(resultsLine).toBeTruthy(); + + const results = JSON.parse(resultsLine!.replace('RESULTS: ', '')); + + expect(results.test2.sentryTrace).toBeDefined(); + expect(results.test2.sentryTrace).toMatch(/^[a-f0-9]{32}-[a-f0-9]{16}(-[01])?$/); + expect(results.test2.baggage).toBeDefined(); + expect(results.test2.sentryBaggageCount).toBeGreaterThan(0); + const baselineSentryBaggageCount = results.test2.sentryBaggageCount; + + expect(results.test1.hasDuplicateSentryTrace).toBe(false); + expect(results.test1.sentryTrace).toBeDefined(); + expect(results.test1.sentryTrace).toMatch(/^[a-f0-9]{32}-[a-f0-9]{16}(-[01])?$/); + expect(results.test1.baggage).toBeDefined(); + expect(results.test1.sentryBaggageCount).toBe(baselineSentryBaggageCount); + + expect(results.test3.hasDuplicateSentryTrace).toBe(false); + expect(results.test3.sentryTrace).toBeDefined(); + expect(results.test3.sentryTrace).toMatch(/^[a-f0-9]{32}-[a-f0-9]{16}(-[01])?$/); + expect(results.test3.baggage).toBeDefined(); + expect(results.test3.sentryBaggageCount).toBe(baselineSentryBaggageCount); + + expect(results.test4.hasDuplicateSentryTrace).toBe(false); + expect(results.test4.hasCustomBaggage).toBe(true); + expect(results.test4.sentryTrace).toBeDefined(); + expect(results.test4.baggage).toContain('custom-key=value'); + expect(results.test4.sentryBaggageCount).toBe(baselineSentryBaggageCount); + }); + }); +}); diff --git a/packages/node-core/src/utils/baggage.ts b/packages/node-core/src/utils/baggage.ts index d236851559db..8f7703c55edb 100644 --- a/packages/node-core/src/utils/baggage.ts +++ b/packages/node-core/src/utils/baggage.ts @@ -1,13 +1,8 @@ -import { objectToBaggageHeader, parseBaggageHeader } from '@sentry/core'; +import { objectToBaggageHeader, parseBaggageHeader, SENTRY_BAGGAGE_KEY_PREFIX } from '@sentry/core'; /** * Merge two baggage headers into one. - * - Sentry-specific entries (keys starting with "sentry-") from the new baggage take precedence - * - Non-Sentry entries from existing baggage take precedence * The order of the existing baggage will be preserved, and new entries will be added to the end. - * - * This matches the behavior of OTEL's propagation.inject() which uses baggage.setEntry() - * to overwrite existing entries with the same key. */ export function mergeBaggageHeaders( existing: Existing, @@ -24,12 +19,9 @@ 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..c9dcef898851 100644 --- a/packages/node-core/src/utils/outgoingFetchRequest.ts +++ b/packages/node-core/src/utils/outgoingFetchRequest.ts @@ -64,7 +64,7 @@ export function addTracePropagationHeadersToFetchRequest( const existingBaggagePos = requestHeaders.findIndex(header => header === SENTRY_BAGGAGE_HEADER); if (baggage && existingBaggagePos === -1) { requestHeaders.push(SENTRY_BAGGAGE_HEADER, baggage); - } else if (baggage) { + } else if (baggage && existingBaggagePos !== -1) { const existingBaggage = requestHeaders[existingBaggagePos + 1]; const merged = mergeBaggageHeaders(existingBaggage, baggage); if (merged) { @@ -85,7 +85,7 @@ export function addTracePropagationHeadersToFetchRequest( const existingBaggage = request.headers.match(BAGGAGE_HEADER_REGEX)?.[1]; if (baggage && !existingBaggage) { request.headers += `${SENTRY_BAGGAGE_HEADER}: ${baggage}\r\n`; - } else if (baggage) { + } else if (baggage && existingBaggage) { const merged = mergeBaggageHeaders(existingBaggage, baggage); if (merged) { request.headers = request.headers.replace(BAGGAGE_HEADER_REGEX, `baggage: ${merged}\r\n`); diff --git a/packages/node-core/src/utils/outgoingHttpRequest.ts b/packages/node-core/src/utils/outgoingHttpRequest.ts index 7eafa941286a..b69887e8070e 100644 --- a/packages/node-core/src/utils/outgoingHttpRequest.ts +++ b/packages/node-core/src/utils/outgoingHttpRequest.ts @@ -92,7 +92,8 @@ 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..85879b8f71a0 100644 --- a/packages/node-core/test/utils/baggage.test.ts +++ b/packages/node-core/test/utils/baggage.test.ts @@ -3,18 +3,14 @@ import { mergeBaggageHeaders } from '../../src/utils/baggage'; describe('mergeBaggageHeaders', () => { it('returns new baggage when existing is undefined', () => { - const result = mergeBaggageHeaders(undefined, 'foo=bar'); - expect(result).toBe('foo=bar'); - }); - - it('returns existing baggage when new baggage is empty', () => { - const result = mergeBaggageHeaders('foo=bar', ''); - expect(result).toBe('foo=bar'); + const result = mergeBaggageHeaders(undefined, 'sentry-environment=production'); + expect(result).toBe('sentry-environment=production'); }); it('returns existing baggage when new baggage is invalid', () => { - const result = mergeBaggageHeaders('foo=bar', 'invalid'); - expect(result).toBe('foo=bar'); + const existing = 'custom-key=value'; + const result = mergeBaggageHeaders(existing, ''); + expect(result).toBe(existing); }); it('handles empty existing baggage', () => { @@ -87,7 +83,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 +111,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 +124,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'); + }); });