Skip to content

Commit 2274b27

Browse files
authored
ref(v8/angular): Remove instrumentAngularRouting and fix tests (#11021)
ref #10898 ref #10353
1 parent 0be633a commit 2274b27

File tree

11 files changed

+302
-656
lines changed

11 files changed

+302
-656
lines changed

dev-packages/e2e-tests/test-applications/angular-17/src/app/app.routes.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
import { Routes } from '@angular/router';
2+
import { cancelGuard } from './cancel-guard.guard';
3+
import { CancelComponent } from './cancel/cancel.components';
4+
import { ComponentTrackingComponent } from './component-tracking/component-tracking.components';
25
import { HomeComponent } from './home/home.component';
36
import { UserComponent } from './user/user.component';
47

@@ -11,6 +14,15 @@ export const routes: Routes = [
1114
path: 'home',
1215
component: HomeComponent,
1316
},
17+
{
18+
path: 'cancel',
19+
component: CancelComponent,
20+
canActivate: [cancelGuard],
21+
},
22+
{
23+
path: 'component-tracking',
24+
component: ComponentTrackingComponent,
25+
},
1426
{
1527
path: 'redirect1',
1628
redirectTo: '/redirect2',
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import { ActivatedRouteSnapshot, CanActivateFn, RouterStateSnapshot } from '@angular/router';
2+
3+
export const cancelGuard: CanActivateFn = (_next: ActivatedRouteSnapshot, _state: RouterStateSnapshot) => {
4+
return false;
5+
};
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
import { Component } from '@angular/core';
2+
3+
@Component({
4+
selector: 'app-cancel',
5+
standalone: true,
6+
template: `<div></div>`,
7+
})
8+
export class CancelComponent {}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import { AfterViewInit, Component, OnInit } from '@angular/core';
2+
import { TraceClassDecorator, TraceMethodDecorator, TraceModule } from '@sentry/angular-ivy';
3+
import { SampleComponent } from '../sample-component/sample-component.components';
4+
5+
@Component({
6+
selector: 'app-cancel',
7+
standalone: true,
8+
imports: [TraceModule, SampleComponent],
9+
template: `<app-sample-component [trace]="'sample-component'"></app-sample-component>`,
10+
})
11+
@TraceClassDecorator()
12+
export class ComponentTrackingComponent implements OnInit, AfterViewInit {
13+
@TraceMethodDecorator()
14+
ngOnInit() {}
15+
16+
@TraceMethodDecorator()
17+
ngAfterViewInit() {}
18+
}

dev-packages/e2e-tests/test-applications/angular-17/src/app/home/home.component.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ import { RouterLink } from '@angular/router';
1111
<ul>
1212
<li> <a id="navLink" [routerLink]="['/users', '123']">Visit User 123</a> </li>
1313
<li> <a id="redirectLink" [routerLink]="['/redirect1']">Redirect</a> </li>
14+
<li> <a id="cancelLink" [routerLink]="['/cancel']">Cancel</a> </li>
15+
<li> <a id="nonExistentLink" [routerLink]="['/non-existent']">Error</a> </li>
16+
<li> <a id="componentTracking" [routerLink]="['/component-tracking']">Error</a> </li>
1417
</ul>
1518
<button id="errorBtn" (click)="throwError()">Throw error</button>
1619
</main>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import { Component, OnInit } from '@angular/core';
2+
3+
@Component({
4+
selector: 'app-sample-component',
5+
standalone: true,
6+
template: `<div></div>`,
7+
})
8+
export class SampleComponent implements OnInit {
9+
ngOnInit() {
10+
console.log('SampleComponent');
11+
}
12+
}

dev-packages/e2e-tests/test-applications/angular-17/tests/performance.test.ts

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { expect, test } from '@playwright/test';
2+
import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core';
23
import { waitForTransaction } from '../event-proxy-server';
34

45
test('sends a pageload transaction with a parameterized URL', async ({ page }) => {
@@ -126,3 +127,178 @@ test('groups redirects within one navigation root span', async ({ page }) => {
126127
expect(routingSpan).toBeDefined();
127128
expect(routingSpan?.description).toBe('/redirect1');
128129
});
130+
131+
test.describe('finish routing span', () => {
132+
test('finishes routing span on navigation cancel', async ({ page }) => {
133+
const navigationTxnPromise = waitForTransaction('angular-17', async transactionEvent => {
134+
return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'navigation';
135+
});
136+
137+
await page.goto(`/`);
138+
139+
// immediately navigate to a different route
140+
const [_, navigationTxn] = await Promise.all([page.locator('#cancelLink').click(), navigationTxnPromise]);
141+
142+
expect(navigationTxn).toMatchObject({
143+
contexts: {
144+
trace: {
145+
op: 'navigation',
146+
origin: 'auto.navigation.angular',
147+
},
148+
},
149+
transaction: '/cancel',
150+
transaction_info: {
151+
source: 'url',
152+
},
153+
});
154+
155+
const routingSpan = navigationTxn.spans?.find(span => span.op === 'ui.angular.routing');
156+
157+
expect(routingSpan).toBeDefined();
158+
expect(routingSpan?.description).toBe('/cancel');
159+
});
160+
161+
test('finishes routing span on navigation error', async ({ page }) => {
162+
const navigationTxnPromise = waitForTransaction('angular-17', async transactionEvent => {
163+
return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'navigation';
164+
});
165+
166+
await page.goto(`/`);
167+
168+
// immediately navigate to a different route
169+
const [_, navigationTxn] = await Promise.all([page.locator('#nonExistentLink').click(), navigationTxnPromise]);
170+
171+
const nonExistentRoute = '/non-existent';
172+
173+
expect(navigationTxn).toMatchObject({
174+
contexts: {
175+
trace: {
176+
op: 'navigation',
177+
origin: 'auto.navigation.angular',
178+
},
179+
},
180+
transaction: nonExistentRoute,
181+
transaction_info: {
182+
source: 'url',
183+
},
184+
});
185+
186+
const routingSpan = navigationTxn.spans?.find(span => span.op === 'ui.angular.routing');
187+
188+
expect(routingSpan).toBeDefined();
189+
expect(routingSpan?.description).toBe(nonExistentRoute);
190+
});
191+
});
192+
193+
test.describe('TraceDirective', () => {
194+
test('creates a child tracingSpan with component name as span name on ngOnInit', async ({ page }) => {
195+
const navigationTxnPromise = waitForTransaction('angular-17', async transactionEvent => {
196+
return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'navigation';
197+
});
198+
199+
await page.goto(`/`);
200+
201+
// immediately navigate to a different route
202+
const [_, navigationTxn] = await Promise.all([page.locator('#componentTracking').click(), navigationTxnPromise]);
203+
204+
const traceDirectiveSpan = navigationTxn.spans?.find(
205+
span => span?.data && span?.data[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN] === 'auto.ui.angular.trace_directive',
206+
);
207+
208+
expect(traceDirectiveSpan).toBeDefined();
209+
expect(traceDirectiveSpan).toEqual(
210+
expect.objectContaining({
211+
data: {
212+
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'ui.angular.init',
213+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.angular.trace_directive',
214+
},
215+
description: '<sample-component>',
216+
op: 'ui.angular.init',
217+
origin: 'auto.ui.angular.trace_directive',
218+
start_timestamp: expect.any(Number),
219+
timestamp: expect.any(Number),
220+
}),
221+
);
222+
});
223+
});
224+
225+
test.describe('TraceClassDecorator', () => {
226+
test('adds init span for decorated class', async ({ page }) => {
227+
const navigationTxnPromise = waitForTransaction('angular-17', async transactionEvent => {
228+
return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'navigation';
229+
});
230+
231+
await page.goto(`/`);
232+
233+
// immediately navigate to a different route
234+
const [_, navigationTxn] = await Promise.all([page.locator('#componentTracking').click(), navigationTxnPromise]);
235+
236+
const classDecoratorSpan = navigationTxn.spans?.find(
237+
span => span?.data && span?.data[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN] === 'auto.ui.angular.trace_class_decorator',
238+
);
239+
240+
expect(classDecoratorSpan).toBeDefined();
241+
expect(classDecoratorSpan).toEqual(
242+
expect.objectContaining({
243+
data: {
244+
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'ui.angular.init',
245+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.angular.trace_class_decorator',
246+
},
247+
// todo: right now, it shows the minified version of the component name - we will add a name input to the Decorator
248+
description: expect.any(String),
249+
op: 'ui.angular.init',
250+
origin: 'auto.ui.angular.trace_class_decorator',
251+
start_timestamp: expect.any(Number),
252+
timestamp: expect.any(Number),
253+
}),
254+
);
255+
});
256+
});
257+
258+
test.describe('TraceMethodDecorator', () => {
259+
test('instruments decorated methods (`ngOnInit` and `ngAfterViewInit`)', async ({ page }) => {
260+
const navigationTxnPromise = waitForTransaction('angular-17', async transactionEvent => {
261+
return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'navigation';
262+
});
263+
264+
await page.goto(`/`);
265+
266+
// immediately navigate to a different route
267+
const [_, navigationTxn] = await Promise.all([page.locator('#componentTracking').click(), navigationTxnPromise]);
268+
269+
const ngInitSpan = navigationTxn.spans?.find(span => span.op === 'ui.angular.ngOnInit');
270+
const ngAfterViewInitSpan = navigationTxn.spans?.find(span => span.op === 'ui.angular.ngAfterViewInit');
271+
272+
expect(ngInitSpan).toBeDefined();
273+
expect(ngInitSpan).toEqual(
274+
expect.objectContaining({
275+
data: {
276+
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'ui.angular.ngOnInit',
277+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.angular.trace_method_decorator',
278+
},
279+
// todo: right now, it shows the minified version of the component name - we will add a name input to the Decorator
280+
description: expect.any(String),
281+
op: 'ui.angular.ngOnInit',
282+
origin: 'auto.ui.angular.trace_method_decorator',
283+
start_timestamp: expect.any(Number),
284+
timestamp: expect.any(Number),
285+
}),
286+
);
287+
288+
expect(ngAfterViewInitSpan).toBeDefined();
289+
expect(ngAfterViewInitSpan).toEqual(
290+
expect.objectContaining({
291+
data: {
292+
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'ui.angular.ngAfterViewInit',
293+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.angular.trace_method_decorator',
294+
},
295+
// todo: right now, it shows the minified version of the component name - we will add a name input to the Decorator
296+
description: expect.any(String),
297+
op: 'ui.angular.ngAfterViewInit',
298+
origin: 'auto.ui.angular.trace_method_decorator',
299+
start_timestamp: expect.any(Number),
300+
timestamp: expect.any(Number),
301+
}),
302+
);
303+
});
304+
});

packages/angular/src/index.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,6 @@ export { createErrorHandler, SentryErrorHandler } from './errorhandler';
77
export {
88
// eslint-disable-next-line deprecation/deprecation
99
getActiveTransaction,
10-
// eslint-disable-next-line deprecation/deprecation
11-
instrumentAngularRouting, // new name
12-
// eslint-disable-next-line deprecation/deprecation
13-
routingInstrumentation, // legacy name
1410
browserTracingIntegration,
1511
TraceClassDecorator,
1612
TraceMethodDecorator,

0 commit comments

Comments
 (0)