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..71a5cf60daae --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/double-baggage-prevention/instrument.mjs @@ -0,0 +1,10 @@ +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, + tracePropagationTargets: [/localhost/], + 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..f293a2479efa --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/double-baggage-prevention/scenario.mjs @@ -0,0 +1,95 @@ +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, + }, + }; + + } catch (error) { + throw error; + } finally { + targetServer.close(); + } +}); diff --git a/packages/node-core/src/utils/baggage.ts b/packages/node-core/src/utils/baggage.ts index be8e62b9497b..3def81e237d1 100644 --- a/packages/node-core/src/utils/baggage.ts +++ b/packages/node-core/src/utils/baggage.ts @@ -1,4 +1,20 @@ -import { objectToBaggageHeader, parseBaggageHeader } from '@sentry/core'; +import { objectToBaggageHeader, parseBaggageHeader, SENTRY_BAGGAGE_KEY_PREFIX } from '@sentry/core'; + +/** + * To check if a baggage header contains any Sentry baggage values. + * + * @param baggageHeader The baggage header to check + * @returns true if the baggage header contains any keys with the 'sentry-' prefix + */ +export function hasSentryBaggageValues(baggageHeader: string | string[] | undefined): boolean { + if (!baggageHeader) { + return false; + } + + const baggageString = Array.isArray(baggageHeader) ? baggageHeader.join(',') : baggageHeader; + + return baggageString.split(',').some(entry => entry.trim().startsWith(SENTRY_BAGGAGE_KEY_PREFIX)); +} /** * Merge two baggage headers into one, where the existing one takes precedence. diff --git a/packages/node-core/src/utils/outgoingFetchRequest.ts b/packages/node-core/src/utils/outgoingFetchRequest.ts index ce04a26db1e0..837a9466e520 100644 --- a/packages/node-core/src/utils/outgoingFetchRequest.ts +++ b/packages/node-core/src/utils/outgoingFetchRequest.ts @@ -9,7 +9,7 @@ import { shouldPropagateTraceForUrl, } from '@sentry/core'; import type { UndiciRequest, UndiciResponse } from '../integrations/node-fetch/types'; -import { mergeBaggageHeaders } from './baggage'; +import { hasSentryBaggageValues, mergeBaggageHeaders } from './baggage'; const SENTRY_TRACE_HEADER = 'sentry-trace'; const SENTRY_BAGGAGE_HEADER = 'baggage'; @@ -64,11 +64,14 @@ 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) { - requestHeaders[existingBaggagePos + 1] = merged; + // if existing baggage already has Sentry values, just skip it + if (!hasSentryBaggageValues(existingBaggage)) { + const merged = mergeBaggageHeaders(existingBaggage, baggage); + if (merged) { + requestHeaders[existingBaggagePos + 1] = merged; + } } } } else { @@ -85,10 +88,12 @@ 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) { - const merged = mergeBaggageHeaders(existingBaggage, baggage); - if (merged) { - request.headers = request.headers.replace(BAGGAGE_HEADER_REGEX, `baggage: ${merged}\r\n`); + } else if (baggage && existingBaggage) { + if (!hasSentryBaggageValues(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 5292018e31ef..960a00fae7d4 100644 --- a/packages/node-core/src/utils/outgoingHttpRequest.ts +++ b/packages/node-core/src/utils/outgoingHttpRequest.ts @@ -12,7 +12,7 @@ import { } from '@sentry/core'; import type { ClientRequest, IncomingMessage, RequestOptions } from 'http'; import { DEBUG_BUILD } from '../debug-build'; -import { mergeBaggageHeaders } from './baggage'; +import { hasSentryBaggageValues, mergeBaggageHeaders } from './baggage'; const LOG_PREFIX = '@sentry/instrumentation-http'; @@ -92,18 +92,22 @@ export function addTracePropagationHeadersToOutgoingRequest( } if (baggage) { - const newBaggage = mergeBaggageHeaders(request.getHeader('baggage'), baggage); - if (newBaggage) { - try { - request.setHeader('baggage', newBaggage); - DEBUG_BUILD && debug.log(LOG_PREFIX, 'Added baggage header to outgoing request'); - } catch (error) { - DEBUG_BUILD && - debug.error( - LOG_PREFIX, - 'Failed to add baggage header to outgoing request:', - isError(error) ? error.message : 'Unknown error', - ); + const existingBaggage = request.getHeader('baggage'); + // if existing baggage already has Sentry values, just skip it + if (!hasSentryBaggageValues(existingBaggage as string | string[] | undefined)) { + const newBaggage = mergeBaggageHeaders(existingBaggage, baggage); + if (newBaggage) { + try { + request.setHeader('baggage', newBaggage); + DEBUG_BUILD && debug.log(LOG_PREFIX, 'Added baggage header to outgoing request'); + } catch (error) { + DEBUG_BUILD && + debug.error( + LOG_PREFIX, + 'Failed to add baggage header to outgoing request:', + isError(error) ? error.message : 'Unknown error', + ); + } } } } diff --git a/packages/node-core/test/utils/baggage.test.ts b/packages/node-core/test/utils/baggage.test.ts new file mode 100644 index 000000000000..a9597d617dd8 --- /dev/null +++ b/packages/node-core/test/utils/baggage.test.ts @@ -0,0 +1,90 @@ +import { describe, expect, it } from 'vitest'; +import { hasSentryBaggageValues, mergeBaggageHeaders } from '../../src/utils/baggage'; + +describe('hasSentryBaggageValues', () => { + it('returns true for baggage with sentry- prefix', () => { + const baggage = 'sentry-environment=production,sentry-public_key=abc123'; + expect(hasSentryBaggageValues(baggage)).toBe(true); + }); + + it('returns true for baggage with mixed sentry and non-sentry values', () => { + const baggage = 'custom-key=value,sentry-trace_id=123,another-key=foo'; + expect(hasSentryBaggageValues(baggage)).toBe(true); + }); + + it('returns false for baggage without sentry values', () => { + const baggage = 'custom-key=value,another-key=foo'; + expect(hasSentryBaggageValues(baggage)).toBe(false); + }); + + it('returns false for empty baggage', () => { + expect(hasSentryBaggageValues('')).toBe(false); + }); + + it('returns false for undefined baggage', () => { + expect(hasSentryBaggageValues(undefined)).toBe(false); + }); + + it('handles array of baggage headers', () => { + const baggage = ['custom-key=value', 'sentry-environment=production']; + expect(hasSentryBaggageValues(baggage)).toBe(true); + }); + + it('returns false for array without sentry values', () => { + const baggage = ['custom-key=value', 'another-key=foo']; + expect(hasSentryBaggageValues(baggage)).toBe(false); + }); + + it('handles baggage with whitespace', () => { + const baggage = 'custom-key=value, sentry-environment=production , another-key=foo'; + expect(hasSentryBaggageValues(baggage)).toBe(true); + }); + + it('handles single sentry entry', () => { + const baggage = 'sentry-trace_id=abc123'; + expect(hasSentryBaggageValues(baggage)).toBe(true); + }); + + it('is case-sensitive (does not match Sentry- or SENTRY-)', () => { + const baggage = 'Sentry-environment=production,SENTRY-trace=123'; + expect(hasSentryBaggageValues(baggage)).toBe(false); + }); +}); + +describe('mergeBaggageHeaders', () => { + it('returns new baggage when existing is undefined', () => { + const result = mergeBaggageHeaders(undefined, 'sentry-environment=production'); + expect(result).toBe('sentry-environment=production'); + }); + + it('returns existing baggage when new baggage is invalid', () => { + const existing = 'custom-key=value'; + const result = mergeBaggageHeaders(existing, ''); + expect(result).toBe(existing); + }); + + 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('preserves existing entries when keys conflict', () => { + const existing = 'sentry-environment=staging'; + const newBaggage = 'sentry-environment=production'; + const result = mergeBaggageHeaders(existing, newBaggage); + expect(result).toBe('sentry-environment=staging'); + }); + + it('handles multiple entries with 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=staging'); + expect(result).toContain('sentry-trace_id=123'); + expect(result).not.toContain('sentry-environment=production'); + }); +});