Cache initial telemetry events to avoid wrong value assumption (#140651)

* Remote telemetry caching + new telemetry channel

* Few small fixes

* Fix dumb typos

src/vs/server/remoteAgentEnvironmentImpl.ts

@@ -25,11 +25,9 @@ import { IDiagnosticInfoOptions, IDiagnosticInfo } from 'vs/platform/diagnostics
 import { basename, isAbsolute, join, normalize } from 'vs/base/common/path';
 import { ProcessItem } from 'vs/base/common/processes';
 import { ILog, Translations } from 'vs/workbench/services/extensions/common/extensionPoints';
-import { ITelemetryAppender } from 'vs/platform/telemetry/common/telemetryUtils';
 import { IBuiltInExtension } from 'vs/base/common/product';
 import { IExtensionManagementCLIService, InstallOptions } from 'vs/platform/extensionManagement/common/extensionManagement';
 import { cwd } from 'vs/base/common/process';
-import { IRemoteTelemetryService } from 'vs/server/remoteTelemetryService';
 import { Promises } from 'vs/base/node/pfs';
 import { IProductService } from 'vs/platform/product/common/productService';
 
@@ -60,8 +58,6 @@ export class RemoteAgentEnvironmentChannel implements IServerChannel {
 		private readonly environmentService: IServerEnvironmentService,
 		extensionManagementCLIService: IExtensionManagementCLIService,
 		private readonly logService: ILogService,
-		private readonly telemetryService: IRemoteTelemetryService,
-		private readonly telemetryAppender: ITelemetryAppender | null,
 		private readonly productService: IProductService
 	) {
 		this._logger = new class implements ILog {
@@ -98,11 +94,8 @@ export class RemoteAgentEnvironmentChannel implements IServerChannel {
 	}
 
 	async call(_: any, command: string, arg?: any): Promise<any> {
+		console.log(`Command received: ${command}`);
 		switch (command) {
-			case 'disableTelemetry': {
-				this.telemetryService.permanentlyDisableTelemetry();
-				return;
-			}
 
 			case 'getEnvironmentData': {
 				const args = <IGetEnvironmentDataArguments>arg;
@@ -196,26 +189,6 @@ export class RemoteAgentEnvironmentChannel implements IServerChannel {
 					return diagnosticInfo;
 				});
 			}
-
-			case 'logTelemetry': {
-				const { eventName, data } = arg;
-				// Logging is done directly to the appender instead of through the telemetry service
-				// as the data sent from the client has already had common properties added to it and
-				// has already been sent to the telemetry output channel
-				if (this.telemetryAppender) {
-					return this.telemetryAppender.log(eventName, data);
-				}
-
-				return Promise.resolve();
-			}
-
-			case 'flushTelemetry': {
-				if (this.telemetryAppender) {
-					return this.telemetryAppender.flush();
-				}
-
-				return Promise.resolve();
-			}
 		}
 
 		throw new Error(`IPC Command ${command} not found`);

src/vs/server/remoteExtensionHostAgentServer.ts

@@ -83,6 +83,7 @@ import { ICredentialsService } from 'vs/platform/credentials/common/credentials'
 import { CredentialsMainService } from 'vs/platform/credentials/node/credentialsMainService';
 import { IEncryptionService } from 'vs/workbench/services/encryption/common/encryptionService';
 import { EncryptionMainService } from 'vs/platform/encryption/node/encryptionMainService';
+import { RemoteTelemetryChannel } from 'vs/server/remoteTelemetryChannel';
 
 const SHUTDOWN_TIMEOUT = 5 * 60 * 1000;
 
@@ -305,7 +306,7 @@ export class RemoteExtensionHostAgentServer extends Disposable {
 				piiPaths: [this._environmentService.appRoot]
 			};
 
-			services.set(IRemoteTelemetryService, new SyncDescriptor(RemoteTelemetryService, [config]));
+			services.set(IRemoteTelemetryService, new SyncDescriptor(RemoteTelemetryService, [config, undefined]));
 		} else {
 			services.set(IRemoteTelemetryService, RemoteNullTelemetryService);
 		}
@@ -340,9 +341,12 @@ export class RemoteExtensionHostAgentServer extends Disposable {
 		services.set(ICredentialsService, credentialsService);
 
 		return instantiationService.invokeFunction(accessor => {
-			const remoteExtensionEnvironmentChannel = new RemoteAgentEnvironmentChannel(this._connectionToken, this._environmentService, extensionManagementCLIService, this._logService, accessor.get(IRemoteTelemetryService), appInsightsAppender, this._productService);
+			const remoteExtensionEnvironmentChannel = new RemoteAgentEnvironmentChannel(this._connectionToken, this._environmentService, extensionManagementCLIService, this._logService, this._productService);
 			this._socketServer.registerChannel('remoteextensionsenvironment', remoteExtensionEnvironmentChannel);
 
+			const telemetryChannel = new RemoteTelemetryChannel(accessor.get(IRemoteTelemetryService), appInsightsAppender);
+			this._socketServer.registerChannel('telemetry', telemetryChannel);
+
 			this._socketServer.registerChannel(REMOTE_TERMINAL_CHANNEL_NAME, new RemoteTerminalChannel(this._environmentService, this._logService, ptyService, this._productService));
 
 			const remoteFileSystemChannel = new RemoteAgentFileSystemProviderChannel(this._logService, this._environmentService);

src/vs/server/remoteTelemetryChannel.ts

@@ -0,0 +1,65 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import { Event } from 'vs/base/common/event';
+import { Disposable } from 'vs/base/common/lifecycle';
+import { IServerChannel } from 'vs/base/parts/ipc/common/ipc';
+import { TelemetryLevel } from 'vs/platform/telemetry/common/telemetry';
+import { ITelemetryAppender } from 'vs/platform/telemetry/common/telemetryUtils';
+import { IRemoteTelemetryService } from 'vs/server/remoteTelemetryService';
+
+export class RemoteTelemetryChannel extends Disposable implements IServerChannel {
+	constructor(
+		private readonly telemetryService: IRemoteTelemetryService,
+		private readonly telemetryAppender: ITelemetryAppender | null
+	) {
+		super();
+	}
+
+
+	async call(_: any, command: string, arg?: any): Promise<any> {
+		switch (command) {
+			case 'updateTelemetryLevel': {
+				const { telemetryLevel } = arg;
+				return this.telemetryService.updateInjectedTelemetryLevel(telemetryLevel);
+			}
+
+			case 'logTelemetry': {
+				const { eventName, data } = arg;
+				// Logging is done directly to the appender instead of through the telemetry service
+				// as the data sent from the client has already had common properties added to it and
+				// has already been sent to the telemetry output channel
+				if (this.telemetryAppender) {
+					return this.telemetryAppender.log(eventName, data);
+				}
+
+				return Promise.resolve();
+			}
+
+			case 'flushTelemetry': {
+				if (this.telemetryAppender) {
+					return this.telemetryAppender.flush();
+				}
+
+				return Promise.resolve();
+			}
+		}
+		// Command we cannot handle so we throw an error
+		throw new Error(`IPC Command ${command} not found`);
+	}
+
+	listen(_: any, event: string, arg: any): Event<any> {
+		throw new Error('Not supported');
+	}
+
+	/**
+	 * Disposing the channel also disables the telemetryService as there is
+	 * no longer a way to control it
+	 */
+	public override dispose(): void {
+		this.telemetryService.updateInjectedTelemetryLevel(TelemetryLevel.NONE);
+		super.dispose();
+	}
+}

src/vs/server/remoteTelemetryService.ts

@@ -6,59 +6,104 @@
 import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
 import { refineServiceDecorator } from 'vs/platform/instantiation/common/instantiation';
 import { ClassifiedEvent, GDPRClassification, StrictPropertyCheck } from 'vs/platform/telemetry/common/gdprTypings';
-import { ITelemetryData, ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
+import { ITelemetryData, ITelemetryService, TelemetryLevel } from 'vs/platform/telemetry/common/telemetry';
 import { ITelemetryServiceConfig, TelemetryService } from 'vs/platform/telemetry/common/telemetryService';
 import { NullTelemetryServiceShape } from 'vs/platform/telemetry/common/telemetryUtils';
 
 export interface IRemoteTelemetryService extends ITelemetryService {
-	permanentlyDisableTelemetry(): void
+	updateInjectedTelemetryLevel(telemetryLevel: TelemetryLevel): Promise<void>
+}
+
+interface CachedTelemetryEvent {
+	eventName: string;
+	data?: ITelemetryData;
+	anonymizeFilePaths?: boolean;
+	eventType: 'usage' | 'error';
 }
 
 export class RemoteTelemetryService extends TelemetryService implements IRemoteTelemetryService {
-	private _isDisabled = false;
+	private _telemetryCache: CachedTelemetryEvent[] = [];
+	// Because we cannot read the workspace config on the remote site
+	// the RemoteTeelemtryService is respeonsible for knowing its telemetry level
+	// this is done through IPC calls and initial value injections
+	private _injectedTelemetryLevel: TelemetryLevel | undefined;
 	constructor(
 		config: ITelemetryServiceConfig,
+		injectedTelemetryLevel: TelemetryLevel | undefined,
 		@IConfigurationService _configurationService: IConfigurationService
 	) {
 		super(config, _configurationService);
+		this._injectedTelemetryLevel = injectedTelemetryLevel;
 	}
 
 	override publicLog(eventName: string, data?: ITelemetryData, anonymizeFilePaths?: boolean): Promise<void> {
-		if (this._isDisabled) {
+		if (this._injectedTelemetryLevel === undefined) {
+			// Undefined safety with cache in case super class calls log before cache is initialized in subclass constructor
+			this._telemetryCache?.push({ eventName, data, anonymizeFilePaths, eventType: 'usage' });
+			return Promise.resolve();
+		}
+		if (this._injectedTelemetryLevel < TelemetryLevel.USAGE) {
 			return Promise.resolve(undefined);
 		}
+		console.log(`Logging telemetry: ${eventName}`);
 		return super.publicLog(eventName, data, anonymizeFilePaths);
 	}
 
 	override publicLog2<E extends ClassifiedEvent<T> = never, T extends GDPRClassification<T> = never>(eventName: string, data?: StrictPropertyCheck<T, E>, anonymizeFilePaths?: boolean): Promise<void> {
-		if (this._isDisabled) {
-			return Promise.resolve(undefined);
-		}
-		return super.publicLog2(eventName, data, anonymizeFilePaths);
+		return this.publicLog(eventName, data as ITelemetryData | undefined, anonymizeFilePaths);
 	}
 
 	override publicLogError(errorEventName: string, data?: ITelemetryData): Promise<void> {
-		if (this._isDisabled) {
+		if (this._injectedTelemetryLevel === undefined) {
+			// Undefined safety with cache in case super class calls log before cache is initialized in subclass constructor
+			this._telemetryCache?.push({ eventName: errorEventName, data, eventType: 'error' });
+			return Promise.resolve();
+		}
+		if (this._injectedTelemetryLevel < TelemetryLevel.ERROR) {
 			return Promise.resolve(undefined);
 		}
+		console.log(`Logging telemetry: ${errorEventName}`);
 		return super.publicLogError(errorEventName, data);
 	}
 
 	override publicLogError2<E extends ClassifiedEvent<T> = never, T extends GDPRClassification<T> = never>(eventName: string, data?: StrictPropertyCheck<T, E>): Promise<void> {
-		if (this._isDisabled) {
-			return Promise.resolve(undefined);
-		}
-		return super.publicLogError2(eventName, data);
+		return this.publicLogError(eventName, data as ITelemetryData | undefined);
 	}
 
-	permanentlyDisableTelemetry(): void {
-		this._isDisabled = true;
-		this.dispose();
+	// Flushes all the cached events with the new level
+	async flushTelemetryCache(): Promise<void> {
+		if (this._telemetryCache?.length === 0) {
+			return;
+		}
+		for (const cacheItem of this._telemetryCache) {
+			if (cacheItem.eventType === 'usage') {
+				await this.publicLog(cacheItem.eventName, cacheItem.data, cacheItem.anonymizeFilePaths);
+			} else {
+				await this.publicLogError(cacheItem.eventName, cacheItem.data);
+			}
+		}
+		this._telemetryCache = [];
+	}
+
+	async updateInjectedTelemetryLevel(telemetryLevel: TelemetryLevel): Promise<void> {
+		if (telemetryLevel === undefined) {
+			this._injectedTelemetryLevel = TelemetryLevel.NONE;
+			throw new Error('Telemetry level cannot be undefined. This will cause infinite looping!');
+		}
+		// We always take the most restrictive level because we don't want multiple clients to connect and send data when one client does not consent
+		this._injectedTelemetryLevel = this._injectedTelemetryLevel ? Math.min(this._injectedTelemetryLevel, telemetryLevel) : telemetryLevel;
+		if (this._injectedTelemetryLevel === TelemetryLevel.NONE) {
+			this._telemetryCache = [];
+			this.dispose();
+		} else {
+			// Level was set we're no longer in a pending state we flush the telemetry cache.
+			return this.flushTelemetryCache();
+		}
 	}
 }
 
 export const RemoteNullTelemetryService = new class extends NullTelemetryServiceShape implements IRemoteTelemetryService {
-	permanentlyDisableTelemetry(): void { return; } // No-op, telemetry is already disabled
+	async updateInjectedTelemetryLevel(): Promise<void> { return; } // No-op, telemetry is already disabled
 };
 
 export const IRemoteTelemetryService = refineServiceDecorator<ITelemetryService, IRemoteTelemetryService>(ITelemetryService);

src/vs/workbench/contrib/remote/electron-sandbox/remote.contribution.ts

@@ -29,7 +29,7 @@ import { IRemoteAuthorityResolverService } from 'vs/platform/remote/common/remot
 import { IDownloadService } from 'vs/platform/download/common/download';
 import { OpenLocalFileFolderCommand, OpenLocalFileCommand, OpenLocalFolderCommand, SaveLocalFileCommand, RemoteFileDialogContext } from 'vs/workbench/services/dialogs/browser/simpleFileDialog';
 import { IWorkspaceContextService, WorkbenchState } from 'vs/platform/workspace/common/workspace';
-import { TelemetryLevel, TELEMETRY_SETTING_ID } from 'vs/platform/telemetry/common/telemetry';
+import { TELEMETRY_SETTING_ID } from 'vs/platform/telemetry/common/telemetry';
 import { getTelemetryLevel } from 'vs/platform/telemetry/common/telemetryUtils';
 
 class RemoteChannelsContribution implements IWorkbenchContribution {
@@ -113,11 +113,7 @@ class RemoteTelemetryEnablementUpdater extends Disposable implements IWorkbenchC
 	}
 
 	private updateRemoteTelemetryEnablement(): Promise<void> {
-		if (getTelemetryLevel(this.configurationService) === TelemetryLevel.NONE) {
-			return this.remoteAgentService.disableTelemetry();
-		}
-
-		return Promise.resolve();
+		return this.remoteAgentService.updateTelemetryLevel(getTelemetryLevel(this.configurationService));
 	}
 }
 

src/vs/workbench/services/remote/common/abstractRemoteAgentService.ts

@@ -16,7 +16,7 @@ import { IDiagnosticInfoOptions, IDiagnosticInfo } from 'vs/platform/diagnostics
 import { Emitter } from 'vs/base/common/event';
 import { ISignService } from 'vs/platform/sign/common/sign';
 import { ILogService } from 'vs/platform/log/common/log';
-import { ITelemetryData } from 'vs/platform/telemetry/common/telemetry';
+import { ITelemetryData, TelemetryLevel } from 'vs/platform/telemetry/common/telemetry';
 import { ExtensionIdentifier, IExtensionDescription } from 'vs/platform/extensions/common/extensions';
 import { IProductService } from 'vs/platform/product/common/productService';
 import { URI } from 'vs/base/common/uri';
@@ -97,22 +97,22 @@ export abstract class AbstractRemoteAgentService extends Disposable implements I
 		);
 	}
 
-	disableTelemetry(): Promise<void> {
-		return this._withChannel(
-			channel => RemoteExtensionEnvironmentChannelClient.disableTelemetry(channel),
+	updateTelemetryLevel(telemetryLevel: TelemetryLevel): Promise<void> {
+		return this._withTelemetryChannel(
+			channel => RemoteExtensionEnvironmentChannelClient.updateTelemetryLevel(channel, telemetryLevel),
 			undefined
 		);
 	}
 
 	logTelemetry(eventName: string, data: ITelemetryData): Promise<void> {
-		return this._withChannel(
+		return this._withTelemetryChannel(
 			channel => RemoteExtensionEnvironmentChannelClient.logTelemetry(channel, eventName, data),
 			undefined
 		);
 	}
 
 	flushTelemetry(): Promise<void> {
-		return this._withChannel(
+		return this._withTelemetryChannel(
 			channel => RemoteExtensionEnvironmentChannelClient.flushTelemetry(channel),
 			undefined
 		);
@@ -125,6 +125,14 @@ export abstract class AbstractRemoteAgentService extends Disposable implements I
 		}
 		return connection.withChannel('remoteextensionsenvironment', (channel) => callback(channel, connection));
 	}
+
+	private _withTelemetryChannel<R>(callback: (channel: IChannel, connection: IRemoteAgentConnection) => Promise<R>, fallback: R): Promise<R> {
+		const connection = this.getConnection();
+		if (!connection) {
+			return Promise.resolve(fallback);
+		}
+		return connection.withChannel('telemetry', (channel) => callback(channel, connection));
+	}
 }
 
 export class RemoteAgentConnection extends Disposable implements IRemoteAgentConnection {

src/vs/workbench/services/remote/common/remoteAgentEnvironmentChannel.ts

@@ -10,7 +10,7 @@ import { IChannel } from 'vs/base/parts/ipc/common/ipc';
 import { IExtensionDescription, ExtensionIdentifier } from 'vs/platform/extensions/common/extensions';
 import { IRemoteAgentEnvironment } from 'vs/platform/remote/common/remoteAgentEnvironment';
 import { IDiagnosticInfoOptions, IDiagnosticInfo } from 'vs/platform/diagnostics/common/diagnostics';
-import { ITelemetryData } from 'vs/platform/telemetry/common/telemetry';
+import { ITelemetryData, TelemetryLevel } from 'vs/platform/telemetry/common/telemetry';
 
 export interface IGetEnvironmentDataArguments {
 	remoteAuthority: string;
@@ -111,8 +111,8 @@ export class RemoteExtensionEnvironmentChannelClient {
 		return channel.call<IDiagnosticInfo>('getDiagnosticInfo', options);
 	}
 
-	static disableTelemetry(channel: IChannel): Promise<void> {
-		return channel.call<void>('disableTelemetry');
+	static updateTelemetryLevel(channel: IChannel, telemetryLevel: TelemetryLevel): Promise<void> {
+		return channel.call<void>('updateTelemetryLevel', { telemetryLevel });
 	}
 
 	static logTelemetry(channel: IChannel, eventName: string, data: ITelemetryData): Promise<void> {

src/vs/workbench/services/remote/common/remoteAgentService.ts

@@ -9,7 +9,7 @@ import { IChannel, IServerChannel } from 'vs/base/parts/ipc/common/ipc';
 import { IDiagnosticInfoOptions, IDiagnosticInfo } from 'vs/platform/diagnostics/common/diagnostics';
 import { Event } from 'vs/base/common/event';
 import { PersistentConnectionEvent, ISocketFactory } from 'vs/platform/remote/common/remoteAgentConnection';
-import { ITelemetryData } from 'vs/platform/telemetry/common/telemetry';
+import { ITelemetryData, TelemetryLevel } from 'vs/platform/telemetry/common/telemetry';
 import { ExtensionIdentifier, IExtensionDescription } from 'vs/platform/extensions/common/extensions';
 import { URI } from 'vs/base/common/uri';
 
@@ -42,7 +42,7 @@ export interface IRemoteAgentService {
 	 */
 	scanSingleExtension(extensionLocation: URI, isBuiltin: boolean): Promise<IExtensionDescription | null>;
 	getDiagnosticInfo(options: IDiagnosticInfoOptions): Promise<IDiagnosticInfo | undefined>;
-	disableTelemetry(): Promise<void>;
+	updateTelemetryLevel(telemetryLevel: TelemetryLevel): Promise<void>;
 	logTelemetry(eventName: string, data?: ITelemetryData): Promise<void>;
 	flushTelemetry(): Promise<void>;
 }

src/vs/workbench/services/remote/test/common/testServices.ts

@@ -8,7 +8,7 @@ import { IDiagnosticInfoOptions, IDiagnosticInfo } from 'vs/platform/diagnostics
 import { ExtensionIdentifier, IExtensionDescription } from 'vs/platform/extensions/common/extensions';
 import { ISocketFactory } from 'vs/platform/remote/common/remoteAgentConnection';
 import { IRemoteAgentEnvironment } from 'vs/platform/remote/common/remoteAgentEnvironment';
-import { ITelemetryData } from 'vs/platform/telemetry/common/telemetry';
+import { ITelemetryData, TelemetryLevel } from 'vs/platform/telemetry/common/telemetry';
 import { IRemoteAgentConnection, IRemoteAgentService } from 'vs/workbench/services/remote/common/remoteAgentService';
 
 export class TestRemoteAgentService implements IRemoteAgentService {
@@ -37,7 +37,7 @@ export class TestRemoteAgentService implements IRemoteAgentService {
 	getDiagnosticInfo(options: IDiagnosticInfoOptions): Promise<IDiagnosticInfo | undefined> {
 		throw new Error('Method not implemented.');
 	}
-	disableTelemetry(): Promise<void> {
+	updateTelemetryLevel(telemetryLevel: TelemetryLevel): Promise<void> {
 		throw new Error('Method not implemented.');
 	}
 	logTelemetry(eventName: string, data?: ITelemetryData): Promise<void> {