Skip to content

Commit

Permalink
fix(angular): Fall back to element tagName when name is not provide…
Browse files Browse the repository at this point in the history
…d 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 <[email protected]>
  • Loading branch information
arturovt and Lms24 authored Dec 23, 2024
1 parent 4443cb7 commit dc08b82
Show file tree
Hide file tree
Showing 6 changed files with 86 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ import { SampleComponent } from '../sample-component/sample-component.components
selector: 'app-cancel',
standalone: true,
imports: [TraceModule, SampleComponent],
template: `<app-sample-component [trace]="'sample-component'"></app-sample-component>`,
template: `
<app-sample-component [trace]="'sample-component'"></app-sample-component>
<app-sample-component trace></app-sample-component>
`,
})
@TraceClass({ name: 'ComponentTrackingComponent' })
export class ComponentTrackingComponent implements OnInit, AfterViewInit {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
});
Expand All @@ -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: '<sample-component>',
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: '<sample-component>', // 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: '<app-sample-component>', // fallback selector name
op: 'ui.angular.init',
origin: 'auto.ui.angular.trace_directive',
start_timestamp: expect.any(Number),
timestamp: expect.any(Number),
}),
]),
);
});
});
Expand Down
2 changes: 2 additions & 0 deletions dev-packages/e2e-tests/test-applications/angular-19/.npmrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
@sentry:registry=http://127.0.0.1:4873
@sentry-internal:registry=http://127.0.0.1:4873
Original file line number Diff line number Diff line change
Expand Up @@ -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: `<app-sample-component [trace]="'sample-component'"></app-sample-component>`,
template: `
<app-sample-component trace="sample-component"></app-sample-component>
<app-sample-component trace></app-sample-component>
`,
})
@TraceClass({ name: 'ComponentTrackingComponent' })
export class ComponentTrackingComponent implements OnInit, AfterViewInit {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
});
Expand All @@ -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: '<sample-component>',
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: '<sample-component>', // 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: '<app-sample-component>', // fallback selector name
op: 'ui.angular.init',
origin: 'auto.ui.angular.trace_directive',
start_timestamp: expect.any(Number),
timestamp: expect.any(Number),
}),
]),
);
});
});
Expand Down
23 changes: 19 additions & 4 deletions packages/angular/src/tracing.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -235,24 +237,37 @@ 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
* <app-my-component trace="myComponent"></app-my-component>
* ```
*/
@Directive({ selector: '[trace]' })
export class TraceDirective implements OnInit, AfterViewInit {
@Input('trace') public componentName?: string;

private _tracingSpan?: Span;

public constructor(private readonly _host: ElementRef<HTMLElement>) {}

/**
* 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., `<app-component trace />`), we fall back to using `tagName`
// (which is e.g. `APP-COMPONENT`).
this.componentName = this._host.nativeElement.tagName.toLowerCase();
}

if (getActiveSpan()) {
Expand Down

0 comments on commit dc08b82

Please sign in to comment.