Skip to content

Refactor logging system to use exchange for targeted client notifications #131

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

Closed
tzolov opened this issue Apr 9, 2025 · 0 comments
Closed

Comments

@tzolov
Copy link
Contributor

tzolov commented Apr 9, 2025

Bug description
The current MCP logging implementation has several limitations:

  • Logging notifications are broadcast to all connected clients regardless of their interest level
  • There's a single global minimum logging level for the entire server
  • Clients cannot individually configure which log levels they want to receive
  • The current approach is inefficient as it sends unnecessary messages to clients

This design doesn't align with the per-session approach used for other MCP features like sampling and roots, where each client can have its own configuration.

Expected behavior

  • Use the exchange mechanism to send logging notifications only to specific client sessions
  • Track minimum logging level per client session rather than globally
  • Filter logging notifications at the server side based on each client's configured level
  • Change setLoggingLevel from a notification to a request/response pattern for proper acknowledgment
  • Deprecate the old broadcasting behavior.

Acceptance Criteria

  • Each client can set its own minimum logging level
  • Logging notifications below a client's minimum level are filtered out
  • Logging notifications at or above a client's minimum level are delivered
  • Ddd integration tests

Environment
Affects all MCP versions including 0.9.0-SN

Related to #97

@tzolov tzolov added this to the 0.9.0 milestone Apr 9, 2025
tzolov added a commit that referenced this issue Apr 9, 2025
…ications

Refactors the MCP logging system to use the exchange mechanism for sending
logging notifications only to specific client sessions rather than broadcasting to all clients.

- Move logging notification delivery from server-wide broadcast to per-session exchange
- Implement per-session minimum logging level tracking and filtering
- Change setLoggingLevel from notification to request/response pattern
- Deprecate global server.loggingNotification in favor of exchange.loggingNotification
- Add integration test demonstrating filtered logging notifications

Resolves #131

Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com>
tzolov added a commit that referenced this issue Apr 9, 2025
…ications

Refactors the MCP logging system to use the exchange mechanism for sending
logging notifications only to specific client sessions rather than broadcasting to all clients.

- Move logging notification delivery from server-wide broadcast to per-session exchange
- Implement per-session minimum logging level tracking and filtering
- Change setLoggingLevel from notification to request/response pattern
- Deprecate global server.loggingNotification in favor of exchange.loggingNotification
- Add integration test demonstrating filtered logging notifications

Resolves #131

Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com>
tzolov added a commit that referenced this issue Apr 10, 2025
…ications

Refactors the MCP logging system to use the exchange mechanism for sending
logging notifications only to specific client sessions rather than broadcasting to all clients.

- Move logging notification delivery from server-wide broadcast to per-session exchange
- Implement per-session minimum logging level tracking and filtering
- Change setLoggingLevel from notification to request/response pattern
- Deprecate global server.loggingNotification in favor of exchange.loggingNotification
- Add integration test demonstrating filtered logging notifications

Resolves #131

Signed-off-by: Christian Tzolov <christian.tzolov@broadcom.com>
@tzolov tzolov closed this as completed in c88ac93 Apr 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant