Skip to content

feat(react-router): Set navigation.route.id from the matched route id #22373

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
msonnb wants to merge 1 commit into from
Closed

feat(react-router): Set navigation.route.id from the matched route id #22373

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ...ions/react-router-7-framework-instrumentation/tests/performance/navigation.client.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ test.describe('client - hybrid navigation (instrumentation API span + legacy par
data: {
'sentry.source': 'route',
'url.template': '/performance/with/:param',
'navigation.route.id': 'routes/performance/dynamic-param',
'url.path': '/performance/with/sentry',
'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/performance\/with\/sentry$/),
},
Expand Down
1 change: 1 addition & 0 deletions ...ations/react-router-7-framework-instrumentation/tests/performance/pageload.client.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ test.describe('client - instrumentation API pageload', () => {
data: {
'sentry.source': 'route',
'url.template': '/performance/with/:param',
'navigation.route.id': 'routes/performance/dynamic-param',
'url.path': '/performance/with/some-param',
'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/performance\/with\/some-param$/),
},
Expand Down
1 change: 1 addition & 0 deletions ...ts/test-applications/react-router-7-framework/tests/performance/navigation.client.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ test.describe('client - navigation performance', () => {
data: {
'sentry.source': 'route',
'url.template': '/performance/with/:param',
'navigation.route.id': 'routes/performance/dynamic-param',
'url.path': '/performance/with/object-nav',
'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/performance\/with\/object-nav\?foo=bar$/),
},
Expand Down
1 change: 1 addition & 0 deletions ...ests/test-applications/react-router-7-framework/tests/performance/pageload.client.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ test.describe('client - pageload performance', () => {
'sentry.op': 'pageload',
'sentry.source': 'route',
'url.template': '/performance/with/:param',
'navigation.route.id': 'routes/performance/dynamic-param',
'url.path': '/performance/with/sentry',
'url.full': expect.stringMatching(/^https?:\/\/localhost:\d+\/performance\/with\/sentry$/),
},
Expand Down
14 changes: 9 additions & 5 deletions packages/react-router/src/client/createClientInstrumentation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import {
finalizeNavigationSpanFromHydratedRouter,
updateNavigationSpanUrlFromLocation,
} from './utils';
import { URL_TEMPLATE } from '@sentry/conventions/attributes';
import { NAVIGATION_ROUTE_ID, URL_TEMPLATE } from '@sentry/conventions/attributes';

const WINDOW = GLOBAL_OBJ as typeof GLOBAL_OBJ & Window;

Expand Down Expand Up @@ -249,7 +249,7 @@ export function createSentryClientInstrumentation(
const routePattern = pattern || urlPath;
// Parameterize the active navigation root span. (Route hooks don't fire on initial
// pageload, so this only affects navigations.)
updateRootSpanRoute(routePattern, !!pattern);
updateRootSpanRoute(routePattern, !!pattern, routeId);

await startSpan(
{
Expand All @@ -275,7 +275,7 @@ export function createSentryClientInstrumentation(
const urlPath = getPathFromRequest(info.request);
const pattern = normalizeRoutePath(getPattern(info));
const routePattern = pattern || urlPath;
updateRootSpanRoute(routePattern, !!pattern);
updateRootSpanRoute(routePattern, !!pattern, routeId);

await startSpan(
{
Expand Down Expand Up @@ -360,7 +360,7 @@ export function createSentryClientInstrumentation(
* Updates the active navigation/pageload root span name with the parameterized route, so the
* transaction reflects the parameterized route pattern (e.g. `/users/:id`).
*/
function updateRootSpanRoute(routeName: string, hasPattern: boolean): void {
function updateRootSpanRoute(routeName: string, hasPattern: boolean, routeId?: string): void {
if (!hasPattern) {
return;
}
Expand All @@ -377,7 +377,11 @@ function updateRootSpanRoute(routeName: string, hasPattern: boolean): void {
}

updateSpanName(rootSpan, routeName);
rootSpan.setAttributes({ [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', [URL_TEMPLATE]: routeName });
rootSpan.setAttributes({
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
[URL_TEMPLATE]: routeName,
...(routeId && { [NAVIGATION_ROUTE_ID]: routeId }),
});

@cursor cursor Bot Jul 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-leaf route id can stick

Medium Severity

In the instrumentation-API path, navigation.route.id is taken from whichever route's loader/action hook runs, not from the leaf match. Once that sets sentry.source to route, the hydrated-router subscribe path early-returns and never corrects it via getRouteId. Nested apps where only a parent has a client loader can therefore keep a parent route id on the navigation span.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit f597246. Configure here.

@msonnb msonnb Jul 17, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this is fine since we just follow the exisitng behavior of url.template here

}

/**
Comment on lines 377 to 387

@sentry sentry Bot Jul 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: A race condition between parallel loaders on nested routes can cause the NAVIGATION_ROUTE_ID attribute to be set to a non-leaf route's ID.
Severity: MEDIUM

Suggested Fix

Instead of each loader setting the route ID, the instrumentation should deterministically identify the leaf route from the matched routes and set the NAVIGATION_ROUTE_ID once. This could be done by accessing the router state after the loaders have resolved, similar to how hydratedRouter.ts gets the leaf route ID using router.state.matches[last]?.route.id.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: packages/react-router/src/client/createClientInstrumentation.ts#L377-L387

Potential issue: A race condition is introduced in `createClientInstrumentation.ts` when
instrumenting route loaders and actions. When a navigation involves nested routes and
multiple routes in the hierarchy have a loader or action, React Router executes them in
parallel. Each instrumented function calls `updateRootSpanRoute` with its own `routeId`.
Due to the parallel execution, these calls race against each other. The last one to
complete sets the `NAVIGATION_ROUTE_ID` on the root span. This can result in the ID of a
parent route, rather than the intended leaf route, being set, leading to incorrect and
non-deterministic tracing data.

Also affects:

  • packages/react-router/src/client/createClientInstrumentation.ts:249~255
  • packages/react-router/src/client/createClientInstrumentation.ts:275~281

Did we get this right? 👍 / 👎 to inform future reviews.

@msonnb msonnb Jul 20, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as cursor above

Expand Down
12 changes: 10 additions & 2 deletions packages/react-router/src/client/hydratedRouter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,12 @@ import { isClientInstrumentationApiUsed } from './createClientInstrumentation';
import {
finalizeNavigationSpanFromRouterState,
getParameterizedRoute,
getRouteId,
normalizePathname,
resolveNavigateAbsoluteUrl,
resolveNavigateArg,
} from './utils';
import { URL_PATH, URL_TEMPLATE } from '@sentry/conventions/attributes';
import { NAVIGATION_ROUTE_ID, URL_PATH, URL_TEMPLATE } from '@sentry/conventions/attributes';

const GLOBAL_OBJ_WITH_DATA_ROUTER = GLOBAL_OBJ as typeof GLOBAL_OBJ & {
__reactRouterDataRouter?: DataRouter;
Expand Down Expand Up @@ -56,11 +57,13 @@ export function instrumentHydratedRouter(): void {
// this event is for the currently active pageload
normalizePathname(router.state.location.pathname) === normalizePathname(pageloadName)
) {
const pageloadRouteId = getRouteId(router.state);
pageloadSpan.updateName(parameterizePageloadRoute);
pageloadSpan.setAttributes({
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.react_router',
[URL_TEMPLATE]: parameterizePageloadRoute,
...(pageloadRouteId && { [NAVIGATION_ROUTE_ID]: pageloadRouteId }),
});
}
}
Expand Down Expand Up @@ -149,8 +152,13 @@ export function instrumentHydratedRouter(): void {
(destinationPathname === normalizePathname(rootSpanName) ||
(spanPathname && destinationPathname === normalizePathname(spanPathname)))
) {
const routeId = getRouteId(newState);
rootSpan.updateName(parameterizedRoute);
rootSpan.setAttributes({ [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', [URL_TEMPLATE]: parameterizedRoute });
rootSpan.setAttributes({
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
[URL_TEMPLATE]: parameterizedRoute,
...(routeId && { [NAVIGATION_ROUTE_ID]: routeId }),
});
}
});
return true;
Expand Down
17 changes: 15 additions & 2 deletions packages/react-router/src/client/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { getAbsoluteUrl } from '@sentry/browser';
import type { Span } from '@sentry/core';
import { GLOBAL_OBJ, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core';
import { URL_FULL, URL_PATH, URL_TEMPLATE } from '@sentry/conventions/attributes';
import { NAVIGATION_ROUTE_ID, URL_FULL, URL_PATH, URL_TEMPLATE } from '@sentry/conventions/attributes';
import type { DataRouter, RouterState } from 'react-router';

const WINDOW = GLOBAL_OBJ as typeof GLOBAL_OBJ & Window;
Expand Down Expand Up @@ -122,6 +122,14 @@ export function getParameterizedRoute(routerState: RouterState): string {
return normalizePathname(lastMatch?.route.path || routerState.location.pathname);
}

/**
* Returns the framework-assigned id of the leaf matched route (e.g. `routes/blog.$slug`), which
* is distinct from the parameterized path pattern used for `url.template`.
*/
export function getRouteId(routerState: RouterState): string | undefined {
return routerState.matches[routerState.matches.length - 1]?.route.id;
}

/**
* Updates a navigation span's URL attributes and parameterizes its name from the router state.
* Used after numeric navigations (`navigate(-1)` / `navigate(1)`) where route hooks may not
Expand All @@ -144,8 +152,13 @@ export function finalizeNavigationSpanFromRouterState(span: Span, routerState: R
normalizePathname(routerState.location.pathname) === normalizePathname(pathname)
) {
const parameterizedRoute = getParameterizedRoute(routerState);
const routeId = getRouteId(routerState);
span.updateName(parameterizedRoute);
span.setAttributes({ [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', [URL_TEMPLATE]: parameterizedRoute });
span.setAttributes({
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
[URL_TEMPLATE]: parameterizedRoute,
...(routeId && { [NAVIGATION_ROUTE_ID]: routeId }),
});
}
}

Expand Down
6 changes: 5 additions & 1 deletion packages/react-router/test/client/createClientInstrumentation.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -880,7 +880,11 @@ describe('navigation root parameterization', () => {
});

expect(core.updateSpanName).toHaveBeenCalledWith(mockRootSpan, '/users/:id');
expect(mockRootSpan.setAttributes).toHaveBeenCalledWith({ 'sentry.source': 'route', 'url.template': '/users/:id' });
expect(mockRootSpan.setAttributes).toHaveBeenCalledWith({
'sentry.source': 'route',
'url.template': '/users/:id',
'navigation.route.id': 'r',
});
});

it('does not rename the root span when the route has no pattern', async () => {
Expand Down
18 changes: 18 additions & 0 deletions packages/react-router/test/client/hydratedRouter.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,24 @@ describe('instrumentHydratedRouter', () => {
});
});

it('sets navigation.route.id from the leaf matched route id on state change', () => {
instrumentHydratedRouter();
const callback = mockRouter.subscribe.mock.calls[0][0];
const newState = {
location: { pathname: '/foo/bar' },
matches: [{ route: { path: '/foo/:id', id: 'routes/foo.$id' } }],
navigation: { state: 'idle' },
};
mockRouter.navigate('/foo/bar');
(core.getActiveSpan as any).mockReturnValue(mockNavigationSpan);
callback(newState);
expect(mockNavigationSpan.setAttributes).toHaveBeenCalledWith({
[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
'url.template': '/foo/:id',
'navigation.route.id': 'routes/foo.$id',
});
});

it('does not overwrite pageload origin when the pageload is still active', () => {
// Regression test for #20784: a static-route pageload (where pathname == rootSpanName) was
// being tagged with `origin: auto.navigation.react_router` because the subscribe callback
Expand Down
16 changes: 16 additions & 0 deletions packages/react-router/test/client/utils.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,22 @@ describe('finalizeNavigationSpanFromRouterState', () => {
});
});

it('sets navigation.route.id from the leaf matched route id', () => {
const span = { updateName: vi.fn(), setAttributes: vi.fn() } as any;

finalizeNavigationSpanFromRouterState(span, {
location: { pathname: '/performance/' },
matches: [{ route: { path: '', id: 'routes/performance/index' } }],
navigation: { state: 'idle' },
} as any);

expect(span.setAttributes).toHaveBeenLastCalledWith({
'sentry.source': 'route',
'url.template': '/performance',
'navigation.route.id': 'routes/performance/index',
});
});

it('sets url attributes but skips parameterization when router state is stale', () => {
const span = { updateName: vi.fn(), setAttributes: vi.fn() } as any;

Expand Down
Loading