diff --git a/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/customTargets/init.js b/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/customTargets/init.js new file mode 100644 index 000000000000..eaa53ad8b0a8 --- /dev/null +++ b/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/customTargets/init.js @@ -0,0 +1,10 @@ +import * as Sentry from '@sentry/browser'; +import { Integrations } from '@sentry/tracing'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [new Integrations.BrowserTracing({ tracePropagationTargets: ['http://example.com'] })], + tracesSampleRate: 1, +}); diff --git a/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/customTargets/subject.js b/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/customTargets/subject.js new file mode 100644 index 000000000000..f62499b1e9c5 --- /dev/null +++ b/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/customTargets/subject.js @@ -0,0 +1 @@ +fetch('http://example.com/0').then(fetch('http://example.com/1').then(fetch('http://example.com/2'))); diff --git a/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/customTargets/test.ts b/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/customTargets/test.ts new file mode 100644 index 000000000000..5281b549454c --- /dev/null +++ b/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/customTargets/test.ts @@ -0,0 +1,27 @@ +import { expect, Request } from '@playwright/test'; + +import { sentryTest } from '../../../../../utils/fixtures'; + +sentryTest( + 'should attach `sentry-trace` and `baggage` header to request matching tracePropagationTargets', + async ({ getLocalTestPath, page }) => { + const url = await getLocalTestPath({ testDir: __dirname }); + + const requests = ( + await Promise.all([ + page.goto(url), + Promise.all([0, 1, 2].map(idx => page.waitForRequest(`http://example.com/${idx}`))), + ]) + )[1]; + + expect(requests).toHaveLength(3); + + requests?.forEach(async (request: Request) => { + const requestHeaders = await request.allHeaders(); + expect(requestHeaders).toMatchObject({ + 'sentry-trace': expect.any(String), + baggage: expect.any(String), + }); + }); + }, +); diff --git a/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/customTargetsAndOrigins/init.js b/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/customTargetsAndOrigins/init.js new file mode 100644 index 000000000000..f98812f56e8f --- /dev/null +++ b/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/customTargetsAndOrigins/init.js @@ -0,0 +1,12 @@ +import * as Sentry from '@sentry/browser'; +import { Integrations } from '@sentry/tracing'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [ + new Integrations.BrowserTracing({ tracePropagationTargets: [], tracingOrigins: ['http://example.com'] }), + ], + tracesSampleRate: 1, +}); diff --git a/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/customTargetsAndOrigins/subject.js b/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/customTargetsAndOrigins/subject.js new file mode 100644 index 000000000000..f62499b1e9c5 --- /dev/null +++ b/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/customTargetsAndOrigins/subject.js @@ -0,0 +1 @@ +fetch('http://example.com/0').then(fetch('http://example.com/1').then(fetch('http://example.com/2'))); diff --git a/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/customTargetsAndOrigins/test.ts b/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/customTargetsAndOrigins/test.ts new file mode 100644 index 000000000000..e39093012d9e --- /dev/null +++ b/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/customTargetsAndOrigins/test.ts @@ -0,0 +1,27 @@ +import { expect, Request } from '@playwright/test'; + +import { sentryTest } from '../../../../../utils/fixtures'; + +sentryTest( + '[pre-v8] should prefer custom tracePropagationTargets over tracingOrigins', + async ({ getLocalTestPath, page }) => { + const url = await getLocalTestPath({ testDir: __dirname }); + + const requests = ( + await Promise.all([ + page.goto(url), + Promise.all([0, 1, 2].map(idx => page.waitForRequest(`http://example.com/${idx}`))), + ]) + )[1]; + + expect(requests).toHaveLength(3); + + requests?.forEach(async (request: Request) => { + const requestHeaders = await request.allHeaders(); + expect(requestHeaders).not.toMatchObject({ + 'sentry-trace': expect.any(String), + baggage: expect.any(String), + }); + }); + }, +); diff --git a/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/customTracingOrigins/init.js b/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/customTracingOrigins/init.js new file mode 100644 index 000000000000..505fab06b330 --- /dev/null +++ b/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/customTracingOrigins/init.js @@ -0,0 +1,10 @@ +import * as Sentry from '@sentry/browser'; +import { Integrations } from '@sentry/tracing'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [new Integrations.BrowserTracing({ tracingOrigins: ['http://example.com'] })], + tracesSampleRate: 1, +}); diff --git a/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/customTracingOrigins/subject.js b/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/customTracingOrigins/subject.js new file mode 100644 index 000000000000..f62499b1e9c5 --- /dev/null +++ b/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/customTracingOrigins/subject.js @@ -0,0 +1 @@ +fetch('http://example.com/0').then(fetch('http://example.com/1').then(fetch('http://example.com/2'))); diff --git a/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/customTracingOrigins/test.ts b/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/customTracingOrigins/test.ts new file mode 100644 index 000000000000..bc54b6bb0687 --- /dev/null +++ b/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/customTracingOrigins/test.ts @@ -0,0 +1,27 @@ +import { expect, Request } from '@playwright/test'; + +import { sentryTest } from '../../../../../utils/fixtures'; + +sentryTest( + '[pre-v8] should attach `sentry-trace` and `baggage` header to request matching tracingOrigins', + async ({ getLocalTestPath, page }) => { + const url = await getLocalTestPath({ testDir: __dirname }); + + const requests = ( + await Promise.all([ + page.goto(url), + Promise.all([0, 1, 2].map(idx => page.waitForRequest(`http://example.com/${idx}`))), + ]) + )[1]; + + expect(requests).toHaveLength(3); + + requests?.forEach(async (request: Request) => { + const requestHeaders = await request.allHeaders(); + expect(requestHeaders).toMatchObject({ + 'sentry-trace': expect.any(String), + baggage: expect.any(String), + }); + }); + }, +); diff --git a/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/defaultTargetsMatch/init.js b/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/defaultTargetsMatch/init.js new file mode 100644 index 000000000000..ce4e0c4ad7f7 --- /dev/null +++ b/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/defaultTargetsMatch/init.js @@ -0,0 +1,10 @@ +import * as Sentry from '@sentry/browser'; +import { Integrations } from '@sentry/tracing'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [new Integrations.BrowserTracing()], + tracesSampleRate: 1, +}); diff --git a/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/defaultTargetsMatch/subject.js b/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/defaultTargetsMatch/subject.js new file mode 100644 index 000000000000..4e9cf0d01004 --- /dev/null +++ b/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/defaultTargetsMatch/subject.js @@ -0,0 +1 @@ +fetch('http://localhost:4200/0').then(fetch('http://localhost:4200/1').then(fetch('http://localhost:4200/2'))); diff --git a/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/defaultTargetsMatch/test.ts b/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/defaultTargetsMatch/test.ts new file mode 100644 index 000000000000..3fe23f53e37c --- /dev/null +++ b/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/defaultTargetsMatch/test.ts @@ -0,0 +1,27 @@ +import { expect, Request } from '@playwright/test'; + +import { sentryTest } from '../../../../../utils/fixtures'; + +sentryTest( + 'should attach `sentry-trace` and `baggage` header to request matching default tracePropagationTargets', + async ({ getLocalTestPath, page }) => { + const url = await getLocalTestPath({ testDir: __dirname }); + + const requests = ( + await Promise.all([ + page.goto(url), + Promise.all([0, 1, 2].map(idx => page.waitForRequest(`http://localhost:4200/${idx}`))), + ]) + )[1]; + + expect(requests).toHaveLength(3); + + requests?.forEach(async (request: Request) => { + const requestHeaders = await request.allHeaders(); + expect(requestHeaders).toMatchObject({ + 'sentry-trace': expect.any(String), + baggage: expect.any(String), + }); + }); + }, +); diff --git a/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/defaultTargetsNoMatch/init.js b/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/defaultTargetsNoMatch/init.js new file mode 100644 index 000000000000..ce4e0c4ad7f7 --- /dev/null +++ b/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/defaultTargetsNoMatch/init.js @@ -0,0 +1,10 @@ +import * as Sentry from '@sentry/browser'; +import { Integrations } from '@sentry/tracing'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [new Integrations.BrowserTracing()], + tracesSampleRate: 1, +}); diff --git a/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/defaultTargetsNoMatch/subject.js b/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/defaultTargetsNoMatch/subject.js new file mode 100644 index 000000000000..f62499b1e9c5 --- /dev/null +++ b/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/defaultTargetsNoMatch/subject.js @@ -0,0 +1 @@ +fetch('http://example.com/0').then(fetch('http://example.com/1').then(fetch('http://example.com/2'))); diff --git a/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/defaultTargetsNoMatch/test.ts b/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/defaultTargetsNoMatch/test.ts new file mode 100644 index 000000000000..50bce1fdc8fa --- /dev/null +++ b/packages/integration-tests/suites/tracing/browsertracing/tracePropagationTargets/defaultTargetsNoMatch/test.ts @@ -0,0 +1,27 @@ +import { expect, Request } from '@playwright/test'; + +import { sentryTest } from '../../../../../utils/fixtures'; + +sentryTest( + 'should not attach `sentry-trace` and `baggage` header to request not matching default tracePropagationTargets', + async ({ getLocalTestPath, page }) => { + const url = await getLocalTestPath({ testDir: __dirname }); + + const requests = ( + await Promise.all([ + page.goto(url), + Promise.all([0, 1, 2].map(idx => page.waitForRequest(`http://example.com/${idx}`))), + ]) + )[1]; + + expect(requests).toHaveLength(3); + + requests?.forEach(async (request: Request) => { + const requestHeaders = await request.allHeaders(); + expect(requestHeaders).not.toMatchObject({ + 'sentry-trace': expect.any(String), + baggage: expect.any(String), + }); + }); + }, +); diff --git a/packages/tracing/src/browser/browsertracing.ts b/packages/tracing/src/browser/browsertracing.ts index 113f6963eee4..ea0385be404a 100644 --- a/packages/tracing/src/browser/browsertracing.ts +++ b/packages/tracing/src/browser/browsertracing.ts @@ -112,7 +112,7 @@ export interface BrowserTracingOptions extends RequestInstrumentationOptions { ): void; } -const DEFAULT_BROWSER_TRACING_OPTIONS = { +const DEFAULT_BROWSER_TRACING_OPTIONS: BrowserTracingOptions = { idleTimeout: DEFAULT_IDLE_TIMEOUT, finalTimeout: DEFAULT_FINAL_TIMEOUT, heartbeatInterval: DEFAULT_HEARTBEAT_INTERVAL, @@ -153,6 +153,15 @@ export class BrowserTracing implements Integration { ..._options, }; + // TODO (v8): remove this block after tracingOrigins is removed + // Set tracePropagationTargets to tracingOrigins if specified by the user + // In case both are specified, tracePropagationTargets takes precedence + // eslint-disable-next-line deprecation/deprecation + if (_options && !_options.tracePropagationTargets && _options.tracingOrigins) { + // eslint-disable-next-line deprecation/deprecation + this.options.tracePropagationTargets = _options.tracingOrigins; + } + const { _metricOptions } = this.options; startTrackingWebVitals(_metricOptions && _metricOptions._reportAllChanges); if (this.options._experiments?.enableLongTask) { @@ -174,8 +183,7 @@ export class BrowserTracing implements Integration { markBackgroundTransactions, traceFetch, traceXHR, - // eslint-disable-next-line deprecation/deprecation - tracingOrigins, + tracePropagationTargets, shouldCreateSpanForRequest, } = this.options; @@ -189,7 +197,12 @@ export class BrowserTracing implements Integration { registerBackgroundTabDetection(); } - instrumentOutgoingRequests({ traceFetch, traceXHR, tracingOrigins, shouldCreateSpanForRequest }); + instrumentOutgoingRequests({ + traceFetch, + traceXHR, + tracePropagationTargets, + shouldCreateSpanForRequest, + }); } /** Create routing idle transaction. */ diff --git a/packages/tracing/src/browser/request.ts b/packages/tracing/src/browser/request.ts index dce0d95044d5..c065318ae360 100644 --- a/packages/tracing/src/browser/request.ts +++ b/packages/tracing/src/browser/request.ts @@ -113,7 +113,7 @@ export const defaultRequestInstrumentationOptions: RequestInstrumentationOptions /** Registers span creators for xhr and fetch requests */ export function instrumentOutgoingRequests(_options?: Partial): void { // eslint-disable-next-line deprecation/deprecation - const { traceFetch, traceXHR, tracingOrigins, tracePropagationTargets, shouldCreateSpanForRequest } = { + const { traceFetch, traceXHR, tracePropagationTargets, tracingOrigins, shouldCreateSpanForRequest } = { traceFetch: defaultRequestInstrumentationOptions.traceFetch, traceXHR: defaultRequestInstrumentationOptions.traceXHR, ..._options, @@ -122,8 +122,11 @@ export function instrumentOutgoingRequests(_options?: Partial true; + // TODO(v8) Remove tracingOrigins here + // The only reason we're passing it in here is because this instrumentOutgoingRequests function is publicly exported + // and we don't want to break the API. We can remove it in v8. const shouldAttachHeadersWithTargets = (url: string): boolean => - shouldAttachHeaders(url, tracingOrigins, tracePropagationTargets); + shouldAttachHeaders(url, tracePropagationTargets || tracingOrigins); const spans: Record = {}; @@ -144,20 +147,9 @@ export function instrumentOutgoingRequests(_options?: Partial { jest.mock('../../src/browser/metrics'); +const instrumentOutgoingRequestsMock = jest.fn(); +jest.mock('./../../src/browser/request', () => { + const actual = jest.requireActual('./../../src/browser/request'); + return { + ...actual, + instrumentOutgoingRequests: (options: Partial) => instrumentOutgoingRequestsMock(options), + }; +}); + beforeAll(() => { const dom = new JSDOM(); // @ts-ignore need to override global document @@ -128,6 +137,7 @@ describe('BrowserTracing', () => { expect(transaction.endTimestamp).toBe(span.endTimestamp); }); + // TODO (v8): remove these tests describe('tracingOrigins', () => { it('sets tracing origins if provided and does not warn', () => { const sampleTracingOrigins = ['something']; @@ -152,6 +162,43 @@ describe('BrowserTracing', () => { }); }); + describe('tracePropagationTargets', () => { + it('sets tracePropagationTargets if provided', () => { + const sampleTracePropagationTargets = ['something']; + const inst = createBrowserTracing(true, { + routingInstrumentation: customInstrumentRouting, + tracePropagationTargets: sampleTracePropagationTargets, + }); + + expect(inst.options.tracePropagationTargets).toEqual(sampleTracePropagationTargets); + }); + + it('sets tracePropagationTargets to an empty array and does not warn', () => { + const sampleTracePropagationTargets: string[] = []; + const inst = createBrowserTracing(true, { + routingInstrumentation: customInstrumentRouting, + tracePropagationTargets: sampleTracePropagationTargets, + }); + + expect(inst.options.tracePropagationTargets).toEqual(sampleTracePropagationTargets); + }); + + it('correctly passes tracePropagationTargets to `instrumentOutgoingRequests` in `setupOnce`', () => { + jest.clearAllMocks(); + const sampleTracePropagationTargets = ['something']; + createBrowserTracing(true, { + routingInstrumentation: customInstrumentRouting, + tracePropagationTargets: sampleTracePropagationTargets, + }); + + expect(instrumentOutgoingRequestsMock).toHaveBeenCalledWith({ + traceFetch: true, + traceXHR: true, + tracePropagationTargets: ['something'], + }); + }); + }); + describe('beforeNavigate', () => { it('is called on transaction creation', () => { const mockBeforeNavigation = jest.fn().mockReturnValue({ name: 'here/is/my/path' }); diff --git a/packages/tracing/test/browser/request.test.ts b/packages/tracing/test/browser/request.test.ts index 4e41e22a9a51..732d6ea9b4f8 100644 --- a/packages/tracing/test/browser/request.test.ts +++ b/packages/tracing/test/browser/request.test.ts @@ -391,33 +391,18 @@ describe('callbacks', () => { }); }); -// TODO (v8): Adapt these tests once we remove `tracingOrigins` -describe('[pre-v8]: shouldAttachHeaders', () => { - describe('prefer `tracePropagationTargets` over `tracingOrigins`', () => { +describe('shouldAttachHeaders', () => { + describe('should prefer `tracePropagationTargets` over defaults', () => { it('should return `true` if the url matches the new tracePropagationTargets', () => { - expect(shouldAttachHeaders('http://example.com', ['example.com'], undefined)).toBe(true); + expect(shouldAttachHeaders('http://example.com', ['example.com'])).toBe(true); }); it('should return `false` if tracePropagationTargets array is empty', () => { - expect(shouldAttachHeaders('http://localhost:3000/test', [], ['localhost'])).toBe(false); + expect(shouldAttachHeaders('http://localhost:3000/test', [])).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); + expect(shouldAttachHeaders('http://localhost:3000/test', ['example.com'])).toBe(false); }); }); @@ -428,11 +413,11 @@ describe('[pre-v8]: shouldAttachHeaders', () => { '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); + expect(shouldAttachHeaders(url, 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); + expect(shouldAttachHeaders(url, undefined)).toBe(false); }); }); });