Skip to content

Commit

Permalink
fix: propagate log-config changes
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 committed Sep 25, 2023
1 parent d2b7f3b commit 8ed7854
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 9 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
17 changes: 11 additions & 6 deletions packages/core/src/common/logger-watcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,15 @@ 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();
},
};
}

Expand All @@ -36,8 +40,9 @@ export class LoggerWatcher {
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);
private 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 8ed7854

Please sign in to comment.