Repository: kubernetes/klog
Pull request #20 (2018-11-15 00:26:32)

Add SetLogger function to intercept klog output with logr
This is an example of how we can modify klog and the work required in order to allow intercepting logs, including preserving the `V` level and severity.

In order to do this, I've had to change the type signature of `Verbose` from `type Verbose bool` to:

```
type Verbose struct {
	enabled bool
	logr    logr.InfoLogger
}
```

Consequently, it requires a corresponding change in k/k in the instances where `if klog.V(x) {` (replacing them with `if klog.V(x).Enabled()`).

The total changeset in k/k to get `bazel build //...` to pass:

```
 pkg/cloudprovider/providers/gce/cloud/gen.go                | 160 +++++++++++++++++++++++++++++++++++++++++++++++++--------------------------------------------------
 pkg/cloudprovider/providers/vsphere/vclib/connection.go     |   2 +-
 pkg/controller/nodelifecycle/node_lifecycle_controller.go   |   4 +--
 pkg/controller/volume/attachdetach/reconciler/reconciler.go |   6 ++--
 pkg/kubectl/cmd/util/helpers.go                             |   2 +-
 pkg/kubelet/eviction/helpers.go                             |   4 +--
 pkg/proxy/endpoints.go                                      |   2 +-
 pkg/scheduler/algorithm/predicates/predicates.go            |   8 ++---
 pkg/scheduler/algorithm/priorities/interpod_affinity.go     |   2 +-
 pkg/scheduler/algorithm/priorities/resource_allocation.go   |   2 +-
 pkg/scheduler/algorithm/priorities/resource_limits.go       |   2 +-
 pkg/scheduler/algorithm/priorities/selector_spreading.go    |   2 +-
 pkg/scheduler/core/generic_scheduler.go                     |   6 ++--
 plugin/pkg/auth/authorizer/rbac/rbac.go                     |   2 +-
 staging/src/k8s.io/apiserver/pkg/server/httplog/httplog.go  |   4 +--
 staging/src/k8s.io/apiserver/pkg/util/trace/trace.go        |   4 +--
 staging/src/k8s.io/client-go/rest/request.go                |   8 ++---
 staging/src/k8s.io/client-go/transport/round_trippers.go    |   8 ++---
 18 files changed, 114 insertions(+), 114 deletions(-)
```

We can still preserve the old behaviour in the `k8s.io/klog/glog` package, for those that are using the dep `replace` method to implement glog.

ref #19

munnerz: /cc @dims
dims: cc @thockin @tallclair 
neolit123: @munnerz is this change still needed after the merge of:
https://github.com/kubernetes/klog/blob/master/klogr

munnerz: @neolit123 the klogr package allows you to use `klog` with `logr` like interface - this patch would, allow you to intercept *klog* output and pipe it back via a `logr` interface, in order to make it possible to 'hijack' logs from code that currently uses `klog` and pipe it via your own logr interface.

My use case for this, is so that I can grab all output from the k8s apimachinery and pipe it via my own logr interface (so that it can be post-processed accordingly)
munnerz: Is there still an interest in something like this in klog? If so, I'll pick this up again. I can address to above comments if it's likely we want to go down this route 😄
dims: @munnerz yes please

/assign @thockin @pohly 
k8s-ci-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes/klog/pull/20#" title="Author self-approved">munnerz</a>*
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: **thockin**

If they are not already assigned, you can assign the PR to them by writing `/assign @thockin` in a comment when ready.

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 open>
Needs approval from an approver in each of these files:

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

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":["thockin"]} -->
munnerz: I've added a few additional changes on top of this PR, notably:

* if `klog.Errorf` is called, the actual log message will be propagated to the logr implementations `Error` method. This is implemented in a sort-of wonky way. I've added a TODO, but if anyone has any thoughts and thinks we should implement this here, I can add it before we merge 😄 

* No longer include log line headers if output is being redirected to logr

* Including `file` and `line` attributes as logr values

Hopefully that addresses the above comments 😄 
munnerz: > So I can use klog as the backend for a logr or a logr as the backend for klog? Can I make a loop?

Yes, you can create a loop 🙈 I don't really know if there's a good way to handle this case. If you are hijacking klog output and piping via logr, then you need to make sure that logr instance isn't logging back to klog (else 🔥)

> Also, logr already suggests `"caller"` s a special key for the calling information (file/line) of a particular log line.

Didn't realise this - I'll dig into the logr repo some more to find docs on the format of 'caller' 😄 

> Can you show what the output here would look like?

I'll put a demo case together shortly 😄 
dims: /assign @DirectXMan12 
thockin: On Wed, Feb 27, 2019 at 3:44 AM James Munnelly <notifications@github.com>
wrote:

> So I can use klog as the backend for a logr or a logr as the backend for
> klog? Can I make a loop?
>
> Yes, you can create a loop 🙈 I don't really know if there's a good way
> to handle this case. If you are hijacking klog output and piping via logr,
> then you need to make sure that logr instance isn't logging back to klog
> (else 🔥)
>
> Also, logr already suggests "caller" s a special key for the calling
> information (file/line) of a particular log line.
>
> Didn't realise this - I'll dig into the logr repo some more to find docs
> on the format of 'caller' 😄
>

It's just suggested as a key that users should not use because
implementations might.  I don't know if that is sufficient, or if we should
get more prescriptive.



> Can you show what the output here would look like?
>
> I'll put a demo case together shortly 😄
>
> —
> You are receiving this because you were assigned.
> Reply to this email directly, view it on GitHub
> <https://github.com/kubernetes/klog/pull/20#issuecomment-467831478>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/AFVgVNsFmx_6iIBlHo64dElE9MUMzOmLks5vRm-4gaJpZM4Ye2GK>
> .
>

thockin: If we want to fix all the users of V() as a bool, I am OK with that.  logr
is the same, obviously.

On Wed, Feb 27, 2019 at 3:48 AM James Munnelly <notifications@github.com>
wrote:

> *@munnerz* commented on this pull request.
> ------------------------------
>
> In klog.go
> <https://github.com/kubernetes/klog/pull/20#discussion_r260713599>:
>
> >  // and Infof. These methods will write to the Info log if called.
>
>  // Thus, one may write either
>
> -//	if glog.V(2) { glog.Info("log this") }
>
> +//	if glog.V(2).Enabled() { glog.Info("log this") }
>
>
> Yep, there is.
>
> There's not really any way we can do this cleanly without this, as a bool
> does not contain enough information to determine which V level should be
> logged at when Info is called on that V.
>
> I mentioned it in the initial PR body, but the actual changeset needed to
> make kubernetes/kubernetes compile is:
>
>  pkg/cloudprovider/providers/gce/cloud/gen.go                | 160 +++++++++++++++++++++++++++++++++++++++++++++++++--------------------------------------------------
>
>  pkg/cloudprovider/providers/vsphere/vclib/connection.go     |   2 +-
>
>  pkg/controller/nodelifecycle/node_lifecycle_controller.go   |   4 +--
>
>  pkg/controller/volume/attachdetach/reconciler/reconciler.go |   6 ++--
>
>  pkg/kubectl/cmd/util/helpers.go                             |   2 +-
>
>  pkg/kubelet/eviction/helpers.go                             |   4 +--
>
>  pkg/proxy/endpoints.go                                      |   2 +-
>
>  pkg/scheduler/algorithm/predicates/predicates.go            |   8 ++---
>
>  pkg/scheduler/algorithm/priorities/interpod_affinity.go     |   2 +-
>
>  pkg/scheduler/algorithm/priorities/resource_allocation.go   |   2 +-
>
>  pkg/scheduler/algorithm/priorities/resource_limits.go       |   2 +-
>
>  pkg/scheduler/algorithm/priorities/selector_spreading.go    |   2 +-
>
>  pkg/scheduler/core/generic_scheduler.go                     |   6 ++--
>
>  plugin/pkg/auth/authorizer/rbac/rbac.go                     |   2 +-
>
>  staging/src/k8s.io/apiserver/pkg/server/httplog/httplog.go  |   4 +--
>
>  staging/src/k8s.io/apiserver/pkg/util/trace/trace.go        |   4 +--
>
>  staging/src/k8s.io/client-go/rest/request.go                |   8 ++---
>
>  staging/src/k8s.io/client-go/transport/round_trippers.go    |   8 ++---
>
>  18 files changed, 114 insertions(+), 114 deletions(-)
>
>
> (as of a couple of months ago).
>
> I would be keen to hear any suggestions on how we can avoid this breaking
> change, but I fear we don't have many options if we want to preserve V
> levels 🙈
>
> —
> You are receiving this because you were assigned.
> Reply to this email directly, view it on GitHub
> <https://github.com/kubernetes/klog/pull/20#discussion_r260713599>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/AFVgVGREJvA0uM-zVF9QSr9PcdGRQ5bXks5vRnCGgaJpZM4Ye2GK>
> .
>

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

This pull-request has been approved by: *<a href="https://github.com/kubernetes/klog/pull/20#" title="Author self-approved">munnerz</a>*
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: **directxman12**

If they are not already assigned, you can assign the PR to them by writing `/assign @directxman12` in a comment when ready.

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 open>
Needs approval from an approver in each of these files:

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

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":["directxman12"]} -->
munnerz: I've updated the PR with the above changes, which make the code in `output` a lot less ugly (no more typecasting 😅)

> If we want to fix all the users of V() as a bool, I am OK with that. logr is the same, obviously.

Would it be helpful for me to open a pull request against kubernetes/kubernetes with this change included, so we can see how it works/the work involved in practice?

> Also, logr already suggests "caller" s a special key for the calling information (file/line) of a particular log line.

I'm a bit unsure what we're best to do here - would/should we assume the underlying logr implementation is going to do this already? Should we set it ourselves here?

For context for anyone reading along, the text in logr about this:

```
// Key Naming Conventions
//
// While users are generally free to use key names of their choice, it's generally best to avoid
// using the following keys, as they're frequently used by implementations:
//
// - `"error"`: the underlying error value in the `Error` method.
// - `"stacktrace"`: the stack trace associated with a particular log line or error
//                   (often from the `Error` message).
// - `"caller"`: the calling information (file/line) of a particular log line.
// - `"msg"`: the log message.
// - `"level"`: the log level.
// - `"ts"`: the timestamp for a log line.
//
// Implementations are encouraged to make use of these keys to represent the above
// concepts, when neccessary (for example, in a pure-JSON output form, it would be
// necessary to represent at least message and timestamp as ordinary named values).
```
thockin: >> Also, logr already suggests "caller" s a special key for the calling information (file/line) of a particular log line.
>
> I'm a bit unsure what we're best to do here - would/should we assume the underlying logr implementation is going to do this already? Should we set it ourselves here?

Ahh, right.  If you set `caller` here then a logr implementation might step on it.  If you don't set it, the logr implementation will almost certainly get it wrong, and all of your log lines will come from klog.go.

Do we need to make logr spec this better?  Maybe we define a handful of keys that the implementation is strongly advised to use, and if they are already set to respect them?

@DirectXMan12 
DirectXMan12: Hmm... requiring implementations to respect those passed-in fields makes some amount of sense, but does make implementations more complicated, potentially.

Brief testing with Zap seems to show that it simply passes all passed fields to the underlying JSON without deduping (which makes sense from a performance perspective), so without any modification, it'd depend on how your JSON parsing library handles duplicates.  Making Zap respect a passed-in caller should be possible, but not straightforward (it'd require either changing the zap encoder's config on the fly, or always reproducing zap's lower-level logic).

How often do we think this case is going to come up (feeding an existing logging implementation to another logr implementation, as opposed to the other way around)?  If you *know* this is going to happen, there are generally work-arounds you can do in the underlying logging libraries to make it sane to just allow overwriting the caller, etc and still getting the right info (e.g. `CallerSkip`).
munnerz: I'm not 100% certain on the actionable options from the above discussion - would it be possible to omit these fields altogether for now, in favour of implementing them later on once we've had a chance to consider the design a bit more? I feel like we may not fully understand how users want to use this, and it may well involve changes to logr itself.

Getting this PR merged will (almost immediately - pending a PR to kubernetes/kubernetes) allow users to begin funnelling the output of Kubernetes apimachinery etc. through their own logging implementation, which is currently only possible using `klog.SetOutput(io.Writer)`.

munnerz: > Do we need to make logr spec this better? Maybe we define a handful of keys that the implementation is strongly advised to use, and if they are already set to respect them?

This sounds sensible to me. More generally, I think some information about how different implementations might handle duplicate keys would be useful, although I wouldn't want to be too prescriptive here as it may well be useful to record duplicate keys in some implementations, and not in others.

DirectXMan12: > would it be possible to omit these fields altogether for now, in favour of implementing them later on once we've had a chance to consider the design a bit more?

Yeah, I think that's fine.
DirectXMan12: @munnerz in that light, is this ready for final review?
munnerz: Barring the fact I've not removed setting these fields from this PR yet,
yep. I can remove that line tomorrow GMT 🙂

On Fri, 15 Mar 2019 at 20:09, Solly Ross <notifications@github.com> wrote:

> @munnerz <https://github.com/munnerz> in that light, is this ready for
> final review?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/kubernetes/klog/pull/20#issuecomment-473424986>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/AAMbP8EUfwyPMgNStzBxFTfYbNp86_82ks5vW_3pgaJpZM4Ye2GK>
> .
>

dims: Dude! Do it monday :)

On Fri, Mar 15, 2019 at 4:16 PM James Munnelly <notifications@github.com>
wrote:

> Barring the fact I've not removed setting these fields from this PR yet,
> yep. I can remove that line tomorrow GMT 🙂
>
> On Fri, 15 Mar 2019 at 20:09, Solly Ross <notifications@github.com> wrote:
>
> > @munnerz <https://github.com/munnerz> in that light, is this ready for
> > final review?
> >
> > —
> > You are receiving this because you were mentioned.
> > Reply to this email directly, view it on GitHub
> > <https://github.com/kubernetes/klog/pull/20#issuecomment-473424986>, or
> mute
> > the thread
> > <
> https://github.com/notifications/unsubscribe-auth/AAMbP8EUfwyPMgNStzBxFTfYbNp86_82ks5vW_3pgaJpZM4Ye2GK
> >
> > .
> >
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/kubernetes/klog/pull/20#issuecomment-473426902>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/AABbCPNw2DD7ym4mb8hl1p3B2LFj4-iSks5vW_-EgaJpZM4Ye2GK>
> .
>


-- 
Davanum Srinivas :: https://twitter.com/dims

DirectXMan12: agree with @dims -- it can wait unless it's really bugging you.
DirectXMan12: just wanted to make sure you weren't waiting on me.
munnerz: I've updated the PR to remove the extra structured log arguments and rebase it 😄 
dims: /approve
/assign @thockin @DirectXMan12 


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

This pull-request has been approved by: *<a href="https://github.com/kubernetes/klog/pull/20#issuecomment-482618033" title="Approved">dims</a>*, *<a href="https://github.com/kubernetes/klog/pull/20#" title="Author self-approved">munnerz</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":[]} -->
dims: /uncc

Patch:
diff --git a/klog.go b/klog.go
index 17d2975..53abca9 100644
--- a/klog.go
+++ b/klog.go
@@ -87,6 +87,8 @@ import (
 	"sync"
 	"sync/atomic"
 	"time"
+
+	"github.com/go-logr/logr"
 )
 
 // severity identifies the sort of log: info, warning etc. It also implements
@@ -505,6 +507,9 @@ type loggingT struct {
 
 	// If true, add the file directory to the header
 	addDirHeader bool
+
+	// If set, all output will be redirected unconditionally to the provided logr.Logger
+	logr logr.Logger
 }
 
 // buffer holds a byte Buffer for reuse. The zero value is ready for use.
@@ -687,44 +692,68 @@ func (buf *buffer) someDigits(i, d int) int {
 	return copy(buf.tmp[i:], buf.tmp[j:])
 }
 
-func (l *loggingT) println(s severity, args ...interface{}) {
+func (l *loggingT) println(s severity, logr logr.InfoLogger, 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 {
+		l.putBuffer(buf)
+		buf = l.getBuffer()
+	}
 	fmt.Fprintln(buf, args...)
-	l.output(s, buf, file, line, false)
+	l.output(s, logr, buf, file, line, false)
 }
 
-func (l *loggingT) print(s severity, args ...interface{}) {
-	l.printDepth(s, 1, args...)
+func (l *loggingT) print(s severity, logr logr.InfoLogger, args ...interface{}) {
+	l.printDepth(s, logr, 1, args...)
 }
 
-func (l *loggingT) printDepth(s severity, depth int, args ...interface{}) {
+func (l *loggingT) printDepth(s severity, logr logr.InfoLogger, 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 {
+		l.putBuffer(buf)
+		buf = l.getBuffer()
+	}
 	fmt.Fprint(buf, args...)
 	if buf.Bytes()[buf.Len()-1] != '\n' {
 		buf.WriteByte('\n')
 	}
-	l.output(s, buf, file, line, false)
+	l.output(s, logr, buf, file, line, false)
 }
 
-func (l *loggingT) printf(s severity, format string, args ...interface{}) {
+func (l *loggingT) printf(s severity, logr logr.InfoLogger, 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 {
+		l.putBuffer(buf)
+		buf = l.getBuffer()
+	}
 	fmt.Fprintf(buf, format, args...)
 	if buf.Bytes()[buf.Len()-1] != '\n' {
 		buf.WriteByte('\n')
 	}
-	l.output(s, buf, file, line, false)
+	l.output(s, logr, buf, 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, file string, line int, alsoToStderr bool, args ...interface{}) {
+func (l *loggingT) printWithFileLine(s severity, logr logr.InfoLogger, 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 {
+		l.putBuffer(buf)
+		buf = l.getBuffer()
+	}
 	fmt.Fprint(buf, args...)
 	if buf.Bytes()[buf.Len()-1] != '\n' {
 		buf.WriteByte('\n')
 	}
-	l.output(s, buf, file, line, alsoToStderr)
+	l.output(s, logr, buf, file, line, alsoToStderr)
 }
 
 // redirectBuffer is used to set an alternate destination for the logs
@@ -744,6 +773,18 @@ func (rb *redirectBuffer) Write(bytes []byte) (n int, err error) {
 	return rb.w.Write(bytes)
 }
 
+// SetLogger will set the backing logr implementation for klog.
+// If set, all log lines will be supressed from the regular Output, and
+// redirected to the logr implementation.
+// All log lines include the 'severity', 'file' and 'line' values attached as
+// structured logging values.
+// Use as:
+//   ...
+//   klog.SetLogger(zapr.NewLogger(zapLog))
+func SetLogger(logr logr.Logger) {
+	logging.logr = logr
+}
+
 // SetOutput sets the output destination for all severities
 func SetOutput(w io.Writer) {
 	for s := fatalLog; s >= infoLog; s-- {
@@ -767,7 +808,7 @@ func SetOutputBySeverity(name string, w io.Writer) {
 }
 
 // output writes the data to the log files and releases the buffer.
-func (l *loggingT) output(s severity, buf *buffer, file string, line int, alsoToStderr bool) {
+func (l *loggingT) output(s severity, log logr.InfoLogger, buf *buffer, file string, line int, alsoToStderr bool) {
 	l.mu.Lock()
 	if l.traceLocation.isSet() {
 		if l.traceLocation.match(file, line) {
@@ -775,7 +816,15 @@ func (l *loggingT) output(s severity, buf *buffer, file string, line int, alsoTo
 		}
 	}
 	data := buf.Bytes()
-	if l.toStderr {
+	if log != nil {
+		// TODO: set 'severity' and caller information as structured log info
+		// keysAndValues := []interface{}{"severity", severityName[s], "file", file, "line", line}
+		if s == errorLog {
+			l.logr.Error(nil, string(data))
+		} else {
+			log.Info(string(data))
+		}
+	} else if l.toStderr {
 		os.Stderr.Write(data)
 	} else {
 		if alsoToStderr || l.alsoToStderr || s >= l.stderrThreshold.get() {
@@ -1066,7 +1115,7 @@ func (lb logBridge) Write(b []byte) (n int, err error) {
 	}
 	// printWithFileLine with alsoToStderr=true, so standard log messages
 	// always appear on standard error.
-	logging.printWithFileLine(severity(lb), file, line, true, text)
+	logging.printWithFileLine(severity(lb), logging.logr, file, line, true, text)
 	return len(b), nil
 }
 
@@ -1098,13 +1147,23 @@ func (l *loggingT) setV(pc uintptr) Level {
 
 // Verbose is a boolean type that implements Infof (like Printf) etc.
 // See the documentation of V for more information.
-type Verbose bool
+type Verbose struct {
+	enabled bool
+	logr    logr.InfoLogger
+}
+
+func newVerbose(level Level, b bool) Verbose {
+	if logging.logr == nil {
+		return Verbose{b, nil}
+	}
+	return Verbose{b, logging.logr.V(int(level))}
+}
 
 // V reports whether verbosity at the call site is at least the requested level.
-// The returned value is a boolean of type Verbose, which implements Info, Infoln
+// The returned value is a struct of type Verbose, which implements Info, Infoln
 // and Infof. These methods will write to the Info log if called.
 // Thus, one may write either
-//	if klog.V(2) { klog.Info("log this") }
+//	if glog.V(2).Enabled() { klog.Info("log this") }
 // or
 //	klog.V(2).Info("log this")
 // The second form is shorter but the first is cheaper if logging is off because it does
@@ -1120,7 +1179,7 @@ func V(level Level) Verbose {
 
 	// Here is a cheap but safe test to see if V logging is enabled globally.
 	if logging.verbosity.get() >= level {
-		return Verbose(true)
+		return newVerbose(level, true)
 	}
 
 	// It's off globally but it vmodule may still be set.
@@ -1132,138 +1191,145 @@ func V(level Level) Verbose {
 		logging.mu.Lock()
 		defer logging.mu.Unlock()
 		if runtime.Callers(2, logging.pcs[:]) == 0 {
-			return Verbose(false)
+			return newVerbose(level, false)
 		}
 		v, ok := logging.vmap[logging.pcs[0]]
 		if !ok {
 			v = logging.setV(logging.pcs[0])
 		}
-		return Verbose(v >= level)
+		return newVerbose(level, v >= level)
 	}
-	return Verbose(false)
+	return newVerbose(level, false)
+}
+
+// Enabled will return true if this log level is enabled, guarded by the value
+// of v.
+// See the documentation of V for usage.
+func (v Verbose) Enabled() bool {
+	return v.enabled
 }
 
 // Info is equivalent to the global Info function, guarded by the value of v.
 // See the documentation of V for usage.
 func (v Verbose) Info(args ...interface{}) {
-	if v {
-		logging.print(infoLog, args...)
+	if v.enabled {
+		logging.print(infoLog, v.logr, args...)
 	}
 }
 
 // Infoln is equivalent to the global Infoln function, guarded by the value of v.
 // See the documentation of V for usage.
 func (v Verbose) Infoln(args ...interface{}) {
-	if v {
-		logging.println(infoLog, args...)
+	if v.enabled {
+		logging.println(infoLog, v.logr, args...)
 	}
 }
 
 // Infof is equivalent to the global Infof function, guarded by the value of v.
 // See the documentation of V for usage.
 func (v Verbose) Infof(format string, args ...interface{}) {
-	if v {
-		logging.printf(infoLog, format, args...)
+	if v.enabled {
+		logging.printf(infoLog, v.logr, format, args...)
 	}
 }
 
 // Info logs to the INFO log.
 // Arguments are handled in the manner of fmt.Print; a newline is appended if missing.
 func Info(args ...interface{}) {
-	logging.print(infoLog, args...)
+	logging.print(infoLog, logging.logr, args...)
 }
 
 // InfoDepth acts as Info but uses depth to determine which call frame to log.
 // InfoDepth(0, "msg") is the same as Info("msg").
 func InfoDepth(depth int, args ...interface{}) {
-	logging.printDepth(infoLog, depth, args...)
+	logging.printDepth(infoLog, logging.logr, depth, args...)
 }
 
 // Infoln logs to the INFO log.
 // Arguments are handled in the manner of fmt.Println; a newline is always appended.
 func Infoln(args ...interface{}) {
-	logging.println(infoLog, args...)
+	logging.println(infoLog, logging.logr, args...)
 }
 
 // Infof logs to the INFO log.
 // Arguments are handled in the manner of fmt.Printf; a newline is appended if missing.
 func Infof(format string, args ...interface{}) {
-	logging.printf(infoLog, format, args...)
+	logging.printf(infoLog, logging.logr, format, args...)
 }
 
 // Warning logs to the WARNING and INFO logs.
 // Arguments are handled in the manner of fmt.Print; a newline is appended if missing.
 func Warning(args ...interface{}) {
-	logging.print(warningLog, args...)
+	logging.print(warningLog, logging.logr, args...)
 }
 
 // WarningDepth acts as Warning but uses depth to determine which call frame to log.
 // WarningDepth(0, "msg") is the same as Warning("msg").
 func WarningDepth(depth int, args ...interface{}) {
-	logging.printDepth(warningLog, depth, args...)
+	logging.printDepth(warningLog, logging.logr, depth, args...)
 }
 
 // Warningln logs to the WARNING and INFO logs.
 // Arguments are handled in the manner of fmt.Println; a newline is always appended.
 func Warningln(args ...interface{}) {
-	logging.println(warningLog, args...)
+	logging.println(warningLog, logging.logr, args...)
 }
 
 // Warningf logs to the WARNING and INFO logs.
 // Arguments are handled in the manner of fmt.Printf; a newline is appended if missing.
 func Warningf(format string, args ...interface{}) {
-	logging.printf(warningLog, format, args...)
+	logging.printf(warningLog, logging.logr, format, args...)
 }
 
 // Error logs to the ERROR, WARNING, and INFO logs.
 // Arguments are handled in the manner of fmt.Print; a newline is appended if missing.
 func Error(args ...interface{}) {
-	logging.print(errorLog, args...)
+	logging.print(errorLog, logging.logr, args...)
 }
 
 // ErrorDepth acts as Error but uses depth to determine which call frame to log.
 // ErrorDepth(0, "msg") is the same as Error("msg").
 func ErrorDepth(depth int, args ...interface{}) {
-	logging.printDepth(errorLog, depth, args...)
+	logging.printDepth(errorLog, logging.logr, depth, args...)
 }
 
 // Errorln logs to the ERROR, WARNING, and INFO logs.
 // Arguments are handled in the manner of fmt.Println; a newline is always appended.
 func Errorln(args ...interface{}) {
-	logging.println(errorLog, args...)
+	logging.println(errorLog, logging.logr, args...)
 }
 
 // Errorf logs to the ERROR, WARNING, and INFO logs.
 // Arguments are handled in the manner of fmt.Printf; a newline is appended if missing.
 func Errorf(format string, args ...interface{}) {
-	logging.printf(errorLog, format, args...)
+	logging.printf(errorLog, logging.logr, format, args...)
 }
 
 // Fatal logs to the FATAL, ERROR, WARNING, and INFO logs,
 // including a stack trace of all running goroutines, then calls os.Exit(255).
 // Arguments are handled in the manner of fmt.Print; a newline is appended if missing.
 func Fatal(args ...interface{}) {
-	logging.print(fatalLog, args...)
+	logging.print(fatalLog, logging.logr, args...)
 }
 
 // FatalDepth acts as Fatal but uses depth to determine which call frame to log.
 // FatalDepth(0, "msg") is the same as Fatal("msg").
 func FatalDepth(depth int, args ...interface{}) {
-	logging.printDepth(fatalLog, depth, args...)
+	logging.printDepth(fatalLog, logging.logr, depth, args...)
 }
 
 // Fatalln logs to the FATAL, ERROR, WARNING, and INFO logs,
 // including a stack trace of all running goroutines, then calls os.Exit(255).
 // Arguments are handled in the manner of fmt.Println; a newline is always appended.
 func Fatalln(args ...interface{}) {
-	logging.println(fatalLog, args...)
+	logging.println(fatalLog, logging.logr, args...)
 }
 
 // Fatalf logs to the FATAL, ERROR, WARNING, and INFO logs,
 // including a stack trace of all running goroutines, then calls os.Exit(255).
 // Arguments are handled in the manner of fmt.Printf; a newline is appended if missing.
 func Fatalf(format string, args ...interface{}) {
-	logging.printf(fatalLog, format, args...)
+	logging.printf(fatalLog, logging.logr, format, args...)
 }
 
 // fatalNoStacks is non-zero if we are to exit without dumping goroutine stacks.
@@ -1274,25 +1340,25 @@ var fatalNoStacks uint32
 // Arguments are handled in the manner of fmt.Print; a newline is appended if missing.
 func Exit(args ...interface{}) {
 	atomic.StoreUint32(&fatalNoStacks, 1)
-	logging.print(fatalLog, args...)
+	logging.print(fatalLog, logging.logr, args...)
 }
 
 // ExitDepth acts as Exit but uses depth to determine which call frame to log.
 // ExitDepth(0, "msg") is the same as Exit("msg").
 func ExitDepth(depth int, args ...interface{}) {
 	atomic.StoreUint32(&fatalNoStacks, 1)
-	logging.printDepth(fatalLog, depth, args...)
+	logging.printDepth(fatalLog, logging.logr, depth, args...)
 }
 
 // Exitln logs to the FATAL, ERROR, WARNING, and INFO logs, then calls os.Exit(1).
 func Exitln(args ...interface{}) {
 	atomic.StoreUint32(&fatalNoStacks, 1)
-	logging.println(fatalLog, args...)
+	logging.println(fatalLog, logging.logr, args...)
 }
 
 // Exitf logs to the FATAL, ERROR, WARNING, and INFO logs, then calls os.Exit(1).
 // Arguments are handled in the manner of fmt.Printf; a newline is appended if missing.
 func Exitf(format string, args ...interface{}) {
 	atomic.StoreUint32(&fatalNoStacks, 1)
-	logging.printf(fatalLog, format, args...)
+	logging.printf(fatalLog, logging.logr, format, args...)
 }
diff --git a/klogr/klogr.go b/klogr/klogr.go
index f63557d..f80360c 100644
--- a/klogr/klogr.go
+++ b/klogr/klogr.go
@@ -150,7 +150,7 @@ func (l klogger) Info(msg string, kvList ...interface{}) {
 }
 
 func (l klogger) Enabled() bool {
-	return bool(klog.V(klog.Level(l.level)))
+	return bool(klog.V(klog.Level(l.level)).Enabled())
 }
 
 func (l klogger) Error(err error, msg string, kvList ...interface{}) {


Test patch:
diff --git a/klog_test.go b/klog_test.go
index d8455ae..46b1e1c 100644
--- a/klog_test.go
+++ b/klog_test.go
@@ -285,13 +285,13 @@ func TestVmoduleOn(t *testing.T) {
 	defer logging.swap(logging.newBuffers())
 	logging.vmodule.Set("klog_test=2")
 	defer logging.vmodule.Set("")
-	if !V(1) {
+	if !V(1).Enabled() {
 		t.Error("V not enabled for 1")
 	}
-	if !V(2) {
+	if !V(2).Enabled() {
 		t.Error("V not enabled for 2")
 	}
-	if V(3) {
+	if V(3).Enabled() {
 		t.Error("V enabled for 3")
 	}
 	V(2).Info("test")
@@ -310,7 +310,7 @@ func TestVmoduleOff(t *testing.T) {
 	logging.vmodule.Set("notthisfile=2")
 	defer logging.vmodule.Set("")
 	for i := 1; i <= 3; i++ {
-		if V(Level(i)) {
+		if V(Level(i)).Enabled() {
 			t.Errorf("V enabled for %d", i)
 		}
 	}
@@ -344,7 +344,7 @@ func testVmoduleGlob(pat string, match bool, t *testing.T) {
 	defer logging.swap(logging.newBuffers())
 	defer logging.vmodule.Set("")
 	logging.vmodule.Set(pat)
-	if V(2) != Verbose(match) {
+	if V(2).Enabled() != match {
 		t.Errorf("incorrect match for %q: got %t expected %t", pat, V(2), match)
 	}
 }