Skip to content

Add telemetry for tree view concurrent refresh#291540

Merged
alexr00 merged 3 commits intomainfrom
alexr00/solid-locust
Jan 29, 2026
Merged

Add telemetry for tree view concurrent refresh#291540
alexr00 merged 3 commits intomainfrom
alexr00/solid-locust

Conversation

@alexr00
Copy link
Member

@alexr00 alexr00 commented Jan 29, 2026

No description provided.

Copilot AI review requested due to automatic review settings January 29, 2026 09:59
@alexr00 alexr00 enabled auto-merge (squash) January 29, 2026 09:59
@alexr00 alexr00 self-assigned this Jan 29, 2026
@vs-code-engineering vs-code-engineering bot added this to the January 2026 milestone Jan 29, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds telemetry and retry handling around tree view node resolution when concurrent refreshes invalidate child fetches.

Changes:

  • Add bounded retry logic to ExtHostTreeView._resolveTreeNode when getChildren() returns undefined due to concurrent refresh invalidation, and emit retry telemetry via the main thread.
  • Extend the ext host ↔ main thread protocol with $logResolveTreeNodeRetry and implement telemetry emission in MainThreadTreeViews.
  • Update the MainThreadTreeViews browser test to pass a telemetry service.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
src/vs/workbench/api/test/browser/mainThreadTreeViews.test.ts Passes NullTelemetryService to MainThreadTreeViews in the test setup.
src/vs/workbench/api/common/extHostTreeViews.ts Implements retry-on-invalidated-children logic and calls into the main thread to log retry telemetry.
src/vs/workbench/api/common/extHost.protocol.ts Adds $logResolveTreeNodeRetry to the MainThreadTreeViewsShape RPC surface.
src/vs/workbench/api/browser/mainThreadTreeViews.ts Injects ITelemetryService and logs the new treeView.resolveRetry telemetry event.
Comments suppressed due to low confidence (2)

src/vs/workbench/api/common/extHostTreeViews.ts:714

  • The retry limit 3 is hard-coded in multiple places (condition, log message, telemetry semantics). To avoid accidental divergence if this value changes, consider introducing a single const MAX_RESOLVE_RETRIES = 3 (or similar) and reusing it for the comparison and for the attempt x/y message.
		if (children === undefined && retryCount < 3) {
			const newRetryCount = retryCount + 1;
			this._logService.warn(`[${this._viewId}] Retrying _resolveTreeNode due to concurrent refresh (attempt ${newRetryCount}/3) for element ${handle} from extension ${this._extension.identifier.value}`);

src/vs/workbench/api/common/extHostTreeViews.ts:720

  • The new retry/telemetry behavior in _resolveTreeNode isn’t covered by tests. Since this code path is race-sensitive (concurrent refresh invalidating _fetchChildrenNodes leading to getChildren() returning undefined), it would be good to add a focused unit test (e.g. in src/vs/workbench/api/test/browser/extHostTreeViews.test.ts) that simulates token invalidation and asserts: (1) _resolveTreeNode retries up to the limit and (2) $logResolveTreeNodeRetry is called with the expected parameters.
	private async _resolveTreeNode(element: T, parent?: TreeNode, retryCount = 0): Promise<TreeNode> {
		const node = this._nodes.get(element);
		if (node) {
			return node;
		}
		const extTreeItem = await asPromise(() => this._dataProvider.getTreeItem(element));
		const handle = this._createHandle(element, extTreeItem, parent, true);
		const children = await this.getChildren(parent ? parent.item.handle : undefined);
		// If getChildren returned undefined, it means a concurrent refresh invalidated
		// the fetch. Retry a limited number of times to handle transient refresh races.
		if (children === undefined && retryCount < 3) {
			const newRetryCount = retryCount + 1;
			this._logService.warn(`[${this._viewId}] Retrying _resolveTreeNode due to concurrent refresh (attempt ${newRetryCount}/3) for element ${handle} from extension ${this._extension.identifier.value}`);
			if (newRetryCount === 1) {
				// Log telemetry on first retry
				this._proxy.$logResolveTreeNodeRetry(this._viewId, this._extension.identifier.value, newRetryCount, false);
			}
			return this._resolveTreeNode(element, parent, newRetryCount);
		}

Comment on lines 143 to 156
$logResolveTreeNodeRetry(_treeViewId: string, extensionId: string, retryCount: number, exhausted: boolean): void {
type TreeViewResolveRetryEvent = {
extensionId: string;
retryCount: number;
exhausted: boolean;
};
type TreeViewResolveRetryClassification = {
extensionId: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; comment: 'The extension identifier.' };
retryCount: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth'; isMeasurement: true; comment: 'Number of retry attempts made.' };
exhausted: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth'; comment: 'Whether all retry attempts were exhausted.' };
owner: 'alexr00';
comment: 'Tracks tree view resolve retries due to concurrent refresh races.';
};
this.telemetryService.publicLog2<TreeViewResolveRetryEvent, TreeViewResolveRetryClassification>('treeView.resolveRetry', {
Copy link

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

treeViewId is passed over RPC but intentionally unused here. If it’s not meant to be collected, consider removing it from the protocol/method signature to avoid confusion and unnecessary payload; if it is useful, consider including a sanitized/appropriate representation in the telemetry event so the parameter has a purpose.

Copilot uses AI. Check for mistakes.
sandy081
sandy081 previously approved these changes Jan 29, 2026
@alexr00 alexr00 marked this pull request as draft January 29, 2026 10:24
auto-merge was automatically disabled January 29, 2026 10:24

Pull request was converted to draft

@alexr00 alexr00 marked this pull request as ready for review January 29, 2026 11:04
@alexr00 alexr00 enabled auto-merge (squash) January 29, 2026 11:04
@alexr00 alexr00 merged commit 5181d05 into main Jan 29, 2026
22 checks passed
@alexr00 alexr00 deleted the alexr00/solid-locust branch January 29, 2026 11:10
alexr00 added a commit that referenced this pull request Jan 30, 2026
alexr00 added a commit that referenced this pull request Feb 2, 2026
* Revert "Add telemetry for tree view concurrent refresh (#291540)"

This reverts commit 5181d05.

* Add back telemetry

* Polish

* Also log

* Remove retry
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants