Replace Nest in KeyValue interface with LogMarshalerFunc type (uber-go#91)

This reduces the KeyValue interface while still allowing a function
to be used to build an object using LogMarshalerFunc

Author: prashantv

field_test.go

@@ -163,6 +163,11 @@ func TestNestField(t *testing.T) {
 	assertCanBeReused(t, nest)
 }
 
+func TestLogMarshalerFunc(t *testing.T) {
+	assertFieldJSON(t, `"foo":{"name":"phil"}`,
+		Marshaler("foo", LogMarshalerFunc(fakeUser{"phil"}.MarshalLog)))
+}
+
 func TestStackField(t *testing.T) {
 	enc := newJSONEncoder()
 	defer enc.Free()

json_encoder.go

@@ -117,22 +117,15 @@ func (enc *jsonEncoder) AddFloat64(key string, val float64) {
 	}
 }
 
-// Nest allows the caller to populate a nested object under the provided key.
-func (enc *jsonEncoder) Nest(key string, f func(KeyValue) error) error {
-	enc.addKey(key)
-	enc.bytes = append(enc.bytes, '{')
-	err := f(enc)
-	enc.bytes = append(enc.bytes, '}')
-	return err
-}
-
 // AddMarshaler adds a LogMarshaler to the encoder's fields.
 //
 // TODO: Encode the error into the message instead of returning.
 func (enc *jsonEncoder) AddMarshaler(key string, obj LogMarshaler) error {
-	return enc.Nest(key, func(kv KeyValue) error {
-		return obj.MarshalLog(kv)
-	})
+	enc.addKey(key)
+	enc.bytes = append(enc.bytes, '{')
+	err := obj.MarshalLog(enc)
+	enc.bytes = append(enc.bytes, '}')
+	return err
 }
 
 // AddObject uses reflection to add an arbitrary object to the logging context.

json_encoder_bench_test.go

@@ -34,15 +34,20 @@ type logRecord struct {
 	Fields  map[string]interface{} `json:"fields"`
 }
 
-var s string
+// newEncoder returns the encoder interface, which is how end users would use
+// the LogMarshalerFunc type. Using the JSON encoder type directly allows
+// inlining which reduces allocs artificially.
+func newEncoder() encoder {
+	return newJSONEncoder()
+}
 
-func BenchmarkJSONEncoderNest(b *testing.B) {
+func BenchmarkLogMarshalerFunc(b *testing.B) {
 	for i := 0; i < b.N; i++ {
-		enc := newJSONEncoder()
-		enc.Nest("nested", func(kv KeyValue) error {
+		enc := newEncoder()
+		enc.AddMarshaler("nested", LogMarshalerFunc(func(kv KeyValue) error {
 			kv.AddInt("i", i)
 			return nil
-		})
+		}))
 		enc.Free()
 	}
 }

json_encoder_test.go

@@ -149,19 +149,6 @@ func TestJSONWriteMessage(t *testing.T) {
 	})
 }
 
-func TestJSONNest(t *testing.T) {
-	withJSONEncoder(func(enc *jsonEncoder) {
-		err := enc.Nest("nested", func(kv KeyValue) error {
-			kv.AddString("sub-foo", "sub-bar")
-			return nil
-		})
-		require.NoError(t, err, "Unexpected error using Nest.")
-		enc.AddString("baz", "bing")
-
-		assertJSON(t, `"foo":"bar","nested":{"sub-foo":"sub-bar"},"baz":"bing"`, enc)
-	})
-}
-
 type loggable struct{}
 
 func (l loggable) MarshalLog(kv KeyValue) error {

keyvalue.go

@@ -35,5 +35,4 @@ type KeyValue interface {
 	// allocation-heavy. Consider implementing the LogMarshaler interface instead.
 	AddObject(key string, value interface{})
 	AddString(key, value string)
-	Nest(key string, f func(KeyValue) error) error
 }

marshaler.go

@@ -26,3 +26,12 @@ package zap
 type LogMarshaler interface {
 	MarshalLog(KeyValue) error
 }
+
+// LogMarshalerFunc is a type adapter that allows using a function as a
+// LogMarshaler.
+type LogMarshalerFunc func(KeyValue) error
+
+// MarshalLog calls the underlying function.
+func (f LogMarshalerFunc) MarshalLog(kv KeyValue) error {
+	return f(kv)
+}