Skip to content

Commit 40fb54e

Browse files
alwxalwx
authored
feat(core): Name navigation spans using dispatched action payload (#5982)
* feat(core): Name navigation spans using dispatched action payload When `useDispatchedActionData` is enabled, extract the route name from the dispatched action's `payload.name` (available for NAVIGATE, PUSH, REPLACE, JUMP_TO actions) and use it to name the navigation span immediately at dispatch time instead of the generic 'Route Change'. Actions without a known route name in the payload (GO_BACK, POP, POP_TO_TOP, RESET) no longer create navigation spans when `useDispatchedActionData` is enabled, reducing noise. The span name is still updated with the final route name from the router's state change, unless `beforeStartSpan` has customized it. Resolves #4429 * docs: Add changelog entry for navigation span naming (#5982) * fix(core): Gate dispatched route name extraction behind useDispatchedActionData Ensure getRouteNameFromAction is only called when useDispatchedActionData is enabled. Previously, the route name was extracted from the action payload even when the feature was disabled, which could change the span name and beforeStartSpan input for users who hadn't opted in. * fix(core): Fix initial span and orphaned span handling with useDispatchedActionData Two fixes based on PR review feedback: 1. Allow setup/programmatic calls (no event) to create the initial navigation span even when useDispatchedActionData is enabled. The early return for actions without payload.name now checks that an event exists, so afterAllSetup and registerNavigationContainer still work correctly. 2. Pass the actual span name to ignoreEmptyRouteChangeTransactions instead of the hardcoded default. This ensures orphaned spans created with a dispatched route name are still discarded when the state listener is never called. * chore(test): Remove unused emitGoBackWithoutStateChange helper
1 parent 3ce5254 commit 40fb54e

4 files changed

Lines changed: 270 additions & 14 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
- Enable "Open Sentry" button in Playground for Expo apps ([#5947](https://github.com/getsentry/sentry-react-native/pull/5947))
1414
- 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))
15+
- Name navigation spans using dispatched action payload when `useDispatchedActionData` is enabled ([#5982](https://github.com/getsentry/sentry-react-native/pull/5982))
1516

1617
### Fixes
1718

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

Lines changed: 37 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,7 @@ export const reactNavigationIntegration = ({
198198
let latestRoute: NavigationRoute | undefined;
199199

200200
let latestNavigationSpan: Span | undefined;
201+
let latestNavigationSpanNameCustomized: boolean = false;
201202
let navigationProcessingSpan: Span | undefined;
202203

203204
let initialStateHandled: boolean = false;
@@ -376,31 +377,40 @@ export const reactNavigationIntegration = ({
376377
return;
377378
}
378379

380+
// Extract route name from dispatch action payload when available
381+
const dispatchedRouteName = useDispatchedActionData ? getRouteNameFromAction(event) : undefined;
382+
if (useDispatchedActionData && event && !dispatchedRouteName && !isAppRestart) {
383+
debug.log(`${INTEGRATION_NAME} Navigation action has no route name in payload, not starting navigation span.`);
384+
return;
385+
}
386+
379387
if (latestNavigationSpan) {
380388
debug.log(`${INTEGRATION_NAME} A transaction was detected that turned out to be a noop, discarding.`);
381389
_discardLatestTransaction();
382390
clearStateChangeTimeout();
383391
}
384392

385-
latestNavigationSpan = startGenericIdleNavigationSpan(
386-
tracing?.options.beforeStartSpan
387-
? tracing.options.beforeStartSpan(getDefaultIdleNavigationSpanOptions())
388-
: getDefaultIdleNavigationSpanOptions(),
389-
{ ...idleSpanOptions, isAppRestart },
390-
);
393+
const spanOptions = getDefaultIdleNavigationSpanOptions();
394+
if (dispatchedRouteName) {
395+
spanOptions.name = dispatchedRouteName;
396+
}
397+
398+
const originalName = spanOptions.name;
399+
const finalSpanOptions = tracing?.options.beforeStartSpan
400+
? tracing.options.beforeStartSpan(spanOptions)
401+
: spanOptions;
402+
latestNavigationSpanNameCustomized = finalSpanOptions.name !== originalName;
403+
404+
latestNavigationSpan = startGenericIdleNavigationSpan(finalSpanOptions, { ...idleSpanOptions, isAppRestart });
391405
latestNavigationSpan?.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SPAN_ORIGIN_AUTO_NAVIGATION_REACT_NAVIGATION);
392406
latestNavigationSpan?.setAttribute(SEMANTIC_ATTRIBUTE_NAVIGATION_ACTION_TYPE, navigationActionType);
393407
if (ignoreEmptyBackNavigationTransactions) {
394408
ignoreEmptyBackNavigation(getClient(), latestNavigationSpan);
395409
}
396410
// Always discard transactions that never receive route information
397411
const spanToCheck = latestNavigationSpan;
398-
ignoreEmptyRouteChangeTransactions(
399-
getClient(),
400-
spanToCheck,
401-
DEFAULT_NAVIGATION_SPAN_NAME,
402-
() => latestNavigationSpan === spanToCheck,
403-
);
412+
const spanName = finalSpanOptions.name ?? DEFAULT_NAVIGATION_SPAN_NAME;
413+
ignoreEmptyRouteChangeTransactions(getClient(), spanToCheck, spanName, () => latestNavigationSpan === spanToCheck);
404414

405415
if (enableTimeToInitialDisplay && latestNavigationSpan) {
406416
NATIVE.setActiveSpanId(latestNavigationSpan.spanContext().spanId);
@@ -472,7 +482,7 @@ export const reactNavigationIntegration = ({
472482
navigationProcessingSpan?.end(stateChangedTimestamp);
473483
navigationProcessingSpan = undefined;
474484

475-
if (spanToJSON(latestNavigationSpan).description === DEFAULT_NAVIGATION_SPAN_NAME) {
485+
if (!latestNavigationSpanNameCustomized) {
476486
latestNavigationSpan.updateName(routeName);
477487
}
478488
const sendDefaultPii = getClient()?.getOptions()?.sendDefaultPii ?? false;
@@ -578,6 +588,20 @@ interface NavigationContainer {
578588
getState: () => NavigationState | undefined;
579589
}
580590

591+
/**
592+
* Extracts the route name from a React Navigation dispatch action payload.
593+
*
594+
* Actions like NAVIGATE, PUSH, REPLACE, JUMP_TO carry the target route name
595+
* in `action.payload.name`. Actions like GO_BACK, POP, POP_TO_TOP do not.
596+
*/
597+
function getRouteNameFromAction(event: UnsafeAction | undefined): string | undefined {
598+
const payload = event?.data?.action?.payload;
599+
if (payload && typeof payload === 'object' && 'name' in payload && typeof payload.name === 'string') {
600+
return payload.name;
601+
}
602+
return undefined;
603+
}
604+
581605
/**
582606
* Returns React Navigation integration of the given client.
583607
*/

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

Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -943,6 +943,186 @@ describe('ReactNavigationInstrumentation', () => {
943943
},
944944
);
945945

946+
describe('useDispatchedActionData names spans from action payload', () => {
947+
beforeEach(async () => {
948+
setupTestClient({ useDispatchedActionData: true });
949+
await jest.runOnlyPendingTimers(); // Flush the initial navigation span
950+
client.event = undefined;
951+
});
952+
953+
test('NAVIGATE action with payload.name sets the span name', async () => {
954+
mockNavigation.navigateToNewScreenWithPayload();
955+
jest.runOnlyPendingTimers();
956+
957+
await client.flush();
958+
959+
expect(client.event?.transaction).toBe('New Screen');
960+
});
961+
962+
test('span name is updated with final route from state change when it differs from payload', async () => {
963+
mockNavigation.navigateToScreenWithMismatchedPayload();
964+
jest.runOnlyPendingTimers();
965+
966+
await client.flush();
967+
968+
expect(client.event?.transaction).toBe('ScreenB');
969+
});
970+
971+
test('GO_BACK action (no payload.name) does not create a navigation span', async () => {
972+
mockNavigation.emitGoBackWithStateChange();
973+
jest.runOnlyPendingTimers();
974+
975+
await client.flush();
976+
977+
expect(client.event).toBeUndefined();
978+
});
979+
980+
test.each(['GO_BACK', 'POP', 'POP_TO_TOP', 'RESET'])(
981+
'%s action does not create a navigation span',
982+
async actionType => {
983+
mockNavigation.emitWithStateChange({
984+
data: {
985+
action: {
986+
type: actionType,
987+
},
988+
noop: false,
989+
stack: undefined,
990+
},
991+
});
992+
await jest.runOnlyPendingTimersAsync();
993+
await client.flush();
994+
995+
expect(client.event).toBeUndefined();
996+
},
997+
);
998+
999+
test('PUSH action with payload.name creates a named span', async () => {
1000+
mockNavigation.emitWithStateChange(
1001+
{
1002+
data: {
1003+
action: {
1004+
type: 'PUSH',
1005+
payload: { name: 'PushedScreen' },
1006+
},
1007+
noop: false,
1008+
stack: undefined,
1009+
},
1010+
},
1011+
{ key: 'pushed_screen', name: 'PushedScreen' },
1012+
);
1013+
jest.runOnlyPendingTimers();
1014+
1015+
await client.flush();
1016+
1017+
expect(client.event?.transaction).toBe('PushedScreen');
1018+
});
1019+
1020+
test('REPLACE action with payload.name creates a named span', async () => {
1021+
mockNavigation.emitWithStateChange(
1022+
{
1023+
data: {
1024+
action: {
1025+
type: 'REPLACE',
1026+
payload: { name: 'ReplacedScreen' },
1027+
},
1028+
noop: false,
1029+
stack: undefined,
1030+
},
1031+
},
1032+
{ key: 'replaced_screen', name: 'ReplacedScreen' },
1033+
);
1034+
jest.runOnlyPendingTimers();
1035+
1036+
await client.flush();
1037+
1038+
expect(client.event?.transaction).toBe('ReplacedScreen');
1039+
});
1040+
1041+
test('JUMP_TO action with payload.name creates a named span', async () => {
1042+
mockNavigation.emitWithStateChange(
1043+
{
1044+
data: {
1045+
action: {
1046+
type: 'JUMP_TO',
1047+
payload: { name: 'TabScreen' },
1048+
},
1049+
noop: false,
1050+
stack: undefined,
1051+
},
1052+
},
1053+
{ key: 'tab_screen', name: 'TabScreen' },
1054+
);
1055+
jest.runOnlyPendingTimers();
1056+
1057+
await client.flush();
1058+
1059+
expect(client.event?.transaction).toBe('TabScreen');
1060+
});
1061+
1062+
test('cancelled navigation with dispatched route name is discarded', async () => {
1063+
mockNavigation.emitWithoutStateChange({
1064+
data: {
1065+
action: {
1066+
type: 'NAVIGATE',
1067+
payload: { name: 'OrphanedScreen' },
1068+
},
1069+
noop: false,
1070+
stack: undefined,
1071+
},
1072+
});
1073+
jest.runOnlyPendingTimers(); // Trigger the timeout
1074+
1075+
await client.flush();
1076+
1077+
expect(client.event).toBeUndefined();
1078+
});
1079+
1080+
test('initial navigation span is created during setup', async () => {
1081+
// Reset and create fresh client to test initial span
1082+
const rNavigation = reactNavigationIntegration({
1083+
routeChangeTimeoutMs: 200,
1084+
useDispatchedActionData: true,
1085+
});
1086+
const freshMockNavigation = createMockNavigationAndAttachTo(rNavigation);
1087+
1088+
const rnTracing = reactNativeTracingIntegration();
1089+
const options = getDefaultTestClientOptions({
1090+
enableNativeFramesTracking: false,
1091+
enableStallTracking: false,
1092+
tracesSampleRate: 1.0,
1093+
integrations: [rNavigation, rnTracing],
1094+
enableAppStartTracking: false,
1095+
});
1096+
const freshClient = new TestClient(options);
1097+
setCurrentClient(freshClient);
1098+
freshClient.init();
1099+
1100+
jest.runOnlyPendingTimers(); // Flush the initial navigation span
1101+
1102+
await freshClient.flush();
1103+
1104+
// Initial span should be created with 'Initial Screen' from the mock container
1105+
expect(freshClient.event?.transaction).toBe('Initial Screen');
1106+
});
1107+
});
1108+
1109+
describe('useDispatchedActionData disabled (default) still uses generic span name', () => {
1110+
beforeEach(async () => {
1111+
setupTestClient({ useDispatchedActionData: false });
1112+
await jest.runOnlyPendingTimers(); // Flush the initial navigation span
1113+
client.event = undefined;
1114+
});
1115+
1116+
test('GO_BACK action still creates a navigation span', async () => {
1117+
mockNavigation.emitGoBackWithStateChange();
1118+
jest.runOnlyPendingTimers();
1119+
1120+
await client.flush();
1121+
1122+
expect(client.event?.transaction).toBe('Previous Screen');
1123+
});
1124+
});
1125+
9461126
describe('setCurrentRoute', () => {
9471127
let mockSetCurrentRoute: jest.Mock;
9481128

‎packages/core/test/tracing/reactnavigationutils.ts‎

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,29 @@ const navigationAction: UnsafeAction = {
1111
},
1212
};
1313

14+
function navigationActionWithPayload(name: string): UnsafeAction {
15+
return {
16+
data: {
17+
action: {
18+
type: 'NAVIGATE',
19+
payload: { name },
20+
},
21+
noop: false,
22+
stack: undefined,
23+
},
24+
};
25+
}
26+
27+
const goBackAction: UnsafeAction = {
28+
data: {
29+
action: {
30+
type: 'GO_BACK',
31+
},
32+
noop: false,
33+
stack: undefined,
34+
},
35+
};
36+
1437
export function createMockNavigationAndAttachTo(sut: ReturnType<typeof reactNavigationIntegration>) {
1538
const mockedNavigationContained = mockNavigationContainer();
1639
const mockedNavigation = {
@@ -62,8 +85,11 @@ export function createMockNavigationAndAttachTo(sut: ReturnType<typeof reactNavi
6285
emitWithoutStateChange: (action: UnsafeAction) => {
6386
mockedNavigationContained.listeners['__unsafe_action__'](action);
6487
},
65-
emitWithStateChange: (action: UnsafeAction) => {
88+
emitWithStateChange: (action: UnsafeAction, route?: NavigationRoute) => {
6689
mockedNavigationContained.listeners['__unsafe_action__'](action);
90+
if (route) {
91+
mockedNavigationContained.currentRoute = route;
92+
}
6793
mockedNavigationContained.listeners['state']({
6894
// this object is not used by the instrumentation
6995
});
@@ -95,6 +121,31 @@ export function createMockNavigationAndAttachTo(sut: ReturnType<typeof reactNavi
95121
};
96122
mockedNavigationContained.listeners['state']({});
97123
},
124+
navigateToNewScreenWithPayload: () => {
125+
mockedNavigationContained.listeners['__unsafe_action__'](navigationActionWithPayload('New Screen'));
126+
mockedNavigationContained.currentRoute = {
127+
key: 'new_screen',
128+
name: 'New Screen',
129+
};
130+
mockedNavigationContained.listeners['state']({});
131+
},
132+
navigateToScreenWithMismatchedPayload: () => {
133+
// Dispatch says "ScreenA" but router resolves to "ScreenB" (e.g. nested navigation)
134+
mockedNavigationContained.listeners['__unsafe_action__'](navigationActionWithPayload('ScreenA'));
135+
mockedNavigationContained.currentRoute = {
136+
key: 'screen_b',
137+
name: 'ScreenB',
138+
};
139+
mockedNavigationContained.listeners['state']({});
140+
},
141+
emitGoBackWithStateChange: () => {
142+
mockedNavigationContained.listeners['__unsafe_action__'](goBackAction);
143+
mockedNavigationContained.currentRoute = {
144+
key: 'previous_screen',
145+
name: 'Previous Screen',
146+
};
147+
mockedNavigationContained.listeners['state']({});
148+
},
98149
emitNavigationWithUndefinedRoute: () => {
99150
mockedNavigationContained.listeners['__unsafe_action__'](navigationAction);
100151
mockedNavigationContained.currentRoute = undefined as any;

0 commit comments

Comments
 (0)