From fed1bce6c715eb31b83764a94a1546ea6f6e64f0 Mon Sep 17 00:00:00 2001 From: Harald Kirschner Date: Mon, 4 Apr 2022 18:13:55 -0700 Subject: [PATCH 1/3] Instrument contributed extension commands --- .../workbench/api/common/extHostCommands.ts | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/vs/workbench/api/common/extHostCommands.ts b/src/vs/workbench/api/common/extHostCommands.ts index c8fb9cf0ed6..392f622e370 100644 --- a/src/vs/workbench/api/common/extHostCommands.ts +++ b/src/vs/workbench/api/common/extHostCommands.ts @@ -8,7 +8,7 @@ import { ICommandHandlerDescription } from 'vs/platform/commands/common/commands import * as extHostTypes from 'vs/workbench/api/common/extHostTypes'; import * as extHostTypeConverter from 'vs/workbench/api/common/extHostTypeConverters'; import { cloneAndChange } from 'vs/base/common/objects'; -import { MainContext, MainThreadCommandsShape, ExtHostCommandsShape, ICommandDto, ICommandHandlerDescriptionDto } from './extHost.protocol'; +import { MainContext, MainThreadCommandsShape, ExtHostCommandsShape, ICommandDto, ICommandHandlerDescriptionDto, MainThreadTelemetryShape } from './extHost.protocol'; import { isNonEmptyArray } from 'vs/base/common/arrays'; import * as languages from 'vs/editor/common/languages'; import type * as vscode from 'vscode'; @@ -46,6 +46,7 @@ export class ExtHostCommands implements ExtHostCommandsShape { private readonly _commands = new Map(); private readonly _apiCommands = new Map(); + private readonly _telemetry: MainThreadTelemetryShape; private readonly _logService: ILogService; private readonly _argumentProcessors: ArgumentProcessor[]; @@ -58,6 +59,7 @@ export class ExtHostCommands implements ExtHostCommandsShape { ) { this.#proxy = extHostRpc.getProxy(MainContext.MainThreadCommands); this._logService = logService; + this._telemetry = extHostRpc.getProxy(MainContext.MainThreadTelemetry); this.converter = new CommandsConverter( this, id => { @@ -219,6 +221,20 @@ export class ExtHostCommands implements ExtHostCommandsShape { if (!command) { throw new Error('Unknown command'); } + type ExtensionActionTelemetry = { + extensionId: string; + id: string; + }; + type ExtensionActionTelemetryMeta = { + extensionId: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; owner: 'digitarald'; comment: 'The id of the extension handling the command, informing which extensions provide most-used functionality.' }; + id: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; owner: 'digitarald'; comment: 'The id of the command, to understand which specific extension features are most popular.' }; + }; + if (command.extension && !command.extension.isBuiltin) { + this._telemetry.$publicLog2('Extension:ActionExecuted', { + extensionId: command.extension?.identifier.value, + id: id, + }); + } let { callback, thisArg, description } = command; if (description) { for (let i = 0; i < description.args.length; i++) { @@ -230,6 +246,7 @@ export class ExtHostCommands implements ExtHostCommandsShape { } } + try { return await callback.apply(thisArg, args); } catch (err) { From 9820963898b97e48d6f99541a412489e8264344e Mon Sep 17 00:00:00 2001 From: Harald Kirschner Date: Tue, 5 Apr 2022 09:19:47 -0700 Subject: [PATCH 2/3] Refactor based on feedback --- .../workbench/api/common/extHostCommands.ts | 38 ++++++++++--------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/src/vs/workbench/api/common/extHostCommands.ts b/src/vs/workbench/api/common/extHostCommands.ts index 392f622e370..51dcbf54ccc 100644 --- a/src/vs/workbench/api/common/extHostCommands.ts +++ b/src/vs/workbench/api/common/extHostCommands.ts @@ -46,7 +46,7 @@ export class ExtHostCommands implements ExtHostCommandsShape { private readonly _commands = new Map(); private readonly _apiCommands = new Map(); - private readonly _telemetry: MainThreadTelemetryShape; + #telemetry: MainThreadTelemetryShape; private readonly _logService: ILogService; private readonly _argumentProcessors: ArgumentProcessor[]; @@ -59,7 +59,7 @@ export class ExtHostCommands implements ExtHostCommandsShape { ) { this.#proxy = extHostRpc.getProxy(MainContext.MainThreadCommands); this._logService = logService; - this._telemetry = extHostRpc.getProxy(MainContext.MainThreadTelemetry); + this.#telemetry = extHostRpc.getProxy(MainContext.MainThreadTelemetry); this.converter = new CommandsConverter( this, id => { @@ -221,20 +221,7 @@ export class ExtHostCommands implements ExtHostCommandsShape { if (!command) { throw new Error('Unknown command'); } - type ExtensionActionTelemetry = { - extensionId: string; - id: string; - }; - type ExtensionActionTelemetryMeta = { - extensionId: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; owner: 'digitarald'; comment: 'The id of the extension handling the command, informing which extensions provide most-used functionality.' }; - id: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; owner: 'digitarald'; comment: 'The id of the command, to understand which specific extension features are most popular.' }; - }; - if (command.extension && !command.extension.isBuiltin) { - this._telemetry.$publicLog2('Extension:ActionExecuted', { - extensionId: command.extension?.identifier.value, - id: id, - }); - } + this.reportTelemetry(command, id); let { callback, thisArg, description } = command; if (description) { for (let i = 0; i < description.args.length; i++) { @@ -246,7 +233,6 @@ export class ExtHostCommands implements ExtHostCommandsShape { } } - try { return await callback.apply(thisArg, args); } catch (err) { @@ -274,6 +260,24 @@ export class ExtHostCommands implements ExtHostCommandsShape { } } + private reportTelemetry(command: CommandHandler, id: string) { + if (!command.extension || command.extension.isBuiltin) { + return; + } + type ExtensionActionTelemetry = { + extensionId: string; + id: string; + }; + type ExtensionActionTelemetryMeta = { + extensionId: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; owner: 'digitarald'; comment: 'The id of the extension handling the command, informing which extensions provide most-used functionality.' }; + id: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; owner: 'digitarald'; comment: 'The id of the command, to understand which specific extension features are most popular.' }; + }; + this.#telemetry.$publicLog2('Extension:ActionExecuted', { + extensionId: command.extension?.identifier.value, + id: id, + }); + } + $executeContributedCommand(id: string, ...args: any[]): Promise { this._logService.trace('ExtHostCommands#$executeContributedCommand', id); From f182622565b4c4d79ece5dd6e8e22c596aa29e9f Mon Sep 17 00:00:00 2001 From: Johannes Date: Wed, 6 Apr 2022 10:17:46 +0200 Subject: [PATCH 3/3] :lipstick: --- src/vs/workbench/api/common/extHostCommands.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/vs/workbench/api/common/extHostCommands.ts b/src/vs/workbench/api/common/extHostCommands.ts index 51dcbf54ccc..8d04fb1d03d 100644 --- a/src/vs/workbench/api/common/extHostCommands.ts +++ b/src/vs/workbench/api/common/extHostCommands.ts @@ -221,7 +221,7 @@ export class ExtHostCommands implements ExtHostCommandsShape { if (!command) { throw new Error('Unknown command'); } - this.reportTelemetry(command, id); + this._reportTelemetry(command, id); let { callback, thisArg, description } = command; if (description) { for (let i = 0; i < description.args.length; i++) { @@ -260,7 +260,7 @@ export class ExtHostCommands implements ExtHostCommandsShape { } } - private reportTelemetry(command: CommandHandler, id: string) { + private _reportTelemetry(command: CommandHandler, id: string) { if (!command.extension || command.extension.isBuiltin) { return; } @@ -273,7 +273,7 @@ export class ExtHostCommands implements ExtHostCommandsShape { id: { classification: 'SystemMetaData'; purpose: 'FeatureInsight'; owner: 'digitarald'; comment: 'The id of the command, to understand which specific extension features are most popular.' }; }; this.#telemetry.$publicLog2('Extension:ActionExecuted', { - extensionId: command.extension?.identifier.value, + extensionId: command.extension.identifier.value, id: id, }); }