Skip to content

Commit 3f505fe

Browse files
committed
fix(vue): Ensure root component render span always ends
1 parent b1fd4a1 commit 3f505fe

File tree

2 files changed

+40
-22
lines changed

2 files changed

+40
-22
lines changed

packages/vue/src/tracing.ts

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,10 @@ const HOOKS: { [key in Operation]: Hook[] } = {
3131
update: ['beforeUpdate', 'updated'],
3232
};
3333

34-
/** Finish top-level span and activity with a debounce configured using `timeout` option */
35-
function finishRootSpan(vm: VueSentry, timestamp: number, timeout: number): void {
34+
/**
35+
* End the top-level span and with a debounce configured using `timeout` option
36+
*/
37+
function maybeEndRootSpan(vm: VueSentry, timestamp: number, timeout: number): void {
3638
if (vm.$_sentryRootSpanTimer) {
3739
clearTimeout(vm.$_sentryRootSpanTimer);
3840
}
@@ -66,6 +68,8 @@ export const createTracingMixins = (options: Partial<TracingOptions> = {}): Mixi
6668

6769
const mixins: Mixins = {};
6870

71+
const rootComponentSpanFinalTimeout = options.timeout || 2000;
72+
6973
for (const operation of hooks) {
7074
// Retrieve corresponding hooks from Vue lifecycle.
7175
// eg. mount => ['beforeMount', 'mounted']
@@ -80,6 +84,14 @@ export const createTracingMixins = (options: Partial<TracingOptions> = {}): Mixi
8084
const isRoot = this.$root === this;
8185

8286
if (isRoot) {
87+
/**
88+
* The root component span is basically an idle child span that remains active
89+
* as long as child components are processed (mounted, updated, etc).
90+
* Every time a child component span ends, we debounce the ending of the root
91+
* component span so that the root span duration still reflects potential future
92+
* child spans.
93+
* See {@link maybeEndRootSpan} for the debounging mechanism.
94+
*/
8395
this.$_sentryRootSpan =
8496
this.$_sentryRootSpan ||
8597
startInactiveSpan({
@@ -90,6 +102,9 @@ export const createTracingMixins = (options: Partial<TracingOptions> = {}): Mixi
90102
},
91103
onlyIfParent: true,
92104
});
105+
106+
// call debounced end function once directly, just in case no child components call it
107+
maybeEndRootSpan(this, timestampInSeconds(), rootComponentSpanFinalTimeout);
93108
}
94109

95110
// Skip components that we don't want to track to minimize the noise and give a more granular control to the user
@@ -101,6 +116,8 @@ export const createTracingMixins = (options: Partial<TracingOptions> = {}): Mixi
101116

102117
// We always want to track root component
103118
if (!isRoot && !shouldTrack) {
119+
// even if we don't track `this` component, we still want to end the root span eventually
120+
maybeEndRootSpan(this, timestampInSeconds(), rootComponentSpanFinalTimeout);
104121
return;
105122
}
106123

@@ -137,7 +154,7 @@ export const createTracingMixins = (options: Partial<TracingOptions> = {}): Mixi
137154
if (!span) return;
138155
span.end();
139156

140-
finishRootSpan(this, timestampInSeconds(), options.timeout || 2000);
157+
maybeEndRootSpan(this, timestampInSeconds(), rootComponentSpanFinalTimeout);
141158
}
142159
};
143160
}

packages/vue/test/tracing/tracingMixin.test.ts

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,10 @@ vi.mock('../../src/vendor/components', () => {
2727
};
2828
});
2929

30-
const mockSpanFactory = (): { name?: string; op?: string; end: Mock; startChild: Mock } => ({
30+
const mockSpanFactory = (): { name?: string; op?: string; end: Mock } => ({
3131
name: undefined,
3232
op: undefined,
3333
end: vi.fn(),
34-
startChild: vi.fn(),
3534
});
3635

3736
vi.useFakeTimers();
@@ -127,23 +126,25 @@ describe('Vue Tracing Mixins', () => {
127126
);
128127
});
129128

130-
it('should finish root component span on timer after component spans end', () => {
131-
// todo/fixme: This root component span is only finished if trackComponents is true --> it should probably be always finished
132-
const mixins = createTracingMixins({ trackComponents: true, timeout: 1000 });
133-
const rootMockSpan = mockSpanFactory();
134-
mockRootInstance.$_sentryRootSpan = rootMockSpan;
135-
136-
// Create and finish a component span
137-
mixins.beforeMount.call(mockVueInstance);
138-
mixins.mounted.call(mockVueInstance);
139-
140-
// Root component span should not end immediately
141-
expect(rootMockSpan.end).not.toHaveBeenCalled();
142-
143-
// After timeout, root component span should end
144-
vi.advanceTimersByTime(1001);
145-
expect(rootMockSpan.end).toHaveBeenCalled();
146-
});
129+
it.each([true, false])(
130+
'should finish root component span on timer after component spans end, if trackComponents is %s',
131+
() => {
132+
const mixins = createTracingMixins({ trackComponents: false, timeout: 1000 });
133+
const rootMockSpan = mockSpanFactory();
134+
mockRootInstance.$_sentryRootSpan = rootMockSpan;
135+
136+
// Create and finish a component span
137+
mixins.beforeMount.call(mockVueInstance);
138+
mixins.mounted.call(mockVueInstance);
139+
140+
// Root component span should not end immediately
141+
expect(rootMockSpan.end).not.toHaveBeenCalled();
142+
143+
// After timeout, root component span should end
144+
vi.advanceTimersByTime(1001);
145+
expect(rootMockSpan.end).toHaveBeenCalled();
146+
},
147+
);
147148
});
148149

149150
describe('Component Span Lifecycle', () => {

0 commit comments

Comments
 (0)