From 22a571535787fd589262bd4ef90b0fe0b201aec7 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 25 Jan 2024 14:45:38 -0500 Subject: [PATCH 1/7] feat(angular): Export custom `browserTracingIntegration()` Also deprecate the routing Instrumentation. --- packages/angular-ivy/README.md | 10 ++-- packages/angular/README.md | 8 ++- packages/angular/src/index.ts | 4 +- packages/angular/src/tracing.ts | 77 ++++++++++++++++++++++++--- packages/angular/test/tracing.test.ts | 2 + packages/angular/test/utils/index.ts | 1 + 6 files changed, 84 insertions(+), 18 deletions(-) diff --git a/packages/angular-ivy/README.md b/packages/angular-ivy/README.md index 6967e7570a82..438bb0fb5ff5 100644 --- 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 index 302b060bdb39..a3e15a426196 100644 --- 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 index f7f0536463a2..a2b1195c4e3c 100644 --- 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 index efd2c840420b..790eddf27e56 100644 --- a/packages/angular/src/tracing.ts +++ b/packages/angular/src/tracing.ts @@ -7,9 +7,15 @@ 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, + disableDefaultBrowserTracingNavigationSpan, + getCurrentScope, + startBrowserTracingNavigationSpan, +} from '@sentry/browser'; +import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, getActiveSpan, 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 +29,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 +57,30 @@ 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[0], +): Integration { + instrumentationInitialized = true; + hooksBasedInstrumentation = true; + + disableDefaultBrowserTracingNavigationSpan(); + + return originalBrowserTracingIntegration(options); +} + /** * Grabs active transaction off scope. * @@ -74,7 +106,43 @@ export class TraceService implements OnDestroy { return; } + if (this._routingSpan) { + this._routingSpan.end(); + this._routingSpan = null; + } + const strippedUrl = stripUrlQueryAndFragment(navigationEvent.url); + + if (hooksBasedInstrumentation) { + if (!getActiveSpan()) { + startBrowserTracingNavigationSpan({ + 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 +158,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}`, diff --git a/packages/angular/test/tracing.test.ts b/packages/angular/test/tracing.test.ts index c2406f628128..31bd13473253 100644 --- 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 index 83a416ca2a03..390d7fbe14ac 100644 --- 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 { + // eslint-disable-next-line deprecation/deprecation instrumentAngularRouting( conf.customStartTransaction || jest.fn(), conf.startTransactionOnPageLoad !== undefined ? conf.startTransactionOnPageLoad : true, From debc9d1e0a4de9d20e5e1b93ba24f313702c9e4e Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 30 Jan 2024 10:48:27 +0100 Subject: [PATCH 2/7] ref: changes to browser tracing --- packages/angular/src/tracing.ts | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/packages/angular/src/tracing.ts b/packages/angular/src/tracing.ts index 790eddf27e56..b813cbfb2be4 100644 --- a/packages/angular/src/tracing.ts +++ b/packages/angular/src/tracing.ts @@ -10,7 +10,6 @@ import { NavigationEnd, NavigationStart, ResolveEnd } from '@angular/router'; import { WINDOW, browserTracingIntegration as originalBrowserTracingIntegration, - disableDefaultBrowserTracingNavigationSpan, getCurrentScope, startBrowserTracingNavigationSpan, } from '@sentry/browser'; @@ -71,14 +70,19 @@ export const instrumentAngularRouting = routingInstrumentation; * Use this integration in combination with `TraceService` */ export function browserTracingIntegration( - options?: Parameters[0], + options: Parameters[0] = {}, ): Integration { - instrumentationInitialized = true; - hooksBasedInstrumentation = true; - - disableDefaultBrowserTracingNavigationSpan(); + // 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); + return originalBrowserTracingIntegration({ + ...options, + instrumentNavigation: false, + }); } /** From 471785bd0649fa64181853f1eca5b05946a1a0ba Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 30 Jan 2024 15:28:46 +0100 Subject: [PATCH 3/7] fix --- packages/angular/src/tracing.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/packages/angular/src/tracing.ts b/packages/angular/src/tracing.ts index b813cbfb2be4..a194bc2424cf 100644 --- a/packages/angular/src/tracing.ts +++ b/packages/angular/src/tracing.ts @@ -13,7 +13,13 @@ import { getCurrentScope, startBrowserTracingNavigationSpan, } from '@sentry/browser'; -import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, getActiveSpan, spanToJSON, startInactiveSpan } from '@sentry/core'; +import { + 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'; @@ -115,11 +121,12 @@ export class TraceService implements OnDestroy { this._routingSpan = null; } + const client = getClient(); const strippedUrl = stripUrlQueryAndFragment(navigationEvent.url); - if (hooksBasedInstrumentation) { + if (client && hooksBasedInstrumentation) { if (!getActiveSpan()) { - startBrowserTracingNavigationSpan({ + startBrowserTracingNavigationSpan(client, { name: strippedUrl, op: 'navigation', origin: 'auto.navigation.angular', From df42b73ff9471e0cab649358f67c74a06af7dd69 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 31 Jan 2024 10:11:41 +0100 Subject: [PATCH 4/7] update e2e test --- .../e2e-tests/test-applications/angular-17/src/main.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) 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 index 7732c602bb28..761a7329a91f 100644 --- 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, }); From f6f70baa86e4ac96a47e7c9638b3b1b0cff75896 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 31 Jan 2024 10:38:53 +0100 Subject: [PATCH 5/7] fix tracing --- .../src/browser/browserTracingIntegration.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/tracing-internal/src/browser/browserTracingIntegration.ts b/packages/tracing-internal/src/browser/browserTracingIntegration.ts index aaf30e7e6a63..31660eff00a7 100644 --- 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 Date: Wed, 31 Jan 2024 11:09:31 +0100 Subject: [PATCH 6/7] update origin for spans --- packages/angular/src/tracing.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/angular/src/tracing.ts b/packages/angular/src/tracing.ts index a194bc2424cf..b52d51e405a6 100644 --- a/packages/angular/src/tracing.ts +++ b/packages/angular/src/tracing.ts @@ -14,6 +14,7 @@ import { startBrowserTracingNavigationSpan, } from '@sentry/browser'; import { + SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, getActiveSpan, getClient, @@ -208,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`); } }), ); From 4d3828bcce07af09453aec9207d92851d78718db Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 31 Jan 2024 12:17:52 +0100 Subject: [PATCH 7/7] fix it --- packages/angular/src/tracing.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/angular/src/tracing.ts b/packages/angular/src/tracing.ts index b52d51e405a6..5b2f74615c45 100644 --- a/packages/angular/src/tracing.ts +++ b/packages/angular/src/tracing.ts @@ -81,7 +81,7 @@ export function browserTracingIntegration( ): 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) { + if (options.instrumentNavigation !== false) { instrumentationInitialized = true; hooksBasedInstrumentation = true; }