feat: Update default trace propagation targets logic in the browser (#10621)

Author: Luca Forstner

MIGRATION.md

@@ -1,5 +1,33 @@
 # Upgrading from 7.x to 8.x
 
+## Updated behaviour of `tracePropagationTargets` in the browser (HTTP tracing headers & CORS)
+
+We updated the behaviour of the SDKs when no `tracePropagationTargets` option was defined. As a reminder, you can
+provide a list of strings or RegExes that will be matched against URLs to tell the SDK, to which outgoing requests
+tracing HTTP headers should be attached to. These tracing headers are used for distributed tracing.
+
+Previously, on the browser, when `tracePropagationTargets` were not defined, they defaulted to the following:
+`['localhost', /^\/(?!\/)/]`. This meant that all request targets to that had "localhost" in the URL, or started with a
+`/` were equipped with tracing headers. This default was chosen to prevent CORS errors in your browser applications.
+However, this default had a few flaws.
+
+Going forward, when the `tracePropagationTargets` option is not set, tracing headers will be attached to all outgoing
+requests on the same origin. For example, if you're on `https://example.com/` and you send a request to
+`https://example.com/api`, the request will be traced (ie. will have trace headers attached). Requests to
+`https://api.example.com/` will not, because it is on a different origin. The same goes for all applications running on
+`localhost`.
+
+When you provide a `tracePropagationTargets` option, all of the entries you defined will now be matched be matched
+against the full URL of the outgoing request. Previously, it was only matched against what you called request APIs with.
+For example, if you made a request like `fetch("/api/posts")`, the provided `tracePropagationTargets` were only compared
+against `"/api/posts"`. Going forward they will be matched against the entire URL, for example, if you were on the page
+`https://example.com/` and you made the same request, it would be matched against `"https://example.com/api/posts"`.
+
+But that is not all. Because it would be annoying having to create matchers for the entire URL, if the request is a
+same-origin request, we also match the `tracePropagationTargets` against the resolved `pathname` of the request.
+Meaning, a matcher like `/^\/api/` would match a request call like `fetch('/api/posts')`, or
+`fetch('https://same-origin.com/api/posts')` but not `fetch('https://different-origin.com/api/posts')`.
+
 ## Removal of the `tracingOrigins` option
 
 After its deprecation in v7 the `tracingOrigins` option is now removed in favor of the `tracePropagationTargets` option.

dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/tracePropagationTargets/defaultTargetsMatch/subject.js

@@ -1 +1 @@
-fetch('http://localhost:4200/0').then(fetch('http://localhost:4200/1').then(fetch('http://localhost:4200/2')));
+fetch(`/0`).then(fetch('/1').then(fetch('/2')));

dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/tracePropagationTargets/defaultTargetsMatch/test.ts

@@ -4,19 +4,16 @@ import { sentryTest } from '../../../../../utils/fixtures';
 import { shouldSkipTracingTest } from '../../../../../utils/helpers';
 
 sentryTest(
-  'should attach `sentry-trace` and `baggage` header to request matching default tracePropagationTargets',
-  async ({ getLocalTestPath, page }) => {
+  'should attach `sentry-trace` and `baggage` header to same-origin requests when no tracePropagationTargets are defined',
+  async ({ getLocalTestUrl, page }) => {
     if (shouldSkipTracingTest()) {
       sentryTest.skip();
     }
 
-    const url = await getLocalTestPath({ testDir: __dirname });
+    const url = await getLocalTestUrl({ testDir: __dirname });
 
     const requests = (
-      await Promise.all([
-        page.goto(url),
-        Promise.all([0, 1, 2].map(idx => page.waitForRequest(`http://localhost:4200/${idx}`))),
-      ])
+      await Promise.all([page.goto(url), Promise.all([0, 1, 2].map(idx => page.waitForRequest(`**/${idx}`)))])
     )[1];
 
     expect(requests).toHaveLength(3);

dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/tracePropagationTargets/defaultTargetsNoMatch/test.ts

@@ -4,7 +4,7 @@ import { sentryTest } from '../../../../../utils/fixtures';
 import { shouldSkipTracingTest } from '../../../../../utils/helpers';
 
 sentryTest(
-  'should not attach `sentry-trace` and `baggage` header to request not matching default tracePropagationTargets',
+  'should not attach `sentry-trace` and `baggage` header to cross-origin requests when no tracePropagationTargets are defined',
   async ({ getLocalTestPath, page }) => {
     if (shouldSkipTracingTest()) {
       sentryTest.skip();

dev-packages/browser-integration-tests/suites/tracing/browsertracing/tracePropagationTargets/defaultTargetsMatch/subject.js

@@ -1 +1 @@
-fetch('http://localhost:4200/0').then(fetch('http://localhost:4200/1').then(fetch('http://localhost:4200/2')));
+fetch('/0').then(fetch('/1').then(fetch('/2')));

dev-packages/browser-integration-tests/suites/tracing/browsertracing/tracePropagationTargets/defaultTargetsMatch/test.ts

@@ -4,19 +4,16 @@ import { sentryTest } from '../../../../../utils/fixtures';
 import { shouldSkipTracingTest } from '../../../../../utils/helpers';
 
 sentryTest(
-  'should attach `sentry-trace` and `baggage` header to request matching default tracePropagationTargets',
-  async ({ getLocalTestPath, page }) => {
+  'should attach `sentry-trace` and `baggage` header to same-origin requests when no tracePropagationTargets are defined',
+  async ({ getLocalTestUrl, page }) => {
     if (shouldSkipTracingTest()) {
       sentryTest.skip();
     }
 
-    const url = await getLocalTestPath({ testDir: __dirname });
+    const url = await getLocalTestUrl({ testDir: __dirname });
 
     const requests = (
-      await Promise.all([
-        page.goto(url),
-        Promise.all([0, 1, 2].map(idx => page.waitForRequest(`http://localhost:4200/${idx}`))),
-      ])
+      await Promise.all([page.goto(url), Promise.all([0, 1, 2].map(idx => page.waitForRequest(`**/${idx}`)))])
     )[1];
 
     expect(requests).toHaveLength(3);

dev-packages/browser-integration-tests/suites/tracing/browsertracing/tracePropagationTargets/defaultTargetsNoMatch/init.js

@@ -1,10 +1,9 @@
 import * as Sentry from '@sentry/browser';
-import { Integrations } from '@sentry/tracing';
 
 window.Sentry = Sentry;
 
 Sentry.init({
   dsn: 'https://[email protected]/1337',
-  integrations: [new Integrations.BrowserTracing()],
+  integrations: [Sentry.browserTracingIntegration()],
   tracesSampleRate: 1,
 });

dev-packages/browser-integration-tests/suites/tracing/browsertracing/tracePropagationTargets/defaultTargetsNoMatch/test.ts

@@ -4,19 +4,16 @@ import { sentryTest } from '../../../../../utils/fixtures';
 import { shouldSkipTracingTest } from '../../../../../utils/helpers';
 
 sentryTest(
-  'should not attach `sentry-trace` and `baggage` header to request not matching default tracePropagationTargets',
-  async ({ getLocalTestPath, page }) => {
+  'should not attach `sentry-trace` and `baggage` header to cross-origin requests when no tracePropagationTargets are defined',
+  async ({ getLocalTestUrl, page }) => {
     if (shouldSkipTracingTest()) {
       sentryTest.skip();
     }
 
-    const url = await getLocalTestPath({ testDir: __dirname });
+    const url = await getLocalTestUrl({ testDir: __dirname });
 
     const requests = (
-      await Promise.all([
-        page.goto(url),
-        Promise.all([0, 1, 2].map(idx => page.waitForRequest(`http://example.com/${idx}`))),
-      ])
+      await Promise.all([page.goto(url), Promise.all([0, 1, 2].map(idx => page.waitForRequest(`**/${idx}`)))])
     )[1];
 
     expect(requests).toHaveLength(3);

packages/browser/src/index.ts

@@ -62,7 +62,6 @@ export {
   browserTracingIntegration,
   startBrowserTracingNavigationSpan,
   startBrowserTracingPageLoadSpan,
-  DEFAULT_TRACE_PROPAGATION_TARGETS,
 } from '@sentry-internal/tracing';
 export type { RequestInstrumentationOptions } from '@sentry-internal/tracing';
 export {

packages/nextjs/src/client/index.ts

@@ -1,10 +1,6 @@
 import { addEventProcessor, applySdkMetadata, hasTracingEnabled, setTag } from '@sentry/core';
 import type { BrowserOptions } from '@sentry/react';
-import {
-  DEFAULT_TRACE_PROPAGATION_TARGETS,
-  getDefaultIntegrations as getReactDefaultIntegrations,
-  init as reactInit,
-} from '@sentry/react';
+import { getDefaultIntegrations as getReactDefaultIntegrations, init as reactInit } from '@sentry/react';
 import type { EventProcessor, Integration } from '@sentry/types';
 
 import { devErrorSymbolicationEventProcessor } from '../common/devErrorSymbolicationEventProcessor';
@@ -28,17 +24,6 @@ declare const __SENTRY_TRACING__: boolean;
 export function init(options: BrowserOptions): void {
   const opts = {
     environment: getVercelEnv(true) || process.env.NODE_ENV,
-
-    tracePropagationTargets:
-      process.env.NODE_ENV === 'development'
-        ? [
-            // Will match any URL that contains "localhost" but not "webpack.hot-update.json" - The webpack dev-server
-            // has cors and it doesn't like extra headers when it's accessed from a different URL.
-            // TODO(v8): Ideally we rework our tracePropagationTargets logic so this hack won't be necessary anymore (see issue #9764)
-            /^(?=.*localhost)(?!.*webpack\.hot-update\.json).*/,
-            /^\/(?!\/)/,
-          ]
-        : [...DEFAULT_TRACE_PROPAGATION_TARGETS, /^(api\/)/],
     defaultIntegrations: getDefaultIntegrations(options),
     ...options,
   } satisfies BrowserOptions;