diff --git a/packages/tracing/src/browser/request.ts b/packages/tracing/src/browser/request.ts index c3ebc1e00240..dce0d95044d5 100644 --- a/packages/tracing/src/browser/request.ts +++ b/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,7 +105,8 @@ 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, }; @@ -115,31 +114,52 @@ export const defaultRequestInstrumentationOptions: RequestInstrumentationOptions export function instrumentOutgoingRequests(_options?: Partial): 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 = {}; 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 */ diff --git a/packages/tracing/test/browser/request.test.ts b/packages/tracing/test/browser/request.test.ts index b69d9b1e5664..4e41e22a9a51 100644 --- a/packages/tracing/test/browser/request.test.ts +++ b/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); + }); + }); +});