diff --git a/dev-packages/e2e-tests/test-applications/vue-3/src/views/DelayedView.vue b/dev-packages/e2e-tests/test-applications/vue-3/src/views/DelayedView.vue index c5dad2ae5e76..f426ab742fca 100644 --- a/dev-packages/e2e-tests/test-applications/vue-3/src/views/DelayedView.vue +++ b/dev-packages/e2e-tests/test-applications/vue-3/src/views/DelayedView.vue @@ -5,8 +5,10 @@ import { defineAsyncComponent, h } from 'vue'; // Must stay in sync with `ASYNC_CHILD_DELAY_S` in `tests/performance.test.ts`. const ASYNC_CHILD_DELAY_MS = 300; -// A child that mounts a fixed delay after the rest of the page, so tests can prove the -// `Application Render` span does not wait for late children on either instrumentation path. +// A child that mounts a fixed delay after the rest of the page, so tests can prove how each +// instrumentation path treats late children: +// - mixin: extends the `Application Render` span until the child mounts +// - `app.mount()` wrap: does not observe late children const DelayedChild = defineAsyncComponent( () => new Promise(resolve => { 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 107ac0fa9f82..a7ae9b1599ca 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 @@ -172,10 +172,13 @@ test('sends a pageload span with a route name as span name if available', async }); }); -// True on both variants: the mixin arms one debounce timer per component (`tracing.ts`), so a -// late child never clears the root's earlier timer and the span ends at the root's mount. The -// `app.mount()` wrap only observes the root, so it matches. -test('ends the application render span before a delayed async component mounts', async ({ page }) => { +// Mixin: records render activity from every component, so the late child pushes the root render span's end out to its own mount. +// `app.mount()` wrap: only observes the root, so without the Options API the span still ends before the child mounts. +const [minRenderSpanDuration, maxRenderSpanDuration] = OPTIONS_API_DISABLED + ? [0, ASYNC_CHILD_DELAY_S] + : [ASYNC_CHILD_DELAY_S, 10]; + +test('ends the application render span at the last observed render activity', async ({ page }) => { const spansPromise = collectStreamedSpans('vue-3', spans => spans.some( span => span.is_segment && getSpanOp(span) === 'pageload' && span.attributes['url.path']?.value === '/delayed', @@ -197,7 +200,8 @@ test('ends the application render span before a delayed async component mounts', expect(applicationRenderSpan?.end_timestamp).toEqual(expect.any(Number)); const duration = (applicationRenderSpan?.end_timestamp ?? 0) - (applicationRenderSpan?.start_timestamp ?? 0); - expect(duration).toBeLessThan(ASYNC_CHILD_DELAY_S); + expect(duration).toBeGreaterThanOrEqual(minRenderSpanDuration); + expect(duration).toBeLessThan(maxRenderSpanDuration); }); test('sends a lifecycle span for the root and for each tracked component only', async ({ page }) => { diff --git a/packages/vue/src/rootInstrumentation.ts b/packages/vue/src/rootInstrumentation.ts index 4f0e30dd2b2c..5b16a7346b4b 100644 --- a/packages/vue/src/rootInstrumentation.ts +++ b/packages/vue/src/rootInstrumentation.ts @@ -18,7 +18,8 @@ function createRootViewModel(): VueSentry { * builds where the Options API is compiled out and `app.mixin()` is a silent no-op (Nuxt 5 default). * * Vue runs all `mounted` hooks before `mount()` returns, so the wrap covers the same window as the - * mixin's root hooks. Late mounts extend neither path; the mixin's debounce timers are per component. + * mixin's root hooks. Late mounts extend the span only when mixins are used: they record render activity + * from every component, while the `app.mount()` wrap only observes the root component. */ export function instrumentAppMountWithoutMixin(app: Vue, mixins: Mixins): void { // A second wrap would duplicate the root spans (e.g. user and Nuxt SDK both add the integration). diff --git a/packages/vue/src/tracing.ts b/packages/vue/src/tracing.ts index bbcd1002a376..a6084cd58ff0 100644 --- a/packages/vue/src/tracing.ts +++ b/packages/vue/src/tracing.ts @@ -45,6 +45,7 @@ export interface VueSentry extends ViewModel { }; $_sentryRootComponentSpan?: Span; $_sentryRootComponentSpanTimer?: ReturnType; + $_sentryRootComponentSpanActivity?: number; } // Mappings from operation to corresponding lifecycle hook. @@ -59,18 +60,41 @@ const HOOKS: { [key in Operation]: Hook[] } = { update: ['beforeUpdate', 'updated'], }; -/** End the top-level component span and activity with a debounce configured using `timeout` option */ +/** + * End the root component span once no render activity happened for `timeout` (a debounce). + * + * All debounce state lives on `$root`, so hooks from every component share one timer. Each component + * only writes a timestamp (no new timer is created). So mounting any number of components uses a single timer. + */ function maybeEndRootComponentSpan(vm: VueSentry, timestamp: number, timeout: number): void { - if (vm.$_sentryRootComponentSpanTimer) { - clearTimeout(vm.$_sentryRootComponentSpanTimer); + const root = vm.$root; + root.$_sentryRootComponentSpanActivity = timestamp; + + if (!root.$_sentryRootComponentSpanTimer) { + scheduleRootComponentSpanEnd(root, timestamp, timeout); } +} + +/** + * Fires `delayMs` after the activity that scheduled it. Activity recorded in the meantime pushes + * the deadline out by the recorded gap, so the span always ends at the last activity timestamp. + */ +function scheduleRootComponentSpanEnd(root: VueSentry, activityWhenScheduled: number, delayMs: number): void { + root.$_sentryRootComponentSpanTimer = setTimeout(() => { + root.$_sentryRootComponentSpanTimer = undefined; + + const lastActivity = root.$_sentryRootComponentSpanActivity ?? activityWhenScheduled; + if (lastActivity > activityWhenScheduled) { + // activity happened after this timer was scheduled: push the deadline out + scheduleRootComponentSpanEnd(root, lastActivity, (lastActivity - activityWhenScheduled) * 1000); + return; + } - vm.$_sentryRootComponentSpanTimer = setTimeout(() => { - if (vm.$root?.$_sentryRootComponentSpan) { - vm.$root.$_sentryRootComponentSpan.end(timestamp); - vm.$root.$_sentryRootComponentSpan = undefined; + if (root.$_sentryRootComponentSpan) { + root.$_sentryRootComponentSpan.end(lastActivity); + root.$_sentryRootComponentSpan = undefined; } - }, timeout); + }, delayMs); } /** Find if the current component exists in the provided `TracingOptions.trackComponents` array option. */ diff --git a/packages/vue/test/integration/mixinRegistration.test.ts b/packages/vue/test/integration/mixinRegistration.test.ts index 9f9a03aa71d4..8b5eb9de4048 100644 --- a/packages/vue/test/integration/mixinRegistration.test.ts +++ b/packages/vue/test/integration/mixinRegistration.test.ts @@ -2,6 +2,7 @@ * @vitest-environment jsdom */ +import type * as SentryCore from '@sentry/core'; import { spanToJSON } from '@sentry/core'; import type { MockInstance } from 'vitest'; import { afterEach, beforeEach, describe, expect, it as baseIt, vi } from 'vitest'; @@ -10,6 +11,13 @@ import { createApp, defineAsyncComponent, h, nextTick, ref } from 'vue'; import * as Sentry from '../../src'; import type { Options, TracingOptions } from '../../src/types'; +vi.mock('@sentry/core', async importOriginal => { + return { + ...(await importOriginal()), + timestampInSeconds: () => Date.now() / 1000, + }; +}); + const PUBLIC_DSN = 'https://username@domain/123'; const ROOT_SPAN_TIMEOUT_MS = 100; @@ -259,9 +267,8 @@ describe('tracing mixin span creation', () => { ]); }); - // `maybeEndRootComponentSpan` arms one debounce timer per component, so a late child never - // clears the root's earlier timer, and the root's timer ends the span first. The twin test in - // the disabled describe below proves the `app.mount()` wrap matches. + // The debounce only waits for recorded render activity, so a child that never mounts records + // none and cannot keep the root render span open. it('ends the root render span before a deferred child mounts', ({ uiSpans, initSentry }) => { const { app } = createAppWithDeferredChild(); initSentry({ sdk: { app } }); @@ -274,6 +281,36 @@ describe('tracing mixin span creation', () => { ]); }); + it('extends the root render span while children keep mounting within the timeout', async ({ + uiSpans, + initSentry, + }) => { + const showChild = ref(false); + const child = { name: 'ChildComponent', render: () => h('p', 'child') }; + const app = createApp({ name: 'RootComponent', render: () => h('div', [showChild.value ? h(child) : null]) }); + initSentry({ sdk: { app } }); + const container = document.createElement('div'); + + await Sentry.startSpan({ name: 'pageload' }, async () => { + app.mount(container); + + vi.advanceTimersByTime(ROOT_SPAN_TIMEOUT_MS - 1); + showChild.value = true; + await nextTick(); + + // The original deadline passes, but the child's mount pushed it out. + vi.advanceTimersByTime(2); + expect(uiSpans.map(span => span.op)).not.toContain(UI_RENDER_SPAN_OP); + + vi.advanceTimersByTime(ROOT_SPAN_TIMEOUT_MS + 1); + }); + + expect(uiSpans).toEqual([ + { name: 'Vue ', op: UI_MOUNT_SPAN_OP }, + { name: 'Application Render', op: UI_RENDER_SPAN_OP }, + ]); + }); + it('names UI spans after the component and preserves the original description when span streaming is enabled', ({ app, uiSpans, @@ -350,7 +387,8 @@ describe('tracing mixin span creation', () => { expect(rootInstance.$el).toBe(container.firstElementChild); }); - // Matches the mixin-path twin above: the mixin never waited for late children either. + // Unlike the mixin path, the `app.mount()` wrap only observes the root, so a late child can + // never extend the root render span here. it('ends the root render span before a deferred child mounts', ({ uiSpans, initSentry }) => { const { app } = createAppWithDeferredChild(); disableOptionsApi(app); diff --git a/packages/vue/test/tracing/tracingMixin.test.ts b/packages/vue/test/tracing/tracingMixin.test.ts index 72767c1005b6..5598c98f2f5c 100644 --- a/packages/vue/test/tracing/tracingMixin.test.ts +++ b/packages/vue/test/tracing/tracingMixin.test.ts @@ -1,9 +1,18 @@ import { getActiveSpan, startInactiveSpan } from '@sentry/browser'; +import type * as SentryCore from '@sentry/core'; import type { Mock } from 'vitest'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { DEFAULT_HOOKS } from '../../src/constants'; import { createTracingMixins } from '../../src/tracing'; +const clock = vi.hoisted(() => ({ now: 0 })); +vi.mock('@sentry/core', async importOriginal => { + return { + ...(await importOriginal()), + timestampInSeconds: () => clock.now, + }; +}); + vi.mock('@sentry/browser', () => { return { getActiveSpan: vi.fn(), @@ -42,6 +51,7 @@ describe('Vue Tracing Mixins', () => { beforeEach(() => { vi.clearAllMocks(); + clock.now = 0; mockRootInstance = { $root: null, @@ -148,6 +158,53 @@ describe('Vue Tracing Mixins', () => { ); }); + describe('Root Span Debounce', () => { + it('arms a single shared timer on the root for any number of untracked components', () => { + const mixins = createTracingMixins({ trackComponents: false, timeout: 1000 }); + + mixins.beforeMount.call(mockRootInstance); + for (let i = 0; i < 100; i++) { + const child = { $root: mockRootInstance, componentName: `ChildComponent${i}` }; + mixins.beforeMount.call(child); + mixins.mounted.call(child); + } + + expect(vi.getTimerCount()).toBe(1); + }); + + it('ends the root span at the last recorded activity, pushing the deadline out for late activity', () => { + const mixins = createTracingMixins({ trackComponents: false, timeout: 1000 }); + clock.now = 10; + mixins.beforeMount.call(mockRootInstance); + const rootSpan = mockRootInstance.$_sentryRootComponentSpan; + + clock.now = 10.8; + mixins.mounted.call(mockVueInstance); + + vi.advanceTimersByTime(1000); + expect(rootSpan.end).not.toHaveBeenCalled(); + + vi.advanceTimersByTime(800); + expect(rootSpan.end).toHaveBeenCalledWith(10.8); + }); + + it('records no span when a component mounts after the root span already ended', () => { + const mixins = createTracingMixins({ trackComponents: false, timeout: 1000 }); + mixins.beforeMount.call(mockRootInstance); + const rootSpan = mockRootInstance.$_sentryRootComponentSpan; + vi.advanceTimersByTime(1001); + expect(rootSpan.end).toHaveBeenCalledTimes(1); + + clock.now = 5; + mixins.beforeMount.call(mockVueInstance); + mixins.mounted.call(mockVueInstance); + vi.advanceTimersByTime(1001); + + expect(rootSpan.end).toHaveBeenCalledTimes(1); + expect(mockRootInstance.$_sentryRootComponentSpan).toBeUndefined(); + }); + }); + describe('Component Span Lifecycle', () => { it('should create and end spans correctly through lifecycle hooks', () => { const mixins = createTracingMixins({ trackComponents: true });