ref: Use shouldSendTransaction for both instrumentations

Author: jennmueng

sample/src/App.tsx

@@ -21,7 +21,7 @@ import {SENTRY_INTERNAL_DSN} from './dsn';
 
 const reactNavigationV5Instrumentation = new Sentry.Tracing.ReactNavigationV5Instrumentation(
   {
-    shouldAttachTransaction: (route) => {
+    shouldSendTransaction: (route, previousRoute) => {
       if (route.name === 'ManualTracker') {
         return false;
       }

src/js/tracing/reactnavigationv4.ts

@@ -3,28 +3,28 @@ import { logger } from "@sentry/utils";
 
 import { RoutingInstrumentation } from "./router";
 
-interface NavigationRoute {
+interface NavigationRouteV4 {
   routeName: string;
   key: string;
   params?: Record<any, any>;
 }
 
-interface NavigationState {
+interface NavigationStateV4 {
   index: number;
   key: string;
   isTransitioning: boolean;
   routeName?: string;
-  routes: (NavigationRoute | NavigationState)[];
+  routes: (NavigationRouteV4 | NavigationStateV4)[];
 }
 
 interface AppContainerInstance {
   _navigation: {
-    state: NavigationState;
+    state: NavigationStateV4;
     router: {
       getStateForAction: (
         action: any,
-        state: NavigationState
-      ) => NavigationState;
+        state: NavigationStateV4
+      ) => NavigationStateV4;
     };
   };
 }
@@ -34,19 +34,19 @@ interface AppContainerRef {
 }
 
 type ReactNavigationV4ShouldAttachTransaction = (
-  prevRoute: NavigationRoute | null,
-  newRoute: NavigationRoute
+  route: NavigationRouteV4,
+  previousRoute: NavigationRouteV4 | null
 ) => boolean;
 
 interface ReactNavigationV4InstrumentationOptions {
-  shouldAttachTransaction: ReactNavigationV4ShouldAttachTransaction;
+  shouldSendTransaction: ReactNavigationV4ShouldAttachTransaction;
 }
 
 const defaultShouldAttachTransaction: ReactNavigationV4ShouldAttachTransaction = () =>
   true;
 
 const DEFAULT_OPTIONS: ReactNavigationV4InstrumentationOptions = {
-  shouldAttachTransaction: defaultShouldAttachTransaction,
+  shouldSendTransaction: defaultShouldAttachTransaction,
 };
 
 /**
@@ -60,7 +60,7 @@ class ReactNavigationV4Instrumentation extends RoutingInstrumentation {
 
   private _options: ReactNavigationV4InstrumentationOptions;
 
-  private _prevRoute: NavigationRoute | null = null;
+  private _prevRoute: NavigationRouteV4 | null = null;
 
   constructor(options: Partial<ReactNavigationV4InstrumentationOptions>) {
     super();
@@ -103,17 +103,9 @@ class ReactNavigationV4Instrumentation extends RoutingInstrumentation {
    */
   private _handleInitialState(): void {
     if (this._appContainerRef.current) {
-      const navigationState = this._appContainerRef.current._navigation.state;
+      const state = this._appContainerRef.current._navigation.state;
 
-      const currentRoute = this._getCurrentRouteFromState(navigationState);
-
-      if (
-        this._options.shouldAttachTransaction(this._prevRoute, currentRoute)
-      ) {
-        this.onRouteWillChange(this._getTransactionContext(currentRoute));
-      }
-
-      this._prevRoute = currentRoute;
+      this._onStateChange(state);
     }
   }
 
@@ -130,35 +122,41 @@ class ReactNavigationV4Instrumentation extends RoutingInstrumentation {
         action,
         state
       ) => {
-        const newNavigationState = originalGetStateForAction(action, state);
-        const currentRoute = this._getCurrentRouteFromState(newNavigationState);
-
-        // If the route is a different key, this is so we ignore actions that pertain to the same screen.
-        if (!this._prevRoute || currentRoute.key !== this._prevRoute.key) {
-          const context = this._getTransactionContext(currentRoute);
-
-          if (
-            this._options.shouldAttachTransaction(this._prevRoute, currentRoute)
-          ) {
-            this.onRouteWillChange(context);
-          } else {
-            logger.log(
-              `[ReactNavigationV4Instrumentation] Will not send transaction "${context.name}" due to shouldAttachTransaction.`
-            );
-          }
-
-          this._prevRoute = currentRoute;
-        }
-
-        return newNavigationState;
+        const newState = originalGetStateForAction(action, state);
+
+        this._onStateChange(newState);
+
+        return newState;
       };
     }
   }
 
   /**
-   * Gets the transaction context for a `NavigationRoute`
+   * To be called on navigation state changes and creates the transaction.
+   */
+  private _onStateChange(state: NavigationStateV4): void {
+    const currentRoute = this._getCurrentRouteFromState(state);
+
+    // If the route is a different key, this is so we ignore actions that pertain to the same screen.
+    if (!this._prevRoute || currentRoute.key !== this._prevRoute.key) {
+      const context = this._getTransactionContext(currentRoute);
+
+      if (this._options.shouldSendTransaction(currentRoute, this._prevRoute)) {
+        this.onRouteWillChange(context);
+      } else {
+        logger.log(
+          `[ReactNavigationV4Instrumentation] Will not send transaction "${context.name}" due to shouldSendTransaction.`
+        );
+      }
+
+      this._prevRoute = currentRoute;
+    }
+  }
+
+  /**
+   * Gets the transaction context for a `NavigationRouteV4`
    */
-  private _getTransactionContext(route: NavigationRoute): TransactionContext {
+  private _getTransactionContext(route: NavigationRouteV4): TransactionContext {
     return {
       name: route.routeName,
       op: "navigation",
@@ -174,7 +172,9 @@ class ReactNavigationV4Instrumentation extends RoutingInstrumentation {
   /**
    * Gets the current route given a navigation state
    */
-  private _getCurrentRouteFromState(state: NavigationState): NavigationRoute {
+  private _getCurrentRouteFromState(
+    state: NavigationStateV4
+  ): NavigationRouteV4 {
     const parentRoute = state.routes[state.index];
 
     if (
@@ -186,7 +186,7 @@ class ReactNavigationV4Instrumentation extends RoutingInstrumentation {
       return this._getCurrentRouteFromState(parentRoute);
     }
 
-    return parentRoute as NavigationRoute;
+    return parentRoute as NavigationRouteV4;
   }
 }
 

src/js/tracing/reactnavigationv5.ts

@@ -14,7 +14,7 @@ interface NavigationContainer {
 }
 
 interface ReactNavigationInstrumentationOptions {
-  shouldAttachTransaction(
+  shouldSendTransaction?(
     route: NavigationRoute,
     previousRoute?: NavigationRoute
   ): boolean;
@@ -24,17 +24,6 @@ type NavigationContainerRef = {
   current: NavigationContainer | null;
 };
 
-const defaultShouldAttachTransaction = (
-  route: NavigationRoute,
-  previousRoute?: NavigationRoute
-): boolean => {
-  return !previousRoute || previousRoute.key !== route.key;
-};
-
-const DEFAULT_OPTIONS: ReactNavigationInstrumentationOptions = {
-  shouldAttachTransaction: defaultShouldAttachTransaction,
-};
-
 const STATE_CHANGE_TIMEOUT_DURATION = 200;
 
 /**
@@ -61,10 +50,7 @@ export class ReactNavigationV5Instrumentation extends RoutingInstrumentation {
   constructor(_options: Partial<ReactNavigationInstrumentationOptions> = {}) {
     super();
 
-    this._options = {
-      ...DEFAULT_OPTIONS,
-      ..._options,
-    };
+    this._options = _options;
   }
 
   /**
@@ -115,23 +101,32 @@ export class ReactNavigationV5Instrumentation extends RoutingInstrumentation {
    */
   private _onStateChange(): void {
     // Use the getCurrentRoute method to be accurate.
+    const previousRoute = this._latestRoute;
     const route = this._navigationContainerRef?.current?.getCurrentRoute();
 
     if (route) {
-      if (this._latestTransaction) {
+      if (
+        this._latestTransaction &&
+        (!previousRoute || previousRoute.key !== route.key)
+      ) {
         this._latestTransaction.setName(route.name);
         this._latestTransaction.setTag("routing.route.key", route.key);
         this._latestTransaction.setData("routing.params", route.params);
 
-        if (this._options.shouldAttachTransaction(route, this._latestRoute)) {
+        const willSendTransaction =
+          typeof this._options.shouldSendTransaction === "function"
+            ? this._options.shouldSendTransaction(route, previousRoute)
+            : true;
+
+        if (willSendTransaction) {
           // Clear the timeout so the transaction does not get cancelled.
           if (typeof this._stateChangeTimeout !== "undefined") {
             clearTimeout(this._stateChangeTimeout);
             this._stateChangeTimeout = undefined;
           }
         } else {
           logger.log(
-            `[ReactNavigationV5Instrumentation] Will not send transaction "${this._latestTransaction.name}" due to shouldAttachTransaction.`
+            `[ReactNavigationV5Instrumentation] Will not send transaction "${this._latestTransaction.name}" due to shouldSendTransaction.`
           );
         }
       }