fix(tracing): Name transaction when re-navigating to focused route with new params

CHANGELOG.md

@@ -15,6 +15,7 @@
 ### Fixes
 
 - Fix Android fragment memory leak in `RNSentryReactFragmentLifecycleTracer` ([#6599](https://github.com/getsentry/sentry-react-native/pull/6599))
+- Ship a named navigation transaction when re-navigating to the focused route with new params, instead of an unnamed "Route Change" ([#6606](https://github.com/getsentry/sentry-react-native/pull/6606))
 
 ### Dependencies
 

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

@@ -719,13 +719,9 @@ export const reactNavigationIntegration = ({
 
     if (previousRoute?.key === route.key) {
       debug.log(`[${INTEGRATION_NAME}] Navigation state changed, but route is the same as previous.`);
-      // Even a same-route state change is a legitimate destination for a
-      // deep link (e.g. deep-linking to the screen you're already on). Make
-      // sure the pending link still gets attributed before we drop the span
-      // reference.
+      // Attribute a pending deep link even to a same-route change (e.g. a deep
+      // link to the screen you're already on) before deciding on the span.
       applyPendingDeepLinkToSpan(latestNavigationSpan, routeChangeTimeoutMs);
-      pushRecentRouteKey(route.key);
-      latestRoute = route;
 
       // A `POP_TO` span that landed on the route we were already on (same
       // `route.key`) was a params-only bookkeeping dispatch — e.g. Expo
@@ -740,14 +736,33 @@ export const reactNavigationIntegration = ({
       // never affects a genuine navigation that actually changed the route.
       if (!taggedDeepLinkSpans.has(latestNavigationSpan) && latestNavigationActionType === 'POP_TO') {
         debug.log(`[${INTEGRATION_NAME}] Discarding POP_TO navigation span that did not change the route.`);
+        pushRecentRouteKey(route.key);
+        latestRoute = route;
         clearStateChangeTimeout();
         _discardLatestTransaction();
         return undefined;
       }
 
-      // Clear the latest transaction as it has been handled.
-      latestNavigationSpan = undefined;
-      return undefined;
+      // Same key and unchanged params is a no-op (drawer toggle, empty
+      // SET_PARAMS, deep link to the current screen): keep the idle-path
+      // behaviour and let the span end via the idle timeout. Clear the pending
+      // discard timeout and end the processing span here so a leftover timer
+      // cannot fire later and discard the next navigation's span.
+      if (!haveRouteParamsChanged(previousRoute?.params, route.params)) {
+        pushRecentRouteKey(route.key);
+        latestRoute = route;
+        clearStateChangeTimeout();
+        navigationProcessingSpan?.setStatus({ code: SPAN_STATUS_OK });
+        navigationProcessingSpan?.end(stateChangedTimestamp);
+        navigationProcessingSpan = undefined;
+        latestNavigationSpan = undefined;
+        return undefined;
+      }
+
+      // Same key but changed params is a genuine navigation onto the focused
+      // route (e.g. navigate('Chat', { id: 2 }) from Chat/id:1). Fall through
+      // to name and finalize the transaction instead of leaking an unnamed
+      // "Route Change" (#6593).
     }
 
     const routeHasBeenSeen = recentRouteKeys.includes(route.key);
@@ -963,6 +978,28 @@ function getRouteNameFromAction(event: UnsafeAction | undefined): string | undef
   return undefined;
 }
 
+/**
+ * Shallow-compares two route param bags to tell a genuine re-navigation onto
+ * the already-focused route (same `route.key`, changed params) apart from a
+ * no-op same-route state change (#6593). Missing and empty bags are equal.
+ */
+function haveRouteParamsChanged(
+  a: Record<string, unknown> | undefined,
+  b: Record<string, unknown> | undefined,
+): boolean {
+  const aKeys = a ? Object.keys(a) : [];
+  const bKeys = b ? Object.keys(b) : [];
+  if (aKeys.length !== bKeys.length) {
+    return true;
+  }
+  // Counts are equal, so `a` non-empty implies `b` non-empty. Require every key
+  // to exist on `b` too — otherwise disjoint bags with matching `undefined`
+  // values (e.g. `{ x: undefined }` vs `{ y: undefined }`) would compare equal.
+  const aBag = a ?? {};
+  const bBag = b ?? {};
+  return aKeys.some(key => !Object.prototype.hasOwnProperty.call(bBag, key) || !Object.is(aBag[key], bBag[key]));
+}
+
 /**
  * Returns React Navigation integration of the given client.
  */

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

@@ -1451,6 +1451,199 @@ describe('ReactNavigationInstrumentation', () => {
     });
   });
 
+  describe('NAVIGATE onto the already-focused route with new params (#6593)', () => {
+    // navigate('Chat', { id: 2 }) while Chat/id:1 is focused keeps the same
+    // route.key (only params change), but is a genuine screen entry and must
+    // ship a named transaction, not an unnamed "Route Change".
+    const navigateToChat: UnsafeAction = {
+      data: {
+        action: { type: 'NAVIGATE', payload: { name: 'Chat' } },
+        noop: false,
+        stack: undefined,
+      },
+    };
+
+    function countTransactionsNamed(name: string): number {
+      return client.eventQueue.filter(e => e.type === 'transaction' && e.transaction === name).length;
+    }
+
+    test('ships a named transaction instead of an unnamed "Route Change"', async () => {
+      // ignoreEmptyBackNavigationTransactions: false isolates the naming
+      // behaviour from the empty-back-navigation discard.
+      const instrumentation = reactNavigationIntegration({
+        routeChangeTimeoutMs: 200,
+        ignoreEmptyBackNavigationTransactions: false,
+        useDispatchedActionData: true,
+      });
+      const options = getDefaultTestClientOptions({
+        enableNativeFramesTracking: false,
+        enableStallTracking: false,
+        tracesSampleRate: 1.0,
+        integrations: [instrumentation],
+        enableAppStartTracking: false,
+      });
+      client = new TestClient(options);
+      setCurrentClient(client);
+      client.init();
+
+      mockNavigation = createMockNavigationAndAttachTo(instrumentation);
+      await jest.runOnlyPendingTimersAsync(); // Flush the initial span
+
+      // Enter Chat for conversation 1.
+      mockNavigation.emitWithStateChange(navigateToChat, { key: 'chat', name: 'Chat', params: { id: 1 } });
+      await jest.runOnlyPendingTimersAsync();
+      client.eventQueue = [];
+
+      // Tap a notification for conversation 2 — same route key, new params.
+      mockNavigation.emitWithStateChange(navigateToChat, { key: 'chat', name: 'Chat', params: { id: 2 } });
+      await jest.runOnlyPendingTimersAsync();
+      await client.flush();
+
+      expect(countTransactionsNamed('Chat')).toBe(1);
+      expect(countTransactionsNamed(DEFAULT_NAVIGATION_SPAN_NAME)).toBe(0);
+      const chat = client.eventQueue.find(e => e.type === 'transaction' && e.transaction === 'Chat');
+      expect(chat?.contexts?.trace?.op).toBe('navigation');
+      expect(chat?.contexts?.trace?.data?.[SEMANTIC_ATTRIBUTE_ROUTE_NAME]).toBe('Chat');
+      expect(chat?.contexts?.trace?.data?.[SEMANTIC_ATTRIBUTE_ROUTE_KEY]).toBe('chat');
+    });
+
+    test('detects a param change when only disjoint undefined-valued keys differ', async () => {
+      // { a: undefined } -> { b: undefined }: same key count, disjoint keys.
+      // haveRouteParamsChanged must still report a change, so the span goes
+      // through the naming path (which sets route.key/route.name) rather than
+      // the no-op idle path (which does not).
+      const instrumentation = reactNavigationIntegration({
+        routeChangeTimeoutMs: 200,
+        ignoreEmptyBackNavigationTransactions: false,
+        useDispatchedActionData: true,
+      });
+      const options = getDefaultTestClientOptions({
+        enableNativeFramesTracking: false,
+        enableStallTracking: false,
+        tracesSampleRate: 1.0,
+        integrations: [instrumentation],
+        enableAppStartTracking: false,
+      });
+      client = new TestClient(options);
+      setCurrentClient(client);
+      client.init();
+
+      mockNavigation = createMockNavigationAndAttachTo(instrumentation);
+      await jest.runOnlyPendingTimersAsync(); // Flush the initial span
+
+      mockNavigation.emitWithStateChange(navigateToChat, { key: 'chat', name: 'Chat', params: { a: undefined } });
+      await jest.runOnlyPendingTimersAsync();
+      client.eventQueue = [];
+
+      mockNavigation.emitWithStateChange(navigateToChat, { key: 'chat', name: 'Chat', params: { b: undefined } });
+      await jest.runOnlyPendingTimersAsync();
+      await client.flush();
+
+      const chat = client.eventQueue.find(e => e.type === 'transaction' && e.transaction === 'Chat');
+      expect(chat?.contexts?.trace?.data?.[SEMANTIC_ATTRIBUTE_ROUTE_KEY]).toBe('chat');
+      expect(chat?.contexts?.trace?.data?.[SEMANTIC_ATTRIBUTE_ROUTE_NAME]).toBe('Chat');
+    });
+
+    test('does not leak an unnamed transaction when the re-navigation is empty (default config)', async () => {
+      // Under the default config an empty same-key re-nav is discarded as an
+      // empty back navigation, not leaked as an unnamed "Route Change".
+      setupTestClient({ useDispatchedActionData: true });
+      await jest.runOnlyPendingTimersAsync(); // Flush the initial span
+
+      mockNavigation.emitWithStateChange(navigateToChat, { key: 'chat', name: 'Chat', params: { id: 1 } });
+      await jest.runOnlyPendingTimersAsync();
+      client.eventQueue = [];
+
+      // Re-navigate onto the already-focused "Chat" with new params (same key).
+      mockNavigation.emitWithStateChange(navigateToChat, { key: 'chat', name: 'Chat', params: { id: 2 } });
+      await jest.runOnlyPendingTimersAsync();
+      await client.flush();
+
+      // Under the default config the re-nav is intentionally discarded as an
+      // empty back navigation, so neither an unnamed nor a named "Chat"
+      // transaction ships.
+      expect(countTransactionsNamed(DEFAULT_NAVIGATION_SPAN_NAME)).toBe(0);
+      expect(countTransactionsNamed('Chat')).toBe(0);
+    });
+
+    test('names the transaction with useDispatchedActionData off (true #6593 repro)', async () => {
+      // With useDispatchedActionData off the idle span starts unnamed
+      // (DEFAULT_NAVIGATION_SPAN_NAME). Before the fix, the same-key/changed-
+      // params branch dropped the span reference without naming it, so it
+      // self-finalized as an unnamed "Route Change". This is the actual bug
+      // mode; the fix must derive the name from the mounted route instead.
+      const instrumentation = reactNavigationIntegration({
+        routeChangeTimeoutMs: 200,
+        ignoreEmptyBackNavigationTransactions: false,
+        useDispatchedActionData: false,
+      });
+      const options = getDefaultTestClientOptions({
+        enableNativeFramesTracking: false,
+        enableStallTracking: false,
+        tracesSampleRate: 1.0,
+        integrations: [instrumentation],
+        enableAppStartTracking: false,
+      });
+      client = new TestClient(options);
+      setCurrentClient(client);
+      client.init();
+
+      mockNavigation = createMockNavigationAndAttachTo(instrumentation);
+      await jest.runOnlyPendingTimersAsync(); // Flush the initial span
+
+      mockNavigation.emitWithStateChange(navigateToChat, { key: 'chat', name: 'Chat', params: { id: 1 } });
+      await jest.runOnlyPendingTimersAsync();
+      client.eventQueue = [];
+
+      mockNavigation.emitWithStateChange(navigateToChat, { key: 'chat', name: 'Chat', params: { id: 2 } });
+      await jest.runOnlyPendingTimersAsync();
+      await client.flush();
+
+      expect(countTransactionsNamed('Chat')).toBe(1);
+      expect(countTransactionsNamed(DEFAULT_NAVIGATION_SPAN_NAME)).toBe(0);
+    });
+
+    test('takes the no-op idle path when the same-key re-nav does not change params', async () => {
+      // Same key and unchanged params ({ id: 1 } -> { id: 1 }) is a no-op: the
+      // span must end via the idle timeout without going through the naming
+      // path. useDispatchedActionData keeps it named "Chat" (from dispatch), so
+      // the discriminator is the absence of the route.key/route.name attributes
+      // that only the naming path sets.
+      const instrumentation = reactNavigationIntegration({
+        routeChangeTimeoutMs: 200,
+        ignoreEmptyBackNavigationTransactions: false,
+        useDispatchedActionData: true,
+      });
+      const options = getDefaultTestClientOptions({
+        enableNativeFramesTracking: false,
+        enableStallTracking: false,
+        tracesSampleRate: 1.0,
+        integrations: [instrumentation],
+        enableAppStartTracking: false,
+      });
+      client = new TestClient(options);
+      setCurrentClient(client);
+      client.init();
+
+      mockNavigation = createMockNavigationAndAttachTo(instrumentation);
+      await jest.runOnlyPendingTimersAsync(); // Flush the initial span
+
+      mockNavigation.emitWithStateChange(navigateToChat, { key: 'chat', name: 'Chat', params: { id: 1 } });
+      await jest.runOnlyPendingTimersAsync();
+      client.eventQueue = [];
+
+      // Re-dispatch onto the focused route with identical params.
+      mockNavigation.emitWithStateChange(navigateToChat, { key: 'chat', name: 'Chat', params: { id: 1 } });
+      await jest.runOnlyPendingTimersAsync();
+      await client.flush();
+
+      const chat = client.eventQueue.find(e => e.type === 'transaction' && e.transaction === 'Chat');
+      expect(chat).toBeDefined();
+      expect(chat?.contexts?.trace?.data?.[SEMANTIC_ATTRIBUTE_ROUTE_KEY]).toBeUndefined();
+      expect(chat?.contexts?.trace?.data?.[SEMANTIC_ATTRIBUTE_ROUTE_NAME]).toBeUndefined();
+    });
+  });
+
   test('noop does not remove the previous navigation span from scope', async () => {
     setupTestClient({ useDispatchedActionData: true });
     await jest.runOnlyPendingTimers(); // Flushes the initial navigation span