Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions mcp/logging.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,23 @@ type LoggingHandler struct {
handler slog.Handler
}

// discardHandler is a slog.Handler that drops all logs.
// TODO: use slog.NewNopHandler when we require Go 1.24+.
type discardHandler struct{}

func (discardHandler) Enabled(context.Context, slog.Level) bool { return false }
func (discardHandler) Handle(context.Context, slog.Record) error { return nil }
func (discardHandler) WithAttrs([]slog.Attr) slog.Handler { return discardHandler{} }
func (discardHandler) WithGroup(string) slog.Handler { return discardHandler{} }

// ensureLogger returns l if non-nil, otherwise a discard logger.
func ensureLogger(l *slog.Logger) *slog.Logger {
if l != nil {
return l
}
return slog.New(discardHandler{})
}

// NewLoggingHandler creates a [LoggingHandler] that logs to the given [ServerSession] using a
// [slog.JSONHandler].
func NewLoggingHandler(ss *ServerSession, opts *LoggingHandlerOptions) *LoggingHandler {
Expand Down
36 changes: 33 additions & 3 deletions mcp/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"encoding/json"
"fmt"
"iter"
"log/slog"
"maps"
"net/url"
"path/filepath"
Expand All @@ -35,8 +36,9 @@ const DefaultPageSize = 1000
// sessions by using [Server.Run].
type Server struct {
// fixed at creation
impl *Implementation
opts ServerOptions
impl *Implementation
opts ServerOptions
logger *slog.Logger
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use opts.Logger?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated code to use opts.Logger


mu sync.Mutex
prompts *featureSet[*serverPrompt]
Expand All @@ -53,6 +55,8 @@ type Server struct {
type ServerOptions struct {
// Optional instructions for connected clients.
Instructions string
// Logger is used for server-side logging. If nil, disable logging.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// If non-nil, log server activity.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated comment

Logger *slog.Logger
// If non-nil, called when "notifications/initialized" is received.
InitializedHandler func(context.Context, *InitializedRequest)
// PageSize is the maximum number of items to return in a single page for
Expand Down Expand Up @@ -132,6 +136,7 @@ func NewServer(impl *Implementation, options *ServerOptions) *Server {
return &Server{
impl: impl,
opts: opts,
logger: ensureLogger(opts.Logger),
prompts: newFeatureSet(func(p *serverPrompt) string { return p.prompt.Name }),
tools: newFeatureSet(func(t *serverTool) string { return t.tool.Name }),
resources: newFeatureSet(func(r *serverResource) string { return r.resource.URI }),
Expand Down Expand Up @@ -659,6 +664,7 @@ func (s *Server) ResourceUpdated(ctx context.Context, params *ResourceUpdatedNot
sessions := slices.Collect(maps.Keys(subscribedSessions))
s.mu.Unlock()
notifySessions(sessions, notificationResourceUpdated, params)
s.logger.Info("resource updated notification sent", "uri", params.URI, "subscriber_count", len(sessions))
return nil
}

Expand All @@ -676,6 +682,7 @@ func (s *Server) subscribe(ctx context.Context, req *SubscribeRequest) (*emptyRe
s.resourceSubscriptions[req.Params.URI] = make(map[*ServerSession]bool)
}
s.resourceSubscriptions[req.Params.URI][req.Session] = true
s.logger.Info("resource subscribed", "uri", req.Params.URI, "session_id", req.Session.ID())

return &emptyResult{}, nil
}
Expand All @@ -697,6 +704,7 @@ func (s *Server) unsubscribe(ctx context.Context, req *UnsubscribeRequest) (*emp
delete(s.resourceSubscriptions, req.Params.URI)
}
}
s.logger.Info("resource unsubscribed", "uri", req.Params.URI, "session_id", req.Session.ID())

return &emptyResult{}, nil
}
Expand All @@ -715,8 +723,10 @@ func (s *Server) unsubscribe(ctx context.Context, req *UnsubscribeRequest) (*emp
// It need not be called on servers that are used for multiple concurrent connections,
// as with [StreamableHTTPHandler].
func (s *Server) Run(ctx context.Context, t Transport) error {
s.logger.Info("server run start")
ss, err := s.Connect(ctx, t, nil)
if err != nil {
s.logger.Error("server connect failed", "error", err)
return err
}

Expand All @@ -728,8 +738,14 @@ func (s *Server) Run(ctx context.Context, t Transport) error {
select {
case <-ctx.Done():
ss.Close()
s.logger.Info("server run cancelled", "error", ctx.Err())
return ctx.Err()
case err := <-ssClosed:
if err != nil {
s.logger.Error("server session ended with error", "error", err)
} else {
s.logger.Info("server session ended")
}
return err
}
}
Expand All @@ -745,6 +761,7 @@ func (s *Server) bind(mcpConn Connection, conn *jsonrpc2.Connection, state *Serv
s.mu.Lock()
s.sessions = append(s.sessions, ss)
s.mu.Unlock()
s.logger.Info("server session connected", "session_id", ss.ID())
return ss
}

Expand All @@ -760,6 +777,7 @@ func (s *Server) disconnect(cc *ServerSession) {
for _, subscribedSessions := range s.resourceSubscriptions {
delete(subscribedSessions, cc)
}
s.logger.Info("server session disconnected", "session_id", cc.ID())
}

// ServerSessionOptions configures the server session.
Expand All @@ -784,7 +802,14 @@ func (s *Server) Connect(ctx context.Context, t Transport, opts *ServerSessionOp
state = opts.State
onClose = opts.onClose
}
return connect(ctx, t, s, state, onClose)

s.logger.Info("server connecting")
ss, err := connect(ctx, t, s, state, onClose)
if err != nil {
s.logger.Error("server connect error", "error", err)
return nil, err
}
return ss, nil
}

// TODO: (nit) move all ServerSession methods below the ServerSession declaration.
Expand All @@ -804,9 +829,11 @@ func (ss *ServerSession) initialized(ctx context.Context, params *InitializedPar
})

if !wasInit {
ss.server.logger.Warn("initialized before initialize")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are returning an error, shouldn't we log these as errors?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed from Warn to Error

return nil, fmt.Errorf("%q before %q", notificationInitialized, methodInitialize)
}
if wasInitd {
ss.server.logger.Warn("duplicate initialized notification")
return nil, fmt.Errorf("duplicate %q received", notificationInitialized)
}
if ss.server.opts.KeepAlive > 0 {
Expand All @@ -815,6 +842,7 @@ func (ss *ServerSession) initialized(ctx context.Context, params *InitializedPar
if h := ss.server.opts.InitializedHandler; h != nil {
h(ctx, serverRequestFor(ss, params))
}
ss.server.logger.Info("session initialized")
return nil, nil
}

Expand Down Expand Up @@ -1052,6 +1080,7 @@ func (ss *ServerSession) handle(ctx context.Context, req *jsonrpc.Request) (any,
case methodInitialize, methodPing, notificationInitialized:
default:
if !initialized {
ss.server.logger.Warn("method invalid during initialization", "method", req.Method)
return nil, fmt.Errorf("method %q is invalid during session initialization", req.Method)
}
}
Expand Down Expand Up @@ -1108,6 +1137,7 @@ func (ss *ServerSession) setLevel(_ context.Context, params *SetLoggingLevelPara
ss.updateState(func(state *ServerSessionState) {
state.LogLevel = params.Level
})
ss.server.logger.Info("client log level set", "level", params.Level)
return &emptyResult{}, nil
}

Expand Down
27 changes: 25 additions & 2 deletions mcp/streamable.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"fmt"
"io"
"iter"
"log/slog"
"math"
"math/rand/v2"
"net/http"
Expand Down Expand Up @@ -39,6 +40,7 @@ const (
type StreamableHTTPHandler struct {
getServer func(*http.Request) *Server
opts StreamableHTTPOptions
logger *slog.Logger
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use opts.Logger

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated code to use opts.Logger


onTransportDeletion func(sessionID string) // for testing only

Expand Down Expand Up @@ -67,6 +69,10 @@ type StreamableHTTPOptions struct {
//
// [§2.1.5]: https://modelcontextprotocol.io/specification/2025-06-18/basic/transports#sending-messages-to-the-server
JSONResponse bool

// Logger specifies the logger to use.
// If nil, do not log.
Logger *slog.Logger
}

// NewStreamableHTTPHandler returns a new [StreamableHTTPHandler].
Expand All @@ -82,6 +88,12 @@ func NewStreamableHTTPHandler(getServer func(*http.Request) *Server, opts *Strea
if opts != nil {
h.opts = *opts
}

if h.opts.Logger == nil { // ensure we have a logger
h.opts.Logger = ensureLogger(nil)
}
h.logger = h.opts.Logger

return h
}

Expand Down Expand Up @@ -367,6 +379,8 @@ type StreamableServerTransport struct {
// StreamableHTTPOptions.JSONResponse is exported.
jsonResponse bool

logger *slog.Logger

// connection is non-nil if and only if the transport has been connected.
connection *streamableServerConn
}
Expand All @@ -381,6 +395,7 @@ func (t *StreamableServerTransport) Connect(ctx context.Context) (Connection, er
stateless: t.Stateless,
eventStore: t.EventStore,
jsonResponse: t.jsonResponse,
logger: t.logger,
incoming: make(chan jsonrpc.Message, 10),
done: make(chan struct{}),
streams: make(map[string]*stream),
Expand All @@ -407,6 +422,8 @@ type streamableServerConn struct {
jsonResponse bool
eventStore EventStore

logger *slog.Logger

incoming chan jsonrpc.Message // messages from the client to the server

mu sync.Mutex // guards all fields below
Expand Down Expand Up @@ -754,7 +771,7 @@ func (c *streamableServerConn) respondSSE(stream *stream, w http.ResponseWriter,
}
if _, err := writeEvent(w, e); err != nil {
// Connection closed or broken.
// TODO(#170): log when we add server-side logging.
c.logger.Warn("error writing event", "error", err)
return false
}
writes++
Expand All @@ -773,7 +790,13 @@ func (c *streamableServerConn) respondSSE(stream *stream, w http.ResponseWriter,
// simplify.
http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
} else {
// TODO(#170): log when we add server-side logging
if ctx.Err() != nil {
// Client disconnected or cancelled the request.
c.logger.Info("stream context done", "error", ctx.Err())
} else {
// Some other error.
c.logger.Warn("error receiving message", "error", err)
}
}
return
}
Expand Down