Repository: kubernetes/klog
Pull request #242 (2021-05-30 02:42:14)

Switching to logr tag v1.0.0
This is a **BREAKING** change. We should probably switch to klog to v3 as it changes the interface :(

Signed-off-by: Davanum Srinivas <davanum@gmail.com>

<!--  Thanks for sending a pull request!  Here are some tips for you:
1. If this is your first time, read our contributor guidelines https://git.k8s.io/community/contributors/guide/pull-requests.md#the-pull-request-submit-process and developer guide https://git.k8s.io/community/contributors/devel/development.md#development-guide
2. If you want *faster* PR reviews, read how: https://git.k8s.io/community/contributors/guide/pull-requests.md#best-practices-for-faster-reviews
3. Follow the instructions for writing a release note: https://git.k8s.io/community/contributors/guide/release-notes.md
4. If the PR is unfinished, see how to mark it: https://git.k8s.io/community/contributors/guide/pull-requests.md#marking-unfinished-pull-requests
5. If this PR changes image versions, please title this PR "Bump <image name> from x.x.x to y.y.y."
-->

**What this PR does / why we need it**:

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes #

**Special notes for your reviewer**:

_Please confirm that if this PR changes any image versions, then that's the sole change this PR makes._

**Release note**:
<!--  Write your release note:
1. Enter your extended release note in the below block. If the PR requires additional action from users switching to the new release, include the string "action required".
2. If no release note is required, just write "NONE".
-->
```release-note

```

k8s-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes/klog/pull/242#" title="Author self-approved">dims</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes%2Fklog).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/kubernetes/klog/blob/master/OWNERS)~~ [dims]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
serathius: /cc
k8s-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes/klog/pull/242#" title="Author self-approved">dims</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes%2Fklog).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/kubernetes/klog/blob/main/OWNERS)~~ [dims]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
dims: @thockin we still have other API breaks (after fixing up the `SetLogger`)

```
FAIL: k8s.io/klog/v2/klogr contains incompatible changes:
- klogger.Enabled: changed from func() bool to func(int) bool
- klogger.Info: changed from func(string, ...interface{}) to func(int, string, ...interface{})
- klogger.V: removed
- klogger.WithCallDepth: changed from func(int) github.com/go-logr/logr.Logger to func(int) github.com/go-logr/logr.LogSink
- klogger.WithName: changed from func(string) github.com/go-logr/logr.Logger to func(string) github.com/go-logr/logr.LogSink
- klogger.WithValues: changed from func(...interface{}) github.com/go-logr/logr.Logger to func(...interface{}) github.com/go-logr/logr.LogSink
```
thockin: None of those are user-facing APIs, though, right?   You should be able to make this change, re-vendor into k/k and recompile, I think?
dims: @thockin true. 
dims: /hold for discussion especially with @liggitt and @thockin 
thockin: Look at it from an end-user point of view.  End users will never actually
hold a LogSink to pass in.  They will usually do something like:

```
logger := zapr.New(/* args... */)
klog.SetLogger(logger)
```

logr.Logger has no exported fields, so they can't reach in an extract the
LogSink.  Implementations like glogr, zapr, etc will not typically have a
direct LogSink constructor, just Logger.

If I understand correctly - the point of this capability is to let users
who use logr in their app send klog messages to the same place, right?  So
they are already going to have a logr.Logger.

On Wed, Jul 28, 2021 at 12:29 PM Marek Siarkowicz ***@***.***>
wrote:

> ***@***.**** commented on this pull request.
> ------------------------------
>
> In klog.go
> <https://github.com/kubernetes/klog/pull/242#discussion_r678593424>:
>
> > @@ -866,7 +866,14 @@ func SetLogger(logr logr.Logger) {
>  	logging.mu.Lock()
>
> Wouldn't it make more sense to accept logr.LogSink interface and construct
> the logr.Logger instead of accepting the struct itself.
>
> I ask as I have seen multiple cases of users being confused with
> logr.Logger now being struct. Even though this change is purely for
> performance, and doesn't have any impact, some users misread this change.
> For example #252 <https://github.com/kubernetes/klog/pull/252>
>
> My first though about this change is that logr just splits client API
> (logr.Logger) from implementation interface (logr.LogSInk). Why then klog
> and would accept the client log.Logger instead of implementation
> logr.LogSink.
>
> It might be a subtle difference in API, but accepting interface makes it
> more clearer that intention of using logr is the same.
>
> —
> You are receiving this because you were assigned.
> Reply to this email directly, view it on GitHub
> <https://github.com/kubernetes/klog/pull/242#pullrequestreview-717389414>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ABKWAVAL2OOOV7GUNXGTERTT2BLCVANCNFSM45Y3WIIQ>
> .
>

dims: @thockin we have a verify script that fails :) so the question is do we disable that or we bump klog from v2 to v3

```
FAIL: k8s.io/klog/v2/klogr contains incompatible changes:
- klogger.Enabled: changed from func() bool to func(int) bool
- klogger.Info: changed from func(string, ...interface{}) to func(int, string, ...interface{})
- klogger.V: removed
- klogger.WithCallDepth: changed from func(int) github.com/go-logr/logr.Logger to func(int) github.com/go-logr/logr.LogSink
- klogger.WithName: changed from func(string) github.com/go-logr/logr.Logger to func(string) github.com/go-logr/logr.LogSink
- klogger.WithValues: changed from func(...interface{}) github.com/go-logr/logr.Logger to func(...interface{}) github.com/go-logr/logr.LogSink
```
aojea: > v2 to v3. we need to agree on how to roll this out. that's the discussion needed.

v3 looks easier to explain, no? I hit this today updating some dependencies that broke because of the logr changes, but I started to see more people doing `go get -u ...` that are starting to have problems ...

thockin: I see.  Yeah, if we are versioning klog then v3 is "most correct".  That said, are users really having problems that are not solved by a recompile?
aojea: > I see. Yeah, if we are versioning klog then v3 is "most correct". That said, are users really having problems that are not solved by a recompile?

yeah, this was mine today and I fixed using a replace statement, but you can similar ones on forums https://forum.golangbridge.org/t/go-lgr-compiler-error-despite-source-code-not-having-changed/23725
```
go get -u github.com/openshift/api                                  
# k8s.io/klog/v2
../../../../pkg/mod/k8s.io/klog/v2@v2.10.0/klog.go:705:10: invalid operation: logr != nil (mismatched types logr.Logger and nil)
../../../../pkg/mod/k8s.io/klog/v2@v2.10.0/klog.go:724:10: invalid operation: logr != nil (mismatched types logr.Logger and nil)
../../../../pkg/mod/k8s.io/klog/v2@v2.10.0/klog.go:742:10: invalid operation: logr != nil (mismatched types logr.Logger and nil)
../../../../pkg/mod/k8s.io/klog/v2@v2.10.0/klog.go:763:10: invalid operation: logr != nil (mismatched types logr.Logger and nil)
../../../../pkg/mod/k8s.io/klog/v2@v2.10.0/klog.go:782:11: invalid operation: loggr != nil (mismatched types logr.Logger and nil)
../../../../pkg/mod/k8s.io/klog/v2@v2.10.0/klog.go:783:3: undefined: logr.WithCallDepth
```


thockin: Yeah, the nil-ability of an interface is the one thing that has popped
up repeatedly and I don't have a great answer to it.

On Wed, Jul 28, 2021 at 3:54 PM Antonio Ojea ***@***.***> wrote:
>
> I see. Yeah, if we are versioning klog then v3 is "most correct". That said, are users really having problems that are not solved by a recompile?
>
> yeah, this was mine today and I fixed using a replace statement, but you can similar ones on forums https://forum.golangbridge.org/t/go-lgr-compiler-error-despite-source-code-not-having-changed/23725
>
> go get -u github.com/openshift/api
> # k8s.io/klog/v2
> ..***@***.***/klog.go:705:10: invalid operation: logr != nil (mismatched types logr.Logger and nil)
> ..***@***.***/klog.go:724:10: invalid operation: logr != nil (mismatched types logr.Logger and nil)
> ..***@***.***/klog.go:742:10: invalid operation: logr != nil (mismatched types logr.Logger and nil)
> ..***@***.***/klog.go:763:10: invalid operation: logr != nil (mismatched types logr.Logger and nil)
> ..***@***.***/klog.go:782:11: invalid operation: loggr != nil (mismatched types logr.Logger and nil)
> ..***@***.***/klog.go:783:3: undefined: logr.WithCallDepth
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or unsubscribe.

pohly: > > v2 to v3. we need to agree on how to roll this out. that's the discussion needed.
> 
> v3 looks easier to explain, no? I hit this today updating some dependencies that broke because of the logr changes, but I started to see more people doing `go get -u ...` that are starting to have problems ...

Doing a klog v3 will not avoid those problems, will it? `go get -u ...` will update logr to v1 but not klog from v2 to v3.

While I agree that klog v3 would be cleaner, I still think that sticking with v2 will be less pain overall. After a new minor release of klog, `go get -u` will produce a functional combination.

The rationale for that not being an API break in klog would be to retro-actively declare any klog API which depends on the unstable logr v0.4 API as "alpha".

thockin: we can pin klog v2 at 0.4.0 if we need, but that infects the whole
ecosystem, so moving to 1.0 is better.  AIUI converting the whole ecosystem
to v3 is equally challenging, of course

On Mon, Aug 2, 2021 at 1:43 AM Patrick Ohly ***@***.***>
wrote:

> v2 to v3. we need to agree on how to roll this out. that's the discussion
> needed.
>
> v3 looks easier to explain, no? I hit this today updating some
> dependencies that broke because of the logr changes, but I started to see
> more people doing go get -u ... that are starting to have problems ...
>
> Doing a klog v3 will not avoid those problems, will it? go get -u ...
> will update logr to v1 but not klog from v2 to v3.
>
> While I agree that klog v3 would be cleaner, I still think that sticking
> with v2 will be less pain overall. After a new minor release of klog, go
> get -u will produce a functional combination.
>
> The rationale for that not being an API break in klog would be to
> retro-actively declare any klog API which depends on the unstable logr v0.4
> API as "alpha".
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/kubernetes/klog/pull/242#issuecomment-890844321>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ABKWAVCMCPP2JBTXUVBNNBDT2ZLDTANCNFSM45Y3WIIQ>
> .
>

pohly: > we can pin klog v2 at 0.4.0 if we need, but that infects the whole
> ecosystem, so moving to 1.0 is better. 

Agreed. Even if we never update klog v2 itself to a logr > 0.4.0, there's no way to express in go.mod that updating to 1.0 breaks klog, so all downstream users will have to do that pinning themselves.

> AIUI converting the whole ecosystem to v3 is equally challenging

Equal to pinning v2 to logr 0.4.0 or equal to updating v2 to logr 1.0.0?

I think a migration to v3 is going to be a major pain and I am not even sure whether it can be done seamlessly once logr backends are involved. Compared to that, updating v2 to logr 1.0.0 seems easier to me.

liggitt: > so all downstream users will have to do that pinning themselves

and/or avoid running `go get -u` with incompatible `v0....` dependencies in their graph... that doesn't work well generally
dims: Longer discussion here: https://kubernetes.slack.com/archives/CHGFYJVAN/p1627926227001800
dims: Essentially,
- Anyone using `SetLogger` will have to cross check what they are using and will have to move to newer struct than the earlier interface
- A recompile should get folks back on their feet
- When we update k/k to this version of klog we may want to move to zapr (https://github.com/kubernetes/kubernetes/issues/104097)
- Hopefully there will be some guidance in a markdown in logr repo to help with transition ( cc @thockin )
- We should highlight this in kubernetes release notes for 1.23 (next release)

Patch:
diff --git a/go.mod b/go.mod
index eb297b6..08a2d0f 100644
--- a/go.mod
+++ b/go.mod
@@ -2,4 +2,4 @@ module k8s.io/klog/v2
 
 go 1.13
 
-require github.com/go-logr/logr v0.4.0
+require github.com/go-logr/logr v1.0.0
diff --git a/go.sum b/go.sum
index 5778f81..a1b90e4 100644
--- a/go.sum
+++ b/go.sum
@@ -1,2 +1,4 @@
-github.com/go-logr/logr v0.4.0 h1:K7/B1jt6fIBQVd4Owv2MqGQClcgf0R266+7C/QjRcLc=
-github.com/go-logr/logr v0.4.0/go.mod h1:z6/tIYblkpsD+a4lm/fGIIU9mZ+XfAiaFtq7xTgseGU=
+github.com/go-logr/logr v1.0.0-rc1 h1:+ul9F74rBkPajeP8m4o3o0tiglmzNFsPnuhYyBCQ0Sc=
+github.com/go-logr/logr v1.0.0-rc1/go.mod h1:z6/tIYblkpsD+a4lm/fGIIU9mZ+XfAiaFtq7xTgseGU=
+github.com/go-logr/logr v1.0.0 h1:kH951GinvFVaQgy/ki/B3YYmQtRpExGigSJg6O8z5jo=
+github.com/go-logr/logr v1.0.0/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A=
diff --git a/klog.go b/klog.go
index d24087d..d6f52bd 100644
--- a/klog.go
+++ b/klog.go
@@ -509,7 +509,7 @@ type loggingT struct {
 	addDirHeader bool
 
 	// If set, all output will be redirected unconditionally to the provided logr.Logger
-	logr logr.Logger
+	logr *logr.Logger
 
 	// If true, messages will not be propagated to lower severity log levels
 	oneOutput bool
@@ -698,11 +698,11 @@ func (buf *buffer) someDigits(i, d int) int {
 	return copy(buf.tmp[i:], buf.tmp[j:])
 }
 
-func (l *loggingT) println(s severity, logr logr.Logger, filter LogFilter, args ...interface{}) {
+func (l *loggingT) println(s severity, logger *logr.Logger, filter LogFilter, args ...interface{}) {
 	buf, file, line := l.header(s, 0)
-	// if logr is set, we clear the generated header as we rely on the backing
-	// logr implementation to print headers
-	if logr != nil {
+	// if logger is set, we clear the generated header as we rely on the backing
+	// logger implementation to print headers
+	if logger != nil {
 		l.putBuffer(buf)
 		buf = l.getBuffer()
 	}
@@ -710,18 +710,18 @@ func (l *loggingT) println(s severity, logr logr.Logger, filter LogFilter, args
 		args = filter.Filter(args)
 	}
 	fmt.Fprintln(buf, args...)
-	l.output(s, logr, buf, 0 /* depth */, file, line, false)
+	l.output(s, logger, buf, 0 /* depth */, file, line, false)
 }
 
-func (l *loggingT) print(s severity, logr logr.Logger, filter LogFilter, args ...interface{}) {
-	l.printDepth(s, logr, filter, 1, args...)
+func (l *loggingT) print(s severity, logger *logr.Logger, filter LogFilter, args ...interface{}) {
+	l.printDepth(s, logger, filter, 1, args...)
 }
 
-func (l *loggingT) printDepth(s severity, logr logr.Logger, filter LogFilter, depth int, args ...interface{}) {
+func (l *loggingT) printDepth(s severity, logger *logr.Logger, filter LogFilter, depth int, args ...interface{}) {
 	buf, file, line := l.header(s, depth)
 	// if logr is set, we clear the generated header as we rely on the backing
 	// logr implementation to print headers
-	if logr != nil {
+	if logger != nil {
 		l.putBuffer(buf)
 		buf = l.getBuffer()
 	}
@@ -732,14 +732,14 @@ func (l *loggingT) printDepth(s severity, logr logr.Logger, filter LogFilter, de
 	if buf.Bytes()[buf.Len()-1] != '\n' {
 		buf.WriteByte('\n')
 	}
-	l.output(s, logr, buf, depth, file, line, false)
+	l.output(s, logger, buf, depth, file, line, false)
 }
 
-func (l *loggingT) printf(s severity, logr logr.Logger, filter LogFilter, format string, args ...interface{}) {
+func (l *loggingT) printf(s severity, logger *logr.Logger, filter LogFilter, format string, args ...interface{}) {
 	buf, file, line := l.header(s, 0)
 	// if logr is set, we clear the generated header as we rely on the backing
 	// logr implementation to print headers
-	if logr != nil {
+	if logger != nil {
 		l.putBuffer(buf)
 		buf = l.getBuffer()
 	}
@@ -750,17 +750,17 @@ func (l *loggingT) printf(s severity, logr logr.Logger, filter LogFilter, format
 	if buf.Bytes()[buf.Len()-1] != '\n' {
 		buf.WriteByte('\n')
 	}
-	l.output(s, logr, buf, 0 /* depth */, file, line, false)
+	l.output(s, logger, buf, 0 /* depth */, file, line, false)
 }
 
 // printWithFileLine behaves like print but uses the provided file and line number.  If
 // alsoLogToStderr is true, the log message always appears on standard error; it
 // will also appear in the log file unless --logtostderr is set.
-func (l *loggingT) printWithFileLine(s severity, logr logr.Logger, filter LogFilter, file string, line int, alsoToStderr bool, args ...interface{}) {
+func (l *loggingT) printWithFileLine(s severity, logger *logr.Logger, filter LogFilter, file string, line int, alsoToStderr bool, args ...interface{}) {
 	buf := l.formatHeader(s, file, line)
 	// if logr is set, we clear the generated header as we rely on the backing
 	// logr implementation to print headers
-	if logr != nil {
+	if logger != nil {
 		l.putBuffer(buf)
 		buf = l.getBuffer()
 	}
@@ -771,28 +771,28 @@ func (l *loggingT) printWithFileLine(s severity, logr logr.Logger, filter LogFil
 	if buf.Bytes()[buf.Len()-1] != '\n' {
 		buf.WriteByte('\n')
 	}
-	l.output(s, logr, buf, 2 /* depth */, file, line, alsoToStderr)
+	l.output(s, logger, buf, 2 /* depth */, file, line, alsoToStderr)
 }
 
 // if loggr is specified, will call loggr.Error, otherwise output with logging module.
-func (l *loggingT) errorS(err error, loggr logr.Logger, filter LogFilter, depth int, msg string, keysAndValues ...interface{}) {
+func (l *loggingT) errorS(err error, logger *logr.Logger, filter LogFilter, depth int, msg string, keysAndValues ...interface{}) {
 	if filter != nil {
 		msg, keysAndValues = filter.FilterS(msg, keysAndValues)
 	}
-	if loggr != nil {
-		logr.WithCallDepth(loggr, depth+2).Error(err, msg, keysAndValues...)
+	if logger != nil {
+		logger.WithCallDepth(depth+2).Error(err, msg, keysAndValues...)
 		return
 	}
 	l.printS(err, errorLog, depth+1, msg, keysAndValues...)
 }
 
 // if loggr is specified, will call loggr.Info, otherwise output with logging module.
-func (l *loggingT) infoS(loggr logr.Logger, filter LogFilter, depth int, msg string, keysAndValues ...interface{}) {
+func (l *loggingT) infoS(logger *logr.Logger, filter LogFilter, depth int, msg string, keysAndValues ...interface{}) {
 	if filter != nil {
 		msg, keysAndValues = filter.FilterS(msg, keysAndValues)
 	}
-	if loggr != nil {
-		logr.WithCallDepth(loggr, depth+2).Info(msg, keysAndValues...)
+	if logger != nil {
+		logger.WithCallDepth(depth+2).Info(msg, keysAndValues...)
 		return
 	}
 	l.printS(nil, infoLog, depth+1, msg, keysAndValues...)
@@ -866,7 +866,14 @@ func SetLogger(logr logr.Logger) {
 	logging.mu.Lock()
 	defer logging.mu.Unlock()
 
-	logging.logr = logr
+	logging.logr = &logr
+}
+
+func clearLogger() {
+	logging.mu.Lock()
+	defer logging.mu.Unlock()
+
+	logging.logr = nil
 }
 
 // SetOutput sets the output destination for all severities
@@ -904,7 +911,7 @@ func LogToStderr(stderr bool) {
 }
 
 // output writes the data to the log files and releases the buffer.
-func (l *loggingT) output(s severity, log logr.Logger, buf *buffer, depth int, file string, line int, alsoToStderr bool) {
+func (l *loggingT) output(s severity, log *logr.Logger, buf *buffer, depth int, file string, line int, alsoToStderr bool) {
 	l.mu.Lock()
 	if l.traceLocation.isSet() {
 		if l.traceLocation.match(file, line) {
@@ -916,9 +923,9 @@ func (l *loggingT) output(s severity, log logr.Logger, buf *buffer, depth int, f
 		// TODO: set 'severity' and caller information as structured log info
 		// keysAndValues := []interface{}{"severity", severityName[s], "file", file, "line", line}
 		if s == errorLog {
-			logr.WithCallDepth(l.logr, depth+3).Error(nil, string(data))
+			l.logr.WithCallDepth(depth+3).Error(nil, string(data))
 		} else {
-			logr.WithCallDepth(log, depth+3).Info(string(data))
+			log.WithCallDepth(depth + 3).Info(string(data))
 		}
 	} else if l.toStderr {
 		os.Stderr.Write(data)
@@ -1269,7 +1276,7 @@ func (l *loggingT) setV(pc uintptr) Level {
 // See the documentation of V for more information.
 type Verbose struct {
 	enabled bool
-	logr    logr.Logger
+	logr    *logr.Logger
 	filter  LogFilter
 }
 
@@ -1277,7 +1284,8 @@ func newVerbose(level Level, b bool) Verbose {
 	if logging.logr == nil {
 		return Verbose{b, nil, logging.filter}
 	}
-	return Verbose{b, logging.logr.V(int(level)), logging.filter}
+	v := logging.logr.V(int(level))
+	return Verbose{b, &v, logging.filter}
 }
 
 // V reports whether verbosity at the call site is at least the requested level.
diff --git a/klogr/klogr.go b/klogr/klogr.go
index 4cf80dd..a347a88 100644
--- a/klogr/klogr.go
+++ b/klogr/klogr.go
@@ -6,12 +6,10 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
-	"runtime"
-	"sort"
-	"strings"
-
 	"github.com/go-logr/logr"
 	"k8s.io/klog/v2"
+	"sort"
+	"strings"
 )
 
 // Option is a functional option that reconfigures the logger created with New.
@@ -57,7 +55,7 @@ func NewWithOptions(options ...Option) logr.Logger {
 	for _, option := range options {
 		option(&l)
 	}
-	return l
+	return logr.New(&l)
 }
 
 type klogger struct {
@@ -68,40 +66,8 @@ type klogger struct {
 	format    Format
 }
 
-func (l klogger) clone() klogger {
-	return klogger{
-		level:  l.level,
-		prefix: l.prefix,
-		values: copySlice(l.values),
-		format: l.format,
-	}
-}
-
-func copySlice(in []interface{}) []interface{} {
-	out := make([]interface{}, len(in))
-	copy(out, in)
-	return out
-}
-
-// Magic string for intermediate frames that we should ignore.
-const autogeneratedFrameName = "<autogenerated>"
-
-// Discover how many frames we need to climb to find the caller. This approach
-// was suggested by Ian Lance Taylor of the Go team, so it *should* be safe
-// enough (famous last words).
-//
-// It is needed because binding the specific klogger functions to the
-// logr interface creates one additional call frame that neither we nor
-// our caller know about.
-func framesToCaller() int {
-	// 1 is the immediate caller.  3 should be too many.
-	for i := 1; i < 3; i++ {
-		_, file, _, _ := runtime.Caller(i + 1) // +1 for this function's frame
-		if file != autogeneratedFrameName {
-			return i
-		}
-	}
-	return 1 // something went wrong, this is safe
+func (l *klogger) Init(info logr.RuntimeInfo) {
+	l.callDepth += info.CallDepth
 }
 
 // trimDuplicates will deduplicate elements provided in multiple KV tuple
@@ -191,27 +157,25 @@ func pretty(value interface{}) string {
 	return strings.TrimSpace(string(buffer.Bytes()))
 }
 
-func (l klogger) Info(msg string, kvList ...interface{}) {
-	if l.Enabled() {
-		switch l.format {
-		case FormatSerialize:
-			msgStr := flatten("msg", msg)
-			trimmed := trimDuplicates(l.values, kvList)
-			fixedStr := flatten(trimmed[0]...)
-			userStr := flatten(trimmed[1]...)
-			klog.InfoDepth(framesToCaller()+l.callDepth, l.prefix, " ", msgStr, " ", fixedStr, " ", userStr)
-		case FormatKlog:
-			trimmed := trimDuplicates(l.values, kvList)
-			if l.prefix != "" {
-				msg = l.prefix + ": " + msg
-			}
-			klog.InfoSDepth(framesToCaller()+l.callDepth, msg, append(trimmed[0], trimmed[1]...)...)
+func (l klogger) Info(level int, msg string, kvList ...interface{}) {
+	switch l.format {
+	case FormatSerialize:
+		msgStr := flatten("msg", msg)
+		trimmed := trimDuplicates(l.values, kvList)
+		fixedStr := flatten(trimmed[0]...)
+		userStr := flatten(trimmed[1]...)
+		klog.InfoDepth(l.callDepth+1, l.prefix, " ", msgStr, " ", fixedStr, " ", userStr)
+	case FormatKlog:
+		trimmed := trimDuplicates(l.values, kvList)
+		if l.prefix != "" {
+			msg = l.prefix + ": " + msg
 		}
+		klog.InfoSDepth(l.callDepth+1, msg, append(trimmed[0], trimmed[1]...)...)
 	}
 }
 
-func (l klogger) Enabled() bool {
-	return bool(klog.V(klog.Level(l.level)).Enabled())
+func (l klogger) Enabled(level int) bool {
+	return klog.V(klog.Level(level)).Enabled()
 }
 
 func (l klogger) Error(err error, msg string, kvList ...interface{}) {
@@ -226,45 +190,38 @@ func (l klogger) Error(err error, msg string, kvList ...interface{}) {
 		trimmed := trimDuplicates(l.values, kvList)
 		fixedStr := flatten(trimmed[0]...)
 		userStr := flatten(trimmed[1]...)
-		klog.ErrorDepth(framesToCaller()+l.callDepth, l.prefix, " ", msgStr, " ", errStr, " ", fixedStr, " ", userStr)
+		klog.ErrorDepth(l.callDepth+1, l.prefix, " ", msgStr, " ", errStr, " ", fixedStr, " ", userStr)
 	case FormatKlog:
 		trimmed := trimDuplicates(l.values, kvList)
 		if l.prefix != "" {
 			msg = l.prefix + ": " + msg
 		}
-		klog.ErrorSDepth(framesToCaller()+l.callDepth, err, msg, append(trimmed[0], trimmed[1]...)...)
+		klog.ErrorSDepth(l.callDepth+1, err, msg, append(trimmed[0], trimmed[1]...)...)
 	}
 }
 
-func (l klogger) V(level int) logr.Logger {
-	new := l.clone()
-	new.level = level
-	return new
-}
-
 // WithName returns a new logr.Logger with the specified name appended.  klogr
 // uses '/' characters to separate name elements.  Callers should not pass '/'
 // in the provided name string, but this library does not actually enforce that.
-func (l klogger) WithName(name string) logr.Logger {
-	new := l.clone()
+func (l klogger) WithName(name string) logr.LogSink {
 	if len(l.prefix) > 0 {
-		new.prefix = l.prefix + "/"
+		l.prefix = l.prefix + "/"
 	}
-	new.prefix += name
-	return new
+	l.prefix += name
+	return &l
 }
 
-func (l klogger) WithValues(kvList ...interface{}) logr.Logger {
-	new := l.clone()
-	new.values = append(new.values, kvList...)
-	return new
+func (l klogger) WithValues(kvList ...interface{}) logr.LogSink {
+	// Three slice args forces a copy.
+	n := len(l.values)
+	l.values = append(l.values[:n:n], kvList...)
+	return &l
 }
 
-func (l klogger) WithCallDepth(depth int) logr.Logger {
-	new := l.clone()
-	new.callDepth += depth
-	return new
+func (l klogger) WithCallDepth(depth int) logr.LogSink {
+	l.callDepth += depth
+	return &l
 }
 
-var _ logr.Logger = klogger{}
-var _ logr.CallDepthLogger = klogger{}
+var _ logr.LogSink = &klogger{}
+var _ logr.CallDepthLogSink = &klogger{}


Test patch:
diff --git a/klog_test.go b/klog_test.go
index 7f6a647..0df1420 100644
--- a/klog_test.go
+++ b/klog_test.go
@@ -446,7 +446,7 @@ func testVmoduleGlob(pat string, match bool, t *testing.T) {
 	defer logging.vmodule.Set("")
 	logging.vmodule.Set(pat)
 	if V(2).Enabled() != match {
-		t.Errorf("incorrect match for %q: got %t expected %t", pat, V(2), match)
+		t.Errorf("incorrect match for %q: got %#v expected %#v", pat, V(2), match)
 	}
 }
 
@@ -1375,8 +1375,9 @@ func TestInfoSWithLogr(t *testing.T) {
 
 	for _, data := range testDataInfo {
 		t.Run(data.msg, func(t *testing.T) {
-			SetLogger(logger)
-			defer SetLogger(nil)
+			l := logr.New(logger)
+			SetLogger(l)
+			defer clearLogger()
 			defer logger.reset()
 
 			InfoS(data.msg, data.keysValues...)
@@ -1442,8 +1443,9 @@ func TestErrorSWithLogr(t *testing.T) {
 
 	for _, data := range testDataInfo {
 		t.Run(data.msg, func(t *testing.T) {
-			SetLogger(logger)
-			defer SetLogger(nil)
+			l := logr.New(logger)
+			SetLogger(l)
+			defer clearLogger()
 			defer logger.reset()
 
 			ErrorS(data.err, data.msg, data.keysValues...)
@@ -1499,8 +1501,9 @@ func TestCallDepthLogr(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			SetLogger(logger)
-			defer SetLogger(nil)
+			l := logr.New(logger)
+			SetLogger(l)
+			defer clearLogger()
 			defer logger.reset()
 			defer logger.resetCallDepth()
 
@@ -1520,7 +1523,8 @@ func TestCallDepthLogr(t *testing.T) {
 func TestCallDepthLogrInfoS(t *testing.T) {
 	logger := &callDepthTestLogr{}
 	logger.resetCallDepth()
-	SetLogger(logger)
+	l := logr.New(logger)
+	SetLogger(l)
 
 	// Add wrapper to ensure callDepthTestLogr +2 offset is correct.
 	logFunc := func() {
@@ -1541,7 +1545,8 @@ func TestCallDepthLogrInfoS(t *testing.T) {
 func TestCallDepthLogrErrorS(t *testing.T) {
 	logger := &callDepthTestLogr{}
 	logger.resetCallDepth()
-	SetLogger(logger)
+	l := logr.New(logger)
+	SetLogger(l)
 
 	// Add wrapper to ensure callDepthTestLogr +2 offset is correct.
 	logFunc := func() {
@@ -1562,7 +1567,8 @@ func TestCallDepthLogrErrorS(t *testing.T) {
 func TestCallDepthLogrGoLog(t *testing.T) {
 	logger := &callDepthTestLogr{}
 	logger.resetCallDepth()
-	SetLogger(logger)
+	l := logr.New(logger)
+	SetLogger(l)
 	CopyStandardLogTo("INFO")
 
 	// Add wrapper to ensure callDepthTestLogr +2 offset is correct.
@@ -1588,7 +1594,7 @@ func TestCallDepthTestLogr(t *testing.T) {
 	logger.resetCallDepth()
 
 	logFunc := func() {
-		logger.Info("some info log")
+		logger.Info(0, "some info log")
 	}
 	// Keep these lines together.
 	_, wantFile, wantLine, _ := runtime.Caller(0)
@@ -1634,7 +1640,7 @@ func (l *testLogr) reset() {
 	l.entries = []testLogrEntry{}
 }
 
-func (l *testLogr) Info(msg string, keysAndValues ...interface{}) {
+func (l *testLogr) Info(level int, msg string, keysAndValues ...interface{}) {
 	l.mutex.Lock()
 	defer l.mutex.Unlock()
 	l.entries = append(l.entries, testLogrEntry{
@@ -1655,12 +1661,15 @@ func (l *testLogr) Error(err error, msg string, keysAndValues ...interface{}) {
 	})
 }
 
-func (l *testLogr) Enabled() bool               { panic("not implemented") }
-func (l *testLogr) V(int) logr.Logger           { panic("not implemented") }
-func (l *testLogr) WithName(string) logr.Logger { panic("not implemented") }
-func (l *testLogr) WithValues(...interface{}) logr.Logger {
-	panic("not implemented")
-}
+func (l *testLogr) Init(info logr.RuntimeInfo)             {}
+func (l *testLogr) Enabled(level int) bool                 { return true }
+func (l *testLogr) V(int) logr.Logger                      { panic("not implemented") }
+func (l *testLogr) WithName(string) logr.LogSink           { panic("not implemented") }
+func (l *testLogr) WithValues(...interface{}) logr.LogSink { panic("not implemented") }
+func (l *testLogr) WithCallDepth(depth int) logr.LogSink   { return l }
+
+var _ logr.LogSink = &testLogr{}
+var _ logr.CallDepthLogSink = &testLogr{}
 
 type callDepthTestLogr struct {
 	testLogr
@@ -1673,17 +1682,17 @@ func (l *callDepthTestLogr) resetCallDepth() {
 	l.callDepth = 0
 }
 
-func (l *callDepthTestLogr) WithCallDepth(depth int) logr.Logger {
+func (l *callDepthTestLogr) WithCallDepth(depth int) logr.LogSink {
 	l.mutex.Lock()
 	defer l.mutex.Unlock()
 	// Note: Usually WithCallDepth would be implemented by cloning l
 	// and setting the call depth on the clone. We modify l instead in
 	// this test helper for simplicity.
-	l.callDepth = depth
+	l.callDepth = depth + 1
 	return l
 }
 
-func (l *callDepthTestLogr) Info(msg string, keysAndValues ...interface{}) {
+func (l *callDepthTestLogr) Info(level int, msg string, keysAndValues ...interface{}) {
 	l.mutex.Lock()
 	defer l.mutex.Unlock()
 	// Add 2 to depth for the wrapper function caller and for invocation in
@@ -1710,6 +1719,9 @@ func (l *callDepthTestLogr) Error(err error, msg string, keysAndValues ...interf
 	})
 }
 
+var _ logr.LogSink = &callDepthTestLogr{}
+var _ logr.CallDepthLogSink = &callDepthTestLogr{}
+
 func checkLogrEntryCorrectCaller(t *testing.T, wantFile string, wantLine int, entry testLogrEntry) {
 	t.Helper()
 
diff --git a/klogr/calldepth-test/call_depth_helper_test.go b/klogr/calldepth-test/call_depth_helper_test.go
index be605fb..03326bf 100644
--- a/klogr/calldepth-test/call_depth_helper_test.go
+++ b/klogr/calldepth-test/call_depth_helper_test.go
@@ -8,9 +8,9 @@ import (
 // their source code file is *not* logged because of WithCallDepth(1).
 
 func myInfo(l logr.Logger, msg string) {
-	logr.WithCallDepth(l, 1).Info(msg)
+	l.WithCallDepth(2).Info(msg)
 }
 
 func myInfo2(l logr.Logger, msg string) {
-	myInfo(logr.WithCallDepth(l, 1), msg)
+	myInfo(l.WithCallDepth(2), msg)
 }
diff --git a/klogr/klogr_test.go b/klogr/klogr_test.go
index cb86cc3..7775b43 100644
--- a/klogr/klogr_test.go
+++ b/klogr/klogr_test.go
@@ -110,6 +110,7 @@ func testOutput(t *testing.T, format string) {
 `,
 		},
 		"should correctly handle odd-numbers of KVs": {
+			klogr:         new(),
 			text:          "test",
 			keysAndValues: []interface{}{"akey", "avalue", "akey2"},
 			expectedOutput: ` "msg"="test"  "akey"="avalue" "akey2"=null
@@ -118,6 +119,7 @@ func testOutput(t *testing.T, format string) {
 `,
 		},
 		"should correctly html characters": {
+			klogr:         new(),
 			text:          "test",
 			keysAndValues: []interface{}{"akey", "<&>"},
 			expectedOutput: ` "msg"="test"  "akey"="<&>"
@@ -169,19 +171,15 @@ func testOutput(t *testing.T, format string) {
 	}
 	for n, test := range tests {
 		t.Run(n, func(t *testing.T) {
-			klogr := test.klogr
-			if klogr == nil {
-				klogr = new()
-			}
 
 			// hijack the klog output
 			tmpWriteBuffer := bytes.NewBuffer(nil)
 			klog.SetOutput(tmpWriteBuffer)
 
 			if test.err != nil {
-				klogr.Error(test.err, test.text, test.keysAndValues...)
+				test.klogr.Error(test.err, test.text, test.keysAndValues...)
 			} else {
-				klogr.Info(test.text, test.keysAndValues...)
+				test.klogr.Info(test.text, test.keysAndValues...)
 			}
 
 			// call Flush to ensure the text isn't still buffered