-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Also push client messages (info/warn/debug) to server debug log #2063
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Implements CLIENT and CLIENT_AND_SERVER destinations for Context logging methods. By default, ctx.log() and convenience methods send only to MCP clients (CLIENT). Using log_to='CLIENT_AND_SERVER' also writes to the server's Python logger before sending to client, enabling server-side monitoring, debugging, and observability. - Added LogDestination Literal type with CLIENT and CLIENT_AND_SERVER values - Added _map_mcp_to_python_level() helper to map MCP levels to Python logging - Updated Context.log() and all convenience methods (debug, info, warning, error) - Server logging happens before client send for failure safety - Custom logger names supported via logger_name parameter - Extra fields passed through to server logger for structured logging - Comprehensive test coverage with mocked logger assertions - Updated documentation with usage examples and guidelines Fixes #2059 Co-authored-by: William Easton <strawgate@users.noreply.github.com>
|
I think the substance of this PR addresses a real pain point ("I logged but I don't see it!") where users don't realize that an MCP log is sent to clients, not visible on the server. But I've been hemming and hawing a little bit on implementatino. On the one hand I think visibility on the server is a huge observability benefit so I love this to the point I'm wondering if it should be default ON. However, I am worried that if a user sends e.g. a CRITICAL log to a client (for some reason) then we do not necessarily want to freak out their platform team by presenting a critical log on the server. Therefore my suggestion is to split the baby and always emit a server-side DEBUG log for every ctx.log() call (and have no new kwargs): This provides an audit trail of all client communications without polluting operational logs. It can be clearly documented and doesn't risk users starting to think that the MCP log is actually a server convenience. I think it would be actively bad if users used ctx.log() to log e.g. information they think is staying on the server. |
|
We could do something like add a global setting which acts as a clamp on the log level for the logs teed to the server. That would allow someone to force all to debug or errors/critical to just warnings, etc Maybe we could log at the level the user picks but to a namespaced logger that's off by default and we document how to turn on the logger We can include a comment about the server logging enablement in the docstrings on ctx I'll noodle on this a little more with your comments in mind |
|
I still need to update the tests and noodle on this a little more. We don't currently show the logger names in the logs (perhaps this should be configurable), but I do like prefixing the messages with something like [To Client] so it doesn't quite feel like directly logging to the server but instead that we're logging what was sent to the client. We can have the utility function that enables the logger take a "max_level" which can be set to debug, etc which can act as a clamp on the logger level for logs teed to the server. Or maybe as you said everything goes to debug by default, the utility function can take a log_level_map which users can use to push to other log levels (and we provide a default map which maps them to be corresponding levels) |
|
I like the idea of letting users map stuff -- feels like a good balance of very universal default with new debug verbosity, and letting them opt in to elevating |
e17ebf0 to
b3b02e5
Compare
|
@jlowin updated the PR body with the current behavior |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds logging functionality for client messages sent via ctx.info|debug|warning|etc methods. The main purpose is to provide server-side visibility into messages being sent to MCP clients, with the ability to control logging levels.
Key changes:
- Messages sent to clients are now also logged to a dedicated server logger at DEBUG level by default
- Added log level clamping functionality to control how client messages appear in server logs
- Updated test expectations to reflect new logger names and message formats
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/fastmcp/server/context.py |
Added client message logging with level mapping and clamping functionality |
src/fastmcp/utilities/logging.py |
Added log level clamping filter and utility functions for controlling message levels |
src/fastmcp/client/logging.py |
Updated logger names and message formatting for received server messages |
tests/client/test_logs.py |
Updated test expectations for new logger names and message formats |
tests/server/test_context.py |
Minor import formatting change |
docs/servers/logging.mdx |
Updated documentation with incorrect parameter descriptions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@jlowin i think this is ready |
Summary
Log client messages sent via
ctx.info|debug|warning|etc. Client messages are logged with their level mapped to Python levels and then "clamped" todebug.Users can enable these logs without having to enable debug logging on the server (as described in the docs) via:
We can evaluate letting users control how these log levels map to server levels, but for now users who want them logged at their original level can call
_unclamp_logger(logger=to_client_logger)on the logger as a private workaround. Or if they would prefer to log these asINFO, they can apply_clamp_logger(logger=to_client_logger,min_level=INFO,max_level=INFO)Fixes: #2059 and #2078