diff --git a/go.mod b/go.mod index fabf9b8f66..19331398ef 100644 --- a/go.mod +++ b/go.mod @@ -5,8 +5,8 @@ go 1.16 require ( github.com/evanphx/json-patch v4.11.0+incompatible github.com/fsnotify/fsnotify v1.4.9 - github.com/go-logr/logr v0.4.0 - github.com/go-logr/zapr v0.4.0 + github.com/go-logr/logr v1.1.0 + github.com/go-logr/zapr v1.1.0 github.com/imdario/mergo v0.3.12 // indirect github.com/onsi/ginkgo v1.16.4 github.com/onsi/gomega v1.15.0 @@ -23,6 +23,7 @@ require ( k8s.io/apimachinery v0.22.2 k8s.io/client-go v0.22.2 k8s.io/component-base v0.22.2 + k8s.io/klog/v2 v2.20.0 // indirect k8s.io/utils v0.0.0-20210819203725-bdf08cb9a70a sigs.k8s.io/yaml v1.2.0 ) diff --git a/go.sum b/go.sum index 293f15d330..6d444292be 100644 --- a/go.sum +++ b/go.sum @@ -124,10 +124,12 @@ github.com/go-logfmt/logfmt v0.4.0/go.mod h1:3RMwSq7FuexP4Kalkev3ejPJsZTpXXBr9+V github.com/go-logfmt/logfmt v0.5.0/go.mod h1:wCYkCAKZfumFQihp8CzCvQ3paCTfi41vtzG1KdI/P7A= github.com/go-logr/logr v0.1.0/go.mod h1:ixOQHD9gLJUVQQ2ZOR7zLEifBX6tGkNJF4QyIY7sIas= github.com/go-logr/logr v0.2.0/go.mod h1:z6/tIYblkpsD+a4lm/fGIIU9mZ+XfAiaFtq7xTgseGU= -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/zapr v0.4.0 h1:uc1uML3hRYL9/ZZPdgHS/n8Nzo+eaYL/Efxkkamf7OM= -github.com/go-logr/zapr v0.4.0/go.mod h1:tabnROwaDl0UNxkVeFRbY8bwB37GwRv0P8lg6aAiEnk= +github.com/go-logr/logr v1.0.0/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= +github.com/go-logr/logr v1.1.0 h1:nAbevmWlS2Ic4m4+/An5NXkaGqlqpbBgdcuThZxnZyI= +github.com/go-logr/logr v1.1.0/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= +github.com/go-logr/zapr v1.1.0 h1:rZHor2gcVGCG11UlKl+WUsfCMOOi2k/mTCDKDK6zZws= +github.com/go-logr/zapr v1.1.0/go.mod h1:YShqdLLTU346TNVu8Tvwe3bOo6gc75oZ1joeE+1lYdQ= github.com/go-openapi/jsonpointer v0.19.3/go.mod h1:Pl9vOtqEWErmShwVjC8pYs9cog34VGT37dQOVbmoatg= github.com/go-openapi/jsonpointer v0.19.5/go.mod h1:Pl9vOtqEWErmShwVjC8pYs9cog34VGT37dQOVbmoatg= github.com/go-openapi/jsonreference v0.19.3/go.mod h1:rjx6GuL8TTa9VaixXglHmQmIL98+wF9xc8zWvFonSJ8= @@ -752,8 +754,9 @@ k8s.io/gengo v0.0.0-20200413195148-3a45101e95ac/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8 k8s.io/gengo v0.0.0-20201214224949-b6c5ce23f027/go.mod h1:FiNAH4ZV3gBg2Kwh89tzAEV2be7d5xI0vBa/VySYy3E= k8s.io/klog/v2 v2.0.0/go.mod h1:PBfzABfn139FHAV07az/IF9Wp1bkk3vpT2XSJ76fSDE= k8s.io/klog/v2 v2.2.0/go.mod h1:Od+F08eJP+W3HUb4pSrPpgp9DGU4GzlpG/TmITuYh/Y= -k8s.io/klog/v2 v2.9.0 h1:D7HV+n1V57XeZ0m6tdRkfknthUaM06VFbWldOFh8kzM= k8s.io/klog/v2 v2.9.0/go.mod h1:hy9LJ/NvuK+iVyP4Ehqva4HxZG/oXyIS3n3Jmire4Ec= +k8s.io/klog/v2 v2.20.0 h1:tlyxlSvd63k7axjhuchckaRJm+a92z5GSOrTOQY5sHw= +k8s.io/klog/v2 v2.20.0/go.mod h1:Gm8eSIfQN6457haJuPaMxZw4wyP5k+ykPFlrhQDvhvw= k8s.io/kube-openapi v0.0.0-20210421082810-95288971da7e h1:KLHHjkdQFomZy8+06csTWZ0m1343QqxZhR2LJ1OxCYM= k8s.io/kube-openapi v0.0.0-20210421082810-95288971da7e/go.mod h1:vHXdDvt9+2spS2Rx9ql3I8tycm3H9FDfdUoIuKCefvw= k8s.io/utils v0.0.0-20210819203725-bdf08cb9a70a h1:8dYfu/Fc9Gz2rNJKB9IQRGgQOh2clmRzNIPPY1xLY5g= diff --git a/pkg/builder/controller.go b/pkg/builder/controller.go index 2cd4ce9de1..9a74d6ec9a 100644 --- a/pkg/builder/controller.go +++ b/pkg/builder/controller.go @@ -305,7 +305,7 @@ func (blder *Builder) doController(r reconcile.Reconciler) error { } // Setup the logger. - if ctrlOptions.Log == nil { + if ctrlOptions.Log.GetSink() == nil { ctrlOptions.Log = blder.mgr.GetLogger() } ctrlOptions.Log = ctrlOptions.Log.WithValues("reconciler group", gvk.Group, "reconciler kind", gvk.Kind) diff --git a/pkg/builder/controller_test.go b/pkg/builder/controller_test.go index 62c5044e99..99aa2d185d 100644 --- a/pkg/builder/controller_test.go +++ b/pkg/builder/controller_test.go @@ -57,10 +57,22 @@ type testLogger struct { logr.Logger } -func (l *testLogger) WithName(_ string) logr.Logger { +func (l *testLogger) Init(info logr.RuntimeInfo) { +} + +func (l *testLogger) Enabled(level int) bool { + return true +} + +func (l *testLogger) Info(level int, msg string, keysAndValues ...interface{}) { + +} + +func (l *testLogger) WithValues(keysAndValues ...interface{}) logr.LogSink { return l } -func (l *testLogger) WithValues(_ ...interface{}) logr.Logger { + +func (l *testLogger) WithName(name string) logr.LogSink { return l } @@ -227,7 +239,7 @@ var _ = Describe("application", func() { logger := &testLogger{} newController = func(name string, mgr manager.Manager, options controller.Options) (controller.Controller, error) { - if options.Log == logger { + if options.Log.GetSink() == logger { return controller.New(name, mgr, options) } return nil, fmt.Errorf("logger expected %T but found %T", logger, options.Log) @@ -240,7 +252,7 @@ var _ = Describe("application", func() { instance, err := ControllerManagedBy(m). For(&appsv1.ReplicaSet{}). Owns(&appsv1.ReplicaSet{}). - WithLogger(logger). + WithLogger(logr.New(logger)). Build(noop) Expect(err).NotTo(HaveOccurred()) Expect(instance).NotTo(BeNil()) diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index dfd0fa9dd8..4b8ee8e7c5 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -245,7 +245,7 @@ func setOptionsDefaults(options Options) Options { } } - if options.Logger == nil { + if options.Logger.GetSink() == nil { options.Logger = logf.RuntimeLog.WithName("cluster") } diff --git a/pkg/config/v1alpha1/zz_generated.deepcopy.go b/pkg/config/v1alpha1/zz_generated.deepcopy.go index 752fa9754c..5329bef667 100644 --- a/pkg/config/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/config/v1alpha1/zz_generated.deepcopy.go @@ -1,3 +1,4 @@ +//go:build !ignore_autogenerated // +build !ignore_autogenerated // Code generated by controller-gen. DO NOT EDIT. diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 88ba786717..a5c850274e 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -104,7 +104,7 @@ func NewUnmanaged(name string, mgr manager.Manager, options Options) (Controller return nil, fmt.Errorf("must specify Name for Controller") } - if options.Log == nil { + if options.Log.GetSink() == nil { options.Log = mgr.GetLogger() } diff --git a/pkg/internal/recorder/recorder_test.go b/pkg/internal/recorder/recorder_test.go index 08d60f9deb..86bcdd36f6 100644 --- a/pkg/internal/recorder/recorder_test.go +++ b/pkg/internal/recorder/recorder_test.go @@ -29,7 +29,7 @@ var _ = Describe("recorder.Provider", func() { makeBroadcaster := func() (record.EventBroadcaster, bool) { return record.NewBroadcaster(), true } Describe("NewProvider", func() { It("should return a provider instance and a nil error.", func() { - provider, err := recorder.NewProvider(cfg, scheme.Scheme, logr.DiscardLogger{}, makeBroadcaster) + provider, err := recorder.NewProvider(cfg, scheme.Scheme, logr.Discard(), makeBroadcaster) Expect(provider).NotTo(BeNil()) Expect(err).NotTo(HaveOccurred()) }) @@ -38,14 +38,14 @@ var _ = Describe("recorder.Provider", func() { // Invalid the config cfg1 := *cfg cfg1.Host = "invalid host" - _, err := recorder.NewProvider(&cfg1, scheme.Scheme, logr.DiscardLogger{}, makeBroadcaster) + _, err := recorder.NewProvider(&cfg1, scheme.Scheme, logr.Discard(), makeBroadcaster) Expect(err).NotTo(BeNil()) Expect(err.Error()).To(ContainSubstring("failed to init client")) }) }) Describe("GetEventRecorder", func() { It("should return a recorder instance.", func() { - provider, err := recorder.NewProvider(cfg, scheme.Scheme, logr.DiscardLogger{}, makeBroadcaster) + provider, err := recorder.NewProvider(cfg, scheme.Scheme, logr.Discard(), makeBroadcaster) Expect(err).NotTo(HaveOccurred()) recorder := provider.GetEventRecorderFor("test") diff --git a/pkg/log/deleg.go b/pkg/log/deleg.go index 9d73947dac..5c2cecb765 100644 --- a/pkg/log/deleg.go +++ b/pkg/log/deleg.go @@ -25,16 +25,15 @@ import ( // loggerPromise knows how to populate a concrete logr.Logger // with options, given an actual base logger later on down the line. type loggerPromise struct { - logger *DelegatingLogger + logger *DelegatingLogSink childPromises []*loggerPromise promisesLock sync.Mutex - name *string - tags []interface{} - level int + name *string + tags []interface{} } -func (p *loggerPromise) WithName(l *DelegatingLogger, name string) *loggerPromise { +func (p *loggerPromise) WithName(l *DelegatingLogSink, name string) *loggerPromise { res := &loggerPromise{ logger: l, name: &name, @@ -48,7 +47,7 @@ func (p *loggerPromise) WithName(l *DelegatingLogger, name string) *loggerPromis } // WithValues provides a new Logger with the tags appended. -func (p *loggerPromise) WithValues(l *DelegatingLogger, tags ...interface{}) *loggerPromise { +func (p *loggerPromise) WithValues(l *DelegatingLogSink, tags ...interface{}) *loggerPromise { res := &loggerPromise{ logger: l, tags: tags, @@ -61,61 +60,54 @@ func (p *loggerPromise) WithValues(l *DelegatingLogger, tags ...interface{}) *lo return res } -func (p *loggerPromise) V(l *DelegatingLogger, level int) *loggerPromise { - res := &loggerPromise{ - logger: l, - level: level, - promisesLock: sync.Mutex{}, - } - - p.promisesLock.Lock() - defer p.promisesLock.Unlock() - p.childPromises = append(p.childPromises, res) - return res -} - // Fulfill instantiates the Logger with the provided logger. -func (p *loggerPromise) Fulfill(parentLogger logr.Logger) { - logger := logr.WithCallDepth(parentLogger, 1) +func (p *loggerPromise) Fulfill(parentLogSink logr.LogSink) { + sink := parentLogSink + if p.name != nil { - logger = logger.WithName(*p.name) + sink = sink.WithName(*p.name) } if p.tags != nil { - logger = logger.WithValues(p.tags...) - } - if p.level != 0 { - logger = logger.V(p.level) + sink = sink.WithValues(p.tags...) } p.logger.lock.Lock() - p.logger.logger = logger + p.logger.logger = sink p.logger.promise = nil p.logger.lock.Unlock() for _, childPromise := range p.childPromises { - childPromise.Fulfill(logger) + childPromise.Fulfill(sink) } } -// DelegatingLogger is a logr.Logger that delegates to another logr.Logger. +// DelegatingLogSink is a logr.LogSink that delegates to another logr.LogSink. // If the underlying promise is not nil, it registers calls to sub-loggers with // the logging factory to be populated later, and returns a new delegating // logger. It expects to have *some* logr.Logger set at all times (generally // a no-op logger before the promises are fulfilled). -type DelegatingLogger struct { +type DelegatingLogSink struct { lock sync.RWMutex - logger logr.Logger + logger logr.LogSink promise *loggerPromise + info logr.RuntimeInfo +} + +// Init implements logr.LogSink. +func (l *DelegatingLogSink) Init(info logr.RuntimeInfo) { + l.lock.Lock() + defer l.lock.Unlock() + l.info = info } // 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 *DelegatingLogger) Enabled() bool { +func (l *DelegatingLogSink) Enabled(v int) bool { l.lock.RLock() defer l.lock.RUnlock() - return l.logger.Enabled() + return l.logger.Enabled(v) } // Info logs a non-error message with the given key/value pairs as context. @@ -124,10 +116,10 @@ func (l *DelegatingLogger) Enabled() bool { // the log line. The key/value pairs can then be used to add additional // variable information. The key/value pairs should alternate string // keys and arbitrary values. -func (l *DelegatingLogger) Info(msg string, keysAndValues ...interface{}) { +func (l *DelegatingLogSink) Info(level int, msg string, keysAndValues ...interface{}) { l.lock.RLock() defer l.lock.RUnlock() - l.logger.Info(msg, keysAndValues...) + l.logger.Info(level, msg, keysAndValues...) } // Error logs an error, with the given message and key/value pairs as context. @@ -138,33 +130,14 @@ func (l *DelegatingLogger) Info(msg string, keysAndValues ...interface{}) { // The msg field should be used to add context to any underlying error, // while the err field should be used to attach the actual error that // triggered this log line, if present. -func (l *DelegatingLogger) Error(err error, msg string, keysAndValues ...interface{}) { +func (l *DelegatingLogSink) Error(err error, msg string, keysAndValues ...interface{}) { l.lock.RLock() defer l.lock.RUnlock() l.logger.Error(err, msg, keysAndValues...) } -// V returns an Logger value for a specific verbosity level, relative to -// this Logger. In other words, V values are additive. V higher verbosity -// level means a log message is less important. It's illegal to pass a log -// level less than zero. -func (l *DelegatingLogger) V(level int) logr.Logger { - l.lock.RLock() - defer l.lock.RUnlock() - - if l.promise == nil { - return l.logger.V(level) - } - - res := &DelegatingLogger{logger: l.logger} - promise := l.promise.V(res, level) - res.promise = promise - - return res -} - // WithName provides a new Logger with the name appended. -func (l *DelegatingLogger) WithName(name string) logr.Logger { +func (l *DelegatingLogSink) WithName(name string) logr.LogSink { l.lock.RLock() defer l.lock.RUnlock() @@ -172,7 +145,7 @@ func (l *DelegatingLogger) WithName(name string) logr.Logger { return l.logger.WithName(name) } - res := &DelegatingLogger{logger: l.logger} + res := &DelegatingLogSink{logger: l.logger} promise := l.promise.WithName(res, name) res.promise = promise @@ -180,7 +153,7 @@ func (l *DelegatingLogger) WithName(name string) logr.Logger { } // WithValues provides a new Logger with the tags appended. -func (l *DelegatingLogger) WithValues(tags ...interface{}) logr.Logger { +func (l *DelegatingLogSink) WithValues(tags ...interface{}) logr.LogSink { l.lock.RLock() defer l.lock.RUnlock() @@ -188,7 +161,7 @@ func (l *DelegatingLogger) WithValues(tags ...interface{}) logr.Logger { return l.logger.WithValues(tags...) } - res := &DelegatingLogger{logger: l.logger} + res := &DelegatingLogSink{logger: l.logger} promise := l.promise.WithValues(res, tags...) res.promise = promise @@ -198,16 +171,16 @@ func (l *DelegatingLogger) WithValues(tags ...interface{}) logr.Logger { // Fulfill switches the logger over to use the actual logger // provided, instead of the temporary initial one, if this method // has not been previously called. -func (l *DelegatingLogger) Fulfill(actual logr.Logger) { +func (l *DelegatingLogSink) Fulfill(actual logr.LogSink) { if l.promise != nil { l.promise.Fulfill(actual) } } -// NewDelegatingLogger constructs a new DelegatingLogger which uses +// NewDelegatingLogSink constructs a new DelegatingLogSink which uses // the given logger before it's promise is fulfilled. -func NewDelegatingLogger(initial logr.Logger) *DelegatingLogger { - l := &DelegatingLogger{ +func NewDelegatingLogSink(initial logr.LogSink) *DelegatingLogSink { + l := &DelegatingLogSink{ logger: initial, promise: &loggerPromise{promisesLock: sync.Mutex{}}, } diff --git a/pkg/log/log.go b/pkg/log/log.go index 229ac7ec35..76950cc34b 100644 --- a/pkg/log/log.go +++ b/pkg/log/log.go @@ -47,14 +47,14 @@ func SetLogger(l logr.Logger) { defer loggerWasSetLock.Unlock() loggerWasSet = true - Log.Fulfill(l) + dlog.Fulfill(l.GetSink()) } // It is safe to assume that if this wasn't set within the first 30 seconds of a binaries -// lifetime, it will never get set. The DelegatingLogger causes a high number of memory -// allocations when not given an actual Logger, so we set a NullLogger to avoid that. +// lifetime, it will never get set. The DelegatingLogSink causes a high number of memory +// allocations when not given an actual Logger, so we set a NullLogSink to avoid that. // -// We need to keep the DelegatingLogger because we have various inits() that get a logger from +// We need to keep the DelegatingLogSink because we have various inits() that get a logger from // here. They will always get executed before any code that imports controller-runtime // has a chance to run and hence to set an actual logger. func init() { @@ -64,7 +64,7 @@ func init() { loggerWasSetLock.Lock() defer loggerWasSetLock.Unlock() if !loggerWasSet { - Log.Fulfill(NullLogger{}) + dlog.Fulfill(NullLogSink{}) } }() } @@ -78,14 +78,17 @@ var ( // to another logr.Logger. You *must* call SetLogger to // get any actual logging. If SetLogger is not called within // the first 30 seconds of a binaries lifetime, it will get -// set to a NullLogger. -var Log = NewDelegatingLogger(NullLogger{}) +// set to a NullLogSink. +var ( + dlog = NewDelegatingLogSink(NullLogSink{}) + Log = logr.New(dlog) +) // FromContext returns a logger with predefined values from a context.Context. func FromContext(ctx context.Context, keysAndValues ...interface{}) logr.Logger { - var log logr.Logger = Log + log := Log if ctx != nil { - if logger := logr.FromContext(ctx); logger != nil { + if logger, err := logr.FromContext(ctx); err == nil { log = logger } } diff --git a/pkg/log/log_test.go b/pkg/log/log_test.go index be3ad87997..38d55ed3ab 100644 --- a/pkg/log/log_test.go +++ b/pkg/log/log_test.go @@ -25,7 +25,7 @@ import ( . "github.com/onsi/gomega" ) -var _ logr.Logger = &DelegatingLogger{} +var _ logr.LogSink = &DelegatingLogSink{} // logInfo is the information for a particular fakeLogger message. type logInfo struct { @@ -49,7 +49,10 @@ type fakeLogger struct { root *fakeLoggerRoot } -func (f *fakeLogger) WithName(name string) logr.Logger { +func (f *fakeLogger) Init(info logr.RuntimeInfo) { +} + +func (f *fakeLogger) WithName(name string) logr.LogSink { names := append([]string(nil), f.name...) names = append(names, name) return &fakeLogger{ @@ -59,7 +62,7 @@ func (f *fakeLogger) WithName(name string) logr.Logger { } } -func (f *fakeLogger) WithValues(vals ...interface{}) logr.Logger { +func (f *fakeLogger) WithValues(vals ...interface{}) logr.LogSink { tags := append([]interface{}(nil), f.tags...) tags = append(tags, vals...) return &fakeLogger{ @@ -80,7 +83,7 @@ func (f *fakeLogger) Error(err error, msg string, vals ...interface{}) { }) } -func (f *fakeLogger) Info(msg string, vals ...interface{}) { +func (f *fakeLogger) Info(l int, msg string, vals ...interface{}) { tags := append([]interface{}(nil), f.tags...) tags = append(tags, vals...) f.root.messages = append(f.root.messages, logInfo{ @@ -90,8 +93,9 @@ func (f *fakeLogger) Info(msg string, vals ...interface{}) { }) } -func (f *fakeLogger) Enabled() bool { return true } -func (f *fakeLogger) V(lvl int) logr.Logger { return f } +func (f *fakeLogger) Enabled(l int) bool { + return true +} var _ = Describe("logging", func() { @@ -103,7 +107,7 @@ var _ = Describe("logging", func() { By("actually setting the logger") logger := &fakeLogger{root: &fakeLoggerRoot{}} - SetLogger(logger) + SetLogger(logr.New(logger)) By("grabbing another sub-logger and logging to both loggers") l2 := Log.WithName("runtimeLog").WithValues("newtag", "newvalue2") @@ -121,24 +125,24 @@ var _ = Describe("logging", func() { Describe("lazy logger initialization", func() { var ( root *fakeLoggerRoot - baseLog logr.Logger - delegLog *DelegatingLogger + baseLog logr.LogSink + delegLog *DelegatingLogSink ) BeforeEach(func() { root = &fakeLoggerRoot{} baseLog = &fakeLogger{root: root} - delegLog = NewDelegatingLogger(NullLogger{}) + delegLog = NewDelegatingLogSink(NullLogSink{}) }) It("should delegate with name", func() { By("asking for a logger with a name before fulfill, and logging") - befFulfill1 := delegLog.WithName("before-fulfill") + befFulfill1 := logr.New(delegLog).WithName("before-fulfill") befFulfill2 := befFulfill1.WithName("two") befFulfill1.Info("before fulfill") By("logging on the base logger before fulfill") - delegLog.Info("before fulfill base") + logr.New(delegLog).Info("before fulfill base") By("ensuring that no messages were actually recorded") Expect(root.messages).To(BeEmpty()) @@ -154,7 +158,7 @@ var _ = Describe("logging", func() { befFulfill1.WithName("after-from-before").Info("after 3") By("logging with new loggers") - delegLog.WithName("after-fulfill").Info("after 4") + logr.New(delegLog).WithName("after-fulfill").Info("after 4") By("ensuring that the messages are appropriately named") Expect(root.messages).To(ConsistOf( @@ -179,10 +183,10 @@ var _ = Describe("logging", func() { // Constructing the child in the goroutine does not reliably // trigger the race detector - child := delegLog.WithName("child") + child := logr.New(delegLog).WithName("child") go func() { defer GinkgoRecover() - delegLog.Fulfill(NullLogger{}) + delegLog.Fulfill(NullLogSink{}) close(fulfillDone) }() go func() { @@ -202,12 +206,12 @@ var _ = Describe("logging", func() { }() go func() { defer GinkgoRecover() - delegLog.Enabled() + logr.New(delegLog).Enabled() close(logEnabledDone) }() go func() { defer GinkgoRecover() - delegLog.Info("hello world") + logr.New(delegLog).Info("hello world") close(logInfoDone) }() go func() { @@ -217,7 +221,7 @@ var _ = Describe("logging", func() { }() go func() { defer GinkgoRecover() - delegLog.V(1) + logr.New(delegLog).V(1) close(logVDone) }() @@ -233,12 +237,12 @@ var _ = Describe("logging", func() { It("should delegate with tags", func() { By("asking for a logger with a name before fulfill, and logging") - befFulfill1 := delegLog.WithValues("tag1", "val1") + befFulfill1 := logr.New(delegLog).WithValues("tag1", "val1") befFulfill2 := befFulfill1.WithValues("tag2", "val2") befFulfill1.Info("before fulfill") By("logging on the base logger before fulfill") - delegLog.Info("before fulfill base") + logr.New(delegLog).Info("before fulfill base") By("ensuring that no messages were actually recorded") Expect(root.messages).To(BeEmpty()) @@ -254,7 +258,7 @@ var _ = Describe("logging", func() { befFulfill1.WithValues("tag3", "val3").Info("after 3") By("logging with new loggers") - delegLog.WithValues("tag3", "val3").Info("after 4") + logr.New(delegLog).WithValues("tag3", "val3").Info("after 4") By("ensuring that the messages are appropriately named") Expect(root.messages).To(ConsistOf( @@ -270,13 +274,13 @@ var _ = Describe("logging", func() { delegLog.Fulfill(baseLog) By("logging a bit") - delegLog.Info("msg 1") + logr.New(delegLog).Info("msg 1") By("fulfilling with a new logger") delegLog.Fulfill(&fakeLogger{}) By("logging some more") - delegLog.Info("msg 2") + logr.New(delegLog).Info("msg 2") By("checking that all log messages are present") Expect(root.messages).To(ConsistOf( @@ -296,7 +300,7 @@ var _ = Describe("logging", func() { root := &fakeLoggerRoot{} baseLog := &fakeLogger{root: root} - wantLog := baseLog.WithName("my-logger") + wantLog := logr.New(baseLog).WithName("my-logger") ctx := IntoContext(context.Background(), wantLog) gotLog := FromContext(ctx) @@ -312,7 +316,7 @@ var _ = Describe("logging", func() { root := &fakeLoggerRoot{} baseLog := &fakeLogger{root: root} - wantLog := baseLog.WithName("my-logger") + wantLog := logr.New(baseLog).WithName("my-logger") ctx := IntoContext(context.Background(), wantLog) gotLog := FromContext(ctx, "tag1", "value1") diff --git a/pkg/log/null.go b/pkg/log/null.go index 09a5a02eb6..e3eb01488a 100644 --- a/pkg/log/null.go +++ b/pkg/log/null.go @@ -24,37 +24,36 @@ import ( // but avoids accidentally adding the testing flags to // all binaries. -// NullLogger is a logr.Logger that does nothing. -type NullLogger struct{} +// NullLogSink is a logr.Logger that does nothing. +type NullLogSink struct{} -var _ logr.Logger = NullLogger{} +var _ logr.LogSink = NullLogSink{} -// Info implements logr.InfoLogger. -func (NullLogger) Info(_ string, _ ...interface{}) { +// Init implements logr.LogSink. +func (log NullLogSink) Init(_ logr.RuntimeInfo) { +} + +// Info implements logr.LogSink. +func (NullLogSink) Info(_ int, _ string, _ ...interface{}) { // Do nothing. } -// Enabled implements logr.InfoLogger. -func (NullLogger) Enabled() bool { +// Enabled implements logr.LogSink. +func (NullLogSink) Enabled(_ int) bool { return false } -// Error implements logr.Logger. -func (NullLogger) Error(_ error, _ string, _ ...interface{}) { +// Error implements logr.LogSink. +func (NullLogSink) Error(_ error, _ string, _ ...interface{}) { // Do nothing. } -// V implements logr.Logger. -func (log NullLogger) V(_ int) logr.Logger { - return log -} - -// WithName implements logr.Logger. -func (log NullLogger) WithName(_ string) logr.Logger { +// WithName implements logr.LogSink. +func (log NullLogSink) WithName(_ string) logr.LogSink { return log } -// WithValues implements logr.Logger. -func (log NullLogger) WithValues(_ ...interface{}) logr.Logger { +// WithValues implements logr.LogSink. +func (log NullLogSink) WithValues(_ ...interface{}) logr.LogSink { return log } diff --git a/pkg/log/zap/zap_test.go b/pkg/log/zap/zap_test.go index 095fc54598..11f38ce7e3 100644 --- a/pkg/log/zap/zap_test.go +++ b/pkg/log/zap/zap_test.go @@ -61,7 +61,7 @@ type fakeLoggerRoot struct { messages []logInfo } -var _ logr.Logger = &fakeLogger{} +var _ logr.LogSink = &fakeLogger{} // fakeLogger is a fake implementation of logr.Logger that records // messages, tags, and names, @@ -73,7 +73,14 @@ type fakeLogger struct { root *fakeLoggerRoot } -func (f *fakeLogger) WithName(name string) logr.Logger { +func (f *fakeLogger) Init(info logr.RuntimeInfo) { +} + +func (f *fakeLogger) Enabled(level int) bool { + return true +} + +func (f *fakeLogger) WithName(name string) logr.LogSink { names := append([]string(nil), f.name...) names = append(names, name) return &fakeLogger{ @@ -83,7 +90,7 @@ func (f *fakeLogger) WithName(name string) logr.Logger { } } -func (f *fakeLogger) WithValues(vals ...interface{}) logr.Logger { +func (f *fakeLogger) WithValues(vals ...interface{}) logr.LogSink { tags := append([]interface{}(nil), f.tags...) tags = append(tags, vals...) return &fakeLogger{ @@ -104,7 +111,7 @@ func (f *fakeLogger) Error(err error, msg string, vals ...interface{}) { }) } -func (f *fakeLogger) Info(msg string, vals ...interface{}) { +func (f *fakeLogger) Info(level int, msg string, vals ...interface{}) { tags := append([]interface{}(nil), f.tags...) tags = append(tags, vals...) f.root.messages = append(f.root.messages, logInfo{ @@ -114,9 +121,6 @@ func (f *fakeLogger) Info(msg string, vals ...interface{}) { }) } -func (f *fakeLogger) Enabled() bool { return true } -func (f *fakeLogger) V(lvl int) logr.Logger { return f } - var _ = Describe("Zap options setup", func() { var opts *Options @@ -535,9 +539,10 @@ var _ = Describe("Zap log level flag options setup", func() { logOut.Truncate(0) logger.V(4).Info("test 4") // Should not be logged Expect(logOut.String()).To(BeEmpty()) - logger.V(-3).Info("test -3") // Log a panic, since V(-1*N) for all N > 0 is not permitted. - Expect(logOut.String()).To(ContainSubstring(`"level":"dpanic"`)) + logger.V(-3).Info("test -3") // Since logr v1.0.0, negative logr level will be force to 0 https://github.com/go-logr/logr/blob/master/logr.go#L269-L279、. + Expect(logOut.String()).To(ContainSubstring("test -3")) }) + It("does not log with positive logr level", func() { By("setting up the logger") logger := New(WriteTo(logOut), Level(zapcore.Level(1))) diff --git a/pkg/manager/manager.go b/pkg/manager/manager.go index 2d2733f0a6..d8c8e01c51 100644 --- a/pkg/manager/manager.go +++ b/pkg/manager/manager.go @@ -571,7 +571,7 @@ func setOptionsDefaults(options Options) Options { options.GracefulShutdownTimeout = &gracefulShutdownTimeout } - if options.Logger == nil { + if options.Logger.GetSink() == nil { options.Logger = log.Log } diff --git a/pkg/webhook/admission/webhook.go b/pkg/webhook/admission/webhook.go index cf7dbcf68d..3dcff5fadd 100644 --- a/pkg/webhook/admission/webhook.go +++ b/pkg/webhook/admission/webhook.go @@ -243,7 +243,7 @@ func StandaloneWebhook(hook *Webhook, opts StandaloneOptions) (http.Handler, err return nil, err } - if opts.Logger == nil { + if opts.Logger.GetSink() == nil { opts.Logger = logf.RuntimeLog.WithName("webhook") } hook.log = opts.Logger