ref(browser): Update browser profiling to avoid deprecated APIs (#11007)

mydea · web-flow · commit b1a6a437ddc9 · 2024-03-11T14:17:16.000Z
Esp. migrate away from `startTransaction` to `spanStart` hooks.
diff --git a/packages/browser/src/profiling/integration.ts b/packages/browser/src/profiling/integration.ts
@@ -1,18 +1,17 @@
-import { defineIntegration, getCurrentScope } from '@sentry/core';
-import type { EventEnvelope, IntegrationFn, Transaction } from '@sentry/types';
+import { defineIntegration, getActiveSpan, getRootSpan } from '@sentry/core';
+import type { EventEnvelope, IntegrationFn, Span } from '@sentry/types';
 import type { Profile } from '@sentry/types/src/profiling';
 import { logger } from '@sentry/utils';
 
 import { DEBUG_BUILD } from '../debug-build';
-import { startProfileForTransaction } from './startProfileForTransaction';
+import { startProfileForSpan } from './startProfileForSpan';
 import type { ProfiledEvent } from './utils';
+import { isAutomatedPageLoadSpan, shouldProfileSpan } from './utils';
 import {
   addProfilesToEnvelope,
   createProfilingEvent,
   findProfiledTransactionsFromEnvelope,
   getActiveProfilesCount,
-  isAutomatedPageLoadTransaction,
-  shouldProfileTransaction,
   takeProfileFromGlobalCache,
 } from './utils';
 
@@ -21,22 +20,19 @@ const INTEGRATION_NAME = 'BrowserProfiling';
 const _browserProfilingIntegration = (() => {
   return {
     name: INTEGRATION_NAME,
-    // TODO v8: Remove this
     setup(client) {
-      const scope = getCurrentScope();
+      const activeSpan = getActiveSpan();
+      const rootSpan = activeSpan && getRootSpan(activeSpan);
 
-      // eslint-disable-next-line deprecation/deprecation
-      const transaction = scope.getTransaction();
-
-      if (transaction && isAutomatedPageLoadTransaction(transaction)) {
-        if (shouldProfileTransaction(transaction)) {
-          startProfileForTransaction(transaction);
+      if (rootSpan && isAutomatedPageLoadSpan(rootSpan)) {
+        if (shouldProfileSpan(rootSpan)) {
+          startProfileForSpan(rootSpan);
         }
       }
 
-      client.on('startTransaction', (transaction: Transaction) => {
-        if (shouldProfileTransaction(transaction)) {
-          startProfileForTransaction(transaction);
+      client.on('spanStart', (span: Span) => {
+        if (span === getRootSpan(span) && shouldProfileSpan(span)) {
+          startProfileForSpan(span);
         }
       });
 
@@ -59,23 +55,23 @@ const _browserProfilingIntegration = (() => {
           const start_timestamp = context && context['profile'] && context['profile']['start_timestamp'];
 
           if (typeof profile_id !== 'string') {
-            DEBUG_BUILD && logger.log('[Profiling] cannot find profile for a transaction without a profile context');
+            DEBUG_BUILD && logger.log('[Profiling] cannot find profile for a span without a profile context');
             continue;
           }
 
           if (!profile_id) {
-            DEBUG_BUILD && logger.log('[Profiling] cannot find profile for a transaction without a profile context');
+            DEBUG_BUILD && logger.log('[Profiling] cannot find profile for a span without a profile context');
             continue;
           }
 
-          // Remove the profile from the transaction context before sending, relay will take care of the rest.
+          // Remove the profile from the span context before sending, relay will take care of the rest.
           if (context && context['profile']) {
             delete context.profile;
           }
 
           const profile = takeProfileFromGlobalCache(profile_id);
           if (!profile) {
-            DEBUG_BUILD && logger.log(`[Profiling] Could not retrieve profile for transaction: ${profile_id}`);
+            DEBUG_BUILD && logger.log(`[Profiling] Could not retrieve profile for span: ${profile_id}`);
             continue;
           }
 
diff --git a/packages/browser/src/profiling/startProfileForSpan.ts b/packages/browser/src/profiling/startProfileForSpan.ts
@@ -1,140 +1,132 @@
-import { spanToJSON } from '@sentry/core';
-import type { Transaction } from '@sentry/types';
+import { getCurrentScope, spanToJSON } from '@sentry/core';
+import type { Span } from '@sentry/types';
 import { logger, timestampInSeconds, uuid4 } from '@sentry/utils';
 
 import { DEBUG_BUILD } from '../debug-build';
 import { WINDOW } from '../helpers';
 import type { JSSelfProfile } from './jsSelfProfiling';
-import {
-  MAX_PROFILE_DURATION_MS,
-  addProfileToGlobalCache,
-  isAutomatedPageLoadTransaction,
-  startJSSelfProfile,
-} from './utils';
+import { MAX_PROFILE_DURATION_MS, addProfileToGlobalCache, isAutomatedPageLoadSpan, startJSSelfProfile } from './utils';
 
 /**
  * Wraps startTransaction and stopTransaction with profiling related logic.
  * startProfileForTransaction is called after the call to startTransaction in order to avoid our own code from
  * being profiled. Because of that same reason, stopProfiling is called before the call to stopTransaction.
  */
-export function startProfileForTransaction(transaction: Transaction): Transaction {
+export function startProfileForSpan(span: Span): void {
   // Start the profiler and get the profiler instance.
   let startTimestamp: number | undefined;
-  if (isAutomatedPageLoadTransaction(transaction)) {
+  if (isAutomatedPageLoadSpan(span)) {
     startTimestamp = timestampInSeconds() * 1000;
   }
 
   const profiler = startJSSelfProfile();
 
-  // We failed to construct the profiler, fallback to original transaction.
+  // We failed to construct the profiler, so we skip.
   // No need to log anything as this has already been logged in startProfile.
   if (!profiler) {
-    return transaction;
+    return;
   }
 
   if (DEBUG_BUILD) {
-    logger.log(`[Profiling] started profiling transaction: ${spanToJSON(transaction).description}`);
+    logger.log(`[Profiling] started profiling span: ${spanToJSON(span).description}`);
   }
 
-  // We create "unique" transaction names to avoid concurrent transactions with same names
-  // from being ignored by the profiler. From here on, only this transaction name should be used when
+  // We create "unique" span names to avoid concurrent spans with same names
+  // from being ignored by the profiler. From here on, only this span name should be used when
   // calling the profiler methods. Note: we log the original name to the user to avoid confusion.
   const profileId = uuid4();
 
   // A couple of important things to note here:
   // `CpuProfilerBindings.stopProfiling` will be scheduled to run in 30seconds in order to exceed max profile duration.
-  // Whichever of the two (transaction.finish/timeout) is first to run, the profiling will be stopped and the gathered profile
-  // will be processed when the original transaction is finished. Since onProfileHandler can be invoked multiple times in the
-  // event of an error or user mistake (calling transaction.finish multiple times), it is important that the behavior of onProfileHandler
+  // Whichever of the two (span.finish/timeout) is first to run, the profiling will be stopped and the gathered profile
+  // will be processed when the original span is finished. Since onProfileHandler can be invoked multiple times in the
+  // event of an error or user mistake (calling span.finish multiple times), it is important that the behavior of onProfileHandler
   // is idempotent as we do not want any timings or profiles to be overriden by the last call to onProfileHandler.
   // After the original finish method is called, the event will be reported through the integration and delegated to transport.
   const processedProfile: JSSelfProfile | null = null;
 
+  getCurrentScope().setContext('profile', {
+    profile_id: profileId,
+    start_timestamp: startTimestamp,
+  });
+
   /**
    * Idempotent handler for profile stop
    */
-  async function onProfileHandler(): Promise<null> {
-    // Check if the profile exists and return it the behavior has to be idempotent as users may call transaction.finish multiple times.
-    if (!transaction) {
-      return null;
+  async function onProfileHandler(): Promise<void> {
+    // Check if the profile exists and return it the behavior has to be idempotent as users may call span.finish multiple times.
+    if (!span) {
+      return;
     }
     // Satisfy the type checker, but profiler will always be defined here.
     if (!profiler) {
-      return null;
+      return;
     }
     if (processedProfile) {
       if (DEBUG_BUILD) {
-        logger.log('[Profiling] profile for:', spanToJSON(transaction).description, 'already exists, returning early');
+        logger.log('[Profiling] profile for:', spanToJSON(span).description, 'already exists, returning early');
       }
-      return null;
+      return;
     }
 
     return profiler
       .stop()
-      .then((profile: JSSelfProfile): null => {
+      .then((profile: JSSelfProfile): void => {
         if (maxDurationTimeoutID) {
           WINDOW.clearTimeout(maxDurationTimeoutID);
           maxDurationTimeoutID = undefined;
         }
 
         if (DEBUG_BUILD) {
-          logger.log(`[Profiling] stopped profiling of transaction: ${spanToJSON(transaction).description}`);
+          logger.log(`[Profiling] stopped profiling of span: ${spanToJSON(span).description}`);
         }
 
-        // In case of an overlapping transaction, stopProfiling may return null and silently ignore the overlapping profile.
+        // In case of an overlapping span, stopProfiling may return null and silently ignore the overlapping profile.
         if (!profile) {
           if (DEBUG_BUILD) {
             logger.log(
-              `[Profiling] profiler returned null profile for: ${spanToJSON(transaction).description}`,
-              'this may indicate an overlapping transaction or a call to stopProfiling with a profile title that was never started',
+              `[Profiling] profiler returned null profile for: ${spanToJSON(span).description}`,
+              'this may indicate an overlapping span or a call to stopProfiling with a profile title that was never started',
             );
           }
-          return null;
+          return;
         }
 
         addProfileToGlobalCache(profileId, profile);
-        return null;
       })
       .catch(error => {
         if (DEBUG_BUILD) {
           logger.log('[Profiling] error while stopping profiler:', error);
         }
-        return null;
       });
   }
 
   // Enqueue a timeout to prevent profiles from running over max duration.
   let maxDurationTimeoutID: number | undefined = WINDOW.setTimeout(() => {
     if (DEBUG_BUILD) {
-      logger.log(
-        '[Profiling] max profile duration elapsed, stopping profiling for:',
-        spanToJSON(transaction).description,
-      );
+      logger.log('[Profiling] max profile duration elapsed, stopping profiling for:', spanToJSON(span).description);
     }
-    // If the timeout exceeds, we want to stop profiling, but not finish the transaction
+    // If the timeout exceeds, we want to stop profiling, but not finish the span
     // eslint-disable-next-line @typescript-eslint/no-floating-promises
     onProfileHandler();
   }, MAX_PROFILE_DURATION_MS);
 
   // We need to reference the original end call to avoid creating an infinite loop
-  const originalEnd = transaction.end.bind(transaction);
+  const originalEnd = span.end.bind(span);
 
   /**
-   * Wraps startTransaction and stopTransaction with profiling related logic.
-   * startProfiling is called after the call to startTransaction in order to avoid our own code from
-   * being profiled. Because of that same reason, stopProfiling is called before the call to stopTransaction.
+   * Wraps span `end()` with profiling related logic.
+   * startProfiling is called after the call to spanStart in order to avoid our own code from
+   * being profiled. Because of that same reason, stopProfiling is called before the call to spanEnd.
    */
-  function profilingWrappedTransactionEnd(): Transaction {
-    if (!transaction) {
+  function profilingWrappedSpanEnd(): Span {
+    if (!span) {
       return originalEnd();
     }
     // onProfileHandler should always return the same profile even if this is called multiple times.
     // Always call onProfileHandler to ensure stopProfiling is called and the timeout is cleared.
     void onProfileHandler().then(
       () => {
-        // TODO: Can we rewrite this to use attributes?
-        // eslint-disable-next-line deprecation/deprecation
-        transaction.setContext('profile', { profile_id: profileId, start_timestamp: startTimestamp });
         originalEnd();
       },
       () => {
@@ -143,9 +135,8 @@ export function startProfileForTransaction(transaction: Transaction): Transactio
       },
     );
 
-    return transaction;
+    return span;
   }
 
-  transaction.end = profilingWrappedTransactionEnd;
-  return transaction;
+  span.end = profilingWrappedSpanEnd;
 }
diff --git a/packages/browser/src/profiling/utils.ts b/packages/browser/src/profiling/utils.ts
@@ -1,7 +1,7 @@
 /* eslint-disable max-lines */
 
 import { DEFAULT_ENVIRONMENT, getClient, spanToJSON } from '@sentry/core';
-import type { DebugImage, Envelope, Event, EventEnvelope, StackFrame, StackParser, Transaction } from '@sentry/types';
+import type { DebugImage, Envelope, Event, EventEnvelope, Span, StackFrame, StackParser } from '@sentry/types';
 import type { Profile, ThreadCpuProfile } from '@sentry/types/src/profiling';
 import { GLOBAL_OBJ, browserPerformanceTimeOrigin, forEachEnvelopeItem, logger, uuid4 } from '@sentry/utils';
 
@@ -201,8 +201,8 @@ export function isProfiledTransactionEvent(event: Event): event is ProfiledEvent
 /**
  *
  */
-export function isAutomatedPageLoadTransaction(transaction: Transaction): boolean {
-  return spanToJSON(transaction).op === 'pageload';
+export function isAutomatedPageLoadSpan(span: Span): boolean {
+  return spanToJSON(span).op === 'pageload';
 }
 
 /**
@@ -506,7 +506,7 @@ export function startJSSelfProfile(): JSSelfProfiler | undefined {
 /**
  * Determine if a profile should be profiled.
  */
-export function shouldProfileTransaction(transaction: Transaction): boolean {
+export function shouldProfileSpan(span: Span): boolean {
   // If constructor failed once, it will always fail, so we can early return.
   if (PROFILING_CONSTRUCTOR_FAILED) {
     if (DEBUG_BUILD) {
@@ -515,7 +515,7 @@ export function shouldProfileTransaction(transaction: Transaction): boolean {
     return false;
   }
 
-  if (!transaction.isRecording()) {
+  if (!span.isRecording()) {
     if (DEBUG_BUILD) {
       logger.log('[Profiling] Discarding profile because transaction was not sampled.');
     }
