diff --git a/internal/logging/logging.go b/internal/logging/logging.go index 674e2db34..611530159 100644 --- a/internal/logging/logging.go +++ b/internal/logging/logging.go @@ -104,18 +104,20 @@ type LogHolder struct { logger atomic.Pointer[slog.Logger] } -// Logger returns the logger for the LogHolder. If nil, returns slog.Default(). +// Logger returns the logger for the LogHolder. When no logger has been set, +// it returns slog.Default() so log records are not silently dropped. func (l *LogHolder) Logger() *slog.Logger { if lg := l.logger.Load(); lg != nil { return lg } - return slog.New(slog.DiscardHandler) // Should never be reached + return slog.Default() } -// SetLogger sets the logger for the LogHolder. If nil, sets the default logger. +// SetLogger sets the logger for the LogHolder. A nil handler clears the logger, +// so Logger() falls back to slog.Default() again. func (l *LogHolder) SetLogger(newHandler slog.Handler) { if newHandler == nil { - l.logger.Store(slog.New(slog.DiscardHandler)) // Assume nil as discarding logs + l.logger.Store(nil) return } l.logger.Store(slog.New(newHandler)) diff --git a/internal/logging/logging_test.go b/internal/logging/logging_test.go index 1d6eaf852..4fd23e638 100644 --- a/internal/logging/logging_test.go +++ b/internal/logging/logging_test.go @@ -43,14 +43,75 @@ func TestLogHolder_Logger(t *testing.T) { assert.Contains(t, buf.String(), "test message") }) - t.Run("should return discard - defaultlogger when no handler is set", func(t *testing.T) { + t.Run("should return the slog default logger when no handler is set", func(t *testing.T) { holder := &LogHolder{} logger := holder.Logger() - assert.Equal(t, slog.Handler(slog.DiscardHandler), logger.Handler()) + assert.Equal(t, slog.Default(), logger) }) } +// TestLogHolder_DefaultHandler covers where a LogHolder sends records for each +// way of (not) configuring it, against a capture handler installed with +// slog.SetDefault. +func TestLogHolder_DefaultHandler(t *testing.T) { + tests := []struct { + name string + setup func(holder *LogHolder, ownHandler slog.Handler) + wantDefault bool // record lands in the slog default buffer + wantOwn bool // record lands in the handler set on the holder + }{ + { + name: "fresh holder logs through the slog default", + setup: func(_ *LogHolder, _ slog.Handler) {}, + wantDefault: true, + }, + { + name: "handler set on the holder wins over the slog default", + setup: func(holder *LogHolder, ownHandler slog.Handler) { + holder.SetLogger(ownHandler) + }, + wantOwn: true, + }, + { + name: "nil handler falls back to the slog default", + setup: func(holder *LogHolder, ownHandler slog.Handler) { + holder.SetLogger(ownHandler) + holder.SetLogger(nil) + }, + wantDefault: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + defaultBuf := &bytes.Buffer{} + previous := slog.Default() + slog.SetDefault(slog.New(slog.NewTextHandler(defaultBuf, &slog.HandlerOptions{Level: slog.LevelDebug}))) + t.Cleanup(func() { slog.SetDefault(previous) }) + + ownBuf := &bytes.Buffer{} + ownHandler := slog.NewTextHandler(ownBuf, &slog.HandlerOptions{Level: slog.LevelDebug}) + + holder := &LogHolder{} + tt.setup(holder, ownHandler) + + holder.Logger().Info("test message") + + if tt.wantDefault { + assert.Contains(t, defaultBuf.String(), "test message") + } else { + assert.Empty(t, defaultBuf.String()) + } + if tt.wantOwn { + assert.Contains(t, ownBuf.String(), "test message") + } else { + assert.Empty(t, ownBuf.String()) + } + }) + } +} + func TestLogHolder_SetLogger(t *testing.T) { t.Run("sets logger with valid handler", func(t *testing.T) { holder := &LogHolder{} @@ -66,15 +127,17 @@ func TestLogHolder_SetLogger(t *testing.T) { assert.Equal(t, handler, logger.Handler()) }) - t.Run("sets discard logger with nil handler", func(t *testing.T) { + t.Run("clears the logger with nil handler", func(t *testing.T) { holder := &LogHolder{} + buf := &bytes.Buffer{} + holder.SetLogger(slog.NewTextHandler(buf, &slog.HandlerOptions{Level: slog.LevelDebug})) holder.SetLogger(nil) logger := holder.Logger() assert.NotNil(t, logger) - assert.Equal(t, slog.Handler(slog.DiscardHandler), logger.Handler()) + assert.Equal(t, slog.Default(), logger) }) t.Run("can replace existing logger", func(t *testing.T) {