Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 7 commits into from
Jan 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
Expand Down
10 changes: 4 additions & 6 deletions packages/angular-ivy/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
```
Expand Down
8 changes: 3 additions & 5 deletions packages/angular/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
```
Expand Down
4 changes: 3 additions & 1 deletion packages/angular/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
90 changes: 84 additions & 6 deletions packages/angular/src/tracing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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,
Expand All @@ -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.
*
Expand All @@ -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();

Expand All @@ -90,9 +170,6 @@ export class TraceService implements OnDestroy {
}

if (activeTransaction) {
if (this._routingSpan) {
this._routingSpan.end();
}
// eslint-disable-next-line deprecation/deprecation
Lms24 marked this conversation as resolved.
Show resolved Hide resolved
this._routingSpan = activeTransaction.startChild({
description: `${navigationEvent.url}`,
Expand Down Expand Up @@ -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`);
}
}),
);
Expand Down
2 changes: 2 additions & 0 deletions packages/angular/test/tracing.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -57,6 +58,7 @@ describe('Angular Tracing', () => {
});
});
});
/* eslint-enable deprecation/deprecation */

describe('getParameterizedRouteFromSnapshot', () => {
it.each([
Expand Down
1 change: 1 addition & 0 deletions packages/angular/test/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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);
Expand Down
Loading