make Discard logger equal to null logger

discard.go

@@ -20,35 +20,5 @@ package logr
 // used whenever the caller is not interested in the logs.  Logger instances
 // produced by this function always compare as equal.
 func Discard() Logger {
-	return Logger{
-		level: 0,
-		sink:  discardLogSink{},
-	}
-}
-
-// discardLogSink is a LogSink that discards all messages.
-type discardLogSink struct{}
-
-// Verify that it actually implements the interface
-var _ LogSink = discardLogSink{}
-
-func (l discardLogSink) Init(RuntimeInfo) {
-}
-
-func (l discardLogSink) Enabled(int) bool {
-	return false
-}
-
-func (l discardLogSink) Info(int, string, ...interface{}) {
-}
-
-func (l discardLogSink) Error(error, string, ...interface{}) {
-}
-
-func (l discardLogSink) WithValues(...interface{}) LogSink {
-	return l
-}
-
-func (l discardLogSink) WithName(string) LogSink {
-	return l
+	return New(nil)
 }

discard_test.go

@@ -24,26 +24,31 @@ import (
 
 func TestDiscard(t *testing.T) {
 	l := Discard()
-	if _, ok := l.GetSink().(discardLogSink); !ok {
+	if l.GetSink() != nil {
 		t.Error("did not return the expected underlying type")
 	}
 	// Verify that none of the methods panic, there is not more we can test.
 	l.WithName("discard").WithValues("z", 5).Info("Hello world")
 	l.Info("Hello world", "x", 1, "y", 2)
 	l.V(1).Error(errors.New("foo"), "a", 123)
 	if l.Enabled() {
-		t.Error("discardLogSink must always say it is disabled")
+		t.Error("discard loggers must always be disabled")
 	}
 }
 
 func TestComparable(t *testing.T) {
 	a := Discard()
 	if !reflect.TypeOf(a).Comparable() {
-		t.Fatal("discardLogSink must be comparable")
+		t.Fatal("discard loggers must be comparable")
 	}
 
 	b := Discard()
 	if a != b {
-		t.Fatal("any two discardLogSink must be equal")
+		t.Fatal("any two discard Loggers must be equal")
+	}
+
+	c := Discard().V(2)
+	if b != c {
+		t.Fatal("any two discard Loggers must be equal")
 	}
 }

logr.go

@@ -212,11 +212,14 @@ import (
 )
 
 // New returns a new Logger instance.  This is primarily used by libraries
-// implementing LogSink, rather than end users.
+// implementing LogSink, rather than end users.  Passing a nil sink will create
+// a Logger which discards all log lines.
 func New(sink LogSink) Logger {
 	logger := Logger{}
 	logger.setSink(sink)
-	sink.Init(runtimeInfo)
+	if sink != nil {
+		sink.Init(runtimeInfo)
+	}
 	return logger
 }
 
@@ -265,6 +268,9 @@ func (l Logger) Enabled() bool {
 // information.  The key/value pairs must alternate string keys and arbitrary
 // values.
 func (l Logger) Info(msg string, keysAndValues ...interface{}) {
+	if l.sink == nil {
+		return
+	}
 	if l.Enabled() {
 		if withHelper, ok := l.sink.(CallStackHelperLogSink); ok {
 			withHelper.GetCallStackHelper()()
@@ -298,6 +304,9 @@ func (l Logger) Error(err error, msg string, keysAndValues ...interface{}) {
 // level means a log message is less important.  Negative V-levels are treated
 // as 0.
 func (l Logger) V(level int) Logger {
+	if l.sink == nil {
+		return l
+	}
 	if level < 0 {
 		level = 0
 	}
@@ -344,6 +353,9 @@ func (l Logger) WithName(name string) Logger {
 // WithCallDepth(1) because it works with implementions that support the
 // CallDepthLogSink and/or CallStackHelperLogSink interfaces.
 func (l Logger) WithCallDepth(depth int) Logger {
+	if l.sink == nil {
+		return l
+	}
 	if withCallDepth, ok := l.sink.(CallDepthLogSink); ok {
 		l.setSink(withCallDepth.WithCallDepth(depth))
 	}
@@ -365,6 +377,9 @@ func (l Logger) WithCallDepth(depth int) Logger {
 // implementation does not support either of these, the original Logger will be
 // returned.
 func (l Logger) WithCallStackHelper() (func(), Logger) {
+	if l.sink == nil {
+		return func() {}, l
+	}
 	var helper func()
 	if withCallDepth, ok := l.sink.(CallDepthLogSink); ok {
 		l.setSink(withCallDepth.WithCallDepth(1))

logr_test.go

@@ -384,8 +384,8 @@ func TestContext(t *testing.T) {
 	}
 
 	out := FromContextOrDiscard(ctx)
-	if _, ok := out.sink.(discardLogSink); !ok {
-		t.Errorf("expected a discardLogSink, got %#v", out)
+	if out.sink != nil {
+		t.Errorf("expected a nil sink, got %#v", out)
 	}
 
 	sink := &testLogSink{}
@@ -412,11 +412,10 @@ func TestIsZero(t *testing.T) {
 	if l.IsZero() {
 		t.Errorf("expected not IsZero")
 	}
-	// Discard is not considered a zero unset logger, but an intentional choice
-	// to ignore messages that should not be overridden by a component.
+	// Discard is the same as a nil sink.
 	l = Discard()
-	if l.IsZero() {
-		t.Errorf("expected not IsZero")
+	if !l.IsZero() {
+		t.Errorf("expected IsZero")
 	}
 }