From b2c6d1ec70f7333366880b5ebcc9b714ac02925e Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 3 Feb 2021 13:01:43 -0800 Subject: [PATCH 1/8] s/startTransaction/customStartTransaction --- packages/angular/src/tracing.ts | 6 +-- .../sentry-performance.ts | 4 +- packages/react/src/reactrouter.tsx | 6 +-- .../tracing/src/browser/browsertracing.ts | 2 +- packages/tracing/src/browser/router.ts | 6 +-- .../test/browser/browsertracing.test.ts | 4 +- packages/tracing/test/browser/router.test.ts | 42 +++++++++---------- 7 files changed, 35 insertions(+), 35 deletions(-) diff --git a/packages/angular/src/tracing.ts b/packages/angular/src/tracing.ts index 30752ef6a0d7..4f6732564830 100644 --- a/packages/angular/src/tracing.ts +++ b/packages/angular/src/tracing.ts @@ -14,16 +14,16 @@ 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', }); diff --git a/packages/ember/addon/instance-initializers/sentry-performance.ts b/packages/ember/addon/instance-initializers/sentry-performance.ts index 1e5e9d7d8fad..99e5c96565cb 100644 --- a/packages/ember/addon/instance-initializers/sentry-performance.ts +++ b/packages/ember/addon/instance-initializers/sentry-performance.ts @@ -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, }), diff --git a/packages/react/src/reactrouter.tsx b/packages/react/src/reactrouter.tsx index 66117d3e453c..0dd006419fb4 100644 --- a/packages/react/src/reactrouter.tsx +++ b/packages/react/src/reactrouter.tsx @@ -80,9 +80,9 @@ 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({ + activeTransaction = customStartTransaction({ name: getName(global.location.pathname), op: 'pageload', tags: { @@ -101,7 +101,7 @@ function reactRouterInstrumentation( 'routing.instrumentation': name, }; - activeTransaction = startTransaction({ + activeTransaction = customStartTransaction({ name: getName(location.pathname), op: 'navigation', tags, diff --git a/packages/tracing/src/browser/browsertracing.ts b/packages/tracing/src/browser/browsertracing.ts index 05370b8e384d..432c6c08ef34 100644 --- a/packages/tracing/src/browser/browsertracing.ts +++ b/packages/tracing/src/browser/browsertracing.ts @@ -77,7 +77,7 @@ export interface BrowserTracingOptions extends RequestInstrumentationOptions { * pageload and navigation transactions. */ routingInstrumentation( - startTransaction: (context: TransactionContext) => T | undefined, + customStartTransaction: (context: TransactionContext) => T | undefined, startTransactionOnPageLoad?: boolean, startTransactionOnLocationChange?: boolean, ): void; diff --git a/packages/tracing/src/browser/router.ts b/packages/tracing/src/browser/router.ts index 4ed79ffe88c3..6559726b59e5 100644 --- a/packages/tracing/src/browser/router.ts +++ b/packages/tracing/src/browser/router.ts @@ -7,7 +7,7 @@ const global = getGlobalObject(); * Default function implementing pageload and navigation transactions */ export function defaultRoutingInstrumentation( - startTransaction: (context: TransactionContext) => T | undefined, + customStartTransaction: (context: TransactionContext) => T | undefined, startTransactionOnPageLoad: boolean = true, startTransactionOnLocationChange: boolean = true, ): void { @@ -20,7 +20,7 @@ export function defaultRoutingInstrumentation( let activeTransaction: T | undefined; if (startTransactionOnPageLoad) { - activeTransaction = startTransaction({ name: global.location.pathname, op: 'pageload' }); + activeTransaction = customStartTransaction({ name: global.location.pathname, op: 'pageload' }); } if (startTransactionOnLocationChange) { @@ -47,7 +47,7 @@ export function defaultRoutingInstrumentation( // 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', diff --git a/packages/tracing/test/browser/browsertracing.test.ts b/packages/tracing/test/browser/browsertracing.test.ts index 3ff774e4dbd2..5f5542542abf 100644 --- a/packages/tracing/test/browser/browsertracing.test.ts +++ b/packages/tracing/test/browser/browsertracing.test.ts @@ -96,8 +96,8 @@ describe('BrowserTracing', () => { * so that we can show this functionality works independent of the default routing integration. */ describe('route transaction', () => { - const customRoutingInstrumentation = (startTransaction: (obj: any) => void) => { - startTransaction({ name: 'a/path', op: 'pageload' }); + const customRoutingInstrumentation = (customStartTransaction: (obj: any) => void) => { + customStartTransaction({ name: 'a/path', op: 'pageload' }); }; it('calls custom routing instrumenation', () => { diff --git a/packages/tracing/test/browser/router.test.ts b/packages/tracing/test/browser/router.test.ts index bbaf5e31863f..c2125f18a290 100644 --- a/packages/tracing/test/browser/router.test.ts +++ b/packages/tracing/test/browser/router.test.ts @@ -17,7 +17,7 @@ jest.mock('@sentry/utils', () => { describe('defaultRoutingInstrumentation', () => { const mockFinish = jest.fn(); - const startTransaction = jest.fn().mockReturnValue({ finish: mockFinish }); + const customStartTransaction = jest.fn().mockReturnValue({ finish: mockFinish }); beforeEach(() => { const dom = new JSDOM(); // @ts-ignore need to override global document @@ -27,26 +27,26 @@ describe('defaultRoutingInstrumentation', () => { // @ts-ignore need to override global document global.location = dom.window.location; - startTransaction.mockClear(); + customStartTransaction.mockClear(); mockFinish.mockClear(); }); it('does not start transactions if global location is undefined', () => { // @ts-ignore need to override global document global.location = undefined; - defaultRoutingInstrumentation(startTransaction); - expect(startTransaction).toHaveBeenCalledTimes(0); + defaultRoutingInstrumentation(customStartTransaction); + expect(customStartTransaction).toHaveBeenCalledTimes(0); }); it('starts a pageload transaction', () => { - defaultRoutingInstrumentation(startTransaction); - expect(startTransaction).toHaveBeenCalledTimes(1); - expect(startTransaction).toHaveBeenLastCalledWith({ name: 'blank', op: 'pageload' }); + defaultRoutingInstrumentation(customStartTransaction); + expect(customStartTransaction).toHaveBeenCalledTimes(1); + expect(customStartTransaction).toHaveBeenLastCalledWith({ name: 'blank', op: 'pageload' }); }); it('does not start a pageload transaction if startTransactionOnPageLoad is false', () => { - defaultRoutingInstrumentation(startTransaction, false); - expect(startTransaction).toHaveBeenCalledTimes(0); + defaultRoutingInstrumentation(customStartTransaction, false); + expect(customStartTransaction).toHaveBeenCalledTimes(0); }); describe('navigation transaction', () => { @@ -56,29 +56,29 @@ describe('defaultRoutingInstrumentation', () => { }); it('it is not created automatically', () => { - defaultRoutingInstrumentation(startTransaction); - expect(startTransaction).not.toHaveBeenLastCalledWith({ name: 'blank', op: 'navigation' }); + defaultRoutingInstrumentation(customStartTransaction); + expect(customStartTransaction).not.toHaveBeenLastCalledWith({ name: 'blank', op: 'navigation' }); }); it('is created on location change', () => { - defaultRoutingInstrumentation(startTransaction); + defaultRoutingInstrumentation(customStartTransaction); mockChangeHistory({ to: 'here', from: 'there' }); expect(addInstrumentationHandlerType).toBe('history'); - expect(startTransaction).toHaveBeenCalledTimes(2); - expect(startTransaction).toHaveBeenLastCalledWith({ name: 'blank', op: 'navigation' }); + expect(customStartTransaction).toHaveBeenCalledTimes(2); + expect(customStartTransaction).toHaveBeenLastCalledWith({ name: 'blank', op: 'navigation' }); }); it('is not created if startTransactionOnLocationChange is false', () => { - defaultRoutingInstrumentation(startTransaction, true, false); + defaultRoutingInstrumentation(customStartTransaction, true, false); mockChangeHistory({ to: 'here', from: 'there' }); expect(addInstrumentationHandlerType).toBe(''); - expect(startTransaction).toHaveBeenCalledTimes(1); + expect(customStartTransaction).toHaveBeenCalledTimes(1); }); it('finishes the last active transaction', () => { - defaultRoutingInstrumentation(startTransaction); + defaultRoutingInstrumentation(customStartTransaction); expect(mockFinish).toHaveBeenCalledTimes(0); mockChangeHistory({ to: 'here', from: 'there' }); @@ -86,7 +86,7 @@ describe('defaultRoutingInstrumentation', () => { }); it('will finish active transaction multiple times', () => { - defaultRoutingInstrumentation(startTransaction); + defaultRoutingInstrumentation(customStartTransaction); expect(mockFinish).toHaveBeenCalledTimes(0); mockChangeHistory({ to: 'here', from: 'there' }); @@ -98,12 +98,12 @@ describe('defaultRoutingInstrumentation', () => { }); it('not created if `from` is equal to `to`', () => { - defaultRoutingInstrumentation(startTransaction); + defaultRoutingInstrumentation(customStartTransaction); mockChangeHistory({ to: 'first/path', from: 'first/path' }); expect(addInstrumentationHandlerType).toBe('history'); - expect(startTransaction).toHaveBeenCalledTimes(1); - expect(startTransaction).not.toHaveBeenLastCalledWith('navigation'); + expect(customStartTransaction).toHaveBeenCalledTimes(1); + expect(customStartTransaction).not.toHaveBeenLastCalledWith('navigation'); }); }); }); From c85993d570aca950193e25da95cf16e5b550b6a4 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 17 Mar 2021 18:57:14 -0700 Subject: [PATCH 2/8] s/registerRequestInstrumentation/instrumentOutgoingRequests --- packages/tracing/src/browser/browsertracing.ts | 4 ++-- packages/tracing/src/browser/index.ts | 2 +- packages/tracing/src/browser/request.ts | 2 +- packages/tracing/src/index.ts | 3 ++- packages/tracing/test/browser/request.test.ts | 16 +++++----------- 5 files changed, 11 insertions(+), 16 deletions(-) diff --git a/packages/tracing/src/browser/browsertracing.ts b/packages/tracing/src/browser/browsertracing.ts index 432c6c08ef34..de6b44ba331d 100644 --- a/packages/tracing/src/browser/browsertracing.ts +++ b/packages/tracing/src/browser/browsertracing.ts @@ -10,7 +10,7 @@ import { registerBackgroundTabDetection } from './backgroundtab'; import { MetricsInstrumentation } from './metrics'; import { defaultRequestInstrumentationOptions, - registerRequestInstrumentation, + instrumentOutgoingRequests, RequestInstrumentationOptions, } from './request'; import { defaultRoutingInstrumentation } from './router'; @@ -178,7 +178,7 @@ export class BrowserTracing implements Integration { registerBackgroundTabDetection(); } - registerRequestInstrumentation({ traceFetch, traceXHR, tracingOrigins, shouldCreateSpanForRequest }); + instrumentOutgoingRequests({ traceFetch, traceXHR, tracingOrigins, shouldCreateSpanForRequest }); } /** Create routing idle transaction. */ diff --git a/packages/tracing/src/browser/index.ts b/packages/tracing/src/browser/index.ts index 753b65041f58..dd022fe2b8ec 100644 --- a/packages/tracing/src/browser/index.ts +++ b/packages/tracing/src/browser/index.ts @@ -1,6 +1,6 @@ export { BrowserTracing } from './browsertracing'; export { - registerRequestInstrumentation, + instrumentOutgoingRequests, RequestInstrumentationOptions, defaultRequestInstrumentationOptions, } from './request'; diff --git a/packages/tracing/src/browser/request.ts b/packages/tracing/src/browser/request.ts index 303f2fb4565b..4f430e69d713 100644 --- a/packages/tracing/src/browser/request.ts +++ b/packages/tracing/src/browser/request.ts @@ -83,7 +83,7 @@ export const defaultRequestInstrumentationOptions: RequestInstrumentationOptions }; /** Registers span creators for xhr and fetch requests */ -export function registerRequestInstrumentation(_options?: Partial): void { +export function instrumentOutgoingRequests(_options?: Partial): void { // eslint-disable-next-line @typescript-eslint/unbound-method const { traceFetch, traceXHR, tracingOrigins, shouldCreateSpanForRequest } = { ...defaultRequestInstrumentationOptions, diff --git a/packages/tracing/src/index.ts b/packages/tracing/src/index.ts index 5652aa63333e..608bd0630810 100644 --- a/packages/tracing/src/index.ts +++ b/packages/tracing/src/index.ts @@ -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'; diff --git a/packages/tracing/test/browser/request.test.ts b/packages/tracing/test/browser/request.test.ts index 1203824ccf64..7761f6a2660b 100644 --- a/packages/tracing/test/browser/request.test.ts +++ b/packages/tracing/test/browser/request.test.ts @@ -3,13 +3,7 @@ import { Hub, makeMain } from '@sentry/hub'; import * as utils from '@sentry/utils'; import { Span, SpanStatus, Transaction } from '../../src'; -import { - fetchCallback, - FetchData, - registerRequestInstrumentation, - xhrCallback, - XHRData, -} from '../../src/browser/request'; +import { fetchCallback, FetchData, instrumentOutgoingRequests, xhrCallback, XHRData } from '../../src/browser/request'; import { addExtensionMethods } from '../../src/hubextensions'; import * as tracingUtils from '../../src/utils'; @@ -24,13 +18,13 @@ const hasTracingEnabled = jest.spyOn(tracingUtils, 'hasTracingEnabled'); const addInstrumentationHandler = jest.spyOn(utils, 'addInstrumentationHandler'); const setRequestHeader = jest.fn(); -describe('registerRequestInstrumentation', () => { +describe('instrumentOutgoingRequests', () => { beforeEach(() => { jest.clearAllMocks(); }); it('instruments fetch and xhr requests', () => { - registerRequestInstrumentation(); + instrumentOutgoingRequests(); expect(addInstrumentationHandler).toHaveBeenCalledWith({ callback: expect.any(Function), @@ -43,13 +37,13 @@ describe('registerRequestInstrumentation', () => { }); it('does not instrument fetch requests if traceFetch is false', () => { - registerRequestInstrumentation({ traceFetch: false }); + instrumentOutgoingRequests({ traceFetch: false }); expect(addInstrumentationHandler).not.toHaveBeenCalledWith({ callback: expect.any(Function), type: 'fetch' }); }); it('does not instrument xhr requests if traceXHR is false', () => { - registerRequestInstrumentation({ traceXHR: false }); + instrumentOutgoingRequests({ traceXHR: false }); expect(addInstrumentationHandler).not.toHaveBeenCalledWith({ callback: expect.any(Function), type: 'xhr' }); }); From 046027a0d32d87c623f7b534f84507d0987592d9 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 17 Mar 2021 18:58:08 -0700 Subject: [PATCH 3/8] s/defaultRoutingInstrumentation/instrumentRoutingWithDefaults --- .../tracing/src/browser/browsertracing.ts | 4 ++-- packages/tracing/src/browser/router.ts | 2 +- .../test/browser/browsertracing.test.ts | 4 ++-- packages/tracing/test/browser/router.test.ts | 22 +++++++++---------- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/packages/tracing/src/browser/browsertracing.ts b/packages/tracing/src/browser/browsertracing.ts index de6b44ba331d..406e01961184 100644 --- a/packages/tracing/src/browser/browsertracing.ts +++ b/packages/tracing/src/browser/browsertracing.ts @@ -13,7 +13,7 @@ import { instrumentOutgoingRequests, RequestInstrumentationOptions, } from './request'; -import { defaultRoutingInstrumentation } from './router'; +import { instrumentRoutingWithDefaults } from './router'; export const DEFAULT_MAX_TRANSACTION_DURATION_SECONDS = 600; @@ -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, diff --git a/packages/tracing/src/browser/router.ts b/packages/tracing/src/browser/router.ts index 6559726b59e5..172a11592279 100644 --- a/packages/tracing/src/browser/router.ts +++ b/packages/tracing/src/browser/router.ts @@ -6,7 +6,7 @@ const global = getGlobalObject(); /** * Default function implementing pageload and navigation transactions */ -export function defaultRoutingInstrumentation( +export function instrumentRoutingWithDefaults( customStartTransaction: (context: TransactionContext) => T | undefined, startTransactionOnPageLoad: boolean = true, startTransactionOnLocationChange: boolean = true, diff --git a/packages/tracing/test/browser/browsertracing.test.ts b/packages/tracing/test/browser/browsertracing.test.ts index 5f5542542abf..4578ad59ad68 100644 --- a/packages/tracing/test/browser/browsertracing.test.ts +++ b/packages/tracing/test/browser/browsertracing.test.ts @@ -12,7 +12,7 @@ import { getMetaContent, } from '../../src/browser/browsertracing'; import { defaultRequestInstrumentationOptions } from '../../src/browser/request'; -import { defaultRoutingInstrumentation } from '../../src/browser/router'; +import { instrumentRoutingWithDefaults } from '../../src/browser/router'; import * as hubExtensions from '../../src/hubextensions'; import { DEFAULT_IDLE_TIMEOUT, IdleTransaction } from '../../src/idletransaction'; import { getActiveTransaction, secToMs } from '../../src/utils'; @@ -83,7 +83,7 @@ describe('BrowserTracing', () => { idleTimeout: DEFAULT_IDLE_TIMEOUT, markBackgroundTransactions: true, maxTransactionDuration: DEFAULT_MAX_TRANSACTION_DURATION_SECONDS, - routingInstrumentation: defaultRoutingInstrumentation, + routingInstrumentation: instrumentRoutingWithDefaults, startTransactionOnLocationChange: true, startTransactionOnPageLoad: true, ...defaultRequestInstrumentationOptions, diff --git a/packages/tracing/test/browser/router.test.ts b/packages/tracing/test/browser/router.test.ts index c2125f18a290..6c565b26655b 100644 --- a/packages/tracing/test/browser/router.test.ts +++ b/packages/tracing/test/browser/router.test.ts @@ -1,6 +1,6 @@ import { JSDOM } from 'jsdom'; -import { defaultRoutingInstrumentation } from '../../src/browser/router'; +import { instrumentRoutingWithDefaults } from '../../src/browser/router'; let mockChangeHistory: ({ to, from }: { to: string; from?: string }) => void = () => undefined; let addInstrumentationHandlerType: string = ''; @@ -15,7 +15,7 @@ jest.mock('@sentry/utils', () => { }; }); -describe('defaultRoutingInstrumentation', () => { +describe('instrumentRoutingWithDefaults', () => { const mockFinish = jest.fn(); const customStartTransaction = jest.fn().mockReturnValue({ finish: mockFinish }); beforeEach(() => { @@ -34,18 +34,18 @@ describe('defaultRoutingInstrumentation', () => { it('does not start transactions if global location is undefined', () => { // @ts-ignore need to override global document global.location = undefined; - defaultRoutingInstrumentation(customStartTransaction); + instrumentRoutingWithDefaults(customStartTransaction); expect(customStartTransaction).toHaveBeenCalledTimes(0); }); it('starts a pageload transaction', () => { - defaultRoutingInstrumentation(customStartTransaction); + instrumentRoutingWithDefaults(customStartTransaction); expect(customStartTransaction).toHaveBeenCalledTimes(1); expect(customStartTransaction).toHaveBeenLastCalledWith({ name: 'blank', op: 'pageload' }); }); it('does not start a pageload transaction if startTransactionOnPageLoad is false', () => { - defaultRoutingInstrumentation(customStartTransaction, false); + instrumentRoutingWithDefaults(customStartTransaction, false); expect(customStartTransaction).toHaveBeenCalledTimes(0); }); @@ -56,12 +56,12 @@ describe('defaultRoutingInstrumentation', () => { }); it('it is not created automatically', () => { - defaultRoutingInstrumentation(customStartTransaction); + instrumentRoutingWithDefaults(customStartTransaction); expect(customStartTransaction).not.toHaveBeenLastCalledWith({ name: 'blank', op: 'navigation' }); }); it('is created on location change', () => { - defaultRoutingInstrumentation(customStartTransaction); + instrumentRoutingWithDefaults(customStartTransaction); mockChangeHistory({ to: 'here', from: 'there' }); expect(addInstrumentationHandlerType).toBe('history'); @@ -70,7 +70,7 @@ describe('defaultRoutingInstrumentation', () => { }); it('is not created if startTransactionOnLocationChange is false', () => { - defaultRoutingInstrumentation(customStartTransaction, true, false); + instrumentRoutingWithDefaults(customStartTransaction, true, false); mockChangeHistory({ to: 'here', from: 'there' }); expect(addInstrumentationHandlerType).toBe(''); @@ -78,7 +78,7 @@ describe('defaultRoutingInstrumentation', () => { }); it('finishes the last active transaction', () => { - defaultRoutingInstrumentation(customStartTransaction); + instrumentRoutingWithDefaults(customStartTransaction); expect(mockFinish).toHaveBeenCalledTimes(0); mockChangeHistory({ to: 'here', from: 'there' }); @@ -86,7 +86,7 @@ describe('defaultRoutingInstrumentation', () => { }); it('will finish active transaction multiple times', () => { - defaultRoutingInstrumentation(customStartTransaction); + instrumentRoutingWithDefaults(customStartTransaction); expect(mockFinish).toHaveBeenCalledTimes(0); mockChangeHistory({ to: 'here', from: 'there' }); @@ -98,7 +98,7 @@ describe('defaultRoutingInstrumentation', () => { }); it('not created if `from` is equal to `to`', () => { - defaultRoutingInstrumentation(customStartTransaction); + instrumentRoutingWithDefaults(customStartTransaction); mockChangeHistory({ to: 'first/path', from: 'first/path' }); expect(addInstrumentationHandlerType).toBe('history'); From 00d42d8956844264860e200ba356710735305d50 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 17 Mar 2021 18:58:54 -0700 Subject: [PATCH 4/8] s/routingInstrumentation/instrumentRouting when calling it --- packages/tracing/src/browser/browsertracing.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/tracing/src/browser/browsertracing.ts b/packages/tracing/src/browser/browsertracing.ts index 406e01961184..c0db1e0c0e7b 100644 --- a/packages/tracing/src/browser/browsertracing.ts +++ b/packages/tracing/src/browser/browsertracing.ts @@ -158,7 +158,7 @@ export class BrowserTracing implements Integration { // eslint-disable-next-line @typescript-eslint/unbound-method const { - routingInstrumentation, + routingInstrumentation: instrumentRouting, startTransactionOnLocationChange, startTransactionOnPageLoad, markBackgroundTransactions, @@ -168,7 +168,7 @@ export class BrowserTracing implements Integration { shouldCreateSpanForRequest, } = this.options; - routingInstrumentation( + instrumentRouting( (context: TransactionContext) => this._createRouteTransaction(context), startTransactionOnPageLoad, startTransactionOnLocationChange, From 849d3612d841396d663d37496906d03c9408dac1 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 17 Mar 2021 19:16:37 -0700 Subject: [PATCH 5/8] s/routingInstrumentation/instrumentAngularRouting (as alias) --- packages/angular/README.md | 4 ++-- packages/angular/src/index.ts | 4 +++- packages/angular/src/tracing.ts | 2 ++ 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/angular/README.md b/packages/angular/README.md index 860291ce060f..d3f82ec7b685 100644 --- a/packages/angular/README.md +++ b/packages/angular/README.md @@ -82,7 +82,7 @@ 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({ @@ -90,7 +90,7 @@ init({ integrations: [ new TracingIntegrations.BrowserTracing({ tracingOrigins: ['localhost', 'https://yourserver.io/api'], - routingInstrumentation: routingInstrumentation, + routingInstrumentation: instrumentAngularRouting, }), ], tracesSampleRate: 1, diff --git a/packages/angular/src/index.ts b/packages/angular/src/index.ts index 19c411fbc8d0..c3ba495323f7 100644 --- a/packages/angular/src/index.ts +++ b/packages/angular/src/index.ts @@ -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, diff --git a/packages/angular/src/tracing.ts b/packages/angular/src/tracing.ts index 4f6732564830..49a40400549e 100644 --- a/packages/angular/src/tracing.ts +++ b/packages/angular/src/tracing.ts @@ -30,6 +30,8 @@ export function routingInstrumentation( } } +export const instrumentAngularRouting = routingInstrumentation; + /** * Grabs active transaction off scope */ From 572e982036f9cac95f23e66f4e95cd0f1308da92 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 17 Mar 2021 19:19:50 -0700 Subject: [PATCH 6/8] s/customRoutingInstrumentation/customInstrumentRouting --- .../test/browser/browsertracing.test.ts | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/packages/tracing/test/browser/browsertracing.test.ts b/packages/tracing/test/browser/browsertracing.test.ts index 4578ad59ad68..7c7d1d6306cc 100644 --- a/packages/tracing/test/browser/browsertracing.test.ts +++ b/packages/tracing/test/browser/browsertracing.test.ts @@ -92,17 +92,17 @@ describe('BrowserTracing', () => { /** * All of these tests under `describe('route transaction')` are tested with - * `browserTracing.options = { routingInstrumentation: customRoutingInstrumentation }`, + * `browserTracing.options = { routingInstrumentation: customInstrumentRouting }`, * so that we can show this functionality works independent of the default routing integration. */ describe('route transaction', () => { - const customRoutingInstrumentation = (customStartTransaction: (obj: any) => void) => { + const customInstrumentRouting = (customStartTransaction: (obj: any) => void) => { customStartTransaction({ name: 'a/path', op: 'pageload' }); }; it('calls custom routing instrumenation', () => { createBrowserTracing(true, { - routingInstrumentation: customRoutingInstrumentation, + routingInstrumentation: customInstrumentRouting, }); const transaction = getActiveTransaction(hub) as IdleTransaction; @@ -113,7 +113,7 @@ describe('BrowserTracing', () => { it('trims all transactions', () => { createBrowserTracing(true, { - routingInstrumentation: customRoutingInstrumentation, + routingInstrumentation: customInstrumentRouting, }); const transaction = getActiveTransaction(hub) as IdleTransaction; @@ -129,7 +129,7 @@ describe('BrowserTracing', () => { describe('tracingOrigins', () => { it('warns and uses default tracing origins if none are provided', () => { const inst = createBrowserTracing(true, { - routingInstrumentation: customRoutingInstrumentation, + routingInstrumentation: customInstrumentRouting, }); expect(warnSpy).toHaveBeenCalledTimes(2); @@ -138,7 +138,7 @@ describe('BrowserTracing', () => { it('warns and uses default tracing origins if empty array given', () => { const inst = createBrowserTracing(true, { - routingInstrumentation: customRoutingInstrumentation, + routingInstrumentation: customInstrumentRouting, tracingOrigins: [], }); @@ -148,7 +148,7 @@ describe('BrowserTracing', () => { it('warns and uses default tracing origins if tracing origins are not defined', () => { const inst = createBrowserTracing(true, { - routingInstrumentation: customRoutingInstrumentation, + routingInstrumentation: customInstrumentRouting, tracingOrigins: undefined, }); @@ -158,7 +158,7 @@ describe('BrowserTracing', () => { it('sets tracing origins if provided and does not warn', () => { const inst = createBrowserTracing(true, { - routingInstrumentation: customRoutingInstrumentation, + routingInstrumentation: customInstrumentRouting, tracingOrigins: ['something'], }); @@ -172,7 +172,7 @@ describe('BrowserTracing', () => { const mockBeforeNavigation = jest.fn().mockReturnValue({ name: 'here/is/my/path' }); createBrowserTracing(true, { beforeNavigate: mockBeforeNavigation, - routingInstrumentation: customRoutingInstrumentation, + routingInstrumentation: customInstrumentRouting, }); const transaction = getActiveTransaction(hub) as IdleTransaction; expect(transaction).toBeDefined(); @@ -184,7 +184,7 @@ describe('BrowserTracing', () => { const mockBeforeNavigation = jest.fn().mockReturnValue(undefined); createBrowserTracing(true, { beforeNavigate: mockBeforeNavigation, - routingInstrumentation: customRoutingInstrumentation, + routingInstrumentation: customInstrumentRouting, }); const transaction = getActiveTransaction(hub) as IdleTransaction; expect(transaction.sampled).toBe(false); @@ -199,7 +199,7 @@ describe('BrowserTracing', () => { })); createBrowserTracing(true, { beforeNavigate: mockBeforeNavigation, - routingInstrumentation: customRoutingInstrumentation, + routingInstrumentation: customInstrumentRouting, }); const transaction = getActiveTransaction(hub) as IdleTransaction; expect(transaction).toBeDefined(); @@ -215,7 +215,7 @@ describe('BrowserTracing', () => { document.head.innerHTML = ``; const startIdleTransaction = jest.spyOn(hubExtensions, 'startIdleTransaction'); - createBrowserTracing(true, { routingInstrumentation: customRoutingInstrumentation }); + createBrowserTracing(true, { routingInstrumentation: customInstrumentRouting }); expect(startIdleTransaction).toHaveBeenCalledWith( expect.any(Object), @@ -232,7 +232,7 @@ describe('BrowserTracing', () => { describe('idleTimeout', () => { it('is created by default', () => { - createBrowserTracing(true, { routingInstrumentation: customRoutingInstrumentation }); + createBrowserTracing(true, { routingInstrumentation: customInstrumentRouting }); const mockFinish = jest.fn(); const transaction = getActiveTransaction(hub) as IdleTransaction; transaction.finish = mockFinish; @@ -246,7 +246,7 @@ describe('BrowserTracing', () => { }); it('can be a custom value', () => { - createBrowserTracing(true, { idleTimeout: 2000, routingInstrumentation: customRoutingInstrumentation }); + createBrowserTracing(true, { idleTimeout: 2000, routingInstrumentation: customInstrumentRouting }); const mockFinish = jest.fn(); const transaction = getActiveTransaction(hub) as IdleTransaction; transaction.finish = mockFinish; @@ -262,7 +262,7 @@ describe('BrowserTracing', () => { describe('maxTransactionDuration', () => { it('cancels a transaction if exceeded', () => { - createBrowserTracing(true, { routingInstrumentation: customRoutingInstrumentation }); + createBrowserTracing(true, { routingInstrumentation: customInstrumentRouting }); const transaction = getActiveTransaction(hub) as IdleTransaction; transaction.finish(transaction.startTimestamp + secToMs(DEFAULT_MAX_TRANSACTION_DURATION_SECONDS) + 1); @@ -271,7 +271,7 @@ describe('BrowserTracing', () => { }); it('does not cancel a transaction if not exceeded', () => { - createBrowserTracing(true, { routingInstrumentation: customRoutingInstrumentation }); + createBrowserTracing(true, { routingInstrumentation: customInstrumentRouting }); const transaction = getActiveTransaction(hub) as IdleTransaction; transaction.finish(transaction.startTimestamp + secToMs(DEFAULT_MAX_TRANSACTION_DURATION_SECONDS)); @@ -285,7 +285,7 @@ describe('BrowserTracing', () => { expect(DEFAULT_MAX_TRANSACTION_DURATION_SECONDS < customMaxTransactionDuration).toBe(true); createBrowserTracing(true, { maxTransactionDuration: customMaxTransactionDuration, - routingInstrumentation: customRoutingInstrumentation, + routingInstrumentation: customInstrumentRouting, }); const transaction = getActiveTransaction(hub) as IdleTransaction; From 431cb4cf66329ad6976313645eb57c2e79fce030 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 18 Mar 2021 10:41:18 -0700 Subject: [PATCH 7/8] s/getName/getTransacionName in react router --- packages/react/src/reactrouter.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/react/src/reactrouter.tsx b/packages/react/src/reactrouter.tsx index 0dd006419fb4..bf4d140c1d60 100644 --- a/packages/react/src/reactrouter.tsx +++ b/packages/react/src/reactrouter.tsx @@ -64,7 +64,7 @@ function reactRouterInstrumentation( allRoutes: RouteConfig[] = [], matchPath?: MatchPath, ): ReactRouterInstrumentation { - function getName(pathname: string): string { + function getTransactionName(pathname: string): string { if (allRoutes === [] || !matchPath) { return pathname; } @@ -83,7 +83,7 @@ function reactRouterInstrumentation( return (customStartTransaction, startTransactionOnPageLoad = true, startTransactionOnLocationChange = true): void => { if (startTransactionOnPageLoad && global && global.location) { activeTransaction = customStartTransaction({ - name: getName(global.location.pathname), + name: getTransactionName(global.location.pathname), op: 'pageload', tags: { 'routing.instrumentation': name, @@ -102,7 +102,7 @@ function reactRouterInstrumentation( }; activeTransaction = customStartTransaction({ - name: getName(location.pathname), + name: getTransactionName(location.pathname), op: 'navigation', tags, }); From fefb6c8dd48fb35429313321792ee6146cb82555 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 18 Mar 2021 10:41:57 -0700 Subject: [PATCH 8/8] s/reactRouterInstrumentation/createrReactRouterInstrumentation --- packages/react/src/reactrouter.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/react/src/reactrouter.tsx b/packages/react/src/reactrouter.tsx index bf4d140c1d60..00a69d3b0ea8 100644 --- a/packages/react/src/reactrouter.tsx +++ b/packages/react/src/reactrouter.tsx @@ -47,7 +47,7 @@ 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( @@ -55,10 +55,10 @@ export function reactRouterV5Instrumentation( 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[] = [],