fix(logging): fall back to the slog default logger in LogHolder

LogHolder.Logger() documented "If nil, returns slog.Default()" but returned
slog.New(slog.DiscardHandler) with a "Should never be reached" comment, so any
LogHolder whose SetLogger was never called silently dropped every record.
SetLogger's comment claimed it "sets the default logger" for a nil handler while
it also stored a discard handler.

Return slog.Default() when no logger is set, and treat SetLogger(nil) as
clearing the logger so the same default fallback applies. Fix both doc comments
to match. The Helm CLI is unaffected because pkg/cmd/root.go injects its own
handler, but SDK consumers now get logs on the standard slog default.

Closes #31433

Signed-off-by: davidpavlovschi <davidpavlov2048@gmail.com>
pull/32499/head
davidpavlovschi 2 months ago committed by David PAVLOVSCHII
parent a8ab76e86f
commit 134ee02d29

@ -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))

@ -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) {

Loading…
Cancel
Save