From 24911e550391cf6a7a8d3e225be6dece0e6f55c4 Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Fri, 2 May 2025 15:35:10 +0200 Subject: [PATCH 01/14] add router instrumentation --- .../react-router/src/client/hydratedRouter.ts | 138 ++++++++++++++++++ 1 file changed, 138 insertions(+) create mode 100644 packages/react-router/src/client/hydratedRouter.ts diff --git a/packages/react-router/src/client/hydratedRouter.ts b/packages/react-router/src/client/hydratedRouter.ts new file mode 100644 index 000000000000..b0371a8040d4 --- /dev/null +++ b/packages/react-router/src/client/hydratedRouter.ts @@ -0,0 +1,138 @@ +import { startBrowserTracingNavigationSpan } from '@sentry/browser'; +import type { Span } from '@sentry/core'; +import { + getActiveSpan, + getClient, + getRootSpan, + GLOBAL_OBJ, + SEMANTIC_ATTRIBUTE_SENTRY_OP, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, + spanToJSON, +} from '@sentry/core'; +import type { DataRouter, RouterState } from 'react-router'; + +const GLOBAL_OBJ_WITH_DATA_ROUTER = GLOBAL_OBJ as typeof GLOBAL_OBJ & { + __reactRouterDataRouter?: DataRouter; +}; + +const MAX_RETRIES = 40; // 2 seconds at 50ms interval + +/** + * Instruments the React Router Data Router for pageloads and navigation. + * + * This function waits for the router to be available after hydration, then: + * 1. Updates the pageload transaction with parameterized route info + * 2. Patches router.navigate() to create navigation transactions + * 3. Subscribes to router state changes to update navigation transactions with parameterized routes + */ +export function instrumentHydratedRouter(): void { + function trySubscribe(): boolean { + const router = GLOBAL_OBJ_WITH_DATA_ROUTER.__reactRouterDataRouter; + + if (router) { + // The first time we hit the router, we try to update the pageload transaction + // todo: update pageload tx here + const pageloadSpan = getActiveRootSpan(); + const pageloadName = pageloadSpan ? spanToJSON(pageloadSpan).description : undefined; + const parameterizePageloadRoute = getParameterizedRoute(router.state); + if ( + pageloadName && + normalizePathname(router.state.location.pathname) === normalizePathname(pageloadName) && // this event is for the currently active pageload + normalizePathname(parameterizePageloadRoute) !== normalizePathname(pageloadName) // route is not parameterized yet + ) { + pageloadSpan?.updateName(parameterizePageloadRoute); + pageloadSpan?.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route'); + } + + // Patching navigate for creating accurate navigation transactions + if (typeof router.navigate === 'function') { + const originalNav = router.navigate.bind(router); + router.navigate = function patchedNavigate(...args) { + maybeCreateNavigationTransaction( + String(args[0]) || '', // will be updated anyway + 'url', // this also will be updated once we have the parameterized route + ); + return originalNav(...args); + }; + } + + // Subscribe to router state changes to update navigation transactions with parameterized routes + router.subscribe(newState => { + const navigationSpan = getActiveRootSpan(); + const navigationSpanName = navigationSpan ? spanToJSON(navigationSpan).description : undefined; + const parameterizedNavRoute = getParameterizedRoute(newState); + + if ( + navigationSpanName && // we have an active pageload tx + newState.navigation.state === 'idle' && // navigation has completed + normalizePathname(newState.location.pathname) === normalizePathname(navigationSpanName) && // this event is for the currently active navigation + normalizePathname(parameterizedNavRoute) !== normalizePathname(navigationSpanName) // route is not parameterized yet + ) { + navigationSpan?.updateName(parameterizedNavRoute); + navigationSpan?.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route'); + } + }); + return true; + } + return false; + } + + // Wait until the router is available (since the SDK loads before hydration) + if (!trySubscribe()) { + let retryCount = 0; + // Retry until the router is available or max retries reached + const interval = setInterval(() => { + if (trySubscribe() || retryCount >= MAX_RETRIES) { + clearInterval(interval); + } + retryCount++; + }, 50); + } +} + +function maybeCreateNavigationTransaction(name: string, source: 'url' | 'route'): Span | undefined { + const client = getClient(); + + if (!client) { + return undefined; + } + + return startBrowserTracingNavigationSpan(client, { + name, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source, + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react-router', + }, + }); +} + +function getActiveRootSpan(): Span | undefined { + const activeSpan = getActiveSpan(); + if (!activeSpan) { + return undefined; + } + + const rootSpan = getRootSpan(activeSpan); + + const op = spanToJSON(rootSpan).op; + + // Only use this root span if it is a pageload or navigation span + return op === 'navigation' || op === 'pageload' ? rootSpan : undefined; +} + +function getParameterizedRoute(routerState: RouterState): string { + const lastMatch = routerState.matches[routerState.matches.length - 1]; + return lastMatch?.route.path ?? routerState.location.pathname; +} + +function normalizePathname(pathname: string): string { + // Ensure it starts with a single slash + let normalized = pathname.startsWith('/') ? pathname : `/${pathname}`; + // Remove trailing slash unless it's the root + if (normalized.length > 1 && normalized.endsWith('/')) { + normalized = normalized.slice(0, -1); + } + return normalized; +} From ec310ed13d750c9ae2cb14182ed1bd30274031be Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Sat, 3 May 2025 22:17:23 +0200 Subject: [PATCH 02/14] add integration --- packages/react-router/src/client/sdk.ts | 21 +++++++++++++----- .../src/client/tracingIntegration.ts | 22 +++++++++++++++++++ 2 files changed, 38 insertions(+), 5 deletions(-) create mode 100644 packages/react-router/src/client/tracingIntegration.ts diff --git a/packages/react-router/src/client/sdk.ts b/packages/react-router/src/client/sdk.ts index 688a8ba460f1..d774c46d4376 100644 --- a/packages/react-router/src/client/sdk.ts +++ b/packages/react-router/src/client/sdk.ts @@ -2,18 +2,29 @@ import type { BrowserOptions } from '@sentry/browser'; import { init as browserInit } from '@sentry/browser'; import type { Client } from '@sentry/core'; import { applySdkMetadata, setTag } from '@sentry/core'; +import { BROWSER_TRACING_INTEGRATION_ID } from '../../../browser/build/npm/types/tracing/browserTracingIntegration'; +import { reactRouterTracingIntegration } from './tracingIntegration'; /** * Initializes the client side of the React Router SDK. */ export function init(options: BrowserOptions): Client | undefined { - const opts = { - ...options, - }; + // If BrowserTracing integration was passed to options, replace it with React Router tracing integration + if (options.integrations && Array.isArray(options.integrations)) { + const hasBrowserTracing = options.integrations.some( + integration => integration.name === BROWSER_TRACING_INTEGRATION_ID, + ); + if (hasBrowserTracing) { + options.integrations = options.integrations.filter( + integration => integration.name !== BROWSER_TRACING_INTEGRATION_ID, + ); + options.integrations.push(reactRouterTracingIntegration()); + } + } - applySdkMetadata(opts, 'react-router', ['react-router', 'browser']); + applySdkMetadata(options, 'react-router', ['react-router', 'browser']); - const client = browserInit(opts); + const client = browserInit(options); setTag('runtime', 'browser'); diff --git a/packages/react-router/src/client/tracingIntegration.ts b/packages/react-router/src/client/tracingIntegration.ts new file mode 100644 index 000000000000..829dc68a76da --- /dev/null +++ b/packages/react-router/src/client/tracingIntegration.ts @@ -0,0 +1,22 @@ +import { browserTracingIntegration as originalBrowserTracingIntegration } from '@sentry/browser'; +import type { Integration } from '@sentry/core'; +import { instrumentHydratedRouter } from './hydratedRouter'; + +/** + * Browser tracing integration for React Router (Framework) applications. + * This integration will create navigation spans and enhance transactions names with parameterized routes. + */ +export function reactRouterTracingIntegration(): Integration { + const browserTracingIntegrationInstance = originalBrowserTracingIntegration({ + // Navigation transactions are started within the hydrated router instrumentation + instrumentNavigation: false, + }); + + return { + ...browserTracingIntegrationInstance, + afterAllSetup(client) { + browserTracingIntegrationInstance.afterAllSetup(client); + instrumentHydratedRouter(); + }, + }; +} From ec1d1889638ecedc1ffddd0cd73db69d23a5e254 Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Sat, 3 May 2025 22:20:52 +0200 Subject: [PATCH 03/14] whoopsie --- packages/react-router/src/client/sdk.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/react-router/src/client/sdk.ts b/packages/react-router/src/client/sdk.ts index d774c46d4376..d6cbdca013d8 100644 --- a/packages/react-router/src/client/sdk.ts +++ b/packages/react-router/src/client/sdk.ts @@ -2,9 +2,10 @@ import type { BrowserOptions } from '@sentry/browser'; import { init as browserInit } from '@sentry/browser'; import type { Client } from '@sentry/core'; import { applySdkMetadata, setTag } from '@sentry/core'; -import { BROWSER_TRACING_INTEGRATION_ID } from '../../../browser/build/npm/types/tracing/browserTracingIntegration'; import { reactRouterTracingIntegration } from './tracingIntegration'; +const BROWSER_TRACING_INTEGRATION_ID = 'BrowserTracing'; + /** * Initializes the client side of the React Router SDK. */ From 6de5f66d08711be3788a6772c144c5486ba13c42 Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Sun, 4 May 2025 11:38:01 +0200 Subject: [PATCH 04/14] export ReactRouterTracingIntegration --- packages/react-router/src/client/index.ts | 1 + packages/react-router/src/client/sdk.ts | 16 +++++++++------- .../src/client/tracingIntegration.ts | 1 + 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/packages/react-router/src/client/index.ts b/packages/react-router/src/client/index.ts index 8e25b84c4a0c..1bb86ec16deb 100644 --- a/packages/react-router/src/client/index.ts +++ b/packages/react-router/src/client/index.ts @@ -1,3 +1,4 @@ export * from '@sentry/browser'; export { init } from './sdk'; +export { reactRouterTracingIntegration } from './tracingIntegration'; diff --git a/packages/react-router/src/client/sdk.ts b/packages/react-router/src/client/sdk.ts index d6cbdca013d8..a9b13f2e7cd0 100644 --- a/packages/react-router/src/client/sdk.ts +++ b/packages/react-router/src/client/sdk.ts @@ -1,8 +1,7 @@ import type { BrowserOptions } from '@sentry/browser'; import { init as browserInit } from '@sentry/browser'; import type { Client } from '@sentry/core'; -import { applySdkMetadata, setTag } from '@sentry/core'; -import { reactRouterTracingIntegration } from './tracingIntegration'; +import { applySdkMetadata, consoleSandbox, setTag } from '@sentry/core'; const BROWSER_TRACING_INTEGRATION_ID = 'BrowserTracing'; @@ -10,16 +9,19 @@ const BROWSER_TRACING_INTEGRATION_ID = 'BrowserTracing'; * Initializes the client side of the React Router SDK. */ export function init(options: BrowserOptions): Client | undefined { - // If BrowserTracing integration was passed to options, replace it with React Router tracing integration + // If BrowserTracing integration was passed to options, emit a warning if (options.integrations && Array.isArray(options.integrations)) { const hasBrowserTracing = options.integrations.some( integration => integration.name === BROWSER_TRACING_INTEGRATION_ID, ); + if (hasBrowserTracing) { - options.integrations = options.integrations.filter( - integration => integration.name !== BROWSER_TRACING_INTEGRATION_ID, - ); - options.integrations.push(reactRouterTracingIntegration()); + consoleSandbox(() => { + // eslint-disable-next-line no-console + console.warn( + 'browserTracingIntegration is not fully compatible with @sentry/react-router. Please use reactRouterTracingIntegration instead.', + ); + }); } } diff --git a/packages/react-router/src/client/tracingIntegration.ts b/packages/react-router/src/client/tracingIntegration.ts index 829dc68a76da..01b71f36d92a 100644 --- a/packages/react-router/src/client/tracingIntegration.ts +++ b/packages/react-router/src/client/tracingIntegration.ts @@ -14,6 +14,7 @@ export function reactRouterTracingIntegration(): Integration { return { ...browserTracingIntegrationInstance, + name: 'ReactRouterTracingIntegration', afterAllSetup(client) { browserTracingIntegrationInstance.afterAllSetup(client); instrumentHydratedRouter(); From 3b485fd80a50958ba75eeb9596c4946ee1dc7fb9 Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Sun, 4 May 2025 11:43:33 +0200 Subject: [PATCH 05/14] add client tests --- packages/react-router/test/client/sdk.test.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/packages/react-router/test/client/sdk.test.ts b/packages/react-router/test/client/sdk.test.ts index 118ddd775217..d6767ccfff23 100644 --- a/packages/react-router/test/client/sdk.test.ts +++ b/packages/react-router/test/client/sdk.test.ts @@ -1,9 +1,12 @@ import * as SentryBrowser from '@sentry/browser'; import { getCurrentScope, getGlobalScope, getIsolationScope, SDK_VERSION } from '@sentry/browser'; +import * as SentryCore from '@sentry/core'; import { afterEach, describe, expect, it, vi } from 'vitest'; import { init as reactRouterInit } from '../../src/client'; const browserInit = vi.spyOn(SentryBrowser, 'init'); +const setTag = vi.spyOn(SentryCore, 'setTag'); +const consoleWarn = vi.spyOn(console, 'warn').mockImplementation(() => {}); describe('React Router client SDK', () => { describe('init', () => { @@ -41,5 +44,20 @@ describe('React Router client SDK', () => { it('returns client from init', () => { expect(reactRouterInit({})).not.toBeUndefined(); }); + + it('sets the runtime tag to browser', () => { + reactRouterInit({}); + expect(setTag).toHaveBeenCalledWith('runtime', 'browser'); + }); + + it('warns if BrowserTracing integration is present', () => { + reactRouterInit({ + integrations: [{ name: 'BrowserTracing' }], + }); + + expect(consoleWarn).toHaveBeenCalledWith( + 'browserTracingIntegration is not fully compatible with @sentry/react-router. Please use reactRouterTracingIntegration instead.', + ); + }); }); }); From 8461de24b7ab2c2c1f0a95e162747f2b99fa3cfa Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Sun, 4 May 2025 12:00:02 +0200 Subject: [PATCH 06/14] tracing test --- .../test/client/tracingIntegration.test.ts | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 packages/react-router/test/client/tracingIntegration.test.ts diff --git a/packages/react-router/test/client/tracingIntegration.test.ts b/packages/react-router/test/client/tracingIntegration.test.ts new file mode 100644 index 000000000000..9d511b4c6bde --- /dev/null +++ b/packages/react-router/test/client/tracingIntegration.test.ts @@ -0,0 +1,30 @@ +import * as sentryBrowser from '@sentry/browser'; +import type { Client } from '@sentry/core'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import * as hydratedRouterModule from '../../src/client/hydratedRouter'; +import { reactRouterTracingIntegration } from '../../src/client/tracingIntegration'; + +describe('reactRouterTracingIntegration', () => { + afterEach(() => { + vi.clearAllMocks(); + }); + + it('returns an integration with the correct name and properties', () => { + const integration = reactRouterTracingIntegration(); + expect(integration.name).toBe('ReactRouterTracingIntegration'); + expect(typeof integration.afterAllSetup).toBe('function'); + }); + + it('calls instrumentHydratedRouter and browserTracingIntegrationInstance.afterAllSetup in afterAllSetup', () => { + const browserTracingSpy = vi.spyOn(sentryBrowser, 'browserTracingIntegration').mockImplementation(() => ({ + afterAllSetup: vi.fn(), + name: 'BrowserTracing', + })); + const instrumentSpy = vi.spyOn(hydratedRouterModule, 'instrumentHydratedRouter').mockImplementation(() => null); + const integration = reactRouterTracingIntegration(); + integration.afterAllSetup?.({} as Client); + + expect(browserTracingSpy).toHaveBeenCalled(); + expect(instrumentSpy).toHaveBeenCalled(); + }); +}); From 001da094181ac0b9d023e81451232608f6fcb4bf Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Sun, 4 May 2025 12:04:03 +0200 Subject: [PATCH 07/14] add tests for hydrated router --- .../test/client/hydratedRouter.test.ts | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 packages/react-router/test/client/hydratedRouter.test.ts diff --git a/packages/react-router/test/client/hydratedRouter.test.ts b/packages/react-router/test/client/hydratedRouter.test.ts new file mode 100644 index 000000000000..ec26d05a9212 --- /dev/null +++ b/packages/react-router/test/client/hydratedRouter.test.ts @@ -0,0 +1,91 @@ +import * as browser from '@sentry/browser'; +import * as core from '@sentry/core'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { instrumentHydratedRouter } from '../../src/client/hydratedRouter'; + +vi.mock('@sentry/core', async () => { + const actual = await vi.importActual('@sentry/core'); + return { + ...actual, + getActiveSpan: vi.fn(), + getRootSpan: vi.fn(), + spanToJSON: vi.fn(), + getClient: vi.fn(), + SEMANTIC_ATTRIBUTE_SENTRY_OP: 'op', + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN: 'origin', + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE: 'source', + GLOBAL_OBJ: globalThis, + }; +}); +vi.mock('@sentry/browser', () => ({ + startBrowserTracingNavigationSpan: vi.fn(), +})); + +describe('instrumentHydratedRouter', () => { + let originalRouter: any; + let mockRouter: any; + let mockPageloadSpan: any; + let mockNavigationSpan: any; + + beforeEach(() => { + originalRouter = (globalThis as any).__reactRouterDataRouter; + mockRouter = { + state: { + location: { pathname: '/foo' }, + matches: [{ route: { path: '/foo/:id' } }], + }, + navigate: vi.fn(), + subscribe: vi.fn(), + }; + (globalThis as any).__reactRouterDataRouter = mockRouter; + + mockPageloadSpan = { updateName: vi.fn(), setAttribute: vi.fn() }; + mockNavigationSpan = { updateName: vi.fn(), setAttribute: vi.fn() }; + + (core.getActiveSpan as any).mockReturnValue(mockPageloadSpan); + (core.getRootSpan as any).mockImplementation((span: any) => span); + (core.spanToJSON as any).mockImplementation((_span: any) => ({ + description: '/foo', + op: 'pageload', + })); + (core.getClient as any).mockReturnValue({}); + (browser.startBrowserTracingNavigationSpan as any).mockReturnValue(mockNavigationSpan); + }); + + afterEach(() => { + (globalThis as any).__reactRouterDataRouter = originalRouter; + vi.clearAllMocks(); + }); + + it('subscribes to the router and patches navigate', () => { + instrumentHydratedRouter(); + expect(typeof mockRouter.navigate).toBe('function'); + expect(mockRouter.subscribe).toHaveBeenCalled(); + }); + + it('updates pageload transaction name if needed', () => { + instrumentHydratedRouter(); + expect(mockPageloadSpan.updateName).toHaveBeenCalled(); + expect(mockPageloadSpan.setAttribute).toHaveBeenCalled(); + }); + + it('creates navigation transaction on navigate', () => { + instrumentHydratedRouter(); + mockRouter.navigate('/bar'); + expect(browser.startBrowserTracingNavigationSpan).toHaveBeenCalled(); + }); + + it('updates navigation transaction on state change', () => { + instrumentHydratedRouter(); + // Simulate a state change to idle + const callback = mockRouter.subscribe.mock.calls[0][0]; + const newState = { + location: { pathname: '/foo' }, + matches: [{ route: { path: '/foo/:id' } }], + navigation: { state: 'idle' }, + }; + callback(newState); + expect(mockPageloadSpan.updateName).toHaveBeenCalled(); + expect(mockPageloadSpan.setAttribute).toHaveBeenCalled(); + }); +}); From a34fdc0b0f632e6e46ea5b80686d99902eec110c Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Sun, 4 May 2025 15:35:12 +0200 Subject: [PATCH 08/14] fix parameterized routes --- packages/react-router/src/client/hydratedRouter.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-router/src/client/hydratedRouter.ts b/packages/react-router/src/client/hydratedRouter.ts index b0371a8040d4..9733b5baea86 100644 --- a/packages/react-router/src/client/hydratedRouter.ts +++ b/packages/react-router/src/client/hydratedRouter.ts @@ -124,7 +124,7 @@ function getActiveRootSpan(): Span | undefined { function getParameterizedRoute(routerState: RouterState): string { const lastMatch = routerState.matches[routerState.matches.length - 1]; - return lastMatch?.route.path ?? routerState.location.pathname; + return normalizePathname(lastMatch?.route.path ?? routerState.location.pathname); } function normalizePathname(pathname: string): string { From 7d13493309a84f83ee0516cfbdbf81a70676e01f Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Sun, 4 May 2025 15:35:18 +0200 Subject: [PATCH 09/14] update e2e test --- .../app/entry.client.tsx | 2 +- .../tests/performance/pageload.client.test.ts | 51 +++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/entry.client.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/entry.client.tsx index 2200fcea97c3..925c1e6ab143 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/entry.client.tsx +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/entry.client.tsx @@ -8,7 +8,7 @@ Sentry.init({ // todo: get this from env dsn: 'https://username@domain/123', tunnel: `http://localhost:3031/`, // proxy server - integrations: [Sentry.browserTracingIntegration()], + integrations: [Sentry.reactRouterTracingIntegration()], tracesSampleRate: 1.0, tracePropagationTargets: [/^\//], }); diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework/tests/performance/pageload.client.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-framework/tests/performance/pageload.client.test.ts index c53494c723b4..cd76bf06a62e 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-framework/tests/performance/pageload.client.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework/tests/performance/pageload.client.test.ts @@ -53,6 +53,57 @@ test.describe('client - pageload performance', () => { }); }); + test('should update pageload transaction for dynamic routes', async ({ page }) => { + const txPromise = waitForTransaction(APP_NAME, async transactionEvent => { + console.log('tx', transactionEvent.transaction); + return transactionEvent.transaction === '/performance/with/:param'; + }); + + await page.goto(`/performance/with/sentry`); + + const transaction = await txPromise; + + expect(transaction).toMatchObject({ + contexts: { + trace: { + span_id: expect.any(String), + trace_id: expect.any(String), + data: { + 'sentry.origin': 'auto.pageload.browser', + 'sentry.op': 'pageload', + 'sentry.source': 'route', + }, + op: 'pageload', + origin: 'auto.pageload.browser', + }, + }, + spans: expect.any(Array), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + transaction: '/performance/with/:param', + type: 'transaction', + transaction_info: { source: 'route' }, + measurements: expect.any(Object), + platform: 'javascript', + request: { + url: expect.stringContaining('/performance/with/sentry'), + headers: expect.any(Object), + }, + event_id: expect.any(String), + environment: 'qa', + sdk: { + integrations: expect.arrayContaining([expect.any(String)]), + name: 'sentry.javascript.react-router', + version: expect.any(String), + packages: [ + { name: 'npm:@sentry/react-router', version: expect.any(String) }, + { name: 'npm:@sentry/browser', version: expect.any(String) }, + ], + }, + tags: { runtime: 'browser' }, + }); + }); + // todo: this page is currently not prerendered (see react-router.config.ts) test('should send pageload transaction for prerendered pages', async ({ page }) => { const txPromise = waitForTransaction(APP_NAME, async transactionEvent => { From e03e691d34625d99fbeb506c2932bb20fa9c0595 Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Sun, 4 May 2025 15:57:17 +0200 Subject: [PATCH 10/14] add navigation e2e tests --- .../react-router-7-framework/app/routes.ts | 1 + .../app/routes/performance/index.tsx | 12 +- .../app/routes/performance/ssr.tsx | 7 ++ .../performance/navigation.client.test.ts | 109 ++++++++++++++++++ .../tests/performance/pageload.client.test.ts | 1 - 5 files changed, 128 insertions(+), 2 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/react-router-7-framework/app/routes/performance/ssr.tsx create mode 100644 dev-packages/e2e-tests/test-applications/react-router-7-framework/tests/performance/navigation.client.test.ts diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/routes.ts b/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/routes.ts index bb7472366681..1d8ab1b24a74 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/routes.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/routes.ts @@ -12,6 +12,7 @@ export default [ ]), ...prefix('performance', [ index('routes/performance/index.tsx'), + route('ssr', 'routes/performance/ssr.tsx'), route('with/:param', 'routes/performance/dynamic-param.tsx'), route('static', 'routes/performance/static.tsx'), ]), diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/routes/performance/index.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/routes/performance/index.tsx index 9d55975e61a5..99086aadfeae 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/routes/performance/index.tsx +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/routes/performance/index.tsx @@ -1,3 +1,13 @@ +import { Link } from 'react-router'; + export default function PerformancePage() { - return

Performance Page

; + return ( +
+

Performance Page

+ +
+ ); } diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/routes/performance/ssr.tsx b/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/routes/performance/ssr.tsx new file mode 100644 index 000000000000..253e964ff15d --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework/app/routes/performance/ssr.tsx @@ -0,0 +1,7 @@ +export default function SsrPage() { + return ( +
+

SSR Page

+
+ ); +} diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework/tests/performance/navigation.client.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-framework/tests/performance/navigation.client.test.ts new file mode 100644 index 000000000000..b73d6b2907e1 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework/tests/performance/navigation.client.test.ts @@ -0,0 +1,109 @@ +import { expect, test } from '@playwright/test'; +import { waitForTransaction } from '@sentry-internal/test-utils'; +import { APP_NAME } from '../constants'; + +test.describe('client - navigation performance', () => { + test('should create navigation transaction', async ({ page }) => { + const navigationPromise = waitForTransaction(APP_NAME, async transactionEvent => { + console.log('tx', transactionEvent.transaction); + return transactionEvent.transaction === '/performance/ssr'; + }); + + await page.goto(`/performance`); // pageload + await page.waitForTimeout(1000); // give it a sec before navigation + await page.getByRole('link', { name: 'SSR Page' }).click(); // navigation + + const transaction = await navigationPromise; + + expect(transaction).toMatchObject({ + contexts: { + trace: { + span_id: expect.any(String), + trace_id: expect.any(String), + data: { + 'sentry.origin': 'auto.navigation.react-router', + 'sentry.op': 'navigation', + 'sentry.source': 'url', + }, + op: 'navigation', + origin: 'auto.navigation.react-router', + }, + }, + spans: expect.any(Array), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + transaction: '/performance/ssr', + type: 'transaction', + transaction_info: { source: 'url' }, + platform: 'javascript', + request: { + url: expect.stringContaining('/performance/ssr'), + headers: expect.any(Object), + }, + event_id: expect.any(String), + environment: 'qa', + sdk: { + integrations: expect.arrayContaining([expect.any(String)]), + name: 'sentry.javascript.react-router', + version: expect.any(String), + packages: [ + { name: 'npm:@sentry/react-router', version: expect.any(String) }, + { name: 'npm:@sentry/browser', version: expect.any(String) }, + ], + }, + tags: { runtime: 'browser' }, + }); + }); + + test('should update navigation transaction for dynamic routes', async ({ page }) => { + const txPromise = waitForTransaction(APP_NAME, async transactionEvent => { + console.log('tx', transactionEvent.transaction); + return transactionEvent.transaction === '/performance/with/:param'; + }); + + await page.goto(`/performance`); // pageload + await page.waitForTimeout(1000); // give it a sec before navigation + await page.getByRole('link', { name: 'With Param Page' }).click(); // navigation + + const transaction = await txPromise; + + expect(transaction).toMatchObject({ + contexts: { + trace: { + span_id: expect.any(String), + trace_id: expect.any(String), + data: { + 'sentry.origin': 'auto.navigation.react-router', + 'sentry.op': 'navigation', + 'sentry.source': 'route', + }, + op: 'navigation', + origin: 'auto.navigation.react-router', + }, + }, + spans: expect.any(Array), + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + transaction: '/performance/with/:param', + type: 'transaction', + transaction_info: { source: 'route' }, + platform: 'javascript', + request: { + url: expect.stringContaining('/performance/with/sentry'), + headers: expect.any(Object), + }, + event_id: expect.any(String), + environment: 'qa', + sdk: { + integrations: expect.arrayContaining([expect.any(String)]), + name: 'sentry.javascript.react-router', + version: expect.any(String), + packages: [ + { name: 'npm:@sentry/react-router', version: expect.any(String) }, + { name: 'npm:@sentry/browser', version: expect.any(String) }, + ], + }, + tags: { runtime: 'browser' }, + }); + }); +}); diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework/tests/performance/pageload.client.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-framework/tests/performance/pageload.client.test.ts index cd76bf06a62e..e283ea522c4a 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-framework/tests/performance/pageload.client.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework/tests/performance/pageload.client.test.ts @@ -55,7 +55,6 @@ test.describe('client - pageload performance', () => { test('should update pageload transaction for dynamic routes', async ({ page }) => { const txPromise = waitForTransaction(APP_NAME, async transactionEvent => { - console.log('tx', transactionEvent.transaction); return transactionEvent.transaction === '/performance/with/:param'; }); From 816cfd82950ca4ca1eb5b32d22286e4957b9519e Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Sun, 4 May 2025 15:57:42 +0200 Subject: [PATCH 11/14] . --- .../tests/performance/navigation.client.test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/react-router-7-framework/tests/performance/navigation.client.test.ts b/dev-packages/e2e-tests/test-applications/react-router-7-framework/tests/performance/navigation.client.test.ts index b73d6b2907e1..57e3e764d6a8 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-7-framework/tests/performance/navigation.client.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-7-framework/tests/performance/navigation.client.test.ts @@ -5,7 +5,6 @@ import { APP_NAME } from '../constants'; test.describe('client - navigation performance', () => { test('should create navigation transaction', async ({ page }) => { const navigationPromise = waitForTransaction(APP_NAME, async transactionEvent => { - console.log('tx', transactionEvent.transaction); return transactionEvent.transaction === '/performance/ssr'; }); @@ -57,7 +56,6 @@ test.describe('client - navigation performance', () => { test('should update navigation transaction for dynamic routes', async ({ page }) => { const txPromise = waitForTransaction(APP_NAME, async transactionEvent => { - console.log('tx', transactionEvent.transaction); return transactionEvent.transaction === '/performance/with/:param'; }); From 2033387a0eb4a6993ea82bcc7d9e778249f8bf88 Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Mon, 5 May 2025 10:53:48 +0200 Subject: [PATCH 12/14] update tests --- .../test/client/hydratedRouter.test.ts | 32 +++++++++++++++---- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/packages/react-router/test/client/hydratedRouter.test.ts b/packages/react-router/test/client/hydratedRouter.test.ts index ec26d05a9212..98ed1a241d93 100644 --- a/packages/react-router/test/client/hydratedRouter.test.ts +++ b/packages/react-router/test/client/hydratedRouter.test.ts @@ -31,7 +31,7 @@ describe('instrumentHydratedRouter', () => { originalRouter = (globalThis as any).__reactRouterDataRouter; mockRouter = { state: { - location: { pathname: '/foo' }, + location: { pathname: '/foo/bar' }, matches: [{ route: { path: '/foo/:id' } }], }, navigate: vi.fn(), @@ -45,7 +45,7 @@ describe('instrumentHydratedRouter', () => { (core.getActiveSpan as any).mockReturnValue(mockPageloadSpan); (core.getRootSpan as any).mockImplementation((span: any) => span); (core.spanToJSON as any).mockImplementation((_span: any) => ({ - description: '/foo', + description: '/foo/bar', op: 'pageload', })); (core.getClient as any).mockReturnValue({}); @@ -75,17 +75,37 @@ describe('instrumentHydratedRouter', () => { expect(browser.startBrowserTracingNavigationSpan).toHaveBeenCalled(); }); - it('updates navigation transaction on state change', () => { + it('updates navigation transaction on state change to idle', () => { instrumentHydratedRouter(); // Simulate a state change to idle const callback = mockRouter.subscribe.mock.calls[0][0]; const newState = { - location: { pathname: '/foo' }, + location: { pathname: '/foo/bar' }, matches: [{ route: { path: '/foo/:id' } }], navigation: { state: 'idle' }, }; + mockRouter.navigate('/foo/bar'); + // After navigation, the active span should be the navigation span + (core.getActiveSpan as any).mockReturnValue(mockNavigationSpan); callback(newState); - expect(mockPageloadSpan.updateName).toHaveBeenCalled(); - expect(mockPageloadSpan.setAttribute).toHaveBeenCalled(); + expect(mockNavigationSpan.updateName).toHaveBeenCalled(); + expect(mockNavigationSpan.setAttribute).toHaveBeenCalled(); + }); + + it('does not update navigation transaction on state change to loading', () => { + instrumentHydratedRouter(); + // Simulate a state change to loading (non-idle) + const callback = mockRouter.subscribe.mock.calls[0][0]; + const newState = { + location: { pathname: '/foo/bar' }, + matches: [{ route: { path: '/foo/:id' } }], + navigation: { state: 'loading' }, + }; + mockRouter.navigate('/foo/bar'); + // After navigation, the active span should be the navigation span + (core.getActiveSpan as any).mockReturnValue(mockNavigationSpan); + callback(newState); + expect(mockNavigationSpan.updateName).not.toHaveBeenCalled(); + expect(mockNavigationSpan.setAttribute).not.toHaveBeenCalled(); }); }); From 7576af6db56801035a0303c0e341ccceefe87d3f Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Mon, 5 May 2025 10:54:05 +0200 Subject: [PATCH 13/14] make andrei happy --- packages/react-router/src/client/hydratedRouter.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-router/src/client/hydratedRouter.ts b/packages/react-router/src/client/hydratedRouter.ts index 9733b5baea86..6e9ac0bfe91f 100644 --- a/packages/react-router/src/client/hydratedRouter.ts +++ b/packages/react-router/src/client/hydratedRouter.ts @@ -48,7 +48,7 @@ export function instrumentHydratedRouter(): void { // Patching navigate for creating accurate navigation transactions if (typeof router.navigate === 'function') { const originalNav = router.navigate.bind(router); - router.navigate = function patchedNavigate(...args) { + router.navigate = function sentryPatchedNavigate(...args) { maybeCreateNavigationTransaction( String(args[0]) || '', // will be updated anyway 'url', // this also will be updated once we have the parameterized route From a529211d294a42ce833e9f5c30048765ab6a6099 Mon Sep 17 00:00:00 2001 From: Charly Gomez Date: Mon, 5 May 2025 10:59:01 +0200 Subject: [PATCH 14/14] add log if instrumentation fails --- packages/react-router/src/client/hydratedRouter.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/react-router/src/client/hydratedRouter.ts b/packages/react-router/src/client/hydratedRouter.ts index 6e9ac0bfe91f..e5ec2d65d5ef 100644 --- a/packages/react-router/src/client/hydratedRouter.ts +++ b/packages/react-router/src/client/hydratedRouter.ts @@ -1,6 +1,7 @@ import { startBrowserTracingNavigationSpan } from '@sentry/browser'; import type { Span } from '@sentry/core'; import { + consoleSandbox, getActiveSpan, getClient, getRootSpan, @@ -11,6 +12,7 @@ import { spanToJSON, } from '@sentry/core'; import type { DataRouter, RouterState } from 'react-router'; +import { DEBUG_BUILD } from '../common/debug-build'; const GLOBAL_OBJ_WITH_DATA_ROUTER = GLOBAL_OBJ as typeof GLOBAL_OBJ & { __reactRouterDataRouter?: DataRouter; @@ -84,6 +86,13 @@ export function instrumentHydratedRouter(): void { // Retry until the router is available or max retries reached const interval = setInterval(() => { if (trySubscribe() || retryCount >= MAX_RETRIES) { + if (retryCount >= MAX_RETRIES) { + DEBUG_BUILD && + consoleSandbox(() => { + // eslint-disable-next-line no-console + console.warn('Unable to instrument React Router: router not found after hydration.'); + }); + } clearInterval(interval); } retryCount++;