From 222c761f60500a8e95b9a70b887fe828a6730fbe Mon Sep 17 00:00:00 2001 From: Artur Date: Mon, 23 Dec 2024 12:02:04 +0000 Subject: [PATCH 1/2] fix(angular): Fall back to element `tagName` when name is not provided to `TraceDirective` (#14778) The `trace` directive should typically be declared on components to validly trace the lifecycle (from `ngOnInit` to `ngAfterViewInit`, when child views are also rendered). If `trace` is mistakenly not provided, we fall back to `tagName` instead of "unknown component". --------- Co-authored-by: Lukas Stracke --- .../component-tracking.components.ts | 5 ++- .../angular-17/tests/performance.test.ts | 43 ++++++++++++------- .../test-applications/angular-19/.npmrc | 2 + .../component-tracking.components.ts | 7 ++- .../angular-19/tests/performance.test.ts | 43 ++++++++++++------- packages/angular/src/tracing.ts | 23 ++++++++-- 6 files changed, 86 insertions(+), 37 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/angular-19/.npmrc diff --git a/dev-packages/e2e-tests/test-applications/angular-17/src/app/component-tracking/component-tracking.components.ts b/dev-packages/e2e-tests/test-applications/angular-17/src/app/component-tracking/component-tracking.components.ts index d437a1d43fdd..1e43d5c6c096 100644 --- a/dev-packages/e2e-tests/test-applications/angular-17/src/app/component-tracking/component-tracking.components.ts +++ b/dev-packages/e2e-tests/test-applications/angular-17/src/app/component-tracking/component-tracking.components.ts @@ -6,7 +6,10 @@ import { SampleComponent } from '../sample-component/sample-component.components selector: 'app-cancel', standalone: true, imports: [TraceModule, SampleComponent], - template: ``, + template: ` + + + `, }) @TraceClass({ name: 'ComponentTrackingComponent' }) export class ComponentTrackingComponent implements OnInit, AfterViewInit { diff --git a/dev-packages/e2e-tests/test-applications/angular-17/tests/performance.test.ts b/dev-packages/e2e-tests/test-applications/angular-17/tests/performance.test.ts index 29c88a6108e2..03a715ce646c 100644 --- a/dev-packages/e2e-tests/test-applications/angular-17/tests/performance.test.ts +++ b/dev-packages/e2e-tests/test-applications/angular-17/tests/performance.test.ts @@ -191,7 +191,7 @@ test.describe('finish routing span', () => { }); test.describe('TraceDirective', () => { - test('creates a child tracingSpan with component name as span name on ngOnInit', async ({ page }) => { + test('creates a child span with the component name as span name on ngOnInit', async ({ page }) => { const navigationTxnPromise = waitForTransaction('angular-17', async transactionEvent => { return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'navigation'; }); @@ -201,23 +201,36 @@ test.describe('TraceDirective', () => { // immediately navigate to a different route const [_, navigationTxn] = await Promise.all([page.locator('#componentTracking').click(), navigationTxnPromise]); - const traceDirectiveSpan = navigationTxn.spans?.find( + const traceDirectiveSpans = navigationTxn.spans?.filter( span => span?.data && span?.data[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN] === 'auto.ui.angular.trace_directive', ); - expect(traceDirectiveSpan).toBeDefined(); - expect(traceDirectiveSpan).toEqual( - expect.objectContaining({ - data: { - [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'ui.angular.init', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.angular.trace_directive', - }, - description: '', - op: 'ui.angular.init', - origin: 'auto.ui.angular.trace_directive', - start_timestamp: expect.any(Number), - timestamp: expect.any(Number), - }), + expect(traceDirectiveSpans).toHaveLength(2); + expect(traceDirectiveSpans).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + data: { + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'ui.angular.init', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.angular.trace_directive', + }, + description: '', // custom component name passed to trace directive + op: 'ui.angular.init', + origin: 'auto.ui.angular.trace_directive', + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + }), + expect.objectContaining({ + data: { + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'ui.angular.init', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.angular.trace_directive', + }, + description: '', // fallback selector name + op: 'ui.angular.init', + origin: 'auto.ui.angular.trace_directive', + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + }), + ]), ); }); }); diff --git a/dev-packages/e2e-tests/test-applications/angular-19/.npmrc b/dev-packages/e2e-tests/test-applications/angular-19/.npmrc new file mode 100644 index 000000000000..070f80f05092 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/angular-19/.npmrc @@ -0,0 +1,2 @@ +@sentry:registry=http://127.0.0.1:4873 +@sentry-internal:registry=http://127.0.0.1:4873 diff --git a/dev-packages/e2e-tests/test-applications/angular-19/src/app/component-tracking/component-tracking.components.ts b/dev-packages/e2e-tests/test-applications/angular-19/src/app/component-tracking/component-tracking.components.ts index d437a1d43fdd..a82e5b1acce6 100644 --- a/dev-packages/e2e-tests/test-applications/angular-19/src/app/component-tracking/component-tracking.components.ts +++ b/dev-packages/e2e-tests/test-applications/angular-19/src/app/component-tracking/component-tracking.components.ts @@ -3,10 +3,13 @@ import { TraceClass, TraceMethod, TraceModule } from '@sentry/angular'; import { SampleComponent } from '../sample-component/sample-component.components'; @Component({ - selector: 'app-cancel', + selector: 'app-component-tracking', standalone: true, imports: [TraceModule, SampleComponent], - template: ``, + template: ` + + + `, }) @TraceClass({ name: 'ComponentTrackingComponent' }) export class ComponentTrackingComponent implements OnInit, AfterViewInit { diff --git a/dev-packages/e2e-tests/test-applications/angular-19/tests/performance.test.ts b/dev-packages/e2e-tests/test-applications/angular-19/tests/performance.test.ts index af85b8ffc405..c2cb2eca34b6 100644 --- a/dev-packages/e2e-tests/test-applications/angular-19/tests/performance.test.ts +++ b/dev-packages/e2e-tests/test-applications/angular-19/tests/performance.test.ts @@ -191,7 +191,7 @@ test.describe('finish routing span', () => { }); test.describe('TraceDirective', () => { - test('creates a child tracingSpan with component name as span name on ngOnInit', async ({ page }) => { + test('creates a child span with the component name as span name on ngOnInit', async ({ page }) => { const navigationTxnPromise = waitForTransaction('angular-18', async transactionEvent => { return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'navigation'; }); @@ -201,23 +201,36 @@ test.describe('TraceDirective', () => { // immediately navigate to a different route const [_, navigationTxn] = await Promise.all([page.locator('#componentTracking').click(), navigationTxnPromise]); - const traceDirectiveSpan = navigationTxn.spans?.find( + const traceDirectiveSpans = navigationTxn.spans?.filter( span => span?.data && span?.data[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN] === 'auto.ui.angular.trace_directive', ); - expect(traceDirectiveSpan).toBeDefined(); - expect(traceDirectiveSpan).toEqual( - expect.objectContaining({ - data: { - [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'ui.angular.init', - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.angular.trace_directive', - }, - description: '', - op: 'ui.angular.init', - origin: 'auto.ui.angular.trace_directive', - start_timestamp: expect.any(Number), - timestamp: expect.any(Number), - }), + expect(traceDirectiveSpans).toHaveLength(2); + expect(traceDirectiveSpans).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + data: { + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'ui.angular.init', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.angular.trace_directive', + }, + description: '', // custom component name passed to trace directive + op: 'ui.angular.init', + origin: 'auto.ui.angular.trace_directive', + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + }), + expect.objectContaining({ + data: { + [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'ui.angular.init', + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.angular.trace_directive', + }, + description: '', // fallback selector name + op: 'ui.angular.init', + origin: 'auto.ui.angular.trace_directive', + start_timestamp: expect.any(Number), + timestamp: expect.any(Number), + }), + ]), ); }); }); diff --git a/packages/angular/src/tracing.ts b/packages/angular/src/tracing.ts index c347a5e19b2e..a5b9391e6ee4 100644 --- a/packages/angular/src/tracing.ts +++ b/packages/angular/src/tracing.ts @@ -1,3 +1,5 @@ +// eslint-disable-next-line @typescript-eslint/consistent-type-imports +import { ElementRef } from '@angular/core'; import type { AfterViewInit, OnDestroy, OnInit } from '@angular/core'; import { Directive, Injectable, Input, NgModule } from '@angular/core'; import type { ActivatedRouteSnapshot, Event, RouterState } from '@angular/router'; @@ -235,10 +237,17 @@ export class TraceService implements OnDestroy { } } -const UNKNOWN_COMPONENT = 'unknown'; - /** - * A directive that can be used to capture initialization lifecycle of the whole component. + * Captures the initialization lifecycle of the component this directive is applied to. + * Specifically, this directive measures the time between `ngOnInit` and `ngAfterViewInit` + * of the component. + * + * Falls back to the component's selector if no name is provided. + * + * @example + * ```html + * + * ``` */ @Directive({ selector: '[trace]' }) export class TraceDirective implements OnInit, AfterViewInit { @@ -246,13 +255,19 @@ export class TraceDirective implements OnInit, AfterViewInit { private _tracingSpan?: Span; + public constructor(private readonly _host: ElementRef) {} + /** * Implementation of OnInit lifecycle method * @inheritdoc */ public ngOnInit(): void { if (!this.componentName) { - this.componentName = UNKNOWN_COMPONENT; + // Technically, the `trace` binding should always be provided. + // However, if it is incorrectly declared on the element without a + // value (e.g., ``), we fall back to using `tagName` + // (which is e.g. `APP-COMPONENT`). + this.componentName = this._host.nativeElement.tagName.toLowerCase(); } if (getActiveSpan()) { From 2a4f6ce26902df8d20de63839ba8988cc8015208 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 23 Dec 2024 14:20:12 +0100 Subject: [PATCH 2/2] changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index eac3aa34e426..bf80db840060 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott +Work in this release was contributed by @arturovt. Thank you for your contribution! + ## 8.47.0 - feat(v8/core): Add `updateSpanName` helper function (#14736)