fix(core): Handle manually set in sentry tracing headers in fetch calls

Author: Lms24

dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-custom-sentry-headers/init.js

@@ -0,0 +1,10 @@
+import * as Sentry from '@sentry/browser';
+
+window.Sentry = Sentry;
+
+Sentry.init({
+  dsn: 'https://[email protected]/1337',
+  integrations: [Sentry.browserTracingIntegration()],
+  tracePropagationTargets: ['http://sentry-test-site.example'],
+  tracesSampleRate: 1,
+});

dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-custom-sentry-headers/subject.js

@@ -0,0 +1,3 @@
+fetch('http://sentry-test-site.example/api/test/', {
+  headers: { 'sentry-trace': 'abc-123-1', baggage: 'sentry-trace_id=abc' },
+});

dev-packages/browser-integration-tests/suites/tracing/request/fetch-with-custom-sentry-headers/test.ts

@@ -0,0 +1,22 @@
+import { expect } from '@playwright/test';
+import { sentryTest } from '../../../../utils/fixtures';
+import { shouldSkipTracingTest } from '../../../../utils/helpers';
+
+sentryTest("instrumentation doesn't override manually added sentry headers", async ({ getLocalTestUrl, page }) => {
+  if (shouldSkipTracingTest()) {
+    sentryTest.skip();
+  }
+
+  const requestPromise = page.waitForRequest('http://sentry-test-site.example/api/test/');
+
+  const url = await getLocalTestUrl({ testDir: __dirname });
+
+  await page.goto(url);
+
+  const request = await requestPromise;
+
+  const headers = await request.allHeaders();
+
+  expect(headers['sentry-trace']).toBe('abc-123-1');
+  expect(headers.baggage).toBe('sentry-trace_id=abc');
+});

packages/core/src/fetch.ts

@@ -103,8 +103,10 @@ export function instrumentFetchRequest(
 
 /**
  * Adds sentry-trace and baggage headers to the various forms of fetch headers.
+ * exported only for testing purposes
  */
-function _addTracingHeadersToFetchRequest(
+// eslint-disable-next-line complexity -- yup it's this complicated :(
+export function _addTracingHeadersToFetchRequest(
   request: string | Request,
   fetchOptionsObj: {
     headers?:
@@ -124,78 +126,74 @@ function _addTracingHeadersToFetchRequest(
     return undefined;
   }
 
-  const headers = fetchOptionsObj.headers || (isRequest(request) ? request.headers : undefined);
+  const originalHeaders = fetchOptionsObj.headers || (isRequest(request) ? request.headers : undefined);
 
-  if (!headers) {
+  if (!originalHeaders) {
     return { ...traceHeaders };
-  } else if (isHeaders(headers)) {
-    const newHeaders = new Headers(headers);
-    newHeaders.set('sentry-trace', sentryTrace);
+  } else if (isHeaders(originalHeaders)) {
+    const newHeaders = new Headers(originalHeaders);
+
+    // We don't want to override manually added sentry headers
+    if (!newHeaders.get('sentry-trace')) {
+      newHeaders.set('sentry-trace', sentryTrace);
+    }
 
     if (baggage) {
       const prevBaggageHeader = newHeaders.get('baggage');
-      if (prevBaggageHeader) {
-        const prevHeaderStrippedFromSentryBaggage = stripBaggageHeaderOfSentryBaggageValues(prevBaggageHeader);
-        newHeaders.set(
-          'baggage',
-          // If there are non-sentry entries (i.e. if the stripped string is non-empty/truthy) combine the stripped header and sentry baggage header
-          // otherwise just set the sentry baggage header
-          prevHeaderStrippedFromSentryBaggage ? `${prevHeaderStrippedFromSentryBaggage},${baggage}` : baggage,
-        );
-      } else {
+
+      // 3 cases:
+      // 1. No previous baggage header -> add baggage
+      // 2. Previous baggage header has no sentry baggage values -> add our baggage
+      // 3. Previous baggage header has sentry baggage values -> do nothing (might have been added manually by users)
+      if (!prevBaggageHeader) {
         newHeaders.set('baggage', baggage);
+      } else if (!baggageHeaderHasSentryBaggageValues(prevBaggageHeader)) {
+        newHeaders.set('baggage', `${prevBaggageHeader},${baggage}`);
       }
     }
 
     return newHeaders;
-  } else if (Array.isArray(headers)) {
-    const newHeaders = [
-      ...headers
-        // Remove any existing sentry-trace headers
-        .filter(header => {
-          return !(Array.isArray(header) && header[0] === 'sentry-trace');
-        })
-        // Get rid of previous sentry baggage values in baggage header
-        .map(header => {
-          if (Array.isArray(header) && header[0] === 'baggage' && typeof header[1] === 'string') {
-            const [headerName, headerValue, ...rest] = header;
-            return [headerName, stripBaggageHeaderOfSentryBaggageValues(headerValue), ...rest];
-          } else {
-            return header;
-          }
-        }),
-      // Attach the new sentry-trace header
-      ['sentry-trace', sentryTrace],
-    ];
+  } else if (Array.isArray(originalHeaders)) {
+    const newHeaders = [...originalHeaders];
 
-    if (baggage) {
+    if (!originalHeaders.find(header => header[0] === 'sentry-trace')) {
+      newHeaders.push(['sentry-trace', sentryTrace]);
+    }
+
+    const prevBaggageHeaderWithSentryValues = originalHeaders.find(
+      header => header[0] === 'baggage' && baggageHeaderHasSentryBaggageValues(header[1]),
+    );
+
+    if (baggage && !prevBaggageHeaderWithSentryValues) {
       // If there are multiple entries with the same key, the browser will merge the values into a single request header.
       // Its therefore safe to simply push a "baggage" entry, even though there might already be another baggage header.
       newHeaders.push(['baggage', baggage]);
     }
 
     return newHeaders as PolymorphicRequestHeaders;
   } else {
-    const existingBaggageHeader = 'baggage' in headers ? headers.baggage : undefined;
-    let newBaggageHeaders: string[] = [];
-
-    if (Array.isArray(existingBaggageHeader)) {
-      newBaggageHeaders = existingBaggageHeader
-        .map(headerItem =>
-          typeof headerItem === 'string' ? stripBaggageHeaderOfSentryBaggageValues(headerItem) : headerItem,
-        )
-        .filter(headerItem => headerItem === '');
-    } else if (existingBaggageHeader) {
-      newBaggageHeaders.push(stripBaggageHeaderOfSentryBaggageValues(existingBaggageHeader));
-    }
-
-    if (baggage) {
+    const existingSentryTraceHeader = 'sentry-trace' in originalHeaders ? originalHeaders['sentry-trace'] : undefined;
+
+    const existingBaggageHeader = 'baggage' in originalHeaders ? originalHeaders.baggage : undefined;
+    const newBaggageHeaders: string[] = existingBaggageHeader
+      ? Array.isArray(existingBaggageHeader)
+        ? [...existingBaggageHeader]
+        : [existingBaggageHeader]
+      : [];
+
+    const prevBaggageHeaderWithSentryValues =
+      existingBaggageHeader &&
+      (Array.isArray(existingBaggageHeader)
+        ? existingBaggageHeader.find(headerItem => baggageHeaderHasSentryBaggageValues(headerItem))
+        : baggageHeaderHasSentryBaggageValues(existingBaggageHeader));
+
+    if (baggage && !prevBaggageHeaderWithSentryValues) {
       newBaggageHeaders.push(baggage);
     }
 
     return {
-      ...(headers as Exclude<typeof headers, Headers>),
-      'sentry-trace': sentryTrace,
+      ...(originalHeaders as Exclude<typeof originalHeaders, Headers>),
+      'sentry-trace': (existingSentryTraceHeader as string | undefined) ?? sentryTrace,
       baggage: newBaggageHeaders.length > 0 ? newBaggageHeaders.join(',') : undefined,
     };
   }
@@ -219,14 +217,10 @@ function endSpan(span: Span, handlerData: HandlerDataFetch): void {
   span.end();
 }
 
-function stripBaggageHeaderOfSentryBaggageValues(baggageHeader: string): string {
-  return (
-    baggageHeader
-      .split(',')
-      // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-      .filter(baggageEntry => !baggageEntry.split('=')[0]!.startsWith(SENTRY_BAGGAGE_KEY_PREFIX))
-      .join(',')
-  );
+function baggageHeaderHasSentryBaggageValues(baggageHeader: string): boolean {
+  return baggageHeader
+    .split(',')
+    .some(baggageEntry => baggageEntry.split('=')[0]?.startsWith(SENTRY_BAGGAGE_KEY_PREFIX));
 }
 
 function isHeaders(headers: unknown): headers is Headers {