From 76e99de4808f3767b306b09c00fc92d7ac77ef9b Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Wed, 6 Jul 2022 19:06:52 -0400 Subject: [PATCH] ref(react): Add source to react-router-v3 --- packages/react/src/reactrouterv3.ts | 41 +++++++---- packages/react/test/reactrouterv3.test.tsx | 82 +++++++++++++++++++--- 2 files changed, 101 insertions(+), 22 deletions(-) diff --git a/packages/react/src/reactrouterv3.ts b/packages/react/src/reactrouterv3.ts index 5c6f9dda9155..494d16c2bbca 100644 --- a/packages/react/src/reactrouterv3.ts +++ b/packages/react/src/reactrouterv3.ts @@ -1,4 +1,4 @@ -import { Primitive, Transaction, TransactionContext } from '@sentry/types'; +import { Primitive, Transaction, TransactionContext, TransactionSource } from '@sentry/types'; import { getGlobalObject } from '@sentry/utils'; import { Location, ReactRouterInstrumentation } from './types'; @@ -19,6 +19,8 @@ export type Match = ( cb: (error?: Error, _redirectLocation?: Location, renderProps?: { routes?: Route[] }) => void, ) => void; +type ReactRouterV3TransactionSource = Extract; + const global = getGlobalObject(); /** @@ -44,16 +46,24 @@ export function reactRouterV3Instrumentation( // Have to use global.location because history.location might not be defined. if (startTransactionOnPageLoad && global && global.location) { - normalizeTransactionName(routes, global.location as unknown as Location, match, (localName: string) => { - prevName = localName; - activeTransaction = startTransaction({ - name: prevName, - op: 'pageload', - tags: { - 'routing.instrumentation': 'react-router-v3', - }, - }); - }); + normalizeTransactionName( + routes, + global.location as unknown as Location, + match, + (localName: string, source: ReactRouterV3TransactionSource = 'url') => { + prevName = localName; + activeTransaction = startTransaction({ + name: prevName, + op: 'pageload', + tags: { + 'routing.instrumentation': 'react-router-v3', + }, + metadata: { + source, + }, + }); + }, + ); } if (startTransactionOnLocationChange && history.listen) { @@ -68,12 +78,15 @@ export function reactRouterV3Instrumentation( if (prevName) { tags.from = prevName; } - normalizeTransactionName(routes, location, match, (localName: string) => { + normalizeTransactionName(routes, location, match, (localName: string, source: TransactionSource = 'url') => { prevName = localName; activeTransaction = startTransaction({ name: prevName, op: 'navigation', tags, + metadata: { + source, + }, }); }); } @@ -89,7 +102,7 @@ function normalizeTransactionName( appRoutes: Route[], location: Location, match: Match, - callback: (pathname: string) => void, + callback: (pathname: string, source?: ReactRouterV3TransactionSource) => void, ): void { let name = location.pathname; match( @@ -108,7 +121,7 @@ function normalizeTransactionName( } name = routePath; - return callback(name); + return callback(name, 'route'); }, ); } diff --git a/packages/react/test/reactrouterv3.test.tsx b/packages/react/test/reactrouterv3.test.tsx index 1faf7c7ce6f0..336f52b74268 100644 --- a/packages/react/test/reactrouterv3.test.tsx +++ b/packages/react/test/reactrouterv3.test.tsx @@ -15,6 +15,14 @@ declare module 'react-router-3' { export const createRoutes: (routes: any) => RouteType[]; } +function createMockStartTransaction(opts: { finish?: jest.FunctionLike; setMetadata?: jest.FunctionLike } = {}) { + const { finish = jest.fn(), setMetadata = jest.fn() } = opts; + return jest.fn().mockReturnValue({ + finish, + setMetadata, + }); +} + describe('React Router V3', () => { const routes = (
{children}
}> @@ -37,24 +45,27 @@ describe('React Router V3', () => { const instrumentation = reactRouterV3Instrumentation(history, instrumentationRoutes, match); it('starts a pageload transaction when instrumentation is started', () => { - const mockStartTransaction = jest.fn(); + const mockStartTransaction = createMockStartTransaction(); instrumentation(mockStartTransaction); expect(mockStartTransaction).toHaveBeenCalledTimes(1); expect(mockStartTransaction).toHaveBeenLastCalledWith({ name: '/', op: 'pageload', tags: { 'routing.instrumentation': 'react-router-v3' }, + metadata: { + source: 'route', + }, }); }); it('does not start pageload transaction if option is false', () => { - const mockStartTransaction = jest.fn(); + const mockStartTransaction = createMockStartTransaction(); instrumentation(mockStartTransaction, false); expect(mockStartTransaction).toHaveBeenCalledTimes(0); }); it('starts a navigation transaction', () => { - const mockStartTransaction = jest.fn(); + const mockStartTransaction = createMockStartTransaction(); instrumentation(mockStartTransaction); render({routes}); @@ -66,6 +77,9 @@ describe('React Router V3', () => { name: '/about', op: 'navigation', tags: { from: '/', 'routing.instrumentation': 'react-router-v3' }, + metadata: { + source: 'route', + }, }); act(() => { @@ -76,18 +90,21 @@ describe('React Router V3', () => { name: '/features', op: 'navigation', tags: { from: '/about', 'routing.instrumentation': 'react-router-v3' }, + metadata: { + source: 'route', + }, }); }); it('does not start a transaction if option is false', () => { - const mockStartTransaction = jest.fn(); + const mockStartTransaction = createMockStartTransaction(); instrumentation(mockStartTransaction, true, false); render({routes}); expect(mockStartTransaction).toHaveBeenCalledTimes(1); }); it('only starts a navigation transaction on push', () => { - const mockStartTransaction = jest.fn(); + const mockStartTransaction = createMockStartTransaction(); instrumentation(mockStartTransaction); render({routes}); @@ -99,7 +116,7 @@ describe('React Router V3', () => { it('finishes a transaction on navigation', () => { const mockFinish = jest.fn(); - const mockStartTransaction = jest.fn().mockReturnValue({ finish: mockFinish }); + const mockStartTransaction = createMockStartTransaction({ finish: mockFinish }); instrumentation(mockStartTransaction); render({routes}); expect(mockStartTransaction).toHaveBeenCalledTimes(1); @@ -112,7 +129,7 @@ describe('React Router V3', () => { }); it('normalizes transaction names', () => { - const mockStartTransaction = jest.fn(); + const mockStartTransaction = createMockStartTransaction(); instrumentation(mockStartTransaction); const { container } = render({routes}); @@ -126,11 +143,14 @@ describe('React Router V3', () => { name: '/users/:userid', op: 'navigation', tags: { from: '/', 'routing.instrumentation': 'react-router-v3' }, + metadata: { + source: 'route', + }, }); }); it('normalizes nested transaction names', () => { - const mockStartTransaction = jest.fn(); + const mockStartTransaction = createMockStartTransaction(); instrumentation(mockStartTransaction); const { container } = render({routes}); @@ -144,6 +164,9 @@ describe('React Router V3', () => { name: '/organizations/:orgid/v1/:teamid', op: 'navigation', tags: { from: '/', 'routing.instrumentation': 'react-router-v3' }, + metadata: { + source: 'route', + }, }); act(() => { @@ -156,6 +179,49 @@ describe('React Router V3', () => { name: '/organizations/:orgid', op: 'navigation', tags: { from: '/organizations/:orgid/v1/:teamid', 'routing.instrumentation': 'react-router-v3' }, + metadata: { + source: 'route', + }, + }); + }); + + it('sets metadata to url if on an unknown route', () => { + const mockStartTransaction = createMockStartTransaction(); + instrumentation(mockStartTransaction); + render({routes}); + + act(() => { + history.push('/organizations/1234/some/other/route'); + }); + + expect(mockStartTransaction).toHaveBeenCalledTimes(2); + expect(mockStartTransaction).toHaveBeenLastCalledWith({ + name: '/organizations/1234/some/other/route', + op: 'navigation', + tags: { from: '/', 'routing.instrumentation': 'react-router-v3' }, + metadata: { + source: 'url', + }, + }); + }); + + it('sets metadata to url if no routes are provided', () => { + const fakeRoutes =
hello
; + const mockStartTransaction = createMockStartTransaction(); + const mockInstrumentation = reactRouterV3Instrumentation(history, createRoutes(fakeRoutes), match); + mockInstrumentation(mockStartTransaction); + // We render here with `routes` instead of `fakeRoutes` from above to validate the case + // where users provided the instrumentation with a bad set of routes. + render({routes}); + + expect(mockStartTransaction).toHaveBeenCalledTimes(1); + expect(mockStartTransaction).toHaveBeenLastCalledWith({ + name: '/', + op: 'pageload', + tags: { 'routing.instrumentation': 'react-router-v3' }, + metadata: { + source: 'url', + }, }); }); });