ref(browser): Improve active span handling for browserTracingIntegration (#14959)

Extracted this out of
#14955

We used to rely on implied stuff here quite a bit, which breaks if we
start returning non recording spans. Honestly this just surfaces that
this is not really ideal as it is 😬 We already pass the client around
there everywhere, so this PR updates this so we keep the active idle
span as non-enumerable prop on the client, ensuring this is consistent
and "pure".

Author: mydea

Diff for: packages/browser/src/tracing/browserTracingIntegration.ts

@@ -16,9 +16,9 @@ import {
   SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
   SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
   TRACING_DEFAULTS,
+  addNonEnumerableProperty,
   browserPerformanceTimeOrigin,
   generateTraceId,
-  getActiveSpan,
   getClient,
   getCurrentScope,
   getDynamicSamplingContextFromSpan,
@@ -247,7 +247,7 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
   };
 
   /** Create routing idle transaction. */
-  function _createRouteSpan(client: Client, startSpanOptions: StartSpanOptions): Span {
+  function _createRouteSpan(client: Client, startSpanOptions: StartSpanOptions): void {
     const isPageloadTransaction = startSpanOptions.op === 'pageload';
 
     const finalStartSpanOptions: StartSpanOptions = beforeStartSpan
@@ -275,8 +275,10 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
       beforeSpanEnd: span => {
         _collectWebVitals();
         addPerformanceEntries(span, { recordClsOnPageloadSpan: !enableStandaloneClsSpans });
+        setActiveIdleSpan(client, undefined);
       },
     });
+    setActiveIdleSpan(client, idleSpan);
 
     function emitFinish(): void {
       if (optionalWindowDocument && ['interactive', 'complete'].includes(optionalWindowDocument.readyState)) {
@@ -291,17 +293,16 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
 
       emitFinish();
     }
-
-    return idleSpan;
   }
 
   return {
     name: BROWSER_TRACING_INTEGRATION_ID,
     afterAllSetup(client) {
-      let activeSpan: Span | undefined;
       let startingUrl: string | undefined = getLocationHref();
 
       function maybeEndActiveSpan(): void {
+        const activeSpan = getActiveIdleSpan(client);
+
         if (activeSpan && !spanToJSON(activeSpan).timestamp) {
           DEBUG_BUILD && logger.log(`[Tracing] Finishing current active span with op: ${spanToJSON(activeSpan).op}`);
           // If there's an open active span, we need to finish it before creating an new one.
@@ -316,7 +317,7 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
 
         maybeEndActiveSpan();
 
-        activeSpan = _createRouteSpan(client, {
+        _createRouteSpan(client, {
           op: 'navigation',
           ...startSpanOptions,
         });
@@ -334,7 +335,7 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
         const propagationContext = propagationContextFromHeaders(sentryTrace, baggage);
         getCurrentScope().setPropagationContext(propagationContext);
 
-        activeSpan = _createRouteSpan(client, {
+        _createRouteSpan(client, {
           op: 'pageload',
           ...startSpanOptions,
         });
@@ -409,7 +410,7 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
       }
 
       if (enableInteractions) {
-        registerInteractionListener(idleTimeout, finalTimeout, childSpanTimeout, latestRoute);
+        registerInteractionListener(client, idleTimeout, finalTimeout, childSpanTimeout, latestRoute);
       }
 
       if (enableInp) {
@@ -441,12 +442,9 @@ export function startBrowserTracingPageLoadSpan(
   traceOptions?: { sentryTrace?: string | undefined; baggage?: string | undefined },
 ): Span | undefined {
   client.emit('startPageLoadSpan', spanOptions, traceOptions);
-
   getCurrentScope().setTransactionName(spanOptions.name);
 
-  const span = getActiveSpan();
-  const op = span && spanToJSON(span).op;
-  return op === 'pageload' ? span : undefined;
+  return getActiveIdleSpan(client);
 }
 
 /**
@@ -461,9 +459,7 @@ export function startBrowserTracingNavigationSpan(client: Client, spanOptions: S
 
   getCurrentScope().setTransactionName(spanOptions.name);
 
-  const span = getActiveSpan();
-  const op = span && spanToJSON(span).op;
-  return op === 'navigation' ? span : undefined;
+  return getActiveIdleSpan(client);
 }
 
 /** Returns the value of a meta tag */
@@ -480,6 +476,7 @@ export function getMetaContent(metaName: string): string | undefined {
 
 /** Start listener for interaction transactions */
 function registerInteractionListener(
+  client: Client,
   idleTimeout: BrowserTracingOptions['idleTimeout'],
   finalTimeout: BrowserTracingOptions['finalTimeout'],
   childSpanTimeout: BrowserTracingOptions['childSpanTimeout'],
@@ -495,10 +492,9 @@ function registerInteractionListener(
   const registerInteractionTransaction = (): void => {
     const op = 'ui.action.click';
 
-    const activeSpan = getActiveSpan();
-    const rootSpan = activeSpan && getRootSpan(activeSpan);
-    if (rootSpan) {
-      const currentRootSpanOp = spanToJSON(rootSpan).op;
+    const activeIdleSpan = getActiveIdleSpan(client);
+    if (activeIdleSpan) {
+      const currentRootSpanOp = spanToJSON(activeIdleSpan).op;
       if (['navigation', 'pageload'].includes(currentRootSpanOp as string)) {
         DEBUG_BUILD &&
           logger.warn(`[Tracing] Did not create ${op} span because a pageload or navigation span is in progress.`);
@@ -537,3 +533,13 @@ function registerInteractionListener(
     addEventListener('click', registerInteractionTransaction, { once: false, capture: true });
   }
 }
+
+// We store the active idle span on the client object, so we can access it from exported functions
+const ACTIVE_IDLE_SPAN_PROPERTY = '_sentry_idleSpan';
+function getActiveIdleSpan(client: Client): Span | undefined {
+  return (client as { [ACTIVE_IDLE_SPAN_PROPERTY]?: Span })[ACTIVE_IDLE_SPAN_PROPERTY];
+}
+
+function setActiveIdleSpan(client: Client, span: Span | undefined): void {
+  addNonEnumerableProperty(client, ACTIVE_IDLE_SPAN_PROPERTY, span);
+}