fix(browser): Ensure pageload & navigation spans have correct data

Author: mydea

dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-aborting-pageload/init.js

@@ -9,4 +9,4 @@ Sentry.init({
 });
 
 // Immediately navigate to a new page to abort the pageload
-window.location.href = '#foo';
+window.history.pushState({}, '', '/sub-page');

dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-aborting-pageload/test.ts

@@ -40,6 +40,9 @@ sentryTest(
     expect(navigationTraceId).toBeDefined();
     expect(pageloadTraceId).not.toEqual(navigationTraceId);
 
+    expect(pageloadRequest.transaction).toEqual('/index.html');
+    expect(navigationRequest.transaction).toEqual('/sub-page');
+
     expect(pageloadRequest.contexts?.trace?.data).toMatchObject({
       [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.browser',
       [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1,
@@ -54,5 +57,17 @@ sentryTest(
       [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
       ['sentry.idle_span_finish_reason']: 'idleTimeout',
     });
+    expect(pageloadRequest.request).toEqual({
+      headers: {
+        'User-Agent': expect.any(String),
+      },
+      url: 'http://sentry-test.io/index.html',
+    });
+    expect(navigationRequest.request).toEqual({
+      headers: {
+        'User-Agent': expect.any(String),
+      },
+      url: 'http://sentry-test.io/sub-page',
+    });
   },
 );

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

@@ -31,6 +31,10 @@ sentryTest('should create a navigation transaction on page navigation', async ({
   expect(navigationTraceId).toBeDefined();
   expect(pageloadTraceId).not.toEqual(navigationTraceId);
 
+  expect(pageloadRequest.transaction).toEqual('/index.html');
+  // Fragment is not in transaction name
+  expect(navigationRequest.transaction).toEqual('/index.html');
+
   expect(pageloadRequest.contexts?.trace?.data).toMatchObject({
     [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.browser',
     [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1,
@@ -45,6 +49,18 @@ sentryTest('should create a navigation transaction on page navigation', async ({
     [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
     ['sentry.idle_span_finish_reason']: 'idleTimeout',
   });
+  expect(pageloadRequest.request).toEqual({
+    headers: {
+      'User-Agent': expect.any(String),
+    },
+    url: 'http://sentry-test.io/index.html',
+  });
+  expect(navigationRequest.request).toEqual({
+    headers: {
+      'User-Agent': expect.any(String),
+    },
+    url: 'http://sentry-test.io/index.html#foo',
+  });
 
   const pageloadSpans = pageloadRequest.spans;
   const navigationSpans = navigationRequest.spans;

packages/browser/src/helpers.ts

@@ -4,6 +4,7 @@ import {
   addExceptionTypeValue,
   addNonEnumerableProperty,
   captureException,
+  getLocationHref,
   getOriginalFunction,
   GLOBAL_OBJ,
   markFunctionWrapped,
@@ -175,3 +176,24 @@ export function wrap<T extends WrappableFunction, NonFunction>(
 
   return sentryWrapped;
 }
+
+/**
+ * Get HTTP request data from the current page.
+ */
+export function getHttpRequestData(): { url: string; headers: Record<string, string> } {
+  // grab as much info as exists and add it to the event
+  const url = getLocationHref();
+  const { referrer } = WINDOW.document || {};
+  const { userAgent } = WINDOW.navigator || {};
+
+  const headers = {
+    ...(referrer && { Referer: referrer }),
+    ...(userAgent && { 'User-Agent': userAgent }),
+  };
+  const request = {
+    url,
+    headers,
+  };
+
+  return request;
+}

packages/browser/src/integrations/httpcontext.ts

@@ -1,5 +1,5 @@
-import { defineIntegration, getLocationHref } from '@sentry/core';
-import { WINDOW } from '../helpers';
+import { defineIntegration } from '@sentry/core';
+import { getHttpRequestData, WINDOW } from '../helpers';
 
 /**
  * Collects information about HTTP request headers and
@@ -14,23 +14,17 @@ export const httpContextIntegration = defineIntegration(() => {
         return;
       }
 
-      // grab as much info as exists and add it to the event
-      const url = event.request?.url || getLocationHref();
-      const { referrer } = WINDOW.document || {};
-      const { userAgent } = WINDOW.navigator || {};
-
+      const reqData = getHttpRequestData();
       const headers = {
+        ...reqData.headers,
         ...event.request?.headers,
-        ...(referrer && { Referer: referrer }),
-        ...(userAgent && { 'User-Agent': userAgent }),
       };
-      const request = {
+
+      event.request = {
+        ...reqData,
         ...event.request,
-        ...(url && { url }),
         headers,
       };
-
-      event.request = request;
     },
   };
 });

packages/browser/src/tracing/browserTracingIntegration.ts

@@ -33,7 +33,7 @@ import {
   startTrackingWebVitals,
 } from '@sentry-internal/browser-utils';
 import { DEBUG_BUILD } from '../debug-build';
-import { WINDOW } from '../helpers';
+import { getHttpRequestData, WINDOW } from '../helpers';
 import { registerBackgroundTabDetection } from './backgroundtab';
 import { linkTraces } from './linkedTraces';
 import { defaultRequestInstrumentationOptions, instrumentOutgoingRequests } from './request';
@@ -360,6 +360,12 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
 
     setActiveIdleSpan(client, idleSpan);
 
+    // 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
+    getCurrentScope().setSDKProcessingMetadata({
+      normalizedRequest: getHttpRequestData(),
+    });
+
     function emitFinish(): void {
       if (optionalWindowDocument && ['interactive', 'complete'].includes(optionalWindowDocument.readyState)) {
         client.emit('idleSpanEnableAutoFinish', idleSpan);
@@ -459,16 +465,17 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
               return;
             }
 
-            if (from !== to) {
-              startingUrl = undefined;
+            startingUrl = undefined;
+            // We wait a tick here to ensure that WINDOW.location.pathname is updated
+            setTimeout(() => {
               startBrowserTracingNavigationSpan(client, {
                 name: WINDOW.location.pathname,
                 attributes: {
                   [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
                   [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.browser',
                 },
               });
-            }
+            });
           });
         }
       }

packages/core/src/tracing/sentrySpan.ts

@@ -336,6 +336,8 @@ export class SentrySpan implements Span {
 
     const { scope: capturedSpanScope, isolationScope: capturedSpanIsolationScope } = getCapturedScopesOnSpan(this);
 
+    const normalizedRequest = capturedSpanScope?.getScopeData().sdkProcessingMetadata?.normalizedRequest;
+
     if (this._sampled !== true) {
       return undefined;
     }
@@ -374,6 +376,7 @@ export class SentrySpan implements Span {
         capturedSpanIsolationScope,
         dynamicSamplingContext: getDynamicSamplingContextFromSpan(this),
       },
+      request: normalizedRequest,
       ...(source && {
         transaction_info: {
           source,