Merge pull request #325 from pohly/logr-marshaler

support logr.Marshaler

internal/serialize/keyvalues.go

@@ -20,6 +20,8 @@ import (
 	"bytes"
 	"fmt"
 	"strconv"
+
+	"github.com/go-logr/logr"
 )
 
 // WithValues implements LogSink.WithValues. The old key/value pairs are
@@ -176,6 +178,24 @@ func KVListFormat(b *bytes.Buffer, keysAndValues ...interface{}) {
 			writeStringValue(b, true, v)
 		case error:
 			writeStringValue(b, true, ErrorToString(v))
+		case logr.Marshaler:
+			value := MarshalerToValue(v)
+			// A marshaler that returns a string is useful for
+			// delayed formatting of complex values. We treat this
+			// case like a normal string. This is useful for
+			// multi-line support.
+			//
+			// We could do this by recursively formatting a value,
+			// but that comes with the risk of infinite recursion
+			// if a marshaler returns itself. Instead we call it
+			// only once and rely on it returning the intended
+			// value directly.
+			switch value := value.(type) {
+			case string:
+				writeStringValue(b, true, value)
+			default:
+				writeStringValue(b, false, fmt.Sprintf("%+v", v))
+			}
 		case []byte:
 			// In https://github.com/kubernetes/klog/pull/237 it was decided
 			// to format byte slices with "%+q". The advantages of that are:
@@ -208,6 +228,18 @@ func StringerToString(s fmt.Stringer) (ret string) {
 	return
 }
 
+// MarshalerToValue invokes a marshaler and catches
+// panics.
+func MarshalerToValue(m logr.Marshaler) (ret interface{}) {
+	defer func() {
+		if err := recover(); err != nil {
+			ret = fmt.Sprintf("<panic: %s>", err)
+		}
+	}()
+	ret = m.MarshalLog()
+	return
+}
+
 // ErrorToString converts an error to a string,
 // handling panics if they occur.
 func ErrorToString(err error) (ret string) {

test/output.go

@@ -358,7 +358,13 @@ I output.go:<LINE>] "test" firstKey=1 secondKey=3
 		"MarshalLog() that panics": {
 			text:   "marshaler panic",
 			values: []interface{}{"obj", faultyMarshaler{}},
-			expectedOutput: `I output.go:<LINE>] "marshaler panic" obj={}
+			expectedOutput: `I output.go:<LINE>] "marshaler panic" obj="<panic: fake MarshalLog panic>"
+`,
+		},
+		"MarshalLog() that returns itself": {
+			text:   "marshaler recursion",
+			values: []interface{}{"obj", recursiveMarshaler{}},
+			expectedOutput: `I output.go:<LINE>] "marshaler recursion" obj={}
 `,
 		},
 	}
@@ -802,6 +808,15 @@ func (f faultyMarshaler) MarshalLog() interface{} {
 
 var _ logr.Marshaler = faultyMarshaler{}
 
+type recursiveMarshaler struct{}
+
+// MarshalLog returns itself, which could cause the logger to recurse infinitely.
+func (r recursiveMarshaler) MarshalLog() interface{} {
+	return r
+}
+
+var _ logr.Marshaler = recursiveMarshaler{}
+
 type faultyError struct{}
 
 // Error always panics.

test/zapr.go

@@ -161,8 +161,12 @@ I output.go:<LINE>] "odd WithValues" keyWithoutValue="(MISSING)"
 `: `{"caller":"test/output.go:<LINE>","msg":"error panic","errError":"PANIC=fake Error panic"}
 `,
 
-		`I output.go:<LINE>] "marshaler panic" obj={}
+		`I output.go:<LINE>] "marshaler panic" obj="<panic: fake MarshalLog panic>"
 `: `{"caller":"test/output.go:<LINE>","msg":"marshaler panic","v":0,"objError":"PANIC=fake MarshalLog panic"}
+`,
+
+		`I output.go:<LINE>] "marshaler recursion" obj={}
+`: `{"caller":"test/output.go:<LINE>","msg":"marshaler recursion","v":0,"obj":{}}
 `,
 
 		// klog.Info