feat(Angular): Add URL Parameterization of Transaction Names

packages/angular/src/tracing.ts

@@ -1,5 +1,14 @@
+/* eslint-disable max-lines */
 import { AfterViewInit, Directive, Injectable, Input, NgModule, OnDestroy, OnInit } from '@angular/core';
-import { Event, NavigationEnd, NavigationStart, Router } from '@angular/router';
+import {
+  ActivatedRouteSnapshot,
+  Event,
+  NavigationEnd,
+  NavigationStart,
+  Params,
+  ResolveEnd,
+  Router,
+} from '@angular/router';
 import { getCurrentHub } from '@sentry/browser';
 import { Span, Transaction, TransactionContext } from '@sentry/types';
 import { getGlobalObject, logger, stripUrlQueryAndFragment, timestampWithMs } from '@sentry/utils';
@@ -83,6 +92,7 @@ export class TraceService implements OnDestroy {
       }
 
       if (activeTransaction) {
+        this._activeTransaction = activeTransaction;
         if (this._routingSpan) {
           this._routingSpan.finish();
         }
@@ -101,6 +111,38 @@ export class TraceService implements OnDestroy {
     }),
   );
 
+  // The ResolveEnd event is fired when the Angular router has resolved the URL
+  // and activated the route. It holds the new resolved router state and the
+  // new URL.
+  // Only After this event, the route is activated, meaning that the transaction
+  // can be updated with the parameterized route name before e.g. the route's root
+  // component is initialized. This should be early enough before outgoing requests
+  // are made from the new route.
+  // In any case, this is the earliest stage where we can reliably get a resolved
+  //
+  public resEnd$: Observable<Event> = this._router.events.pipe(
+    filter(event => event instanceof ResolveEnd),
+    tap(event => {
+      const ev = event as ResolveEnd;
+
+      const params = getParamsOfRoute(ev.state.root);
+
+      // ev.urlAfterRedirects is the one we prefer because it should hold the most recent
+      // one that holds information about a redirect to another route if this was specified
+      // in the Angular router config. In case this doesn't exist (for whatever reason),
+      // we fall back to ev.url which holds the primarily resolved URL before a potential
+      // redirect.
+      const url = ev.urlAfterRedirects || ev.url;
+      const route = getParameterizedRouteFromUrlAndParams(url, params);
+
+      const transaction = this._activeTransaction;
+      if (transaction && transaction.metadata.source === 'url') {
+        transaction.setName(route);
+        transaction.setMetadata({ source: 'route' });
+      }
+    }),
+  );
+
   public navEnd$: Observable<Event> = this._router.events.pipe(
     filter(event => event instanceof NavigationEnd),
     tap(() => {
@@ -115,10 +157,13 @@ export class TraceService implements OnDestroy {
   );
 
   private _routingSpan: Span | null = null;
+  private _activeTransaction?: Transaction;
+
   private _subscription: Subscription = new Subscription();
 
   public constructor(private readonly _router: Router) {
     this._subscription.add(this.navStart$.subscribe());
+    this._subscription.add(this.resEnd$.subscribe());
     this._subscription.add(this.navEnd$.subscribe());
   }
 
@@ -241,3 +286,24 @@ export function TraceMethodDecorator(): MethodDecorator {
     return descriptor;
   };
 }
+
+function getParamsOfRoute(activatedRouteSnapshot: ActivatedRouteSnapshot): Params {
+  let params = {};
+  const stack: ActivatedRouteSnapshot[] = [activatedRouteSnapshot];
+  while (stack.length > 0) {
+    const route = stack.pop();
+    params = { ...params, ...(route && route.params) };
+    route && stack.push(...route.children);
+  }
+  return params;
+}
+
+function getParameterizedRouteFromUrlAndParams(url: string, params: Params): string {
+  if (params && typeof params === 'object') {
+    const parameterized = Object.keys(params).reduce((prev, curr: string) => {
+      return prev.replace(params[curr], `:${curr}`);
+    }, url);
+    return parameterized;
+  }
+  return url;
+}

packages/angular/test/tracing.test.ts

@@ -1,10 +1,11 @@
-import { NavigationStart, Router, RouterEvent } from '@angular/router';
+import { Event, NavigationStart, ResolveEnd, Router } from '@angular/router';
 import { Subject } from 'rxjs';
 
 import { instrumentAngularRouting, TraceService } from '../src/index';
 
 describe('Angular Tracing', () => {
   const startTransaction = jest.fn();
+
   describe('instrumentAngularRouting', () => {
     it('should attach the transaction source on the pageload transaction', () => {
       instrumentAngularRouting(startTransaction);
@@ -18,15 +19,15 @@ describe('Angular Tracing', () => {
 
   describe('TraceService', () => {
     let traceService: TraceService;
-    const routerEvents$: Subject<RouterEvent> = new Subject();
+    const routerEvents$: Subject<Event> = new Subject();
+    const mockedRouter: Partial<Router> = {
+      events: routerEvents$,
+    };
 
-    beforeAll(() => instrumentAngularRouting(startTransaction));
     beforeEach(() => {
+      instrumentAngularRouting(startTransaction);
       jest.resetAllMocks();
-
-      traceService = new TraceService({
-        events: routerEvents$,
-      } as unknown as Router);
+      traceService = new TraceService(mockedRouter as Router);
     });
 
     afterEach(() => {
@@ -43,5 +44,96 @@ describe('Angular Tracing', () => {
         metadata: { source: 'url' },
       });
     });
+
+    describe('URL parameterization', () => {
+      // TODO: These tests are real unit tests in the sense that they only test TraceService
+      //       and we essentially just simulate a router navigation by firing the respective
+      //       routing events and providing the raw URL + the resolved route parameters.
+      //       In the future we should add more "wholesome" tests that let the Angular router
+      //       do its thing (e.g. by calling router.navigate) and we check how our service
+      //       reacts to it.
+      //       Once we set up Jest for testing Angular, we can use TestBed to inject an actual
+      //       router instance into TraceService and add more tests.
+
+      let transaction: any;
+      let customStartTransaction: any;
+      beforeEach(() => {
+        transaction = {
+          setName: jest.fn(name => (transaction.name = name)),
+          setMetadata: jest.fn(metadata => (transaction.metadata = metadata)),
+        };
+
+        customStartTransaction = jest.fn((ctx: any) => {
+          transaction.name = ctx.name;
+          transaction.op = ctx.op;
+          transaction.metadata = ctx.metadata;
+          return transaction;
+        });
+      });
+
+      it.each([
+        ['does not alter static routes', '/books/', {}, '/books/'],
+        ['parameterizes number IDs in the URL', '/books/1/details', { bookId: '1' }, '/books/:bookId/details'],
+        [
+          'parameterizes string IDs in the URL',
+          '/books/asd123/details',
+          { bookId: 'asd123' },
+          '/books/:bookId/details',
+        ],
+        [
+          'parameterizes UUID4 IDs in the URL',
+          '/books/04bc6846-4a1e-4af5-984a-003258f33e31/details',
+          { bookId: '04bc6846-4a1e-4af5-984a-003258f33e31' },
+          '/books/:bookId/details',
+        ],
+        [
+          'parameterizes multiple IDs in the URL',
+          '/org/sentry/projects/1234/events/04bc6846-4a1e-4af5-984a-003258f33e31',
+          { orgId: 'sentry', projId: '1234', eventId: '04bc6846-4a1e-4af5-984a-003258f33e31' },
+          '/org/:orgId/projects/:projId/events/:eventId',
+        ],
+      ])('%s and sets the source to `route`', (_, url, params, result) => {
+        instrumentAngularRouting(customStartTransaction);
+
+        // this event starts the transaction
+        routerEvents$.next(new NavigationStart(0, url));
+
+        expect(customStartTransaction).toHaveBeenCalledWith({
+          name: url,
+          op: 'navigation',
+          metadata: { source: 'url' },
+        });
+
+        // this event starts the parameterization
+        routerEvents$.next(new ResolveEnd(1, url, url, { root: { params, children: [] } } as any));
+
+        expect(transaction.setName).toHaveBeenCalledWith(result);
+        expect(transaction.setMetadata).toHaveBeenCalledWith({ source: 'route' });
+      });
+
+      it('does not change the transaction name if the source is something other than `url`', () => {
+        instrumentAngularRouting(customStartTransaction);
+
+        const url = '/user/12345/test';
+
+        routerEvents$.next(new NavigationStart(0, url));
+
+        expect(customStartTransaction).toHaveBeenCalledWith({
+          name: url,
+          op: 'navigation',
+          metadata: { source: 'url' },
+        });
+
+        // Simulate that this transaction has a custom name:
+        transaction.metadata.source = 'custom';
+
+        // this event starts the parameterization
+        routerEvents$.next(new ResolveEnd(1, url, url, { root: { params: { userId: '12345' }, children: [] } } as any));
+
+        expect(transaction.setName).toHaveBeenCalledTimes(0);
+        expect(transaction.setMetadata).toHaveBeenCalledTimes(0);
+        expect(transaction.name).toEqual(url);
+      });
+    });
   });
 });