fix(tracing): Make shouldAttachHeaders not fall back to default values

packages/tracing/src/browser/request.ts

@@ -10,8 +10,6 @@ import {
 
 import { getActiveTransaction, hasTracingEnabled } from '../utils';
 
-// TODO (v8): Remove `tracingOrigins`
-export const DEFAULT_TRACING_ORIGINS = ['localhost', /^\//];
 export const DEFAULT_TRACE_PROPAGATION_TARGETS = ['localhost', /^\//];
 
 /** Options for Request Instrumentation */
@@ -107,39 +105,61 @@ type PolymorphicRequestHeaders =
 export const defaultRequestInstrumentationOptions: RequestInstrumentationOptions = {
   traceFetch: true,
   traceXHR: true,
-  tracingOrigins: DEFAULT_TRACING_ORIGINS,
+  // TODO (v8): Remove this property
+  tracingOrigins: DEFAULT_TRACE_PROPAGATION_TARGETS,
   tracePropagationTargets: DEFAULT_TRACE_PROPAGATION_TARGETS,
 };
 
 /** Registers span creators for xhr and fetch requests  */
 export function instrumentOutgoingRequests(_options?: Partial<RequestInstrumentationOptions>): void {
   // eslint-disable-next-line deprecation/deprecation
   const { traceFetch, traceXHR, tracingOrigins, tracePropagationTargets, shouldCreateSpanForRequest } = {
-    ...defaultRequestInstrumentationOptions,
+    traceFetch: defaultRequestInstrumentationOptions.traceFetch,
+    traceXHR: defaultRequestInstrumentationOptions.traceXHR,
     ..._options,
   };
 
   const shouldCreateSpan =
     typeof shouldCreateSpanForRequest === 'function' ? shouldCreateSpanForRequest : (_: string) => true;
 
-  const shouldAttachHeaders = (url: string): boolean =>
-    stringMatchesSomePattern(url, tracingOrigins) || stringMatchesSomePattern(url, tracePropagationTargets);
+  const shouldAttachHeadersWithTargets = (url: string): boolean =>
+    shouldAttachHeaders(url, tracingOrigins, tracePropagationTargets);
 
   const spans: Record<string, Span> = {};
 
   if (traceFetch) {
     addInstrumentationHandler('fetch', (handlerData: FetchData) => {
-      fetchCallback(handlerData, shouldCreateSpan, shouldAttachHeaders, spans);
+      fetchCallback(handlerData, shouldCreateSpan, shouldAttachHeadersWithTargets, spans);
     });
   }
 
   if (traceXHR) {
     addInstrumentationHandler('xhr', (handlerData: XHRData) => {
-      xhrCallback(handlerData, shouldCreateSpan, shouldAttachHeaders, spans);
+      xhrCallback(handlerData, shouldCreateSpan, shouldAttachHeadersWithTargets, spans);
     });
   }
 }
 
+/**
+ * A function that determines whether to attach tracing headers to a request.
+ * This was extracted from `instrumentOutgoingRequests` to make it easier to test shouldAttachHeaders.
+ * We only export this fuction for testing purposes.
+ *
+ * TODO (v8): Remove `tracingOrigins` which should drastically simplify this function.
+ */
+export function shouldAttachHeaders(
+  url: string,
+  tracePropagationTargets: (string | RegExp)[] | undefined,
+  tracingOrigins: (string | RegExp)[] | undefined,
+): boolean {
+  // TODO (v8): Replace the entire code below with this one-liner:
+  // return stringMatchesSomePattern(url, tracePropagationTargets || DEFAULT_TRACE_PROPAGATION_TARGETS);
+  if (tracePropagationTargets || tracingOrigins) {
+    return stringMatchesSomePattern(url, tracePropagationTargets || tracingOrigins);
+  }
+  return stringMatchesSomePattern(url, DEFAULT_TRACE_PROPAGATION_TARGETS);
+}
+
 /**
  * Create and track fetch request spans
  */

packages/tracing/test/browser/request.test.ts

@@ -3,7 +3,14 @@ import { Hub, makeMain } from '@sentry/core';
 import * as utils from '@sentry/utils';
 
 import { Span, spanStatusfromHttpCode, Transaction } from '../../src';
-import { fetchCallback, FetchData, instrumentOutgoingRequests, xhrCallback, XHRData } from '../../src/browser/request';
+import {
+  fetchCallback,
+  FetchData,
+  instrumentOutgoingRequests,
+  shouldAttachHeaders,
+  xhrCallback,
+  XHRData,
+} from '../../src/browser/request';
 import { addExtensionMethods } from '../../src/hubextensions';
 import * as tracingUtils from '../../src/utils';
 import { getDefaultBrowserClientOptions } from '../testutils';
@@ -383,3 +390,49 @@ describe('callbacks', () => {
     });
   });
 });
+
+// TODO (v8): Adapt these tests once we remove `tracingOrigins`
+describe('[pre-v8]: shouldAttachHeaders', () => {
+  describe('prefer `tracePropagationTargets` over `tracingOrigins`', () => {
+    it('should return `true` if the url matches the new tracePropagationTargets', () => {
+      expect(shouldAttachHeaders('http://example.com', ['example.com'], undefined)).toBe(true);
+    });
+
+    it('should return `false` if tracePropagationTargets array is empty', () => {
+      expect(shouldAttachHeaders('http://localhost:3000/test', [], ['localhost'])).toBe(false);
+    });
+
+    it("should return `false` if tracePropagationTargets array doesn't match", () => {
+      expect(shouldAttachHeaders('http://localhost:3000/test', ['example.com'], ['localhost'])).toBe(false);
+    });
+  });
+
+  describe('tracingOrigins backwards compatibility (tracePropagationTargets not defined)', () => {
+    it('should return `true` if the url matches tracingOrigns', () => {
+      expect(shouldAttachHeaders('http://example.com', undefined, ['example.com'])).toBe(true);
+    });
+
+    it('should return `false` if tracePropagationTargets array is empty', () => {
+      expect(shouldAttachHeaders('http://localhost:3000/test', undefined, [])).toBe(false);
+    });
+
+    it("should return `false` if tracePropagationTargets array doesn't match", () => {
+      expect(shouldAttachHeaders('http://localhost:3000/test', undefined, ['example.com'])).toBe(false);
+    });
+  });
+
+  describe('should fall back to defaults if no options are specified', () => {
+    it.each([
+      '/api/test',
+      'http://localhost:3000/test',
+      'http://somewhere.com/test/localhost/123',
+      'http://somewhere.com/test?url=localhost:3000&test=123',
+    ])('return `true` for urls matching defaults (%s)', url => {
+      expect(shouldAttachHeaders(url, undefined, undefined)).toBe(true);
+    });
+
+    it.each(['notmydoman/api/test', 'example.com'])('return `false` for urls not matching defaults (%s)', url => {
+      expect(shouldAttachHeaders(url, undefined, undefined)).toBe(false);
+    });
+  });
+});