From 8582fbc144568dbab1f385a45c85ed5dc874cf78 Mon Sep 17 00:00:00 2001
From: Francesco Novy <francesco.novy@sentry.io>
Date: Mon, 29 Jan 2024 14:24:30 +0100
Subject: [PATCH] ref(tracing): Implement changes to new
 `browserTracingIntegration`

This implements changes to the new integration, based on feedback from the team.
---
 packages/browser/src/index.ts                 |  2 -
 .../src/browser/browserTracingIntegration.ts  | 58 ++++---------------
 .../tracing-internal/src/browser/index.ts     |  2 -
 packages/tracing-internal/src/index.ts        |  2 -
 4 files changed, 11 insertions(+), 53 deletions(-)

diff --git a/packages/browser/src/index.ts b/packages/browser/src/index.ts
index 658dff61716b..408a64081a02 100644
--- a/packages/browser/src/index.ts
+++ b/packages/browser/src/index.ts
@@ -60,8 +60,6 @@ export {
   browserTracingIntegration,
   startBrowserTracingNavigationSpan,
   startBrowserTracingPageLoadSpan,
-  disableDefaultBrowserTracingNavigationSpan,
-  disableDefaultBrowserTracingPageLoadSpan,
 } from '@sentry-internal/tracing';
 export type { RequestInstrumentationOptions } from '@sentry-internal/tracing';
 export {
diff --git a/packages/tracing-internal/src/browser/browserTracingIntegration.ts b/packages/tracing-internal/src/browser/browserTracingIntegration.ts
index 8184ad058039..3f7979895db9 100644
--- a/packages/tracing-internal/src/browser/browserTracingIntegration.ts
+++ b/packages/tracing-internal/src/browser/browserTracingIntegration.ts
@@ -75,12 +75,14 @@ export interface BrowserTracingOptions extends RequestInstrumentationOptions {
 
   /**
    * If a span should be created on page load.
+   * If this is set to `false`, this integration will not start the default page load span.
    * Default: true
    */
   instrumentPageLoad: boolean;
 
   /**
    * If a span should be created on navigation (history change).
+   * If this is set to `false`, this integration will not start the default navigation spans.
    * Default: true
    */
   instrumentNavigation: boolean;
@@ -144,9 +146,6 @@ const DEFAULT_BROWSER_TRACING_OPTIONS: BrowserTracingOptions = {
   ...defaultRequestInstrumentationOptions,
 };
 
-let shouldUseDefaultPageLoadSpan = true;
-let shouldUseDefaultNavigationSpan = true;
-
 /**
  * The Browser Tracing integration automatically instruments browser pageload/navigation
  * actions as transactions, and captures requests, metrics and errors as spans.
@@ -323,13 +322,6 @@ export const _browserTracingIntegration = ((_options: Partial<BrowserTracingOpti
 
       if (client.on) {
         client.on('startNavigationSpan', (context: StartSpanOptions) => {
-          // We check this inside of the hook handler, so that if a custom instrumentation triggers this,
-          // we don't need to check this option in the instrumentation, but can simply invoke it
-          // without needing to know the options of this integration
-          if (!options.instrumentNavigation) {
-            return;
-          }
-
           if (activeSpan) {
             DEBUG_BUILD && logger.log(`[Tracing] Finishing current transaction with op: ${spanToJSON(activeSpan).op}`);
             // If there's an open transaction on the scope, we need to finish it before creating an new one.
@@ -339,13 +331,6 @@ export const _browserTracingIntegration = ((_options: Partial<BrowserTracingOpti
         });
 
         client.on('startPageLoadSpan', (context: StartSpanOptions) => {
-          // We check this inside of the hook handler, so that if a custom instrumentation triggers this,
-          // we don't need to check this option in the instrumentation, but can simply invoke it
-          // without needing to know the options of this integration
-          if (!options.instrumentPageLoad) {
-            return;
-          }
-
           if (activeSpan) {
             DEBUG_BUILD && logger.log(`[Tracing] Finishing current transaction with op: ${spanToJSON(activeSpan).op}`);
             // If there's an open transaction on the scope, we need to finish it before creating an new one.
@@ -355,7 +340,7 @@ export const _browserTracingIntegration = ((_options: Partial<BrowserTracingOpti
         });
       }
 
-      if (options.instrumentPageLoad && client.emit && shouldUseDefaultPageLoadSpan) {
+      if (options.instrumentPageLoad && client.emit) {
         const context: StartSpanOptions = {
           name: WINDOW.location.pathname,
           // pageload should always start at timeOrigin (and needs to be in s, not ms)
@@ -385,17 +370,14 @@ export const _browserTracingIntegration = ((_options: Partial<BrowserTracingOpti
 
           if (from !== to) {
             startingUrl = undefined;
-            // We check this in here again, as a custom instrumentation may have been triggered in the meanwhile
-            if (shouldUseDefaultNavigationSpan) {
-              const context: StartSpanOptions = {
-                name: WINDOW.location.pathname,
-                op: 'navigation',
-                origin: 'auto.navigation.browser',
-                metadata: { source: 'url' },
-              };
-
-              startBrowserTracingNavigationSpan(context);
-            }
+            const context: StartSpanOptions = {
+              name: WINDOW.location.pathname,
+              op: 'navigation',
+              origin: 'auto.navigation.browser',
+              metadata: { source: 'url' },
+            };
+
+            startBrowserTracingNavigationSpan(context);
           }
         });
       }
@@ -435,7 +417,6 @@ export function startBrowserTracingPageLoadSpan(spanOptions: StartSpanOptions):
   }
 
   client.emit('startPageLoadSpan', spanOptions);
-  shouldUseDefaultPageLoadSpan = false;
 }
 
 /**
@@ -449,23 +430,6 @@ export function startBrowserTracingNavigationSpan(spanOptions: StartSpanOptions)
   }
 
   client.emit('startNavigationSpan', spanOptions);
-  shouldUseDefaultNavigationSpan = false;
-}
-
-/**
- * Use this method if you want to disable the default navigation span.
- * This is useful if you want to add custom routing instrumentation.
- */
-export function disableDefaultBrowserTracingNavigationSpan(disable = true): void {
-  shouldUseDefaultNavigationSpan = !disable;
-}
-
-/**
- * Use this method if you want to disable the default page load span.
- * This is useful if you want to add custom routing instrumentation.
- */
-export function disableDefaultBrowserTracingPageLoadSpan(disable = true): void {
-  shouldUseDefaultPageLoadSpan = !disable;
 }
 
 /** Returns the value of a meta tag */
diff --git a/packages/tracing-internal/src/browser/index.ts b/packages/tracing-internal/src/browser/index.ts
index d9b0c347bb4e..7d29d2f11e9e 100644
--- a/packages/tracing-internal/src/browser/index.ts
+++ b/packages/tracing-internal/src/browser/index.ts
@@ -7,8 +7,6 @@ export {
   browserTracingIntegration,
   startBrowserTracingNavigationSpan,
   startBrowserTracingPageLoadSpan,
-  disableDefaultBrowserTracingNavigationSpan,
-  disableDefaultBrowserTracingPageLoadSpan,
 } from './browserTracingIntegration';
 
 export { instrumentOutgoingRequests, defaultRequestInstrumentationOptions } from './request';
diff --git a/packages/tracing-internal/src/index.ts b/packages/tracing-internal/src/index.ts
index d3ace7e56b77..233baae9c308 100644
--- a/packages/tracing-internal/src/index.ts
+++ b/packages/tracing-internal/src/index.ts
@@ -17,8 +17,6 @@ export {
   browserTracingIntegration,
   startBrowserTracingNavigationSpan,
   startBrowserTracingPageLoadSpan,
-  disableDefaultBrowserTracingNavigationSpan,
-  disableDefaultBrowserTracingPageLoadSpan,
   BROWSER_TRACING_INTEGRATION_ID,
   instrumentOutgoingRequests,
   defaultRequestInstrumentationOptions,