From 81dd8fe28e74b63490a1905df3ca244233aa2785 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Wed, 11 Aug 2021 10:33:09 +0200 Subject: [PATCH] export Logger.Sink This is necessary to support the "breaking class" use cases. One example for that, access to the underlying implementation, gets implemented for funcr and tested with a runable example. --- discard.go | 2 +- discard_test.go | 2 +- funcr/example_test.go | 36 ++++++++++++++++++++++++++++++++ funcr/funcr.go | 13 ++++++++++++ logr.go | 48 +++++++++++++++++++++++++++++++++++-------- logr_test.go | 18 ++++++++-------- 6 files changed, 100 insertions(+), 19 deletions(-) create mode 100644 funcr/example_test.go diff --git a/discard.go b/discard.go index 9d92a38..389f6a3 100644 --- a/discard.go +++ b/discard.go @@ -22,7 +22,7 @@ package logr func Discard() Logger { return Logger{ level: 0, - sink: discardLogSink{}, + Sink: discardLogSink{}, } } diff --git a/discard_test.go b/discard_test.go index 55aff48..89ffb43 100644 --- a/discard_test.go +++ b/discard_test.go @@ -24,7 +24,7 @@ import ( func TestDiscard(t *testing.T) { l := Discard() - if _, ok := l.sink.(discardLogSink); !ok { + if _, ok := l.Sink.(discardLogSink); !ok { t.Error("did not return the expected underlying type") } // Verify that none of the methods panic, there is not more we can test. diff --git a/funcr/example_test.go b/funcr/example_test.go new file mode 100644 index 0000000..ebd8d20 --- /dev/null +++ b/funcr/example_test.go @@ -0,0 +1,36 @@ +/* +Copyright 2021 The logr Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package funcr_test + +import ( + "fmt" + + "github.com/go-logr/logr" + "github.com/go-logr/logr/funcr" +) + +func ExampleUnderlier() { + var log logr.Logger = funcr.New(func(prefix, args string) { + fmt.Println(prefix, args) + }, funcr.Options{}) + + if underlier, ok := log.Sink.(funcr.Underlier); ok { + fn := underlier.GetUnderlying() + fn("hello", "world") + } + // Output: hello world +} diff --git a/funcr/funcr.go b/funcr/funcr.go index c02bb18..261d8ec 100644 --- a/funcr/funcr.go +++ b/funcr/funcr.go @@ -35,6 +35,14 @@ func New(fn func(prefix, args string), opts Options) logr.Logger { return logr.New(newSink(fn, opts)) } +// Underlier exposes access to the underlying logging function. Since +// callers only have a logr.Logger, they have to know which +// implementation is in use, so this interface is less of an +// abstraction and more of a way to test type conversion. +type Underlier interface { + GetUnderlying() func(prefix, args string) +} + func newSink(fn func(prefix, args string), opts Options) logr.LogSink { return &fnlogger{ prefix: "", @@ -79,6 +87,7 @@ type fnlogger struct { // Assert conformance to the interfaces. var _ logr.LogSink = &fnlogger{} var _ logr.CallDepthLogSink = &fnlogger{} +var _ Underlier = &fnlogger{} func flatten(kvList ...interface{}) string { if len(kvList)%2 != 0 { @@ -313,3 +322,7 @@ func (l fnlogger) WithCallDepth(depth int) logr.LogSink { l.depth += depth return &l } + +func (l fnlogger) GetUnderlying() func(prefix, args string) { + return l.write +} diff --git a/logr.go b/logr.go index a2303e6..eecdaf8 100644 --- a/logr.go +++ b/logr.go @@ -161,6 +161,33 @@ limitations under the License. // type Underlier interface { // GetUnderlying() // } +// +// Logger exports the Sink field to enable type assertions like this: +// func DoSomethingWithImpl(log logr.Logger) { +// if underlier, ok := log.Sink(impl.Underlier) { +// implLogger := underlier.GetUnderlying() +// ... +// } +// } +// +// Custom `With*` functions can be implemented by copying the complete +// Logger struct and replacing the Sink field in the copy: +// // WithFooBar changes the foobar parameter in the log sink and returns a new logger with that modified sink. +// // It does nothing for loggers where the sink doesn't support that parameter. +// func WithFoobar(log logr.Logger, foobar int) logr.Logger { +// if foobarLogSink, ok := log.Sink(FoobarSink); ok { +// log.Sink = foobarLogSink.WithFooBar(foobar) +// } +// return log +// } +// +// Don't use New to construct a new Logger with a LogSink retrieved +// from an existing Logger. Source code attribution might not work +// correctly and unexported fields in Logger get lost. +// +// Beware that the same LogSink instance may be shared by different +// logger instances. Calling functions that modify the LogSink will +// affect all of those. package logr import ( @@ -171,7 +198,7 @@ import ( // implementing LogSink, rather than end users. func New(sink LogSink) Logger { logger := Logger{ - sink: sink, + Sink: sink, } if withCallDepth, ok := sink.(CallDepthLogSink); ok { logger.withCallDepth = withCallDepth @@ -185,8 +212,13 @@ func New(sink LogSink) Logger { // to a LogSink. Implementations of LogSink should provide their own // constructors that return Logger, not LogSink. type Logger struct { + // Sink is the underlying logger implementation and normally + // should not be accessed directly. The field gets exported to + // support the implementation of custom extensions (see "Break + // Glass" in the package documentation). + Sink LogSink + level int - sink LogSink withCallDepth CallDepthLogSink } @@ -194,7 +226,7 @@ type Logger struct { // flags might be used to set the logging verbosity and disable some info // logs. func (l Logger) Enabled() bool { - return l.sink.Enabled(l.level) + return l.Sink.Enabled(l.level) } // Info logs a non-error message with the given key/value pairs as context. @@ -205,7 +237,7 @@ func (l Logger) Enabled() bool { // keys and arbitrary values. func (l Logger) Info(msg string, keysAndValues ...interface{}) { if l.Enabled() { - l.sink.Info(l.level, msg, keysAndValues...) + l.Sink.Info(l.level, msg, keysAndValues...) } } @@ -218,7 +250,7 @@ func (l Logger) Info(msg string, keysAndValues ...interface{}) { // while the err argument should be used to attach the actual error that // triggered this log line, if present. func (l Logger) Error(err error, msg string, keysAndValues ...interface{}) { - l.sink.Error(err, msg, keysAndValues...) + l.Sink.Error(err, msg, keysAndValues...) } // V returns a new Logger instance for a specific verbosity level, relative to @@ -236,7 +268,7 @@ func (l Logger) V(level int) Logger { // WithValues returns a new Logger instance with additional key/value pairs. // See Info for documentation on how key/value pairs work. func (l Logger) WithValues(keysAndValues ...interface{}) Logger { - l.sink = l.sink.WithValues(keysAndValues...) + l.Sink = l.Sink.WithValues(keysAndValues...) return l } @@ -246,7 +278,7 @@ func (l Logger) WithValues(keysAndValues ...interface{}) Logger { // contain only letters, digits, and hyphens (see the package documentation for // more information). func (l Logger) WithName(name string) Logger { - l.sink = l.sink.WithName(name) + l.Sink = l.Sink.WithName(name) return l } @@ -265,7 +297,7 @@ func (l Logger) WithCallDepth(depth int) Logger { if l.withCallDepth == nil { return l } - l.sink = l.withCallDepth.WithCallDepth(depth) + l.Sink = l.withCallDepth.WithCallDepth(depth) return l } diff --git a/logr_test.go b/logr_test.go index 8d27537..9b89a20 100644 --- a/logr_test.go +++ b/logr_test.go @@ -103,8 +103,8 @@ func TestNew(t *testing.T) { } logger := New(sink) - if logger.sink == nil { - t.Errorf("expected sink to be set, got %v", logger.sink) + if logger.Sink == nil { + t.Errorf("expected sink to be set, got %v", logger.Sink) } if calledInit != 1 { t.Errorf("expected sink.Init() to be called once, got %d", calledInit) @@ -277,7 +277,7 @@ func TestWithValues(t *testing.T) { if calledWithValues != 1 { t.Errorf("expected sink.WithValues() to be called once, got %d", calledWithValues) } - if p := out.sink.(*testLogSink); p == sink { + if p := out.Sink.(*testLogSink); p == sink { t.Errorf("expected output to be different from input, got in=%p, out=%p", sink, p) } } @@ -299,7 +299,7 @@ func TestWithName(t *testing.T) { if calledWithName != 1 { t.Errorf("expected sink.WithName() to be called once, got %d", calledWithName) } - if p := out.sink.(*testLogSink); p == sink { + if p := out.Sink.(*testLogSink); p == sink { t.Errorf("expected output to be different from input, got in=%p, out=%p", sink, p) } } @@ -311,7 +311,7 @@ func TestWithCallDepthNotImplemented(t *testing.T) { logger := New(sink) out := logger.WithCallDepth(depthInput) - if p := out.sink.(*testLogSink); p != sink { + if p := out.Sink.(*testLogSink); p != sink { t.Errorf("expected output to be the same as input, got in=%p, out=%p", sink, p) } } @@ -333,7 +333,7 @@ func TestWithCallDepthImplemented(t *testing.T) { if calledWithCallDepth != 1 { t.Errorf("expected sink.WithCallDepth() to be called once, got %d", calledWithCallDepth) } - if p := out.sink.(*testCallDepthLogSink); p == sink { + if p := out.Sink.(*testCallDepthLogSink); p == sink { t.Errorf("expected output to be different from input, got in=%p, out=%p", sink, p) } } @@ -348,7 +348,7 @@ func TestContext(t *testing.T) { } out := FromContextOrDiscard(ctx) - if _, ok := out.sink.(discardLogSink); !ok { + if _, ok := out.Sink.(discardLogSink); !ok { t.Errorf("expected a discardLogSink, got %#v", out) } @@ -357,11 +357,11 @@ func TestContext(t *testing.T) { lctx := NewContext(ctx, logger) if out, err := FromContext(lctx); err != nil { t.Errorf("unexpected error: %v", err) - } else if p := out.sink.(*testLogSink); p != sink { + } else if p := out.Sink.(*testLogSink); p != sink { t.Errorf("expected output to be the same as input, got in=%p, out=%p", sink, p) } out = FromContextOrDiscard(lctx) - if p := out.sink.(*testLogSink); p != sink { + if p := out.Sink.(*testLogSink); p != sink { t.Errorf("expected output to be the same as input, got in=%p, out=%p", sink, p) } }