From c4bcc25e4723ba1d5a08dfaa249306e62685fa06 Mon Sep 17 00:00:00 2001
From: Francesco Novy <francesco.novy@sentry.io>
Date: Tue, 21 Jan 2025 13:15:18 +0100
Subject: [PATCH 1/4] ref(browser): Improve when/how propagation scope is kept
 for navigations

---
 .../src/tracing/browserTracingIntegration.ts  | 61 +++++++++++--------
 1 file changed, 37 insertions(+), 24 deletions(-)

diff --git a/packages/browser/src/tracing/browserTracingIntegration.ts b/packages/browser/src/tracing/browserTracingIntegration.ts
index 5bbda349b25d..721419e2b4b6 100644
--- a/packages/browser/src/tracing/browserTracingIntegration.ts
+++ b/packages/browser/src/tracing/browserTracingIntegration.ts
@@ -9,7 +9,7 @@ import {
   startTrackingLongTasks,
   startTrackingWebVitals,
 } from '@sentry-internal/browser-utils';
-import type { Client, IntegrationFn, Span, StartSpanOptions, TransactionSource } from '@sentry/core';
+import { Client, IntegrationFn, Span, StartSpanOptions, TransactionSource, dropUndefinedKeys } from '@sentry/core';
 import {
   GLOBAL_OBJ,
   SEMANTIC_ATTRIBUTE_SENTRY_IDLE_SPAN_FINISH_REASON,
@@ -276,6 +276,17 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
         _collectWebVitals();
         addPerformanceEntries(span, { recordClsOnPageloadSpan: !enableStandaloneClsSpans });
         setActiveIdleSpan(client, undefined);
+
+        // Ensure that DSC is updated with possibly final transaction etc.
+        const scope = getCurrentScope();
+        const oldPropagationContext = scope.getPropagationContext();
+
+        scope.setPropagationContext({
+          ...oldPropagationContext,
+          traceId: idleSpan.spanContext().traceId,
+          sampled: spanIsSampled(idleSpan),
+          dsc: getDynamicSamplingContextFromSpan(span),
+        });
       },
     });
     setActiveIdleSpan(client, idleSpan);
@@ -293,6 +304,22 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
 
       emitFinish();
     }
+
+    // A trace should to stay the consistent over the entire time span of one route.
+    // Therefore, we update the traceId/sampled properties on the propagation context.
+    // The DSC is only set once the span has ended
+    const scope = getCurrentScope();
+    const oldPropagationContext = scope.getPropagationContext();
+
+    scope.setPropagationContext(
+      dropUndefinedKeys({
+        ...oldPropagationContext,
+        traceId: idleSpan.spanContext().traceId,
+        sampled: spanIsSampled(idleSpan),
+        // Reset the DSC, it should be read from the span while it is active
+        dsc: undefined,
+      }),
+    );
   }
 
   return {
@@ -341,27 +368,6 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
         });
       });
 
-      // A trace should to stay the consistent over the entire time span of one route.
-      // Therefore, when the initial pageload or navigation root span ends, we update the
-      // scope's propagation context to keep span-specific attributes like the `sampled` decision and
-      // the dynamic sampling context valid, even after the root span has ended.
-      // This ensures that the trace data is consistent for the entire duration of the route.
-      client.on('spanEnd', span => {
-        const op = spanToJSON(span).op;
-        if (span !== getRootSpan(span) || (op !== 'navigation' && op !== 'pageload')) {
-          return;
-        }
-
-        const scope = getCurrentScope();
-        const oldPropagationContext = scope.getPropagationContext();
-
-        scope.setPropagationContext({
-          ...oldPropagationContext,
-          sampled: oldPropagationContext.sampled !== undefined ? oldPropagationContext.sampled : spanIsSampled(span),
-          dsc: oldPropagationContext.dsc || getDynamicSamplingContextFromSpan(span),
-        });
-      });
-
       if (WINDOW.location) {
         if (instrumentPageLoad) {
           const origin = browserPerformanceTimeOrigin();
@@ -453,8 +459,15 @@ export function startBrowserTracingPageLoadSpan(
  * This will only do something if a browser tracing integration has been setup.
  */
 export function startBrowserTracingNavigationSpan(client: Client, spanOptions: StartSpanOptions): Span | undefined {
-  getIsolationScope().setPropagationContext({ traceId: generateTraceId(), sampleRand: Math.random() });
-  getCurrentScope().setPropagationContext({ traceId: generateTraceId(), sampleRand: Math.random() });
+  // Reset this to ensure we start a new trace, instead of continuing the last pageload/navigation trace
+  getIsolationScope().setPropagationContext({
+    traceId: generateTraceId(),
+    sampleRand: Math.random(),
+  });
+  getCurrentScope().setPropagationContext({
+    traceId: generateTraceId(),
+    sampleRand: Math.random(),
+  });
 
   client.emit('startNavigationSpan', spanOptions);
 

From 5398ae3fb62e6549cd7c8b8f9df3de1c87f082dc Mon Sep 17 00:00:00 2001
From: Francesco Novy <francesco.novy@sentry.io>
Date: Tue, 21 Jan 2025 13:26:03 +0100
Subject: [PATCH 2/4] cleanup

---
 packages/browser/src/tracing/browserTracingIntegration.ts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/packages/browser/src/tracing/browserTracingIntegration.ts b/packages/browser/src/tracing/browserTracingIntegration.ts
index 721419e2b4b6..e4cbd271892f 100644
--- a/packages/browser/src/tracing/browserTracingIntegration.ts
+++ b/packages/browser/src/tracing/browserTracingIntegration.ts
@@ -9,7 +9,7 @@ import {
   startTrackingLongTasks,
   startTrackingWebVitals,
 } from '@sentry-internal/browser-utils';
-import { Client, IntegrationFn, Span, StartSpanOptions, TransactionSource, dropUndefinedKeys } from '@sentry/core';
+import type { Client, IntegrationFn, Span, StartSpanOptions, TransactionSource } from '@sentry/core';
 import {
   GLOBAL_OBJ,
   SEMANTIC_ATTRIBUTE_SENTRY_IDLE_SPAN_FINISH_REASON,
@@ -18,13 +18,13 @@ import {
   TRACING_DEFAULTS,
   addNonEnumerableProperty,
   browserPerformanceTimeOrigin,
+  dropUndefinedKeys,
   generateTraceId,
   getClient,
   getCurrentScope,
   getDynamicSamplingContextFromSpan,
   getIsolationScope,
   getLocationHref,
-  getRootSpan,
   logger,
   propagationContextFromHeaders,
   registerSpanErrorInstrumentation,

From 1241510f2b09a3bff6e2eb083da80fc45080077c Mon Sep 17 00:00:00 2001
From: Francesco Novy <francesco.novy@sentry.io>
Date: Tue, 21 Jan 2025 13:29:26 +0100
Subject: [PATCH 3/4] only set on end?

---
 .../src/tracing/browserTracingIntegration.ts  | 31 +++----------------
 1 file changed, 5 insertions(+), 26 deletions(-)

diff --git a/packages/browser/src/tracing/browserTracingIntegration.ts b/packages/browser/src/tracing/browserTracingIntegration.ts
index e4cbd271892f..fd0e00d52769 100644
--- a/packages/browser/src/tracing/browserTracingIntegration.ts
+++ b/packages/browser/src/tracing/browserTracingIntegration.ts
@@ -18,7 +18,6 @@ import {
   TRACING_DEFAULTS,
   addNonEnumerableProperty,
   browserPerformanceTimeOrigin,
-  dropUndefinedKeys,
   generateTraceId,
   getClient,
   getCurrentScope,
@@ -277,7 +276,9 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
         addPerformanceEntries(span, { recordClsOnPageloadSpan: !enableStandaloneClsSpans });
         setActiveIdleSpan(client, undefined);
 
-        // Ensure that DSC is updated with possibly final transaction etc.
+        // A trace should to stay the consistent over the entire time span of one route.
+        // Therefore, we update the traceId/sampled properties on the propagation context.
+        // When a navigation happens, this is overwritten
         const scope = getCurrentScope();
         const oldPropagationContext = scope.getPropagationContext();
 
@@ -304,22 +305,6 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
 
       emitFinish();
     }
-
-    // A trace should to stay the consistent over the entire time span of one route.
-    // Therefore, we update the traceId/sampled properties on the propagation context.
-    // The DSC is only set once the span has ended
-    const scope = getCurrentScope();
-    const oldPropagationContext = scope.getPropagationContext();
-
-    scope.setPropagationContext(
-      dropUndefinedKeys({
-        ...oldPropagationContext,
-        traceId: idleSpan.spanContext().traceId,
-        sampled: spanIsSampled(idleSpan),
-        // Reset the DSC, it should be read from the span while it is active
-        dsc: undefined,
-      }),
-    );
   }
 
   return {
@@ -460,14 +445,8 @@ export function startBrowserTracingPageLoadSpan(
  */
 export function startBrowserTracingNavigationSpan(client: Client, spanOptions: StartSpanOptions): Span | undefined {
   // Reset this to ensure we start a new trace, instead of continuing the last pageload/navigation trace
-  getIsolationScope().setPropagationContext({
-    traceId: generateTraceId(),
-    sampleRand: Math.random(),
-  });
-  getCurrentScope().setPropagationContext({
-    traceId: generateTraceId(),
-    sampleRand: Math.random(),
-  });
+  getIsolationScope().setPropagationContext({ traceId: generateTraceId(), sampleRand: Math.random() });
+  getCurrentScope().setPropagationContext({ traceId: generateTraceId(), sampleRand: Math.random() });
 
   client.emit('startNavigationSpan', spanOptions);
 

From ffc20385002c78aa2881a818057c9ea549ffa785 Mon Sep 17 00:00:00 2001
From: Francesco Novy <francesco.novy@sentry.io>
Date: Tue, 21 Jan 2025 13:43:44 +0100
Subject: [PATCH 4/4] better comments

---
 packages/browser/src/tracing/browserTracingIntegration.ts | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/packages/browser/src/tracing/browserTracingIntegration.ts b/packages/browser/src/tracing/browserTracingIntegration.ts
index fd0e00d52769..d907e5b3d8d6 100644
--- a/packages/browser/src/tracing/browserTracingIntegration.ts
+++ b/packages/browser/src/tracing/browserTracingIntegration.ts
@@ -276,9 +276,9 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
         addPerformanceEntries(span, { recordClsOnPageloadSpan: !enableStandaloneClsSpans });
         setActiveIdleSpan(client, undefined);
 
-        // A trace should to stay the consistent over the entire time span of one route.
-        // Therefore, we update the traceId/sampled properties on the propagation context.
-        // When a navigation happens, this is overwritten
+        // A trace should stay consistent over the entire timespan of one route - even after the pageload/navigation ended.
+        // Only when another navigation happens, we want to create a new trace.
+        // This way, e.g. errors that occur after the pageload span ended are still associated to the pageload trace.
         const scope = getCurrentScope();
         const oldPropagationContext = scope.getPropagationContext();
 
@@ -444,7 +444,6 @@ export function startBrowserTracingPageLoadSpan(
  * This will only do something if a browser tracing integration has been setup.
  */
 export function startBrowserTracingNavigationSpan(client: Client, spanOptions: StartSpanOptions): Span | undefined {
-  // Reset this to ensure we start a new trace, instead of continuing the last pageload/navigation trace
   getIsolationScope().setPropagationContext({ traceId: generateTraceId(), sampleRand: Math.random() });
   getCurrentScope().setPropagationContext({ traceId: generateTraceId(), sampleRand: Math.random() });