Automatic handling of logging level #882
Conversation
…. Update sendLoggingMessage method to respect current log level.
Add sendLoggingMessage passthrough method on McpServer class since it doesn't use inheritance.
Update examples to use the sendLoggingMessage method so that it can be filtered according to the log level.
* In src/server/index.ts
- in constructor
- if capabilities object has logging, set request handler for SetLevelRequestSchema which sets this.logLevel to the level value from the request params and return empty object as per spec
- define private _logLevel property of type LoggingLevel initialized to "debug"
- define private _msgLevels property with list of objects with level property set to the ascending logging levels according to RFC 5424 as per spec. This gives us a lookup to determine if level is greater or less than a target level
- add private isMessageIgnored method which takes a LoggingLevel type arg returns boolean, and looks up the currently set level and the target level, returning true if the target level is below the current level
- in sendLoggingMessage method, only send the notification if the params.level value is not ignored.
* In mcp.ts
- import LoggingMessageNotification
- add async sendLoggingMessage method that returns takes a params object from LoggingMessageNotification, and returns the result of a call to this.server.sendLoggingMessage (since McpServer doesn't extend Server, but rather uses composition.
* In src/examples/server/
- jsonResponseStreamableHttp.ts
- simpleSseServer.ts
- simpleStreamableHttp.ts
- simpleStatelessStreamableHttp.ts
- in tool call callback functions that send logging message via the included sendNotification function, replace with calls to server.sendLoggingMessage, passing only the params of the message.
* In src/server/index.ts
- in Server class constructor when setting request handler for SetLevelRequestSchema
- take the sessionId OR 'mcp-session-id' header and store the session id in the _loggingLevels map by session id. Doesn't store the level if there is no session id
- change _logLevel variable to a Map, string to LoggingLevel type
- rename _msgLevels array to _levelNames and just make it strings for simplicity
- in isMessageIgnored function
- take a sessionId argument
- change _msgLevels to _levelNames
- match against the level stored in the _loggingLevels map by session id
- in sendLoggingLevelMessage,
- add optional sessionId argument
- make all action conditional upon _capabilities.logging existing
- if there is no session id OR if the message is not ignored based on the level and session id, send the notification
* In src/server/mcp.ts
- in sendLoggingMessage
- add optional sessionId argument
- pass sessionId to server.sendLoggingMessage
* In src/examples/server
- jsonResponseStreamableHttp.ts
- simpleSseServer.ts
- simpleStatelessStreamableHttp.ts
- simpleStreamableHttp.ts
- sseAndStreamableHttpCompatibleServer.ts
- update tool callbacks to get the extra param
- update erver.sendLoggingMessage calls to include extra.sessionId
|
Hi, We have to be careful with in-memory storage like the below: Because this will only work for a single instance of the MCP server. Scale the MCP server to two versions and this will start acting strange (one instance where the setLevel request arrived will have it in-memory, and the other instance will not, thus any further request arriving at the 2nd+ or more instance will not respect the level). Thus for now, storing the level should be left out of the core MCP SDK until a better way to handle it is done. On how it could be handled in the future: Persisting the "log level" state needs to be similar to how the EventsStore is done ( typescript-sdk/src/server/streamableHttp.ts Lines 17 to 29 in 3bc2235 My general feel on this is any such areas should be abstracted in an interface which users can implement, and the SDK can export an "inMemory" implementation of the interface. |
@KKonstantinov If you are talking about horizontal scaling ot servers behind a load balancer, that currently isn't possible since sessions are tied to the transport and do not exist in the data layer. Thus a session will always be tied to a single server in today's MCP. When and if sessions are divorced from the transport layer (as I believe they will be; there are a couple of competing SEPs to do this), this can easily be refactored to store the level in the session data that could be shared between servers. But for the moment, a single server handling multiple sessions is possible, and this handles that, and also fixes a huge problem of people advertising logging support but not actually building it. To do so means a lot of boilerplate code everyone would need to duplicate. |
There was a problem hiding this comment.
I've verified the fix by trying to connect the latest inspector on both main and this branch with simpleStreamableHttp.ts example server:
main |
this branch |
|---|---|
 |
 |
While the concern of @KKonstantinov is valid, I don't think we actually have a clear horizontal scalability story for MCP servers yet - so I wouldn't block this clear improvement over it, especially as it would require deeper refactors of our transport models to accommodate anyway.
Requesting changes mainly for the duplication of levelNames which would be great to avoid.
If we could add some lightweight test coverage for this new feature as well that'd be great, but could be a follow-up too.
| private _levelNames = [ | ||
| "debug", | ||
| "info", | ||
| "notice", | ||
| "warning", | ||
| "error", | ||
| "critical", | ||
| "alert", | ||
| "emergency", | ||
| ]; |
There was a problem hiding this comment.
This seems like we're duplicating information that should already be available as an enum from LoggingLevelSchema?
Line 1021 in c7887c0
Is there a way we can use that directly without creating a duplicated _levelNames array and iterating through it each time?
For example:
// Create a severity map from the LoggingLevelSchema once
private readonly LOG_LEVEL_SEVERITY = new Map(
LoggingLevelSchema.options.map((level, index) => [level, index])
);
private isMessageIgnored = (level: LoggingLevel, sessionId: string): boolean => {
const currentLevel = this._loggingLevels.get(sessionId);
if (!currentLevel) return false;
return this.LOG_LEVEL_SEVERITY.get(level)! < this.LOG_LEVEL_SEVERITY.get(currentLevel)!;
};
// In the setLevel handler:
const parseResult = LoggingLevelSchema.safeParse(level);
if (transportSessionId && parseResult.success) {
this._loggingLevels.set(transportSessionId, parseResult.data);
}
Yes agreed with this completely and I think it should proceed, just have to be mindful of it in the future when scalability becomes available. |
Description
setLevelrequest handling toServerclass when logging capability is advertised.sendLoggingMessagemethod inServerclass to respect current log level.sendLoggingMessagepassthrough method onMcpServerclass since it doesn't use inheritance.sendNotificationto send log messages to instead use thesendLoggingMessagemethod so that it can be filtered according to the log level.In src/server/index.ts
SetLevelRequestSchemawhichextraparamLoggingLeveltypeisMessageIgnoredmethod which takes aLoggingLevelandsessionId, returnsboolean, and looks up the currently set level for the given session id and the target level, returning true if the target level is below the current levelsendLoggingMessagemethod,sessionIdargumentparams.levelvalue is not ignored for the given session id.In mcp.ts
LoggingMessageNotificationsendLoggingMessagemethod with the same signature as the corresponding method inServerclassthis.server, passing the argsIn src/examples/server/
sendNotification, we won't use it nowextraparamsendNotificationcalls withserver.sendLoggingMessage, passing only the params section of the message andextra.sessionIdMotivation and Context
We discovered in this issue in the Inspector repo that when the Inspector sends an
setLevelrequest to a server that has advertised logging support but has not set a listener forlogging/setLevelrequests, the initialization phase will fail and the server will not connect. Rather than remove thesetLevelrequest, we instead added a toast popup that saysServer declares logging capability but doesn't implement method "logging/setLevel". We did this, because in that case, the Inspector is operating in-spec, but arguably, the server is not.Regarding logging, the spec has the following to say:
All of the examples in the SDK that advertise logging capability support actually fail to do so properly. The only example of adding support for logging capability exists in the Everything Server in the servers repo. It is composed of two parts:
setLevelrequest listenerIt is a lot to ask for every server developer to have to implement this boilerplate functionality, particularly when it could be done automatically by the SDK's
ServerandMcpServerclasses.The examples in the SDK send logging messages, so they must advertise logging capability. However they do not listen for
setLevelrequests nor implement the ability to respect a requested log level. This means the Inspector will not connect with them. This is the same situation developers face with with their own servers until they add such support.How Has This Been Tested?
All of the example servers have been refactored to use the
sendLoggingMessagemethod, so they do not have to implement any logic to support log level handling.In this screen recording, I demonstrate connecting multiple clients to the same server instance of the
simpleStreamableHttps.tsexample. Its multi-greet tool sends three messages atinfolevel then sends a response. I adjust the debug levels independently for each client then run the tool, showing that the logging level is tracked per session and one session's level does not interfere with the other's.Breaking Changes
Nope.
Types of changes
Checklist
Additional context