From 884166783ea127e5c9113fa51f3895d52cdb88f1 Mon Sep 17 00:00:00 2001 From: Filipe C Menezes Date: Thu, 24 Apr 2025 11:45:56 +0100 Subject: [PATCH 1/5] refactor: split loggers --- src/logger.ts | 102 +++++++++++++++++++++++++++++++++++++------------- 1 file changed, 76 insertions(+), 26 deletions(-) diff --git a/src/logger.ts b/src/logger.ts index 425f56b9..9cb7c93e 100644 --- a/src/logger.ts +++ b/src/logger.ts @@ -7,7 +7,12 @@ import { LoggingMessageNotification } from "@modelcontextprotocol/sdk/types.js"; export type LogLevel = LoggingMessageNotification["params"]["level"]; abstract class LoggerBase { + async initialize(): Promise { + return Promise.resolve(); + } + abstract log(level: LogLevel, id: MongoLogId, context: string, message: string): void; + info(id: MongoLogId, context: string, message: string): void { this.log("info", id, context, message); } @@ -47,22 +52,41 @@ class ConsoleLogger extends LoggerBase { } } -class Logger extends LoggerBase { +class DiskLogger extends LoggerBase { + private logWriter?: MongoLogWriter; + constructor( - private logWriter: MongoLogWriter, - private server: McpServer + private logPath: string ) { super(); } +async initialize(): Promise { + await fs.mkdir(this.logPath, { recursive: true }); + + const manager = new MongoLogManager({ + directory: this.logPath, + retentionDays: 30, + onwarn: console.warn, + onerror: console.error, + gzip: false, + retentionGB: 1, + }); + + await manager.cleanupOldLogFiles(); + + this.logWriter = await manager.createLogWriter(); +} + log(level: LogLevel, id: MongoLogId, context: string, message: string): void { message = redact(message); const mongoDBLevel = this.mapToMongoDBLogLevel(level); + + if (!this.logWriter) { + throw new Error("DiskLogger is not initialized"); + } + this.logWriter[mongoDBLevel]("MONGODB-MCP", id, context, message); - void this.server.server.sendLoggingMessage({ - level, - data: `[${context}]: ${message}`, - }); } private mapToMongoDBLogLevel(level: LogLevel): "info" | "warn" | "error" | "debug" | "fatal" { @@ -86,31 +110,57 @@ class Logger extends LoggerBase { } } -class ProxyingLogger extends LoggerBase { - private internalLogger: LoggerBase = new ConsoleLogger(); - log(level: LogLevel, id: MongoLogId, context: string, message: string): void { - this.internalLogger.log(level, id, context, message); +class McpLogger extends LoggerBase { + constructor( + private server: McpServer + ) { + super(); + } + + log(level: LogLevel, _: MongoLogId, context: string, message: string): void { + void this.server.server.sendLoggingMessage({ + level, + data: `[${context}]: ${message}`, + }); } } -const logger = new ProxyingLogger(); -export default logger; +class CompositeLogger extends LoggerBase { + private loggers: LoggerBase[]; -export async function initializeLogger(server: McpServer, logPath: string): Promise { - await fs.mkdir(logPath, { recursive: true }); + constructor(...loggers: LoggerBase[]) { + super(); + if (loggers.length === 0) { // default to ConsoleLogger + loggers.push(new ConsoleLogger()); + } + this.loggers = [...loggers]; + } - const manager = new MongoLogManager({ - directory: logPath, - retentionDays: 30, - onwarn: console.warn, - onerror: console.error, - gzip: false, - retentionGB: 1, - }); + async initialize(): Promise { + for(const logger of this.loggers) { + await logger.initialize(); + } + } - await manager.cleanupOldLogFiles(); + setLoggers(...loggers: LoggerBase[]): void { + this.loggers = [...loggers]; + } + + log(level: LogLevel, id: MongoLogId, context: string, message: string): void { + for(const logger of this.loggers) { + logger.log(level, id, context, message); + } + } +} - const logWriter = await manager.createLogWriter(); - logger["internalLogger"] = new Logger(logWriter, server); +const logger = new CompositeLogger(); +export default logger; + +export async function initializeLogger(server: McpServer, logPath: string): Promise { + logger.setLoggers( + new McpLogger(server), + new DiskLogger(logPath), + ) + await logger.initialize(); } From ed64115f585067afc325129e7fccb0f37a454350 Mon Sep 17 00:00:00 2001 From: Filipe C Menezes Date: Thu, 24 Apr 2025 11:46:34 +0100 Subject: [PATCH 2/5] fix: styles --- src/logger.ts | 55 ++++++++++++++++++++++----------------------------- 1 file changed, 24 insertions(+), 31 deletions(-) diff --git a/src/logger.ts b/src/logger.ts index 9cb7c93e..cbe47ba7 100644 --- a/src/logger.ts +++ b/src/logger.ts @@ -54,38 +54,36 @@ class ConsoleLogger extends LoggerBase { class DiskLogger extends LoggerBase { private logWriter?: MongoLogWriter; - - constructor( - private logPath: string - ) { + + constructor(private logPath: string) { super(); } -async initialize(): Promise { - await fs.mkdir(this.logPath, { recursive: true }); - - const manager = new MongoLogManager({ - directory: this.logPath, - retentionDays: 30, - onwarn: console.warn, - onerror: console.error, - gzip: false, - retentionGB: 1, - }); + async initialize(): Promise { + await fs.mkdir(this.logPath, { recursive: true }); + + const manager = new MongoLogManager({ + directory: this.logPath, + retentionDays: 30, + onwarn: console.warn, + onerror: console.error, + gzip: false, + retentionGB: 1, + }); - await manager.cleanupOldLogFiles(); + await manager.cleanupOldLogFiles(); - this.logWriter = await manager.createLogWriter(); -} + this.logWriter = await manager.createLogWriter(); + } log(level: LogLevel, id: MongoLogId, context: string, message: string): void { message = redact(message); const mongoDBLevel = this.mapToMongoDBLogLevel(level); - + if (!this.logWriter) { throw new Error("DiskLogger is not initialized"); } - + this.logWriter[mongoDBLevel]("MONGODB-MCP", id, context, message); } @@ -110,11 +108,8 @@ async initialize(): Promise { } } - class McpLogger extends LoggerBase { - constructor( - private server: McpServer - ) { + constructor(private server: McpServer) { super(); } @@ -131,14 +126,15 @@ class CompositeLogger extends LoggerBase { constructor(...loggers: LoggerBase[]) { super(); - if (loggers.length === 0) { // default to ConsoleLogger + if (loggers.length === 0) { + // default to ConsoleLogger loggers.push(new ConsoleLogger()); } this.loggers = [...loggers]; } async initialize(): Promise { - for(const logger of this.loggers) { + for (const logger of this.loggers) { await logger.initialize(); } } @@ -148,7 +144,7 @@ class CompositeLogger extends LoggerBase { } log(level: LogLevel, id: MongoLogId, context: string, message: string): void { - for(const logger of this.loggers) { + for (const logger of this.loggers) { logger.log(level, id, context, message); } } @@ -158,9 +154,6 @@ const logger = new CompositeLogger(); export default logger; export async function initializeLogger(server: McpServer, logPath: string): Promise { - logger.setLoggers( - new McpLogger(server), - new DiskLogger(logPath), - ) + logger.setLoggers(new McpLogger(server), new DiskLogger(logPath)); await logger.initialize(); } From 3e5b6d84152935504679397f478dbab173bcc4a7 Mon Sep 17 00:00:00 2001 From: Filipe C Menezes Date: Thu, 24 Apr 2025 11:48:39 +0100 Subject: [PATCH 3/5] refactor: CompositeLogger constructor --- src/logger.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/logger.ts b/src/logger.ts index cbe47ba7..f856e192 100644 --- a/src/logger.ts +++ b/src/logger.ts @@ -126,10 +126,13 @@ class CompositeLogger extends LoggerBase { constructor(...loggers: LoggerBase[]) { super(); + if (loggers.length === 0) { // default to ConsoleLogger - loggers.push(new ConsoleLogger()); + this.loggers = [new ConsoleLogger()]; + return; } + this.loggers = [...loggers]; } From 92e247eb4635df6d917f62b9562e2b6611a4026f Mon Sep 17 00:00:00 2001 From: Filipe C Menezes Date: Thu, 24 Apr 2025 11:54:40 +0100 Subject: [PATCH 4/5] fix: check for loggers --- src/logger.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/logger.ts b/src/logger.ts index f856e192..062129f3 100644 --- a/src/logger.ts +++ b/src/logger.ts @@ -143,6 +143,9 @@ class CompositeLogger extends LoggerBase { } setLoggers(...loggers: LoggerBase[]): void { + if (loggers.length === 0) { + throw new Error("At least one logger must be provided"); + } this.loggers = [...loggers]; } From 6733c9eae390696bf39a45069efc78f94f8b4db4 Mon Sep 17 00:00:00 2001 From: Filipe C Menezes Date: Fri, 25 Apr 2025 09:59:04 +0100 Subject: [PATCH 5/5] fix: address comments --- src/logger.ts | 38 ++++++++++++++------------------------ 1 file changed, 14 insertions(+), 24 deletions(-) diff --git a/src/logger.ts b/src/logger.ts index 062129f3..0ff292cc 100644 --- a/src/logger.ts +++ b/src/logger.ts @@ -7,10 +7,6 @@ import { LoggingMessageNotification } from "@modelcontextprotocol/sdk/types.js"; export type LogLevel = LoggingMessageNotification["params"]["level"]; abstract class LoggerBase { - async initialize(): Promise { - return Promise.resolve(); - } - abstract log(level: LogLevel, id: MongoLogId, context: string, message: string): void; info(id: MongoLogId, context: string, message: string): void { @@ -53,17 +49,15 @@ class ConsoleLogger extends LoggerBase { } class DiskLogger extends LoggerBase { - private logWriter?: MongoLogWriter; - - constructor(private logPath: string) { + private constructor(private logWriter: MongoLogWriter) { super(); } - async initialize(): Promise { - await fs.mkdir(this.logPath, { recursive: true }); + static async fromPath(logPath: string): Promise { + await fs.mkdir(logPath, { recursive: true }); const manager = new MongoLogManager({ - directory: this.logPath, + directory: logPath, retentionDays: 30, onwarn: console.warn, onerror: console.error, @@ -73,17 +67,15 @@ class DiskLogger extends LoggerBase { await manager.cleanupOldLogFiles(); - this.logWriter = await manager.createLogWriter(); + const logWriter = await manager.createLogWriter(); + + return new DiskLogger(logWriter); } log(level: LogLevel, id: MongoLogId, context: string, message: string): void { message = redact(message); const mongoDBLevel = this.mapToMongoDBLogLevel(level); - if (!this.logWriter) { - throw new Error("DiskLogger is not initialized"); - } - this.logWriter[mongoDBLevel]("MONGODB-MCP", id, context, message); } @@ -136,12 +128,6 @@ class CompositeLogger extends LoggerBase { this.loggers = [...loggers]; } - async initialize(): Promise { - for (const logger of this.loggers) { - await logger.initialize(); - } - } - setLoggers(...loggers: LoggerBase[]): void { if (loggers.length === 0) { throw new Error("At least one logger must be provided"); @@ -159,7 +145,11 @@ class CompositeLogger extends LoggerBase { const logger = new CompositeLogger(); export default logger; -export async function initializeLogger(server: McpServer, logPath: string): Promise { - logger.setLoggers(new McpLogger(server), new DiskLogger(logPath)); - await logger.initialize(); +export async function initializeLogger(server: McpServer, logPath: string): Promise { + const diskLogger = await DiskLogger.fromPath(logPath); + const mcpLogger = new McpLogger(server); + + logger.setLoggers(mcpLogger, diskLogger); + + return logger; }