Enhance logger service (microsoft#171174)

- ILoggerService centrailised place for all loggers
- Provide options to display logger while creating/registering logger
- Listen to logger service and register log output channels accordingly
- Adopt this at all places where loggers are created

Author: sandy081

src/vs/code/electron-browser/sharedProcess/sharedProcessMain.ts

@@ -114,6 +114,7 @@ import { ISharedProcessLifecycleService, SharedProcessLifecycleService } from 'v
 import { UserDataSyncResourceProviderService } from 'vs/platform/userDataSync/common/userDataSyncResourceProvider';
 import { ExtensionsContributions } from 'vs/code/electron-browser/sharedProcess/contrib/extensions';
 import { ExtensionsProfileScannerService } from 'vs/platform/extensionManagement/electron-sandbox/extensionsProfileScannerService';
+import { localize } from 'vs/nls';
 
 class SharedProcessMain extends Disposable {
 
@@ -222,7 +223,7 @@ class SharedProcessMain extends Disposable {
 		// Log
 		const multiplexLogger = this._register(new MultiplexLogService([
 			this._register(new ConsoleLogger(this.configuration.logLevel)),
-			this._register(loggerService.createLogger(joinPath(URI.file(environmentService.logsPath), 'sharedprocess.log'), { name: 'sharedprocess' }))
+			this._register(loggerService.createLogger(joinPath(URI.file(environmentService.logsPath), 'sharedprocess.log'), { id: 'sharedLog', name: localize('sharedLog', "Shared") }))
 		]));
 
 		const logService = this._register(new FollowerLogService(logLevelClient, multiplexLogger));
@@ -304,7 +305,7 @@ class SharedProcessMain extends Disposable {
 		const appenders: ITelemetryAppender[] = [];
 		const internalTelemetry = isInternalTelemetry(productService, configurationService);
 		if (supportsTelemetry(productService, environmentService)) {
-			const logAppender = new TelemetryLogAppender(loggerService, environmentService);
+			const logAppender = new TelemetryLogAppender(logService, loggerService, environmentService, productService);
 			appenders.push(logAppender);
 			const { installSourcePath } = environmentService;
 			if (productService.aiConfig?.ariaKey) {
@@ -329,7 +330,7 @@ class SharedProcessMain extends Disposable {
 		services.set(ITelemetryService, telemetryService);
 
 		// Custom Endpoint Telemetry
-		const customEndpointTelemetryService = new CustomEndpointTelemetryService(configurationService, telemetryService, loggerService, environmentService, productService);
+		const customEndpointTelemetryService = new CustomEndpointTelemetryService(configurationService, telemetryService, logService, loggerService, environmentService, productService);
 		services.set(ICustomEndpointTelemetryService, customEndpointTelemetryService);
 
 		// Extension Management

src/vs/code/electron-main/main.ts

@@ -129,7 +129,7 @@ class CodeMain {
 				});
 
 				// Delay creation of spdlog for perf reasons (https://github.com/microsoft/vscode/issues/72906)
-				bufferLogService.logger = loggerService.createLogger(URI.file(join(environmentMainService.logsPath, 'main.log')), { name: 'main' });
+				bufferLogService.logger = loggerService.createLogger(URI.file(join(environmentMainService.logsPath, 'main.log')), { id: 'mainLog', name: localize('mainLog', "Main") });
 
 				// Lifecycle
 				once(lifecycleMainService.onWillShutdown)(evt => {

src/vs/platform/log/common/fileLog.ts

@@ -20,7 +20,6 @@ export class FileLogger extends AbstractMessageLogger implements ILogger {
 	private backupIndex: number = 1;
 
 	constructor(
-		name: string,
 		private readonly resource: URI,
 		level: LogLevel,
 		private readonly donotUseFormatters: boolean,
@@ -107,7 +106,7 @@ export class FileLoggerService extends AbstractLoggerService implements ILoggerS
 
 	protected doCreateLogger(resource: URI, logLevel: LogLevel, options?: ILoggerOptions): ILogger {
 		const logger = new BufferLogService(logLevel);
-		whenProviderRegistered(resource, this.fileService).then(() => (<BufferLogService>logger).logger = new FileLogger(options?.name || basename(resource), resource, logger.getLevel(), !!options?.donotUseFormatters, this.fileService));
+		whenProviderRegistered(resource, this.fileService).then(() => (<BufferLogService>logger).logger = new FileLogger(resource, logger.getLevel(), !!options?.donotUseFormatters, this.fileService));
 		return logger;
 	}
 }

src/vs/platform/log/common/log.ts

@@ -84,6 +84,11 @@ export interface ILogService extends ILogger {
 
 export interface ILoggerOptions {
 
+	/**
+	 * Id of the logger.
+	 */
+	id?: string;
+
 	/**
 	 * Name of the logger.
 	 */
@@ -100,19 +105,31 @@ export interface ILoggerOptions {
 	donotUseFormatters?: boolean;
 
 	/**
-	 * If set, logger logs the message always.
+	 * When to log. Set to `always` to log always.
+	 */
+	logLevel?: 'always' | LogLevel;
+
+	/**
+	 * Whether the log should be hidden from the user.
 	 */
-	always?: boolean;
+	hidden?: boolean;
+
+	/**
+	 * Id of the extension that created this logger.
+	 */
+	extensionId?: string;
 }
 
 export interface ILoggerResource {
 	readonly resource: URI;
+	readonly id: string;
 	readonly name?: string;
-	readonly id?: string;
 	readonly logLevel?: LogLevel;
+	readonly hidden?: boolean;
+	readonly extensionId?: string;
 }
 
-export type DidChangeLoggerResourceEvent = {
+export type DidChangeLoggersEvent = {
 	readonly added: Iterable<ILoggerResource>;
 	readonly removed: Iterable<ILoggerResource>;
 };
@@ -123,14 +140,21 @@ export interface ILoggerService {
 
 	/**
 	 * Creates a logger, or gets one if it already exists.
+	 *
+	 * This will also register the logger with the logger service unless `donotRegister` is set to `true`.
 	 */
-	createLogger(resource: URI, options?: ILoggerOptions, logLevel?: LogLevel): ILogger;
+	createLogger(resource: URI, options?: ILoggerOptions, donotRegister?: boolean): ILogger;
 
 	/**
 	 * Gets an existing logger, if any.
 	 */
 	getLogger(resource: URI): ILogger | undefined;
 
+	/**
+	 * An event which fires when the log level of a logger has changed
+	 */
+	readonly onDidChangeLogLevel: Event<[URI, LogLevel]>;
+
 	/**
 	 * Set log level for a logger.
 	 */
@@ -142,29 +166,45 @@ export interface ILoggerService {
 	getLogLevel(resource: URI): LogLevel | undefined;
 
 	/**
-	 * An event which fires when the log level of a logger has changed
+	 * An event which fires when the visibility of a logger has changed
+	 */
+	readonly onDidChangeVisibility: Event<[URI, boolean]>;
+
+	/**
+	 * Set the visibility of a logger.
 	 */
-	readonly onDidChangeLogLevel: Event<ILoggerResource>;
+	setVisibility(resource: URI, visible: boolean): void;
 
 	/**
 	 * An event which fires when the logger resources are changed
 	 */
-	readonly onDidChangeLoggerResources: Event<DidChangeLoggerResourceEvent>;
+	readonly onDidChangeLoggers: Event<DidChangeLoggersEvent>;
+
+	/**
+	 * Register a logger with the logger service.
+	 *
+	 * Note that this will not create a logger, but only register it.
+	 *
+	 * Use `createLogger` to create a logger and register it.
+	 *
+	 * Use it when you want to register a logger that is not created by the logger service.
+	 */
+	registerLogger(resource: ILoggerResource): void;
 
 	/**
-	 * Register the logger resoruce
+	 * Deregister the logger for the given resource.
 	 */
-	registerLoggerResource(resource: ILoggerResource): void;
+	deregisterLogger(resource: URI): void;
 
 	/**
-	 * Deregister the logger resoruce
+	 * Get all registered loggers
 	 */
-	deregisterLoggerResource(resource: URI): void;
+	getRegisteredLoggers(): Iterable<ILoggerResource>;
 
 	/**
-	 * Get all registered logger resources
+	 * Get the registered logger for the given resource.
 	 */
-	getLoggerResources(): Iterable<ILoggerResource>;
+	getRegisteredLogger(resource: URI): ILoggerResource | undefined;
 }
 
 export abstract class AbstractLogger extends Disposable implements ILogger {
@@ -532,12 +572,15 @@ export abstract class AbstractLoggerService extends Disposable implements ILogge
 
 	private readonly _loggerResources = new ResourceMap<Mutable<ILoggerResource>>();
 
-	private _onDidChangeLoggerResources = this._register(new Emitter<{ added: ILoggerResource[]; removed: ILoggerResource[] }>);
-	readonly onDidChangeLoggerResources = this._onDidChangeLoggerResources.event;
+	private _onDidChangeLoggers = this._register(new Emitter<{ added: ILoggerResource[]; removed: ILoggerResource[] }>);
+	readonly onDidChangeLoggers = this._onDidChangeLoggers.event;
 
-	private _onDidChangeLogLevel = this._register(new Emitter<ILoggerResource>);
+	private _onDidChangeLogLevel = this._register(new Emitter<[URI, LogLevel]>);
 	readonly onDidChangeLogLevel = this._onDidChangeLogLevel.event;
 
+	private _onDidChangeVisibility = this._register(new Emitter<[URI, boolean]>);
+	readonly onDidChangeVisibility = this._onDidChangeVisibility.event;
+
 	constructor(
 		protected logLevel: LogLevel,
 		onDidChangeLogLevel: Event<LogLevel>,
@@ -556,13 +599,15 @@ export abstract class AbstractLoggerService extends Disposable implements ILogge
 		return this._loggers.get(resource);
 	}
 
-	createLogger(resource: URI, options?: ILoggerOptions, logLevel?: LogLevel): ILogger {
+	createLogger(resource: URI, options?: ILoggerOptions, donotRegister?: boolean): ILogger {
 		let logger = this._loggers.get(resource);
 		if (!logger) {
-			logLevel = options?.always ? LogLevel.Trace : logLevel;
+			const logLevel = options?.logLevel === 'always' ? LogLevel.Trace : options?.logLevel;
 			logger = this.doCreateLogger(resource, logLevel ?? this.getLogLevel(resource) ?? this.logLevel, options);
 			this._loggers.set(resource, logger);
-			this.registerLoggerResource({ resource, logLevel, name: options?.name });
+			if (!donotRegister) {
+				this.registerLogger({ resource, id: options?.id ?? resource.toString(), logLevel, name: options?.name, hidden: options?.hidden, extensionId: options?.extensionId });
+			}
 		}
 		return logger;
 	}
@@ -573,7 +618,16 @@ export abstract class AbstractLoggerService extends Disposable implements ILogge
 			loggerResource.logLevel = logLevel === this.logLevel ? undefined : logLevel;
 			this._loggers.get(resource)?.setLevel(logLevel);
 			this._loggerResources.set(loggerResource.resource, loggerResource);
-			this._onDidChangeLogLevel.fire(loggerResource);
+			this._onDidChangeLogLevel.fire([resource, logLevel]);
+		}
+	}
+
+	setVisibility(resource: URI, visibility: boolean): void {
+		const loggerResource = this._loggerResources.get(resource);
+		if (loggerResource && visibility !== !loggerResource.hidden) {
+			loggerResource.hidden = !visibility;
+			this._loggerResources.set(loggerResource.resource, loggerResource);
+			this._onDidChangeVisibility.fire([resource, visibility]);
 		}
 	}
 
@@ -590,26 +644,35 @@ export abstract class AbstractLoggerService extends Disposable implements ILogge
 		return this._loggerResources.get(resource)?.logLevel;
 	}
 
-	registerLoggerResource(resource: ILoggerResource): void {
+	registerLogger(resource: ILoggerResource): void {
 		const existing = this._loggerResources.get(resource.resource);
 		if (!existing) {
 			this._loggerResources.set(resource.resource, resource);
-			this._onDidChangeLoggerResources.fire({ added: [resource], removed: [] });
+			this._onDidChangeLoggers.fire({ added: [resource], removed: [] });
 		}
 	}
 
-	deregisterLoggerResource(resource: URI): void {
+	deregisterLogger(resource: URI): void {
 		const existing = this._loggerResources.get(resource);
 		if (existing) {
 			this._loggerResources.delete(resource);
-			this._onDidChangeLoggerResources.fire({ added: [], removed: [existing] });
+			const logger = this._loggers.get(resource);
+			if (logger) {
+				this._loggers.delete(resource);
+				logger.dispose();
+			}
+			this._onDidChangeLoggers.fire({ added: [], removed: [existing] });
 		}
 	}
 
-	getLoggerResources(): Iterable<ILoggerResource> {
+	getRegisteredLoggers(): Iterable<ILoggerResource> {
 		return this._loggerResources.values();
 	}
 
+	getRegisteredLogger(resource: URI): ILoggerResource | undefined {
+		return this._loggerResources.get(resource);
+	}
+
 	override dispose(): void {
 		this._loggers.forEach(logger => logger.dispose());
 		this._loggers.clear();

src/vs/platform/log/common/logIpc.ts

@@ -6,7 +6,7 @@
 import { Event } from 'vs/base/common/event';
 import { URI, UriDto } from 'vs/base/common/uri';
 import { IChannel, IServerChannel } from 'vs/base/parts/ipc/common/ipc';
-import { AbstractLoggerService, AbstractMessageLogger, AdapterLogger, DidChangeLoggerResourceEvent, ILogger, ILoggerOptions, ILoggerResource, ILoggerService, ILogService, LogLevel, LogService } from 'vs/platform/log/common/log';
+import { AbstractLoggerService, AbstractMessageLogger, AdapterLogger, DidChangeLoggersEvent, ILogger, ILoggerOptions, ILoggerResource, ILoggerService, ILogService, LogLevel, LogService } from 'vs/platform/log/common/log';
 
 export class LogLevelChannel implements IServerChannel {
 
@@ -59,13 +59,14 @@ export class LoggerChannelClient extends AbstractLoggerService implements ILogge
 
 	constructor(private readonly windowId: number | undefined, logLevel: LogLevel, onDidChangeLogLevel: Event<LogLevel>, loggers: UriDto<ILoggerResource>[], private readonly channel: IChannel) {
 		super(logLevel, onDidChangeLogLevel, loggers.map(loggerResource => ({ ...loggerResource, resource: URI.revive(loggerResource.resource) })));
-		this._register(channel.listen<ILoggerResource>('onDidChangeLogLevel', windowId)((loggerResource) => super.setLogLevel(URI.revive(loggerResource.resource), loggerResource.logLevel ?? this.logLevel)));
-		this._register(channel.listen<DidChangeLoggerResourceEvent>('onDidChangeLoggerResources', windowId)(({ added, removed }) => {
+		this._register(channel.listen<[URI, LogLevel]>('onDidChangeLogLevel', windowId)(([resource, logLevel]) => super.setLogLevel(URI.revive(resource), logLevel)));
+		this._register(channel.listen<[URI, boolean]>('onDidChangeVisibility', windowId)(([resource, visibility]) => super.setVisibility(URI.revive(resource), visibility)));
+		this._register(channel.listen<DidChangeLoggersEvent>('onDidChangeLoggers', windowId)(({ added, removed }) => {
 			for (const loggerResource of added) {
-				super.registerLoggerResource({ ...loggerResource, resource: URI.revive(loggerResource.resource) });
+				super.registerLogger({ ...loggerResource, resource: URI.revive(loggerResource.resource) });
 			}
 			for (const loggerResource of removed) {
-				super.deregisterLoggerResource(loggerResource.resource);
+				super.deregisterLogger(loggerResource.resource);
 			}
 		}));
 	}
@@ -78,18 +79,26 @@ export class LoggerChannelClient extends AbstractLoggerService implements ILogge
 		});
 	}
 
-	override registerLoggerResource(resource: ILoggerResource): void {
-		this.channel.call('registerLoggerResource', [resource, this.windowId]);
+	override registerLogger(logger: ILoggerResource): void {
+		super.registerLogger(logger);
+		this.channel.call('registerLogger', [logger, this.windowId]);
 	}
 
-	override deregisterLoggerResource(resource: URI): void {
-		this.channel.call('deregisterLoggerResource', [resource, this.windowId]);
+	override deregisterLogger(resource: URI): void {
+		super.deregisterLogger(resource);
+		this.channel.call('deregisterLogger', [resource, this.windowId]);
 	}
 
 	override setLogLevel(resource: URI, logLevel: LogLevel): void {
+		super.setLogLevel(resource, logLevel);
 		this.channel.call('setLogLevel', [resource, logLevel]);
 	}
 
+	override setVisibility(resource: URI, visibility: boolean): void {
+		super.setVisibility(resource, visibility);
+		this.channel.call('setVisibility', [resource, visibility]);
+	}
+
 	protected doCreateLogger(file: URI, logLevel: LogLevel, options?: ILoggerOptions): ILogger {
 		return new Logger(this.channel, file, logLevel, options);
 	}
@@ -107,7 +116,7 @@ class Logger extends AbstractMessageLogger {
 		logLevel: LogLevel,
 		loggerOptions?: ILoggerOptions,
 	) {
-		super(loggerOptions?.always);
+		super(loggerOptions?.logLevel === 'always');
 		this.setLevel(logLevel);
 		this.channel.call('createLogger', [file, loggerOptions])
 			.then(() => {

src/vs/platform/log/electron-main/logIpc.ts

@@ -17,27 +17,29 @@ export class LoggerChannel implements IServerChannel {
 
 	listen(_: unknown, event: string, windowId?: number): Event<any> {
 		switch (event) {
-			case 'onDidChangeLoggerResources': return windowId ? this.loggerService.getOnDidChangeLoggerResourcesEvent(windowId) : this.loggerService.onDidChangeLoggerResources;
+			case 'onDidChangeLoggers': return windowId ? this.loggerService.getOnDidChangeLoggersEvent(windowId) : this.loggerService.onDidChangeLoggers;
 			case 'onDidChangeLogLevel': return windowId ? this.loggerService.getOnDidChangeLogLevelEvent(windowId) : this.loggerService.onDidChangeLogLevel;
+			case 'onDidChangeVisibility': return windowId ? this.loggerService.getOnDidChangeVisibilityEvent(windowId) : this.loggerService.onDidChangeVisibility;
 		}
 		throw new Error(`Event not found: ${event}`);
 	}
 
 	async call(_: unknown, command: string, arg?: any): Promise<any> {
 		switch (command) {
-			case 'createLogger': this.createLogger(URI.revive(arg[0]), arg[1]); return;
+			case 'createLogger': this.createLogger(URI.revive(arg[0]), arg[1], arg[2]); return;
 			case 'log': return this.log(URI.revive(arg[0]), arg[1]);
 			case 'consoleLog': return this.consoleLog(arg[0], arg[1]);
 			case 'setLogLevel': return this.loggerService.setLogLevel(URI.revive(arg[0]), arg[1]);
-			case 'registerLoggerResource': return this.loggerService.registerLoggerResource({ ...arg[0], resource: URI.revive(arg[0].resource) }, arg[1]);
-			case 'deregisterLoggerResource': return this.loggerService.deregisterLoggerResource(URI.revive(arg[0]));
+			case 'setVisibility': return this.loggerService.setVisibility(URI.revive(arg[0]), arg[1]);
+			case 'registerLogger': return this.loggerService.registerLogger({ ...arg[0], resource: URI.revive(arg[0].resource) }, arg[1]);
+			case 'deregisterLogger': return this.loggerService.deregisterLogger(URI.revive(arg[0]));
 		}
 
 		throw new Error(`Call not found: ${command}`);
 	}
 
-	private createLogger(file: URI, options: ILoggerOptions): void {
-		this.loggers.set(file.toString(), this.loggerService.createLogger(file, options));
+	private createLogger(file: URI, options: ILoggerOptions, donotRegister?: boolean): void {
+		this.loggers.set(file.toString(), this.loggerService.createLogger(file, options, donotRegister));
 	}
 
 	private consoleLog(level: LogLevel, args: any[]): void {