slogr: restore original backend when converting back and forth

This works both ways by checking whether the backend is one of our own
wrappers. There's one exception: when the verbosity level was modified, we
cannot use the original slog.Handler because it doesn't know about that
modification.

The downside is that we have to adjust the stack unwinding each time a record
is handled, because we need the original sink without that adjustment. This
only gets done when actually emitting a log record, so that overhead should be
okay. It will disappear entirely once sinks directly support writing a
slog.Event.

A second copy would work, but then that copy would need to be updated alongside
the other one in WithAttrs, which causes overhead along a callchain also when
the log record does not get emitted. The code also would be more complex.

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)
+	}
+}