From a6000c839a4b92c5bb70652ec6238437104b4ebd Mon Sep 17 00:00:00 2001 From: Vladimir Skipor Date: Sun, 12 Mar 2017 21:44:38 +0300 Subject: [PATCH 1/6] Add ByteString API This is a breaking change that adds a `AddByteString` to `zapcore.ObjectEncoder`, `AppendByteString` to `zapcore.PrimitiveArrayEncoder`. This API optimizes logging UTF-8 encoded []byte data. Fixes #324. --- array.go | 15 +++++ array_test.go | 5 +- field.go | 8 +++ field_test.go | 1 + logger_bench_test.go | 7 +++ zapcore/encoder.go | 7 +++ zapcore/field.go | 4 ++ zapcore/field_test.go | 1 + zapcore/json_encoder.go | 99 ++++++++++++++++++++++--------- zapcore/json_encoder_impl_test.go | 91 +++++++++++++++++++--------- zapcore/memory_encoder.go | 4 ++ 11 files changed, 186 insertions(+), 56 deletions(-) diff --git a/array.go b/array.go index 5e52f3d96..cf03c47dd 100644 --- a/array.go +++ b/array.go @@ -43,6 +43,12 @@ func Bools(key string, bs []bool) zapcore.Field { return Array(key, bools(bs)) } +// ByteStrings constructs a field that carries a slice of []byte +// that assumed to be UTF-8 encoded. +func ByteStrings(key string, bss [][]byte) zapcore.Field { + return Array(key, byteStringsArray(bss)) +} + // Complex128s constructs a field that carries a slice of complex numbers. func Complex128s(key string, nums []complex128) zapcore.Field { return Array(key, complex128s(nums)) @@ -147,6 +153,15 @@ func (bs bools) MarshalLogArray(arr zapcore.ArrayEncoder) error { return nil } +type byteStringsArray [][]byte + +func (bss byteStringsArray) MarshalLogArray(arr zapcore.ArrayEncoder) error { + for i := range bss { + arr.AppendByteString(bss[i]) + } + return nil +} + type complex128s []complex128 func (nums complex128s) MarshalLogArray(arr zapcore.ArrayEncoder) error { diff --git a/array_test.go b/array_test.go index 0c7dc41be..1ebbd4de3 100644 --- a/array_test.go +++ b/array_test.go @@ -25,11 +25,10 @@ import ( "testing" "time" - "go.uber.org/zap/zapcore" - richErrors "github.com/pkg/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.uber.org/zap/zapcore" ) func BenchmarkBoolsArrayMarshaler(b *testing.B) { @@ -59,6 +58,7 @@ func TestArrayWrappers(t *testing.T) { expected []interface{} }{ {"empty bools", Bools("", []bool{}), []interface{}(nil)}, + {"empty byte strings", ByteStrings("", [][]byte{}), []interface{}(nil)}, {"empty complex128s", Complex128s("", []complex128{}), []interface{}(nil)}, {"empty complex64s", Complex64s("", []complex64{}), []interface{}(nil)}, {"empty durations", Durations("", []time.Duration{}), []interface{}(nil)}, @@ -79,6 +79,7 @@ func TestArrayWrappers(t *testing.T) { {"empty uintptrs", Uintptrs("", []uintptr{}), []interface{}(nil)}, {"empty errors", Errors("", []error{}), []interface{}(nil)}, {"bools", Bools("", []bool{true, false}), []interface{}{true, false}}, + {"byte strings", ByteStrings("", [][]byte{{1, 2}, {3, 4}}), []interface{}{[]byte{1, 2}, []byte{3, 4}}}, {"complex128s", Complex128s("", []complex128{1 + 2i, 3 + 4i}), []interface{}{1 + 2i, 3 + 4i}}, {"complex64s", Complex64s("", []complex64{1 + 2i, 3 + 4i}), []interface{}{complex64(1 + 2i), complex64(3 + 4i)}}, {"durations", Durations("", []time.Duration{1, 2}), []interface{}{time.Nanosecond, 2 * time.Nanosecond}}, diff --git a/field.go b/field.go index 905defe87..9a579dc61 100644 --- a/field.go +++ b/field.go @@ -50,6 +50,14 @@ func Bool(key string, val bool) zapcore.Field { return zapcore.Field{Key: key, Type: zapcore.BoolType, Integer: ival} } +// ByteString constructs a field that carries a []byte that assumed to be UTF-8 encoded. +// +// Saves on []byte to string cast copy and allocation, but costs smaller allocation to convert +// the []byte to interface{}. +func ByteString(key string, val []byte) zapcore.Field { + return zapcore.Field{Key: key, Type: zapcore.ByteStringType, Interface: val} +} + // Complex128 constructs a field that carries a complex number. Unlike most // numeric fields, this costs an allocation (to convert the complex128 to // interface{}). diff --git a/field_test.go b/field_test.go index 59c516879..c932c319d 100644 --- a/field_test.go +++ b/field_test.go @@ -74,6 +74,7 @@ func TestFieldConstructors(t *testing.T) { {"Binary", zapcore.Field{Key: "k", Type: zapcore.BinaryType, Interface: []byte("ab12")}, Binary("k", []byte("ab12"))}, {"Bool", zapcore.Field{Key: "k", Type: zapcore.BoolType, Integer: 1}, Bool("k", true)}, {"Bool", zapcore.Field{Key: "k", Type: zapcore.BoolType, Integer: 1}, Bool("k", true)}, + {"ByteString", zapcore.Field{Key: "k", Type: zapcore.ByteStringType, Interface: []byte("ab12")}, ByteString("k", []byte("ab12"))}, {"Complex128", zapcore.Field{Key: "k", Type: zapcore.Complex128Type, Interface: 1 + 2i}, Complex128("k", 1+2i)}, {"Complex64", zapcore.Field{Key: "k", Type: zapcore.Complex64Type, Interface: complex64(1 + 2i)}, Complex64("k", 1+2i)}, {"Duration", zapcore.Field{Key: "k", Type: zapcore.DurationType, Integer: 1}, Duration("k", 1)}, diff --git a/logger_bench_test.go b/logger_bench_test.go index 4d87f8a29..1ba7c759b 100644 --- a/logger_bench_test.go +++ b/logger_bench_test.go @@ -75,6 +75,13 @@ func BenchmarkBoolField(b *testing.B) { }) } +func BenchmarkByteStringField(b *testing.B) { + val := []byte("bar") + withBenchedLogger(b, func(log *Logger) { + log.Info("ByteString.", ByteString("foo", val)) + }) +} + func BenchmarkFloat64Field(b *testing.B) { withBenchedLogger(b, func(log *Logger) { log.Info("Floating point.", Float64("foo", 3.14)) diff --git a/zapcore/encoder.go b/zapcore/encoder.go index 854b2572b..49e772eba 100644 --- a/zapcore/encoder.go +++ b/zapcore/encoder.go @@ -216,11 +216,17 @@ type EncoderConfig struct { // aren't safe for concurrent use (though typical use shouldn't require locks). type ObjectEncoder interface { // Logging-specific marshalers. + AddArray(key string, marshaler ArrayMarshaler) error AddObject(key string, marshaler ObjectMarshaler) error // Built-in types. + + // AddBinary adds raw blob of binary data. AddBinary(key string, value []byte) + // AddByteString adds bytes as UTF-8 string. + // No-alloc equivalent of AddString(string(value)) for []byte values. + AddByteString(key string, value []byte) AddBool(key string, value bool) AddComplex128(key string, value complex128) AddComplex64(key string, value complex64) @@ -277,6 +283,7 @@ type ArrayEncoder interface { type PrimitiveArrayEncoder interface { // Built-in types. AppendBool(bool) + AppendByteString([]byte) AppendComplex128(complex128) AppendComplex64(complex64) AppendFloat64(float64) diff --git a/zapcore/field.go b/zapcore/field.go index d8a245ecb..b8018db42 100644 --- a/zapcore/field.go +++ b/zapcore/field.go @@ -41,6 +41,8 @@ const ( BinaryType // BoolType indicates that the field carries a bool. BoolType + // ByteStringType indicates that the field carries a []byte that assumed to be UTF-8 encoded. + ByteStringType // Complex128Type indicates that the field carries a complex128. Complex128Type // Complex64Type indicates that the field carries a complex128. @@ -112,6 +114,8 @@ func (f Field) AddTo(enc ObjectEncoder) { enc.AddBinary(f.Key, f.Interface.([]byte)) case BoolType: enc.AddBool(f.Key, f.Integer == 1) + case ByteStringType: + enc.AddByteString(f.Key, f.Interface.([]byte)) case Complex128Type: enc.AddComplex128(f.Key, f.Interface.(complex128)) case Complex64Type: diff --git a/zapcore/field_test.go b/zapcore/field_test.go index 368e7f3d0..16a1ff5d0 100644 --- a/zapcore/field_test.go +++ b/zapcore/field_test.go @@ -107,6 +107,7 @@ func TestFields(t *testing.T) { {t: ObjectMarshalerType, iface: users(2), want: map[string]interface{}{"users": 2}}, {t: BinaryType, iface: []byte("foo"), want: []byte("foo")}, {t: BoolType, i: 0, want: false}, + {t: ByteStringType, iface: []byte("foo"), want: []byte("foo")}, {t: Complex128Type, iface: 1 + 2i, want: 1 + 2i}, {t: Complex64Type, iface: complex64(1 + 2i), want: complex64(1 + 2i)}, {t: DurationType, i: 1000, want: time.Microsecond}, diff --git a/zapcore/json_encoder.go b/zapcore/json_encoder.go index e0a19ab37..a91b0b65c 100644 --- a/zapcore/json_encoder.go +++ b/zapcore/json_encoder.go @@ -94,6 +94,11 @@ func (enc *jsonEncoder) AddBinary(key string, val []byte) { enc.AddString(key, base64.StdEncoding.EncodeToString(val)) } +func (enc *jsonEncoder) AddByteString(key string, val []byte) { + enc.addKey(key) + enc.AppendByteString(val) +} + func (enc *jsonEncoder) AddBool(key string, val bool) { enc.addKey(key) enc.AppendBool(val) @@ -171,6 +176,13 @@ func (enc *jsonEncoder) AppendBool(val bool) { enc.buf.AppendBool(val) } +func (enc *jsonEncoder) AppendByteString(val []byte) { + enc.addElementSeparator() + enc.buf.AppendByte('"') + enc.safeAddByteString(val) + enc.buf.AppendByte('"') +} + func (enc *jsonEncoder) AppendComplex128(val complex128) { enc.addElementSeparator() // Cast to a platform-independent, fixed-size type. @@ -379,36 +391,12 @@ func (enc *jsonEncoder) appendFloat(val float64, bitSize int) { // user from browser vulnerabilities or JSONP-related problems. func (enc *jsonEncoder) safeAddString(s string) { for i := 0; i < len(s); { - if b := s[i]; b < utf8.RuneSelf { + if enc.tryAddRuneSelf(s[i]) { i++ - if 0x20 <= b && b != '\\' && b != '"' { - enc.buf.AppendByte(b) - continue - } - switch b { - case '\\', '"': - enc.buf.AppendByte('\\') - enc.buf.AppendByte(b) - case '\n': - enc.buf.AppendByte('\\') - enc.buf.AppendByte('n') - case '\r': - enc.buf.AppendByte('\\') - enc.buf.AppendByte('r') - case '\t': - enc.buf.AppendByte('\\') - enc.buf.AppendByte('t') - default: - // Encode bytes < 0x20, except for the escape sequences above. - enc.buf.AppendString(`\u00`) - enc.buf.AppendByte(_hex[b>>4]) - enc.buf.AppendByte(_hex[b&0xF]) - } continue } - c, size := utf8.DecodeRuneInString(s[i:]) - if c == utf8.RuneError && size == 1 { - enc.buf.AppendString(`\ufffd`) + r, size := utf8.DecodeRuneInString(s[i:]) + if enc.tryAddRuneError(r, size) { i++ continue } @@ -416,3 +404,60 @@ func (enc *jsonEncoder) safeAddString(s string) { i += size } } + +// safeAddByteString is no-alloc equivalent of safeAddString(string(s)) for s []byte. +func (enc *jsonEncoder) safeAddByteString(s []byte) { + for i := 0; i < len(s); { + if enc.tryAddRuneSelf(s[i]) { + i++ + continue + } + r, size := utf8.DecodeRune(s[i:]) + if enc.tryAddRuneError(r, size) { + i++ + continue + } + enc.buf.Write(s[i : i+size]) + i += size + } +} + +// tryAddRuneSelf appends b if it is valid UTF-8 character represented in a single byte. +func (enc *jsonEncoder) tryAddRuneSelf(b byte) (ok bool) { + ok = b < utf8.RuneSelf + if !ok { + return + } + if 0x20 <= b && b != '\\' && b != '"' { + enc.buf.AppendByte(b) + return + } + switch b { + case '\\', '"': + enc.buf.AppendByte('\\') + enc.buf.AppendByte(b) + case '\n': + enc.buf.AppendByte('\\') + enc.buf.AppendByte('n') + case '\r': + enc.buf.AppendByte('\\') + enc.buf.AppendByte('r') + case '\t': + enc.buf.AppendByte('\\') + enc.buf.AppendByte('t') + default: + // Encode bytes < 0x20, except for the escape sequences above. + enc.buf.AppendString(`\u00`) + enc.buf.AppendByte(_hex[b>>4]) + enc.buf.AppendByte(_hex[b&0xF]) + } + return +} + +func (enc *jsonEncoder) tryAddRuneError(r rune, size int) (ok bool) { + ok = r == utf8.RuneError && size == 1 + if ok { + enc.buf.AppendString(`\ufffd`) + } + return +} diff --git a/zapcore/json_encoder_impl_test.go b/zapcore/json_encoder_impl_test.go index dafbe6dab..bc44eca9c 100644 --- a/zapcore/json_encoder_impl_test.go +++ b/zapcore/json_encoder_impl_test.go @@ -83,11 +83,22 @@ func TestJSONEscaping(t *testing.T) { "\xed\xa0\x80": `\ufffd\ufffd\ufffd`, "foo\xed\xa0\x80": `foo\ufffd\ufffd\ufffd`, } - for input, output := range cases { - enc.truncate() - enc.safeAddString(input) - assertJSON(t, output, enc) - } + + t.Run("String", func(t *testing.T) { + for input, output := range cases { + enc.truncate() + enc.safeAddString(input) + assertJSON(t, output, enc) + } + }) + + t.Run("ByteString", func(t *testing.T) { + for input, output := range cases { + enc.truncate() + enc.safeAddByteString([]byte(input)) + assertJSON(t, output, enc) + } + }) } func TestJSONEncoderObjectFields(t *testing.T) { @@ -100,6 +111,10 @@ func TestJSONEncoderObjectFields(t *testing.T) { {"bool", `"k\\":true`, func(e Encoder) { e.AddBool(`k\`, true) }}, // test key escaping once {"bool", `"k":true`, func(e Encoder) { e.AddBool("k", true) }}, {"bool", `"k":false`, func(e Encoder) { e.AddBool("k", false) }}, + {"byteString", `"k":"v\\"`, func(e Encoder) { e.AddByteString(`k`, []byte(`v\`)) }}, + {"byteString", `"k":"v"`, func(e Encoder) { e.AddByteString("k", []byte("v")) }}, + {"byteString", `"k":""`, func(e Encoder) { e.AddByteString("k", []byte{}) }}, + {"byteString", `"k":""`, func(e Encoder) { e.AddByteString("k", nil) }}, {"complex128", `"k":"1+2i"`, func(e Encoder) { e.AddComplex128("k", 1+2i) }}, {"complex64", `"k":"1+2i"`, func(e Encoder) { e.AddComplex64("k", 1+2i) }}, {"duration", `"k":0.000000001`, func(e Encoder) { e.AddDuration("k", 1) }}, @@ -219,6 +234,8 @@ func TestJSONEncoderArrays(t *testing.T) { f func(ArrayEncoder) }{ {"bool", `[true,true]`, func(e ArrayEncoder) { e.AppendBool(true) }}, + {"byteString", `["k","k"]`, func(e ArrayEncoder) { e.AppendByteString([]byte("k")) }}, + {"byteString", `["k\\","k\\"]`, func(e ArrayEncoder) { e.AppendByteString([]byte(`k\`)) }}, {"complex128", `["1+2i","1+2i"]`, func(e ArrayEncoder) { e.AppendComplex128(1 + 2i) }}, {"complex64", `["1+2i","1+2i"]`, func(e ArrayEncoder) { e.AppendComplex64(1 + 2i) }}, {"durations", `[0.000000002,0.000000002]`, func(e ArrayEncoder) { e.AppendDuration(2) }}, @@ -385,22 +402,24 @@ func (nj noJSON) MarshalJSON() ([]byte, error) { return nil, errors.New("no") } -func zapEncodeString(s string) []byte { - enc := &jsonEncoder{buf: bufferpool.Get()} - // Escape and quote a string using our encoder. - var ret []byte - enc.safeAddString(s) - ret = make([]byte, 0, enc.buf.Len()+2) - ret = append(ret, '"') - ret = append(ret, enc.buf.Bytes()...) - ret = append(ret, '"') - return ret +func zapEncode(encode func(*jsonEncoder, string)) func(s string) []byte { + return func(s string) []byte { + enc := &jsonEncoder{buf: bufferpool.Get()} + // Escape and quote a string using our encoder. + var ret []byte + encode(enc, s) + ret = make([]byte, 0, enc.buf.Len()+2) + ret = append(ret, '"') + ret = append(ret, enc.buf.Bytes()...) + ret = append(ret, '"') + return ret + } } -func roundTripsCorrectly(original string) bool { +func roundTripsCorrectly(encode func(string) []byte, original string) bool { // Encode using our encoder, decode using the standard library, and assert // that we haven't lost any information. - encoded := zapEncodeString(original) + encoded := encode(original) var decoded string err := json.Unmarshal(encoded, &decoded) @@ -410,6 +429,18 @@ func roundTripsCorrectly(original string) bool { return original == decoded } +func roundTripsCorrectlyString(original string) bool { + return roundTripsCorrectly(zapEncode((*jsonEncoder).safeAddString), original) +} + +func roundTripsCorrectlyByteString(original string) bool { + return roundTripsCorrectly( + zapEncode(func(enc *jsonEncoder, s string) { + enc.safeAddByteString([]byte(s)) + }), + original) +} + type ASCII string func (s ASCII) Generate(r *rand.Rand, size int) reflect.Value { @@ -421,20 +452,26 @@ func (s ASCII) Generate(r *rand.Rand, size int) reflect.Value { return reflect.ValueOf(a) } -func asciiRoundTripsCorrectly(s ASCII) bool { - return roundTripsCorrectly(string(s)) +func asciiRoundTripsCorrectlyString(s ASCII) bool { + return roundTripsCorrectlyString(string(s)) +} + +func asciiRoundTripsCorrectlyByteString(s ASCII) bool { + return roundTripsCorrectlyByteString(string(s)) } func TestJSONQuick(t *testing.T) { - // Test the full range of UTF-8 strings. - err := quick.Check(roundTripsCorrectly, &quick.Config{MaxCountScale: 100.0}) - if err != nil { - t.Error(err.Error()) + check := func(f interface{}) { + err := quick.Check(f, &quick.Config{MaxCountScale: 100.0}) + assert.NoError(t, err) } + // Test the full range of UTF-8 strings. + check(roundTripsCorrectlyString) // Focus on ASCII strings. - err = quick.Check(asciiRoundTripsCorrectly, &quick.Config{MaxCountScale: 100.0}) - if err != nil { - t.Error(err.Error()) - } + check(asciiRoundTripsCorrectlyString) + + // Same for adding string as []byte. + check(roundTripsCorrectlyByteString) + check(asciiRoundTripsCorrectlyByteString) } diff --git a/zapcore/memory_encoder.go b/zapcore/memory_encoder.go index 01918c1f7..4805c8a94 100644 --- a/zapcore/memory_encoder.go +++ b/zapcore/memory_encoder.go @@ -59,6 +59,9 @@ func (m *MapObjectEncoder) AddObject(k string, v ObjectMarshaler) error { // AddBinary implements ObjectEncoder. func (m *MapObjectEncoder) AddBinary(k string, v []byte) { m.cur[k] = v } +// AddByteString implements ObjectEncoder. +func (m *MapObjectEncoder) AddByteString(k string, v []byte) { m.cur[k] = v } + // AddBool implements ObjectEncoder. func (m *MapObjectEncoder) AddBool(k string, v bool) { m.cur[k] = v } @@ -155,6 +158,7 @@ func (s *sliceArrayEncoder) AppendReflected(v interface{}) error { } func (s *sliceArrayEncoder) AppendBool(v bool) { s.elems = append(s.elems, v) } +func (s *sliceArrayEncoder) AppendByteString(v []byte) { s.elems = append(s.elems, v) } func (s *sliceArrayEncoder) AppendComplex128(v complex128) { s.elems = append(s.elems, v) } func (s *sliceArrayEncoder) AppendComplex64(v complex64) { s.elems = append(s.elems, v) } func (s *sliceArrayEncoder) AppendDuration(v time.Duration) { s.elems = append(s.elems, v) } From 166e1491bcfbe938fb8858410b0234f23db6bf13 Mon Sep 17 00:00:00 2001 From: Vladimir Skipor Date: Sun, 12 Mar 2017 21:46:24 +0300 Subject: [PATCH 2/6] Add ByteString API This is a breaking change that adds a `AddByteString` to `zapcore.ObjectEncoder`, `AppendByteString` to `zapcore.PrimitiveArrayEncoder`. This API optimizes logging UTF-8 encoded []byte data. Fixes #324. --- array_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/array_test.go b/array_test.go index 1ebbd4de3..8f1bf0a50 100644 --- a/array_test.go +++ b/array_test.go @@ -25,10 +25,11 @@ import ( "testing" "time" + "go.uber.org/zap/zapcore" + richErrors "github.com/pkg/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "go.uber.org/zap/zapcore" ) func BenchmarkBoolsArrayMarshaler(b *testing.B) { From 03e7655cf83990617d22c255b5148f9dc6513282 Mon Sep 17 00:00:00 2001 From: Akshay Shah Date: Tue, 14 Mar 2017 14:02:28 -0700 Subject: [PATCH 3/6] Clean up initial implementation --- array.go | 4 ++-- field.go | 10 +++++----- zapcore/encoder.go | 11 +++-------- zapcore/field.go | 2 +- zapcore/json_encoder.go | 19 +++++++++---------- zapcore/json_encoder_impl_test.go | 4 +--- 6 files changed, 21 insertions(+), 29 deletions(-) diff --git a/array.go b/array.go index cf03c47dd..784c6df0f 100644 --- a/array.go +++ b/array.go @@ -43,8 +43,8 @@ func Bools(key string, bs []bool) zapcore.Field { return Array(key, bools(bs)) } -// ByteStrings constructs a field that carries a slice of []byte -// that assumed to be UTF-8 encoded. +// ByteStrings constructs a field that carries a slice of []byte, each of which +// must be UTF-8 encoded text. func ByteStrings(key string, bss [][]byte) zapcore.Field { return Array(key, byteStringsArray(bss)) } diff --git a/field.go b/field.go index 9a579dc61..9a8193266 100644 --- a/field.go +++ b/field.go @@ -36,7 +36,8 @@ func Skip() zapcore.Field { // Binary constructs a field that carries an opaque binary blob. // // Binary data is serialized in an encoding-appropriate format. For example, -// zap's JSON encoder base64-encodes binary blobs. +// zap's JSON encoder base64-encodes binary blobs. To log UTF-8 encoded text, +// use ByteString. func Binary(key string, val []byte) zapcore.Field { return zapcore.Field{Key: key, Type: zapcore.BinaryType, Interface: val} } @@ -50,10 +51,9 @@ func Bool(key string, val bool) zapcore.Field { return zapcore.Field{Key: key, Type: zapcore.BoolType, Integer: ival} } -// ByteString constructs a field that carries a []byte that assumed to be UTF-8 encoded. -// -// Saves on []byte to string cast copy and allocation, but costs smaller allocation to convert -// the []byte to interface{}. +// ByteString constructs a field that carries UTF-8 encoded text as a []byte. +// To log opaque binary blobs (which aren't necessarily valid UTF-8), use +// Binary. func ByteString(key string, val []byte) zapcore.Field { return zapcore.Field{Key: key, Type: zapcore.ByteStringType, Interface: val} } diff --git a/zapcore/encoder.go b/zapcore/encoder.go index 49e772eba..6be047a5c 100644 --- a/zapcore/encoder.go +++ b/zapcore/encoder.go @@ -216,17 +216,12 @@ type EncoderConfig struct { // aren't safe for concurrent use (though typical use shouldn't require locks). type ObjectEncoder interface { // Logging-specific marshalers. - AddArray(key string, marshaler ArrayMarshaler) error AddObject(key string, marshaler ObjectMarshaler) error // Built-in types. - - // AddBinary adds raw blob of binary data. - AddBinary(key string, value []byte) - // AddByteString adds bytes as UTF-8 string. - // No-alloc equivalent of AddString(string(value)) for []byte values. - AddByteString(key string, value []byte) + AddBinary(key string, value []byte) // for arbitrary bytes + AddByteString(key string, value []byte) // for UTF-8 encoded bytes AddBool(key string, value bool) AddComplex128(key string, value complex128) AddComplex64(key string, value complex64) @@ -283,7 +278,7 @@ type ArrayEncoder interface { type PrimitiveArrayEncoder interface { // Built-in types. AppendBool(bool) - AppendByteString([]byte) + AppendByteString([]byte) // for UTF-8 encoded bytes AppendComplex128(complex128) AppendComplex64(complex64) AppendFloat64(float64) diff --git a/zapcore/field.go b/zapcore/field.go index b8018db42..6922e9437 100644 --- a/zapcore/field.go +++ b/zapcore/field.go @@ -41,7 +41,7 @@ const ( BinaryType // BoolType indicates that the field carries a bool. BoolType - // ByteStringType indicates that the field carries a []byte that assumed to be UTF-8 encoded. + // ByteStringType indicates that the field carries a UTF-8 encoded bytes. ByteStringType // Complex128Type indicates that the field carries a complex128. Complex128Type diff --git a/zapcore/json_encoder.go b/zapcore/json_encoder.go index a91b0b65c..45558f2a8 100644 --- a/zapcore/json_encoder.go +++ b/zapcore/json_encoder.go @@ -423,14 +423,13 @@ func (enc *jsonEncoder) safeAddByteString(s []byte) { } // tryAddRuneSelf appends b if it is valid UTF-8 character represented in a single byte. -func (enc *jsonEncoder) tryAddRuneSelf(b byte) (ok bool) { - ok = b < utf8.RuneSelf - if !ok { - return +func (enc *jsonEncoder) tryAddRuneSelf(b byte) bool { + if b >= utf8.RuneSelf { + return false } if 0x20 <= b && b != '\\' && b != '"' { enc.buf.AppendByte(b) - return + return true } switch b { case '\\', '"': @@ -451,13 +450,13 @@ func (enc *jsonEncoder) tryAddRuneSelf(b byte) (ok bool) { enc.buf.AppendByte(_hex[b>>4]) enc.buf.AppendByte(_hex[b&0xF]) } - return + return true } -func (enc *jsonEncoder) tryAddRuneError(r rune, size int) (ok bool) { - ok = r == utf8.RuneError && size == 1 - if ok { +func (enc *jsonEncoder) tryAddRuneError(r rune, size int) bool { + if r == utf8.RuneError && size == 1 { enc.buf.AppendString(`\ufffd`) + return true } - return + return false } diff --git a/zapcore/json_encoder_impl_test.go b/zapcore/json_encoder_impl_test.go index bc44eca9c..5d5319e7f 100644 --- a/zapcore/json_encoder_impl_test.go +++ b/zapcore/json_encoder_impl_test.go @@ -467,11 +467,9 @@ func TestJSONQuick(t *testing.T) { } // Test the full range of UTF-8 strings. check(roundTripsCorrectlyString) + check(roundTripsCorrectlyByteString) // Focus on ASCII strings. check(asciiRoundTripsCorrectlyString) - - // Same for adding string as []byte. - check(roundTripsCorrectlyByteString) check(asciiRoundTripsCorrectlyByteString) } From c7ba68060c87ecc35b4618b6ffeeb3c940802415 Mon Sep 17 00:00:00 2001 From: Akshay Shah Date: Tue, 14 Mar 2017 14:23:51 -0700 Subject: [PATCH 4/6] Add Bytes to Field union To make logging bytestrings and binary more efficient, add a byte slice to the Field union. This adds ~80ns to the `AddingFields` benchmarks, but saves an allocation along a path that particularly performance-sensitive applications will use. --- field.go | 4 ++-- field_test.go | 4 ++-- zapcore/field.go | 5 +++-- zapcore/field_test.go | 7 ++++--- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/field.go b/field.go index 9a8193266..5dc62bc11 100644 --- a/field.go +++ b/field.go @@ -39,7 +39,7 @@ func Skip() zapcore.Field { // zap's JSON encoder base64-encodes binary blobs. To log UTF-8 encoded text, // use ByteString. func Binary(key string, val []byte) zapcore.Field { - return zapcore.Field{Key: key, Type: zapcore.BinaryType, Interface: val} + return zapcore.Field{Key: key, Type: zapcore.BinaryType, Bytes: val} } // Bool constructs a field that carries a bool. @@ -55,7 +55,7 @@ func Bool(key string, val bool) zapcore.Field { // To log opaque binary blobs (which aren't necessarily valid UTF-8), use // Binary. func ByteString(key string, val []byte) zapcore.Field { - return zapcore.Field{Key: key, Type: zapcore.ByteStringType, Interface: val} + return zapcore.Field{Key: key, Type: zapcore.ByteStringType, Bytes: val} } // Complex128 constructs a field that carries a complex number. Unlike most diff --git a/field_test.go b/field_test.go index c932c319d..ef362673a 100644 --- a/field_test.go +++ b/field_test.go @@ -71,10 +71,10 @@ func TestFieldConstructors(t *testing.T) { expect zapcore.Field }{ {"Skip", zapcore.Field{Type: zapcore.SkipType}, Skip()}, - {"Binary", zapcore.Field{Key: "k", Type: zapcore.BinaryType, Interface: []byte("ab12")}, Binary("k", []byte("ab12"))}, + {"Binary", zapcore.Field{Key: "k", Type: zapcore.BinaryType, Bytes: []byte("ab12")}, Binary("k", []byte("ab12"))}, {"Bool", zapcore.Field{Key: "k", Type: zapcore.BoolType, Integer: 1}, Bool("k", true)}, {"Bool", zapcore.Field{Key: "k", Type: zapcore.BoolType, Integer: 1}, Bool("k", true)}, - {"ByteString", zapcore.Field{Key: "k", Type: zapcore.ByteStringType, Interface: []byte("ab12")}, ByteString("k", []byte("ab12"))}, + {"ByteString", zapcore.Field{Key: "k", Type: zapcore.ByteStringType, Bytes: []byte("ab12")}, ByteString("k", []byte("ab12"))}, {"Complex128", zapcore.Field{Key: "k", Type: zapcore.Complex128Type, Interface: 1 + 2i}, Complex128("k", 1+2i)}, {"Complex64", zapcore.Field{Key: "k", Type: zapcore.Complex64Type, Interface: complex64(1 + 2i)}, Complex64("k", 1+2i)}, {"Duration", zapcore.Field{Key: "k", Type: zapcore.DurationType, Integer: 1}, Duration("k", 1)}, diff --git a/zapcore/field.go b/zapcore/field.go index 6922e9437..e1aeb1ece 100644 --- a/zapcore/field.go +++ b/zapcore/field.go @@ -97,6 +97,7 @@ type Field struct { Type FieldType Integer int64 String string + Bytes []byte Interface interface{} } @@ -111,11 +112,11 @@ func (f Field) AddTo(enc ObjectEncoder) { case ObjectMarshalerType: err = enc.AddObject(f.Key, f.Interface.(ObjectMarshaler)) case BinaryType: - enc.AddBinary(f.Key, f.Interface.([]byte)) + enc.AddBinary(f.Key, f.Bytes) case BoolType: enc.AddBool(f.Key, f.Integer == 1) case ByteStringType: - enc.AddByteString(f.Key, f.Interface.([]byte)) + enc.AddByteString(f.Key, f.Bytes) case Complex128Type: enc.AddComplex128(f.Key, f.Interface.(complex128)) case Complex64Type: diff --git a/zapcore/field_test.go b/zapcore/field_test.go index 16a1ff5d0..ca9c59c47 100644 --- a/zapcore/field_test.go +++ b/zapcore/field_test.go @@ -100,14 +100,15 @@ func TestFields(t *testing.T) { t FieldType i int64 s string + b []byte iface interface{} want interface{} }{ {t: ArrayMarshalerType, iface: users(2), want: []interface{}{"user", "user"}}, {t: ObjectMarshalerType, iface: users(2), want: map[string]interface{}{"users": 2}}, - {t: BinaryType, iface: []byte("foo"), want: []byte("foo")}, + {t: BinaryType, b: []byte("foo"), want: []byte("foo")}, + {t: ByteStringType, b: []byte("foo"), want: []byte("foo")}, {t: BoolType, i: 0, want: false}, - {t: ByteStringType, iface: []byte("foo"), want: []byte("foo")}, {t: Complex128Type, iface: 1 + 2i, want: 1 + 2i}, {t: Complex64Type, iface: complex64(1 + 2i), want: complex64(1 + 2i)}, {t: DurationType, i: 1000, want: time.Microsecond}, @@ -133,7 +134,7 @@ func TestFields(t *testing.T) { for _, tt := range tests { enc := NewMapObjectEncoder() - f := Field{Key: "k", Type: tt.t, Integer: tt.i, Interface: tt.iface, String: tt.s} + f := Field{Key: "k", Type: tt.t, Integer: tt.i, Interface: tt.iface, Bytes: tt.b, String: tt.s} f.AddTo(enc) assert.Equal(t, tt.want, enc.Fields["k"], "Unexpected output from field %+v.", f) From ee7e325a4bb39d2fb4625249727cec37c739e9d2 Mon Sep 17 00:00:00 2001 From: Akshay Shah Date: Tue, 14 Mar 2017 14:37:25 -0700 Subject: [PATCH 5/6] Revert "Add Bytes to Field union" This reverts commit c7ba68060c87ecc35b4618b6ffeeb3c940802415. --- field.go | 4 ++-- field_test.go | 4 ++-- zapcore/field.go | 5 ++--- zapcore/field_test.go | 7 +++---- 4 files changed, 9 insertions(+), 11 deletions(-) diff --git a/field.go b/field.go index 5dc62bc11..9a8193266 100644 --- a/field.go +++ b/field.go @@ -39,7 +39,7 @@ func Skip() zapcore.Field { // zap's JSON encoder base64-encodes binary blobs. To log UTF-8 encoded text, // use ByteString. func Binary(key string, val []byte) zapcore.Field { - return zapcore.Field{Key: key, Type: zapcore.BinaryType, Bytes: val} + return zapcore.Field{Key: key, Type: zapcore.BinaryType, Interface: val} } // Bool constructs a field that carries a bool. @@ -55,7 +55,7 @@ func Bool(key string, val bool) zapcore.Field { // To log opaque binary blobs (which aren't necessarily valid UTF-8), use // Binary. func ByteString(key string, val []byte) zapcore.Field { - return zapcore.Field{Key: key, Type: zapcore.ByteStringType, Bytes: val} + return zapcore.Field{Key: key, Type: zapcore.ByteStringType, Interface: val} } // Complex128 constructs a field that carries a complex number. Unlike most diff --git a/field_test.go b/field_test.go index ef362673a..c932c319d 100644 --- a/field_test.go +++ b/field_test.go @@ -71,10 +71,10 @@ func TestFieldConstructors(t *testing.T) { expect zapcore.Field }{ {"Skip", zapcore.Field{Type: zapcore.SkipType}, Skip()}, - {"Binary", zapcore.Field{Key: "k", Type: zapcore.BinaryType, Bytes: []byte("ab12")}, Binary("k", []byte("ab12"))}, + {"Binary", zapcore.Field{Key: "k", Type: zapcore.BinaryType, Interface: []byte("ab12")}, Binary("k", []byte("ab12"))}, {"Bool", zapcore.Field{Key: "k", Type: zapcore.BoolType, Integer: 1}, Bool("k", true)}, {"Bool", zapcore.Field{Key: "k", Type: zapcore.BoolType, Integer: 1}, Bool("k", true)}, - {"ByteString", zapcore.Field{Key: "k", Type: zapcore.ByteStringType, Bytes: []byte("ab12")}, ByteString("k", []byte("ab12"))}, + {"ByteString", zapcore.Field{Key: "k", Type: zapcore.ByteStringType, Interface: []byte("ab12")}, ByteString("k", []byte("ab12"))}, {"Complex128", zapcore.Field{Key: "k", Type: zapcore.Complex128Type, Interface: 1 + 2i}, Complex128("k", 1+2i)}, {"Complex64", zapcore.Field{Key: "k", Type: zapcore.Complex64Type, Interface: complex64(1 + 2i)}, Complex64("k", 1+2i)}, {"Duration", zapcore.Field{Key: "k", Type: zapcore.DurationType, Integer: 1}, Duration("k", 1)}, diff --git a/zapcore/field.go b/zapcore/field.go index e1aeb1ece..6922e9437 100644 --- a/zapcore/field.go +++ b/zapcore/field.go @@ -97,7 +97,6 @@ type Field struct { Type FieldType Integer int64 String string - Bytes []byte Interface interface{} } @@ -112,11 +111,11 @@ func (f Field) AddTo(enc ObjectEncoder) { case ObjectMarshalerType: err = enc.AddObject(f.Key, f.Interface.(ObjectMarshaler)) case BinaryType: - enc.AddBinary(f.Key, f.Bytes) + enc.AddBinary(f.Key, f.Interface.([]byte)) case BoolType: enc.AddBool(f.Key, f.Integer == 1) case ByteStringType: - enc.AddByteString(f.Key, f.Bytes) + enc.AddByteString(f.Key, f.Interface.([]byte)) case Complex128Type: enc.AddComplex128(f.Key, f.Interface.(complex128)) case Complex64Type: diff --git a/zapcore/field_test.go b/zapcore/field_test.go index ca9c59c47..16a1ff5d0 100644 --- a/zapcore/field_test.go +++ b/zapcore/field_test.go @@ -100,15 +100,14 @@ func TestFields(t *testing.T) { t FieldType i int64 s string - b []byte iface interface{} want interface{} }{ {t: ArrayMarshalerType, iface: users(2), want: []interface{}{"user", "user"}}, {t: ObjectMarshalerType, iface: users(2), want: map[string]interface{}{"users": 2}}, - {t: BinaryType, b: []byte("foo"), want: []byte("foo")}, - {t: ByteStringType, b: []byte("foo"), want: []byte("foo")}, + {t: BinaryType, iface: []byte("foo"), want: []byte("foo")}, {t: BoolType, i: 0, want: false}, + {t: ByteStringType, iface: []byte("foo"), want: []byte("foo")}, {t: Complex128Type, iface: 1 + 2i, want: 1 + 2i}, {t: Complex64Type, iface: complex64(1 + 2i), want: complex64(1 + 2i)}, {t: DurationType, i: 1000, want: time.Microsecond}, @@ -134,7 +133,7 @@ func TestFields(t *testing.T) { for _, tt := range tests { enc := NewMapObjectEncoder() - f := Field{Key: "k", Type: tt.t, Integer: tt.i, Interface: tt.iface, Bytes: tt.b, String: tt.s} + f := Field{Key: "k", Type: tt.t, Integer: tt.i, Interface: tt.iface, String: tt.s} f.AddTo(enc) assert.Equal(t, tt.want, enc.Fields["k"], "Unexpected output from field %+v.", f) From 3de17c0f2fe9e29146380243bb9d87c9eaaf09e5 Mon Sep 17 00:00:00 2001 From: Akshay Shah Date: Tue, 14 Mar 2017 14:38:49 -0700 Subject: [PATCH 6/6] Small comment fix --- zapcore/field.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zapcore/field.go b/zapcore/field.go index 6922e9437..063d7ba31 100644 --- a/zapcore/field.go +++ b/zapcore/field.go @@ -41,7 +41,7 @@ const ( BinaryType // BoolType indicates that the field carries a bool. BoolType - // ByteStringType indicates that the field carries a UTF-8 encoded bytes. + // ByteStringType indicates that the field carries UTF-8 encoded bytes. ByteStringType // Complex128Type indicates that the field carries a complex128. Complex128Type