Automatic handling of logging level #882
Open
+115
−79
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
setLevel
request handling toServer
class when logging capability is advertised.sendLoggingMessage
method inServer
class to respect current log level.sendLoggingMessage
passthrough method onMcpServer
class since it doesn't use inheritance.sendNotification
to send log messages to instead use thesendLoggingMessage
method so that it can be filtered according to the log level.In src/server/index.ts
SetLevelRequestSchema
whichextra
paramLoggingLevel
typeisMessageIgnored
method which takes aLoggingLevel
andsessionId
, 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 levelsendLoggingMessage
method,sessionId
argumentparams.level
value is not ignored for the given session id.In mcp.ts
LoggingMessageNotification
sendLoggingMessage
method with the same signature as the corresponding method inServer
classthis.server
, passing the argsIn src/examples/server/
sendNotification
, we won't use it nowextra
paramsendNotification
calls withserver.sendLoggingMessage
, passing only the params section of the message andextra.sessionId
Motivation and Context
We discovered in this issue in the Inspector repo that when the Inspector sends an
setLevel
request to a server that has advertised logging support but has not set a listener forlogging/setLevel
requests, the initialization phase will fail and the server will not connect. Rather than remove thesetLevel
request, 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:
setLevel
request 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
Server
andMcpServer
classes.The examples in the SDK send logging messages, so they must advertise logging capability. However they do not listen for
setLevel
requests 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
sendLoggingMessage
method, 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.ts
example. Its multi-greet tool sends three messages atinfo
level 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