Skip to content

Conversation

@keremgocen
Copy link
Contributor

@keremgocen keremgocen commented Nov 13, 2025

Introduce a logging service to enhance error handling and debugging capabilities across various components of the application. Update configuration to include log level and format, and ensure that logging is integrated into the server and tool dependencies.

WIP: only integrated for the get-schema tool for now. Tests to follow.

Part 1: (in this PR)

  • covers the foundations, a logger service with log levels from the MCP spec
  • An MCP server hook allows MCP clients to set the level
  • log format and output are decided at the start via env variables (although VSCode MCP client sets it to INFO and I could not find a way to change it)
  • logger only implemented for the get-schema tool for now implemented for all tools
  • added basic test coverage

Part 2: (not in this PR but here #95)

  • Add a redaction mechanism to remove sensitive data
  • Add test coverage

@keremgocen keremgocen changed the title Implement logging changes throughout the application Implement unified logger with log level - part 1 Nov 13, 2025
@keremgocen keremgocen marked this pull request as ready for review November 14, 2025 16:02
Copy link
Contributor

@MacondoExpress MacondoExpress left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really good! Left some comments, nothing of these are massive blocker IMO

}
}

func replaceAttr(_ []string, a slog.Attr) slog.Attr {
Copy link
Contributor

@MacondoExpress MacondoExpress Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the reader perspective is not clear the intention of why we need the replaceAttr func rather than relying on the slog formatter.
From what I guess, it's because slog convert levels to string using a small subset of values:

  • DEBUG
  • INFO
  • WARN
  • ERROR

Giving back less granularity than what we desire, but as it's not tested the underline reason could be lost over time!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a doc string to explain what replaceAttr is used for. We also have a test already all log levels can be set dynamically and now we are checking for valid values so hopefully it should be a lot more clear.

@keremgocen keremgocen changed the title Implement unified logger with log level - part 1 Implement unified logger with log level -- part 1 Nov 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants