From 5e3484c0df368bf0dce33098f4ae8359cd43276e Mon Sep 17 00:00:00 2001
From: Patrick Ohly <patrick.ohly@intel.com>
Date: Sun, 26 Nov 2023 17:52:52 +0100
Subject: [PATCH 1/2] move slogr into main package

This is necessary to give the context handling functions access to the slogr
functionality. Functions get renamed to match names in the main package
where needed.

The slogr package is now deprecated, but continues to be supported as part of
the logr v1 API by keeping all exported items as wrappers or aliases for the
main package.
---
 README.md                                |  28 +++----
 {slogr/example => examples/slog}/main.go |   7 +-
 slogr/sloghandler.go => sloghandler.go   |  33 ++++----
 slogr.go                                 | 100 +++++++++++++++++++++++
 slogr/slogr.go                           |  76 +++--------------
 slogr/slogr_test.go => slogr_test.go     |  49 ++++++-----
 slogr/slogsink.go => slogsink.go         |  20 ++---
 7 files changed, 177 insertions(+), 136 deletions(-)
 rename {slogr/example => examples/slog}/main.go (93%)
 rename slogr/sloghandler.go => sloghandler.go (83%)
 create mode 100644 slogr.go
 rename slogr/slogr_test.go => slogr_test.go (82%)
 rename slogr/slogsink.go => slogsink.go (85%)

diff --git a/README.md b/README.md
index a8c29bf..a9b1a4e 100644
--- a/README.md
+++ b/README.md
@@ -94,8 +94,8 @@ logr design but also left out some parts and changed others:
 
 The high-level slog API is explicitly meant to be one of many different APIs
 that can be layered on top of a shared `slog.Handler`. logr is one such
-alternative API, with [interoperability](#slog-interoperability) provided by the [`slogr`](slogr)
-package.
+alternative API, with [interoperability](#slog-interoperability) provided by
+some conversion functions.
 
 ### Inspiration
 
@@ -145,24 +145,24 @@ There are implementations for the following logging libraries:
 ## slog interoperability
 
 Interoperability goes both ways, using the `logr.Logger` API with a `slog.Handler`
-and using the `slog.Logger` API with a `logr.LogSink`. [slogr](./slogr) provides `NewLogr` and
-`NewSlogHandler` API calls to convert between a `logr.Logger` and a `slog.Handler`.
+and using the `slog.Logger` API with a `logr.LogSink`. `FromSlogHandler` and
+`ToSlogHandler` convert between a `logr.Logger` and a `slog.Handler`.
 As usual, `slog.New` can be used to wrap such a `slog.Handler` in the high-level
-slog API. `slogr` itself leaves that to the caller.
+slog API.
 
-## Using a `logr.Sink` as backend for slog
+### Using a `logr.LogSink` as backend for slog
 
 Ideally, a logr sink implementation should support both logr and slog by
-implementing both the normal logr interface(s) and `slogr.SlogSink`.  Because
+implementing both the normal logr interface(s) and `SlogSink`.  Because
 of a conflict in the parameters of the common `Enabled` method, it is [not
 possible to implement both slog.Handler and logr.Sink in the same
 type](https://github.com/golang/go/issues/59110).
 
 If both are supported, log calls can go from the high-level APIs to the backend
-without the need to convert parameters. `NewLogr` and `NewSlogHandler` can
+without the need to convert parameters. `FromSlogHandler` and `ToSlogHandler` can
 convert back and forth without adding additional wrappers, with one exception:
 when `Logger.V` was used to adjust the verbosity for a `slog.Handler`, then
-`NewSlogHandler` has to use a wrapper which adjusts the verbosity for future
+`ToSlogHandler` has to use a wrapper which adjusts the verbosity for future
 log calls.
 
 Such an implementation should also support values that implement specific
@@ -187,13 +187,13 @@ Not supporting slog has several drawbacks:
 These drawbacks are severe enough that applications using a mixture of slog and
 logr should switch to a different backend.
 
-## Using a `slog.Handler` as backend for logr
+### Using a `slog.Handler` as backend for logr
 
 Using a plain `slog.Handler` without support for logr works better than the
 other direction:
 - All logr verbosity levels can be mapped 1:1 to their corresponding slog level
   by negating them.
-- Stack unwinding is done by the `slogr.SlogSink` and the resulting program
+- Stack unwinding is done by the `SlogSink` and the resulting program
   counter is passed to the `slog.Handler`.
 - Names added via `Logger.WithName` are gathered and recorded in an additional
   attribute with `logger` as key and the names separated by slash as value.
@@ -205,7 +205,7 @@ ideally support both `logr.Marshaler` and `slog.Valuer`. If compatibility
 with logr implementations without slog support is not important, then
 `slog.Valuer` is sufficient.
 
-## Context support for slog
+### Context support for slog
 
 Storing a logger in a `context.Context` is not supported by
 slog. `logr.NewContext` and `logr.FromContext` can be used with slog like this
@@ -214,13 +214,13 @@ to fill this gap:
     func HandlerFromContext(ctx context.Context) slog.Handler {
         logger, err := logr.FromContext(ctx)
         if err == nil {
-            return slogr.NewSlogHandler(logger)
+            return ToSlogHandler(logger)
         }
         return slog.Default().Handler()
     }
 
     func ContextWithHandler(ctx context.Context, handler slog.Handler) context.Context {
-        return logr.NewContext(ctx, slogr.NewLogr(handler))
+        return logr.NewContext(ctx, FromSlogHandler(handler))
     }
 
 The downside is that storing and retrieving a `slog.Handler` needs more
diff --git a/slogr/example/main.go b/examples/slog/main.go
similarity index 93%
rename from slogr/example/main.go
rename to examples/slog/main.go
index c6379d2..599d156 100644
--- a/slogr/example/main.go
+++ b/examples/slog/main.go
@@ -28,7 +28,6 @@ import (
 
 	"github.com/go-logr/logr"
 	"github.com/go-logr/logr/funcr"
-	"github.com/go-logr/logr/slogr"
 )
 
 type e struct {
@@ -62,7 +61,7 @@ func main() {
 		Level:     slog.Level(-1),
 	}
 	handler := slog.NewJSONHandler(os.Stderr, &opts)
-	logrLogger := slogr.NewLogr(handler)
+	logrLogger := logr.FromSlogHandler(handler)
 	logrExample(logrLogger)
 
 	logrLogger = funcr.NewJSON(
@@ -72,7 +71,7 @@ func main() {
 			LogTimestamp: true,
 			Verbosity:    1,
 		})
-	slogLogger := slog.New(slogr.NewSlogHandler(logrLogger))
+	slogLogger := slog.New(logr.ToSlogHandler(logrLogger))
 	slogExample(slogLogger)
 }
 
@@ -91,7 +90,7 @@ func logrExample(log logr.Logger) {
 
 func slogExample(log *slog.Logger) {
 	// There's no guarantee that this logs the right source code location.
-	// It works for Go 1.21.0 by compensating in slogr.NewSlogHandler
+	// It works for Go 1.21.0 by compensating in logr.ToSlogHandler
 	// for the additional callers, but those might change.
 	log = log.With("saved", "value")
 	log.Info("1) hello", "val1", 1, "val2", map[string]int{"k": 1})
diff --git a/slogr/sloghandler.go b/sloghandler.go
similarity index 83%
rename from slogr/sloghandler.go
rename to sloghandler.go
index ec6725c..7d406d7 100644
--- a/slogr/sloghandler.go
+++ b/sloghandler.go
@@ -17,18 +17,16 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package slogr
+package logr
 
 import (
 	"context"
 	"log/slog"
-
-	"github.com/go-logr/logr"
 )
 
 type slogHandler struct {
 	// May be nil, in which case all logs get discarded.
-	sink logr.LogSink
+	sink LogSink
 	// Non-nil if sink is non-nil and implements SlogSink.
 	slogSink SlogSink
 
@@ -90,15 +88,15 @@ func (l *slogHandler) Handle(ctx context.Context, record slog.Record) error {
 // 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
+// slog.New(ToSlogHandler(...)).  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
+// This cannot be done when constructing the handler because FromSlogHandler 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 {
+func (l *slogHandler) sinkWithCallDepth() LogSink {
+	if sink, ok := l.sink.(CallDepthLogSink); ok {
 		return sink.WithCallDepth(2)
 	}
 	return l.sink
@@ -148,21 +146,22 @@ func (l *slogHandler) addGroupPrefix(name string) string {
 
 // levelFromSlog adjusts the level by the logger's verbosity and negates it.
 // It ensures that the result is >= 0. This is necessary because the result is
-// passed to a logr.LogSink and that API did not historically document whether
+// passed to a LogSink and that API did not historically document whether
 // levels could be negative or what that meant.
 //
 // Some example usage:
-//     logrV0 := getMyLogger()
-//     logrV2 := logrV0.V(2)
-//     slogV2 := slog.New(slogr.NewSlogHandler(logrV2))
-//     slogV2.Debug("msg") // =~ logrV2.V(4) =~ logrV0.V(6)
-//     slogV2.Info("msg")  // =~  logrV2.V(0) =~ logrV0.V(2)
-//     slogv2.Warn("msg")  // =~ logrV2.V(-4) =~ logrV0.V(0)
+//
+//	logrV0 := getMyLogger()
+//	logrV2 := logrV0.V(2)
+//	slogV2 := slog.New(logr.ToSlogHandler(logrV2))
+//	slogV2.Debug("msg") // =~ logrV2.V(4) =~ logrV0.V(6)
+//	slogV2.Info("msg")  // =~  logrV2.V(0) =~ logrV0.V(2)
+//	slogv2.Warn("msg")  // =~ logrV2.V(-4) =~ logrV0.V(0)
 func (l *slogHandler) levelFromSlog(level slog.Level) int {
 	result := -level
-	result += l.levelBias // in case the original logr.Logger had a V level
+	result += l.levelBias // in case the original Logger had a V level
 	if result < 0 {
-		result = 0 // because logr.LogSink doesn't expect negative V levels
+		result = 0 // because LogSink doesn't expect negative V levels
 	}
 	return int(result)
 }
diff --git a/slogr.go b/slogr.go
new file mode 100644
index 0000000..28a83d0
--- /dev/null
+++ b/slogr.go
@@ -0,0 +1,100 @@
+//go:build go1.21
+// +build go1.21
+
+/*
+Copyright 2023 The logr Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package logr
+
+import (
+	"context"
+	"log/slog"
+)
+
+// FromSlogHandler returns a Logger which writes to the slog.Handler.
+//
+// The logr verbosity level is mapped to slog levels such that V(0) becomes
+// slog.LevelInfo and V(4) becomes slog.LevelDebug.
+func FromSlogHandler(handler slog.Handler) Logger {
+	if handler, ok := handler.(*slogHandler); ok {
+		if handler.sink == nil {
+			return Discard()
+		}
+		return New(handler.sink).V(int(handler.levelBias))
+	}
+	return New(&slogSink{handler: handler})
+}
+
+// ToSlogHandler returns a slog.Handler which writes to the same sink as the Logger.
+//
+// The returned logger writes all records with level >= slog.LevelError as
+// error log entries with LogSink.Error, regardless of the verbosity level of
+// the Logger:
+//
+//	logger := <some Logger with 0 as verbosity level>
+//	slog.New(ToSlogHandler(logger.V(10))).Error(...) -> logSink.Error(...)
+//
+// The level of all other records gets reduced by the verbosity
+// level of the Logger and the result is negated. If it happens
+// to be negative, then it gets replaced by zero because a LogSink
+// is not expected to handled negative levels:
+//
+//	slog.New(ToSlogHandler(logger)).Debug(...) -> logger.GetSink().Info(level=4, ...)
+//	slog.New(ToSlogHandler(logger)).Warning(...) -> logger.GetSink().Info(level=0, ...)
+//	slog.New(ToSlogHandler(logger)).Info(...) -> logger.GetSink().Info(level=0, ...)
+//	slog.New(ToSlogHandler(logger.V(4))).Info(...) -> logger.GetSink().Info(level=4, ...)
+func ToSlogHandler(logger Logger) slog.Handler {
+	if sink, ok := logger.GetSink().(*slogSink); ok && logger.GetV() == 0 {
+		return sink.handler
+	}
+
+	handler := &slogHandler{sink: logger.GetSink(), levelBias: slog.Level(logger.GetV())}
+	if slogSink, ok := handler.sink.(SlogSink); ok {
+		handler.slogSink = slogSink
+	}
+	return handler
+}
+
+// SlogSink is an optional interface that a LogSink can implement to support
+// logging through the slog.Logger or slog.Handler APIs better. It then should
+// also support special slog values like slog.Group. When used as a
+// slog.Handler, the advantages are:
+//
+//   - stack unwinding gets avoided in favor of logging the pre-recorded PC,
+//     as intended by slog
+//   - proper grouping of key/value pairs via WithGroup
+//   - verbosity levels > slog.LevelInfo can be recorded
+//   - less overhead
+//
+// Both APIs (Logger and slog.Logger/Handler) then are supported equally
+// well. Developers can pick whatever API suits them better and/or mix
+// packages which use either API in the same binary with a common logging
+// implementation.
+//
+// This interface is necessary because the type implementing the LogSink
+// interface cannot also implement the slog.Handler interface due to the
+// different prototype of the common Enabled method.
+//
+// An implementation could support both interfaces in two different types, but then
+// additional interfaces would be needed to convert between those types in FromSlogHandler
+// and ToSlogHandler.
+type SlogSink interface {
+	LogSink
+
+	Handle(ctx context.Context, record slog.Record) error
+	WithAttrs(attrs []slog.Attr) SlogSink
+	WithGroup(name string) SlogSink
+}
diff --git a/slogr/slogr.go b/slogr/slogr.go
index eb519ae..4fdf3b6 100644
--- a/slogr/slogr.go
+++ b/slogr/slogr.go
@@ -23,10 +23,11 @@ limitations under the License.
 //
 // See the README in the top-level [./logr] package for a discussion of
 // interoperability.
+//
+// Deprecated: use the main logr package instead.
 package slogr
 
 import (
-	"context"
 	"log/slog"
 
 	"github.com/go-logr/logr"
@@ -34,75 +35,20 @@ import (
 
 // NewLogr returns a logr.Logger which writes to the slog.Handler.
 //
-// The logr verbosity level is mapped to slog levels such that V(0) becomes
-// slog.LevelInfo and V(4) becomes slog.LevelDebug.
+// Deprecated: use [logr.FromSlogHandler] instead.
 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})
+	return logr.FromSlogHandler(handler)
 }
 
-// NewSlogHandler returns a slog.Handler which writes to the same sink as the logr.Logger.
-//
-// The returned logger writes all records with level >= slog.LevelError as
-// error log entries with LogSink.Error, regardless of the verbosity level of
-// the logr.Logger:
-//
-//	logger := <some logr.Logger with 0 as verbosity level>
-//	slog.New(NewSlogHandler(logger.V(10))).Error(...) -> logSink.Error(...)
+// ToSlogHandler returns a slog.Handler which writes to the same sink as the logr.Logger.
 //
-// The level of all other records gets reduced by the verbosity
-// level of the logr.Logger and the result is negated. If it happens
-// to be negative, then it gets replaced by zero because a LogSink
-// is not expected to handled negative levels:
-//
-//	slog.New(NewSlogHandler(logger)).Debug(...) -> logger.GetSink().Info(level=4, ...)
-//	slog.New(NewSlogHandler(logger)).Warning(...) -> logger.GetSink().Info(level=0, ...)
-//	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 {
-	if sink, ok := logger.GetSink().(*slogSink); ok && logger.GetV() == 0 {
-		return sink.handler
-	}
-
-	handler := &slogHandler{sink: logger.GetSink(), levelBias: slog.Level(logger.GetV())}
-	if slogSink, ok := handler.sink.(SlogSink); ok {
-		handler.slogSink = slogSink
-	}
-	return handler
+// Deprecated: use [logr.ToSlogHandler] instead.
+func ToSlogHandler(logger logr.Logger) slog.Handler {
+	return logr.ToSlogHandler(logger)
 }
 
 // SlogSink is an optional interface that a LogSink can implement to support
-// logging through the slog.Logger or slog.Handler APIs better. It then should
-// also support special slog values like slog.Group. When used as a
-// slog.Handler, the advantages are:
+// logging through the slog.Logger or slog.Handler APIs better.
 //
-//   - stack unwinding gets avoided in favor of logging the pre-recorded PC,
-//     as intended by slog
-//   - proper grouping of key/value pairs via WithGroup
-//   - verbosity levels > slog.LevelInfo can be recorded
-//   - less overhead
-//
-// Both APIs (logr.Logger and slog.Logger/Handler) then are supported equally
-// well. Developers can pick whatever API suits them better and/or mix
-// packages which use either API in the same binary with a common logging
-// implementation.
-//
-// This interface is necessary because the type implementing the LogSink
-// interface cannot also implement the slog.Handler interface due to the
-// different prototype of the common Enabled method.
-//
-// An implementation could support both interfaces in two different types, but then
-// additional interfaces would be needed to convert between those types in NewLogr
-// and NewSlogHandler.
-type SlogSink interface {
-	logr.LogSink
-
-	Handle(ctx context.Context, record slog.Record) error
-	WithAttrs(attrs []slog.Attr) SlogSink
-	WithGroup(name string) SlogSink
-}
+// Deprecated: use [logr.SlogSink] instead.
+type SlogSink = logr.SlogSink
diff --git a/slogr/slogr_test.go b/slogr_test.go
similarity index 82%
rename from slogr/slogr_test.go
rename to slogr_test.go
index 2e5f0ff..fff6fb5 100644
--- a/slogr/slogr_test.go
+++ b/slogr_test.go
@@ -17,7 +17,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package slogr_test
+package logr_test
 
 import (
 	"bytes"
@@ -36,7 +36,6 @@ import (
 
 	"github.com/go-logr/logr"
 	"github.com/go-logr/logr/funcr"
-	"github.com/go-logr/logr/slogr"
 )
 
 var debugWithoutTime = &slog.HandlerOptions{
@@ -49,12 +48,12 @@ var debugWithoutTime = &slog.HandlerOptions{
 	Level: slog.LevelDebug,
 }
 
-func ExampleNew() {
-	logger := slogr.NewLogr(slog.NewTextHandler(os.Stdout, debugWithoutTime))
+func ExampleFromSlogHandler() {
+	logrLogger := logr.FromSlogHandler(slog.NewTextHandler(os.Stdout, debugWithoutTime))
 
-	logger.Info("hello world")
-	logger.Error(errors.New("fake error"), "ignore me")
-	logger.WithValues("x", 1, "y", 2).WithValues("str", "abc").WithName("foo").WithName("bar").V(4).Info("with values, verbosity and name")
+	logrLogger.Info("hello world")
+	logrLogger.Error(errors.New("fake error"), "ignore me")
+	logrLogger.WithValues("x", 1, "y", 2).WithValues("str", "abc").WithName("foo").WithName("bar").V(4).Info("with values, verbosity and name")
 
 	// Output:
 	// level=INFO msg="hello world"
@@ -62,7 +61,7 @@ func ExampleNew() {
 	// level=DEBUG msg="with values, verbosity and name" x=1 y=2 str=abc logger=foo/bar
 }
 
-func ExampleNewSlogLogger() {
+func ExampleToSlogHandler() {
 	funcrLogger := funcr.New(func(prefix, args string) {
 		if prefix != "" {
 			fmt.Fprintln(os.Stdout, prefix, args)
@@ -73,13 +72,13 @@ func ExampleNewSlogLogger() {
 		Verbosity: 10,
 	})
 
-	logger := slog.New(slogr.NewSlogHandler(funcrLogger))
-	logger.Info("hello world")
-	logger.Error("ignore me", "err", errors.New("fake error"))
-	logger.With("x", 1, "y", 2).WithGroup("group").With("str", "abc").Warn("with values and group")
+	slogLogger := slog.New(logr.ToSlogHandler(funcrLogger))
+	slogLogger.Info("hello world")
+	slogLogger.Error("ignore me", "err", errors.New("fake error"))
+	slogLogger.With("x", 1, "y", 2).WithGroup("group").With("str", "abc").Warn("with values and group")
 
-	logger = slog.New(slogr.NewSlogHandler(funcrLogger.V(int(-slog.LevelDebug))))
-	logger.Info("info message reduced to debug level")
+	slogLogger = slog.New(logr.ToSlogHandler(funcrLogger.V(int(-slog.LevelDebug))))
+	slogLogger.Info("info message reduced to debug level")
 
 	// Output:
 	// "level"=0 "msg"="hello world"
@@ -92,7 +91,7 @@ func TestWithCallDepth(t *testing.T) {
 	debugWithCaller := *debugWithoutTime
 	debugWithCaller.AddSource = true
 	var buffer bytes.Buffer
-	logger := slogr.NewLogr(slog.NewTextHandler(&buffer, &debugWithCaller))
+	logger := logr.FromSlogHandler(slog.NewTextHandler(&buffer, &debugWithCaller))
 
 	logHelper(logger)
 	_, file, line, _ := runtime.Caller(0)
@@ -116,7 +115,7 @@ func TestJSONHandler(t *testing.T) {
 }
 
 var _ logr.LogSink = testSlogSink{}
-var _ slogr.SlogSink = testSlogSink{}
+var _ logr.SlogSink = testSlogSink{}
 
 // testSlogSink is only used through slog and thus doesn't need to implement the
 // normal LogSink methods.
@@ -134,10 +133,10 @@ func (s testSlogSink) WithValues(...interface{}) logr.LogSink { return s }
 func (s testSlogSink) Handle(ctx context.Context, record slog.Record) error {
 	return s.handler.Handle(ctx, record)
 }
-func (s testSlogSink) WithAttrs(attrs []slog.Attr) slogr.SlogSink {
+func (s testSlogSink) WithAttrs(attrs []slog.Attr) logr.SlogSink {
 	return testSlogSink{handler: s.handler.WithAttrs(attrs)}
 }
-func (s testSlogSink) WithGroup(name string) slogr.SlogSink {
+func (s testSlogSink) WithGroup(name string) logr.SlogSink {
 	return testSlogSink{handler: s.handler.WithGroup(name)}
 }
 
@@ -178,7 +177,7 @@ func TestFuncrHandler(t *testing.T) {
 func testSlog(t *testing.T, createLogger func(buffer *bytes.Buffer) logr.Logger, exceptions ...string) {
 	var buffer bytes.Buffer
 	logger := createLogger(&buffer)
-	handler := slogr.NewSlogHandler(logger)
+	handler := logr.ToSlogHandler(logger)
 	err := slogtest.TestHandler(handler, func() []map[string]any {
 		var ms []map[string]any
 		for _, line := range bytes.Split(buffer.Bytes(), []byte{'\n'}) {
@@ -223,28 +222,28 @@ func containsOne(hay string, needles ...string) bool {
 }
 
 func TestDiscard(t *testing.T) {
-	logger := slog.New(slogr.NewSlogHandler(logr.Discard()))
+	logger := slog.New(logr.ToSlogHandler(logr.Discard()))
 	logger.WithGroup("foo").With("x", 1).Info("hello")
 }
 
 func TestConversion(t *testing.T) {
 	d := logr.Discard()
-	d2 := slogr.NewLogr(slogr.NewSlogHandler(d))
+	d2 := logr.FromSlogHandler(logr.ToSlogHandler(d))
 	expectEqual(t, d, d2)
 
 	e := logr.Logger{}
-	e2 := slogr.NewLogr(slogr.NewSlogHandler(e))
+	e2 := logr.FromSlogHandler(logr.ToSlogHandler(e))
 	expectEqual(t, e, e2)
 
 	f := funcr.New(func(prefix, args string) {}, funcr.Options{})
-	f2 := slogr.NewLogr(slogr.NewSlogHandler(f))
+	f2 := logr.FromSlogHandler(logr.ToSlogHandler(f))
 	expectEqual(t, f, f2)
 
 	text := slog.NewTextHandler(io.Discard, nil)
-	text2 := slogr.NewSlogHandler(slogr.NewLogr(text))
+	text2 := logr.ToSlogHandler(logr.FromSlogHandler(text))
 	expectEqual(t, text, text2)
 
-	text3 := slogr.NewSlogHandler(slogr.NewLogr(text).V(1))
+	text3 := logr.ToSlogHandler(logr.FromSlogHandler(text).V(1))
 	if handler, ok := text3.(interface {
 		GetLevel() slog.Level
 	}); ok {
diff --git a/slogr/slogsink.go b/slogsink.go
similarity index 85%
rename from slogr/slogsink.go
rename to slogsink.go
index 6fbac56..93fa4e5 100644
--- a/slogr/slogsink.go
+++ b/slogsink.go
@@ -17,24 +17,22 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package slogr
+package logr
 
 import (
 	"context"
 	"log/slog"
 	"runtime"
 	"time"
-
-	"github.com/go-logr/logr"
 )
 
 var (
-	_ logr.LogSink          = &slogSink{}
-	_ logr.CallDepthLogSink = &slogSink{}
-	_ Underlier             = &slogSink{}
+	_ LogSink          = &slogSink{}
+	_ CallDepthLogSink = &slogSink{}
+	_ Underlier        = &slogSink{}
 )
 
-// Underlier is implemented by the LogSink returned by NewLogr.
+// Underlier is implemented by the LogSink returned by NewFromLogHandler.
 type Underlier interface {
 	// GetUnderlying returns the Handler used by the LogSink.
 	GetUnderlying() slog.Handler
@@ -54,7 +52,7 @@ type slogSink struct {
 	handler   slog.Handler
 }
 
-func (l *slogSink) Init(info logr.RuntimeInfo) {
+func (l *slogSink) Init(info RuntimeInfo) {
 	l.callDepth = info.CallDepth
 }
 
@@ -62,7 +60,7 @@ func (l *slogSink) GetUnderlying() slog.Handler {
 	return l.handler
 }
 
-func (l *slogSink) WithCallDepth(depth int) logr.LogSink {
+func (l *slogSink) WithCallDepth(depth int) LogSink {
 	newLogger := *l
 	newLogger.callDepth += depth
 	return &newLogger
@@ -96,7 +94,7 @@ func (l *slogSink) log(err error, msg string, level slog.Level, kvList ...interf
 	l.handler.Handle(context.Background(), record)
 }
 
-func (l slogSink) WithName(name string) logr.LogSink {
+func (l slogSink) WithName(name string) LogSink {
 	if l.name != "" {
 		l.name = l.name + "/"
 	}
@@ -104,7 +102,7 @@ func (l slogSink) WithName(name string) logr.LogSink {
 	return &l
 }
 
-func (l slogSink) WithValues(kvList ...interface{}) logr.LogSink {
+func (l slogSink) WithValues(kvList ...interface{}) LogSink {
 	l.handler = l.handler.WithAttrs(kvListToAttrs(kvList...))
 	return &l
 }

From f1edf4f6aed3da9f4599674ea66e013fd37a5446 Mon Sep 17 00:00:00 2001
From: Patrick Ohly <patrick.ohly@intel.com>
Date: Sun, 26 Nov 2023 17:53:45 +0100
Subject: [PATCH 2/2] support a slog.Logger pointer in a context

Conversion is done on demand. A program which uses either slog or logr for
logging remains as efficient as before. What is less efficient is storing a
logger of one type and then retrieving it as the other.

Ideally, reusable packages should avoid using a slog.Logger and prefer
logr.Logger instead when retrieving a logger from a context. There is a
significant amount of packages which already work that way (Kubernetes and
related packages), while storing and retrieving a logr.Logger is new and not
done anywhere yet. Emulating existing behavior ensures better performance.
---
 README.md         | 49 ++++++++++++++++++----------
 context.go        | 33 +++++++++++++++++++
 context_noslog.go | 49 ++++++++++++++++++++++++++++
 context_slog.go   | 83 +++++++++++++++++++++++++++++++++++++++++++++++
 logr.go           | 43 ------------------------
 5 files changed, 196 insertions(+), 61 deletions(-)
 create mode 100644 context.go
 create mode 100644 context_noslog.go
 create mode 100644 context_slog.go

diff --git a/README.md b/README.md
index a9b1a4e..8969526 100644
--- a/README.md
+++ b/README.md
@@ -91,6 +91,7 @@ logr design but also left out some parts and changed others:
 | Adding a name to a logger | `WithName` | no API |
 | Modify verbosity of log entries in a call chain | `V` | no API |
 | Grouping of key/value pairs | not supported | `WithGroup`, `GroupValue` |
+| Pass context for extracting additional values | no API | API variants like `InfoCtx` |
 
 The high-level slog API is explicitly meant to be one of many different APIs
 that can be layered on top of a shared `slog.Handler`. logr is one such
@@ -208,24 +209,36 @@ with logr implementations without slog support is not important, then
 ### Context support for slog
 
 Storing a logger in a `context.Context` is not supported by
-slog. `logr.NewContext` and `logr.FromContext` can be used with slog like this
-to fill this gap:
-
-    func HandlerFromContext(ctx context.Context) slog.Handler {
-        logger, err := logr.FromContext(ctx)
-        if err == nil {
-            return ToSlogHandler(logger)
-        }
-        return slog.Default().Handler()
-    }
-
-    func ContextWithHandler(ctx context.Context, handler slog.Handler) context.Context {
-        return logr.NewContext(ctx, FromSlogHandler(handler))
-    }
-
-The downside is that storing and retrieving a `slog.Handler` needs more
-allocations compared to using a `logr.Logger`. Therefore the recommendation is
-to use the `logr.Logger` API in code which uses contextual logging.
+slog. `NewContextWithSlogLogger` and `FromContextAsSlogLogger` can be
+used to fill this gap. They store and retrieve a `slog.Logger` pointer
+under the same context key that is also used by `NewContext` and
+`FromContext` for `logr.Logger` value.
+
+When `NewContextWithSlogLogger` is followed by `FromContext`, the latter will
+automatically convert the `slog.Logger` to a
+`logr.Logger`. `FromContextAsSlogLogger` does the same for the other direction.
+
+With this approach, binaries which use either slog or logr are as efficient as
+possible with no unnecessary allocations. This is also why the API stores a
+`slog.Logger` pointer: when storing a `slog.Handler`, creating a `slog.Logger`
+on retrieval would need to allocate one.
+
+The downside is that switching back and forth needs more allocations. Because
+logr is the API that is already in use by different packages, in particular
+Kubernetes, the recommendation is to use the `logr.Logger` API in code which
+uses contextual logging.
+
+An alternative to adding values to a logger and storing that logger in the
+context is to store the values in the context and to configure a logging
+backend to extract those values when emitting log entries. This only works when
+log calls are passed the context, which is not supported by the logr API.
+
+With the slog API, it is possible, but not
+required. https://github.com/veqryn/slog-context is a package for slog which
+provides additional support code for this approach. It also contains wrappers
+for the context functions in logr, so developers who prefer to not use the logr
+APIs directly can use those instead and the resulting code will still be
+interoperable with logr.
 
 ## FAQ
 
diff --git a/context.go b/context.go
new file mode 100644
index 0000000..de8bcc3
--- /dev/null
+++ b/context.go
@@ -0,0 +1,33 @@
+/*
+Copyright 2023 The logr Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package logr
+
+// contextKey is how we find Loggers in a context.Context. With Go < 1.21,
+// the value is always a Logger value. With Go >= 1.21, the value can be a
+// Logger value or a slog.Logger pointer.
+type contextKey struct{}
+
+// notFoundError exists to carry an IsNotFound method.
+type notFoundError struct{}
+
+func (notFoundError) Error() string {
+	return "no logr.Logger was present"
+}
+
+func (notFoundError) IsNotFound() bool {
+	return true
+}
diff --git a/context_noslog.go b/context_noslog.go
new file mode 100644
index 0000000..f012f9a
--- /dev/null
+++ b/context_noslog.go
@@ -0,0 +1,49 @@
+//go:build !go1.21
+// +build !go1.21
+
+/*
+Copyright 2019 The logr Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package logr
+
+import (
+	"context"
+)
+
+// FromContext returns a Logger from ctx or an error if no Logger is found.
+func FromContext(ctx context.Context) (Logger, error) {
+	if v, ok := ctx.Value(contextKey{}).(Logger); ok {
+		return v, nil
+	}
+
+	return Logger{}, notFoundError{}
+}
+
+// FromContextOrDiscard returns a Logger from ctx.  If no Logger is found, this
+// returns a Logger that discards all log messages.
+func FromContextOrDiscard(ctx context.Context) Logger {
+	if v, ok := ctx.Value(contextKey{}).(Logger); ok {
+		return v
+	}
+
+	return Discard()
+}
+
+// NewContext returns a new Context, derived from ctx, which carries the
+// provided Logger.
+func NewContext(ctx context.Context, logger Logger) context.Context {
+	return context.WithValue(ctx, contextKey{}, logger)
+}
diff --git a/context_slog.go b/context_slog.go
new file mode 100644
index 0000000..065ef0b
--- /dev/null
+++ b/context_slog.go
@@ -0,0 +1,83 @@
+//go:build go1.21
+// +build go1.21
+
+/*
+Copyright 2019 The logr Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package logr
+
+import (
+	"context"
+	"fmt"
+	"log/slog"
+)
+
+// FromContext returns a Logger from ctx or an error if no Logger is found.
+func FromContext(ctx context.Context) (Logger, error) {
+	v := ctx.Value(contextKey{})
+	if v == nil {
+		return Logger{}, notFoundError{}
+	}
+
+	switch v := v.(type) {
+	case Logger:
+		return v, nil
+	case *slog.Logger:
+		return FromSlogHandler(v.Handler()), nil
+	default:
+		// Not reached.
+		panic(fmt.Sprintf("unexpected value type for logr context key: %T", v))
+	}
+}
+
+// FromContextAsSlogLogger returns a slog.Logger from ctx or nil if no such Logger is found.
+func FromContextAsSlogLogger(ctx context.Context) *slog.Logger {
+	v := ctx.Value(contextKey{})
+	if v == nil {
+		return nil
+	}
+
+	switch v := v.(type) {
+	case Logger:
+		return slog.New(ToSlogHandler(v))
+	case *slog.Logger:
+		return v
+	default:
+		// Not reached.
+		panic(fmt.Sprintf("unexpected value type for logr context key: %T", v))
+	}
+}
+
+// FromContextOrDiscard returns a Logger from ctx.  If no Logger is found, this
+// returns a Logger that discards all log messages.
+func FromContextOrDiscard(ctx context.Context) Logger {
+	if logger, err := FromContext(ctx); err == nil {
+		return logger
+	}
+	return Discard()
+}
+
+// NewContext returns a new Context, derived from ctx, which carries the
+// provided Logger.
+func NewContext(ctx context.Context, logger Logger) context.Context {
+	return context.WithValue(ctx, contextKey{}, logger)
+}
+
+// NewContextWithSlogLogger returns a new Context, derived from ctx, which carries the
+// provided slog.Logger.
+func NewContextWithSlogLogger(ctx context.Context, logger *slog.Logger) context.Context {
+	return context.WithValue(ctx, contextKey{}, logger)
+}
diff --git a/logr.go b/logr.go
index 2a5075a..b4428e1 100644
--- a/logr.go
+++ b/logr.go
@@ -207,10 +207,6 @@ limitations under the License.
 // those.
 package logr
 
-import (
-	"context"
-)
-
 // New returns a new Logger instance.  This is primarily used by libraries
 // implementing LogSink, rather than end users.  Passing a nil sink will create
 // a Logger which discards all log lines.
@@ -410,45 +406,6 @@ func (l Logger) IsZero() bool {
 	return l.sink == nil
 }
 
-// contextKey is how we find Loggers in a context.Context.
-type contextKey struct{}
-
-// FromContext returns a Logger from ctx or an error if no Logger is found.
-func FromContext(ctx context.Context) (Logger, error) {
-	if v, ok := ctx.Value(contextKey{}).(Logger); ok {
-		return v, nil
-	}
-
-	return Logger{}, notFoundError{}
-}
-
-// notFoundError exists to carry an IsNotFound method.
-type notFoundError struct{}
-
-func (notFoundError) Error() string {
-	return "no logr.Logger was present"
-}
-
-func (notFoundError) IsNotFound() bool {
-	return true
-}
-
-// FromContextOrDiscard returns a Logger from ctx.  If no Logger is found, this
-// returns a Logger that discards all log messages.
-func FromContextOrDiscard(ctx context.Context) Logger {
-	if v, ok := ctx.Value(contextKey{}).(Logger); ok {
-		return v
-	}
-
-	return Discard()
-}
-
-// NewContext returns a new Context, derived from ctx, which carries the
-// provided Logger.
-func NewContext(ctx context.Context, logger Logger) context.Context {
-	return context.WithValue(ctx, contextKey{}, logger)
-}
-
 // RuntimeInfo holds information that the logr "core" library knows which
 // LogSinks might want to know.
 type RuntimeInfo struct {