From 7e3c69a7f8b1a9623d9a8a3d0dd87e45f584a95f Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 5 Feb 2024 11:10:01 +0100 Subject: [PATCH] feat(react): Add `browserTracingReactRouterV3Integration` To replace the routing instrumentation. --- packages/react/src/index.ts | 6 +- packages/react/src/reactrouterv3.ts | 105 +++++++-- packages/react/test/reactrouterv3.test.tsx | 262 +++++++++++++++++---- 3 files changed, 310 insertions(+), 63 deletions(-) diff --git a/packages/react/src/index.ts b/packages/react/src/index.ts index ad66d1e77801..3b6adde57a9f 100644 --- a/packages/react/src/index.ts +++ b/packages/react/src/index.ts @@ -5,7 +5,11 @@ export { Profiler, withProfiler, useProfiler } from './profiler'; export type { ErrorBoundaryProps, FallbackRender } from './errorboundary'; export { ErrorBoundary, withErrorBoundary } from './errorboundary'; export { createReduxEnhancer } from './redux'; -export { reactRouterV3Instrumentation } from './reactrouterv3'; +export { + // eslint-disable-next-line deprecation/deprecation + reactRouterV3Instrumentation, + browserTracingReactRouterV3Integration, +} from './reactrouterv3'; export { reactRouterV4Instrumentation, reactRouterV5Instrumentation, withSentryRouting } from './reactrouter'; export { reactRouterV6Instrumentation, diff --git a/packages/react/src/reactrouterv3.ts b/packages/react/src/reactrouterv3.ts index db1ce1320508..38f7b4d701f1 100644 --- a/packages/react/src/reactrouterv3.ts +++ b/packages/react/src/reactrouterv3.ts @@ -1,5 +1,22 @@ -import { WINDOW } from '@sentry/browser'; -import type { Primitive, Transaction, TransactionContext, TransactionSource } from '@sentry/types'; +import { + WINDOW, + browserTracingIntegration, + startBrowserTracingNavigationSpan, + startBrowserTracingPageLoadSpan, +} from '@sentry/browser'; +import { + SEMANTIC_ATTRIBUTE_SENTRY_OP, + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, +} from '@sentry/core'; +import type { + Integration, + SpanAttributes, + StartSpanOptions, + Transaction, + TransactionContext, + TransactionSource, +} from '@sentry/types'; import type { Location, ReactRouterInstrumentation } from './types'; @@ -21,6 +38,52 @@ export type Match = ( type ReactRouterV3TransactionSource = Extract; +interface ReactRouterOptions { + history: HistoryV3; + routes: Route[]; + match: Match; +} + +/** + * A browser tracing integration that uses React Router v3 to instrument navigations. + * Expects `history` (and optionally `routes` and `matchPath`) to be passed as options. + */ +export function browserTracingReactRouterV3Integration( + options: Parameters[0] & ReactRouterOptions, +): Integration { + const integration = browserTracingIntegration({ + ...options, + instrumentPageLoad: false, + instrumentNavigation: false, + }); + + const { history, routes, match, 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 = reactRouterV3Instrumentation(history, routes, match); + + // Now instrument page load & navigation with correct settings + instrumentation(startPageloadCallback, instrumentPageLoad, false); + instrumentation(startNavigationCallback, false, instrumentNavigation); + }, + }; +} + /** * Creates routing instrumentation for React Router v3 * Works for React Router >= 3.2.0 and < 4.0.0 @@ -28,6 +91,8 @@ type ReactRouterV3TransactionSource = Extract = { - 'routing.instrumentation': 'react-router-v3', - }; - if (prevName) { - tags.from = prevName; - } + const from = prevName; normalizeTransactionName(routes, location, match, (localName: string, source: TransactionSource = 'url') => { prevName = localName; + + const attributes: SpanAttributes = { + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v3', + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source, + }; + + if (from) { + attributes.from = from; + } + activeTransaction = startTransaction({ name: prevName, - op: 'navigation', - origin: 'auto.navigation.react.reactrouterv3', - tags, - metadata: { - source, - }, + attributes, }); }); } diff --git a/packages/react/test/reactrouterv3.test.tsx b/packages/react/test/reactrouterv3.test.tsx index 21b9054e45ce..5003c0e8aaad 100644 --- a/packages/react/test/reactrouterv3.test.tsx +++ b/packages/react/test/reactrouterv3.test.tsx @@ -1,8 +1,18 @@ +import { BrowserClient } from '@sentry/browser'; +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 * as React from 'react'; import { IndexRoute, Route, Router, createMemoryHistory, createRoutes, match } from 'react-router-3'; import type { Match, Route as RouteType } from '../src/reactrouterv3'; +import { browserTracingReactRouterV3Integration } from '../src/reactrouterv3'; import { reactRouterV3Instrumentation } from '../src/reactrouterv3'; // Have to manually set types because we are using package-alias @@ -24,7 +34,7 @@ function createMockStartTransaction(opts: { finish?: jest.FunctionLike; setMetad }); } -describe('React Router V3', () => { +describe('reactRouterV3Instrumentation', () => { const routes = (
{children}
}>
Home
} /> @@ -43,6 +53,7 @@ describe('React Router V3', () => { const history = createMemoryHistory(); const instrumentationRoutes = createRoutes(routes); + // eslint-disable-next-line deprecation/deprecation const instrumentation = reactRouterV3Instrumentation(history, instrumentationRoutes, match); it('starts a pageload transaction when instrumentation is started', () => { @@ -51,11 +62,10 @@ describe('React Router V3', () => { expect(mockStartTransaction).toHaveBeenCalledTimes(1); expect(mockStartTransaction).toHaveBeenLastCalledWith({ name: '/', - op: 'pageload', - origin: 'auto.pageload.react.reactrouterv3', - tags: { 'routing.instrumentation': 'react-router-v3' }, - metadata: { - source: 'route', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.react.reactrouter_v3', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', }, }); }); @@ -77,11 +87,11 @@ describe('React Router V3', () => { expect(mockStartTransaction).toHaveBeenCalledTimes(2); expect(mockStartTransaction).toHaveBeenLastCalledWith({ name: '/about', - op: 'navigation', - origin: 'auto.navigation.react.reactrouterv3', - tags: { from: '/', 'routing.instrumentation': 'react-router-v3' }, - metadata: { - source: 'route', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v3', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + from: '/', }, }); @@ -91,11 +101,11 @@ describe('React Router V3', () => { expect(mockStartTransaction).toHaveBeenCalledTimes(3); expect(mockStartTransaction).toHaveBeenLastCalledWith({ name: '/features', - op: 'navigation', - origin: 'auto.navigation.react.reactrouterv3', - tags: { from: '/about', 'routing.instrumentation': 'react-router-v3' }, - metadata: { - source: 'route', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v3', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + from: '/about', }, }); }); @@ -145,11 +155,11 @@ describe('React Router V3', () => { expect(mockStartTransaction).toHaveBeenCalledTimes(2); expect(mockStartTransaction).toHaveBeenLastCalledWith({ name: '/users/:userid', - op: 'navigation', - origin: 'auto.navigation.react.reactrouterv3', - tags: { from: '/', 'routing.instrumentation': 'react-router-v3' }, - metadata: { - source: 'route', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v3', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + from: '/', }, }); }); @@ -167,11 +177,11 @@ describe('React Router V3', () => { expect(mockStartTransaction).toHaveBeenCalledTimes(2); expect(mockStartTransaction).toHaveBeenLastCalledWith({ name: '/organizations/:orgid/v1/:teamid', - op: 'navigation', - origin: 'auto.navigation.react.reactrouterv3', - tags: { from: '/', 'routing.instrumentation': 'react-router-v3' }, - metadata: { - source: 'route', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v3', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + from: '/', }, }); @@ -183,11 +193,11 @@ describe('React Router V3', () => { expect(mockStartTransaction).toHaveBeenCalledTimes(3); expect(mockStartTransaction).toHaveBeenLastCalledWith({ name: '/organizations/:orgid', - op: 'navigation', - origin: 'auto.navigation.react.reactrouterv3', - tags: { from: '/organizations/:orgid/v1/:teamid', 'routing.instrumentation': 'react-router-v3' }, - metadata: { - source: 'route', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v3', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + from: '/organizations/:orgid/v1/:teamid', }, }); }); @@ -204,11 +214,11 @@ describe('React Router V3', () => { expect(mockStartTransaction).toHaveBeenCalledTimes(2); expect(mockStartTransaction).toHaveBeenLastCalledWith({ name: '/organizations/1234/some/other/route', - op: 'navigation', - origin: 'auto.navigation.react.reactrouterv3', - tags: { from: '/', 'routing.instrumentation': 'react-router-v3' }, - metadata: { - source: 'url', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v3', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + from: '/', }, }); }); @@ -216,6 +226,7 @@ describe('React Router V3', () => { it('sets metadata to url if no routes are provided', () => { const fakeRoutes =
hello
; const mockStartTransaction = createMockStartTransaction(); + // eslint-disable-next-line deprecation/deprecation const mockInstrumentation = reactRouterV3Instrumentation(history, createRoutes(fakeRoutes), match); mockInstrumentation(mockStartTransaction); // We render here with `routes` instead of `fakeRoutes` from above to validate the case @@ -225,11 +236,180 @@ describe('React Router V3', () => { expect(mockStartTransaction).toHaveBeenCalledTimes(1); expect(mockStartTransaction).toHaveBeenLastCalledWith({ name: '/', - op: 'pageload', - origin: 'auto.pageload.react.reactrouterv3', - tags: { 'routing.instrumentation': 'react-router-v3' }, - metadata: { - source: 'url', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.react.reactrouter_v3', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', + }, + }); + }); +}); + +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('browserTracingReactRouterV3', () => { + const routes = ( +
{children}
}> +
Home
} /> +
About
} /> +
Features
} /> + }) =>
{params.userid}
} + /> + +
OrgId
} /> +
Team
} /> + + + ); + const history = createMemoryHistory(); + + const instrumentationRoutes = createRoutes(routes); + + 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); + + client.addIntegration(browserTracingReactRouterV3Integration({ history, routes: instrumentationRoutes, match })); + + client.init(); + render({routes}); + + expect(mockStartBrowserTracingPageLoadSpan).toHaveBeenCalledTimes(1); + expect(mockStartBrowserTracingPageLoadSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), { + name: '/', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.react.reactrouter_v3', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload', + }, + }); + }); + + it('starts a navigation transaction', () => { + const client = createMockBrowserClient(); + setCurrentClient(client); + + const history = createMemoryHistory(); + client.addIntegration(browserTracingReactRouterV3Integration({ history, routes: instrumentationRoutes, match })); + + client.init(); + render({routes}); + + act(() => { + history.push('/about'); + }); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), { + name: '/about', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v3', + [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]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v3', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', + from: '/about', + }, + }); + }); + + it('only starts a navigation transaction on push', () => { + const client = createMockBrowserClient(); + setCurrentClient(client); + + const history = createMemoryHistory(); + client.addIntegration(browserTracingReactRouterV3Integration({ history, routes: instrumentationRoutes, match })); + + client.init(); + render({routes}); + + act(() => { + history.replace('hello'); + }); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(0); + }); + + it('normalizes transaction name ', () => { + const client = createMockBrowserClient(); + + const history = createMemoryHistory(); + client.addIntegration(browserTracingReactRouterV3Integration({ history, routes: instrumentationRoutes, match })); + + client.init(); + const { container } = render({routes}); + + act(() => { + history.push('/users/123'); + }); + expect(container.innerHTML).toContain('123'); + + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenCalledTimes(1); + expect(mockStartBrowserTracingNavigationSpan).toHaveBeenLastCalledWith(expect.any(BrowserClient), { + name: '/users/:userid', + attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.react.reactrouter_v3', + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation', }, }); });