Skip to content

Commit

Permalink
fix: propagate log-config changes (#12566)
Browse files Browse the repository at this point in the history
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 <sdirix@eclipsesource.com>

Contributed on behalf of STMicroelectronics
  • Loading branch information
sdirix authored Sep 26, 2023
1 parent f038673 commit fcde5ea
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 10 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions packages/core/src/common/logger-protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export interface ILogLevelChangedEvent {

export interface ILoggerClient {
onLogLevelChanged(event: ILogLevelChangedEvent): void;
onLogConfigChanged(): void;
}

@injectable()
Expand All @@ -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';
Expand Down
19 changes: 12 additions & 7 deletions packages/core/src/common/logger-watcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<ILogLevelChangedEvent>();
protected onLogLevelChangedEmitter = new Emitter<ILogLevelChangedEvent>();

get onLogLevelChanged(): Event<ILogLevelChangedEvent> {
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<void>();

get onLogConfigChanged(): Event<void> {
return this.onLogConfigChangedEmitter.event;
}
}
5 changes: 5 additions & 0 deletions packages/core/src/common/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<void> {
Expand Down
7 changes: 5 additions & 2 deletions packages/core/src/node/console-logger-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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<void> {
Expand All @@ -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<number> {
Expand Down
9 changes: 8 additions & 1 deletion packages/core/src/node/logger-backend-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
};
Expand Down

0 comments on commit fcde5ea

Please sign in to comment.