fix(node): Deduplicate sentry-trace and baggage headers on outgoing requests (#19960)

This patch fixes a bunch of closely related issues with our node fetch and
http integrations for outgoing request header propagation.

### Summary: 
- We now dedupe sentry-trace and baggage headers more aggressively,
resolving multiple scenarios where duplicated sentry headers were
attached to outgoing requests
- We now always prefer the first sentry tracing headers pair set onto a
request. This allows users to set custom sentry headers (for whatever
reason) and ensures our instrumentation doesn't overwrite itself.
- We no longer mix individual `sentry-` baggage entries when merging two
headers where both contain `sentry-` entries. We only take one of the
two and delete the other.

See PR for further details!

closes #19158

Author: Lms24

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

@@ -39,9 +39,10 @@ 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);
+    expect(headers['baggage']).toBe('sentry-release=4.2.0');
   });
 }
 

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();
@@ -33,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',
   ]);
 });
@@ -46,6 +46,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 +68,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',
   ]);

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<string, string | string[] | undefined>): 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<string, string | string[] | undefined>): 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);
+}

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,
+});

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

@@ -0,0 +1,45 @@
+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/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/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/http-custom-headers`);
+    const req = http.request(
+      {
+        hostname: url.hostname,
+        port: url.port,
+        path: url.pathname,
+        method: 'GET',
+        headers: {
+          ...traceData,
+          'x-tracedata-sentry-trace': traceData['sentry-trace'],
+          'x-tracedata-baggage': traceData.baggage,
+        },
+      },
+      res => {
+        res.on('data', () => {});
+        res.on('end', () => resolve());
+      },
+    );
+    req.on('error', reject);
+    req.end();
+  });
+
+  Sentry.captureException(new Error('done'));
+}
+
+run();

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();
+    });
+  });
+});

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,
+});

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

@@ -0,0 +1,45 @@
+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/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/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/http-custom-headers`);
+    const req = http.request(
+      {
+        hostname: url.hostname,
+        port: url.port,
+        path: url.pathname,
+        method: 'GET',
+        headers: {
+          ...traceData,
+          'x-tracedata-sentry-trace': traceData['sentry-trace'],
+          'x-tracedata-baggage': traceData.baggage,
+        },
+      },
+      res => {
+        res.on('data', () => {});
+        res.on('end', () => resolve());
+      },
+    );
+    req.on('error', reject);
+    req.end();
+  });
+
+  Sentry.captureException(new Error('done'));
+}
+
+run();

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

@@ -0,0 +1,46 @@
+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']).not.toContain(',');
+          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}(-[01])?$/));
+          expectNoDuplicateSentryBaggageKeys(headers['baggage']);
+          expectConsistentTraceId(headers);
+        })
+        .get('/api/http-custom-headers', headers => {
+          // http.request with manual getTraceData() headers
+          expect(headers['sentry-trace']).not.toContain(',');
+          expectNoDuplicateSentryBaggageKeys(headers['baggage']);
+          expectConsistentTraceId(headers);
+          expectUserSetTraceId(headers);
+        })
+        .start();
+
+      await createRunner()
+        .withEnv({ SERVER_URL })
+        .expect({
+          event: {
+            exception: {
+              values: [{ type: 'Error', value: 'done' }],
+            },
+          },
+        })
+        .start()
+        .completed();
+      closeTestServer();
+    });
+  });
+});

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,
+});