From 10bc596f10f4bb3fd9583bfb1ba67fd21f5afd15 Mon Sep 17 00:00:00 2001 From: Kaung Zin Hein Date: Fri, 30 Aug 2024 16:33:08 -0400 Subject: [PATCH 1/5] fix(vue): Update Vue `trackComponents` list to match components with or without `<>` Signed-off-by: Kaung Zin Hein --- packages/vue/src/tracing.ts | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/packages/vue/src/tracing.ts b/packages/vue/src/tracing.ts index 70f662559adf..59ec3b98c85a 100644 --- a/packages/vue/src/tracing.ts +++ b/packages/vue/src/tracing.ts @@ -46,6 +46,20 @@ function finishRootSpan(vm: VueSentry, timestamp: number, timeout: number): void }, timeout); } +// Find if the current component exists in the provided `TracingOptions.trackComponents` array option. +function findTrackComponent(trackComponents: string[], formattedName: string): boolean { + const isMatched = trackComponents.some(compo => { + let currentCompo = compo; + + if (!(currentCompo.startsWith('<') && currentCompo.endsWith('>'))) { + currentCompo = `<${currentCompo}>`; + } + return formattedName === currentCompo; + }); + + return isMatched; +} + export const createTracingMixins = (options: TracingOptions): Mixins => { const hooks = (options.hooks || []) .concat(DEFAULT_HOOKS) @@ -85,7 +99,7 @@ export const createTracingMixins = (options: TracingOptions): Mixins => { // Skip components that we don't want to track to minimize the noise and give a more granular control to the user const name = formatComponentName(this, false); const shouldTrack = Array.isArray(options.trackComponents) - ? options.trackComponents.indexOf(name) > -1 + ? findTrackComponent(options.trackComponents, name) : options.trackComponents; // We always want to track root component From 2c67d730aead3fac4069180d89fc612afb46d72b Mon Sep 17 00:00:00 2001 From: Kaung Zin Hein Date: Thu, 5 Sep 2024 18:23:23 -0400 Subject: [PATCH 2/5] test(vue): Add unit test for `findTrackComponent` function Signed-off-by: Kaung Zin Hein --- packages/vue/src/tracing.ts | 20 ++++++--- .../vue/test/tracing/trackComponents.test.ts | 44 +++++++++++++++++++ 2 files changed, 57 insertions(+), 7 deletions(-) create mode 100644 packages/vue/test/tracing/trackComponents.test.ts diff --git a/packages/vue/src/tracing.ts b/packages/vue/src/tracing.ts index 59ec3b98c85a..fc5bf7390a87 100644 --- a/packages/vue/src/tracing.ts +++ b/packages/vue/src/tracing.ts @@ -47,14 +47,19 @@ function finishRootSpan(vm: VueSentry, timestamp: number, timeout: number): void } // Find if the current component exists in the provided `TracingOptions.trackComponents` array option. -function findTrackComponent(trackComponents: string[], formattedName: string): boolean { - const isMatched = trackComponents.some(compo => { - let currentCompo = compo; +/** + * + */ +export function findTrackComponent(trackComponents: string[], formattedName: string): boolean { + /** + * + */ + function extractComponentName(name: string): string { + return name.replace(/^<([^\s]*)>(?: at [^\s]*)?$/, '$1'); + } - if (!(currentCompo.startsWith('<') && currentCompo.endsWith('>'))) { - currentCompo = `<${currentCompo}>`; - } - return formattedName === currentCompo; + const isMatched = trackComponents.some(compo => { + return extractComponentName(formattedName) === extractComponentName(compo); }); return isMatched; @@ -98,6 +103,7 @@ export const createTracingMixins = (options: TracingOptions): Mixins => { // Skip components that we don't want to track to minimize the noise and give a more granular control to the user const name = formatComponentName(this, false); + const shouldTrack = Array.isArray(options.trackComponents) ? findTrackComponent(options.trackComponents, name) : options.trackComponents; diff --git a/packages/vue/test/tracing/trackComponents.test.ts b/packages/vue/test/tracing/trackComponents.test.ts new file mode 100644 index 000000000000..9115522a3033 --- /dev/null +++ b/packages/vue/test/tracing/trackComponents.test.ts @@ -0,0 +1,44 @@ +import { describe, expect, it } from 'vitest'; +import { findTrackComponent } from '../../src/tracing'; + +describe('findTrackComponent', () => { + describe('when user-defined array contains ``', () => { + it('returns true if a match is found', () => { + // arrange + const trackComponents = ['', '']; + const formattedComponentName = ''; + + // act + const shouldTrack = findTrackComponent(trackComponents, formattedComponentName); + + // assert + expect(shouldTrack).toBe(true); + }); + }); + describe('when user-defined array contains `Component` without the `<>`', () => { + it('returns true if a match is found', () => { + // arrange + const trackComponents = ['ABC', 'XYZ']; + const formattedComponentName = ''; + + // act + const shouldTrack = findTrackComponent(trackComponents, formattedComponentName); + + // assert + expect(shouldTrack).toBe(true); + }); + }); + describe('when the vue file name is include in the formatted component name', () => { + it('returns true if a match is found', () => { + // arrange + const trackComponents = ['ABC', 'XYZ']; + const formattedComponentName = ' at XYZ.vue'; + + // act + const shouldTrack = findTrackComponent(trackComponents, formattedComponentName); + + // assert + expect(shouldTrack).toBe(true); + }); + }); +}); From e7bc6a381cb3df0aa63fe040cfc5b0b668bca919 Mon Sep 17 00:00:00 2001 From: Kaung Zin Hein Date: Thu, 5 Sep 2024 22:49:52 -0400 Subject: [PATCH 3/5] test(vue): Add e2e test cases for trackComponents span Signed-off-by: Kaung Zin Hein --- .../test-applications/vue-3/src/main.ts | 1 + .../vue-3/tests/performance.test.ts | 98 +++++++++++++++++++ packages/vue/src/tracing.ts | 8 +- 3 files changed, 100 insertions(+), 7 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/vue-3/src/main.ts b/dev-packages/e2e-tests/test-applications/vue-3/src/main.ts index f4a01d4285c5..a66ae30fd28f 100644 --- a/dev-packages/e2e-tests/test-applications/vue-3/src/main.ts +++ b/dev-packages/e2e-tests/test-applications/vue-3/src/main.ts @@ -19,6 +19,7 @@ Sentry.init({ }), ], tunnel: `http://localhost:3031/`, // proxy server + trackComponents: ['', 'UserIdErrorView'], }); app.use(router); diff --git a/dev-packages/e2e-tests/test-applications/vue-3/tests/performance.test.ts b/dev-packages/e2e-tests/test-applications/vue-3/tests/performance.test.ts index d9a594b5abe7..a3c6e67005e9 100644 --- a/dev-packages/e2e-tests/test-applications/vue-3/tests/performance.test.ts +++ b/dev-packages/e2e-tests/test-applications/vue-3/tests/performance.test.ts @@ -122,3 +122,101 @@ test('sends a pageload transaction with a route name as transaction name if avai }, }); }); + +test('sends a lifecycle span for the tracked HomeView component - with `<>`', async ({ page }) => { + const transactionPromise = waitForTransaction('vue-3', async transactionEvent => { + return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'pageload'; + }); + + await page.goto(`/`); + + const rootSpan = await transactionPromise; + + expect(rootSpan).toMatchObject({ + contexts: { + trace: { + data: { + 'sentry.source': 'route', + 'sentry.origin': 'auto.pageload.vue', + 'sentry.op': 'pageload', + }, + op: 'pageload', + origin: 'auto.pageload.vue', + }, + }, + spans: expect.arrayContaining([ + // enabled by default + expect.objectContaining({ + data: { + 'sentry.op': 'ui.vue.render', + 'sentry.origin': 'auto.ui.vue', + }, + description: 'Application Render', + op: 'ui.vue.render', + origin: 'auto.ui.vue', + }), + // enabled by default + expect.objectContaining({ + data: { + 'sentry.op': 'ui.vue.mount', + 'sentry.origin': 'auto.ui.vue', + }, + description: 'Vue <>', + op: 'ui.vue.mount', + origin: 'auto.ui.vue', + }), + expect.objectContaining({ + data: { + 'sentry.op': 'ui.vue.mount', + 'sentry.origin': 'auto.ui.vue', + }, + description: 'Vue <>', + op: 'ui.vue.mount', + origin: 'auto.ui.vue', + }), + ]), + transaction: '/', + transaction_info: { + source: 'route', + }, + }); +}); + +test('sends a lifecycle span for the tracked UserIdErrorView component - without `<>`', async ({ page }) => { + const transactionPromise = waitForTransaction('vue-3', async transactionEvent => { + return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'pageload'; + }); + + await page.goto(`/users-error/123`); + + const rootSpan = await transactionPromise; + + expect(rootSpan).toMatchObject({ + contexts: { + trace: { + data: { + 'sentry.source': 'route', + 'sentry.origin': 'auto.pageload.vue', + 'sentry.op': 'pageload', + }, + op: 'pageload', + origin: 'auto.pageload.vue', + }, + }, + spans: expect.arrayContaining([ + expect.objectContaining({ + data: { + 'sentry.op': 'ui.vue.mount', + 'sentry.origin': 'auto.ui.vue', + }, + description: 'Vue <>', + op: 'ui.vue.mount', + origin: 'auto.ui.vue', + }), + ]), + transaction: '/users-error/:id', + transaction_info: { + source: 'route', + }, + }); +}); diff --git a/packages/vue/src/tracing.ts b/packages/vue/src/tracing.ts index fc5bf7390a87..1a7d9a33fd34 100644 --- a/packages/vue/src/tracing.ts +++ b/packages/vue/src/tracing.ts @@ -46,14 +46,8 @@ function finishRootSpan(vm: VueSentry, timestamp: number, timeout: number): void }, timeout); } -// Find if the current component exists in the provided `TracingOptions.trackComponents` array option. -/** - * - */ +/** Find if the current component exists in the provided `TracingOptions.trackComponents` array option. */ export function findTrackComponent(trackComponents: string[], formattedName: string): boolean { - /** - * - */ function extractComponentName(name: string): string { return name.replace(/^<([^\s]*)>(?: at [^\s]*)?$/, '$1'); } From 8f48f36522484f1013fea9d5f95728777e29d3e7 Mon Sep 17 00:00:00 2001 From: Kaung Zin Hein Date: Fri, 6 Sep 2024 09:02:55 -0400 Subject: [PATCH 4/5] fix(vue): Remove extra angle bracket in component span description Signed-off-by: Kaung Zin Hein --- .../test-applications/vue-3/tests/performance.test.ts | 6 +++--- packages/vue/src/tracing.ts | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/vue-3/tests/performance.test.ts b/dev-packages/e2e-tests/test-applications/vue-3/tests/performance.test.ts index a3c6e67005e9..34f5a94bee60 100644 --- a/dev-packages/e2e-tests/test-applications/vue-3/tests/performance.test.ts +++ b/dev-packages/e2e-tests/test-applications/vue-3/tests/performance.test.ts @@ -161,7 +161,7 @@ test('sends a lifecycle span for the tracked HomeView component - with `<>`', as 'sentry.op': 'ui.vue.mount', 'sentry.origin': 'auto.ui.vue', }, - description: 'Vue <>', + description: 'Vue ', op: 'ui.vue.mount', origin: 'auto.ui.vue', }), @@ -170,7 +170,7 @@ test('sends a lifecycle span for the tracked HomeView component - with `<>`', as 'sentry.op': 'ui.vue.mount', 'sentry.origin': 'auto.ui.vue', }, - description: 'Vue <>', + description: 'Vue ', op: 'ui.vue.mount', origin: 'auto.ui.vue', }), @@ -209,7 +209,7 @@ test('sends a lifecycle span for the tracked UserIdErrorView component - without 'sentry.op': 'ui.vue.mount', 'sentry.origin': 'auto.ui.vue', }, - description: 'Vue <>', + description: 'Vue ', op: 'ui.vue.mount', origin: 'auto.ui.vue', }), diff --git a/packages/vue/src/tracing.ts b/packages/vue/src/tracing.ts index 1a7d9a33fd34..dba67aee4f5f 100644 --- a/packages/vue/src/tracing.ts +++ b/packages/vue/src/tracing.ts @@ -123,7 +123,7 @@ export const createTracingMixins = (options: TracingOptions): Mixins => { } this.$_sentrySpans[operation] = startInactiveSpan({ - name: `Vue <${name}>`, + name: `Vue ${name}`, op: `${VUE_OP}.${operation}`, attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.vue', From ab13a0b0d56412a232ea360b814d860fd90e268f Mon Sep 17 00:00:00 2001 From: Kaung Zin Hein Date: Fri, 6 Sep 2024 10:03:40 -0400 Subject: [PATCH 5/5] test(vue): Add new route for trackComponent tests Group trackComponent tests together: 1. With <> 2. Without <> 3. Not tracked Signed-off-by: Kaung Zin Hein --- .../test-applications/vue-3/src/main.ts | 2 +- .../vue-3/src/router/index.ts | 4 ++ .../vue-3/src/views/ComponentMainView.vue | 10 ++++ .../vue-3/src/views/ComponentOneView.vue | 10 ++++ .../vue-3/src/views/ComponentTwoView.vue | 10 ++++ .../vue-3/tests/performance.test.ts | 51 +++++++------------ 6 files changed, 53 insertions(+), 34 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/vue-3/src/views/ComponentMainView.vue create mode 100644 dev-packages/e2e-tests/test-applications/vue-3/src/views/ComponentOneView.vue create mode 100644 dev-packages/e2e-tests/test-applications/vue-3/src/views/ComponentTwoView.vue diff --git a/dev-packages/e2e-tests/test-applications/vue-3/src/main.ts b/dev-packages/e2e-tests/test-applications/vue-3/src/main.ts index a66ae30fd28f..13064ce04080 100644 --- a/dev-packages/e2e-tests/test-applications/vue-3/src/main.ts +++ b/dev-packages/e2e-tests/test-applications/vue-3/src/main.ts @@ -19,7 +19,7 @@ Sentry.init({ }), ], tunnel: `http://localhost:3031/`, // proxy server - trackComponents: ['', 'UserIdErrorView'], + trackComponents: ['ComponentMainView', ''], }); app.use(router); diff --git a/dev-packages/e2e-tests/test-applications/vue-3/src/router/index.ts b/dev-packages/e2e-tests/test-applications/vue-3/src/router/index.ts index aac6fb815f43..3a05e4f1055a 100644 --- a/dev-packages/e2e-tests/test-applications/vue-3/src/router/index.ts +++ b/dev-packages/e2e-tests/test-applications/vue-3/src/router/index.ts @@ -30,6 +30,10 @@ const router = createRouter({ }, ], }, + { + path: '/components', + component: () => import('../views/ComponentMainView.vue'), + }, ], }); diff --git a/dev-packages/e2e-tests/test-applications/vue-3/src/views/ComponentMainView.vue b/dev-packages/e2e-tests/test-applications/vue-3/src/views/ComponentMainView.vue new file mode 100644 index 000000000000..69f60c769f54 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/vue-3/src/views/ComponentMainView.vue @@ -0,0 +1,10 @@ + + + diff --git a/dev-packages/e2e-tests/test-applications/vue-3/src/views/ComponentOneView.vue b/dev-packages/e2e-tests/test-applications/vue-3/src/views/ComponentOneView.vue new file mode 100644 index 000000000000..02c5d133ffb3 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/vue-3/src/views/ComponentOneView.vue @@ -0,0 +1,10 @@ + + + diff --git a/dev-packages/e2e-tests/test-applications/vue-3/src/views/ComponentTwoView.vue b/dev-packages/e2e-tests/test-applications/vue-3/src/views/ComponentTwoView.vue new file mode 100644 index 000000000000..f5a1c3c5f46d --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/vue-3/src/views/ComponentTwoView.vue @@ -0,0 +1,10 @@ + + + diff --git a/dev-packages/e2e-tests/test-applications/vue-3/tests/performance.test.ts b/dev-packages/e2e-tests/test-applications/vue-3/tests/performance.test.ts index 34f5a94bee60..03c2cdbb01e5 100644 --- a/dev-packages/e2e-tests/test-applications/vue-3/tests/performance.test.ts +++ b/dev-packages/e2e-tests/test-applications/vue-3/tests/performance.test.ts @@ -123,12 +123,12 @@ test('sends a pageload transaction with a route name as transaction name if avai }); }); -test('sends a lifecycle span for the tracked HomeView component - with `<>`', async ({ page }) => { +test('sends a lifecycle span for each tracked components', async ({ page }) => { const transactionPromise = waitForTransaction('vue-3', async transactionEvent => { return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'pageload'; }); - await page.goto(`/`); + await page.goto(`/components`); const rootSpan = await transactionPromise; @@ -165,56 +165,41 @@ test('sends a lifecycle span for the tracked HomeView component - with `<>`', as op: 'ui.vue.mount', origin: 'auto.ui.vue', }), + + // without `<>` expect.objectContaining({ data: { 'sentry.op': 'ui.vue.mount', 'sentry.origin': 'auto.ui.vue', }, - description: 'Vue ', + description: 'Vue ', op: 'ui.vue.mount', origin: 'auto.ui.vue', }), - ]), - transaction: '/', - transaction_info: { - source: 'route', - }, - }); -}); -test('sends a lifecycle span for the tracked UserIdErrorView component - without `<>`', async ({ page }) => { - const transactionPromise = waitForTransaction('vue-3', async transactionEvent => { - return !!transactionEvent?.transaction && transactionEvent.contexts?.trace?.op === 'pageload'; - }); - - await page.goto(`/users-error/123`); - - const rootSpan = await transactionPromise; - - expect(rootSpan).toMatchObject({ - contexts: { - trace: { + // with `<>` + expect.objectContaining({ data: { - 'sentry.source': 'route', - 'sentry.origin': 'auto.pageload.vue', - 'sentry.op': 'pageload', + 'sentry.op': 'ui.vue.mount', + 'sentry.origin': 'auto.ui.vue', }, - op: 'pageload', - origin: 'auto.pageload.vue', - }, - }, - spans: expect.arrayContaining([ - expect.objectContaining({ + description: 'Vue ', + op: 'ui.vue.mount', + origin: 'auto.ui.vue', + }), + + // not tracked + expect.not.objectContaining({ data: { 'sentry.op': 'ui.vue.mount', 'sentry.origin': 'auto.ui.vue', }, - description: 'Vue ', + description: 'Vue ', op: 'ui.vue.mount', origin: 'auto.ui.vue', }), ]), - transaction: '/users-error/:id', + transaction: '/components', transaction_info: { source: 'route', },