From f9e2251cec7709c80f462b7f8fb8bd06c56acea4 Mon Sep 17 00:00:00 2001 From: Rajat Singh Panwar <55539587+psrajat@users.noreply.github.com> Date: Tue, 14 Dec 2021 20:51:00 +0530 Subject: [PATCH 1/8] feat: add support for custom ReflectType encoder --- zapcore/encoder.go | 4 ++++ zapcore/json_encoder.go | 16 ++++++++++------ zapcore/reflected_encoder.go | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 6 deletions(-) create mode 100644 zapcore/reflected_encoder.go diff --git a/zapcore/encoder.go b/zapcore/encoder.go index ad8712aba..e06b68155 100644 --- a/zapcore/encoder.go +++ b/zapcore/encoder.go @@ -22,6 +22,7 @@ package zapcore import ( "encoding/json" + "io" "time" "go.uber.org/zap/buffer" @@ -331,6 +332,9 @@ type EncoderConfig struct { // Unlike the other primitive type encoders, EncodeName is optional. The // zero value falls back to FullNameEncoder. EncodeName NameEncoder `json:"nameEncoder" yaml:"nameEncoder"` + // Configure the encoder for interface{} type objects + // If not provided, objects are encoded using json.Encoder + NewReflectedEncoder func(io.Writer) ReflectedEncoder `json:"newReflectedEncoder" yaml:"newReflectedEncoder"` // Configures the field separator used by the console encoder. Defaults // to tab. ConsoleSeparator string `json:"consoleSeparator" yaml:"consoleSeparator"` diff --git a/zapcore/json_encoder.go b/zapcore/json_encoder.go index 3e00a6b6e..f38074584 100644 --- a/zapcore/json_encoder.go +++ b/zapcore/json_encoder.go @@ -22,7 +22,7 @@ package zapcore import ( "encoding/base64" - "encoding/json" + "io" "math" "sync" "time" @@ -64,7 +64,7 @@ type jsonEncoder struct { // for encoding generic values by reflection reflectBuf *buffer.Buffer - reflectEnc *json.Encoder + reflectEnc ReflectedEncoder } // NewJSONEncoder creates a fast, low-allocation JSON encoder. The encoder @@ -152,10 +152,14 @@ func (enc *jsonEncoder) AddInt64(key string, val int64) { func (enc *jsonEncoder) resetReflectBuf() { if enc.reflectBuf == nil { enc.reflectBuf = bufferpool.Get() - enc.reflectEnc = json.NewEncoder(enc.reflectBuf) - - // For consistency with our custom JSON encoder. - enc.reflectEnc.SetEscapeHTML(false) + // If no EncoderConfig.NewReflectedEncoder is provided by the user, then use default + var newReflectedEncoder func(io.Writer) ReflectedEncoder + if enc.NewReflectedEncoder == nil { + newReflectedEncoder = GetDefaultReflectedEncoder() + } else { + newReflectedEncoder = enc.NewReflectedEncoder + } + enc.reflectEnc = newReflectedEncoder(enc.reflectBuf) } else { enc.reflectBuf.Reset() } diff --git a/zapcore/reflected_encoder.go b/zapcore/reflected_encoder.go new file mode 100644 index 000000000..5f64c506c --- /dev/null +++ b/zapcore/reflected_encoder.go @@ -0,0 +1,32 @@ +package zapcore + +import ( + "encoding/json" + "io" +) + +// A ReflectedEncoder handles the serialization of Field which have FieldType = ReflectType and writes them to the underlying data stream +// The underlying data stream is provided by zap during initialization. See EncoderConfig.NewReflectedEncoder +type ReflectedEncoder interface { + // Encode encodes and writes to the underlying data stream + Encode(interface{}) error +} + +func GetDefaultReflectedEncoder() func(writer io.Writer) ReflectedEncoder { + return func(writer io.Writer) ReflectedEncoder { + stdJsonEncoder := json.NewEncoder(writer) + // For consistency with our custom JSON encoder. + stdJsonEncoder.SetEscapeHTML(false) + return &stdReflectedEncoder{ + encoder: stdJsonEncoder, + } + } +} + +type stdReflectedEncoder struct { + encoder *json.Encoder +} + +func (enc *stdReflectedEncoder) Encode(obj interface{}) error { + return enc.encoder.Encode(obj) +} From ba0bbf8fdfe740678f1e070080d6ff36ae13140a Mon Sep 17 00:00:00 2001 From: Rajat Singh Panwar <55539587+psrajat@users.noreply.github.com> Date: Tue, 14 Dec 2021 21:38:26 +0530 Subject: [PATCH 2/8] add license --- zapcore/reflected_encoder.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/zapcore/reflected_encoder.go b/zapcore/reflected_encoder.go index 5f64c506c..9253e479d 100644 --- a/zapcore/reflected_encoder.go +++ b/zapcore/reflected_encoder.go @@ -1,3 +1,23 @@ +// Copyright (c) 2016 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + package zapcore import ( From ce7215ac5285c9ba17c46151bf08cf74ec4f0776 Mon Sep 17 00:00:00 2001 From: Rajat Singh Panwar <55539587+psrajat@users.noreply.github.com> Date: Wed, 15 Dec 2021 00:09:34 +0530 Subject: [PATCH 3/8] Add Tests * fix comments * don't export default ReflectedEncoder fn * move newReflectEncoder nil check to NewJsonEncoder() --- zapcore/encoder.go | 2 +- zapcore/json_encoder.go | 14 ++++---- zapcore/json_encoder_test.go | 63 ++++++++++++++++++++++++++++++++++++ zapcore/reflected_encoder.go | 6 ++-- 4 files changed, 73 insertions(+), 12 deletions(-) diff --git a/zapcore/encoder.go b/zapcore/encoder.go index e06b68155..71710a2a0 100644 --- a/zapcore/encoder.go +++ b/zapcore/encoder.go @@ -334,7 +334,7 @@ type EncoderConfig struct { EncodeName NameEncoder `json:"nameEncoder" yaml:"nameEncoder"` // Configure the encoder for interface{} type objects // If not provided, objects are encoded using json.Encoder - NewReflectedEncoder func(io.Writer) ReflectedEncoder `json:"newReflectedEncoder" yaml:"newReflectedEncoder"` + NewReflectedEncoder func(io.Writer) ReflectedEncoder `json:"-" yaml:"-"` // Configures the field separator used by the console encoder. Defaults // to tab. ConsoleSeparator string `json:"consoleSeparator" yaml:"consoleSeparator"` diff --git a/zapcore/json_encoder.go b/zapcore/json_encoder.go index f38074584..bb9b744c0 100644 --- a/zapcore/json_encoder.go +++ b/zapcore/json_encoder.go @@ -88,6 +88,11 @@ func newJSONEncoder(cfg EncoderConfig, spaced bool) *jsonEncoder { cfg.LineEnding = DefaultLineEnding } + // If no EncoderConfig.NewReflectedEncoder is provided by the user, then use default + if cfg.NewReflectedEncoder == nil { + cfg.NewReflectedEncoder = defaultReflectedEncoder() + } + return &jsonEncoder{ EncoderConfig: &cfg, buf: bufferpool.Get(), @@ -152,14 +157,7 @@ func (enc *jsonEncoder) AddInt64(key string, val int64) { func (enc *jsonEncoder) resetReflectBuf() { if enc.reflectBuf == nil { enc.reflectBuf = bufferpool.Get() - // If no EncoderConfig.NewReflectedEncoder is provided by the user, then use default - var newReflectedEncoder func(io.Writer) ReflectedEncoder - if enc.NewReflectedEncoder == nil { - newReflectedEncoder = GetDefaultReflectedEncoder() - } else { - newReflectedEncoder = enc.NewReflectedEncoder - } - enc.reflectEnc = newReflectedEncoder(enc.reflectBuf) + enc.reflectEnc = enc.NewReflectedEncoder(enc.reflectBuf) } else { enc.reflectBuf.Reset() } diff --git a/zapcore/json_encoder_test.go b/zapcore/json_encoder_test.go index 8e82c1a7a..904558e8e 100644 --- a/zapcore/json_encoder_test.go +++ b/zapcore/json_encoder_test.go @@ -21,6 +21,7 @@ package zapcore_test import ( + "io" "testing" "time" @@ -171,3 +172,65 @@ func TestJSONEmptyConfig(t *testing.T) { }) } } + +// Encodes any object into empty json '{}' +type emptyReflectedEncoder struct { + writer io.Writer +} + +func (enc *emptyReflectedEncoder) Encode(obj interface{}) error { + _, err := enc.writer.Write([]byte("{}")) + return err +} + +func TestJSONCustomReflectionEncoder(t *testing.T) { + tests := []struct { + name string + field zapcore.Field + expected string + }{ + { + name: "encode custom map object", + field: zapcore.Field{ + Key: "data", + Type: zapcore.ReflectType, + Interface: map[string]interface{}{ + "foo": "hello", + "bar": 1111, + }, + }, + expected: `{"data":{}}`, + }, + { + name: "encode nil object", + field: zapcore.Field{ + Key: "data", + Type: zapcore.ReflectType, + }, + expected: `{"data":null}`, + }, + } + + enc := zapcore.NewJSONEncoder(zapcore.EncoderConfig{ + NewReflectedEncoder: func(writer io.Writer) zapcore.ReflectedEncoder { + return &emptyReflectedEncoder{ + writer: writer, + } + }, + }) + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + buf, err := enc.EncodeEntry(zapcore.Entry{ + Level: zapcore.DebugLevel, + Time: time.Now(), + LoggerName: "logger", + Message: "things happened", + }, []zapcore.Field{tt.field}) + if assert.NoError(t, err, "Unexpected JSON encoding error.") { + assert.JSONEq(t, tt.expected, buf.String(), "Incorrect encoded JSON entry.") + } + buf.Free() + }) + } +} diff --git a/zapcore/reflected_encoder.go b/zapcore/reflected_encoder.go index 9253e479d..cef8b24d1 100644 --- a/zapcore/reflected_encoder.go +++ b/zapcore/reflected_encoder.go @@ -26,13 +26,13 @@ import ( ) // A ReflectedEncoder handles the serialization of Field which have FieldType = ReflectType and writes them to the underlying data stream -// The underlying data stream is provided by zap during initialization. See EncoderConfig.NewReflectedEncoder +// The underlying data stream is provided by Zap during initialization. See EncoderConfig.NewReflectedEncoder. type ReflectedEncoder interface { - // Encode encodes and writes to the underlying data stream + // Encode encodes and writes to the underlying data stream. Encode(interface{}) error } -func GetDefaultReflectedEncoder() func(writer io.Writer) ReflectedEncoder { +func defaultReflectedEncoder() func(writer io.Writer) ReflectedEncoder { return func(writer io.Writer) ReflectedEncoder { stdJsonEncoder := json.NewEncoder(writer) // For consistency with our custom JSON encoder. From 31bc967a0535e8e6bdffa8a719f739ed59d348e0 Mon Sep 17 00:00:00 2001 From: Rajat Singh Panwar <55539587+psrajat@users.noreply.github.com> Date: Wed, 15 Dec 2021 00:26:21 +0530 Subject: [PATCH 4/8] fix TestJSONEncoderObjectFields test --- zapcore/json_encoder.go | 1 - zapcore/json_encoder_impl_test.go | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/zapcore/json_encoder.go b/zapcore/json_encoder.go index bb9b744c0..94724b589 100644 --- a/zapcore/json_encoder.go +++ b/zapcore/json_encoder.go @@ -22,7 +22,6 @@ package zapcore import ( "encoding/base64" - "io" "math" "sync" "time" diff --git a/zapcore/json_encoder_impl_test.go b/zapcore/json_encoder_impl_test.go index c1c4c629d..6ba25ea15 100644 --- a/zapcore/json_encoder_impl_test.go +++ b/zapcore/json_encoder_impl_test.go @@ -509,6 +509,7 @@ func assertJSON(t *testing.T, expected string, enc *jsonEncoder) { func assertOutput(t testing.TB, cfg EncoderConfig, expected string, f func(Encoder)) { enc := &jsonEncoder{buf: bufferpool.Get(), EncoderConfig: &cfg} + enc.NewReflectedEncoder = defaultReflectedEncoder() f(enc) assert.Equal(t, expected, enc.buf.String(), "Unexpected encoder output after adding.") From 3babc2913fd207f7afc236ca4068cc83a210a30f Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Tue, 14 Dec 2021 13:42:39 -0800 Subject: [PATCH 5/8] defaultReflectedEncoder: Drop closure and wrapper type The defaultReflectedEncoder function can satisfy the signature for `NewReflectedEncoder`, so we don't need to return a closure. And the returned `json.Encoder` matches the ReflectedEncoder interface so we don't need the wrapper type either. --- zapcore/encoder.go | 2 +- zapcore/json_encoder.go | 2 +- zapcore/reflected_encoder.go | 27 ++++++++------------------- 3 files changed, 10 insertions(+), 21 deletions(-) diff --git a/zapcore/encoder.go b/zapcore/encoder.go index 71710a2a0..6e5fd5651 100644 --- a/zapcore/encoder.go +++ b/zapcore/encoder.go @@ -332,7 +332,7 @@ type EncoderConfig struct { // Unlike the other primitive type encoders, EncodeName is optional. The // zero value falls back to FullNameEncoder. EncodeName NameEncoder `json:"nameEncoder" yaml:"nameEncoder"` - // Configure the encoder for interface{} type objects + // Configure the encoder for interface{} type objects. // If not provided, objects are encoded using json.Encoder NewReflectedEncoder func(io.Writer) ReflectedEncoder `json:"-" yaml:"-"` // Configures the field separator used by the console encoder. Defaults diff --git a/zapcore/json_encoder.go b/zapcore/json_encoder.go index 94724b589..505c714ac 100644 --- a/zapcore/json_encoder.go +++ b/zapcore/json_encoder.go @@ -89,7 +89,7 @@ func newJSONEncoder(cfg EncoderConfig, spaced bool) *jsonEncoder { // If no EncoderConfig.NewReflectedEncoder is provided by the user, then use default if cfg.NewReflectedEncoder == nil { - cfg.NewReflectedEncoder = defaultReflectedEncoder() + cfg.NewReflectedEncoder = defaultReflectedEncoder } return &jsonEncoder{ diff --git a/zapcore/reflected_encoder.go b/zapcore/reflected_encoder.go index cef8b24d1..8746360ec 100644 --- a/zapcore/reflected_encoder.go +++ b/zapcore/reflected_encoder.go @@ -25,28 +25,17 @@ import ( "io" ) -// A ReflectedEncoder handles the serialization of Field which have FieldType = ReflectType and writes them to the underlying data stream -// The underlying data stream is provided by Zap during initialization. See EncoderConfig.NewReflectedEncoder. +// ReflectedEncoder serializes log fields that can't be serialized with Zap's +// JSON encoder. These have the ReflectType field type. +// Use EncoderConfig.NewReflectedEncoder to set this. type ReflectedEncoder interface { // Encode encodes and writes to the underlying data stream. Encode(interface{}) error } -func defaultReflectedEncoder() func(writer io.Writer) ReflectedEncoder { - return func(writer io.Writer) ReflectedEncoder { - stdJsonEncoder := json.NewEncoder(writer) - // For consistency with our custom JSON encoder. - stdJsonEncoder.SetEscapeHTML(false) - return &stdReflectedEncoder{ - encoder: stdJsonEncoder, - } - } -} - -type stdReflectedEncoder struct { - encoder *json.Encoder -} - -func (enc *stdReflectedEncoder) Encode(obj interface{}) error { - return enc.encoder.Encode(obj) +func defaultReflectedEncoder(w io.Writer) ReflectedEncoder { + enc := json.NewEncoder(w) + // For consistency with our custom JSON encoder. + enc.SetEscapeHTML(false) + return enc } From 17016b9a4ca614b601e8dcddb25920d71ec1c9a0 Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Tue, 14 Dec 2021 13:45:04 -0800 Subject: [PATCH 6/8] test/assertOutput: Use NewJSONEncoder to instantiate assertOutput manually sets up a jsonEncoder. This means that any config initalization we do in the constructor has to be replicated in assertOutput. Instead, use the constructor and cast to `*jsonEncoder`. --- zapcore/json_encoder_impl_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/zapcore/json_encoder_impl_test.go b/zapcore/json_encoder_impl_test.go index 6ba25ea15..fde241f56 100644 --- a/zapcore/json_encoder_impl_test.go +++ b/zapcore/json_encoder_impl_test.go @@ -508,8 +508,7 @@ func assertJSON(t *testing.T, expected string, enc *jsonEncoder) { } func assertOutput(t testing.TB, cfg EncoderConfig, expected string, f func(Encoder)) { - enc := &jsonEncoder{buf: bufferpool.Get(), EncoderConfig: &cfg} - enc.NewReflectedEncoder = defaultReflectedEncoder() + enc := NewJSONEncoder(cfg).(*jsonEncoder) f(enc) assert.Equal(t, expected, enc.buf.String(), "Unexpected encoder output after adding.") From bd74de74ec75aab4e478024aa353899808d684c1 Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Tue, 14 Dec 2021 16:56:43 -0800 Subject: [PATCH 7/8] Rename TestJSONCustomReflectionEncoder Co-authored-by: Sung Yoon Whang --- zapcore/json_encoder_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zapcore/json_encoder_test.go b/zapcore/json_encoder_test.go index 904558e8e..a1a494733 100644 --- a/zapcore/json_encoder_test.go +++ b/zapcore/json_encoder_test.go @@ -183,7 +183,7 @@ func (enc *emptyReflectedEncoder) Encode(obj interface{}) error { return err } -func TestJSONCustomReflectionEncoder(t *testing.T) { +func TestJSONCustomReflectedEncoder(t *testing.T) { tests := []struct { name string field zapcore.Field From c65627f726cdd88688197d4299fb3b4d8170a1e2 Mon Sep 17 00:00:00 2001 From: Abhinav Gupta Date: Tue, 14 Dec 2021 16:58:09 -0800 Subject: [PATCH 8/8] TestJSONCustomReflectedEncoder: parallelize --- zapcore/json_encoder_test.go | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/zapcore/json_encoder_test.go b/zapcore/json_encoder_test.go index a1a494733..944f23a75 100644 --- a/zapcore/json_encoder_test.go +++ b/zapcore/json_encoder_test.go @@ -211,16 +211,19 @@ func TestJSONCustomReflectedEncoder(t *testing.T) { }, } - enc := zapcore.NewJSONEncoder(zapcore.EncoderConfig{ - NewReflectedEncoder: func(writer io.Writer) zapcore.ReflectedEncoder { - return &emptyReflectedEncoder{ - writer: writer, - } - }, - }) - for _, tt := range tests { + tt := tt t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + enc := zapcore.NewJSONEncoder(zapcore.EncoderConfig{ + NewReflectedEncoder: func(writer io.Writer) zapcore.ReflectedEncoder { + return &emptyReflectedEncoder{ + writer: writer, + } + }, + }) + buf, err := enc.EncodeEntry(zapcore.Entry{ Level: zapcore.DebugLevel, Time: time.Now(),