From 59dab98fc51a75f1a075cc2eb69a09ce490c7d09 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Thu, 7 Jul 2022 12:09:58 -0400 Subject: [PATCH] ref(react): Add transaction source for react router v4/v5 --- packages/react/src/reactrouter.tsx | 38 +++++++++++++++------- packages/react/test/reactrouterv4.test.tsx | 25 +++++++++++--- packages/react/test/reactrouterv5.test.tsx | 25 +++++++++++--- 3 files changed, 66 insertions(+), 22 deletions(-) diff --git a/packages/react/src/reactrouter.tsx b/packages/react/src/reactrouter.tsx index da1a72b129ad..3abc4f647549 100644 --- a/packages/react/src/reactrouter.tsx +++ b/packages/react/src/reactrouter.tsx @@ -1,4 +1,4 @@ -import { Transaction } from '@sentry/types'; +import { Transaction, TransactionSource } from '@sentry/types'; import { getGlobalObject } from '@sentry/utils'; import hoistNonReactStatics from 'hoist-non-react-statics'; import * as React from 'react'; @@ -64,30 +64,42 @@ function createReactRouterInstrumentation( return undefined; } - function getTransactionName(pathname: string): string { + /** + * Normalizes a transaction name. Returns the new name as well as the + * source of the transaction. + * + * @param pathname The initial pathname we normalize + */ + function normalizeTransactionName(pathname: string): [string, TransactionSource] { if (allRoutes.length === 0 || !matchPath) { - return pathname; + return [pathname, 'url']; } const branches = matchRoutes(allRoutes, pathname, matchPath); // eslint-disable-next-line @typescript-eslint/prefer-for-of for (let x = 0; x < branches.length; x++) { if (branches[x].match.isExact) { - return branches[x].match.path; + return [branches[x].match.path, 'route']; } } - return pathname; + 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: getTransactionName(initPathName), + name, op: 'pageload', - tags: { - 'routing.instrumentation': name, + tags, + metadata: { + source, }, }); } @@ -98,14 +110,15 @@ function createReactRouterInstrumentation( if (activeTransaction) { activeTransaction.finish(); } - const tags = { - 'routing.instrumentation': name, - }; + const [name, source] = normalizeTransactionName(location.pathname); activeTransaction = customStartTransaction({ - name: getTransactionName(location.pathname), + name, op: 'navigation', tags, + metadata: { + source, + }, }); } }); @@ -155,6 +168,7 @@ export function withSentryRouting

, R extends React const WrappedRoute: React.FC

= (props: P) => { if (activeTransaction && props && props.computedMatch && props.computedMatch.isExact) { activeTransaction.setName(props.computedMatch.path); + activeTransaction.setMetadata({ source: 'route' }); } // @ts-ignore Setting more specific React Component typing for `R` generic above diff --git a/packages/react/test/reactrouterv4.test.tsx b/packages/react/test/reactrouterv4.test.tsx index 60f617516482..4c4e0b20d979 100644 --- a/packages/react/test/reactrouterv4.test.tsx +++ b/packages/react/test/reactrouterv4.test.tsx @@ -11,7 +11,7 @@ describe('React Router v4', () => { startTransactionOnPageLoad?: boolean; startTransactionOnLocationChange?: boolean; routes?: RouteConfig[]; - }): [jest.Mock, any, { mockSetName: jest.Mock; mockFinish: jest.Mock }] { + }): [jest.Mock, any, { mockSetName: jest.Mock; mockFinish: jest.Mock; mockSetMetadata: jest.Mock }] { const options = { matchPath: _opts && _opts.routes !== undefined ? matchPath : undefined, routes: undefined, @@ -22,13 +22,16 @@ describe('React Router v4', () => { const history = createMemoryHistory(); const mockFinish = jest.fn(); const mockSetName = jest.fn(); - const mockStartTransaction = jest.fn().mockReturnValue({ setName: mockSetName, finish: mockFinish }); + const mockSetMetadata = jest.fn(); + const mockStartTransaction = jest + .fn() + .mockReturnValue({ setName: mockSetName, finish: mockFinish, setMetadata: mockSetMetadata }); reactRouterV4Instrumentation(history, options.routes, options.matchPath)( mockStartTransaction, options.startTransactionOnPageLoad, options.startTransactionOnLocationChange, ); - return [mockStartTransaction, history, { mockSetName, mockFinish }]; + return [mockStartTransaction, history, { mockSetName, mockFinish, mockSetMetadata }]; } it('starts a pageload transaction when instrumentation is started', () => { @@ -38,6 +41,7 @@ describe('React Router v4', () => { name: '/', op: 'pageload', tags: { 'routing.instrumentation': 'react-router-v4' }, + metadata: { source: 'url' }, }); }); @@ -66,6 +70,7 @@ describe('React Router v4', () => { name: '/about', op: 'navigation', tags: { 'routing.instrumentation': 'react-router-v4' }, + metadata: { source: 'url' }, }); act(() => { @@ -76,6 +81,7 @@ describe('React Router v4', () => { name: '/features', op: 'navigation', tags: { 'routing.instrumentation': 'react-router-v4' }, + metadata: { source: 'url' }, }); }); @@ -153,11 +159,12 @@ describe('React Router v4', () => { name: '/users/123', op: 'navigation', tags: { 'routing.instrumentation': 'react-router-v4' }, + metadata: { source: 'url' }, }); }); it('normalizes transaction name with custom Route', () => { - const [mockStartTransaction, history, { mockSetName }] = createInstrumentation(); + const [mockStartTransaction, history, { mockSetName, mockSetMetadata }] = createInstrumentation(); const SentryRoute = withSentryRouting(Route); const { getByText } = render( @@ -179,13 +186,15 @@ describe('React Router v4', () => { name: '/users/123', op: 'navigation', tags: { 'routing.instrumentation': 'react-router-v4' }, + metadata: { source: 'url' }, }); expect(mockSetName).toHaveBeenCalledTimes(2); expect(mockSetName).toHaveBeenLastCalledWith('/users/:userid'); + expect(mockSetMetadata).toHaveBeenLastCalledWith({ source: 'route' }); }); it('normalizes nested transaction names with custom Route', () => { - const [mockStartTransaction, history, { mockSetName }] = createInstrumentation(); + const [mockStartTransaction, history, { mockSetName, mockSetMetadata }] = createInstrumentation(); const SentryRoute = withSentryRouting(Route); const { getByText } = render( @@ -207,9 +216,11 @@ describe('React Router v4', () => { name: '/organizations/1234/v1/758', op: 'navigation', tags: { 'routing.instrumentation': 'react-router-v4' }, + metadata: { source: 'url' }, }); expect(mockSetName).toHaveBeenCalledTimes(2); expect(mockSetName).toHaveBeenLastCalledWith('/organizations/:orgid/v1/:teamid'); + expect(mockSetMetadata).toHaveBeenLastCalledWith({ source: 'route' }); act(() => { history.push('/organizations/543'); @@ -221,9 +232,11 @@ describe('React Router v4', () => { name: '/organizations/543', op: 'navigation', tags: { 'routing.instrumentation': 'react-router-v4' }, + metadata: { source: 'url' }, }); expect(mockSetName).toHaveBeenCalledTimes(3); expect(mockSetName).toHaveBeenLastCalledWith('/organizations/:orgid'); + expect(mockSetMetadata).toHaveBeenLastCalledWith({ source: 'route' }); }); it('matches with route object', () => { @@ -253,6 +266,7 @@ describe('React Router v4', () => { name: '/organizations/:orgid/v1/:teamid', op: 'navigation', tags: { 'routing.instrumentation': 'react-router-v4' }, + metadata: { source: 'route' }, }); act(() => { @@ -263,6 +277,7 @@ describe('React Router v4', () => { name: '/organizations/:orgid', op: 'navigation', tags: { 'routing.instrumentation': 'react-router-v4' }, + metadata: { source: 'route' }, }); }); }); diff --git a/packages/react/test/reactrouterv5.test.tsx b/packages/react/test/reactrouterv5.test.tsx index daa5a4fa5554..0fcb6d3134dd 100644 --- a/packages/react/test/reactrouterv5.test.tsx +++ b/packages/react/test/reactrouterv5.test.tsx @@ -11,7 +11,7 @@ describe('React Router v5', () => { startTransactionOnPageLoad?: boolean; startTransactionOnLocationChange?: boolean; routes?: RouteConfig[]; - }): [jest.Mock, any, { mockSetName: jest.Mock; mockFinish: jest.Mock }] { + }): [jest.Mock, any, { mockSetName: jest.Mock; mockFinish: jest.Mock; mockSetMetadata: jest.Mock }] { const options = { matchPath: _opts && _opts.routes !== undefined ? matchPath : undefined, routes: undefined, @@ -22,13 +22,16 @@ describe('React Router v5', () => { const history = createMemoryHistory(); const mockFinish = jest.fn(); const mockSetName = jest.fn(); - const mockStartTransaction = jest.fn().mockReturnValue({ setName: mockSetName, finish: mockFinish }); + const mockSetMetadata = jest.fn(); + const mockStartTransaction = jest + .fn() + .mockReturnValue({ setName: mockSetName, finish: mockFinish, setMetadata: mockSetMetadata }); reactRouterV5Instrumentation(history, options.routes, options.matchPath)( mockStartTransaction, options.startTransactionOnPageLoad, options.startTransactionOnLocationChange, ); - return [mockStartTransaction, history, { mockSetName, mockFinish }]; + return [mockStartTransaction, history, { mockSetName, mockFinish, mockSetMetadata }]; } it('starts a pageload transaction when instrumentation is started', () => { @@ -38,6 +41,7 @@ describe('React Router v5', () => { name: '/', op: 'pageload', tags: { 'routing.instrumentation': 'react-router-v5' }, + metadata: { source: 'url' }, }); }); @@ -66,6 +70,7 @@ describe('React Router v5', () => { name: '/about', op: 'navigation', tags: { 'routing.instrumentation': 'react-router-v5' }, + metadata: { source: 'url' }, }); act(() => { @@ -76,6 +81,7 @@ describe('React Router v5', () => { name: '/features', op: 'navigation', tags: { 'routing.instrumentation': 'react-router-v5' }, + metadata: { source: 'url' }, }); }); @@ -153,11 +159,12 @@ describe('React Router v5', () => { name: '/users/123', op: 'navigation', tags: { 'routing.instrumentation': 'react-router-v5' }, + metadata: { source: 'url' }, }); }); it('normalizes transaction name with custom Route', () => { - const [mockStartTransaction, history, { mockSetName }] = createInstrumentation(); + const [mockStartTransaction, history, { mockSetName, mockSetMetadata }] = createInstrumentation(); const SentryRoute = withSentryRouting(Route); const { getByText } = render( @@ -179,13 +186,15 @@ describe('React Router v5', () => { name: '/users/123', op: 'navigation', tags: { 'routing.instrumentation': 'react-router-v5' }, + metadata: { source: 'url' }, }); expect(mockSetName).toHaveBeenCalledTimes(2); expect(mockSetName).toHaveBeenLastCalledWith('/users/:userid'); + expect(mockSetMetadata).toHaveBeenLastCalledWith({ source: 'route' }); }); it('normalizes nested transaction names with custom Route', () => { - const [mockStartTransaction, history, { mockSetName }] = createInstrumentation(); + const [mockStartTransaction, history, { mockSetName, mockSetMetadata }] = createInstrumentation(); const SentryRoute = withSentryRouting(Route); const { getByText } = render( @@ -208,9 +217,11 @@ describe('React Router v5', () => { name: '/organizations/1234/v1/758', op: 'navigation', tags: { 'routing.instrumentation': 'react-router-v5' }, + metadata: { source: 'url' }, }); expect(mockSetName).toHaveBeenCalledTimes(2); expect(mockSetName).toHaveBeenLastCalledWith('/organizations/:orgid/v1/:teamid'); + expect(mockSetMetadata).toHaveBeenLastCalledWith({ source: 'route' }); act(() => { history.push('/organizations/543'); @@ -222,9 +233,11 @@ describe('React Router v5', () => { name: '/organizations/543', op: 'navigation', tags: { 'routing.instrumentation': 'react-router-v5' }, + metadata: { source: 'url' }, }); expect(mockSetName).toHaveBeenCalledTimes(3); expect(mockSetName).toHaveBeenLastCalledWith('/organizations/:orgid'); + expect(mockSetMetadata).toHaveBeenLastCalledWith({ source: 'route' }); }); it('matches with route object', () => { @@ -254,6 +267,7 @@ describe('React Router v5', () => { name: '/organizations/:orgid/v1/:teamid', op: 'navigation', tags: { 'routing.instrumentation': 'react-router-v5' }, + metadata: { source: 'route' }, }); act(() => { @@ -264,6 +278,7 @@ describe('React Router v5', () => { name: '/organizations/:orgid', op: 'navigation', tags: { 'routing.instrumentation': 'react-router-v5' }, + metadata: { source: 'route' }, }); }); });