diff --git a/packages/vue/src/tracing.ts b/packages/vue/src/tracing.ts index 5aadfdd876be..202face147d9 100644 --- a/packages/vue/src/tracing.ts +++ b/packages/vue/src/tracing.ts @@ -31,8 +31,8 @@ const HOOKS: { [key in Operation]: Hook[] } = { update: ['beforeUpdate', 'updated'], }; -/** Finish top-level component span and activity with a debounce configured using `timeout` option */ -function finishRootComponentSpan(vm: VueSentry, timestamp: number, timeout: number): void { +/** 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); } @@ -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; } @@ -117,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) { @@ -142,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 - finishRootComponentSpan(this, timestampInSeconds(), options.timeout || 2000); + // For any "after" hook, also schedule the root component span to end + maybeEndRootComponentSpan(this, timestampInSeconds(), rootComponentSpanFinalTimeout); } }; } diff --git a/packages/vue/test/tracing/tracingMixin.test.ts b/packages/vue/test/tracing/tracingMixin.test.ts index d67690271ed2..2c08a20c61cd 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', + trackComponents => { + const mixins = createTracingMixins({ trackComponents, 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', () => {