slogr: restore original backend when converting back and forth

slogr/sloghandler.go

@@ -46,6 +46,11 @@ var _ slog.Handler = &slogHandler{}
 // groupSeparator is used to concatenate WithGroup names and attribute keys.
 const groupSeparator = "."
 
+// GetLevel is used for black box unit testing.
+func (l *slogHandler) GetLevel() slog.Level {
+	return l.levelBias
+}
+
 func (l *slogHandler) Enabled(ctx context.Context, level slog.Level) bool {
 	return l.sink != nil && (level >= slog.LevelError || l.sink.Enabled(l.levelFromSlog(level)))
 }
@@ -62,14 +67,32 @@ func (l *slogHandler) Handle(ctx context.Context, record slog.Record) error {
 		return true
 	})
 	if record.Level >= slog.LevelError {
-		l.sink.Error(nil, record.Message, kvList...)
+		l.sinkWithCallDepth().Error(nil, record.Message, kvList...)
 	} else {
 		level := l.levelFromSlog(record.Level)
-		l.sink.Info(level, record.Message, kvList...)
+		l.sinkWithCallDepth().Info(level, record.Message, kvList...)
 	}
 	return nil
 }
 
+// sinkWithCallDepth adjusts the stack unwinding so that when Error or Info
+// are called by Handle, code in slog gets skipped.
+//
+// This offset currently (Go 1.21.0) works for calls through
+// slog.New(NewSlogHandler(...)).  There's no guarantee that the call
+// chain won't change. Wrapping the handler will also break unwinding. It's
+// still better than not adjusting at all....
+//
+// This cannot be done when constructing the handler because NewLogr needs
+// access to the original sink without this adjustment. A second copy would
+// work, but then WithAttrs would have to be called for both of them.
+func (l *slogHandler) sinkWithCallDepth() logr.LogSink {
+	if sink, ok := l.sink.(logr.CallDepthLogSink); ok {
+		return sink.WithCallDepth(2)
+	}
+	return l.sink
+}
+
 func (l *slogHandler) WithAttrs(attrs []slog.Attr) slog.Handler {
 	if l.sink == nil || len(attrs) == 0 {
 		return l

slogr/slogr.go

@@ -35,6 +35,12 @@ import (
 // The logr verbosity level is mapped to slog levels such that V(0) becomes
 // slog.LevelInfo and V(4) becomes slog.LevelDebug.
 func NewLogr(handler slog.Handler) logr.Logger {
+	if handler, ok := handler.(*slogHandler); ok {
+		if handler.sink == nil {
+			return logr.Discard()
+		}
+		return logr.New(handler.sink).V(int(handler.levelBias))
+	}
 	return logr.New(&slogSink{handler: handler})
 }
 
@@ -57,10 +63,9 @@ func NewLogr(handler slog.Handler) logr.Logger {
 //	slog.New(NewSlogHandler(logger)).Info(...) -> logger.GetSink().Info(level=0, ...)
 //	slog.New(NewSlogHandler(logger.V(4))).Info(...) -> logger.GetSink().Info(level=4, ...)
 func NewSlogHandler(logger logr.Logger) slog.Handler {
-	// This offset currently (Go 1.21.0) works for slog.New(NewSlogHandler(...)).Info.
-	// There's no guarantee that the call chain won't change and wrapping
-	// the handler will also break unwinding, but it's still better than not
-	// adjusting at all.
-	logger = logger.WithCallDepth(2)
+	if sink, ok := logger.GetSink().(*slogSink); ok && logger.GetV() == 0 {
+		return sink.handler
+	}
+
 	return &slogHandler{sink: logger.GetSink(), levelBias: slog.Level(logger.GetV())}
 }

slogr/slogr_test.go

@@ -24,6 +24,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"io"
 	"log/slog"
 	"os"
 	"path"
@@ -184,3 +185,37 @@ func TestDiscard(t *testing.T) {
 	logger := slog.New(slogr.NewSlogHandler(logr.Discard()))
 	logger.WithGroup("foo").With("x", 1).Info("hello")
 }
+
+func TestConversion(t *testing.T) {
+	d := logr.Discard()
+	d2 := slogr.NewLogr(slogr.NewSlogHandler(d))
+	expectEqual(t, d, d2)
+
+	e := logr.Logger{}
+	e2 := slogr.NewLogr(slogr.NewSlogHandler(e))
+	expectEqual(t, e, e2)
+
+	f := funcr.New(func(prefix, args string) {}, funcr.Options{})
+	f2 := slogr.NewLogr(slogr.NewSlogHandler(f))
+	expectEqual(t, f, f2)
+
+	text := slog.NewTextHandler(io.Discard, nil)
+	text2 := slogr.NewSlogHandler(slogr.NewLogr(text))
+	expectEqual(t, text, text2)
+
+	text3 := slogr.NewSlogHandler(slogr.NewLogr(text).V(1))
+	if handler, ok := text3.(interface {
+		GetLevel() slog.Level
+	}); ok {
+		expectEqual(t, handler.GetLevel(), slog.Level(1))
+	} else {
+		t.Errorf("Expected a slogHandler which implements V(1), got instead: %T %+v", text3, text3)
+	}
+}
+
+func expectEqual(t *testing.T, expected, actual any) {
+	if expected != actual {
+		t.Helper()
+		t.Errorf("Expected %T %+v, got instead: %T %+v", expected, expected, actual, actual)
+	}
+}