Condense options and move stalltracking into tracing integration

Author: jennmueng

src/js/integrations/index.ts

@@ -2,4 +2,3 @@ export { DebugSymbolicator } from "./debugsymbolicator";
 export { DeviceContext } from "./devicecontext";
 export { ReactNativeErrorHandlers } from "./reactnativeerrorhandlers";
 export { Release } from "./release";
-export { StallTracking } from "./stalltracking";

src/js/measurements.ts

@@ -1,9 +1,7 @@
 import { getCurrentHub, getMainCarrier, Hub } from "@sentry/hub";
 import { Transaction } from "@sentry/tracing";
 import { CustomSamplingContext, TransactionContext } from "@sentry/types";
-import { logger } from "@sentry/utils";
 
-import { StallTracking } from "./integrations";
 import { ReactNativeTracing } from "./tracing";
 
 /**
@@ -58,40 +56,20 @@ const _patchStartTransaction = (
     const reactNativeTracing = getCurrentHub().getIntegration(
       ReactNativeTracing
     );
-    const stallTracking = getCurrentHub().getIntegration(StallTracking);
 
     if (reactNativeTracing) {
       reactNativeTracing.onTransactionStart(transaction);
-    }
-
-    if (stallTracking) {
-      if (!transactionContext.startTimestamp) {
-        const finishStallTracking = stallTracking.registerTransactionStart(
-          transaction
-        );
-
-        // eslint-disable-next-line @typescript-eslint/unbound-method
-        const originalFinish = transaction.finish;
-
-        transaction.finish = (endTimestamp: number | undefined) => {
-          const stallMeasurements = finishStallTracking();
 
-          // Sometimes the measurements will not be tracked due to some underlying reason, we don't add them in that case.
-          if (stallMeasurements) {
-            transaction.setMeasurements(stallMeasurements);
-          }
+      // eslint-disable-next-line @typescript-eslint/unbound-method
+      const originalFinish = transaction.finish;
 
-          if (reactNativeTracing) {
-            reactNativeTracing.onTransactionFinish(transaction);
-          }
+      transaction.finish = (endTimestamp: number | undefined) => {
+        if (reactNativeTracing) {
+          reactNativeTracing.onTransactionFinish(transaction);
+        }
 
-          return originalFinish.apply(transaction, [endTimestamp]);
-        };
-      } else {
-        logger.log(
-          "[StallTracking] Stalls will not be tracked due to `startTimestamp` being set."
-        );
-      }
+        return originalFinish.apply(transaction, [endTimestamp]);
+      };
     }
 
     return transaction;

src/js/options.ts

@@ -70,9 +70,6 @@ export interface ReactNativeOptions extends BrowserOptions {
     didCallNativeInit: boolean;
   }) => void;
 
-  /** Enable JS event loop stall tracking. Enabled by default. */
-  enableStallTracking?: boolean;
-
   /** Enable auto performance tracking by default. */
   enableAutoPerformanceTracking?: boolean;
 }

src/js/sdk.tsx

@@ -32,7 +32,6 @@ const DEFAULT_OPTIONS: ReactNativeOptions = {
   enableNativeCrashHandling: true,
   enableNativeNagger: true,
   autoInitializeNativeSdk: true,
-  enableStallTracking: true,
   enableAutoPerformanceTracking: true,
 };
 
@@ -95,10 +94,6 @@ function _init<O = ReactNativeOptions>(passedOptions: O): O {
     if (tracingEnabled) {
       if (options.enableAutoPerformanceTracking) {
         options.defaultIntegrations.push(new ReactNativeTracing());
-
-        if (options.enableStallTracking) {
-          options.defaultIntegrations.push(new StallTracking());
-        }
       }
     }
   }

src/js/tracing/nativeframes.ts

@@ -4,19 +4,13 @@ import { logger, timestampInSeconds } from "@sentry/utils";
 
 import { NativeFramesResponse } from "../definitions";
 import { NATIVE } from "../wrapper";
-import { instrumentChildSpanFinish } from "./utils";
+import { instrumentChildSpanFinish, MARGIN_OF_ERROR_SECONDS } from "./utils";
 
 type FramesMeasurements = Record<
   "frames_total" | "frames_slow" | "frames_frozen",
   { value: number }
 >;
 
-/**
- * A margin of error of 50ms is allowed for the async native bridge call.
- * Anything larger would reduce the accuracy of our frames measurements.
- */
-const MARGIN_OF_ERROR_SECONDS = 0.05;
-
 /**
  * Instrumentation to add native slow/frozen frames measurements onto transactions.
  */

src/js/tracing/reactnativetracing.ts

@@ -17,11 +17,12 @@ import {
 import { logger } from "@sentry/utils";
 
 import { NativeAppStartResponse } from "../definitions";
-import { StallTracking } from "../integrations";
+import { ReactNativeOptions } from "../options";
 import { RoutingInstrumentationInstance } from "../tracing/routingInstrumentation";
 import { NATIVE } from "../wrapper";
 import { NativeFramesInstrumentation } from "./nativeframes";
-import { adjustTransactionDuration } from "./utils";
+import { StallTrackingInstrumentation } from "./stalltracking";
+import { adjustTransactionDuration, isNearToNow } from "./utils";
 
 export type BeforeNavigate = (
   context: TransactionContext
@@ -70,19 +71,6 @@ export interface ReactNativeTracingOptions
    * @returns A (potentially) modified context object, with `sampled = false` if the transaction should be dropped.
    */
   beforeNavigate: BeforeNavigate;
-
-  /**
-   * Track the app start time by adding measurements to the first route transaction. If there is no routing instrumentation
-   * an app start transaction will be started.
-   *
-   * Default: true
-   */
-  enableAppStartTracking: boolean;
-
-  /**
-   * Track slow/frozen frames from the native layer and adds them as measurements to all transactions.
-   */
-  enableNativeFramesTracking: boolean;
 }
 
 const defaultReactNativeTracingOptions: ReactNativeTracingOptions = {
@@ -91,8 +79,6 @@ const defaultReactNativeTracingOptions: ReactNativeTracingOptions = {
   maxTransactionDuration: 600,
   ignoreEmptyBackNavigationTransactions: true,
   beforeNavigate: (context) => context,
-  enableAppStartTracking: true,
-  enableNativeFramesTracking: true,
 };
 
 /**
@@ -112,10 +98,12 @@ export class ReactNativeTracing implements Integration {
   public options: ReactNativeTracingOptions;
 
   public nativeFramesInstrumentation?: NativeFramesInstrumentation;
+  public stallTrackingInstrumentation?: StallTrackingInstrumentation;
 
   private _getCurrentHub?: () => Hub;
   private _awaitingAppStartData?: NativeAppStartResponse;
   private _appStartFinishTimestamp?: number;
+  private _sentryOptions?: ReactNativeOptions;
 
   public constructor(options: Partial<ReactNativeTracingOptions> = {}) {
     this.options = {
@@ -140,14 +128,14 @@ export class ReactNativeTracing implements Integration {
       // @ts-ignore TODO
       shouldCreateSpanForRequest,
       routingInstrumentation,
-      enableNativeFramesTracking,
     } = this.options;
 
+    this._sentryOptions = getCurrentHub()?.getClient()?.getOptions();
     this._getCurrentHub = getCurrentHub;
 
     void this._instrumentAppStart();
 
-    if (enableNativeFramesTracking) {
+    if (this._sentryOptions?.enableAutoPerformanceTracking) {
       this.nativeFramesInstrumentation = new NativeFramesInstrumentation(
         addGlobalEventProcessor,
         () => {
@@ -160,6 +148,7 @@ export class ReactNativeTracing implements Integration {
           return false;
         }
       );
+      this.stallTrackingInstrumentation = new StallTrackingInstrumentation();
     } else {
       NATIVE.disableNativeFramesTracking();
     }
@@ -187,14 +176,25 @@ export class ReactNativeTracing implements Integration {
    * To be called on a transaction start. Can have async methods
    */
   public onTransactionStart(transaction: Transaction): void {
-    this.nativeFramesInstrumentation?.onTransactionStart(transaction);
+    if (isNearToNow(transaction.startTimestamp)) {
+      // Only if this method is called at or within margin of error to the start timestamp.
+      this.nativeFramesInstrumentation?.onTransactionStart(transaction);
+      this.stallTrackingInstrumentation?.onTransactionStart(transaction);
+    }
   }
 
   /**
    * To be called on a transaction finish. Cannot have async methods.
    */
-  public onTransactionFinish(transaction: Transaction): void {
+  public onTransactionFinish(
+    transaction: Transaction,
+    endTimestamp?: number
+  ): void {
     this.nativeFramesInstrumentation?.onTransactionFinish(transaction);
+    this.stallTrackingInstrumentation?.onTransactionFinish(
+      transaction,
+      endTimestamp
+    );
   }
 
   /**
@@ -209,7 +209,10 @@ export class ReactNativeTracing implements Integration {
    * Starts a route transaction if there isn't routing instrumentation.
    */
   private async _instrumentAppStart(): Promise<void> {
-    if (!this.options.enableAppStartTracking || !NATIVE.enableNative) {
+    if (
+      !this._sentryOptions?.enableAutoPerformanceTracking ||
+      !NATIVE.enableNative
+    ) {
       return;
     }
 
@@ -316,12 +319,17 @@ export class ReactNativeTracing implements Integration {
       `[ReactNativeTracing] Starting ${context.op} transaction "${context.name}" on scope`
     );
 
-    idleTransaction.registerBeforeFinishCallback((transaction) => {
-      this.onTransactionFinish(transaction);
-    });
+    idleTransaction.registerBeforeFinishCallback(
+      (transaction, endTimestamp) => {
+        this.onTransactionFinish(transaction, endTimestamp);
+      }
+    );
 
     idleTransaction.registerBeforeFinishCallback((transaction) => {
-      if (this.options.enableAppStartTracking && this._awaitingAppStartData) {
+      if (
+        this._sentryOptions?.enableAutoPerformanceTracking &&
+        this._awaitingAppStartData
+      ) {
         transaction.startTimestamp =
           this._awaitingAppStartData.appStartTime / 1000;
         transaction.op = "ui.load";
@@ -332,24 +340,6 @@ export class ReactNativeTracing implements Integration {
       }
     });
 
-    const stallTracking = this._getCurrentHub().getIntegration(StallTracking);
-
-    if (stallTracking) {
-      const stallTrackingFinish = stallTracking.registerTransactionStart(
-        idleTransaction
-      );
-
-      idleTransaction.registerBeforeFinishCallback(
-        (transaction, endTimestamp) => {
-          const stallMeasurements = stallTrackingFinish(endTimestamp);
-
-          if (stallMeasurements) {
-            transaction.setMeasurements(stallMeasurements);
-          }
-        }
-      );
-    }
-
     idleTransaction.registerBeforeFinishCallback(
       (transaction, endTimestamp) => {
         adjustTransactionDuration(