Skip to content

Commit

Permalink
Enabled: always must skip two levels
Browse files Browse the repository at this point in the history
klog does stack unwinding in `LogSink.Enabled` to implement per-source code
verbosity thresholds (`-vmodule`). This worked for `logger.Info` and
`logger.Error` because the code was written such that it matches how logr is
implemented (Logger.Info -> Logger.Enabled -> LogSink.Enabled).

It did not work for direct calls (`if logger.Enabled`) because then the call
chain is `Logger.Enabled -> LogSink.Enabled`. That callchain is less common, so
to fix this problem the callchains get arranges so that all calls go through
one additional intermediate level.

This is unnecessary extra work, but it is necessary to avoid breaking the more
common normal logging calls with klog and -vmodule.
  • Loading branch information
pohly committed Sep 2, 2023
1 parent 7bf6c82 commit 374a35a
Showing 1 changed file with 15 additions and 3 deletions.
18 changes: 15 additions & 3 deletions logr.go
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,15 @@ type Logger struct {
// Enabled tests whether this Logger is enabled. For example, commandline
// flags might be used to set the logging verbosity and disable some info logs.
func (l Logger) Enabled() bool {
return l.sink != nil && l.sink.Enabled(l.level)
return l.sink != nil && l.enabled(l.level)
}

// enabled ensures that Enabled and Info both call LogSink.Enabled with one
// intermediate stack frame. This wouldn't be necessary if Info had called
// LogSink.Enabled directly in logr v1.0.0, but it didn't and now that cannot
// be changed anymore because implementations depend on this behavior.
func (l Logger) enabled() bool {
return l.sink.Enabled(l.level)
}

// Info logs a non-error message with the given key/value pairs as context.
Expand All @@ -271,7 +279,7 @@ func (l Logger) Info(msg string, keysAndValues ...any) {
if l.sink == nil {
return
}
if l.Enabled() {
if l.enabled() {
if withHelper, ok := l.sink.(CallStackHelperLogSink); ok {
withHelper.GetCallStackHelper()()
}
Expand Down Expand Up @@ -447,9 +455,13 @@ func NewContext(ctx context.Context, logger Logger) context.Context {
// LogSinks might want to know.
type RuntimeInfo struct {
// CallDepth is the number of call frames the logr library adds between the
// end-user and the LogSink. LogSink implementations which choose to print
// end-user and the LogSink. LogSink implementations which choose to print
// the original logging site (e.g. file & line) should climb this many
// additional frames to find it.
//
// The CallDepth is given for LogSink.Info and LogSink.Error such that 1 means
// "skip one intermediate level". For historic reasons, LogSink.Enabled must
// skip one additional level compared to those two calls.
CallDepth int
}

Expand Down

0 comments on commit 374a35a

Please sign in to comment.