feat(angular): Export custom browserTracingIntegration() (#10353)

mydea · web-flow · commit 5fd5c5dd265f · 2024-01-31T13:03:22.000+01:00
Also deprecate the routing Instrumentation. This is WIP on top of #10351, to show how that would work. No idea how to get the tests working for this, though...
diff --git a/dev-packages/e2e-tests/test-applications/angular-17/src/main.ts b/dev-packages/e2e-tests/test-applications/angular-17/src/main.ts
@@ -7,11 +7,7 @@ import * as Sentry from '@sentry/angular-ivy';
 Sentry.init({
   dsn: 'https://3b6c388182fb435097f41d181be2b2ba@o4504321058471936.ingest.sentry.io/4504321066008576',
   tracesSampleRate: 1.0,
-  integrations: [
-    new Sentry.BrowserTracing({
-      routingInstrumentation: Sentry.routingInstrumentation,
-    }),
-  ],
+  integrations: [Sentry.browserTracingIntegration({})],
   tunnel: `http://localhost:3031/`, // proxy server
   debug: true,
 });
diff --git a/packages/angular-ivy/README.md b/packages/angular-ivy/README.md
@@ -93,16 +93,14 @@ Registering a Trace Service is a 3-step process.
    instrumentation:
 
 ```javascript
-import { init, instrumentAngularRouting, BrowserTracing } from '@sentry/angular-ivy';
+import { init, browserTracingIntegration } from '@sentry/angular-ivy';
 
 init({
   dsn: '__DSN__',
-  integrations: [
-    new BrowserTracing({
-      tracingOrigins: ['localhost', 'https://yourserver.io/api'],
-      routingInstrumentation: instrumentAngularRouting,
-    }),
+ integrations: [
+    browserTracingIntegration(),
   ],
+  tracePropagationTargets: ['localhost', 'https://yourserver.io/api'],
   tracesSampleRate: 1,
 });
 ```
diff --git a/packages/angular/README.md b/packages/angular/README.md
@@ -93,16 +93,14 @@ Registering a Trace Service is a 3-step process.
    instrumentation:
 
 ```javascript
-import { init, instrumentAngularRouting, BrowserTracing } from '@sentry/angular';
+import { init, browserTracingIntegration } from '@sentry/angular';
 
 init({
   dsn: '__DSN__',
   integrations: [
-    new BrowserTracing({
-      tracingOrigins: ['localhost', 'https://yourserver.io/api'],
-      routingInstrumentation: instrumentAngularRouting,
-    }),
+    browserTracingIntegration(),
   ],
+  tracePropagationTargets: ['localhost', 'https://yourserver.io/api'],
   tracesSampleRate: 1,
 });
 ```
diff --git a/packages/angular/src/index.ts b/packages/angular/src/index.ts
@@ -7,9 +7,11 @@ export { createErrorHandler, SentryErrorHandler } from './errorhandler';
 export {
   // eslint-disable-next-line deprecation/deprecation
   getActiveTransaction,
-  // TODO `instrumentAngularRouting` is just an alias for `routingInstrumentation`; deprecate the latter at some point
+  // eslint-disable-next-line deprecation/deprecation
   instrumentAngularRouting, // new name
+  // eslint-disable-next-line deprecation/deprecation
   routingInstrumentation, // legacy name
+  browserTracingIntegration,
   TraceClassDecorator,
   TraceMethodDecorator,
   TraceDirective,
diff --git a/packages/angular/src/tracing.ts b/packages/angular/src/tracing.ts
@@ -7,9 +7,21 @@ import type { ActivatedRouteSnapshot, Event, RouterState } from '@angular/router
 // eslint-disable-next-line @typescript-eslint/consistent-type-imports
 import { NavigationCancel, NavigationError, Router } from '@angular/router';
 import { NavigationEnd, NavigationStart, ResolveEnd } from '@angular/router';
-import { WINDOW, getCurrentScope } from '@sentry/browser';
-import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, spanToJSON } from '@sentry/core';
-import type { Span, Transaction, TransactionContext } from '@sentry/types';
+import {
+  WINDOW,
+  browserTracingIntegration as originalBrowserTracingIntegration,
+  getCurrentScope,
+  startBrowserTracingNavigationSpan,
+} from '@sentry/browser';
+import {
+  SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
+  SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
+  getActiveSpan,
+  getClient,
+  spanToJSON,
+  startInactiveSpan,
+} from '@sentry/core';
+import type { Integration, Span, Transaction, TransactionContext } from '@sentry/types';
 import { logger, stripUrlQueryAndFragment, timestampInSeconds } from '@sentry/utils';
 import type { Observable } from 'rxjs';
 import { Subscription } from 'rxjs';
@@ -23,8 +35,12 @@ let instrumentationInitialized: boolean;
 let stashedStartTransaction: (context: TransactionContext) => Transaction | undefined;
 let stashedStartTransactionOnLocationChange: boolean;
 
+let hooksBasedInstrumentation = false;
+
 /**
  * Creates routing instrumentation for Angular Router.
+ *
+ * @deprecated Use `browserTracingIntegration()` instead, which includes Angular-specific instrumentation out of the box.
  */
 export function routingInstrumentation(
   customStartTransaction: (context: TransactionContext) => Transaction | undefined,
@@ -47,8 +63,35 @@ export function routingInstrumentation(
   }
 }
 
+/**
+ * Creates routing instrumentation for Angular Router.
+ *
+ * @deprecated Use `browserTracingIntegration()` instead, which includes Angular-specific instrumentation out of the box.
+ */
+// eslint-disable-next-line deprecation/deprecation
 export const instrumentAngularRouting = routingInstrumentation;
 
+/**
+ * A custom BrowserTracing integration for Angular.
+ *
+ * Use this integration in combination with `TraceService`
+ */
+export function browserTracingIntegration(
+  options: Parameters<typeof originalBrowserTracingIntegration>[0] = {},
+): Integration {
+  // If the user opts out to set this up, we just don't initialize this.
+  // That way, the TraceService will not actually do anything, functionally disabling this.
+  if (options.instrumentNavigation !== false) {
+    instrumentationInitialized = true;
+    hooksBasedInstrumentation = true;
+  }
+
+  return originalBrowserTracingIntegration({
+    ...options,
+    instrumentNavigation: false,
+  });
+}
+
 /**
  * Grabs active transaction off scope.
  *
@@ -74,7 +117,44 @@ export class TraceService implements OnDestroy {
         return;
       }
 
+      if (this._routingSpan) {
+        this._routingSpan.end();
+        this._routingSpan = null;
+      }
+
+      const client = getClient();
       const strippedUrl = stripUrlQueryAndFragment(navigationEvent.url);
+
+      if (client && hooksBasedInstrumentation) {
+        if (!getActiveSpan()) {
+          startBrowserTracingNavigationSpan(client, {
+            name: strippedUrl,
+            op: 'navigation',
+            origin: 'auto.navigation.angular',
+            attributes: {
+              [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
+            },
+          });
+        }
+
+        // eslint-disable-next-line deprecation/deprecation
+        this._routingSpan =
+          startInactiveSpan({
+            name: `${navigationEvent.url}`,
+            op: ANGULAR_ROUTING_OP,
+            origin: 'auto.ui.angular',
+            tags: {
+              'routing.instrumentation': '@sentry/angular',
+              url: strippedUrl,
+              ...(navigationEvent.navigationTrigger && {
+                navigationTrigger: navigationEvent.navigationTrigger,
+              }),
+            },
+          }) || null;
+
+        return;
+      }
+
       // eslint-disable-next-line deprecation/deprecation
       let activeTransaction = getActiveTransaction();
 
@@ -90,9 +170,6 @@ export class TraceService implements OnDestroy {
       }
 
       if (activeTransaction) {
-        if (this._routingSpan) {
-          this._routingSpan.end();
-        }
         // eslint-disable-next-line deprecation/deprecation
         this._routingSpan = activeTransaction.startChild({
           description: `${navigationEvent.url}`,
@@ -132,6 +209,7 @@ export class TraceService implements OnDestroy {
       if (transaction && attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] === 'url') {
         transaction.updateName(route);
         transaction.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route');
+        transaction.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, `auto.${spanToJSON(transaction).op}.angular`);
       }
     }),
   );
diff --git a/packages/angular/test/tracing.test.ts b/packages/angular/test/tracing.test.ts
@@ -44,6 +44,7 @@ describe('Angular Tracing', () => {
     transaction = undefined;
   });
 
+  /* eslint-disable deprecation/deprecation */
   describe('instrumentAngularRouting', () => {
     it('should attach the transaction source on the pageload transaction', () => {
       const startTransaction = jest.fn();
@@ -57,6 +58,7 @@ describe('Angular Tracing', () => {
       });
     });
   });
+  /* eslint-enable deprecation/deprecation */
 
   describe('getParameterizedRouteFromSnapshot', () => {
     it.each([
diff --git a/packages/angular/test/utils/index.ts b/packages/angular/test/utils/index.ts
@@ -50,6 +50,7 @@ export class TestEnv {
     useTraceService?: boolean;
     additionalProviders?: Provider[];
   }): Promise<TestEnv> {
+    // eslint-disable-next-line deprecation/deprecation
     instrumentAngularRouting(
       conf.customStartTransaction || jest.fn(),
       conf.startTransactionOnPageLoad !== undefined ? conf.startTransactionOnPageLoad : true,
diff --git a/packages/tracing-internal/src/browser/browserTracingIntegration.ts b/packages/tracing-internal/src/browser/browserTracingIntegration.ts
@@ -336,7 +336,9 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
           startTimestamp: browserPerformanceTimeOrigin ? browserPerformanceTimeOrigin / 1000 : undefined,
           op: 'pageload',
           origin: 'auto.pageload.browser',
-          metadata: { source: 'url' },
+          attributes: {
+            [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
+          },
         };
         startBrowserTracingPageLoadSpan(client, context);
       }
@@ -363,7 +365,9 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
               name: WINDOW.location.pathname,
               op: 'navigation',
               origin: 'auto.navigation.browser',
-              metadata: { source: 'url' },
+              attributes: {
+                [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
+              },
             };
 
             startBrowserTracingNavigationSpan(client, context);
