From 27324975fe69118f9cc8f817d93e2a81921ac885 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 5 Jun 2025 15:58:03 +0200 Subject: [PATCH 1/3] fix(vue): Ensure root component render span always ends --- packages/vue/src/tracing.ts | 12 +++++- .../vue/test/tracing/tracingMixin.test.ts | 39 ++++++++++--------- 2 files changed, 30 insertions(+), 21 deletions(-) diff --git a/packages/vue/src/tracing.ts b/packages/vue/src/tracing.ts index 5aadfdd876be..bc5ec0beca19 100644 --- a/packages/vue/src/tracing.ts +++ b/packages/vue/src/tracing.ts @@ -32,7 +32,7 @@ const HOOKS: { [key in Operation]: Hook[] } = { }; /** Finish top-level component span and activity with a debounce configured using `timeout` option */ -function finishRootComponentSpan(vm: VueSentry, timestamp: number, timeout: number): void { +function maybeEndRootComponentSpan(vm: VueSentry, timestamp: number, timeout: number): void { if (vm.$_sentryRootComponentSpanTimer) { clearTimeout(vm.$_sentryRootComponentSpanTimer); } @@ -66,6 +66,8 @@ export const createTracingMixins = (options: Partial = {}): Mixi const mixins: Mixins = {}; + const rootComponentSpanFinalTimeout = options.timeout || 2000; + for (const operation of hooks) { // Retrieve corresponding hooks from Vue lifecycle. // eg. mount => ['beforeMount', 'mounted'] @@ -91,6 +93,9 @@ export const createTracingMixins = (options: Partial = {}): Mixi }, onlyIfParent: true, }); + + // call debounced end function once directly, just in case no child components call it + maybeEndRootComponentSpan(this, timestampInSeconds(), rootComponentSpanFinalTimeout); } // 2. Component tracking filter @@ -102,7 +107,10 @@ export const createTracingMixins = (options: Partial = {}): Mixi ? findTrackComponent(options.trackComponents, componentName) : options.trackComponents); + // We always want to track root component if (!shouldTrack) { + // even if we don't track `this` component, we still want to end the root span eventually + maybeEndRootComponentSpan(this, timestampInSeconds(), rootComponentSpanFinalTimeout); return; } @@ -143,7 +151,7 @@ export const createTracingMixins = (options: Partial = {}): Mixi span.end(); // For any "after" hook, also schedule the root component span to finish - finishRootComponentSpan(this, timestampInSeconds(), options.timeout || 2000); + maybeEndRootComponentSpan(this, timestampInSeconds(), options.timeout || 2000); } }; } diff --git a/packages/vue/test/tracing/tracingMixin.test.ts b/packages/vue/test/tracing/tracingMixin.test.ts index d67690271ed2..44553c8d6e7c 100644 --- a/packages/vue/test/tracing/tracingMixin.test.ts +++ b/packages/vue/test/tracing/tracingMixin.test.ts @@ -27,11 +27,10 @@ vi.mock('../../src/vendor/components', () => { }; }); -const mockSpanFactory = (): { name?: string; op?: string; end: Mock; startChild: Mock } => ({ +const mockSpanFactory = (): { name?: string; op?: string; end: Mock } => ({ name: undefined, op: undefined, end: vi.fn(), - startChild: vi.fn(), }); vi.useFakeTimers(); @@ -127,23 +126,25 @@ describe('Vue Tracing Mixins', () => { ); }); - it('should finish root component span on timer after component spans end', () => { - // todo/fixme: This root component span is only finished if trackComponents is true --> it should probably be always finished - const mixins = createTracingMixins({ trackComponents: true, timeout: 1000 }); - const rootMockSpan = mockSpanFactory(); - mockRootInstance.$_sentryRootComponentSpan = rootMockSpan; - - // Create and finish a component span - mixins.beforeMount.call(mockVueInstance); - mixins.mounted.call(mockVueInstance); - - // Root component span should not end immediately - expect(rootMockSpan.end).not.toHaveBeenCalled(); - - // After timeout, root component span should end - vi.advanceTimersByTime(1001); - expect(rootMockSpan.end).toHaveBeenCalled(); - }); + it.each([true, false])( + 'should finish root component span on timer after component spans end, if trackComponents is %s', + () => { + const mixins = createTracingMixins({ trackComponents: false, timeout: 1000 }); + const rootMockSpan = mockSpanFactory(); + mockRootInstance.$_sentryRootComponentSpan = rootMockSpan; + + // Create and finish a component span + mixins.beforeMount.call(mockVueInstance); + mixins.mounted.call(mockVueInstance); + + // Root component span should not end immediately + expect(rootMockSpan.end).not.toHaveBeenCalled(); + + // After timeout, root component span should end + vi.advanceTimersByTime(1001); + expect(rootMockSpan.end).toHaveBeenCalled(); + }, + ); }); describe('Component Span Lifecycle', () => { From e317b25154e089cb83f4aba3c8b4d67616bf6944 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 5 Jun 2025 16:05:24 +0200 Subject: [PATCH 2/3] cleanup --- packages/vue/src/tracing.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/vue/src/tracing.ts b/packages/vue/src/tracing.ts index bc5ec0beca19..202face147d9 100644 --- a/packages/vue/src/tracing.ts +++ b/packages/vue/src/tracing.ts @@ -31,7 +31,7 @@ const HOOKS: { [key in Operation]: Hook[] } = { update: ['beforeUpdate', 'updated'], }; -/** Finish top-level component span and activity with a debounce configured using `timeout` option */ +/** End the top-level component span and activity with a debounce configured using `timeout` option */ function maybeEndRootComponentSpan(vm: VueSentry, timestamp: number, timeout: number): void { if (vm.$_sentryRootComponentSpanTimer) { clearTimeout(vm.$_sentryRootComponentSpanTimer); @@ -125,7 +125,7 @@ export const createTracingMixins = (options: Partial = {}): Mixi if (activeSpan) { // Cancel any existing span for this operation (safety measure) // We're actually not sure if it will ever be the case that cleanup hooks were not called. - // However, we had users report that spans didn't get finished, so we finished the span before + // However, we had users report that spans didn't end, so we end the span before // starting a new one, just to be sure. const oldSpan = this.$_sentryComponentSpans[operation]; if (oldSpan) { @@ -150,8 +150,8 @@ export const createTracingMixins = (options: Partial = {}): Mixi if (!span) return; // Skip if no span was created in the "before" hook span.end(); - // For any "after" hook, also schedule the root component span to finish - maybeEndRootComponentSpan(this, timestampInSeconds(), options.timeout || 2000); + // For any "after" hook, also schedule the root component span to end + maybeEndRootComponentSpan(this, timestampInSeconds(), rootComponentSpanFinalTimeout); } }; } From c608506febed7f052bea20ac59115f005632e3bd Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 6 Jun 2025 13:45:55 +0200 Subject: [PATCH 3/3] actually use test parameters --- packages/vue/test/tracing/tracingMixin.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/vue/test/tracing/tracingMixin.test.ts b/packages/vue/test/tracing/tracingMixin.test.ts index 44553c8d6e7c..2c08a20c61cd 100644 --- a/packages/vue/test/tracing/tracingMixin.test.ts +++ b/packages/vue/test/tracing/tracingMixin.test.ts @@ -128,8 +128,8 @@ describe('Vue Tracing Mixins', () => { it.each([true, false])( 'should finish root component span on timer after component spans end, if trackComponents is %s', - () => { - const mixins = createTracingMixins({ trackComponents: false, timeout: 1000 }); + trackComponents => { + const mixins = createTracingMixins({ trackComponents, timeout: 1000 }); const rootMockSpan = mockSpanFactory(); mockRootInstance.$_sentryRootComponentSpan = rootMockSpan;