diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/spans.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/spans.test.ts index d0dc72192dca..e5aed8fda13b 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/spans.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-7-lazy-routes/tests/spans.test.ts @@ -1507,3 +1507,33 @@ test('GQL fetch spans are attributed to correct navigation segments when navigat expect(secondNavSegment.trace_id).toBeDefined(); expect(firstNavSegment.trace_id).not.toBe(secondNavSegment.trace_id); }); + +test('Does not rename the previous navigation span with the route of the next navigation', async ({ page }) => { + const navigationNames: string[] = []; + + const navigationsPromise = waitForStreamedSpan('react-router-7-lazy-routes', span => { + if (getSpanOp(span) === 'navigation' && span.is_segment) { + navigationNames.push(span.name); + } + + return navigationNames.length >= 2; + }); + + await page.goto('/'); + + const toAnotherLazy = page.locator('id=navigation-to-another-deep'); + await expect(toAnotherLazy).toBeVisible(); + await toAnotherLazy.click(); + + // No wait in between: the second navigation has to start while the first navigation's span is + // still open, which is when React Router hands it the second location's routes. + const toInnerLazy = page.locator('id=navigate-to-inner-from-deep'); + await expect(toInnerLazy).toBeVisible(); + await toInnerLazy.click(); + + await expect(page.locator('id=innermost-lazy-route')).toBeVisible(); + + await navigationsPromise; + + expect(navigationNames).toEqual(['/another-lazy/sub/:id/:subId', '/lazy/inner/:id/:anotherId/:someAnotherId']); +}); diff --git a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx index f6019445a2a2..ff860013dd59 100644 --- a/packages/react/src/reactrouter-compat-utils/instrumentation.tsx +++ b/packages/react/src/reactrouter-compat-utils/instrumentation.tsx @@ -374,16 +374,13 @@ export function updateNavigationSpan( ): void { const { name: currentName, end_timestamp, attributes } = spanToJSON(activeRootSpan); - // React Router can start resolving the routes for the next location before the SDK has started - // that navigation's span, so the span captured for the resolution is still the previous - // navigation's. Renaming it would ship it under a route the user never visited on it, so a span - // the SDK is still tracking for a different location is left alone. - const client = getClient(); - const trackedNav = client ? activeNavigationSpans.get(client) : undefined; - if (trackedNav?.span === activeRootSpan && trackedNav?.pathname !== location.pathname) { + // React Router resolves a location's routes either side of the SDK starting that navigation's + // span, so a resolution can arrive holding the previous navigation's span. + const spanPathname = (activeRootSpan as { __sentry_navigation_pathname__?: string })?.__sentry_navigation_pathname__; + if (spanPathname !== undefined && spanPathname !== location.pathname) { DEBUG_BUILD && debug.log( - `[React Router] Not renaming the navigation span for "${trackedNav.pathname}" with the route of "${location.pathname}"`, + `[React Router] Not renaming the navigation span for "${spanPathname}" with the route of "${location.pathname}"`, ); return; } @@ -411,6 +408,7 @@ export function updateNavigationSpan( (currentSource === 'route' && source === 'route' && currentNameHasWildcard)); // Route → better route (only if current has wildcard) if (isImprovement) { // With span streaming, span names have to be low cardinality, so we can't fall back to the URL. + const client = getClient(); const isUnparameterizedStreamedNavigation = source !== 'route' && !!client && hasSpanStreamingEnabled(client); activeRootSpan.updateName(isUnparameterizedStreamedNavigation ? NAVIGATION_SPAN_NAME_FALLBACK : name); activeRootSpan.setAttribute(SENTRY_SEGMENT_NAME_SOURCE, source); @@ -1091,6 +1089,9 @@ export function handleNavigation(opts: { } if (navigationSpan) { + // On the span rather than only in the tracked entry, which a late resolution finds already + // moved on to the next navigation. + addNonEnumerableProperty(navigationSpan, '__sentry_navigation_pathname__', location.pathname); // Update the map with the real span (isPlaceholder omitted, defaults to false) activeNavigationSpans.set(client, { span: navigationSpan, diff --git a/packages/react/test/reactrouter-compat-utils/instrumentation.test.tsx b/packages/react/test/reactrouter-compat-utils/instrumentation.test.tsx index efe7228d778b..fd6f00d13690 100644 --- a/packages/react/test/reactrouter-compat-utils/instrumentation.test.tsx +++ b/packages/react/test/reactrouter-compat-utils/instrumentation.test.tsx @@ -134,6 +134,34 @@ describe('reactrouter-compat-utils/instrumentation', () => { expect(mockUpdateName).not.toHaveBeenCalled(); }); + + it('should not rename a span that was started for another location', () => { + const spanOfPreviousNavigation = { ...mockSpan, __sentry_navigation_pathname__: '/previous' }; + + updateNavigationSpan( + spanOfPreviousNavigation as any, + sampleLocation, + sampleRoutes, + true, + makeMockConfig({ matchRoutes: mockMatchRoutes }), + ); + + expect(mockUpdateName).not.toHaveBeenCalled(); + }); + + it('should rename a span that was started for this location', () => { + const spanOfThisNavigation = { ...mockSpan, __sentry_navigation_pathname__: sampleLocation.pathname }; + + updateNavigationSpan( + spanOfThisNavigation as any, + sampleLocation, + sampleRoutes, + false, + makeMockConfig({ matchRoutes: mockMatchRoutes }), + ); + + expect(mockUpdateName).toHaveBeenCalledWith('Test Route'); + }); }); describe('addResolvedRoutesToParent', () => {