mcp/server.go: implement server-side logging throughout codebase #306
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jba
requested changes
Aug 20, 2025
| type ServerOptions struct { | ||
| // Optional instructions for connected clients. | ||
| Instructions string | ||
| // Logger is used for server-side logging. If nil, slog.Default() is used. |
| if opts.UnsubscribeHandler != nil && opts.SubscribeHandler == nil { | ||
| panic("UnsubscribeHandler requires SubscribeHandler") | ||
| } | ||
| l := ensureLogger(opts.Logger) |
| } | ||
|
|
||
| func (h *SSEHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { | ||
| h.ensureLogger() |
There was a problem hiding this comment.
If this only occurs in one place, inline it.
|
could this also extend to streamable http? |
|
@ramongalate I think that should be separate, since the transport is lower level. Maybe a field on StreamableHTTPOptions and StreamableServerTransport? I think you can make a separate PR, not dependent on this one. |
|
@KamdynS there is no activity on this PR for over a month. Do you mind if I take this up? Will address open review comments. |
|
@IAmSurajBobade Yes please |
|
Hi @KamdynS I have addressed review comments and made minor tweaks. To update the same PR , could you please provide contributer access to your repo/branch. Otherwise, I can create new PR. |
|
Superseded by #501. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
mcp/server.go: implement server‑side logging
*slog.Loggerthrough server paths to improve debuggabilityensureLoggerso loggers are never nilServer,SSEHandler/SSEServerTransport; remove nil checksAll tests are passing and no functionality should change with this PR
Fixes #170