Skip to content

Commit 81b0404

Browse files
authored
Streamline logging API (#14861)
1 parent cbd19da commit 81b0404

File tree

15 files changed

+109
-66
lines changed

15 files changed

+109
-66
lines changed

CHANGELOG.md

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,13 @@
44

55
- [Previous Changelogs](https://github.com/eclipse-theia/theia/tree/master/doc/changelogs/)
66

7+
## 1.59.0
8+
9+
<a name="breaking_changes_1.59.0">[Breaking Changes:](#breaking_changes_1.59.0)</a>
10+
11+
- [core] Adjusted the binding of named `ILogger` injections. These no longer have to be bound explicitly.
12+
If you encounter errors such as `Error: Ambiguous match found for serviceIdentifier: Symbol(ILogger)`, remove your bindings for the `ILogger` symbol.
13+
714
## 1.58.0 - 01/30/2025
815

916
- [ai] added 'required' property to tool call parameters [#14673](https://github.com/eclipse-theia/theia/pull/14673)
@@ -33,7 +40,7 @@
3340
- [core] added support for dragging files in browser [#14756](https://github.com/eclipse-theia/theia/pull/14756)
3441
- [core] fixed dragging file from outside the workspace [#14746](https://github.com/eclipse-theia/theia/pull/14746)
3542
- [core] fixed override of default key bindings [#14668](https://github.com/eclipse-theia/theia/pull/14668)
36-
- [core] fixed workbench.action.files.newUntitledFile [#0](https://github.com/eclipse-theia/theia/pull/0)
43+
- [core] fixed `workbench.action.files.newUntitledFile` command [#14754](https://github.com/eclipse-theia/theia/pull/14754)
3744
- [core] fixed z-index overlay issue in dock panels [#14695](https://github.com/eclipse-theia/theia/pull/14695)
3845
- [core] updated build scripts to use npm instead of yarn to build Theia [#14481](https://github.com/eclipse-theia/theia/pull/14481) - Contributed on behalf of STMicroelectronics
3946
- [core] updated keytar and drivelist [#14306](https://github.com/eclipse-theia/theia/pull/14306)

packages/ai-code-completion/src/browser/ai-code-completion-frontend-module.ts

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
// SPDX-License-Identifier: EPL-2.0 OR GPL-2.0-only WITH Classpath-exception-2.0
1515
// *****************************************************************************
1616

17-
import { ILogger } from '@theia/core';
1817
import { ContainerModule } from '@theia/core/shared/inversify';
1918
import { CodeCompletionAgent, CodeCompletionAgentImpl } from './code-completion-agent';
2019
import { AIFrontendApplicationContribution } from './ai-code-frontend-application-contribution';
@@ -25,10 +24,6 @@ import { AICodeInlineCompletionsProvider } from './ai-code-inline-completion-pro
2524
import { CodeCompletionPostProcessor, DefaultCodeCompletionPostProcessor } from './code-completion-postprocessor';
2625

2726
export default new ContainerModule(bind => {
28-
bind(ILogger).toDynamicValue(ctx => {
29-
const parentLogger = ctx.container.get<ILogger>(ILogger);
30-
return parentLogger.child('code-completion-agent');
31-
}).inSingletonScope().whenTargetNamed('code-completion-agent');
3227
bind(CodeCompletionAgentImpl).toSelf().inSingletonScope();
3328
bind(CodeCompletionAgent).toService(CodeCompletionAgentImpl);
3429
bind(Agent).toService(CodeCompletionAgentImpl);

packages/ai-history/src/browser/ai-history-frontend-module.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import { CommunicationRecordingService } from '@theia/ai-core';
1717
import { ContainerModule } from '@theia/core/shared/inversify';
1818
import { DefaultCommunicationRecordingService } from '../common/communication-recording-service';
1919
import { bindViewContribution, WidgetFactory } from '@theia/core/lib/browser';
20-
import { ILogger } from '@theia/core';
2120
import { AIHistoryViewContribution } from './ai-history-contribution';
2221
import { AIHistoryView } from './ai-history-widget';
2322
import '../../src/browser/style/ai-history.css';
@@ -27,11 +26,6 @@ export default new ContainerModule(bind => {
2726
bind(DefaultCommunicationRecordingService).toSelf().inSingletonScope();
2827
bind(CommunicationRecordingService).toService(DefaultCommunicationRecordingService);
2928

30-
bind(ILogger).toDynamicValue(ctx => {
31-
const parentLogger = ctx.container.get<ILogger>(ILogger);
32-
return parentLogger.child('llm-communication-recorder');
33-
}).inSingletonScope().whenTargetNamed('llm-communication-recorder');
34-
3529
bindViewContribution(bind, AIHistoryViewContribution);
3630

3731
bind(AIHistoryView).toSelf();

packages/core/src/browser/logger-frontend-module.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,12 @@
1616

1717
import { ContainerModule, Container } from 'inversify';
1818
import { ILoggerServer, loggerPath, ConsoleLogger } from '../common/logger-protocol';
19-
import { ILogger, Logger, LoggerFactory, setRootLogger, LoggerName, rootLoggerName } from '../common/logger';
19+
import { ILogger, Logger, LoggerFactory, setRootLogger, LoggerName } from '../common/logger';
2020
import { LoggerWatcher } from '../common/logger-watcher';
2121
import { WebSocketConnectionProvider } from './messaging';
2222
import { FrontendApplicationContribution } from './frontend-application-contribution';
2323
import { EncodingError } from '../common/message-rpc/rpc-message-encoder';
24+
import { bindCommonLogger } from '../common/logger-binding';
2425

2526
export const loggerFrontendModule = new ContainerModule(bind => {
2627
bind(FrontendApplicationContribution).toDynamicValue(ctx => ({
@@ -29,9 +30,7 @@ export const loggerFrontendModule = new ContainerModule(bind => {
2930
}
3031
}));
3132

32-
bind(LoggerName).toConstantValue(rootLoggerName);
33-
bind(ILogger).to(Logger).inSingletonScope().whenTargetIsDefault();
34-
bind(LoggerWatcher).toSelf().inSingletonScope();
33+
bindCommonLogger(bind);
3534
bind(ILoggerServer).toDynamicValue(ctx => {
3635
const loggerWatcher = ctx.container.get(LoggerWatcher);
3736
const connection = ctx.container.get(WebSocketConnectionProvider);
Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// *****************************************************************************
2-
// Copyright (C) 2018 Ericsson and others.
2+
// Copyright (C) 2025 TypeFox and others.
33
//
44
// This program and the accompanying materials are made available under the
55
// terms of the Eclipse Public License v. 2.0 which is available at
@@ -14,17 +14,21 @@
1414
// SPDX-License-Identifier: EPL-2.0 OR GPL-2.0-only WITH Classpath-exception-2.0
1515
// *****************************************************************************
1616

17-
import { interfaces } from '@theia/core/shared/inversify';
18-
import { ILogger } from '@theia/core';
17+
import { interfaces } from 'inversify';
18+
import { ILogger, Logger, LoggerName, rootLoggerName } from './logger';
19+
import { LoggerWatcher } from './logger-watcher';
1920

20-
/**
21-
* Create the bindings common to node and browser.
22-
*
23-
* @param bind The bind function from inversify.
24-
*/
25-
export function createCommonBindings(bind: interfaces.Bind): void {
21+
export function bindCommonLogger(bind: interfaces.Bind): void {
22+
bind(LoggerName).toConstantValue(rootLoggerName);
23+
bind(ILogger).to(Logger).inSingletonScope().when(request => getName(request) === undefined);
2624
bind(ILogger).toDynamicValue(ctx => {
2725
const logger = ctx.container.get<ILogger>(ILogger);
28-
return logger.child('terminal');
29-
}).inSingletonScope().whenTargetNamed('terminal');
26+
return logger.child(getName(ctx.currentRequest)!);
27+
}).when(request => getName(request) !== undefined);
28+
bind(LoggerWatcher).toSelf().inSingletonScope();
29+
}
30+
31+
function getName(request: interfaces.Request): string | undefined {
32+
const named = request.target.metadata.find(e => e.key === 'named');
33+
return named ? named.value?.toString() : undefined;
3034
}

packages/core/src/common/logger-protocol.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,10 +110,12 @@ export namespace ConsoleLogger {
110110
console.trace = consoles.get(LogLevel.TRACE)!;
111111
console.log = originalConsoleLog;
112112
}
113-
export function log(name: string, logLevel: number, message: string, params: any[]): void {
113+
export function log(name: string, logLevel: number, message: string, params: any[]): string {
114114
const console = consoles.get(logLevel) || originalConsoleLog;
115115
const severity = (LogLevel.strings.get(logLevel) || 'unknown').toUpperCase();
116116
const now = new Date();
117-
console(`${now.toISOString()} ${name} ${severity} ${message}`, ...params);
117+
const formattedMessage = `${now.toISOString()} ${name} ${severity} ${message}`;
118+
console(formattedMessage, ...params);
119+
return formattedMessage;
118120
}
119121
}

packages/core/src/common/logger.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,8 @@ export class Logger implements ILogger {
240240
@inject(LoggerFactory) protected readonly factory: LoggerFactory;
241241
@inject(LoggerName) protected name: string;
242242

243+
protected cache = new Map<string, ILogger>();
244+
243245
@postConstruct()
244246
protected init(): void {
245247
if (this.name !== rootLoggerName) {
@@ -384,6 +386,13 @@ export class Logger implements ILogger {
384386
}
385387

386388
child(name: string): ILogger {
387-
return this.factory(name);
389+
const existing = this.cache.get(name);
390+
if (existing) {
391+
return existing;
392+
} else {
393+
const child = this.factory(name);
394+
this.cache.set(name, child);
395+
return child;
396+
}
388397
}
389398
}

packages/core/src/node/console-logger-server.ts

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,18 +18,23 @@ import { inject, injectable, postConstruct } from 'inversify';
1818
import { LoggerWatcher } from '../common/logger-watcher';
1919
import { LogLevelCliContribution } from './logger-cli-contribution';
2020
import { ILoggerServer, ILoggerClient, ConsoleLogger, rootLoggerName } from '../common/logger-protocol';
21+
import { format } from 'util';
22+
import { EOL } from 'os';
23+
import * as fs from 'fs';
2124

2225
@injectable()
2326
export class ConsoleLoggerServer implements ILoggerServer {
2427

25-
protected client: ILoggerClient | undefined = undefined;
28+
protected client?: ILoggerClient;
2629

2730
@inject(LoggerWatcher)
2831
protected watcher: LoggerWatcher;
2932

3033
@inject(LogLevelCliContribution)
3134
protected cli: LogLevelCliContribution;
3235

36+
protected logFileStream?: fs.WriteStream;
37+
3338
@postConstruct()
3439
protected init(): void {
3540
this.setLogLevel(rootLoggerName, this.cli.defaultLogLevel);
@@ -59,7 +64,22 @@ export class ConsoleLoggerServer implements ILoggerServer {
5964
async log(name: string, logLevel: number, message: string, params: any[]): Promise<void> {
6065
const configuredLogLevel = await this.getLogLevel(name);
6166
if (logLevel >= configuredLogLevel) {
62-
ConsoleLogger.log(name, logLevel, message, params);
67+
const fullMessage = ConsoleLogger.log(name, logLevel, message, params);
68+
this.logToFile(fullMessage, params);
69+
}
70+
}
71+
72+
protected logToFile(message: string, params: any[]): void {
73+
if (this.cli.logFile && !this.logFileStream) {
74+
this.logFileStream = fs.createWriteStream(this.cli.logFile, { flags: 'a' });
75+
// Only log errors once to avoid spamming the console
76+
this.logFileStream.once('error', error => {
77+
console.error(`Error writing to log file ${this.cli.logFile}`, error);
78+
});
79+
}
80+
if (this.logFileStream) {
81+
const formatted = format(message, ...params) + EOL;
82+
this.logFileStream.write(formatted);
6383
}
6484
}
6585

packages/core/src/node/logger-backend-module.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,20 +16,19 @@
1616

1717
import { ContainerModule, Container, interfaces } from 'inversify';
1818
import { ConnectionHandler, RpcConnectionHandler } from '../common/messaging';
19-
import { ILogger, LoggerFactory, Logger, setRootLogger, LoggerName, rootLoggerName } from '../common/logger';
19+
import { ILogger, LoggerFactory, Logger, setRootLogger, LoggerName } from '../common/logger';
2020
import { ILoggerServer, ILoggerClient, loggerPath, DispatchingLoggerClient } from '../common/logger-protocol';
2121
import { ConsoleLoggerServer } from './console-logger-server';
2222
import { LoggerWatcher } from '../common/logger-watcher';
2323
import { BackendApplicationContribution } from './backend-application';
2424
import { CliContribution } from './cli';
2525
import { LogLevelCliContribution } from './logger-cli-contribution';
26+
import { bindCommonLogger } from '../common/logger-binding';
2627

2728
export function bindLogger(bind: interfaces.Bind, props?: {
2829
onLoggerServerActivation?: (context: interfaces.Context, server: ILoggerServer) => void
2930
}): void {
30-
bind(LoggerName).toConstantValue(rootLoggerName);
31-
bind(ILogger).to(Logger).inSingletonScope().whenTargetIsDefault();
32-
bind(LoggerWatcher).toSelf().inSingletonScope();
31+
bindCommonLogger(bind);
3332
bind<ILoggerServer>(ILoggerServer).to(ConsoleLoggerServer).inSingletonScope().onActivation((context, server) => {
3433
if (props && props.onLoggerServerActivation) {
3534
props.onLoggerServerActivation(context, server);

packages/core/src/node/logger-cli-contribution.ts

Lines changed: 43 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ export class LogLevelCliContribution implements CliContribution {
4343
*/
4444
protected _defaultLogLevel: LogLevel = LogLevel.INFO;
4545

46+
protected _logFile?: string;
47+
4648
protected logConfigChangedEvent: Emitter<void> = new Emitter<void>();
4749

4850
get defaultLogLevel(): LogLevel {
@@ -53,6 +55,10 @@ export class LogLevelCliContribution implements CliContribution {
5355
return this._logLevels;
5456
}
5557

58+
get logFile(): string | undefined {
59+
return this._logFile;
60+
}
61+
5662
configure(conf: yargs.Argv): void {
5763
conf.option('log-level', {
5864
description: 'Sets the default log level',
@@ -65,6 +71,12 @@ export class LogLevelCliContribution implements CliContribution {
6571
type: 'string',
6672
nargs: 1,
6773
});
74+
75+
conf.option('log-file', {
76+
description: 'Path to the log file',
77+
type: 'string',
78+
nargs: 1
79+
});
6880
}
6981

7082
async setArguments(args: yargs.Arguments): Promise<void> {
@@ -87,22 +99,46 @@ export class LogLevelCliContribution implements CliContribution {
8799
console.error(`Error reading log config file ${filename}: ${e}`);
88100
}
89101
}
102+
103+
if (args['log-file'] !== undefined) {
104+
let filename = args['log-file'] as string;
105+
try {
106+
filename = path.resolve(filename);
107+
try {
108+
const stat = await fs.stat(filename);
109+
if (stat && stat.isFile()) {
110+
// Rename the previous log file to avoid overwriting it
111+
const oldFilename = `${filename}.${stat.ctime.toISOString().replace(/:/g, '-')}.old`;
112+
await fs.rename(filename, oldFilename);
113+
}
114+
} catch {
115+
// File does not exist, just continue to create it
116+
}
117+
await fs.writeFile(filename, '');
118+
this._logFile = filename;
119+
} catch (e) {
120+
console.error(`Error creating log file ${filename}: ${e}`);
121+
}
122+
}
90123
}
91124

92125
protected async watchLogConfigFile(filename: string): Promise<void> {
93-
await subscribe(filename, async (err, events) => {
126+
const dir = path.dirname(filename);
127+
await subscribe(dir, async (err, events) => {
94128
if (err) {
95129
console.log(`Error during log file watching ${filename}: ${err}`);
96130
return;
97131
}
98132
try {
99133
for (const event of events) {
100-
switch (event.type) {
101-
case 'create':
102-
case 'update':
103-
await this.slurpLogConfigFile(filename);
104-
this.logConfigChangedEvent.fire(undefined);
105-
break;
134+
if (event.path === filename) {
135+
switch (event.type) {
136+
case 'create':
137+
case 'update':
138+
await this.slurpLogConfigFile(filename);
139+
this.logConfigChangedEvent.fire(undefined);
140+
break;
141+
}
106142
}
107143
}
108144
} catch (e) {

0 commit comments

Comments
 (0)