From 8f1ae280b821be929b6d3f7350db9742a52c5674 Mon Sep 17 00:00:00 2001 From: Stefan Dirix Date: Thu, 25 May 2023 23:37:07 +0200 Subject: [PATCH] fix: propagate log-config changes If Theia is started with the "--log-config" option the 'LogLevelCliContribution' watches for file changes and fires a 'logConfigChangedEvent'. This event was not listened to. Therefore already existing 'ILogger' levels were not updated. This is now fixed by notifying loggers about log config changes to update their log level. Also refactors the backend architecture to also use the existing client mechanism for notifications. Signed-off-by: Stefan Dirix Contributed on behalf of STMicroelectronics --- CHANGELOG.md | 1 + packages/core/src/common/logger-protocol.ts | 4 ++++ packages/core/src/common/logger-watcher.ts | 19 ++++++++++++------- packages/core/src/common/logger.ts | 5 +++++ .../core/src/node/console-logger-server.ts | 7 +++++-- .../core/src/node/logger-backend-module.ts | 9 ++++++++- 6 files changed, 35 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6348661ba1e7f..6d7ec86fcf408 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ ## v1.42.0 +- [core] fixed logger level propagation when log config file changes at runtime [#12566](https://github.com/eclipse-theia/theia/pull/12566) - Contributed on behalf of STMicroelectronics - [vsx-registry] added a hint to extension fetching ENOTFOUND errors [#12858](https://github.com/eclipse-theia/theia/pull/12858) - Contributed by STMicroelectronics ## v1.41.0 - 08/31/2023 diff --git a/packages/core/src/common/logger-protocol.ts b/packages/core/src/common/logger-protocol.ts index 57327924c901c..8079711d25ee3 100644 --- a/packages/core/src/common/logger-protocol.ts +++ b/packages/core/src/common/logger-protocol.ts @@ -38,6 +38,7 @@ export interface ILogLevelChangedEvent { export interface ILoggerClient { onLogLevelChanged(event: ILogLevelChangedEvent): void; + onLogConfigChanged(): void; } @injectable() @@ -49,6 +50,9 @@ export class DispatchingLoggerClient implements ILoggerClient { this.clients.forEach(client => client.onLogLevelChanged(event)); } + onLogConfigChanged(): void { + this.clients.forEach(client => client.onLogConfigChanged()); + } } export const rootLoggerName = 'root'; diff --git a/packages/core/src/common/logger-watcher.ts b/packages/core/src/common/logger-watcher.ts index 945c68d6cee54..bef03644b6fda 100644 --- a/packages/core/src/common/logger-watcher.ts +++ b/packages/core/src/common/logger-watcher.ts @@ -22,22 +22,27 @@ import { ILoggerClient, ILogLevelChangedEvent } from './logger-protocol'; export class LoggerWatcher { getLoggerClient(): ILoggerClient { - const emitter = this.onLogLevelChangedEmitter; + const logLevelEmitter = this.onLogLevelChangedEmitter; + const logConfigEmitter = this.onLogConfigChangedEmitter; return { onLogLevelChanged(event: ILogLevelChangedEvent): void { - emitter.fire(event); - } + logLevelEmitter.fire(event); + }, + onLogConfigChanged(): void { + logConfigEmitter.fire(); + }, }; } - private onLogLevelChangedEmitter = new Emitter(); + protected onLogLevelChangedEmitter = new Emitter(); get onLogLevelChanged(): Event { return this.onLogLevelChangedEmitter.event; } - // FIXME: get rid of it, backend services should as well set a client on the server - fireLogLevelChanged(event: ILogLevelChangedEvent): void { - this.onLogLevelChangedEmitter.fire(event); + protected onLogConfigChangedEmitter = new Emitter(); + + get onLogConfigChanged(): Event { + return this.onLogConfigChangedEmitter.event; } } diff --git a/packages/core/src/common/logger.ts b/packages/core/src/common/logger.ts index 54900d53ac773..bd4a9b2379e8a 100644 --- a/packages/core/src/common/logger.ts +++ b/packages/core/src/common/logger.ts @@ -261,6 +261,11 @@ export class Logger implements ILogger { } }); }); + + /* Refetch log level if overall config in backend changed. */ + this.loggerWatcher.onLogConfigChanged(() => { + this._logLevel = this.created.then(_ => this.server.getLogLevel(this.name)); + }); } setLogLevel(logLevel: number): Promise { diff --git a/packages/core/src/node/console-logger-server.ts b/packages/core/src/node/console-logger-server.ts index a8068c29c6f69..e15f682e21cf8 100644 --- a/packages/core/src/node/console-logger-server.ts +++ b/packages/core/src/node/console-logger-server.ts @@ -17,7 +17,7 @@ import { inject, injectable, postConstruct } from 'inversify'; import { LoggerWatcher } from '../common/logger-watcher'; import { LogLevelCliContribution } from './logger-cli-contribution'; -import { ILoggerServer, ILoggerClient, ConsoleLogger } from '../common/logger-protocol'; +import { ILoggerServer, ILoggerClient, ConsoleLogger, rootLoggerName } from '../common/logger-protocol'; @injectable() export class ConsoleLoggerServer implements ILoggerServer { @@ -32,9 +32,13 @@ export class ConsoleLoggerServer implements ILoggerServer { @postConstruct() protected init(): void { + this.setLogLevel(rootLoggerName, this.cli.defaultLogLevel); for (const name of Object.keys(this.cli.logLevels)) { this.setLogLevel(name, this.cli.logLevels[name]); } + this.cli.onLogConfigChanged(() => { + this.client?.onLogConfigChanged(); + }); } async setLogLevel(name: string, newLogLevel: number): Promise { @@ -45,7 +49,6 @@ export class ConsoleLoggerServer implements ILoggerServer { if (this.client !== undefined) { this.client.onLogLevelChanged(event); } - this.watcher.fireLogLevelChanged(event); } async getLogLevel(name: string): Promise { diff --git a/packages/core/src/node/logger-backend-module.ts b/packages/core/src/node/logger-backend-module.ts index 368b90885ebd2..c8ac66b935f49 100644 --- a/packages/core/src/node/logger-backend-module.ts +++ b/packages/core/src/node/logger-backend-module.ts @@ -63,7 +63,14 @@ export const loggerBackendModule = new ContainerModule(bind => { bind(DispatchingLoggerClient).toSelf().inSingletonScope(); bindLogger(bind, { onLoggerServerActivation: ({ container }, server) => { - server.setClient(container.get(DispatchingLoggerClient)); + const dispatchingLoggerClient = container.get(DispatchingLoggerClient); + server.setClient(dispatchingLoggerClient); + + // register backend logger watcher as a client + const loggerWatcher = container.get(LoggerWatcher); + dispatchingLoggerClient.clients.add(loggerWatcher.getLoggerClient()); + + // make sure dispatching logger client is the only client server.setClient = () => { throw new Error('use DispatchingLoggerClient'); };