From df5fb86da03b9d0a71f289ae7e108b6b1a724880 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Fri, 3 Mar 2023 09:34:04 +0100 Subject: [PATCH 1/2] feat(vue): Allow to set `routeLabel: 'path'` to opt-out of using name --- packages/vue/src/router.ts | 8 +++-- packages/vue/src/sdk.ts | 1 + packages/vue/src/types.ts | 8 +++++ packages/vue/test/errorHandler.test.ts | 1 + packages/vue/test/router.test.ts | 48 +++++++++++++++++++++++++- 5 files changed, 63 insertions(+), 3 deletions(-) diff --git a/packages/vue/src/router.ts b/packages/vue/src/router.ts index 2bdb088a24cf..88ff27660e82 100644 --- a/packages/vue/src/router.ts +++ b/packages/vue/src/router.ts @@ -1,7 +1,8 @@ -import { captureException, WINDOW } from '@sentry/browser'; +import { captureException, getCurrentHub, WINDOW } from '@sentry/browser'; import type { Transaction, TransactionContext, TransactionSource } from '@sentry/types'; import { getActiveTransaction } from './tracing'; +import type { Options } from './types'; export type VueRouterInstrumentation = ( startTransaction: (context: TransactionContext) => T | undefined, @@ -38,6 +39,9 @@ interface VueRouter { * @param router The Vue Router instance that is used */ export function vueRouterInstrumentation(router: VueRouter): VueRouterInstrumentation { + const client = getCurrentHub().getClient(); + const options = ((client && client.getOptions()) || {}) as Partial; + return ( startTransaction: (context: TransactionContext) => Transaction | undefined, startTransactionOnPageLoad: boolean = true, @@ -81,7 +85,7 @@ export function vueRouterInstrumentation(router: VueRouter): VueRouterInstrument // Determine a name for the routing transaction and where that name came from let transactionName: string = to.path; let transactionSource: TransactionSource = 'url'; - if (to.name) { + if (to.name && options.routeLabel !== 'path') { transactionName = to.name.toString(); transactionSource = 'custom'; } else if (to.matched[0] && to.matched[0].path) { diff --git a/packages/vue/src/sdk.ts b/packages/vue/src/sdk.ts index ecc879bccbd7..9c7d27392f8a 100644 --- a/packages/vue/src/sdk.ts +++ b/packages/vue/src/sdk.ts @@ -16,6 +16,7 @@ const DEFAULT_CONFIG: Options = { hooks: DEFAULT_HOOKS, timeout: 2000, trackComponents: false, + routeLabel: 'name', _metadata: { sdk: { name: 'sentry.javascript.vue', diff --git a/packages/vue/src/types.ts b/packages/vue/src/types.ts index 1cc39b97b887..901bf7216411 100644 --- a/packages/vue/src/types.ts +++ b/packages/vue/src/types.ts @@ -46,6 +46,14 @@ export interface Options extends TracingOptions, BrowserOptions { /** {@link TracingOptions} */ tracingOptions?: Partial; + + /** + * What to use for route labels. + * By default, we use route.name (if set) and else the path. + * + * Default: 'name' + */ + routeLabel: 'name' | 'path'; } /** Vue specific configuration for Tracing Integration */ diff --git a/packages/vue/test/errorHandler.test.ts b/packages/vue/test/errorHandler.test.ts index e7530c91304e..12232c4ece36 100644 --- a/packages/vue/test/errorHandler.test.ts +++ b/packages/vue/test/errorHandler.test.ts @@ -368,6 +368,7 @@ const testHarness = ({ tracingOptions: {}, trackComponents: [], timeout: 0, + routeLabel: 'name', hooks: [] as Operation[], }; diff --git a/packages/vue/test/router.test.ts b/packages/vue/test/router.test.ts index 65482f80f10b..1b8665f9f531 100644 --- a/packages/vue/test/router.test.ts +++ b/packages/vue/test/router.test.ts @@ -1,7 +1,8 @@ import * as SentryBrowser from '@sentry/browser'; import type { Transaction } from '@sentry/types'; +import { createApp } from 'vue'; -import { vueRouterInstrumentation } from '../src'; +import { init, vueRouterInstrumentation } from '../src'; import type { Route } from '../src/router'; import * as vueTracing from '../src/tracing'; @@ -169,6 +170,51 @@ describe('vueRouterInstrumentation()', () => { }, ); + it('allows to configure routeLabel=path', () => { + // Need to setup a proper client with options etc. + const app = createApp({ + template: '
hello
', + }); + const el = document.createElement('div'); + + init({ + app, + defaultIntegrations: false, + routeLabel: 'path', + }); + + app.mount(el); + + // create instrumentation + const instrument = vueRouterInstrumentation(mockVueRouter); + + // instrument + instrument(mockStartTransaction, true, true); + + // check + const beforeEachCallback = mockVueRouter.beforeEach.mock.calls[0][0]; + + const from = testRoutes.normalRoute1; + const to = testRoutes.namedRoute; + beforeEachCallback(to, from, mockNext); + + // first startTx call happens when the instrumentation is initialized (for pageloads) + expect(mockStartTransaction).toHaveBeenLastCalledWith({ + name: '/login', + metadata: { + source: 'route', + }, + data: { + params: to.params, + query: to.query, + }, + op: 'navigation', + tags: { + 'routing.instrumentation': 'vue-router', + }, + }); + }); + it("doesn't overwrite a pageload transaction name it was set to custom before the router resolved the route", () => { const mockedTxn = { setName: jest.fn(), From ab14464bfc2b835496ea1f0a4cf2d05da636a816 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Fri, 3 Mar 2023 12:12:30 +0100 Subject: [PATCH 2/2] ref to option for routing instrumentation --- packages/vue/src/router.ts | 24 +++++++++---- packages/vue/src/sdk.ts | 1 - packages/vue/src/types.ts | 8 ----- packages/vue/test/errorHandler.test.ts | 1 - packages/vue/test/router.test.ts | 48 +++++++++++++++++--------- 5 files changed, 50 insertions(+), 32 deletions(-) diff --git a/packages/vue/src/router.ts b/packages/vue/src/router.ts index 88ff27660e82..617cc516e421 100644 --- a/packages/vue/src/router.ts +++ b/packages/vue/src/router.ts @@ -1,8 +1,17 @@ -import { captureException, getCurrentHub, WINDOW } from '@sentry/browser'; +import { captureException, WINDOW } from '@sentry/browser'; import type { Transaction, TransactionContext, TransactionSource } from '@sentry/types'; import { getActiveTransaction } from './tracing'; -import type { Options } from './types'; + +interface VueRouterInstrumationOptions { + /** + * What to use for route labels. + * By default, we use route.name (if set) and else the path. + * + * Default: 'name' + */ + routeLabel: 'name' | 'path'; +} export type VueRouterInstrumentation = ( startTransaction: (context: TransactionContext) => T | undefined, @@ -36,12 +45,15 @@ interface VueRouter { /** * Creates routing instrumentation for Vue Router v2, v3 and v4 * + * You can optionally pass in an options object with the available option: + * * `routeLabel`: Set this to `route` to opt-out of using `route.name` for transaction names. + * * @param router The Vue Router instance that is used */ -export function vueRouterInstrumentation(router: VueRouter): VueRouterInstrumentation { - const client = getCurrentHub().getClient(); - const options = ((client && client.getOptions()) || {}) as Partial; - +export function vueRouterInstrumentation( + router: VueRouter, + options: Partial = {}, +): VueRouterInstrumentation { return ( startTransaction: (context: TransactionContext) => Transaction | undefined, startTransactionOnPageLoad: boolean = true, diff --git a/packages/vue/src/sdk.ts b/packages/vue/src/sdk.ts index 9c7d27392f8a..ecc879bccbd7 100644 --- a/packages/vue/src/sdk.ts +++ b/packages/vue/src/sdk.ts @@ -16,7 +16,6 @@ const DEFAULT_CONFIG: Options = { hooks: DEFAULT_HOOKS, timeout: 2000, trackComponents: false, - routeLabel: 'name', _metadata: { sdk: { name: 'sentry.javascript.vue', diff --git a/packages/vue/src/types.ts b/packages/vue/src/types.ts index 901bf7216411..1cc39b97b887 100644 --- a/packages/vue/src/types.ts +++ b/packages/vue/src/types.ts @@ -46,14 +46,6 @@ export interface Options extends TracingOptions, BrowserOptions { /** {@link TracingOptions} */ tracingOptions?: Partial; - - /** - * What to use for route labels. - * By default, we use route.name (if set) and else the path. - * - * Default: 'name' - */ - routeLabel: 'name' | 'path'; } /** Vue specific configuration for Tracing Integration */ diff --git a/packages/vue/test/errorHandler.test.ts b/packages/vue/test/errorHandler.test.ts index 12232c4ece36..e7530c91304e 100644 --- a/packages/vue/test/errorHandler.test.ts +++ b/packages/vue/test/errorHandler.test.ts @@ -368,7 +368,6 @@ const testHarness = ({ tracingOptions: {}, trackComponents: [], timeout: 0, - routeLabel: 'name', hooks: [] as Operation[], }; diff --git a/packages/vue/test/router.test.ts b/packages/vue/test/router.test.ts index 1b8665f9f531..2e8fd9a01d0d 100644 --- a/packages/vue/test/router.test.ts +++ b/packages/vue/test/router.test.ts @@ -1,8 +1,7 @@ import * as SentryBrowser from '@sentry/browser'; import type { Transaction } from '@sentry/types'; -import { createApp } from 'vue'; -import { init, vueRouterInstrumentation } from '../src'; +import { vueRouterInstrumentation } from '../src'; import type { Route } from '../src/router'; import * as vueTracing from '../src/tracing'; @@ -171,22 +170,39 @@ describe('vueRouterInstrumentation()', () => { ); it('allows to configure routeLabel=path', () => { - // Need to setup a proper client with options etc. - const app = createApp({ - template: '
hello
', - }); - const el = document.createElement('div'); + // create instrumentation + const instrument = vueRouterInstrumentation(mockVueRouter, { routeLabel: 'path' }); - init({ - app, - defaultIntegrations: false, - routeLabel: 'path', - }); + // instrument + instrument(mockStartTransaction, true, true); + + // check + const beforeEachCallback = mockVueRouter.beforeEach.mock.calls[0][0]; - app.mount(el); + const from = testRoutes.normalRoute1; + const to = testRoutes.namedRoute; + beforeEachCallback(to, from, mockNext); + // first startTx call happens when the instrumentation is initialized (for pageloads) + expect(mockStartTransaction).toHaveBeenLastCalledWith({ + name: '/login', + metadata: { + source: 'route', + }, + data: { + params: to.params, + query: to.query, + }, + op: 'navigation', + tags: { + 'routing.instrumentation': 'vue-router', + }, + }); + }); + + it('allows to configure routeLabel=name', () => { // create instrumentation - const instrument = vueRouterInstrumentation(mockVueRouter); + const instrument = vueRouterInstrumentation(mockVueRouter, { routeLabel: 'name' }); // instrument instrument(mockStartTransaction, true, true); @@ -200,9 +216,9 @@ describe('vueRouterInstrumentation()', () => { // first startTx call happens when the instrumentation is initialized (for pageloads) expect(mockStartTransaction).toHaveBeenLastCalledWith({ - name: '/login', + name: 'login-screen', metadata: { - source: 'route', + source: 'custom', }, data: { params: to.params,