Logging: structured slog messages throughout, and WithLogger options so callers choose the logger

Log messages are lower-case event names with their details as key/value
attributes ("error" for errors), in place of formatted strings.

Loggable package take a WithLogger option as part of their "New*" call,
defaulting to slog.Default(). Each tags its lines with its `component`.

Some debug loggers print directly rather than using slog when in
terminal logging mode.

internal/websocket/client.go

@@ -106,7 +106,7 @@ func (c *client) run(ctx context.Context) {
 		// consumer), after a failed write or ping, or by the caller.
 		c.conn.CloseNow() //nolint:errcheck,gosec // finished with; it may already be closed
 	default:
-		c.hub.log.Debug("read error", "client", c.id, "error", err)
+		c.hub.logger().Debug("read error", "client", c.id, "error", err)
 		c.conn.CloseNow() //nolint:errcheck,gosec // the connection has failed anyway
 	}
 
@@ -178,7 +178,7 @@ func (c *client) writeLoop(ctx context.Context) {
 			err := c.conn.Write(writeCtx, websocket.MessageText, msg)
 			cancel()
 			if err != nil {
-				c.hub.log.Debug("write failed", "client", c.id, "error", err)
+				c.hub.logger().Debug("write failed", "client", c.id, "error", err)
 				c.cancel()
 				return
 			}
@@ -215,7 +215,7 @@ func (c *client) pingLoop(ctx context.Context) {
 			err := c.conn.Ping(pingCtx)
 			cancel()
 			if err != nil {
-				c.hub.log.Debug("ping failed", "client", c.id, "error", err)
+				c.hub.logger().Debug("ping failed", "client", c.id, "error", err)
 				c.conn.CloseNow() //nolint:errcheck,gosec // dropping the client anyway
 				c.cancel()
 				return
@@ -240,11 +240,11 @@ func (c *client) trySend(msg []byte) bool {
 func (c *client) sendEnvelope(env Envelope) {
 	encoded, err := json.Marshal(env)
 	if err != nil {
-		c.hub.log.Error("marshal", "client", c.id, "error", err)
+		c.hub.logger().Error("marshal", "client", c.id, "error", err)
 		return
 	}
 	if !c.trySend(encoded) {
-		c.hub.log.Warn("dropping slow client", "client", c.id, "ip", c.ip)
+		c.hub.logger().Warn("dropping slow client", "client", c.id, "ip", c.ip)
 		go c.close(websocket.StatusPolicyViolation, "slow consumer")
 	}
 }
@@ -273,14 +273,18 @@ func (c *client) close(code websocket.StatusCode, reason string) {
 // level, with its size; at most logMessageLimit bytes of it are logged. It
 // does nothing unless debug logging is enabled.
 func (c *client) logMessage(ctx context.Context, event string, data []byte) {
-	if !c.hub.log.Enabled(ctx, slog.LevelDebug) {
+	logger := c.hub.logger()
+	if !logger.Enabled(ctx, slog.LevelDebug) {
 		return
 	}
 	logged, truncated := data, false
 	if len(logged) > logMessageLimit {
 		logged, truncated = logged[:logMessageLimit], true
 	}
-	c.hub.log.LogAttrs(ctx, slog.LevelDebug, event,
+	logger.LogAttrs(
+		ctx,
+		slog.LevelDebug,
+		event,
 		slog.String("client", c.id),
 		slog.Int("bytes", len(data)),
 		slog.String("message", string(logged)),

internal/websocket/client_test.go

@@ -44,7 +44,7 @@ func (f *fakeConn) CloseNow() error { return nil }
 // newTestClient returns a client of a new hub on f, with no loops running,
 // and its context.
 func newTestClient(f *fakeConn) (*client, context.Context) {
-	h := NewHub(config.JSCServerWebSocket{}, discardLog)
+	h := NewHub(config.JSCServerWebSocket{}, WithLogger(discardLog))
 	ctx, cancel := context.WithCancel(context.Background())
 	return newClient(h, f, "192.0.2.1", cancel), ctx
 }
@@ -114,7 +114,7 @@ func TestClientMessageRateLimit(t *testing.T) {
 		{config.JSCRateLimitRule{Enabled: true, PerSecond: 1, Burst: 1}, true},
 	}
 	for _, tt := range tests {
-		h := NewHub(config.JSCServerWebSocket{MessageRateLimit: tt.rule}, discardLog)
+		h := NewHub(config.JSCServerWebSocket{MessageRateLimit: tt.rule}, WithLogger(discardLog))
 		c := newClient(h, newFakeConn(), "", func() {})
 		c.limiter.Allow()
 		if limited := !c.limiter.Allow(); limited != tt.limited {
@@ -193,7 +193,7 @@ func jsonLog(level slog.Level) (*slog.Logger, *syncBuffer) {
 // At debug level, messages received from and sent to a client are logged.
 func TestLogMessages(t *testing.T) {
 	log, buf := jsonLog(slog.LevelDebug)
-	h := NewHub(config.JSCServerWebSocket{WriteTimeout: time.Second}, log)
+	h := NewHub(config.JSCServerWebSocket{WriteTimeout: time.Second}, WithLogger(log))
 	peer, _ := pipeClient(t, h)
 
 	ping := `{"type":"ping","id":"1"}`
@@ -235,7 +235,7 @@ func TestLogMessages(t *testing.T) {
 // unless debug logging is on.
 func TestLogMessageLimit(t *testing.T) {
 	log, buf := jsonLog(slog.LevelDebug)
-	h := NewHub(config.JSCServerWebSocket{}, log)
+	h := NewHub(config.JSCServerWebSocket{}, WithLogger(log))
 	c := newClient(h, newFakeConn(), "", func() {})
 	long := strings.Repeat("x", logMessageLimit+100)
 	c.logMessage(context.Background(), "message sent", []byte(long))
@@ -251,7 +251,7 @@ func TestLogMessageLimit(t *testing.T) {
 	}
 
 	log, buf = jsonLog(slog.LevelInfo)
-	c.hub = NewHub(config.JSCServerWebSocket{}, log)
+	c.hub = NewHub(config.JSCServerWebSocket{}, WithLogger(log))
 	c.logMessage(context.Background(), "message sent", []byte("x"))
 	if entries := buf.entries(t); len(entries) != 0 {
 		t.Errorf("logged %v at info level", entries)

internal/websocket/http.go

@@ -22,7 +22,7 @@ func (h *Hub) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		CompressionMode: mode,
 	})
 	if err != nil {
-		h.log.Debug("upgrade failed", "error", err)
+		h.logger().Debug("upgrade failed", "error", err)
 		return // Accept has already written the HTTP error.
 	}
 	defer conn.CloseNow() //nolint:errcheck // finished with; it may already be closed
@@ -39,7 +39,7 @@ func (h *Hub) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	}
 	defer h.unregister(c)
 
-	h.log.Info("client connected", "client", c.id, "ip", ip)
+	h.logger().Info("client connected", "client", c.id, "ip", ip)
 	c.run(ctx)
-	h.log.Info("client disconnected", "client", c.id, "ip", ip)
+	h.logger().Info("client disconnected", "client", c.id, "ip", ip)
 }

internal/websocket/http_test.go

@@ -46,7 +46,7 @@ func dialWS(t *testing.T, ctx context.Context, url string) *ws.Conn {
 // A client upgraded over HTTP gets replies. A max_message_size of 0 uses the
 // default limit rather than refusing every message.
 func TestHTTPMessages(t *testing.T) {
-	h := NewHub(config.JSCServerWebSocket{WriteTimeout: time.Second}, discardLog)
+	h := NewHub(config.JSCServerWebSocket{WriteTimeout: time.Second}, WithLogger(discardLog))
 	url := wsHub(t, h)
 	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
 	defer cancel()
@@ -98,7 +98,7 @@ func TestPushBadPayload(t *testing.T) {
 
 // A binary frame closes the connection with 1003 (unsupported data).
 func TestHTTPBinaryFrame(t *testing.T) {
-	h := NewHub(config.JSCServerWebSocket{WriteTimeout: time.Second}, discardLog)
+	h := NewHub(config.JSCServerWebSocket{WriteTimeout: time.Second}, WithLogger(discardLog))
 	url := wsHub(t, h)
 	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
 	defer cancel()
@@ -120,7 +120,7 @@ func TestHTTPMessageRateLimit(t *testing.T) {
 		WriteTimeout:     time.Second,
 		MessageRateLimit: config.JSCRateLimitRule{Enabled: true, PerSecond: 1, Burst: 1},
 	}
-	h := NewHub(conf, discardLog)
+	h := NewHub(conf, WithLogger(discardLog))
 	url := wsHub(t, h)
 	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
 	defer cancel()
@@ -147,7 +147,7 @@ func TestHTTPMessageRateLimit(t *testing.T) {
 // A request that isn't a WebSocket upgrade is refused by the upgrade and no
 // client is registered.
 func TestHTTPNotAnUpgrade(t *testing.T) {
-	h := NewHub(config.JSCServerWebSocket{}, discardLog)
+	h := NewHub(config.JSCServerWebSocket{}, WithLogger(discardLog))
 	srv := httptest.NewServer(h)
 	defer srv.Close()
 	res, err := http.Get(srv.URL)
@@ -166,7 +166,7 @@ func TestHTTPNotAnUpgrade(t *testing.T) {
 // After Shutdown, an upgrade succeeds but the client is closed at once with
 // 1001 (going away).
 func TestHTTPRefusedAfterShutdown(t *testing.T) {
-	h := NewHub(config.JSCServerWebSocket{}, discardLog)
+	h := NewHub(config.JSCServerWebSocket{}, WithLogger(discardLog))
 	url := wsHub(t, h)
 	if err := h.Shutdown(context.Background()); err != nil {
 		t.Fatal(err)

internal/websocket/hub.go

@@ -27,13 +27,35 @@ type Hub struct {
 // max_message_size is not set.
 const defaultMaxMessageSize = 32 << 10
 
-// NewHub creates a hub.
-func NewHub(conf config.JSCServerWebSocket, log *slog.Logger) *Hub {
-	return &Hub{
+// Option configures a Hub.
+type Option func(*Hub)
+
+// WithLogger makes the hub log to logger instead of slog.Default().
+func WithLogger(logger *slog.Logger) Option {
+	return func(h *Hub) {
+		h.log = logger
+	}
+}
+
+// NewHub creates a hub. It logs to slog.Default() unless given WithLogger.
+func NewHub(conf config.JSCServerWebSocket, opts ...Option) *Hub {
+	hub := &Hub{
 		conf:    conf,
-		log:     log.With("component", "ws"),
 		clients: make(map[*client]struct{}),
 	}
+	for _, opt := range opts {
+		opt(hub)
+	}
+	return hub
+}
+
+// logger returns the logger the hub logs to, tagged with its component.
+func (h *Hub) logger() *slog.Logger {
+	log := h.log
+	if log == nil {
+		log = slog.Default()
+	}
+	return log.With("component", "ws")
 }
 
 // Push sends a message of type msgType to every client. The payload is
@@ -45,7 +67,7 @@ func (h *Hub) Push(msgType string, payload any) {
 	if payload != nil {
 		var err error
 		if rawPayload, err = json.Marshal(payload); err != nil {
-			h.log.Error("marshal push payload", "type", msgType, "error", err)
+			h.logger().Error("marshal push payload", "type", msgType, "error", err)
 			return
 		}
 	}
@@ -55,7 +77,7 @@ func (h *Hub) Push(msgType string, payload any) {
 		Payload: rawPayload,
 	})
 	if err != nil {
-		h.log.Error("marshal push", "type", msgType, "error", err)
+		h.logger().Error("marshal push", "type", msgType, "error", err)
 		return
 	}
 
@@ -69,7 +91,7 @@ func (h *Hub) Push(msgType string, payload any) {
 	h.m.RUnlock()
 
 	for _, client := range slow {
-		h.log.Warn("dropping slow client", "client", client.id, "ip", client.ip)
+		h.logger().Warn("dropping slow client", "client", client.id, "ip", client.ip)
 		go client.close(ws.StatusPolicyViolation, "slow consumer")
 	}
 }

internal/websocket/hub_test.go

@@ -32,7 +32,7 @@ func pipeClient(t *testing.T, h *Hub) (net.Conn, chan struct{}) {
 // once and returns ctx's error.
 func TestShutdownTimeout(t *testing.T) {
 	// No write timeout: the client's goodbye blocks until it is cut short.
-	h := NewHub(config.JSCServerWebSocket{}, discardLog)
+	h := NewHub(config.JSCServerWebSocket{}, WithLogger(discardLog))
 	_, done := pipeClient(t, h)
 
 	ctx, cancel := context.WithTimeout(context.Background(), 20*time.Millisecond)
@@ -49,7 +49,7 @@ func TestShutdownTimeout(t *testing.T) {
 
 // Shutdown with no clients returns at once, even with an expired ctx.
 func TestShutdownNoClients(t *testing.T) {
-	h := NewHub(config.JSCServerWebSocket{}, discardLog)
+	h := NewHub(config.JSCServerWebSocket{}, WithLogger(discardLog))
 	ctx, cancel := context.WithCancel(context.Background())
 	cancel()
 	for range 20 {
@@ -64,7 +64,7 @@ func TestPingFailureDropsClient(t *testing.T) {
 	h := NewHub(config.JSCServerWebSocket{
 		PingInterval: 10 * time.Millisecond,
 		PongTimeout:  20 * time.Millisecond,
-	}, discardLog)
+	}, WithLogger(discardLog))
 	_, done := pipeClient(t, h)
 
 	select {
@@ -85,7 +85,7 @@ func TestMaxMessageSize(t *testing.T) {
 		{config.JSCServerWebSocket{MaxMessageSize: 100}, 100},
 	}
 	for _, tt := range tests {
-		h := NewHub(tt.conf, discardLog)
+		h := NewHub(tt.conf, WithLogger(discardLog))
 		if got := h.maxMessageSize(); got != tt.want {
 			t.Errorf("maxMessageSize(%d) = %d; want %d", tt.conf.MaxMessageSize, got, tt.want)
 		}

internal/websocket/tcp.go

@@ -71,10 +71,10 @@ func (h *Hub) ServeTCP(ctx context.Context, newConn net.Conn) {
 	}
 	defer h.unregister(client)
 
-	h.log.Info("client connected", "client", client.id, "ip", ip, "transport", "tcp")
+	h.logger().Info("client connected", "client", client.id, "ip", ip, "transport", "tcp")
 	client.run(ctx)
 	conn.CloseNow() //nolint:errcheck,gosec // finished with; it may already be closed
-	h.log.Info("client disconnected", "client", client.id, "ip", ip, "transport", "tcp")
+	h.logger().Info("client disconnected", "client", client.id, "ip", ip, "transport", "tcp")
 }
 
 // ---------- tcpConn ----------

internal/websocket/tcp_test.go

@@ -31,7 +31,7 @@ func tcpHub(t *testing.T, mutate func(*config.JSCServerWebSocket)) (*Hub, string
 	if mutate != nil {
 		mutate(&conf)
 	}
-	h := NewHub(conf, discardLog)
+	h := NewHub(conf, WithLogger(discardLog))
 	ln, err := net.Listen("tcp", "127.0.0.1:0")
 	if err != nil {
 		t.Fatal(err)
@@ -243,7 +243,9 @@ func TestTCPPing(t *testing.T) {
 // A client that doesn't read is dropped as a slow consumer once its queue is
 // full. net.Pipe has no buffer, so writes to it block until the peer reads.
 func TestTCPSlowConsumer(t *testing.T) {
-	h := NewHub(config.JSCServerWebSocket{WriteTimeout: 50 * time.Millisecond}, discardLog)
+	h := NewHub(
+		config.JSCServerWebSocket{WriteTimeout: 50 * time.Millisecond}, WithLogger(discardLog),
+	)
 	server, peer := net.Pipe()
 	t.Cleanup(func() { server.Close(); peer.Close() })
 	done := make(chan struct{})

internal/websocket/tcpserver/server.go

@@ -33,15 +33,38 @@ type Server struct {
 	hub  *websocket.Hub
 }
 
+// Option configures a Server.
+type Option func(*Server)
+
+// WithLogger makes the server log to logger instead of slog.Default().
+func WithLogger(logger *slog.Logger) Option {
+	return func(s *Server) {
+		s.log = logger
+	}
+}
+
 // New builds a server for hub's plain TCP clients. Run starts it; it shuts
 // hub down when it stops, so hub should not be shared with another server.
-// Push to hub to reach the server's clients.
-func New(conf *config.JSCServer, hub *websocket.Hub, log *slog.Logger) *Server {
-	return &Server{
+// Push to hub to reach the server's clients. It logs to slog.Default()
+// unless given WithLogger.
+func New(conf *config.JSCServer, hub *websocket.Hub, opts ...Option) *Server {
+	server := &Server{
 		conf: conf,
-		log:  log,
 		hub:  hub,
 	}
+	for _, opt := range opts {
+		opt(server)
+	}
+	return server
+}
+
+// logger returns the logger the server logs to, tagged with its component.
+func (s *Server) logger() *slog.Logger {
+	log := s.log
+	if log == nil {
+		log = slog.Default()
+	}
+	return log.With("component", "tcp")
 }
 
 // Run listens on the configured host and port and serves plain TCP clients
@@ -71,14 +94,14 @@ func (s *Server) serve(ctx context.Context, listen net.Listener) error {
 	})
 	defer stop()
 
-	s.log.Info("listening", "addr", listen.Addr().String())
+	s.logger().Info("listening", "addr", listen.Addr().String())
 	acceptErr := s.accept(ctx, connCtx, listen)
 	if ctx.Err() != nil {
 		acceptErr = nil // cancelling ctx is how the server is stopped
 	}
 	listen.Close() //nolint:errcheck,gosec // closed already when ctx was cancelled
 
-	s.log.Info("shutting down")
+	s.logger().Info("shutting down")
 	shutdownCtx := context.Background()
 	if s.conf.Timeouts.ShutdownTimeout > 0 {
 		var cancel context.CancelFunc
@@ -117,7 +140,7 @@ func (s *Server) accept(
 
 			// Back off and retry e.g. when out of file descriptors.
 			delay = min(max(2*delay, 5*time.Millisecond), time.Second)
-			s.log.Warn("tcp accept failed", "error", err, "retry_in", delay)
+			s.logger().Warn("tcp accept failed", "error", err, "retry_in", delay)
 
 			select {
 			case <-time.After(delay):
@@ -131,7 +154,7 @@ func (s *Server) accept(
 		if limiter != nil {
 			ip := remoteIP(newConn)
 			if allowed, _ := limiter.Allow(ip); !allowed {
-				s.log.Warn("tcp connection rate limited", "ip", ip)
+				s.logger().Warn("tcp connection rate limited", "ip", ip)
 				go reject(newConn, "rate limited")
 				continue
 			}

internal/websocket/tcpserver/server_test.go

@@ -36,8 +36,8 @@ func testConf(mutate func(*config.JSCServer)) *config.JSCServer {
 
 // newServer returns a server for conf with its own hub.
 func newServer(conf *config.JSCServer) (*Server, *websocket.Hub) {
-	h := websocket.NewHub(conf.WebSocket, discardLog)
-	return New(conf, h, discardLog), h
+	h := websocket.NewHub(conf.WebSocket, websocket.WithLogger(discardLog))
+	return New(conf, h, WithLogger(discardLog)), h
 }
 
 // startServer serves on a local port, returning the server's hub, its