diff --git a/CHANGELOG.md b/CHANGELOG.md index 39958f463d..1cc577010e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ - Enable "Open Sentry" button in Playground for Expo apps ([#5947](https://github.com/getsentry/sentry-react-native/pull/5947)) - Add `attachAllThreads` option to attach full stack traces for all threads to captured events on iOS ([#5960](https://github.com/getsentry/sentry-react-native/issues/5960)) +- Name navigation spans using dispatched action payload when `useDispatchedActionData` is enabled ([#5982](https://github.com/getsentry/sentry-react-native/pull/5982)) ### Fixes diff --git a/packages/core/src/js/tracing/reactnavigation.ts b/packages/core/src/js/tracing/reactnavigation.ts index e717df4900..6af6a245d9 100644 --- a/packages/core/src/js/tracing/reactnavigation.ts +++ b/packages/core/src/js/tracing/reactnavigation.ts @@ -198,6 +198,7 @@ export const reactNavigationIntegration = ({ let latestRoute: NavigationRoute | undefined; let latestNavigationSpan: Span | undefined; + let latestNavigationSpanNameCustomized: boolean = false; let navigationProcessingSpan: Span | undefined; let initialStateHandled: boolean = false; @@ -376,18 +377,31 @@ export const reactNavigationIntegration = ({ return; } + // Extract route name from dispatch action payload when available + const dispatchedRouteName = useDispatchedActionData ? getRouteNameFromAction(event) : undefined; + if (useDispatchedActionData && event && !dispatchedRouteName && !isAppRestart) { + debug.log(`${INTEGRATION_NAME} Navigation action has no route name in payload, not starting navigation span.`); + return; + } + if (latestNavigationSpan) { debug.log(`${INTEGRATION_NAME} A transaction was detected that turned out to be a noop, discarding.`); _discardLatestTransaction(); clearStateChangeTimeout(); } - latestNavigationSpan = startGenericIdleNavigationSpan( - tracing?.options.beforeStartSpan - ? tracing.options.beforeStartSpan(getDefaultIdleNavigationSpanOptions()) - : getDefaultIdleNavigationSpanOptions(), - { ...idleSpanOptions, isAppRestart }, - ); + const spanOptions = getDefaultIdleNavigationSpanOptions(); + if (dispatchedRouteName) { + spanOptions.name = dispatchedRouteName; + } + + const originalName = spanOptions.name; + const finalSpanOptions = tracing?.options.beforeStartSpan + ? tracing.options.beforeStartSpan(spanOptions) + : spanOptions; + latestNavigationSpanNameCustomized = finalSpanOptions.name !== originalName; + + latestNavigationSpan = startGenericIdleNavigationSpan(finalSpanOptions, { ...idleSpanOptions, isAppRestart }); latestNavigationSpan?.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SPAN_ORIGIN_AUTO_NAVIGATION_REACT_NAVIGATION); latestNavigationSpan?.setAttribute(SEMANTIC_ATTRIBUTE_NAVIGATION_ACTION_TYPE, navigationActionType); if (ignoreEmptyBackNavigationTransactions) { @@ -395,12 +409,8 @@ export const reactNavigationIntegration = ({ } // Always discard transactions that never receive route information const spanToCheck = latestNavigationSpan; - ignoreEmptyRouteChangeTransactions( - getClient(), - spanToCheck, - DEFAULT_NAVIGATION_SPAN_NAME, - () => latestNavigationSpan === spanToCheck, - ); + const spanName = finalSpanOptions.name ?? DEFAULT_NAVIGATION_SPAN_NAME; + ignoreEmptyRouteChangeTransactions(getClient(), spanToCheck, spanName, () => latestNavigationSpan === spanToCheck); if (enableTimeToInitialDisplay && latestNavigationSpan) { NATIVE.setActiveSpanId(latestNavigationSpan.spanContext().spanId); @@ -472,7 +482,7 @@ export const reactNavigationIntegration = ({ navigationProcessingSpan?.end(stateChangedTimestamp); navigationProcessingSpan = undefined; - if (spanToJSON(latestNavigationSpan).description === DEFAULT_NAVIGATION_SPAN_NAME) { + if (!latestNavigationSpanNameCustomized) { latestNavigationSpan.updateName(routeName); } const sendDefaultPii = getClient()?.getOptions()?.sendDefaultPii ?? false; @@ -578,6 +588,20 @@ interface NavigationContainer { getState: () => NavigationState | undefined; } +/** + * Extracts the route name from a React Navigation dispatch action payload. + * + * Actions like NAVIGATE, PUSH, REPLACE, JUMP_TO carry the target route name + * in `action.payload.name`. Actions like GO_BACK, POP, POP_TO_TOP do not. + */ +function getRouteNameFromAction(event: UnsafeAction | undefined): string | undefined { + const payload = event?.data?.action?.payload; + if (payload && typeof payload === 'object' && 'name' in payload && typeof payload.name === 'string') { + return payload.name; + } + return undefined; +} + /** * Returns React Navigation integration of the given client. */ diff --git a/packages/core/test/tracing/reactnavigation.test.ts b/packages/core/test/tracing/reactnavigation.test.ts index 23bb2fb9d6..ec1095d2e0 100644 --- a/packages/core/test/tracing/reactnavigation.test.ts +++ b/packages/core/test/tracing/reactnavigation.test.ts @@ -943,6 +943,186 @@ describe('ReactNavigationInstrumentation', () => { }, ); + describe('useDispatchedActionData names spans from action payload', () => { + beforeEach(async () => { + setupTestClient({ useDispatchedActionData: true }); + await jest.runOnlyPendingTimers(); // Flush the initial navigation span + client.event = undefined; + }); + + test('NAVIGATE action with payload.name sets the span name', async () => { + mockNavigation.navigateToNewScreenWithPayload(); + jest.runOnlyPendingTimers(); + + await client.flush(); + + expect(client.event?.transaction).toBe('New Screen'); + }); + + test('span name is updated with final route from state change when it differs from payload', async () => { + mockNavigation.navigateToScreenWithMismatchedPayload(); + jest.runOnlyPendingTimers(); + + await client.flush(); + + expect(client.event?.transaction).toBe('ScreenB'); + }); + + test('GO_BACK action (no payload.name) does not create a navigation span', async () => { + mockNavigation.emitGoBackWithStateChange(); + jest.runOnlyPendingTimers(); + + await client.flush(); + + expect(client.event).toBeUndefined(); + }); + + test.each(['GO_BACK', 'POP', 'POP_TO_TOP', 'RESET'])( + '%s action does 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).toBeUndefined(); + }, + ); + + test('PUSH action with payload.name creates a named span', async () => { + mockNavigation.emitWithStateChange( + { + data: { + action: { + type: 'PUSH', + payload: { name: 'PushedScreen' }, + }, + noop: false, + stack: undefined, + }, + }, + { key: 'pushed_screen', name: 'PushedScreen' }, + ); + jest.runOnlyPendingTimers(); + + await client.flush(); + + expect(client.event?.transaction).toBe('PushedScreen'); + }); + + test('REPLACE action with payload.name creates a named span', async () => { + mockNavigation.emitWithStateChange( + { + data: { + action: { + type: 'REPLACE', + payload: { name: 'ReplacedScreen' }, + }, + noop: false, + stack: undefined, + }, + }, + { key: 'replaced_screen', name: 'ReplacedScreen' }, + ); + jest.runOnlyPendingTimers(); + + await client.flush(); + + expect(client.event?.transaction).toBe('ReplacedScreen'); + }); + + test('JUMP_TO action with payload.name creates a named span', async () => { + mockNavigation.emitWithStateChange( + { + data: { + action: { + type: 'JUMP_TO', + payload: { name: 'TabScreen' }, + }, + noop: false, + stack: undefined, + }, + }, + { key: 'tab_screen', name: 'TabScreen' }, + ); + jest.runOnlyPendingTimers(); + + await client.flush(); + + expect(client.event?.transaction).toBe('TabScreen'); + }); + + test('cancelled navigation with dispatched route name is discarded', async () => { + mockNavigation.emitWithoutStateChange({ + data: { + action: { + type: 'NAVIGATE', + payload: { name: 'OrphanedScreen' }, + }, + noop: false, + stack: undefined, + }, + }); + jest.runOnlyPendingTimers(); // Trigger the timeout + + await client.flush(); + + expect(client.event).toBeUndefined(); + }); + + test('initial navigation span is created during setup', async () => { + // Reset and create fresh client to test initial span + const rNavigation = reactNavigationIntegration({ + routeChangeTimeoutMs: 200, + useDispatchedActionData: true, + }); + const freshMockNavigation = createMockNavigationAndAttachTo(rNavigation); + + const rnTracing = reactNativeTracingIntegration(); + const options = getDefaultTestClientOptions({ + enableNativeFramesTracking: false, + enableStallTracking: false, + tracesSampleRate: 1.0, + integrations: [rNavigation, rnTracing], + enableAppStartTracking: false, + }); + const freshClient = new TestClient(options); + setCurrentClient(freshClient); + freshClient.init(); + + jest.runOnlyPendingTimers(); // Flush the initial navigation span + + await freshClient.flush(); + + // Initial span should be created with 'Initial Screen' from the mock container + expect(freshClient.event?.transaction).toBe('Initial Screen'); + }); + }); + + describe('useDispatchedActionData disabled (default) still uses generic span name', () => { + beforeEach(async () => { + setupTestClient({ useDispatchedActionData: false }); + await jest.runOnlyPendingTimers(); // Flush the initial navigation span + client.event = undefined; + }); + + test('GO_BACK action still creates a navigation span', async () => { + mockNavigation.emitGoBackWithStateChange(); + jest.runOnlyPendingTimers(); + + await client.flush(); + + expect(client.event?.transaction).toBe('Previous Screen'); + }); + }); + describe('setCurrentRoute', () => { let mockSetCurrentRoute: jest.Mock; diff --git a/packages/core/test/tracing/reactnavigationutils.ts b/packages/core/test/tracing/reactnavigationutils.ts index d5cafd7de4..93ee6e3dab 100644 --- a/packages/core/test/tracing/reactnavigationutils.ts +++ b/packages/core/test/tracing/reactnavigationutils.ts @@ -11,6 +11,29 @@ const navigationAction: UnsafeAction = { }, }; +function navigationActionWithPayload(name: string): UnsafeAction { + return { + data: { + action: { + type: 'NAVIGATE', + payload: { name }, + }, + noop: false, + stack: undefined, + }, + }; +} + +const goBackAction: UnsafeAction = { + data: { + action: { + type: 'GO_BACK', + }, + noop: false, + stack: undefined, + }, +}; + export function createMockNavigationAndAttachTo(sut: ReturnType) { const mockedNavigationContained = mockNavigationContainer(); const mockedNavigation = { @@ -62,8 +85,11 @@ export function createMockNavigationAndAttachTo(sut: ReturnType { mockedNavigationContained.listeners['__unsafe_action__'](action); }, - emitWithStateChange: (action: UnsafeAction) => { + emitWithStateChange: (action: UnsafeAction, route?: NavigationRoute) => { mockedNavigationContained.listeners['__unsafe_action__'](action); + if (route) { + mockedNavigationContained.currentRoute = route; + } mockedNavigationContained.listeners['state']({ // this object is not used by the instrumentation }); @@ -95,6 +121,31 @@ export function createMockNavigationAndAttachTo(sut: ReturnType { + mockedNavigationContained.listeners['__unsafe_action__'](navigationActionWithPayload('New Screen')); + mockedNavigationContained.currentRoute = { + key: 'new_screen', + name: 'New Screen', + }; + mockedNavigationContained.listeners['state']({}); + }, + navigateToScreenWithMismatchedPayload: () => { + // Dispatch says "ScreenA" but router resolves to "ScreenB" (e.g. nested navigation) + mockedNavigationContained.listeners['__unsafe_action__'](navigationActionWithPayload('ScreenA')); + mockedNavigationContained.currentRoute = { + key: 'screen_b', + name: 'ScreenB', + }; + mockedNavigationContained.listeners['state']({}); + }, + emitGoBackWithStateChange: () => { + mockedNavigationContained.listeners['__unsafe_action__'](goBackAction); + mockedNavigationContained.currentRoute = { + key: 'previous_screen', + name: 'Previous Screen', + }; + mockedNavigationContained.listeners['state']({}); + }, emitNavigationWithUndefinedRoute: () => { mockedNavigationContained.listeners['__unsafe_action__'](navigationAction); mockedNavigationContained.currentRoute = undefined as any;