only use for default browser tracing

Author: mydea

dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/opt-out/init.js

@@ -0,0 +1,16 @@
+import * as Sentry from '@sentry/browser';
+
+window.Sentry = Sentry;
+
+Sentry.init({
+  dsn: 'https://public@dsn.ingest.sentry.io/1337',
+  integrations: [
+    Sentry.browserTracingIntegration({
+      detectRedirects: false,
+    }),
+  ],
+  tracesSampleRate: 1,
+});
+
+// trigger redirect immediately
+window.history.pushState({}, '', '/sub-page');

dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-redirect/opt-out/test.ts

@@ -0,0 +1,34 @@
+import { expect } from '@playwright/test';
+import { sentryTest } from '../../../../../utils/fixtures';
+import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../../utils/helpers';
+
+sentryTest(
+  'should not create a navigation.redirect span if `detectRedirects` is set to false',
+  async ({ getLocalTestUrl, page }) => {
+    if (shouldSkipTracingTest()) {
+      sentryTest.skip();
+    }
+
+    const url = await getLocalTestUrl({ testDir: __dirname });
+
+    const pageloadRequestPromise = waitForTransactionRequest(page, event => event.contexts?.trace?.op === 'pageload');
+    const navigationRequestPromise = waitForTransactionRequest(
+      page,
+      event => event.contexts?.trace?.op === 'navigation',
+    );
+
+    await page.goto(url);
+
+    const pageloadRequest = envelopeRequestParser(await pageloadRequestPromise);
+    // Ensure a navigation span is sent, too
+    await navigationRequestPromise;
+
+    const spans = pageloadRequest.spans || [];
+
+    expect(spans).not.toContainEqual(
+      expect.objectContaining({
+        op: 'navigation.redirect',
+      }),
+    );
+  },
+);

packages/browser/src/tracing/browserTracingIntegration.ts

@@ -422,13 +422,12 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
         }
       }
 
-      client.on('startNavigationSpan', startSpanOptions => {
+      client.on('startNavigationSpan', (startSpanOptions, navigationOptions) => {
         if (getClient() !== client) {
           return;
         }
 
-        const activeSpan = getActiveIdleSpan(client);
-        if (detectRedirects && activeSpan && isRedirect(activeSpan, lastClickTimestamp)) {
+        if (navigationOptions?.isRedirect) {
           DEBUG_BUILD && logger.warn('[Tracing] Detected redirect, navigation span will not be the root span.');
           _createRouteSpan(
             client,
@@ -522,7 +521,7 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
             startingUrl = undefined;
             const parsed = parseStringToURLObject(to);
             const activeSpan = getActiveIdleSpan(client);
-            const navigationIsRedirect = activeSpan && isRedirect(activeSpan, lastClickTimestamp);
+            const navigationIsRedirect = activeSpan && detectRedirects && isRedirect(activeSpan, lastClickTimestamp);
             startBrowserTracingNavigationSpan(
               client,
               {
@@ -532,7 +531,7 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
                   [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.browser',
                 },
               },
-              !navigationIsRedirect ? { url: to } : undefined,
+              { url: to, isRedirect: navigationIsRedirect },
             );
           });
         }
@@ -588,17 +587,18 @@ export function startBrowserTracingPageLoadSpan(
 export function startBrowserTracingNavigationSpan(
   client: Client,
   spanOptions: StartSpanOptions,
-  options?: { url?: string },
+  options?: { url?: string; isRedirect?: boolean },
 ): Span | undefined {
-  client.emit('startNavigationSpan', spanOptions);
+  const { url, isRedirect } = options || {};
+
+  client.emit('startNavigationSpan', spanOptions, { isRedirect });
 
   const scope = getCurrentScope();
   scope.setTransactionName(spanOptions.name);
 
   // We store the normalized request data on the scope, so we get the request data at time of span creation
   // otherwise, the URL etc. may already be of the following navigation, and we'd report the wrong URL
-  const url = options?.url;
-  if (url) {
+  if (url && !isRedirect) {
     scope.setSDKProcessingMetadata({
       normalizedRequest: {
         ...getHttpRequestData(),

packages/browser/test/tracing/browserTracingIntegration.test.ts

@@ -53,33 +53,32 @@ Object.defineProperty(global, 'history', { value: dom.window.history, writable:
 const originalGlobalDocument = WINDOW.document;
 const originalGlobalLocation = WINDOW.location;
 const originalGlobalHistory = WINDOW.history;
-afterAll(() => {
-  // Clean up JSDom
-  Object.defineProperty(WINDOW, 'document', { value: originalGlobalDocument });
-  Object.defineProperty(WINDOW, 'location', { value: originalGlobalLocation });
-  Object.defineProperty(WINDOW, 'history', { value: originalGlobalHistory });
-});
-
-afterEach(() => {
-  vi.useRealTimers();
-  performance.clearMarks();
-});
 
 describe('browserTracingIntegration', () => {
   beforeEach(() => {
+    vi.useFakeTimers();
     getCurrentScope().clear();
     getIsolationScope().clear();
     getCurrentScope().setClient(undefined);
     document.head.innerHTML = '';
+
+    const dom = new JSDOM(undefined, { url: 'https://example.com/' });
+    Object.defineProperty(global, 'location', { value: dom.window.document.location, writable: true });
   });
 
   afterEach(() => {
     getActiveSpan()?.end();
+    vi.useRealTimers();
+    performance.clearMarks();
   });
 
   afterAll(() => {
     global.window.TextEncoder = oldTextEncoder;
     global.window.TextDecoder = oldTextDecoder;
+    // Clean up JSDom
+    Object.defineProperty(WINDOW, 'document', { value: originalGlobalDocument });
+    Object.defineProperty(WINDOW, 'location', { value: originalGlobalLocation });
+    Object.defineProperty(WINDOW, 'history', { value: originalGlobalHistory });
   });
 
   it('works with tracing enabled', () => {
@@ -152,7 +151,7 @@ describe('browserTracingIntegration', () => {
     expect(spanIsSampled(span!)).toBe(false);
   });
 
-  it('starts navigation when URL changes', () => {
+  it('starts navigation when URL changes after > 300ms', () => {
     const client = new BrowserClient(
       getDefaultBrowserClientOptions({
         tracesSampleRate: 1,
@@ -185,6 +184,7 @@ describe('browserTracingIntegration', () => {
     const dom = new JSDOM(undefined, { url: 'https://example.com/test' });
     Object.defineProperty(global, 'location', { value: dom.window.document.location, writable: true });
 
+    vi.advanceTimersByTime(400);
     WINDOW.history.pushState({}, '', '/test');
 
     expect(span!.isRecording()).toBe(false);

packages/core/src/client.ts

@@ -607,7 +607,10 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
    * A hook for browser tracing integrations to trigger a span for a navigation.
    * @returns {() => void} A function that, when executed, removes the registered callback.
    */
-  public on(hook: 'startNavigationSpan', callback: (options: StartSpanOptions) => void): () => void;
+  public on(
+    hook: 'startNavigationSpan',
+    callback: (options: StartSpanOptions, navigationOptions?: { isRedirect?: boolean }) => void,
+  ): () => void;
 
   /**
    * A hook for GraphQL client integration to enhance a span with request data.
@@ -782,7 +785,11 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
   /**
    * Emit a hook event for browser tracing integrations to trigger a span for a navigation.
    */
-  public emit(hook: 'startNavigationSpan', options: StartSpanOptions): void;
+  public emit(
+    hook: 'startNavigationSpan',
+    options: StartSpanOptions,
+    navigationOptions?: { isRedirect?: boolean },
+  ): void;
 
   /**
    * Emit a hook event for GraphQL client integration to enhance a span with request data.