feat(tracing): Use React Navigation dispatched action data to discard noops (#4684)

Author: krystofwoldrich

CHANGELOG.md

@@ -11,6 +11,7 @@
 ### Features
 
 - Improve Warm App Start reporting on Android ([#4641](https://github.com/getsentry/sentry-react-native/pull/4641))
+- Add `createTimeToInitialDisplay({useFocusEffect})` and `createTimeToFullDisplay({useFocusEffect})` to allow record full display on screen focus ([#4665](https://github.com/getsentry/sentry-react-native/pull/4665))
 - Add support for measuring Time to Initial Display for already seen routes ([#4661](https://github.com/getsentry/sentry-react-native/pull/4661))
   - Introduce `enableTimeToInitialDisplayForPreloadedRoutes` option to the React Navigation integration.
 
@@ -20,7 +21,14 @@
   });
   ```
 
-- Add `createTimeToInitialDisplay({useFocusEffect})` and `createTimeToFullDisplay({useFocusEffect})` to allow record full display on screen focus ([#4665](https://github.com/getsentry/sentry-react-native/pull/4665))
+- Add `useDispatchedActionData` option to the React Navigation integration to filter out navigation actions that should not create spans ([#4684](https://github.com/getsentry/sentry-react-native/pull/4684))
+  - For example `PRELOAD`, `SET_PARAMS`, `TOGGLE_DRAWER` and others.
+
+  ```js
+  Sentry.reactNavigationIntegration({
+    useDispatchedActionData: true,
+  });
+  ```
 
 ### Fixes
 

packages/core/src/js/tracing/reactnavigation.ts

@@ -16,12 +16,13 @@ import {
 import { getAppRegistryIntegration } from '../integrations/appRegistry';
 import { isSentrySpan } from '../utils/span';
 import { RN_GLOBAL_OBJ } from '../utils/worldwide';
+import type { UnsafeAction } from '../vendor/react-navigation/types';
 import { NATIVE } from '../wrapper';
 import { ignoreEmptyBackNavigation } from './onSpanEndUtils';
 import { SPAN_ORIGIN_AUTO_NAVIGATION_REACT_NAVIGATION } from './origin';
 import type { ReactNativeTracingIntegration } from './reactnativetracing';
 import { getReactNativeTracingIntegration } from './reactnativetracing';
-import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from './semanticAttributes';
+import { SEMANTIC_ATTRIBUTE_NAVIGATION_ACTION_TYPE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from './semanticAttributes';
 import {
   DEFAULT_NAVIGATION_SPAN_NAME,
   defaultIdleOptions,
@@ -65,6 +66,13 @@ interface ReactNavigationIntegrationOptions {
    * @default false
    */
   enableTimeToInitialDisplayForPreloadedRoutes: boolean;
+
+  /**
+   * Whether to use the dispatched action data to populate the transaction metadata.
+   *
+   * @default false
+   */
+  useDispatchedActionData: boolean;
 }
 
 /**
@@ -80,6 +88,7 @@ export const reactNavigationIntegration = ({
   enableTimeToInitialDisplay = false,
   ignoreEmptyBackNavigationTransactions = true,
   enableTimeToInitialDisplayForPreloadedRoutes = false,
+  useDispatchedActionData = false,
 }: Partial<ReactNavigationIntegrationOptions> = {}): Integration & {
   /**
    * Pass the ref to the navigation container to register it to the instrumentation
@@ -201,7 +210,30 @@ export const reactNavigationIntegration = ({
    * It does not name the transaction or populate it with route information. Instead, it waits for the state to fully change
    * and gets the route information from there, @see updateLatestNavigationSpanWithCurrentRoute
    */
-  const startIdleNavigationSpan = (): void => {
+  const startIdleNavigationSpan = (unknownEvent?: unknown): void => {
+    const event = unknownEvent as UnsafeAction | undefined;
+    if (useDispatchedActionData && event?.data.noop) {
+      logger.debug(`${INTEGRATION_NAME} Navigation action is a noop, not starting navigation span.`);
+      return;
+    }
+
+    const navigationActionType = useDispatchedActionData ? event?.data.action.type : undefined;
+    if (
+      useDispatchedActionData &&
+      [
+        // Process common actions
+        'PRELOAD',
+        'SET_PARAMS',
+        // Drawer actions
+        'OPEN_DRAWER',
+        'CLOSE_DRAWER',
+        'TOGGLE_DRAWER',
+      ].includes(navigationActionType)
+    ) {
+      logger.debug(`${INTEGRATION_NAME} Navigation action is ${navigationActionType}, not starting navigation span.`);
+      return;
+    }
+
     if (latestNavigationSpan) {
       logger.log(`${INTEGRATION_NAME} A transaction was detected that turned out to be a noop, discarding.`);
       _discardLatestTransaction();
@@ -215,6 +247,7 @@ export const reactNavigationIntegration = ({
       idleSpanOptions,
     );
     latestNavigationSpan?.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SPAN_ORIGIN_AUTO_NAVIGATION_REACT_NAVIGATION);
+    latestNavigationSpan?.setAttribute(SEMANTIC_ATTRIBUTE_NAVIGATION_ACTION_TYPE, navigationActionType);
     if (ignoreEmptyBackNavigationTransactions) {
       ignoreEmptyBackNavigation(getClient(), latestNavigationSpan);
     }
@@ -357,6 +390,7 @@ export const reactNavigationIntegration = ({
       enableTimeToInitialDisplay,
       ignoreEmptyBackNavigationTransactions,
       enableTimeToInitialDisplayForPreloadedRoutes,
+      useDispatchedActionData,
     },
   };
 };
@@ -369,7 +403,7 @@ export interface NavigationRoute {
 }
 
 interface NavigationContainer {
-  addListener: (type: string, listener: () => void) => void;
+  addListener: (type: string, listener: (event?: unknown) => void) => void;
   getCurrentRoute: () => NavigationRoute;
 }
 

packages/core/src/js/tracing/semanticAttributes.ts

@@ -17,3 +17,4 @@ export const SEMANTIC_ATTRIBUTE_PREVIOUS_ROUTE_KEY = 'previous_route.key';
 export const SEMANTIC_ATTRIBUTE_PREVIOUS_ROUTE_COMPONENT_ID = 'previous_route.component_id';
 export const SEMANTIC_ATTRIBUTE_PREVIOUS_ROUTE_COMPONENT_TYPE = 'previous_route.component_type';
 export const SEMANTIC_ATTRIBUTE_TIME_TO_INITIAL_DISPLAY_FALLBACK = 'route.initial_display_fallback';
+export const SEMANTIC_ATTRIBUTE_NAVIGATION_ACTION_TYPE = 'navigation.action_type';

packages/core/src/js/vendor/react-navigation/types.ts

@@ -0,0 +1,50 @@
+// MIT License
+
+// Copyright (c) 2017 React Navigation Contributors
+
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+
+// The above copyright notice and this permission notice shall be included in all
+// copies or substantial portions of the Software.
+
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+// SOFTWARE.
+
+// https://github.com/react-navigation/react-navigation/blob/a656331009fcb534e0bef535e6df65ae07a228a4/packages/core/src/types.tsx#L777
+
+/**
+ * Event which fires when an action is dispatched.
+ * Only intended for debugging purposes, don't use it for app logic.
+ * This event will be emitted before state changes have been applied.
+ */
+export type UnsafeAction = {
+  data: {
+    /**
+     * The action object which was dispatched.
+     */
+    action: {
+      readonly type: string;
+      readonly payload?: object | undefined;
+      readonly source?: string | undefined;
+      readonly target?: string | undefined;
+    };
+    /**
+     * Whether the action was a no-op, i.e. resulted any state changes.
+     */
+    noop: boolean;
+    /**
+     * Stack trace of the action, this will only be available during development.
+     */
+    stack: string | undefined;
+  };
+};

packages/core/test/tracing/reactnavigation.test.ts

@@ -362,8 +362,8 @@ describe('ReactNavigationInstrumentation', () => {
         current: mockNavigationContainer,
       });
 
-      // Reset mocks after the first registration
-      jest.resetAllMocks();
+      // Clear mocks after the first registration
+      jest.clearAllMocks();
 
       instrumentation.registerNavigationContainer({
         current: mockNavigationContainer,
@@ -579,6 +579,94 @@ describe('ReactNavigationInstrumentation', () => {
     });
   });
 
+  [true, false].forEach(useDispatchedActionData => {
+    describe(`test actions which should not create a navigation span when useDispatchedActionData is ${useDispatchedActionData}`, () => {
+      beforeEach(async () => {
+        setupTestClient({ useDispatchedActionData });
+        await jest.runOnlyPendingTimers(); // Flushes the initial navigation span
+        client.event = undefined;
+      });
+
+      test(`noop does ${useDispatchedActionData ? 'not' : ''}create a navigation span`, async () => {
+        mockNavigation.emitWithStateChange({
+          data: {
+            action: {
+              type: 'UNKNOWN',
+            },
+            noop: true,
+            stack: undefined,
+          },
+        });
+        await jest.runOnlyPendingTimersAsync();
+        await client.flush();
+
+        expect(client.event === undefined).toBe(useDispatchedActionData);
+      });
+
+      test.each(['PRELOAD', 'SET_PARAMS', 'OPEN_DRAWER', 'CLOSE_DRAWER', 'TOGGLE_DRAWER'])(
+        `%s does ${useDispatchedActionData ? 'not' : ''}create a navigation span`,
+        async actionType => {
+          mockNavigation.emitWithStateChange({
+            data: {
+              action: {
+                type: actionType,
+              },
+              noop: false,
+              stack: undefined,
+            },
+          });
+          await jest.runOnlyPendingTimersAsync();
+          await client.flush();
+
+          expect(client.event === undefined).toBe(useDispatchedActionData);
+        },
+      );
+    });
+  });
+
+  test('noop does not remove the previous navigation span from scope', async () => {
+    setupTestClient({ useDispatchedActionData: true });
+    await jest.runOnlyPendingTimers(); // Flushes the initial navigation span
+
+    mockNavigation.emitNavigationWithoutStateChange();
+    const activeSpan = getActiveSpan();
+
+    mockNavigation.emitWithoutStateChange({
+      data: {
+        action: {
+          type: 'UNKNOWN',
+        },
+        noop: true,
+        stack: undefined,
+      },
+    });
+
+    expect(getActiveSpan()).toBe(activeSpan);
+  });
+
+  test.each(['PRELOAD', 'SET_PARAMS', 'OPEN_DRAWER', 'CLOSE_DRAWER', 'TOGGLE_DRAWER'])(
+    '%s does not remove the previous navigation span from scope',
+    async actionType => {
+      setupTestClient({ useDispatchedActionData: true });
+      await jest.runOnlyPendingTimers(); // Flushes the initial navigation span
+
+      mockNavigation.emitNavigationWithoutStateChange();
+      const activeSpan = getActiveSpan();
+
+      mockNavigation.emitWithoutStateChange({
+        data: {
+          action: {
+            type: actionType,
+          },
+          noop: false,
+          stack: undefined,
+        },
+      });
+
+      expect(getActiveSpan()).toBe(activeSpan);
+    },
+  );
+
   describe('setCurrentRoute', () => {
     let mockSetCurrentRoute: jest.Mock;
 
@@ -649,10 +737,12 @@ describe('ReactNavigationInstrumentation', () => {
   function setupTestClient(
     setupOptions: {
       beforeSpanStart?: (options: StartSpanOptions) => StartSpanOptions;
+      useDispatchedActionData?: boolean;
     } = {},
   ) {
     const rNavigation = reactNavigationIntegration({
       routeChangeTimeoutMs: 200,
+      useDispatchedActionData: setupOptions.useDispatchedActionData,
     });
     mockNavigation = createMockNavigationAndAttachTo(rNavigation);
 

packages/core/test/tracing/reactnavigationutils.ts

@@ -1,17 +1,24 @@
 import type { NavigationRoute, reactNavigationIntegration } from '../../src/js/tracing/reactnavigation';
+import type { UnsafeAction } from '../../src/js/vendor/react-navigation/types';
+
+const navigationAction: UnsafeAction = {
+  data: {
+    action: {
+      type: 'NAVIGATE',
+    },
+    noop: false,
+    stack: undefined,
+  },
+};
 
 export function createMockNavigationAndAttachTo(sut: ReturnType<typeof reactNavigationIntegration>) {
   const mockedNavigationContained = mockNavigationContainer();
   const mockedNavigation = {
     emitCancelledNavigation: () => {
-      mockedNavigationContained.listeners['__unsafe_action__']({
-        // this object is not used by the instrumentation
-      });
+      mockedNavigationContained.listeners['__unsafe_action__'](navigationAction);
     },
     navigateToNewScreen: () => {
-      mockedNavigationContained.listeners['__unsafe_action__']({
-        // this object is not used by the instrumentation
-      });
+      mockedNavigationContained.listeners['__unsafe_action__'](navigationAction);
       mockedNavigationContained.currentRoute = {
         key: 'new_screen',
         name: 'New Screen',
@@ -21,9 +28,7 @@ export function createMockNavigationAndAttachTo(sut: ReturnType<typeof reactNavi
       });
     },
     navigateToSecondScreen: () => {
-      mockedNavigationContained.listeners['__unsafe_action__']({
-        // this object is not used by the instrumentation
-      });
+      mockedNavigationContained.listeners['__unsafe_action__'](navigationAction);
       mockedNavigationContained.currentRoute = {
         key: 'second_screen',
         name: 'Second Screen',
@@ -33,9 +38,7 @@ export function createMockNavigationAndAttachTo(sut: ReturnType<typeof reactNavi
       });
     },
     navigateToInitialScreen: () => {
-      mockedNavigationContained.listeners['__unsafe_action__']({
-        // this object is not used by the instrumentation
-      });
+      mockedNavigationContained.listeners['__unsafe_action__'](navigationAction);
       mockedNavigationContained.currentRoute = {
         key: 'initial_screen',
         name: 'Initial Screen',
@@ -53,6 +56,18 @@ export function createMockNavigationAndAttachTo(sut: ReturnType<typeof reactNavi
         // this object is not used by the instrumentation
       });
     },
+    emitNavigationWithoutStateChange: () => {
+      mockedNavigationContained.listeners['__unsafe_action__'](navigationAction);
+    },
+    emitWithoutStateChange: (action: UnsafeAction) => {
+      mockedNavigationContained.listeners['__unsafe_action__'](action);
+    },
+    emitWithStateChange: (action: UnsafeAction) => {
+      mockedNavigationContained.listeners['__unsafe_action__'](action);
+      mockedNavigationContained.listeners['state']({
+        // this object is not used by the instrumentation
+      });
+    },
   };
   sut.registerNavigationContainer(mockRef(mockedNavigationContained));
 

samples/react-native/src/App.tsx

@@ -54,6 +54,7 @@ const reactNavigationIntegration = Sentry.reactNavigationIntegration({
   enableTimeToInitialDisplay: isMobileOs,
   ignoreEmptyBackNavigationTransactions: false,
   enableTimeToInitialDisplayForPreloadedRoutes: true,
+  useDispatchedActionData: true,
 });
 
 Sentry.init({

samples/react-native/src/Screens/PerformanceScreen.tsx

@@ -13,6 +13,7 @@ import { CommonActions } from '@react-navigation/native';
 import * as Sentry from '@sentry/react-native';
 import { Text } from 'react-native';
 import { TimeToFullDisplay } from '../utils';
+import { preloadArticles } from './SpaceflightNewsScreen';
 
 interface Props {
   navigation: StackNavigationProp<any, 'PerformanceScreen'>;
@@ -48,6 +49,14 @@ const PerformanceScreen = (props: Props) => {
             props.navigation.navigate('SpaceflightNewsScreen');
           }}
         />
+        <Button
+          title="Preload Spaceflight News"
+          onPress={() => {
+            console.log('Preloading SpaceflightNewsScreen');
+            props.navigation.preload('SpaceflightNewsScreen');
+            preloadArticles();
+          }}
+        />
         <Button
           title="Auto Tracing Example"
           onPress={() => {