Skip to content

ref(tracing): Clarify naming in BrowserTracing integration #3338

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

Merged
merged 8 commits into from
Mar 22, 2021
4 changes: 2 additions & 2 deletions packages/angular/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -82,15 +82,15 @@ Registering a Trace Service is a 3-step process.
instrumentation:

```javascript
import { init, routingInstrumentation } from '@sentry/angular';
import { init, instrumentAngularRouting } from '@sentry/angular';
import { Integrations as TracingIntegrations } from '@sentry/tracing';

init({
dsn: '__DSN__',
integrations: [
new TracingIntegrations.BrowserTracing({
tracingOrigins: ['localhost', 'https://yourserver.io/api'],
routingInstrumentation: routingInstrumentation,
routingInstrumentation: instrumentAngularRouting,
}),
],
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 @@ -4,7 +4,9 @@ export { init } from './sdk';
export { createErrorHandler, ErrorHandlerOptions } from './errorhandler';
export {
getActiveTransaction,
routingInstrumentation,
// TODO `instrumentAngularRouting` is just an alias for `routingInstrumentation`; deprecate the latter at some point
instrumentAngularRouting, // new name
routingInstrumentation, // legacy name
TraceClassDecorator,
TraceMethodDecorator,
TraceDirective,
Expand Down
8 changes: 5 additions & 3 deletions packages/angular/src/tracing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,22 +14,24 @@ let stashedStartTransactionOnLocationChange: boolean;
* Creates routing instrumentation for Angular Router.
*/
export function routingInstrumentation(
startTransaction: (context: TransactionContext) => Transaction | undefined,
customStartTransaction: (context: TransactionContext) => Transaction | undefined,
startTransactionOnPageLoad: boolean = true,
startTransactionOnLocationChange: boolean = true,
): void {
instrumentationInitialized = true;
stashedStartTransaction = startTransaction;
stashedStartTransaction = customStartTransaction;
stashedStartTransactionOnLocationChange = startTransactionOnLocationChange;

if (startTransactionOnPageLoad) {
startTransaction({
customStartTransaction({
name: window.location.pathname,
op: 'pageload',
});
}
}

export const instrumentAngularRouting = routingInstrumentation;

/**
* Grabs active transaction off scope
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -337,10 +337,10 @@ export async function instrumentForPerformance(appInstance: ApplicationInstance)
sentryConfig['integrations'] = [
...existingIntegrations,
new tracing.Integrations.BrowserTracing({
routingInstrumentation: (startTransaction, startTransactionOnPageLoad) => {
routingInstrumentation: (customStartTransaction, startTransactionOnPageLoad) => {
const routerMain = appInstance.lookup('router:main');
const routerService = appInstance.lookup('service:router');
_instrumentEmberRouter(routerService, routerMain, config, startTransaction, startTransactionOnPageLoad);
_instrumentEmberRouter(routerService, routerMain, config, customStartTransaction, startTransactionOnPageLoad);
},
idleTimeout,
}),
Expand Down
18 changes: 9 additions & 9 deletions packages/react/src/reactrouter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,24 +47,24 @@ export function reactRouterV4Instrumentation(
routes?: RouteConfig[],
matchPath?: MatchPath,
): ReactRouterInstrumentation {
return reactRouterInstrumentation(history, 'react-router-v4', routes, matchPath);
return createReactRouterInstrumentation(history, 'react-router-v4', routes, matchPath);
}

export function reactRouterV5Instrumentation(
history: RouterHistory,
routes?: RouteConfig[],
matchPath?: MatchPath,
): ReactRouterInstrumentation {
return reactRouterInstrumentation(history, 'react-router-v5', routes, matchPath);
return createReactRouterInstrumentation(history, 'react-router-v5', routes, matchPath);
}

function reactRouterInstrumentation(
function createReactRouterInstrumentation(
history: RouterHistory,
name: string,
allRoutes: RouteConfig[] = [],
matchPath?: MatchPath,
): ReactRouterInstrumentation {
function getName(pathname: string): string {
function getTransactionName(pathname: string): string {
if (allRoutes === [] || !matchPath) {
return pathname;
}
Expand All @@ -80,10 +80,10 @@ function reactRouterInstrumentation(
return pathname;
}

return (startTransaction, startTransactionOnPageLoad = true, startTransactionOnLocationChange = true): void => {
return (customStartTransaction, startTransactionOnPageLoad = true, startTransactionOnLocationChange = true): void => {
if (startTransactionOnPageLoad && global && global.location) {
activeTransaction = startTransaction({
name: getName(global.location.pathname),
activeTransaction = customStartTransaction({
name: getTransactionName(global.location.pathname),
op: 'pageload',
tags: {
'routing.instrumentation': name,
Expand All @@ -101,8 +101,8 @@ function reactRouterInstrumentation(
'routing.instrumentation': name,
};

activeTransaction = startTransaction({
name: getName(location.pathname),
activeTransaction = customStartTransaction({
name: getTransactionName(location.pathname),
op: 'navigation',
tags,
});
Expand Down
14 changes: 7 additions & 7 deletions packages/tracing/src/browser/browsertracing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ import { registerBackgroundTabDetection } from './backgroundtab';
import { MetricsInstrumentation } from './metrics';
import {
defaultRequestInstrumentationOptions,
registerRequestInstrumentation,
instrumentOutgoingRequests,
RequestInstrumentationOptions,
} from './request';
import { defaultRoutingInstrumentation } from './router';
import { instrumentRoutingWithDefaults } from './router';

export const DEFAULT_MAX_TRANSACTION_DURATION_SECONDS = 600;

Expand Down Expand Up @@ -77,7 +77,7 @@ export interface BrowserTracingOptions extends RequestInstrumentationOptions {
* pageload and navigation transactions.
*/
routingInstrumentation<T extends Transaction>(
startTransaction: (context: TransactionContext) => T | undefined,
customStartTransaction: (context: TransactionContext) => T | undefined,
startTransactionOnPageLoad?: boolean,
startTransactionOnLocationChange?: boolean,
): void;
Expand All @@ -87,7 +87,7 @@ const DEFAULT_BROWSER_TRACING_OPTIONS = {
idleTimeout: DEFAULT_IDLE_TIMEOUT,
markBackgroundTransactions: true,
maxTransactionDuration: DEFAULT_MAX_TRANSACTION_DURATION_SECONDS,
routingInstrumentation: defaultRoutingInstrumentation,
routingInstrumentation: instrumentRoutingWithDefaults,
startTransactionOnLocationChange: true,
startTransactionOnPageLoad: true,
...defaultRequestInstrumentationOptions,
Expand Down Expand Up @@ -158,7 +158,7 @@ export class BrowserTracing implements Integration {

// eslint-disable-next-line @typescript-eslint/unbound-method
const {
routingInstrumentation,
routingInstrumentation: instrumentRouting,
startTransactionOnLocationChange,
startTransactionOnPageLoad,
markBackgroundTransactions,
Expand All @@ -168,7 +168,7 @@ export class BrowserTracing implements Integration {
shouldCreateSpanForRequest,
} = this.options;

routingInstrumentation(
instrumentRouting(
(context: TransactionContext) => this._createRouteTransaction(context),
startTransactionOnPageLoad,
startTransactionOnLocationChange,
Expand All @@ -178,7 +178,7 @@ export class BrowserTracing implements Integration {
registerBackgroundTabDetection();
}

registerRequestInstrumentation({ traceFetch, traceXHR, tracingOrigins, shouldCreateSpanForRequest });
instrumentOutgoingRequests({ traceFetch, traceXHR, tracingOrigins, shouldCreateSpanForRequest });
}

/** Create routing idle transaction. */
Expand Down
2 changes: 1 addition & 1 deletion packages/tracing/src/browser/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
export { BrowserTracing } from './browsertracing';
export {
registerRequestInstrumentation,
instrumentOutgoingRequests,
RequestInstrumentationOptions,
defaultRequestInstrumentationOptions,
} from './request';
2 changes: 1 addition & 1 deletion packages/tracing/src/browser/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ export const defaultRequestInstrumentationOptions: RequestInstrumentationOptions
};

/** Registers span creators for xhr and fetch requests */
export function registerRequestInstrumentation(_options?: Partial<RequestInstrumentationOptions>): void {
export function instrumentOutgoingRequests(_options?: Partial<RequestInstrumentationOptions>): void {
// eslint-disable-next-line @typescript-eslint/unbound-method
const { traceFetch, traceXHR, tracingOrigins, shouldCreateSpanForRequest } = {
...defaultRequestInstrumentationOptions,
Expand Down
8 changes: 4 additions & 4 deletions packages/tracing/src/browser/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ const global = getGlobalObject<Window>();
/**
* Default function implementing pageload and navigation transactions
*/
export function defaultRoutingInstrumentation<T extends Transaction>(
startTransaction: (context: TransactionContext) => T | undefined,
export function instrumentRoutingWithDefaults<T extends Transaction>(
customStartTransaction: (context: TransactionContext) => T | undefined,
startTransactionOnPageLoad: boolean = true,
startTransactionOnLocationChange: boolean = true,
): void {
Expand All @@ -20,7 +20,7 @@ export function defaultRoutingInstrumentation<T extends Transaction>(

let activeTransaction: T | undefined;
if (startTransactionOnPageLoad) {
activeTransaction = startTransaction({ name: global.location.pathname, op: 'pageload' });
activeTransaction = customStartTransaction({ name: global.location.pathname, op: 'pageload' });
}

if (startTransactionOnLocationChange) {
Expand All @@ -47,7 +47,7 @@ export function defaultRoutingInstrumentation<T extends Transaction>(
// If there's an open transaction on the scope, we need to finish it before creating an new one.
activeTransaction.finish();
}
activeTransaction = startTransaction({ name: global.location.pathname, op: 'navigation' });
activeTransaction = customStartTransaction({ name: global.location.pathname, op: 'navigation' });
}
},
type: 'history',
Expand Down
3 changes: 2 additions & 1 deletion packages/tracing/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ export { Integrations };
export { Span } from './span';
export { Transaction } from './transaction';
export {
registerRequestInstrumentation,
// TODO deprecate old name in v7
instrumentOutgoingRequests as registerRequestInstrumentation,
RequestInstrumentationOptions,
defaultRequestInstrumentationOptions,
} from './browser';
Expand Down
Loading