feat: Use beforeNavigate in routing instrumentation to match behavior on JS (#1313)

* feat: Use beforeNavigate in routing instrumentatin

* feat: Add convenience type for react navigation context

* feat: Sample app beforeNavigate example

* feat: Safeguard for beforeNavigate returning false

* fix: Update hasBeenSeen callback and add a log for debug

* meta: Changelog

* fix: Fix V4 previous route

* test: Updated V4 tests

Author: jennmueng

CHANGELOG.md

@@ -5,6 +5,7 @@
 - Bump: sentry-android to v4.0.0 #1309
 - build(ios): Bump sentry-cocoa to 6.1.4 #1308
 - fix: Handle auto session tracking start on iOS #1308
+- feat: Use beforeNavigate in routing instrumentation to match behavior on JS #1313
 
 ## 2.2.0-beta.0
 

sample/src/App.tsx

@@ -20,17 +20,7 @@ import {store} from './reduxApp';
 import {version as packageVersion} from '../../package.json';
 import {SENTRY_INTERNAL_DSN} from './dsn';
 
-const reactNavigationV5Instrumentation = new Sentry.ReactNavigationV5Instrumentation(
-  {
-    shouldSendTransaction: (route, previousRoute) => {
-      if (route.name === 'ManualTracker') {
-        return false;
-      }
-
-      return true;
-    },
-  },
-);
+const reactNavigationV5Instrumentation = new Sentry.ReactNavigationV5Instrumentation();
 
 Sentry.init({
   // Replace the example DSN below with your own DSN:
@@ -46,6 +36,14 @@ Sentry.init({
       idleTimeout: 5000,
       routingInstrumentation: reactNavigationV5Instrumentation,
       tracingOrigins: ['localhost', /^\//, /^https:\/\//],
+      beforeNavigate: (context: Sentry.ReactNavigationTransactionContext) => {
+        // Example of not sending a transaction for the screen with the name "Manual Tracker"
+        if (context.data.route.name === 'ManualTracker') {
+          context.sampled = false;
+        }
+
+        return context;
+      },
     }),
   ],
   enableAutoSessionTracking: true,

src/js/index.ts

@@ -62,6 +62,7 @@ export {
   ReactNavigationV4Instrumentation,
   ReactNavigationV5Instrumentation,
   RoutingInstrumentation,
+  ReactNavigationTransactionContext,
 } from "./tracing";
 
 /**

src/js/tracing/index.ts

@@ -1,7 +1,14 @@
 export { ReactNativeTracing } from "./reactnativetracing";
-export { ReactNavigationV5Instrumentation } from "./reactnavigationv5";
-export { ReactNavigationV4Instrumentation } from "./reactnavigationv4";
+
 export {
   RoutingInstrumentation,
   RoutingInstrumentationInstance,
 } from "./routingInstrumentation";
+
+export { ReactNavigationV5Instrumentation } from "./reactnavigationv5";
+export { ReactNavigationV4Instrumentation } from "./reactnavigationv4";
+export {
+  ReactNavigationCurrentRoute,
+  ReactNavigationRoute,
+  ReactNavigationTransactionContext,
+} from "./types";

src/js/tracing/reactnativetracing.ts

@@ -16,6 +16,10 @@ import { logger } from "@sentry/utils";
 import { RoutingInstrumentationInstance } from "../tracing/routingInstrumentation";
 import { adjustTransactionDuration } from "./utils";
 
+export type BeforeNavigate = (
+  context: TransactionContext
+) => TransactionContext;
+
 export interface ReactNativeTracingOptions
   extends RequestInstrumentationOptions {
   /**
@@ -48,7 +52,7 @@ export interface ReactNativeTracingOptions
    *
    * Default: true
    */
-  ignoreEmptyBackNavigationTransactions?: boolean;
+  ignoreEmptyBackNavigationTransactions: boolean;
 
   /**
    * beforeNavigate is called before a navigation transaction is created and allows users to modify transaction
@@ -58,14 +62,15 @@ export interface ReactNativeTracingOptions
    *
    * @returns A (potentially) modified context object, with `sampled = false` if the transaction should be dropped.
    */
-  beforeNavigate?(context: TransactionContext): TransactionContext;
+  beforeNavigate: BeforeNavigate;
 }
 
 const defaultReactNativeTracingOptions: ReactNativeTracingOptions = {
   ...defaultRequestInstrumentationOptions,
   idleTimeout: 1000,
   maxTransactionDuration: 600,
   ignoreEmptyBackNavigationTransactions: true,
+  beforeNavigate: (context) => context,
 };
 
 /**
@@ -114,7 +119,8 @@ export class ReactNativeTracing implements Integration {
     this._getCurrentHub = getCurrentHub;
 
     routingInstrumentation?.registerRoutingInstrumentation(
-      this._onRouteWillChange.bind(this)
+      this._onRouteWillChange.bind(this),
+      this.options.beforeNavigate
     );
 
     if (!routingInstrumentation) {
@@ -129,14 +135,6 @@ export class ReactNativeTracing implements Integration {
       tracingOrigins,
       shouldCreateSpanForRequest,
     });
-
-    addGlobalEventProcessor((event) => {
-      // eslint-disable-next-line no-empty
-      if (event.type === "transaction") {
-      }
-
-      return event;
-    });
   }
 
   /** To be called when the route changes, but BEFORE the components of the new route mount. */
@@ -159,37 +157,22 @@ export class ReactNativeTracing implements Integration {
     }
 
     // eslint-disable-next-line @typescript-eslint/unbound-method
-    const {
-      beforeNavigate,
-      idleTimeout,
-      maxTransactionDuration,
-    } = this.options;
+    const { idleTimeout, maxTransactionDuration } = this.options;
 
     const expandedContext = {
       ...context,
       trimEnd: true,
     };
 
-    const modifiedContext =
-      typeof beforeNavigate === "function"
-        ? beforeNavigate(expandedContext)
-        : expandedContext;
-
-    if (modifiedContext.sampled === false) {
-      logger.log(
-        `[ReactNativeTracing] Will not send ${context.op} transaction.`
-      );
-    }
-
     const hub = this._getCurrentHub();
     const idleTransaction = startIdleTransaction(
       hub as any,
-      context,
+      expandedContext,
       idleTimeout,
       true
     );
     logger.log(
-      `[ReactNativeTracing] Starting ${context.op} transaction on scope`
+      `[ReactNativeTracing] Starting ${context.op} transaction "${context.name}" on scope`
     );
     idleTransaction.registerBeforeFinishCallback(
       (transaction, endTimestamp) => {
@@ -204,12 +187,16 @@ export class ReactNativeTracing implements Integration {
     if (this.options.ignoreEmptyBackNavigationTransactions) {
       idleTransaction.registerBeforeFinishCallback((transaction) => {
         if (
-          transaction.data["routing.route.hasBeenSeen"] &&
+          // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
+          transaction.data?.route?.hasBeenSeen &&
           (!transaction.spanRecorder ||
             transaction.spanRecorder.spans.filter(
               (span) => span.spanId !== transaction.spanId
             ).length === 0)
         ) {
+          logger.log(
+            `[ReactNativeTracing] Not sampling transaction as route has been seen before. Pass ignoreEmptyBackNavigationTransactions = false to disable this feature.`
+          );
           // Route has been seen before and has no child spans.
           transaction.sampled = false;
         }

src/js/tracing/reactnavigationv4.ts

@@ -1,12 +1,13 @@
-import { TransactionContext } from "@sentry/types";
 import { logger } from "@sentry/utils";
 
 import { RoutingInstrumentation } from "./routingInstrumentation";
+import { ReactNavigationTransactionContext } from "./types";
 
 export interface NavigationRouteV4 {
   routeName: string;
   key: string;
-  params?: Record<any, any>;
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  params?: Record<string, any>;
 }
 
 export interface NavigationStateV4 {
@@ -22,6 +23,7 @@ export interface AppContainerInstance {
     state: NavigationStateV4;
     router: {
       getStateForAction: (
+        // eslint-disable-next-line @typescript-eslint/no-explicit-any
         action: any,
         state: NavigationStateV4
       ) => NavigationStateV4;
@@ -33,22 +35,6 @@ interface AppContainerRef {
   current?: AppContainerInstance | null;
 }
 
-type ReactNavigationV4ShouldAttachTransaction = (
-  route: NavigationRouteV4,
-  previousRoute: NavigationRouteV4 | null
-) => boolean;
-
-interface ReactNavigationV4InstrumentationOptions {
-  shouldSendTransaction: ReactNavigationV4ShouldAttachTransaction;
-}
-
-const defaultShouldAttachTransaction: ReactNavigationV4ShouldAttachTransaction = () =>
-  true;
-
-const DEFAULT_OPTIONS: ReactNavigationV4InstrumentationOptions = {
-  shouldSendTransaction: defaultShouldAttachTransaction,
-};
-
 /**
  * Instrumentation for React-Navigation V4.
  * Register the app container with `registerAppContainer` to use, or see docs for more details.
@@ -58,22 +44,11 @@ class ReactNavigationV4Instrumentation extends RoutingInstrumentation {
 
   private _appContainerRef: AppContainerRef = { current: null };
 
-  private _options: ReactNavigationV4InstrumentationOptions;
-
   private readonly _maxRecentRouteLen: number = 200;
 
-  private _prevRoute: NavigationRouteV4 | null = null;
+  private _prevRoute?: NavigationRouteV4;
   private _recentRouteKeys: string[] = [];
 
-  constructor(options: Partial<ReactNavigationV4InstrumentationOptions> = {}) {
-    super();
-
-    this._options = {
-      ...DEFAULT_OPTIONS,
-      ...options,
-    };
-  }
-
   /**
    * Pass the ref to the app container to register it to the instrumentation
    * @param appContainerRef Ref to an `AppContainer`
@@ -142,16 +117,31 @@ class ReactNavigationV4Instrumentation extends RoutingInstrumentation {
 
     // 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);
+      const originalContext = this._getTransactionContext(
+        currentRoute,
+        this._prevRoute
+      );
+      let finalContext = this._beforeNavigate?.(originalContext);
+
+      // This block is to catch users not returning a transaction context
+      if (!finalContext) {
+        logger.error(
+          `[ReactNavigationV4Instrumentation] beforeNavigate returned ${finalContext}, return context.sampled = false to not send transaction.`
+        );
+
+        finalContext = {
+          ...originalContext,
+          sampled: false,
+        };
+      }
 
-      if (!this._options.shouldSendTransaction(currentRoute, this._prevRoute)) {
-        context.sampled = false;
+      if (finalContext.sampled === false) {
         logger.log(
-          `[ReactNavigationV4Instrumentation] Will not send transaction "${context.name}" due to shouldSendTransaction.`
+          `[ReactNavigationV4Instrumentation] Will not send transaction "${finalContext.name}" due to beforeNavigate.`
         );
       }
 
-      this.onRouteWillChange(context);
+      this.onRouteWillChange(finalContext);
 
       this._pushRecentRouteKey(currentRoute.key);
       this._prevRoute = currentRoute;
@@ -161,7 +151,10 @@ class ReactNavigationV4Instrumentation extends RoutingInstrumentation {
   /**
    * Gets the transaction context for a `NavigationRouteV4`
    */
-  private _getTransactionContext(route: NavigationRouteV4): TransactionContext {
+  private _getTransactionContext(
+    route: NavigationRouteV4,
+    previousRoute?: NavigationRouteV4
+  ): ReactNavigationTransactionContext {
     return {
       name: route.routeName,
       op: "navigation",
@@ -171,9 +164,19 @@ class ReactNavigationV4Instrumentation extends RoutingInstrumentation {
         "routing.route.name": route.routeName,
       },
       data: {
-        "routing.route.key": route.key,
-        "routing.route.params": route.params,
-        "routing.route.hasBeenSeen": this._recentRouteKeys.includes(route.key),
+        route: {
+          name: route.routeName, // Include name here too for use in `beforeNavigate`
+          key: route.key,
+          params: route.params ?? {},
+          hasBeenSeen: this._recentRouteKeys.includes(route.key),
+        },
+        previousRoute: previousRoute
+          ? {
+              name: previousRoute.routeName,
+              key: previousRoute.key,
+              params: previousRoute.params ?? {},
+            }
+          : null,
       },
     };
   }