ref(browser): Streamline pageload span creation and scope handling (#11679)

Wwe can streamline our pageload span
creation logic now that we actually want to keep the propagation context
on the scope after the transaction finished (see #11599). Previously,
we'd fork a new scope for the pageload span but IMHO (and according to
all our tests) this is no longer necessary.

Author: Lms24

packages/browser/src/tracing/browserTracingIntegration.ts

@@ -10,7 +10,6 @@ import {
   SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
   SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
   TRACING_DEFAULTS,
-  continueTrace,
   getActiveSpan,
   getClient,
   getCurrentScope,
@@ -21,11 +20,16 @@ import {
   spanIsSampled,
   spanToJSON,
   startIdleSpan,
-  withScope,
 } from '@sentry/core';
 import type { Client, IntegrationFn, StartSpanOptions, TransactionSource } from '@sentry/types';
 import type { Span } from '@sentry/types';
-import { browserPerformanceTimeOrigin, getDomElement, logger, uuid4 } from '@sentry/utils';
+import {
+  browserPerformanceTimeOrigin,
+  getDomElement,
+  logger,
+  propagationContextFromHeaders,
+  uuid4,
+} from '@sentry/utils';
 
 import { DEBUG_BUILD } from '../debug-build';
 import { WINDOW } from '../helpers';
@@ -263,31 +267,19 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
         const sentryTrace = traceOptions.sentryTrace || getMetaContent('sentry-trace');
         const baggage = traceOptions.baggage || getMetaContent('baggage');
 
-        // Continue trace updates the scope in the callback only, but we want to break out of it again...
-        // This is a bit hacky, because we want to get the span to use both the correct scope _and_ the correct propagation context
-        // but afterwards, we want to reset it to avoid this also applying to other spans
-        const scope = getCurrentScope();
+        const propagationContext = propagationContextFromHeaders(sentryTrace, baggage);
+        getCurrentScope().setPropagationContext(propagationContext);
 
-        activeSpan = continueTrace({ sentryTrace, baggage }, () => {
-          // We update the outer current scope to have the correct propagation context
-          // this means, the scope active when the pageload span is created will continue to hold the
-          // propagationContext from the incoming trace, even after the pageload span ended.
-          scope.setPropagationContext(getCurrentScope().getPropagationContext());
-
-          // Ensure we are on the original current scope again, so the span is set as active on it
-          return withScope(scope, () => {
-            return _createRouteSpan(client, {
-              op: 'pageload',
-              ...startSpanOptions,
-            });
-          });
+        activeSpan = _createRouteSpan(client, {
+          op: 'pageload',
+          ...startSpanOptions,
         });
       });
 
       // A trace should to stay the consistent over the entire time span of one route.
-      // Therefore, when the initial pageload or navigation transaction ends, we update the
+      // 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 transaction has ended.
+      // 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;

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

@@ -676,7 +676,7 @@ describe('browserTracingIntegration', () => {
       expect(newCurrentScopePropCtx?.traceId).not.toEqual(oldCurrentScopePropCtx?.traceId);
     });
 
-    it("saves the span's sampling decision and its DSC on the propagationContext when the span finishes", () => {
+    it("saves the span's positive sampling decision and its DSC on the propagationContext when the span finishes", () => {
       const client = new BrowserClient(
         getDefaultBrowserClientOptions({
           tracesSampleRate: 1,
@@ -714,6 +714,45 @@ describe('browserTracingIntegration', () => {
         },
       });
     });
+
+    it("saves the span's negative sampling decision and its DSC on the propagationContext when the span finishes", () => {
+      const client = new BrowserClient(
+        getDefaultBrowserClientOptions({
+          tracesSampleRate: 0,
+          integrations: [browserTracingIntegration({ instrumentPageLoad: false })],
+        }),
+      );
+      setCurrentClient(client);
+      client.init();
+
+      const navigationSpan = startBrowserTracingNavigationSpan(client, {
+        name: 'mySpan',
+        attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route' },
+      });
+
+      const propCtxBeforeEnd = getCurrentScope().getPropagationContext();
+      expect(propCtxBeforeEnd).toStrictEqual({
+        spanId: expect.stringMatching(/[a-f0-9]{16}/),
+        traceId: expect.stringMatching(/[a-f0-9]{32}/),
+      });
+
+      navigationSpan!.end();
+
+      const propCtxAfterEnd = getCurrentScope().getPropagationContext();
+      expect(propCtxAfterEnd).toStrictEqual({
+        spanId: propCtxBeforeEnd?.spanId,
+        traceId: propCtxBeforeEnd?.traceId,
+        sampled: false,
+        dsc: {
+          environment: 'production',
+          public_key: 'examplePublicKey',
+          sample_rate: '0',
+          sampled: 'false',
+          transaction: 'mySpan',
+          trace_id: propCtxBeforeEnd?.traceId,
+        },
+      });
+    });
   });
 
   describe('using the <meta> tag data', () => {

packages/utils/src/tracing.ts

@@ -44,7 +44,8 @@ export function extractTraceparentData(traceparent?: string): TraceparentData |
 }
 
 /**
- * Create a propagation context from incoming headers.
+ * Create a propagation context from incoming headers or
+ * creates a minimal new one if the headers are undefined.
  */
 export function propagationContextFromHeaders(
   sentryTrace: string | undefined,