Merge pull request #218 from thockin/calldepth_enabled_consistent

Use same call depth for Enabled, Info, Error

logr.go

@@ -258,6 +258,12 @@ 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 {
+	// Some implementations of LogSink look at the caller in Enabled (e.g.
+	// different verbosity levels per package or file), but we only pass one
+	// CallDepth in (via Init).  This means that all calls from Logger to the
+	// LogSink's Enabled, Info, and Error methods must have the same number of
+	// frames.  In other words, Logger methods can't call other Logger methods
+	// which call these LogSink methods unless we do it the same in all paths.
 	return l.sink != nil && l.sink.Enabled(l.level)
 }
 
@@ -271,7 +277,7 @@ func (l Logger) Info(msg string, keysAndValues ...any) {
 	if l.sink == nil {
 		return
 	}
-	if l.Enabled() {
+	if l.sink.Enabled(l.level) { // see comment in Enabled
 		if withHelper, ok := l.sink.(CallStackHelperLogSink); ok {
 			withHelper.GetCallStackHelper()()
 		}

logr_test.go

@@ -21,6 +21,7 @@ import (
 	"errors"
 	"fmt"
 	"reflect"
+	"runtime"
 	"testing"
 )
 
@@ -443,6 +444,49 @@ func TestZeroValue(t *testing.T) {
 	_, _ = l.WithCallStackHelper()
 }
 
+func TestCallDepthConsistent(t *testing.T) {
+	sink := &testLogSink{}
+
+	depth := 0
+	expect := "github.com/go-logr/logr.TestCallDepthConsistent"
+	sink.fnInit = func(ri RuntimeInfo) {
+		depth = ri.CallDepth + 1 // 1 for these function pointers
+		if caller := getCaller(depth); caller != expect {
+			t.Errorf("identified wrong caller %q", caller)
+		}
+
+	}
+	sink.fnEnabled = func(_ int) bool {
+		if caller := getCaller(depth); caller != expect {
+			t.Errorf("identified wrong caller %q", caller)
+		}
+		return true
+	}
+	sink.fnError = func(_ error, _ string, _ ...any) {
+		if caller := getCaller(depth); caller != expect {
+			t.Errorf("identified wrong caller %q", caller)
+		}
+	}
+	l := New(sink)
+
+	l.Enabled()
+	l.Info("msg")
+	l.Error(nil, "msg")
+}
+
+func getCaller(depth int) string {
+	// +1 for this frame, +1 for Info/Error/Enabled.
+	pc, _, _, ok := runtime.Caller(depth + 2)
+	if !ok {
+		return "<runtime.Caller failed>"
+	}
+	fp := runtime.FuncForPC(pc)
+	if fp == nil {
+		return "<runtime.FuncForPC failed>"
+	}
+	return fp.Name()
+}
+
 func expectEqual[T comparable](tb testing.TB, msg string, expected, actual T) bool {
 	if expected == actual {
 		return true