Skip to content

Commit

Permalink
ref(react): Add transaction source for react router v4/v5 (#5384)
Browse files Browse the repository at this point in the history
  • Loading branch information
@AbhiPrasad
AbhiPrasad committed Jul 8, 2022
1 parent f1b29ef commit 3e7a3ec
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 22 deletions.
38 changes: 26 additions & 12 deletions packages/react/src/reactrouter.tsx
View file
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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,
},
});
}
Expand All @@ -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,
},
});
}
});
Expand Down Expand Up @@ -155,6 +168,7 @@ export function withSentryRouting<P extends Record<string, any>, R extends React
const WrappedRoute: React.FC<P> = (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
Expand Down
25 changes: 20 additions & 5 deletions packages/react/test/reactrouterv4.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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', () => {
Expand All @@ -38,6 +41,7 @@ describe('React Router v4', () => {
name: '/',
op: 'pageload',
tags: { 'routing.instrumentation': 'react-router-v4' },
metadata: { source: 'url' },
});
});

Expand Down Expand Up @@ -66,6 +70,7 @@ describe('React Router v4', () => {
name: '/about',
op: 'navigation',
tags: { 'routing.instrumentation': 'react-router-v4' },
metadata: { source: 'url' },
});

act(() => {
Expand All @@ -76,6 +81,7 @@ describe('React Router v4', () => {
name: '/features',
op: 'navigation',
tags: { 'routing.instrumentation': 'react-router-v4' },
metadata: { source: 'url' },
});
});

Expand Down Expand Up @@ -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(
<Router history={history}>
Expand All @@ -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(
<Router history={history}>
Expand All @@ -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');
Expand All @@ -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', () => {
Expand Down Expand Up @@ -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(() => {
Expand All @@ -263,6 +277,7 @@ describe('React Router v4', () => {
name: '/organizations/:orgid',
op: 'navigation',
tags: { 'routing.instrumentation': 'react-router-v4' },
metadata: { source: 'route' },
});
});
});
25 changes: 20 additions & 5 deletions packages/react/test/reactrouterv5.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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', () => {
Expand All @@ -38,6 +41,7 @@ describe('React Router v5', () => {
name: '/',
op: 'pageload',
tags: { 'routing.instrumentation': 'react-router-v5' },
metadata: { source: 'url' },
});
});

Expand Down Expand Up @@ -66,6 +70,7 @@ describe('React Router v5', () => {
name: '/about',
op: 'navigation',
tags: { 'routing.instrumentation': 'react-router-v5' },
metadata: { source: 'url' },
});

act(() => {
Expand All @@ -76,6 +81,7 @@ describe('React Router v5', () => {
name: '/features',
op: 'navigation',
tags: { 'routing.instrumentation': 'react-router-v5' },
metadata: { source: 'url' },
});
});

Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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');
Expand All @@ -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', () => {
Expand Down Expand Up @@ -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(() => {
Expand All @@ -264,6 +278,7 @@ describe('React Router v5', () => {
name: '/organizations/:orgid',
op: 'navigation',
tags: { 'routing.instrumentation': 'react-router-v5' },
metadata: { source: 'route' },
});
});
});

0 comments on commit 3e7a3ec

Please sign in to comment.