address review comments

Author: IAmSurajBobade

mcp/logging.go

@@ -89,7 +89,7 @@ type LoggingHandler struct {
 }
 
 // discardHandler is a slog.Handler that drops all logs.
-// TODO: use slog.NewNopHandler when we require Go 1.24+.
+// TODO: use slog.DiscardHandler when we require Go 1.24+.
 type discardHandler struct{}
 
 func (discardHandler) Enabled(context.Context, slog.Level) bool  { return false }

mcp/server.go

@@ -36,9 +36,8 @@ const DefaultPageSize = 1000
 // sessions by using [Server.Run].
 type Server struct {
 	// fixed at creation
-	impl   *Implementation
-	opts   ServerOptions
-	logger *slog.Logger
+	impl *Implementation
+	opts ServerOptions
 
 	mu                      sync.Mutex
 	prompts                 *featureSet[*serverPrompt]
@@ -55,7 +54,7 @@ type Server struct {
 type ServerOptions struct {
 	// Optional instructions for connected clients.
 	Instructions string
-	// Logger is used for server-side logging. If nil, disable logging.
+	// If non-nil, log server activity.
 	Logger *slog.Logger
 	// If non-nil, called when "notifications/initialized" is received.
 	InitializedHandler func(context.Context, *InitializedRequest)
@@ -136,7 +135,6 @@ 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 }),
@@ -664,7 +662,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))
+	s.opts.Logger.Info("resource updated notification sent", "uri", params.URI, "subscriber_count", len(sessions))
 	return nil
 }
 
@@ -682,7 +680,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())
+	s.opts.Logger.Info("resource subscribed", "uri", req.Params.URI, "session_id", req.Session.ID())
 
 	return &emptyResult{}, nil
 }
@@ -704,7 +702,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())
+	s.opts.Logger.Info("resource unsubscribed", "uri", req.Params.URI, "session_id", req.Session.ID())
 
 	return &emptyResult{}, nil
 }
@@ -723,10 +721,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")
+	s.opts.Logger.Info("server run start")
 	ss, err := s.Connect(ctx, t, nil)
 	if err != nil {
-		s.logger.Error("server connect failed", "error", err)
+		s.opts.Logger.Error("server connect failed", "error", err)
 		return err
 	}
 
@@ -738,13 +736,13 @@ 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())
+		s.opts.Logger.Error("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)
+			s.opts.Logger.Error("server session ended with error", "error", err)
 		} else {
-			s.logger.Info("server session ended")
+			s.opts.Logger.Info("server session ended")
 		}
 		return err
 	}
@@ -761,7 +759,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())
+	s.opts.Logger.Info("server session connected", "session_id", ss.ID())
 	return ss
 }
 
@@ -777,7 +775,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())
+	s.opts.Logger.Info("server session disconnected", "session_id", cc.ID())
 }
 
 // ServerSessionOptions configures the server session.
@@ -803,10 +801,10 @@ func (s *Server) Connect(ctx context.Context, t Transport, opts *ServerSessionOp
 		onClose = opts.onClose
 	}
 
-	s.logger.Info("server connecting")
+	s.opts.Logger.Info("server connecting")
 	ss, err := connect(ctx, t, s, state, onClose)
 	if err != nil {
-		s.logger.Error("server connect error", "error", err)
+		s.opts.Logger.Error("server connect error", "error", err)
 		return nil, err
 	}
 	return ss, nil
@@ -829,11 +827,11 @@ func (ss *ServerSession) initialized(ctx context.Context, params *InitializedPar
 	})
 
 	if !wasInit {
-		ss.server.logger.Warn("initialized before initialize")
+		ss.server.opts.Logger.Error("initialized before initialize")
 		return nil, fmt.Errorf("%q before %q", notificationInitialized, methodInitialize)
 	}
 	if wasInitd {
-		ss.server.logger.Warn("duplicate initialized notification")
+		ss.server.opts.Logger.Error("duplicate initialized notification")
 		return nil, fmt.Errorf("duplicate %q received", notificationInitialized)
 	}
 	if ss.server.opts.KeepAlive > 0 {
@@ -842,7 +840,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")
+	ss.server.opts.Logger.Info("session initialized")
 	return nil, nil
 }
 
@@ -1080,7 +1078,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)
+			ss.server.opts.Logger.Error("method invalid during initialization", "method", req.Method)
 			return nil, fmt.Errorf("method %q is invalid during session initialization", req.Method)
 		}
 	}
@@ -1137,7 +1135,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)
+	ss.server.opts.Logger.Info("client log level set", "level", params.Level)
 	return &emptyResult{}, nil
 }
 

mcp/streamable.go

@@ -40,7 +40,6 @@ const (
 type StreamableHTTPHandler struct {
 	getServer func(*http.Request) *Server
 	opts      StreamableHTTPOptions
-	logger    *slog.Logger
 
 	onTransportDeletion func(sessionID string) // for testing only
 
@@ -92,7 +91,6 @@ func NewStreamableHTTPHandler(getServer func(*http.Request) *Server, opts *Strea
 	if h.opts.Logger == nil { // ensure we have a logger
 		h.opts.Logger = ensureLogger(nil)
 	}
-	h.logger = h.opts.Logger
 
 	return h
 }