ref(react): Add source to react-router-v3

Author: AbhiPrasad

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<TransactionSource, 'url' | 'route'>;
+
 const global = getGlobalObject<Window>();
 
 /**
@@ -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');
     },
   );
 }

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 = (
     <Route path="/" component={({ children }: { children: JSX.Element }) => <div>{children}</div>}>
@@ -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(<Router history={history}>{routes}</Router>);
 
@@ -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(<Router history={history}>{routes}</Router>);
     expect(mockStartTransaction).toHaveBeenCalledTimes(1);
   });
 
   it('only starts a navigation transaction on push', () => {
-    const mockStartTransaction = jest.fn();
+    const mockStartTransaction = createMockStartTransaction();
     instrumentation(mockStartTransaction);
     render(<Router history={history}>{routes}</Router>);
 
@@ -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(<Router history={history}>{routes}</Router>);
     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(<Router history={history}>{routes}</Router>);
 
@@ -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(<Router history={history}>{routes}</Router>);
 
@@ -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(<Router history={history}>{routes}</Router>);
+
+    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 = <div>hello</div>;
+    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(<Router history={history}>{routes}</Router>);
+
+    expect(mockStartTransaction).toHaveBeenCalledTimes(1);
+    expect(mockStartTransaction).toHaveBeenLastCalledWith({
+      name: '/',
+      op: 'pageload',
+      tags: { 'routing.instrumentation': 'react-router-v3' },
+      metadata: {
+        source: 'url',
+      },
     });
   });
 });