From 4173352fbd0ae52cecea7187c05b85c00f731005 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 5 Feb 2024 10:42:57 +0100 Subject: [PATCH 1/4] feat(react): Add browserTracingIntegrations for router v4 & v5 This adds new `browserTracingReactRouterV4Integration()` and `browserTracingReactRouterV5Integration()` exports, deprecating these old routing instrumentations. I opted to leave as much as possible as-is for now, except for streamlining the attributes/tags we use for the instrumentation. --- .../react/src/browserTracingIntegration.ts | 86 ++++ packages/react/src/index.ts | 13 +- packages/react/src/reactrouter.tsx | 75 ++- packages/react/test/reactrouterv4.test.tsx | 428 +++++++++++++++-- packages/react/test/reactrouterv5.test.tsx | 434 ++++++++++++++++-- 5 files changed, 931 insertions(+), 105 deletions(-) create mode 100644 packages/react/src/browserTracingIntegration.ts diff --git a/packages/react/src/browserTracingIntegration.ts b/packages/react/src/browserTracingIntegration.ts new file mode 100644 index 000000000000..9ae21d3facd5 --- /dev/null +++ b/packages/react/src/browserTracingIntegration.ts @@ -0,0 +1,86 @@ +import { + browserTracingIntegration, + startBrowserTracingNavigationSpan, + startBrowserTracingPageLoadSpan, +} from '@sentry/browser'; +import type { Integration, StartSpanOptions } from '@sentry/types'; +import type { MatchPath, RouteConfig, RouterHistory } from './reactrouter'; +import { reactRouterV5Instrumentation } from './reactrouter'; +import { reactRouterV4Instrumentation } from './reactrouter'; + +interface ReactRouterOptions { + history: RouterHistory; + routes?: RouteConfig[]; + matchPath?: MatchPath; +} + +/** + * A browser tracing integration that uses React Router v4 to instrument navigations. + * Expects `history` (and optionally `routes` and `matchPath`) to be passed as options. + */ +export function browserTracingReactRouterV4Integration( + options: Parameters[0] & ReactRouterOptions, +): Integration { + const integration = browserTracingIntegration(options); + + const { history, routes, matchPath, instrumentPageLoad = true, instrumentNavigation = true } = options; + + return { + ...integration, + afterAllSetup(client) { + integration.afterAllSetup(client); + const startPageloadCallback = (startSpanOptions: StartSpanOptions): undefined => { + startBrowserTracingPageLoadSpan(client, startSpanOptions); + return undefined; + }; + + const startNavigationCallback = (startSpanOptions: StartSpanOptions): undefined => { + startBrowserTracingNavigationSpan(client, startSpanOptions); + return undefined; + }; + + // eslint-disable-next-line deprecation/deprecation + const instrumentation = reactRouterV4Instrumentation(history, routes, matchPath); + + // Now instrument page load & navigation with correct settings + instrumentation(startPageloadCallback, instrumentPageLoad, false); + instrumentation(startNavigationCallback, false, instrumentNavigation); + }, + }; +} + +/** + * A browser tracing integration that uses React Router v5 to instrument navigations. + * Expects `history` (and optionally `routes` and `matchPath`) to be passed as options. + */ +export function browserTracingReactRouterV5Integration( + options: Parameters[0] & ReactRouterOptions, +): Integration { + const integration = browserTracingIntegration(options); + + const { history, routes, matchPath } = options; + + return { + ...integration, + afterAllSetup(client) { + integration.afterAllSetup(client); + + const startPageloadCallback = (startSpanOptions: StartSpanOptions): undefined => { + startBrowserTracingPageLoadSpan(client, startSpanOptions); + return undefined; + }; + + const startNavigationCallback = (startSpanOptions: StartSpanOptions): undefined => { + startBrowserTracingNavigationSpan(client, startSpanOptions); + return undefined; + }; + + // eslint-disable-next-line deprecation/deprecation + const instrumentation = reactRouterV5Instrumentation(history, routes, matchPath); + + // Now instrument page load & navigation with correct settings + instrumentation(startPageloadCallback, options.instrumentPageLoad, false); + instrumentation(startNavigationCallback, false, options.instrumentNavigation); + }, + }; +} diff --git a/packages/react/src/index.ts b/packages/react/src/index.ts index ad66d1e77801..e867048353ec 100644 --- a/packages/react/src/index.ts +++ b/packages/react/src/index.ts @@ -6,10 +6,21 @@ export type { ErrorBoundaryProps, FallbackRender } from './errorboundary'; export { ErrorBoundary, withErrorBoundary } from './errorboundary'; export { createReduxEnhancer } from './redux'; export { reactRouterV3Instrumentation } from './reactrouterv3'; -export { reactRouterV4Instrumentation, reactRouterV5Instrumentation, withSentryRouting } from './reactrouter'; +export { + // eslint-disable-next-line deprecation/deprecation + reactRouterV4Instrumentation, + // eslint-disable-next-line deprecation/deprecation + reactRouterV5Instrumentation, + withSentryRouting, +} from './reactrouter'; export { reactRouterV6Instrumentation, withSentryReactRouterV6Routing, wrapUseRoutes, wrapCreateBrowserRouter, } from './reactrouterv6'; + +export { + browserTracingReactRouterV4Integration, + browserTracingReactRouterV5Integration, +} from './browserTracingIntegration'; diff --git a/packages/react/src/reactrouter.tsx b/packages/react/src/reactrouter.tsx index 04995ee4bc44..ef2fa4113ad2 100644 --- a/packages/react/src/reactrouter.tsx +++ b/packages/react/src/reactrouter.tsx @@ -1,6 +1,13 @@ import { WINDOW } from '@sentry/browser'; -import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core'; -import type { Transaction, TransactionSource } from '@sentry/types'; +import { + SEMANTIC_ATTRIBUTE_SENTRY_OP, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, + getActiveSpan, + getRootSpan, + spanToJSON, +} from '@sentry/core'; +import type { Span, Transaction, TransactionSource } from '@sentry/types'; import hoistNonReactStatics from 'hoist-non-react-statics'; import * as React from 'react'; @@ -23,29 +30,35 @@ export type RouteConfig = { routes?: RouteConfig[]; }; -type MatchPath = (pathname: string, props: string | string[] | any, parent?: Match | null) => Match | null; // eslint-disable-line @typescript-eslint/no-explicit-any +export type MatchPath = (pathname: string, props: string | string[] | any, parent?: Match | null) => Match | null; // eslint-disable-line @typescript-eslint/no-explicit-any let activeTransaction: Transaction | undefined; +/** + * @deprecated Use `browserTracingReactRouterV4()` instead. + */ export function reactRouterV4Instrumentation( history: RouterHistory, routes?: RouteConfig[], matchPath?: MatchPath, ): ReactRouterInstrumentation { - return createReactRouterInstrumentation(history, 'react-router-v4', routes, matchPath); + return createReactRouterInstrumentation(history, 'reactrouter_v4', routes, matchPath); } +/** + * @deprecated Use `browserTracingReactRouterV5()` instead. + */ export function reactRouterV5Instrumentation( history: RouterHistory, routes?: RouteConfig[], matchPath?: MatchPath, ): ReactRouterInstrumentation { - return createReactRouterInstrumentation(history, 'react-router-v5', routes, matchPath); + return createReactRouterInstrumentation(history, 'reactrouter_v5', routes, matchPath); } function createReactRouterInstrumentation( history: RouterHistory, - name: string, + instrumentationName: string, allRoutes: RouteConfig[] = [], matchPath?: MatchPath, ): ReactRouterInstrumentation { @@ -83,21 +96,17 @@ function createReactRouterInstrumentation( return [pathname, 'url']; } - const tags = { - 'routing.instrumentation': name, - }; - return (customStartTransaction, startTransactionOnPageLoad = true, startTransactionOnLocationChange = true): void => { const initPathName = getInitPathName(); + if (startTransactionOnPageLoad && initPathName) { const [name, source] = normalizeTransactionName(initPathName); activeTransaction = customStartTransaction({ name, - op: 'pageload', - origin: 'auto.pageload.react.reactrouter', - tags, - metadata: { - source, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: `auto.pageload.react.${instrumentationName}`, + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source, }, }); } @@ -112,11 +121,10 @@ function createReactRouterInstrumentation( const [name, source] = normalizeTransactionName(location.pathname); activeTransaction = customStartTransaction({ name, - op: 'navigation', - origin: 'auto.navigation.react.reactrouter', - tags, - metadata: { - source, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: `auto.navigation.react.${instrumentationName}`, + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source, }, }); } @@ -164,10 +172,12 @@ function computeRootMatch(pathname: string): Match { export function withSentryRouting

, R extends React.ComponentType

>(Route: R): R { const componentDisplayName = (Route as any).displayName || (Route as any).name; + const activeRootSpan = getActiveRootSpan(); + const WrappedRoute: React.FC

= (props: P) => { - if (activeTransaction && props && props.computedMatch && props.computedMatch.isExact) { - activeTransaction.updateName(props.computedMatch.path); - activeTransaction.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route'); + if (activeRootSpan && props && props.computedMatch && props.computedMatch.isExact) { + activeRootSpan.updateName(props.computedMatch.path); + activeRootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route'); } // @ts-expect-error Setting more specific React Component typing for `R` generic above @@ -184,3 +194,22 @@ export function withSentryRouting

, R extends React return WrappedRoute; } /* eslint-enable @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access */ + +function getActiveRootSpan(): Span | undefined { + // Legacy behavior for "old" react router instrumentation + if (activeTransaction) { + return activeTransaction; + } + + const span = getActiveSpan(); + const rootSpan = span ? getRootSpan(span) : undefined; + + if (!rootSpan) { + return undefined; + } + + 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; +} diff --git a/packages/react/test/reactrouterv4.test.tsx b/packages/react/test/reactrouterv4.test.tsx index 5849bb688598..6b6dc9f73c7b 100644 --- a/packages/react/test/reactrouterv4.test.tsx +++ b/packages/react/test/reactrouterv4.test.tsx @@ -1,14 +1,26 @@ -import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core'; +import { + SEMANTIC_ATTRIBUTE_SENTRY_OP, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, + createTransport, + getCurrentScope, + setCurrentClient, +} from '@sentry/core'; import { act, render } from '@testing-library/react'; import { createMemoryHistory } from 'history-4'; // biome-ignore lint/nursery/noUnusedImports: Need React import for JSX import * as React from 'react'; import { Route, Router, Switch, matchPath } from 'react-router-4'; -import { reactRouterV4Instrumentation, withSentryRouting } from '../src'; +import { + BrowserClient, + browserTracingReactRouterV4Integration, + reactRouterV4Instrumentation, + withSentryRouting, +} from '../src'; import type { RouteConfig } from '../src/reactrouter'; -describe('React Router v4', () => { +describe('reactRouterV4Instrumentation', () => { function createInstrumentation(_opts?: { startTransactionOnPageLoad?: boolean; startTransactionOnLocationChange?: boolean; @@ -28,6 +40,7 @@ describe('React Router v4', () => { const mockStartTransaction = jest .fn() .mockReturnValue({ updateName: mockUpdateName, end: mockFinish, setAttribute: mockSetAttribute }); + // eslint-disable-next-line deprecation/deprecation reactRouterV4Instrumentation(history, options.routes, options.matchPath)( mockStartTransaction, options.startTransactionOnPageLoad, @@ -41,10 +54,11 @@ describe('React Router v4', () => { expect(mockStartTransaction).toHaveBeenCalledTimes(1); expect(mockStartTransaction).toHaveBeenLastCalledWith({ name: '/', - op: 'pageload', - origin: 'auto.pageload.react.reactrouter', - tags: { 'routing.instrumentation': 'react-router-v4' }, - metadata: { source: 'url' }, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.react.reactrouter_v4', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', + }, }); }); @@ -71,10 +85,11 @@ describe('React Router v4', () => { expect(mockStartTransaction).toHaveBeenCalledTimes(2); expect(mockStartTransaction).toHaveBeenLastCalledWith({ name: '/about', - op: 'navigation', - origin: 'auto.navigation.react.reactrouter', - tags: { 'routing.instrumentation': 'react-router-v4' }, - metadata: { source: 'url' }, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v4', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + }, }); act(() => { @@ -83,10 +98,11 @@ describe('React Router v4', () => { expect(mockStartTransaction).toHaveBeenCalledTimes(3); expect(mockStartTransaction).toHaveBeenLastCalledWith({ name: '/features', - op: 'navigation', - origin: 'auto.navigation.react.reactrouter', - tags: { 'routing.instrumentation': 'react-router-v4' }, - metadata: { source: 'url' }, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v4', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + }, }); }); @@ -162,10 +178,11 @@ describe('React Router v4', () => { expect(mockStartTransaction).toHaveBeenCalledTimes(2); expect(mockStartTransaction).toHaveBeenLastCalledWith({ name: '/users/123', - op: 'navigation', - origin: 'auto.navigation.react.reactrouter', - tags: { 'routing.instrumentation': 'react-router-v4' }, - metadata: { source: 'url' }, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v4', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + }, }); }); @@ -190,10 +207,11 @@ describe('React Router v4', () => { expect(mockStartTransaction).toHaveBeenCalledTimes(2); expect(mockStartTransaction).toHaveBeenLastCalledWith({ name: '/users/123', - op: 'navigation', - origin: 'auto.navigation.react.reactrouter', - tags: { 'routing.instrumentation': 'react-router-v4' }, - metadata: { source: 'url' }, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v4', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + }, }); expect(mockUpdateName).toHaveBeenCalledTimes(2); expect(mockUpdateName).toHaveBeenLastCalledWith('/users/:userid'); @@ -221,10 +239,11 @@ describe('React Router v4', () => { expect(mockStartTransaction).toHaveBeenCalledTimes(2); expect(mockStartTransaction).toHaveBeenLastCalledWith({ name: '/organizations/1234/v1/758', - op: 'navigation', - origin: 'auto.navigation.react.reactrouter', - tags: { 'routing.instrumentation': 'react-router-v4' }, - metadata: { source: 'url' }, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v4', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + }, }); expect(mockUpdateName).toHaveBeenCalledTimes(2); expect(mockUpdateName).toHaveBeenLastCalledWith('/organizations/:orgid/v1/:teamid'); @@ -238,10 +257,11 @@ describe('React Router v4', () => { expect(mockStartTransaction).toHaveBeenCalledTimes(3); expect(mockStartTransaction).toHaveBeenLastCalledWith({ name: '/organizations/543', - op: 'navigation', - origin: 'auto.navigation.react.reactrouter', - tags: { 'routing.instrumentation': 'react-router-v4' }, - metadata: { source: 'url' }, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v4', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + }, }); expect(mockUpdateName).toHaveBeenCalledTimes(3); expect(mockUpdateName).toHaveBeenLastCalledWith('/organizations/:orgid'); @@ -273,10 +293,11 @@ describe('React Router v4', () => { expect(mockStartTransaction).toHaveBeenCalledTimes(2); expect(mockStartTransaction).toHaveBeenLastCalledWith({ name: '/organizations/:orgid/v1/:teamid', - op: 'navigation', - origin: 'auto.navigation.react.reactrouter', - tags: { 'routing.instrumentation': 'react-router-v4' }, - metadata: { source: 'route' }, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v4', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + }, }); act(() => { @@ -285,10 +306,339 @@ describe('React Router v4', () => { expect(mockStartTransaction).toHaveBeenCalledTimes(3); expect(mockStartTransaction).toHaveBeenLastCalledWith({ name: '/organizations/:orgid', - op: 'navigation', - origin: 'auto.navigation.react.reactrouter', - tags: { 'routing.instrumentation': 'react-router-v4' }, - metadata: { source: 'route' }, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v4', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + }, + }); + }); +}); + +const mockStartBrowserTracingPageLoadSpan = jest.fn(); +const mockStartBrowserTracingNavigationSpan = jest.fn(); + +const mockRootSpan = { + updateName: jest.fn(), + setAttribute: jest.fn(), + getSpanJSON() { + return { op: 'pageload' }; + }, +}; + +jest.mock('@sentry/browser', () => { + const actual = jest.requireActual('@sentry/browser'); + return { + ...actual, + startBrowserTracingNavigationSpan: (...args: unknown[]) => { + mockStartBrowserTracingNavigationSpan(...args); + return actual.startBrowserTracingNavigationSpan(...args); + }, + startBrowserTracingPageLoadSpan: (...args: unknown[]) => { + mockStartBrowserTracingPageLoadSpan(...args); + return actual.startBrowserTracingPageLoadSpan(...args); + }, + }; +}); + +jest.mock('@sentry/core', () => { + const actual = jest.requireActual('@sentry/core'); + return { + ...actual, + getRootSpan: () => { + return mockRootSpan; + }, + }; +}); + +describe('browserTracingReactRouterV4', () => { + function createMockBrowserClient(): BrowserClient { + return new BrowserClient({ + integrations: [], + transport: () => createTransport({ recordDroppedEvent: () => undefined }, _ => Promise.resolve({})), + stackParser: () => [], + debug: true, + }); + } + + beforeEach(() => { + jest.clearAllMocks(); + getCurrentScope().setClient(undefined); + }); + + it('starts a pageload transaction when instrumentation is started', () => { + const client = createMockBrowserClient(); + setCurrentClient(client); + + const history = createMemoryHistory(); + client.addIntegration(browserTracingReactRouterV4Integration({ history })); + + client.init(); + + expect(mockStartBrowserTracingPageLoadSpan).toHaveBeenCalledTimes(1); + expect(mockStartBrowserTracingPageLoadSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), { + name: '/', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.react.reactrouter_v4', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', + }, + }); + }); + + it('starts a navigation transaction', () => { + const client = createMockBrowserClient(); + setCurrentClient(client); + + const history = createMemoryHistory(); + client.addIntegration(browserTracingReactRouterV4Integration({ history })); + + client.init(); + + render( + + +

Features
} /> +
About
} /> +
Home
} /> + + , + ); + + act(() => { + history.push('/about'); + }); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), { + name: '/about', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v4', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + }, + }); + + act(() => { + history.push('/features'); + }); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(2); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), { + name: '/features', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v4', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + }, + }); + }); + + it('only starts a navigation transaction on push', () => { + const client = createMockBrowserClient(); + setCurrentClient(client); + + const history = createMemoryHistory(); + client.addIntegration(browserTracingReactRouterV4Integration({ history })); + + client.init(); + + render( + + +
Features
} /> +
About
} /> +
Home
} /> + + , + ); + + act(() => { + history.replace('hello'); + }); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(0); + }); + + it('does not normalize transaction name ', () => { + const client = createMockBrowserClient(); + + const history = createMemoryHistory(); + client.addIntegration(browserTracingReactRouterV4Integration({ history })); + + client.init(); + + const { getByText } = render( + + +
UserId
} /> +
Users
} /> +
Home
} /> + + , + ); + + act(() => { + history.push('/users/123'); + }); + getByText('UserId'); + + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), { + name: '/users/123', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v4', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + }, + }); + }); + + it('normalizes transaction name with custom Route', () => { + const client = createMockBrowserClient(); + setCurrentClient(client); + + const history = createMemoryHistory(); + client.addIntegration(browserTracingReactRouterV4Integration({ history })); + + client.init(); + + const SentryRoute = withSentryRouting(Route); + + const { getByText } = render( + + +
UserId
} /> +
Users
} /> +
Home
} /> + + , + ); + + act(() => { + history.push('/users/123'); + }); + getByText('UserId'); + + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), { + name: '/users/123', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v4', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + }, + }); + expect(mockRootSpan.updateName).toHaveBeenCalledTimes(2); + expect(mockRootSpan.updateName).toHaveBeenLastCalledWith('/users/:userid'); + expect(mockRootSpan.setAttribute).toHaveBeenCalledWith(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route'); + }); + + it('normalizes nested transaction names with custom Route', () => { + const client = createMockBrowserClient(); + setCurrentClient(client); + + const history = createMemoryHistory(); + client.addIntegration(browserTracingReactRouterV4Integration({ history })); + + client.init(); + + const SentryRoute = withSentryRouting(Route); + + const { getByText } = render( + + +
Team
} /> +
OrgId
} /> +
Home
} /> + + , + ); + + act(() => { + history.push('/organizations/1234/v1/758'); + }); + getByText('Team'); + + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), { + name: '/organizations/1234/v1/758', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v4', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + }, + }); + expect(mockRootSpan.updateName).toHaveBeenCalledTimes(2); + expect(mockRootSpan.updateName).toHaveBeenLastCalledWith('/organizations/:orgid/v1/:teamid'); + expect(mockRootSpan.setAttribute).toHaveBeenLastCalledWith(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route'); + + act(() => { + history.push('/organizations/543'); + }); + getByText('OrgId'); + + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(2); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), { + name: '/organizations/543', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v4', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + }, + }); + expect(mockRootSpan.updateName).toHaveBeenCalledTimes(3); + expect(mockRootSpan.updateName).toHaveBeenLastCalledWith('/organizations/:orgid'); + expect(mockRootSpan.setAttribute).toHaveBeenLastCalledWith(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route'); + }); + + it('matches with route object', () => { + const routes: RouteConfig[] = [ + { + path: '/organizations/:orgid/v1/:teamid', + }, + { path: '/organizations/:orgid' }, + { path: '/' }, + ]; + const client = createMockBrowserClient(); + setCurrentClient(client); + + const history = createMemoryHistory(); + client.addIntegration(browserTracingReactRouterV4Integration({ history, routes, matchPath })); + + client.init(); + + render( + + +
Team
} /> +
OrgId
} /> +
Home
} /> + + , + ); + + act(() => { + history.push('/organizations/1234/v1/758'); + }); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), { + name: '/organizations/:orgid/v1/:teamid', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v4', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + }, + }); + + act(() => { + history.push('/organizations/1234'); + }); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(2); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), { + name: '/organizations/:orgid', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v4', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + }, }); }); }); diff --git a/packages/react/test/reactrouterv5.test.tsx b/packages/react/test/reactrouterv5.test.tsx index c571b3590b8f..4af35943ce3e 100644 --- a/packages/react/test/reactrouterv5.test.tsx +++ b/packages/react/test/reactrouterv5.test.tsx @@ -1,14 +1,26 @@ -import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core'; +import { + SEMANTIC_ATTRIBUTE_SENTRY_OP, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, + createTransport, + getCurrentScope, + setCurrentClient, +} from '@sentry/core'; import { act, render } from '@testing-library/react'; import { createMemoryHistory } from 'history-4'; // biome-ignore lint/nursery/noUnusedImports: Need React import for JSX import * as React from 'react'; import { Route, Router, Switch, matchPath } from 'react-router-5'; -import { reactRouterV5Instrumentation, withSentryRouting } from '../src'; +import { + BrowserClient, + browserTracingReactRouterV5Integration, + reactRouterV5Instrumentation, + withSentryRouting, +} from '../src'; import type { RouteConfig } from '../src/reactrouter'; -describe('React Router v5', () => { +describe('reactRouterV5Instrumentation', () => { function createInstrumentation(_opts?: { startTransactionOnPageLoad?: boolean; startTransactionOnLocationChange?: boolean; @@ -28,6 +40,7 @@ describe('React Router v5', () => { const mockStartTransaction = jest .fn() .mockReturnValue({ updateName: mockUpdateName, end: mockFinish, setAttribute: mockSetAttribute }); + // eslint-disable-next-line deprecation/deprecation reactRouterV5Instrumentation(history, options.routes, options.matchPath)( mockStartTransaction, options.startTransactionOnPageLoad, @@ -41,10 +54,11 @@ describe('React Router v5', () => { expect(mockStartTransaction).toHaveBeenCalledTimes(1); expect(mockStartTransaction).toHaveBeenLastCalledWith({ name: '/', - op: 'pageload', - origin: 'auto.pageload.react.reactrouter', - tags: { 'routing.instrumentation': 'react-router-v5' }, - metadata: { source: 'url' }, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.react.reactrouter_v5', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', + }, }); }); @@ -71,10 +85,11 @@ describe('React Router v5', () => { expect(mockStartTransaction).toHaveBeenCalledTimes(2); expect(mockStartTransaction).toHaveBeenLastCalledWith({ name: '/about', - op: 'navigation', - origin: 'auto.navigation.react.reactrouter', - tags: { 'routing.instrumentation': 'react-router-v5' }, - metadata: { source: 'url' }, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v5', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + }, }); act(() => { @@ -83,10 +98,11 @@ describe('React Router v5', () => { expect(mockStartTransaction).toHaveBeenCalledTimes(3); expect(mockStartTransaction).toHaveBeenLastCalledWith({ name: '/features', - op: 'navigation', - origin: 'auto.navigation.react.reactrouter', - tags: { 'routing.instrumentation': 'react-router-v5' }, - metadata: { source: 'url' }, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v5', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + }, }); }); @@ -162,17 +178,17 @@ describe('React Router v5', () => { expect(mockStartTransaction).toHaveBeenCalledTimes(2); expect(mockStartTransaction).toHaveBeenLastCalledWith({ name: '/users/123', - op: 'navigation', - origin: 'auto.navigation.react.reactrouter', - tags: { 'routing.instrumentation': 'react-router-v5' }, - metadata: { source: 'url' }, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v5', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + }, }); }); it('normalizes transaction name with custom Route', () => { const [mockStartTransaction, history, { mockUpdateName, mockSetAttribute }] = createInstrumentation(); const SentryRoute = withSentryRouting(Route); - const { getByText } = render( @@ -182,6 +198,7 @@ describe('React Router v5', () => { , ); + act(() => { history.push('/users/123'); }); @@ -190,20 +207,20 @@ describe('React Router v5', () => { expect(mockStartTransaction).toHaveBeenCalledTimes(2); expect(mockStartTransaction).toHaveBeenLastCalledWith({ name: '/users/123', - op: 'navigation', - origin: 'auto.navigation.react.reactrouter', - tags: { 'routing.instrumentation': 'react-router-v5' }, - metadata: { source: 'url' }, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v5', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + }, }); expect(mockUpdateName).toHaveBeenCalledTimes(2); expect(mockUpdateName).toHaveBeenLastCalledWith('/users/:userid'); - expect(mockSetAttribute).toHaveBeenLastCalledWith(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route'); + expect(mockSetAttribute).toHaveBeenCalledWith(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route'); }); it('normalizes nested transaction names with custom Route', () => { const [mockStartTransaction, history, { mockUpdateName, mockSetAttribute }] = createInstrumentation(); const SentryRoute = withSentryRouting(Route); - const { getByText } = render( @@ -222,10 +239,11 @@ describe('React Router v5', () => { expect(mockStartTransaction).toHaveBeenCalledTimes(2); expect(mockStartTransaction).toHaveBeenLastCalledWith({ name: '/organizations/1234/v1/758', - op: 'navigation', - origin: 'auto.navigation.react.reactrouter', - tags: { 'routing.instrumentation': 'react-router-v5' }, - metadata: { source: 'url' }, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v5', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + }, }); expect(mockUpdateName).toHaveBeenCalledTimes(2); expect(mockUpdateName).toHaveBeenLastCalledWith('/organizations/:orgid/v1/:teamid'); @@ -239,13 +257,15 @@ describe('React Router v5', () => { expect(mockStartTransaction).toHaveBeenCalledTimes(3); expect(mockStartTransaction).toHaveBeenLastCalledWith({ name: '/organizations/543', - op: 'navigation', - origin: 'auto.navigation.react.reactrouter', - tags: { 'routing.instrumentation': 'react-router-v5' }, - metadata: { source: 'url' }, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v5', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + }, }); expect(mockUpdateName).toHaveBeenCalledTimes(3); expect(mockUpdateName).toHaveBeenLastCalledWith('/organizations/:orgid'); + expect(mockSetAttribute).toHaveBeenLastCalledWith(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route'); }); it('matches with route object', () => { @@ -273,10 +293,11 @@ describe('React Router v5', () => { expect(mockStartTransaction).toHaveBeenCalledTimes(2); expect(mockStartTransaction).toHaveBeenLastCalledWith({ name: '/organizations/:orgid/v1/:teamid', - op: 'navigation', - origin: 'auto.navigation.react.reactrouter', - tags: { 'routing.instrumentation': 'react-router-v5' }, - metadata: { source: 'route' }, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v5', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + }, }); act(() => { @@ -285,10 +306,339 @@ describe('React Router v5', () => { expect(mockStartTransaction).toHaveBeenCalledTimes(3); expect(mockStartTransaction).toHaveBeenLastCalledWith({ name: '/organizations/:orgid', - op: 'navigation', - origin: 'auto.navigation.react.reactrouter', - tags: { 'routing.instrumentation': 'react-router-v5' }, - metadata: { source: 'route' }, + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v5', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + }, + }); + }); +}); + +const mockStartBrowserTracingPageLoadSpan = jest.fn(); +const mockStartBrowserTracingNavigationSpan = jest.fn(); + +const mockRootSpan = { + updateName: jest.fn(), + setAttribute: jest.fn(), + getSpanJSON() { + return { op: 'pageload' }; + }, +}; + +jest.mock('@sentry/browser', () => { + const actual = jest.requireActual('@sentry/browser'); + return { + ...actual, + startBrowserTracingNavigationSpan: (...args: unknown[]) => { + mockStartBrowserTracingNavigationSpan(...args); + return actual.startBrowserTracingNavigationSpan(...args); + }, + startBrowserTracingPageLoadSpan: (...args: unknown[]) => { + mockStartBrowserTracingPageLoadSpan(...args); + return actual.startBrowserTracingPageLoadSpan(...args); + }, + }; +}); + +jest.mock('@sentry/core', () => { + const actual = jest.requireActual('@sentry/core'); + return { + ...actual, + getRootSpan: () => { + return mockRootSpan; + }, + }; +}); + +describe('browserTracingReactRouterV5', () => { + function createMockBrowserClient(): BrowserClient { + return new BrowserClient({ + integrations: [], + transport: () => createTransport({ recordDroppedEvent: () => undefined }, _ => Promise.resolve({})), + stackParser: () => [], + debug: true, + }); + } + + beforeEach(() => { + jest.clearAllMocks(); + getCurrentScope().setClient(undefined); + }); + + it('starts a pageload transaction when instrumentation is started', () => { + const client = createMockBrowserClient(); + setCurrentClient(client); + + const history = createMemoryHistory(); + client.addIntegration(browserTracingReactRouterV5Integration({ history })); + + client.init(); + + expect(mockStartBrowserTracingPageLoadSpan).toHaveBeenCalledTimes(1); + expect(mockStartBrowserTracingPageLoadSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), { + name: '/', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.react.reactrouter_v5', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', + }, + }); + }); + + it('starts a navigation transaction', () => { + const client = createMockBrowserClient(); + setCurrentClient(client); + + const history = createMemoryHistory(); + client.addIntegration(browserTracingReactRouterV5Integration({ history })); + + client.init(); + + render( + + +
Features
} /> +
About
} /> +
Home
} /> + + , + ); + + act(() => { + history.push('/about'); + }); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), { + name: '/about', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v5', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + }, + }); + + act(() => { + history.push('/features'); + }); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(2); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), { + name: '/features', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v5', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + }, + }); + }); + + it('only starts a navigation transaction on push', () => { + const client = createMockBrowserClient(); + setCurrentClient(client); + + const history = createMemoryHistory(); + client.addIntegration(browserTracingReactRouterV5Integration({ history })); + + client.init(); + + render( + + +
Features
} /> +
About
} /> +
Home
} /> + + , + ); + + act(() => { + history.replace('hello'); + }); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(0); + }); + + it('does not normalize transaction name ', () => { + const client = createMockBrowserClient(); + + const history = createMemoryHistory(); + client.addIntegration(browserTracingReactRouterV5Integration({ history })); + + client.init(); + + const { getByText } = render( + + +
UserId
} /> +
Users
} /> +
Home
} /> + + , + ); + + act(() => { + history.push('/users/123'); + }); + getByText('UserId'); + + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), { + name: '/users/123', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v5', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + }, + }); + }); + + it('normalizes transaction name with custom Route', () => { + const client = createMockBrowserClient(); + setCurrentClient(client); + + const history = createMemoryHistory(); + client.addIntegration(browserTracingReactRouterV5Integration({ history })); + + client.init(); + + const SentryRoute = withSentryRouting(Route); + + const { getByText } = render( + + +
UserId
} /> +
Users
} /> +
Home
} /> + + , + ); + + act(() => { + history.push('/users/123'); + }); + getByText('UserId'); + + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), { + name: '/users/123', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v5', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + }, + }); + expect(mockRootSpan.updateName).toHaveBeenCalledTimes(2); + expect(mockRootSpan.updateName).toHaveBeenLastCalledWith('/users/:userid'); + expect(mockRootSpan.setAttribute).toHaveBeenCalledWith(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route'); + }); + + it('normalizes nested transaction names with custom Route', () => { + const client = createMockBrowserClient(); + setCurrentClient(client); + + const history = createMemoryHistory(); + client.addIntegration(browserTracingReactRouterV5Integration({ history })); + + client.init(); + + const SentryRoute = withSentryRouting(Route); + + const { getByText } = render( + + +
Team
} /> +
OrgId
} /> +
Home
} /> + + , + ); + + act(() => { + history.push('/organizations/1234/v1/758'); + }); + getByText('Team'); + + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), { + name: '/organizations/1234/v1/758', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v5', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + }, + }); + expect(mockRootSpan.updateName).toHaveBeenCalledTimes(2); + expect(mockRootSpan.updateName).toHaveBeenLastCalledWith('/organizations/:orgid/v1/:teamid'); + expect(mockRootSpan.setAttribute).toHaveBeenLastCalledWith(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route'); + + act(() => { + history.push('/organizations/543'); + }); + getByText('OrgId'); + + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(2); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), { + name: '/organizations/543', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v5', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + }, + }); + expect(mockRootSpan.updateName).toHaveBeenCalledTimes(3); + expect(mockRootSpan.updateName).toHaveBeenLastCalledWith('/organizations/:orgid'); + expect(mockRootSpan.setAttribute).toHaveBeenLastCalledWith(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route'); + }); + + it('matches with route object', () => { + const routes: RouteConfig[] = [ + { + path: '/organizations/:orgid/v1/:teamid', + }, + { path: '/organizations/:orgid' }, + { path: '/' }, + ]; + const client = createMockBrowserClient(); + setCurrentClient(client); + + const history = createMemoryHistory(); + client.addIntegration(browserTracingReactRouterV5Integration({ history, routes, matchPath })); + + client.init(); + + render( + + +
Team
} /> +
OrgId
} /> +
Home
} /> + + , + ); + + act(() => { + history.push('/organizations/1234/v1/758'); + }); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), { + name: '/organizations/:orgid/v1/:teamid', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v5', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + }, + }); + + act(() => { + history.push('/organizations/1234'); + }); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(2); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), { + name: '/organizations/:orgid', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v5', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + }, }); }); }); From a19e904d1e0e600a102c5d1c47630184670852a7 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 5 Feb 2024 10:45:15 +0100 Subject: [PATCH 2/4] fix defaults --- packages/react/src/browserTracingIntegration.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/packages/react/src/browserTracingIntegration.ts b/packages/react/src/browserTracingIntegration.ts index 9ae21d3facd5..2ae819a5aaf8 100644 --- a/packages/react/src/browserTracingIntegration.ts +++ b/packages/react/src/browserTracingIntegration.ts @@ -21,7 +21,11 @@ interface ReactRouterOptions { export function browserTracingReactRouterV4Integration( options: Parameters[0] & ReactRouterOptions, ): Integration { - const integration = browserTracingIntegration(options); + const integration = browserTracingIntegration({ + ...options, + instrumentPageLoad: false, + instrumentNavigation: false, + }); const { history, routes, matchPath, instrumentPageLoad = true, instrumentNavigation = true } = options; @@ -29,6 +33,7 @@ export function browserTracingReactRouterV4Integration( ...integration, afterAllSetup(client) { integration.afterAllSetup(client); + const startPageloadCallback = (startSpanOptions: StartSpanOptions): undefined => { startBrowserTracingPageLoadSpan(client, startSpanOptions); return undefined; @@ -56,7 +61,11 @@ export function browserTracingReactRouterV4Integration( export function browserTracingReactRouterV5Integration( options: Parameters[0] & ReactRouterOptions, ): Integration { - const integration = browserTracingIntegration(options); + const integration = browserTracingIntegration({ + ...options, + instrumentPageLoad: false, + instrumentNavigation: false, + }); const { history, routes, matchPath } = options; From 7d156d7aac7a1beba5805ac654c6c3bb8f800358 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 5 Feb 2024 10:48:17 +0100 Subject: [PATCH 3/4] export from router package --- .../react/src/browserTracingIntegration.ts | 95 ------------------- packages/react/src/index.ts | 7 +- packages/react/src/reactrouter.tsx | 95 ++++++++++++++++++- 3 files changed, 95 insertions(+), 102 deletions(-) delete mode 100644 packages/react/src/browserTracingIntegration.ts diff --git a/packages/react/src/browserTracingIntegration.ts b/packages/react/src/browserTracingIntegration.ts deleted file mode 100644 index 2ae819a5aaf8..000000000000 --- a/packages/react/src/browserTracingIntegration.ts +++ /dev/null @@ -1,95 +0,0 @@ -import { - browserTracingIntegration, - startBrowserTracingNavigationSpan, - startBrowserTracingPageLoadSpan, -} from '@sentry/browser'; -import type { Integration, StartSpanOptions } from '@sentry/types'; -import type { MatchPath, RouteConfig, RouterHistory } from './reactrouter'; -import { reactRouterV5Instrumentation } from './reactrouter'; -import { reactRouterV4Instrumentation } from './reactrouter'; - -interface ReactRouterOptions { - history: RouterHistory; - routes?: RouteConfig[]; - matchPath?: MatchPath; -} - -/** - * A browser tracing integration that uses React Router v4 to instrument navigations. - * Expects `history` (and optionally `routes` and `matchPath`) to be passed as options. - */ -export function browserTracingReactRouterV4Integration( - options: Parameters[0] & ReactRouterOptions, -): Integration { - const integration = browserTracingIntegration({ - ...options, - instrumentPageLoad: false, - instrumentNavigation: false, - }); - - const { history, routes, matchPath, instrumentPageLoad = true, instrumentNavigation = true } = options; - - return { - ...integration, - afterAllSetup(client) { - integration.afterAllSetup(client); - - const startPageloadCallback = (startSpanOptions: StartSpanOptions): undefined => { - startBrowserTracingPageLoadSpan(client, startSpanOptions); - return undefined; - }; - - const startNavigationCallback = (startSpanOptions: StartSpanOptions): undefined => { - startBrowserTracingNavigationSpan(client, startSpanOptions); - return undefined; - }; - - // eslint-disable-next-line deprecation/deprecation - const instrumentation = reactRouterV4Instrumentation(history, routes, matchPath); - - // Now instrument page load & navigation with correct settings - instrumentation(startPageloadCallback, instrumentPageLoad, false); - instrumentation(startNavigationCallback, false, instrumentNavigation); - }, - }; -} - -/** - * A browser tracing integration that uses React Router v5 to instrument navigations. - * Expects `history` (and optionally `routes` and `matchPath`) to be passed as options. - */ -export function browserTracingReactRouterV5Integration( - options: Parameters[0] & ReactRouterOptions, -): Integration { - const integration = browserTracingIntegration({ - ...options, - instrumentPageLoad: false, - instrumentNavigation: false, - }); - - const { history, routes, matchPath } = options; - - return { - ...integration, - afterAllSetup(client) { - integration.afterAllSetup(client); - - const startPageloadCallback = (startSpanOptions: StartSpanOptions): undefined => { - startBrowserTracingPageLoadSpan(client, startSpanOptions); - return undefined; - }; - - const startNavigationCallback = (startSpanOptions: StartSpanOptions): undefined => { - startBrowserTracingNavigationSpan(client, startSpanOptions); - return undefined; - }; - - // eslint-disable-next-line deprecation/deprecation - const instrumentation = reactRouterV5Instrumentation(history, routes, matchPath); - - // Now instrument page load & navigation with correct settings - instrumentation(startPageloadCallback, options.instrumentPageLoad, false); - instrumentation(startNavigationCallback, false, options.instrumentNavigation); - }, - }; -} diff --git a/packages/react/src/index.ts b/packages/react/src/index.ts index e867048353ec..573edfe63ed0 100644 --- a/packages/react/src/index.ts +++ b/packages/react/src/index.ts @@ -12,6 +12,8 @@ export { // eslint-disable-next-line deprecation/deprecation reactRouterV5Instrumentation, withSentryRouting, + browserTracingReactRouterV4Integration, + browserTracingReactRouterV5Integration, } from './reactrouter'; export { reactRouterV6Instrumentation, @@ -19,8 +21,3 @@ export { wrapUseRoutes, wrapCreateBrowserRouter, } from './reactrouterv6'; - -export { - browserTracingReactRouterV4Integration, - browserTracingReactRouterV5Integration, -} from './browserTracingIntegration'; diff --git a/packages/react/src/reactrouter.tsx b/packages/react/src/reactrouter.tsx index ef2fa4113ad2..a8b60eac75ad 100644 --- a/packages/react/src/reactrouter.tsx +++ b/packages/react/src/reactrouter.tsx @@ -1,4 +1,9 @@ -import { WINDOW } from '@sentry/browser'; +import { + WINDOW, + browserTracingIntegration, + startBrowserTracingNavigationSpan, + startBrowserTracingPageLoadSpan, +} from '@sentry/browser'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, @@ -7,7 +12,7 @@ import { getRootSpan, spanToJSON, } from '@sentry/core'; -import type { Span, Transaction, TransactionSource } from '@sentry/types'; +import type { Integration, Span, StartSpanOptions, Transaction, TransactionSource } from '@sentry/types'; import hoistNonReactStatics from 'hoist-non-react-statics'; import * as React from 'react'; @@ -32,8 +37,94 @@ export type RouteConfig = { export type MatchPath = (pathname: string, props: string | string[] | any, parent?: Match | null) => Match | null; // eslint-disable-line @typescript-eslint/no-explicit-any +interface ReactRouterOptions { + history: RouterHistory; + routes?: RouteConfig[]; + matchPath?: MatchPath; +} + let activeTransaction: Transaction | undefined; +/** + * A browser tracing integration that uses React Router v4 to instrument navigations. + * Expects `history` (and optionally `routes` and `matchPath`) to be passed as options. + */ +export function browserTracingReactRouterV4Integration( + options: Parameters[0] & ReactRouterOptions, +): Integration { + const integration = browserTracingIntegration({ + ...options, + instrumentPageLoad: false, + instrumentNavigation: false, + }); + + const { history, routes, matchPath, instrumentPageLoad = true, instrumentNavigation = true } = options; + + return { + ...integration, + afterAllSetup(client) { + integration.afterAllSetup(client); + + const startPageloadCallback = (startSpanOptions: StartSpanOptions): undefined => { + startBrowserTracingPageLoadSpan(client, startSpanOptions); + return undefined; + }; + + const startNavigationCallback = (startSpanOptions: StartSpanOptions): undefined => { + startBrowserTracingNavigationSpan(client, startSpanOptions); + return undefined; + }; + + // eslint-disable-next-line deprecation/deprecation + const instrumentation = reactRouterV4Instrumentation(history, routes, matchPath); + + // Now instrument page load & navigation with correct settings + instrumentation(startPageloadCallback, instrumentPageLoad, false); + instrumentation(startNavigationCallback, false, instrumentNavigation); + }, + }; +} + +/** + * A browser tracing integration that uses React Router v5 to instrument navigations. + * Expects `history` (and optionally `routes` and `matchPath`) to be passed as options. + */ +export function browserTracingReactRouterV5Integration( + options: Parameters[0] & ReactRouterOptions, +): Integration { + const integration = browserTracingIntegration({ + ...options, + instrumentPageLoad: false, + instrumentNavigation: false, + }); + + const { history, routes, matchPath } = options; + + return { + ...integration, + afterAllSetup(client) { + integration.afterAllSetup(client); + + const startPageloadCallback = (startSpanOptions: StartSpanOptions): undefined => { + startBrowserTracingPageLoadSpan(client, startSpanOptions); + return undefined; + }; + + const startNavigationCallback = (startSpanOptions: StartSpanOptions): undefined => { + startBrowserTracingNavigationSpan(client, startSpanOptions); + return undefined; + }; + + // eslint-disable-next-line deprecation/deprecation + const instrumentation = reactRouterV5Instrumentation(history, routes, matchPath); + + // Now instrument page load & navigation with correct settings + instrumentation(startPageloadCallback, options.instrumentPageLoad, false); + instrumentation(startNavigationCallback, false, options.instrumentNavigation); + }, + }; +} + /** * @deprecated Use `browserTracingReactRouterV4()` instead. */ From 27b117764964272bfeebb592d7d2de8a0fe7c9a3 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 5 Feb 2024 14:02:55 +0100 Subject: [PATCH 4/4] rename to `reactRouterV4BrowserTracingIntegration` / `reactRouterV5BrowserTracingIntegration` --- packages/react/src/index.ts | 4 ++-- packages/react/src/reactrouter.tsx | 4 ++-- packages/react/test/reactrouterv4.test.tsx | 16 ++++++++-------- packages/react/test/reactrouterv5.test.tsx | 16 ++++++++-------- 4 files changed, 20 insertions(+), 20 deletions(-) diff --git a/packages/react/src/index.ts b/packages/react/src/index.ts index 573edfe63ed0..c8be42b00d3b 100644 --- a/packages/react/src/index.ts +++ b/packages/react/src/index.ts @@ -12,8 +12,8 @@ export { // eslint-disable-next-line deprecation/deprecation reactRouterV5Instrumentation, withSentryRouting, - browserTracingReactRouterV4Integration, - browserTracingReactRouterV5Integration, + reactRouterV4BrowserTracingIntegration, + reactRouterV5BrowserTracingIntegration, } from './reactrouter'; export { reactRouterV6Instrumentation, diff --git a/packages/react/src/reactrouter.tsx b/packages/react/src/reactrouter.tsx index a8b60eac75ad..ba6fc523ee58 100644 --- a/packages/react/src/reactrouter.tsx +++ b/packages/react/src/reactrouter.tsx @@ -49,7 +49,7 @@ let activeTransaction: Transaction | undefined; * A browser tracing integration that uses React Router v4 to instrument navigations. * Expects `history` (and optionally `routes` and `matchPath`) to be passed as options. */ -export function browserTracingReactRouterV4Integration( +export function reactRouterV4BrowserTracingIntegration( options: Parameters[0] & ReactRouterOptions, ): Integration { const integration = browserTracingIntegration({ @@ -89,7 +89,7 @@ export function browserTracingReactRouterV4Integration( * A browser tracing integration that uses React Router v5 to instrument navigations. * Expects `history` (and optionally `routes` and `matchPath`) to be passed as options. */ -export function browserTracingReactRouterV5Integration( +export function reactRouterV5BrowserTracingIntegration( options: Parameters[0] & ReactRouterOptions, ): Integration { const integration = browserTracingIntegration({ diff --git a/packages/react/test/reactrouterv4.test.tsx b/packages/react/test/reactrouterv4.test.tsx index 6b6dc9f73c7b..973bda75d273 100644 --- a/packages/react/test/reactrouterv4.test.tsx +++ b/packages/react/test/reactrouterv4.test.tsx @@ -14,7 +14,7 @@ import { Route, Router, Switch, matchPath } from 'react-router-4'; import { BrowserClient, - browserTracingReactRouterV4Integration, + reactRouterV4BrowserTracingIntegration, reactRouterV4Instrumentation, withSentryRouting, } from '../src'; @@ -371,7 +371,7 @@ describe('browserTracingReactRouterV4', () => { setCurrentClient(client); const history = createMemoryHistory(); - client.addIntegration(browserTracingReactRouterV4Integration({ history })); + client.addIntegration(reactRouterV4BrowserTracingIntegration({ history })); client.init(); @@ -391,7 +391,7 @@ describe('browserTracingReactRouterV4', () => { setCurrentClient(client); const history = createMemoryHistory(); - client.addIntegration(browserTracingReactRouterV4Integration({ history })); + client.addIntegration(reactRouterV4BrowserTracingIntegration({ history })); client.init(); @@ -437,7 +437,7 @@ describe('browserTracingReactRouterV4', () => { setCurrentClient(client); const history = createMemoryHistory(); - client.addIntegration(browserTracingReactRouterV4Integration({ history })); + client.addIntegration(reactRouterV4BrowserTracingIntegration({ history })); client.init(); @@ -461,7 +461,7 @@ describe('browserTracingReactRouterV4', () => { const client = createMockBrowserClient(); const history = createMemoryHistory(); - client.addIntegration(browserTracingReactRouterV4Integration({ history })); + client.addIntegration(reactRouterV4BrowserTracingIntegration({ history })); client.init(); @@ -496,7 +496,7 @@ describe('browserTracingReactRouterV4', () => { setCurrentClient(client); const history = createMemoryHistory(); - client.addIntegration(browserTracingReactRouterV4Integration({ history })); + client.addIntegration(reactRouterV4BrowserTracingIntegration({ history })); client.init(); @@ -536,7 +536,7 @@ describe('browserTracingReactRouterV4', () => { setCurrentClient(client); const history = createMemoryHistory(); - client.addIntegration(browserTracingReactRouterV4Integration({ history })); + client.addIntegration(reactRouterV4BrowserTracingIntegration({ history })); client.init(); @@ -601,7 +601,7 @@ describe('browserTracingReactRouterV4', () => { setCurrentClient(client); const history = createMemoryHistory(); - client.addIntegration(browserTracingReactRouterV4Integration({ history, routes, matchPath })); + client.addIntegration(reactRouterV4BrowserTracingIntegration({ history, routes, matchPath })); client.init(); diff --git a/packages/react/test/reactrouterv5.test.tsx b/packages/react/test/reactrouterv5.test.tsx index 4af35943ce3e..b08f7de702a1 100644 --- a/packages/react/test/reactrouterv5.test.tsx +++ b/packages/react/test/reactrouterv5.test.tsx @@ -14,7 +14,7 @@ import { Route, Router, Switch, matchPath } from 'react-router-5'; import { BrowserClient, - browserTracingReactRouterV5Integration, + reactRouterV5BrowserTracingIntegration, reactRouterV5Instrumentation, withSentryRouting, } from '../src'; @@ -371,7 +371,7 @@ describe('browserTracingReactRouterV5', () => { setCurrentClient(client); const history = createMemoryHistory(); - client.addIntegration(browserTracingReactRouterV5Integration({ history })); + client.addIntegration(reactRouterV5BrowserTracingIntegration({ history })); client.init(); @@ -391,7 +391,7 @@ describe('browserTracingReactRouterV5', () => { setCurrentClient(client); const history = createMemoryHistory(); - client.addIntegration(browserTracingReactRouterV5Integration({ history })); + client.addIntegration(reactRouterV5BrowserTracingIntegration({ history })); client.init(); @@ -437,7 +437,7 @@ describe('browserTracingReactRouterV5', () => { setCurrentClient(client); const history = createMemoryHistory(); - client.addIntegration(browserTracingReactRouterV5Integration({ history })); + client.addIntegration(reactRouterV5BrowserTracingIntegration({ history })); client.init(); @@ -461,7 +461,7 @@ describe('browserTracingReactRouterV5', () => { const client = createMockBrowserClient(); const history = createMemoryHistory(); - client.addIntegration(browserTracingReactRouterV5Integration({ history })); + client.addIntegration(reactRouterV5BrowserTracingIntegration({ history })); client.init(); @@ -496,7 +496,7 @@ describe('browserTracingReactRouterV5', () => { setCurrentClient(client); const history = createMemoryHistory(); - client.addIntegration(browserTracingReactRouterV5Integration({ history })); + client.addIntegration(reactRouterV5BrowserTracingIntegration({ history })); client.init(); @@ -536,7 +536,7 @@ describe('browserTracingReactRouterV5', () => { setCurrentClient(client); const history = createMemoryHistory(); - client.addIntegration(browserTracingReactRouterV5Integration({ history })); + client.addIntegration(reactRouterV5BrowserTracingIntegration({ history })); client.init(); @@ -601,7 +601,7 @@ describe('browserTracingReactRouterV5', () => { setCurrentClient(client); const history = createMemoryHistory(); - client.addIntegration(browserTracingReactRouterV5Integration({ history, routes, matchPath })); + client.addIntegration(reactRouterV5BrowserTracingIntegration({ history, routes, matchPath })); client.init();