From 45dfb4994675cfb4215dd3b1afacdadeb5f69e22 Mon Sep 17 00:00:00 2001 From: Hyeonki Hong Date: Mon, 3 Jul 2023 17:04:50 +0900 Subject: [PATCH 1/5] feat(field): add zap.Dict --- field.go | 6 ++++++ field_test.go | 2 ++ zapcore/field.go | 10 +++++++++- zapcore/field_test.go | 11 +++++++++++ 4 files changed, 28 insertions(+), 1 deletion(-) diff --git a/field.go b/field.go index bbb745db5..a7f73ed5b 100644 --- a/field.go +++ b/field.go @@ -410,6 +410,10 @@ func Inline(val zapcore.ObjectMarshaler) Field { } } +func Dict(key string, val []Field) Field { + return Field{Key: key, Type: zapcore.DictType, Interface: val} +} + // Any takes a key and an arbitrary value and chooses the best way to represent // them as a field, falling back to a reflection-based approach only if // necessary. @@ -423,6 +427,8 @@ func Any(key string, value interface{}) Field { return Object(key, val) case zapcore.ArrayMarshaler: return Array(key, val) + case []Field: + return Dict(key, val) case bool: return Bool(key, val) case *bool: diff --git a/field_test.go b/field_test.go index 5b49eb2f0..407a0a5e9 100644 --- a/field_test.go +++ b/field_test.go @@ -124,8 +124,10 @@ func TestFieldConstructors(t *testing.T) { {"Stringer", Field{Key: "k", Type: zapcore.StringerType, Interface: addr}, Stringer("k", addr)}, {"Object", Field{Key: "k", Type: zapcore.ObjectMarshalerType, Interface: name}, Object("k", name)}, {"Inline", Field{Type: zapcore.InlineMarshalerType, Interface: name}, Inline(name)}, + {"Dict", Field{Key: "k", Type: zapcore.DictType, Interface: []Field{String("k", "v")}}, Dict("k", []Field{String("k", "v")})}, {"Any:ObjectMarshaler", Any("k", name), Object("k", name)}, {"Any:ArrayMarshaler", Any("k", bools([]bool{true})), Array("k", bools([]bool{true}))}, + {"Any:Dict", Any("k", []Field{String("k", "v")}), Dict("k", []Field{String("k", "v")})}, {"Any:Stringer", Any("k", addr), Stringer("k", addr)}, {"Any:Bool", Any("k", true), Bool("k", true)}, {"Any:Bools", Any("k", []bool{true}), Bools("k", []bool{true})}, diff --git a/zapcore/field.go b/zapcore/field.go index 95bdb0a12..81ae01f86 100644 --- a/zapcore/field.go +++ b/zapcore/field.go @@ -96,6 +96,9 @@ const ( // InlineMarshalerType indicates that the field carries an ObjectMarshaler // that should be inlined. InlineMarshalerType + + // DictType indicates that the field carries []Field. + DictType ) // A Field is a marshaling operation used to add a key-value pair to a logger's @@ -121,6 +124,11 @@ func (f Field) AddTo(enc ObjectEncoder) { err = enc.AddObject(f.Key, f.Interface.(ObjectMarshaler)) case InlineMarshalerType: err = f.Interface.(ObjectMarshaler).MarshalLogObject(enc) + case DictType: + err = enc.AddObject(f.Key, ObjectMarshalerFunc(func(inner ObjectEncoder) error { + addFields(inner, f.Interface.([]Field)) + return nil + })) case BinaryType: enc.AddBinary(f.Key, f.Interface.([]byte)) case BoolType: @@ -198,7 +206,7 @@ func (f Field) Equals(other Field) bool { switch f.Type { case BinaryType, ByteStringType: return bytes.Equal(f.Interface.([]byte), other.Interface.([]byte)) - case ArrayMarshalerType, ObjectMarshalerType, ErrorType, ReflectType: + case ArrayMarshalerType, ObjectMarshalerType, ErrorType, ReflectType, DictType: return reflect.DeepEqual(f.Interface, other.Interface) default: return f == other diff --git a/zapcore/field_test.go b/zapcore/field_test.go index c4363297c..2f0e2d93e 100644 --- a/zapcore/field_test.go +++ b/zapcore/field_test.go @@ -137,6 +137,7 @@ func TestFields(t *testing.T) { }{ {t: ArrayMarshalerType, iface: users(2), want: []interface{}{"user", "user"}}, {t: ObjectMarshalerType, iface: users(2), want: map[string]interface{}{"users": 2}}, + {t: DictType, iface: []Field{{Type: StringType, Key: "k", String: "v"}}, want: map[string]interface{}{"k": "v"}}, {t: BoolType, i: 0, want: false}, {t: ByteStringType, iface: []byte("foo"), want: "foo"}, {t: Complex128Type, iface: 1 + 2i, want: 1 + 2i}, @@ -309,6 +310,16 @@ func TestEquals(t *testing.T) { b: zap.Any("k", map[string]string{"a": "d"}), want: false, }, + { + a: zap.Dict("k", []Field{zap.String("a", "b")}), + b: zap.Dict("k", []Field{zap.String("a", "b")}), + want: true, + }, + { + a: zap.Dict("k", []Field{zap.String("a", "b")}), + b: zap.Dict("k", []Field{zap.String("a", "d")}), + want: false, + }, } for _, tt := range tests { From 9a315d09c3558071754b695c2fb6c9387d9f368d Mon Sep 17 00:00:00 2001 From: Hyeonki Hong Date: Mon, 3 Jul 2023 17:37:06 +0900 Subject: [PATCH 2/5] docs(field): add comments to zap.Dict --- field.go | 1 + 1 file changed, 1 insertion(+) diff --git a/field.go b/field.go index a7f73ed5b..bb8efb00c 100644 --- a/field.go +++ b/field.go @@ -410,6 +410,7 @@ func Inline(val zapcore.ObjectMarshaler) Field { } } +// Dict constructs a field with the given key and []Field. func Dict(key string, val []Field) Field { return Field{Key: key, Type: zapcore.DictType, Interface: val} } From 2c7834cad188f0e80842c48ea6093d558073062f Mon Sep 17 00:00:00 2001 From: Hyeonki Hong Date: Mon, 3 Jul 2023 19:26:23 +0900 Subject: [PATCH 3/5] feat(field): use ellipsis parameter instead of slice --- field.go | 6 +++--- field_test.go | 4 ++-- zapcore/field_test.go | 8 ++++---- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/field.go b/field.go index bb8efb00c..430670f6d 100644 --- a/field.go +++ b/field.go @@ -410,8 +410,8 @@ func Inline(val zapcore.ObjectMarshaler) Field { } } -// Dict constructs a field with the given key and []Field. -func Dict(key string, val []Field) Field { +// Dict constructs a field with the given key and Fields. +func Dict(key string, val ...Field) Field { return Field{Key: key, Type: zapcore.DictType, Interface: val} } @@ -429,7 +429,7 @@ func Any(key string, value interface{}) Field { case zapcore.ArrayMarshaler: return Array(key, val) case []Field: - return Dict(key, val) + return Dict(key, val...) case bool: return Bool(key, val) case *bool: diff --git a/field_test.go b/field_test.go index 407a0a5e9..d1dc44bc4 100644 --- a/field_test.go +++ b/field_test.go @@ -124,10 +124,10 @@ func TestFieldConstructors(t *testing.T) { {"Stringer", Field{Key: "k", Type: zapcore.StringerType, Interface: addr}, Stringer("k", addr)}, {"Object", Field{Key: "k", Type: zapcore.ObjectMarshalerType, Interface: name}, Object("k", name)}, {"Inline", Field{Type: zapcore.InlineMarshalerType, Interface: name}, Inline(name)}, - {"Dict", Field{Key: "k", Type: zapcore.DictType, Interface: []Field{String("k", "v")}}, Dict("k", []Field{String("k", "v")})}, + {"Dict", Field{Key: "k", Type: zapcore.DictType, Interface: []Field{String("k", "v")}}, Dict("k", String("k", "v"))}, {"Any:ObjectMarshaler", Any("k", name), Object("k", name)}, {"Any:ArrayMarshaler", Any("k", bools([]bool{true})), Array("k", bools([]bool{true}))}, - {"Any:Dict", Any("k", []Field{String("k", "v")}), Dict("k", []Field{String("k", "v")})}, + {"Any:Dict", Any("k", []Field{String("k", "v")}), Dict("k", String("k", "v"))}, {"Any:Stringer", Any("k", addr), Stringer("k", addr)}, {"Any:Bool", Any("k", true), Bool("k", true)}, {"Any:Bools", Any("k", []bool{true}), Bools("k", []bool{true})}, diff --git a/zapcore/field_test.go b/zapcore/field_test.go index 2f0e2d93e..7e76ea987 100644 --- a/zapcore/field_test.go +++ b/zapcore/field_test.go @@ -311,13 +311,13 @@ func TestEquals(t *testing.T) { want: false, }, { - a: zap.Dict("k", []Field{zap.String("a", "b")}), - b: zap.Dict("k", []Field{zap.String("a", "b")}), + a: zap.Dict("k", zap.String("a", "b")), + b: zap.Dict("k", zap.String("a", "b")), want: true, }, { - a: zap.Dict("k", []Field{zap.String("a", "b")}), - b: zap.Dict("k", []Field{zap.String("a", "d")}), + a: zap.Dict("k", zap.String("a", "b")), + b: zap.Dict("k", zap.String("a", "d")), want: false, }, } From d0bde18faf9e4a57538ac218279a399a55887abc Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Mon, 14 Aug 2023 05:58:25 -0700 Subject: [PATCH 4/5] Implement without new field type This functionality doesn't require a new field type and can be implemented in the Zap package itself as a new ObjectMarshaler. --- field.go | 11 ++++++++++- field_test.go | 25 ++++++++++++++++++++++++- zapcore/field.go | 10 +--------- zapcore/field_test.go | 1 - 4 files changed, 35 insertions(+), 12 deletions(-) diff --git a/field.go b/field.go index a32349f13..c34ad63f6 100644 --- a/field.go +++ b/field.go @@ -417,7 +417,16 @@ func Dict(key string, val ...Field) Field { // We need a function with the signature (string, T) for zap.Any. func dictField(key string, val []Field) Field { - return Field{Key: key, Type: zapcore.DictType, Interface: val} + return Object(key, dictObject(val)) +} + +type dictObject []Field + +func (d dictObject) MarshalLogObject(enc zapcore.ObjectEncoder) error { + for _, f := range d { + f.AddTo(enc) + } + return nil } // We discovered an issue where zap.Any can cause a performance degradation diff --git a/field_test.go b/field_test.go index 3b8b3d2ab..bbb9f79f9 100644 --- a/field_test.go +++ b/field_test.go @@ -125,7 +125,6 @@ func TestFieldConstructors(t *testing.T) { {"Stringer", Field{Key: "k", Type: zapcore.StringerType, Interface: addr}, Stringer("k", addr)}, {"Object", Field{Key: "k", Type: zapcore.ObjectMarshalerType, Interface: name}, Object("k", name)}, {"Inline", Field{Type: zapcore.InlineMarshalerType, Interface: name}, Inline(name)}, - {"Dict", Field{Key: "k", Type: zapcore.DictType, Interface: []Field{String("k", "v")}}, Dict("k", String("k", "v"))}, {"Any:ObjectMarshaler", Any("k", name), Object("k", name)}, {"Any:ArrayMarshaler", Any("k", bools([]bool{true})), Array("k", bools([]bool{true}))}, {"Any:Dict", Any("k", []Field{String("k", "v")}), Dict("k", String("k", "v"))}, @@ -290,3 +289,27 @@ func TestStackSkipFieldWithSkip(t *testing.T) { assert.Equal(t, takeStacktrace(1), f.String, "Unexpected stack trace") assertCanBeReused(t, f) } + +func TestDict(t *testing.T) { + tests := []struct { + desc string + field Field + expected any + }{ + {"empty", Dict(""), map[string]any{}}, + {"single", Dict("", String("k", "v")), map[string]any{"k": "v"}}, + {"multiple", Dict("", String("k", "v"), String("k2", "v2")), map[string]any{"k": "v", "k2": "v2"}}, + } + + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + enc := zapcore.NewMapObjectEncoder() + tt.field.Key = "k" + tt.field.AddTo(enc) + assert.Equal(t, tt.expected, enc.Fields["k"], "unexpected map contents") + assert.Len(t, enc.Fields, 1, "found extra keys in map: %v", enc.Fields) + + assertCanBeReused(t, tt.field) + }) + } +} diff --git a/zapcore/field.go b/zapcore/field.go index 81ae01f86..95bdb0a12 100644 --- a/zapcore/field.go +++ b/zapcore/field.go @@ -96,9 +96,6 @@ const ( // InlineMarshalerType indicates that the field carries an ObjectMarshaler // that should be inlined. InlineMarshalerType - - // DictType indicates that the field carries []Field. - DictType ) // A Field is a marshaling operation used to add a key-value pair to a logger's @@ -124,11 +121,6 @@ func (f Field) AddTo(enc ObjectEncoder) { err = enc.AddObject(f.Key, f.Interface.(ObjectMarshaler)) case InlineMarshalerType: err = f.Interface.(ObjectMarshaler).MarshalLogObject(enc) - case DictType: - err = enc.AddObject(f.Key, ObjectMarshalerFunc(func(inner ObjectEncoder) error { - addFields(inner, f.Interface.([]Field)) - return nil - })) case BinaryType: enc.AddBinary(f.Key, f.Interface.([]byte)) case BoolType: @@ -206,7 +198,7 @@ func (f Field) Equals(other Field) bool { switch f.Type { case BinaryType, ByteStringType: return bytes.Equal(f.Interface.([]byte), other.Interface.([]byte)) - case ArrayMarshalerType, ObjectMarshalerType, ErrorType, ReflectType, DictType: + case ArrayMarshalerType, ObjectMarshalerType, ErrorType, ReflectType: return reflect.DeepEqual(f.Interface, other.Interface) default: return f == other diff --git a/zapcore/field_test.go b/zapcore/field_test.go index 7e76ea987..a0c1e16d3 100644 --- a/zapcore/field_test.go +++ b/zapcore/field_test.go @@ -137,7 +137,6 @@ func TestFields(t *testing.T) { }{ {t: ArrayMarshalerType, iface: users(2), want: []interface{}{"user", "user"}}, {t: ObjectMarshalerType, iface: users(2), want: map[string]interface{}{"users": 2}}, - {t: DictType, iface: []Field{{Type: StringType, Key: "k", String: "v"}}, want: map[string]interface{}{"k": "v"}}, {t: BoolType, i: 0, want: false}, {t: ByteStringType, iface: []byte("foo"), want: "foo"}, {t: Complex128Type, iface: 1 + 2i, want: 1 + 2i}, From c19628d973824eb1fa66f5b5ae3218861375e2e3 Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Mon, 14 Aug 2023 06:04:43 -0700 Subject: [PATCH 5/5] doc: Clarify documentation, add example test --- example_test.go | 13 +++++++++++++ field.go | 3 ++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/example_test.go b/example_test.go index 327e3ef39..f5c79ebc0 100644 --- a/example_test.go +++ b/example_test.go @@ -358,3 +358,16 @@ func ExampleWrapCore_wrap() { // {"level":"info","msg":"doubled"} // {"level":"info","msg":"doubled"} } + +func ExampleDict() { + logger := zap.NewExample() + defer logger.Sync() + + logger.Info("login event", + zap.Dict("event", + zap.Int("id", 123), + zap.String("name", "jane"), + zap.String("status", "pending"))) + // Output: + // {"level":"info","msg":"login event","event":{"id":123,"name":"jane","status":"pending"}} +} diff --git a/field.go b/field.go index c34ad63f6..f40fa7bf7 100644 --- a/field.go +++ b/field.go @@ -410,7 +410,8 @@ func Inline(val zapcore.ObjectMarshaler) Field { } } -// Dict constructs a field with the given key and Fields. +// Dict constructs a field containing the provided key-value pairs. +// It acts similar to [Object], but with the fields specified as arguments. func Dict(key string, val ...Field) Field { return dictField(key, val) }