feat: add logging #35
Conversation
|
have you considered https://modelcontextprotocol.io/specification/2025-03-26/server/utilities/logging ? |
|
Yes, I wanted to integrate that as well, but got pretty late last night, so decided to leave it for today 😅 I still think it'd be valuable to both send the logs to the client, but also store them on disk. |
|
If you want to add it to disk we run the risk of adding to too many logs needing to implement log rotation, where to you want to send the logs to? Should we truncate? |
|
@fmenezes I'm using https://github.com/mongodb-js/devtools-shared/tree/main/packages/mongodb-log-writer for writing the logs on disk. It's the same package we use in mongosh and Compass and it implements log cleanup based on retention policy and storage limits. Right now, I've hardcoded some default values, but once we get to #21, we'd be able to expose them as config options for users to tweak. |
| function getLocalDataPath() { | ||
| if (process.platform === "win32") { | ||
| const appData = process.env.APPDATA; | ||
| const localAppData = process.env.LOCALAPPDATA ?? process.env.APPDATA; | ||
| if (localAppData && appData) { | ||
| return path.join(localAppData, "mongodb", "mongodb-mcp"); | ||
| } | ||
| } | ||
|
|
||
| return path.join(os.homedir(), ".mongodb", "mongodb-mcp"); |
There was a problem hiding this comment.
This is the same parent folder as the one we use for mongosh/compass.
| retentionDays: 30, | ||
| onwarn: console.warn, | ||
| onerror: console.error, | ||
| gzip: false, | ||
| retentionGB: 1, |
There was a problem hiding this comment.
Defaulting to 30 day retention or 1 gb of storage. We'll want to expose these as config options at a later point.
This adds logging to disk as well as sending log messages to the client via the logging capability.
The reason why I believe it's valuable to do both is that sometimes client logs can be noisy (e.g. Claude logs a bunch of error messages related to the fact that we don't provide prompts or resources).