diff --git a/src/vs/workbench/api/common/extHost.api.impl.ts b/src/vs/workbench/api/common/extHost.api.impl.ts index 65648a6b972..9ce04205fda 100644 --- a/src/vs/workbench/api/common/extHost.api.impl.ts +++ b/src/vs/workbench/api/common/extHost.api.impl.ts @@ -81,7 +81,7 @@ import { ExtHostTesting } from 'vs/workbench/api/common/extHostTesting'; import { ExtHostUriOpeners } from 'vs/workbench/api/common/extHostUriOpener'; import { IExtHostSecretState } from 'vs/workbench/api/common/extHostSecretState'; import { IExtHostEditorTabs } from 'vs/workbench/api/common/extHostEditorTabs'; -import { IExtHostTelemetry, isNewAppInstall } from 'vs/workbench/api/common/extHostTelemetry'; +import { ExtHostTelemetryLogger, IExtHostTelemetry, isNewAppInstall } from 'vs/workbench/api/common/extHostTelemetry'; import { ExtHostNotebookKernels } from 'vs/workbench/api/common/extHostNotebookKernels'; import { TextSearchCompleteMessageType } from 'vs/workbench/services/search/common/searchExtTypes'; import { ExtHostNotebookRenderers } from 'vs/workbench/api/common/extHostNotebookRenderers'; @@ -336,9 +336,10 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I get isNewAppInstall() { return isNewAppInstall(initData.telemetryInfo.firstSessionDate); }, - createTelemetryLogger(appender: vscode.TelemetrySender): vscode.TelemetryLogger { + createTelemetryLogger(sender: vscode.TelemetrySender): vscode.TelemetryLogger { checkProposedApiEnabled(extension, 'telemetryLogger'); - return extHostTelemetry.instantiateLogger(extension, appender); + ExtHostTelemetryLogger.validateSender(sender); + return extHostTelemetry.instantiateLogger(extension, sender); }, openExternal(uri: URI, options?: { allowContributedOpeners?: boolean | string }) { return extHostWindow.openUri(uri, { diff --git a/src/vs/workbench/api/common/extHostTelemetry.ts b/src/vs/workbench/api/common/extHostTelemetry.ts index 2209e7ba2f5..d896eaff3e5 100644 --- a/src/vs/workbench/api/common/extHostTelemetry.ts +++ b/src/vs/workbench/api/common/extHostTelemetry.ts @@ -128,13 +128,31 @@ export class ExtHostTelemetry implements ExtHostTelemetryShape { } export class ExtHostTelemetryLogger { - private _appender: vscode.TelemetrySender; + + static validateSender(sender: vscode.TelemetrySender): void { + if (typeof sender !== 'object') { + throw new TypeError('TelemetrySender argument is invalid'); + } + if (typeof sender.sendEventData !== 'function') { + throw new TypeError('TelemetrySender.sendEventData must be a function'); + } + if (typeof sender.sendErrorData !== 'function') { + throw new TypeError('TelemetrySender.sendErrorData must be a function'); + } + if (typeof sender.flush !== 'undefined' && typeof sender.flush !== 'function') { + throw new TypeError('TelemetrySender.flush must be a function or undefined'); + } + } + + private readonly _sender: vscode.TelemetrySender; private readonly _onDidChangeEnableStates = new Emitter(); - private _telemetryEnablements: { isUsageEnabled: boolean; isErrorsEnabled: boolean }; - private _apiObject: vscode.TelemetryLogger | undefined; private readonly _ignoreBuiltinCommonProperties: boolean; private readonly _additionalCommonProperties: Record | undefined; public readonly ignoreUnhandledExtHostErrors: boolean; + + private _telemetryEnablements: { isUsageEnabled: boolean; isErrorsEnabled: boolean }; + private _apiObject: vscode.TelemetryLogger | undefined; + constructor( sender: vscode.TelemetrySender, options: vscode.TelemetryLoggerOptions | undefined, @@ -147,7 +165,7 @@ export class ExtHostTelemetryLogger { this.ignoreUnhandledExtHostErrors = options?.ignoreUnhandledErrors ?? false; this._ignoreBuiltinCommonProperties = options?.ignoreBuiltInCommonProperties ?? false; this._additionalCommonProperties = options?.additionalCommonProperties; - this._appender = sender; + this._sender = sender; this._telemetryEnablements = { isUsageEnabled: telemetryEnablements.isUsageEnabled, isErrorsEnabled: telemetryEnablements.isErrorsEnabled }; } @@ -192,7 +210,7 @@ export class ExtHostTelemetryLogger { } data = this.mixInCommonPropsAndCleanData(data || {}); if (!this._inLoggingOnlyMode) { - this._appender.sendEventData(eventName, data); + this._sender.sendEventData(eventName, data); } this._logger.trace(eventName, data); } @@ -212,7 +230,7 @@ export class ExtHostTelemetryLogger { this.logEvent(eventNameOrException, data); } else { // TODO @lramos15, implement cleaning for and logging for this case - this._appender.sendErrorData(eventNameOrException, data); + this._sender.sendErrorData(eventNameOrException, data); } } @@ -237,8 +255,8 @@ export class ExtHostTelemetryLogger { } dispose(): void { - if (this._appender?.flush) { - this._appender.flush(); + if (this._sender?.flush) { + this._sender.flush(); } } } diff --git a/src/vs/workbench/api/test/browser/extHostTelemetry.test.ts b/src/vs/workbench/api/test/browser/extHostTelemetry.test.ts index c52a59303d2..aa477a17049 100644 --- a/src/vs/workbench/api/test/browser/extHostTelemetry.test.ts +++ b/src/vs/workbench/api/test/browser/extHostTelemetry.test.ts @@ -10,7 +10,7 @@ import { DEFAULT_LOG_LEVEL, LogLevel } from 'vs/platform/log/common/log'; import { ITelemetryInfo, TelemetryLevel } from 'vs/platform/telemetry/common/telemetry'; import { TestTelemetryLoggerService } from 'vs/platform/telemetry/test/common/telemetryLogAppender.test'; import { IExtHostInitDataService } from 'vs/workbench/api/common/extHostInitDataService'; -import { ExtHostTelemetry } from 'vs/workbench/api/common/extHostTelemetry'; +import { ExtHostTelemetry, ExtHostTelemetryLogger } from 'vs/workbench/api/common/extHostTelemetry'; import { IEnvironment } from 'vs/workbench/services/extensions/common/extensionHostProtocol'; import { mock } from 'vs/workbench/test/common/workbenchTestServices'; import type { TelemetrySender } from 'vscode'; @@ -91,6 +91,31 @@ suite('ExtHostTelemetry', function () { return logger; }; + test('Validate sender instances', function () { + assert.throws(() => ExtHostTelemetryLogger.validateSender(null)); + assert.throws(() => ExtHostTelemetryLogger.validateSender(1)); + assert.throws(() => ExtHostTelemetryLogger.validateSender({})); + assert.throws(() => { + ExtHostTelemetryLogger.validateSender({ + sendErrorData: () => { }, + sendEventData: true + }); + }); + assert.throws(() => { + ExtHostTelemetryLogger.validateSender({ + sendErrorData: 123, + sendEventData: () => { }, + }); + }); + assert.throws(() => { + ExtHostTelemetryLogger.validateSender({ + sendErrorData: () => { }, + sendEventData: () => { }, + flush: true + }); + }); + }); + test('Ensure logger gets proper telemetry level during initialization', function () { const extensionTelemetry = createExtHostTelemetry(); let config = extensionTelemetry.getTelemetryDetails();