diff --git a/Makefile b/Makefile index b248e17f4..4258d2377 100644 --- a/Makefile +++ b/Makefile @@ -3,7 +3,7 @@ export GO15VENDOREXPERIMENT=1 BENCH_FLAGS ?= -cpuprofile=cpu.pprof -memprofile=mem.pprof -benchmem PKGS ?= $(shell glide novendor) # Many Go tools take file globs or directories as arguments instead of packages. -PKG_FILES ?= *.go zapcore benchmarks testutils internal/buffers internal/exit +PKG_FILES ?= *.go zapcore benchmarks testutils internal/buffers internal/exit internal/multierror # The linting tools evolve with each Go version, so run them only on the latest # stable release. diff --git a/zapcore/object_encoder.go b/array.go similarity index 60% rename from zapcore/object_encoder.go rename to array.go index 4a546ab5c..7387c628e 100644 --- a/zapcore/object_encoder.go +++ b/array.go @@ -18,19 +18,27 @@ // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN // THE SOFTWARE. -package zapcore +package zap -// ObjectEncoder is an encoding-agnostic interface to add structured data to the -// logging context. Like maps, ObjectEncoders aren't safe for concurrent use -// (though typical use shouldn't require locks). -type ObjectEncoder interface { - AddBool(key string, value bool) - AddFloat64(key string, value float64) - AddInt64(key string, value int64) - AddUint64(key string, value uint64) - AddObject(key string, marshaler ObjectMarshaler) error - // AddReflected uses reflection to serialize arbitrary objects, so it's slow - // and allocation-heavy. - AddReflected(key string, value interface{}) error - AddString(key, value string) +import "go.uber.org/zap/zapcore" + +// Array constructs a field with the given key and ArrayMarshaler. It provides +// a flexible, but still type-safe and efficient, way to add array-like types +// to the logging context. The struct's MarshalLogArray method is called lazily. +func Array(key string, val zapcore.ArrayMarshaler) zapcore.Field { + return zapcore.Field{Key: key, Type: zapcore.ArrayMarshalerType, Interface: val} +} + +// Bools constructs a field that carries a slice of bools. +func Bools(key string, bs []bool) zapcore.Field { + return Array(key, bools(bs)) +} + +type bools []bool + +func (bs bools) MarshalLogArray(arr zapcore.ArrayEncoder) error { + for i := range bs { + arr.AppendBool(bs[i]) + } + return nil } diff --git a/array_test.go b/array_test.go new file mode 100644 index 000000000..6efd1d68f --- /dev/null +++ b/array_test.go @@ -0,0 +1,67 @@ +// 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 zap + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "go.uber.org/zap/zapcore" +) + +func BenchmarkBoolsArrayMarshaler(b *testing.B) { + // Keep this benchmark here to capture the overhead of the ArrayMarshaler + // wrapper. + bs := make([]bool, 50) + enc := zapcore.NewJSONEncoder(zapcore.JSONConfig{}) + b.ResetTimer() + for i := 0; i < b.N; i++ { + Bools("array", bs).AddTo(enc.Clone()) + } +} + +func BenchmarkBoolsReflect(b *testing.B) { + bs := make([]bool, 50) + enc := zapcore.NewJSONEncoder(zapcore.JSONConfig{}) + b.ResetTimer() + for i := 0; i < b.N; i++ { + Reflect("array", bs).AddTo(enc.Clone()) + } +} + +func TestArrayWrappers(t *testing.T) { + tests := []struct { + desc string + field zapcore.Field + expected []interface{} + }{ + {"empty bools", Bools("", []bool{}), []interface{}(nil)}, + {"bools", Bools("", []bool{true, false}), []interface{}{true, false}}, + } + + for _, tt := range tests { + enc := make(zapcore.MapObjectEncoder) + tt.field.Key = "k" + tt.field.AddTo(enc) + assert.Equal(t, tt.expected, enc["k"], "%s: unexpected map contents.", tt.desc) + assert.Equal(t, 1, len(enc), "%s: found extra keys in map: %v", tt.desc, enc) + } +} diff --git a/field.go b/field.go index f69284120..fc6b4ee86 100644 --- a/field.go +++ b/field.go @@ -133,9 +133,9 @@ func Duration(key string, val time.Duration) zapcore.Field { } // Object constructs a field with the given key and ObjectMarshaler. It -// provides a flexible, but still type-safe and efficient, way to add -// user-defined types to the logging context. The struct's MarshalLogObject -// method is called lazily. +// provides a flexible, but still type-safe and efficient, way to add map- or +// struct-like user-defined types to the logging context. The struct's +// MarshalLogObject method is called lazily. func Object(key string, val zapcore.ObjectMarshaler) zapcore.Field { return zapcore.Field{Key: key, Type: zapcore.ObjectMarshalerType, Interface: val} } diff --git a/internal/multierror/multierror.go b/internal/multierror/multierror.go new file mode 100644 index 000000000..ccf84a024 --- /dev/null +++ b/internal/multierror/multierror.go @@ -0,0 +1,71 @@ +// 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 multierror provides a simple way to treat a collection of errors as +// a single error. +package multierror + +import "go.uber.org/zap/internal/buffers" + +// implement the standard lib's error interface on a private type so that we +// can't forget to call Error.AsError(). +type errSlice []error + +func (es errSlice) Error() string { + b := buffers.Get() + for i, err := range es { + if i > 0 { + b = append(b, ';', ' ') + } + b = append(b, err.Error()...) + } + ret := string(b) + buffers.Put(b) + return ret +} + +// Error wraps a []error to implement the error interface. +type Error struct { + errs errSlice +} + +// AsError converts the collection to a single error value. +// +// Note that failing to use AsError will almost certainly lead to bugs with +// non-nil interfaces containing nil concrete values. +func (e Error) AsError() error { + switch len(e.errs) { + case 0: + return nil + case 1: + return e.errs[0] + default: + return e.errs + } +} + +// Append adds an error to the collection. Adding a nil error is a no-op. +func (e Error) Append(err error) Error { + if err == nil { + return e + } + e.errs = append(e.errs, err) + return e +} diff --git a/internal/multierror/multierror_test.go b/internal/multierror/multierror_test.go new file mode 100644 index 000000000..5c3c7ea55 --- /dev/null +++ b/internal/multierror/multierror_test.go @@ -0,0 +1,74 @@ +// 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 multierror + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestErrorSliceString(t *testing.T) { + tests := []struct { + errs errSlice + expected string + }{ + {nil, ""}, + {errSlice{}, ""}, + {errSlice{errors.New("foo")}, "foo"}, + {errSlice{errors.New("foo"), errors.New("bar")}, "foo; bar"}, + } + + for _, tt := range tests { + assert.Equal(t, tt.expected, tt.errs.Error(), "Unexpected output from Error method.") + } +} + +func TestMultiErrorIsntAnError(t *testing.T) { + // Ensure that Error doesn't satisfy the standard lib's error interface, so + // that we're forced to always call AsError. + var errs interface{} = Error{} + _, ok := errs.(error) + assert.False(t, ok, "Error unexpectedly satisfies standard lib's error interface.") +} + +func TestMultiErrorAsError(t *testing.T) { + assert.Nil(t, (Error{}).AsError(), "Expected calling AsError with no accumulated errors to return nil.") + + e := errors.New("foo") + assert.Equal( + t, + e, + (Error{errSlice{e}}).AsError(), + "Expected AsError with single error to return the original error.", + ) + + m := Error{errSlice{errors.New("foo"), errors.New("bar")}} + assert.Equal(t, m.errs, m.AsError(), "Unexpected AsError output with multiple errors.") +} + +func TestErrorAppend(t *testing.T) { + foo := errors.New("foo") + bar := errors.New("bar") + multi := (Error{}).Append(nil).Append(foo).Append(nil).Append(bar) + assert.Equal(t, errSlice{foo, bar}, multi.errs, "Collected errors don't match expectations.") +} diff --git a/zapcore/encoder.go b/zapcore/encoder.go index fe0bdf1ee..d0211bffe 100644 --- a/zapcore/encoder.go +++ b/zapcore/encoder.go @@ -22,6 +22,32 @@ package zapcore import "io" +// ObjectEncoder is a strongly-typed, encoding-agnostic interface for adding a +// map- or struct-like object to the logging context. Like maps, ObjectEncoders +// aren't safe for concurrent use (though typical use shouldn't require locks). +type ObjectEncoder interface { + AddBool(key string, value bool) + AddFloat64(key string, value float64) + AddInt64(key string, value int64) + AddUint64(key string, value uint64) + AddObject(key string, marshaler ObjectMarshaler) error + AddArray(key string, marshaler ArrayMarshaler) error + // AddReflected uses reflection to serialize arbitrary objects, so it's slow + // and allocation-heavy. + AddReflected(key string, value interface{}) error + AddString(key, value string) +} + +// ArrayEncoder is a strongly-typed, encoding-agnostic interface for adding +// array-like objects to the logging context. Of note, it supports mixed-type +// arrays even though they aren't typical in Go. Like slices, ArrayEncoders +// aren't safe for concurrent use (though typical use shouldn't require locks). +type ArrayEncoder interface { + AppendArray(ArrayMarshaler) error + AppendObject(ObjectMarshaler) error + AppendBool(bool) +} + // Encoder is a format-agnostic interface for all log entry marshalers. Since // log encoders don't need to support the same wide range of use cases as // general-purpose marshalers, it's possible to make them faster and diff --git a/zapcore/entry.go b/zapcore/entry.go index fc3f0414b..aba34aba2 100644 --- a/zapcore/entry.go +++ b/zapcore/entry.go @@ -28,6 +28,7 @@ import ( "go.uber.org/zap/internal/buffers" "go.uber.org/zap/internal/exit" + "go.uber.org/zap/internal/multierror" ) var ( @@ -161,14 +162,12 @@ func (ce *CheckedEntry) Write(fields ...Field) { } ce.dirty = true - var errs multiError + var errs multierror.Error for i := range ce.facs { - if err := ce.facs[i].Write(ce.Entry, fields); err != nil { - errs = append(errs, err) - } + errs = errs.Append(ce.facs[i].Write(ce.Entry, fields)) } if ce.ErrorOutput != nil { - if err := errs.asError(); err != nil { + if err := errs.AsError(); err != nil { fmt.Fprintf(ce.ErrorOutput, "%v write error: %v\n", time.Now().UTC(), err) ce.ErrorOutput.Sync() } diff --git a/zapcore/field.go b/zapcore/field.go index f7f413d48..0c862487b 100644 --- a/zapcore/field.go +++ b/zapcore/field.go @@ -44,6 +44,8 @@ const ( StringType // ObjectMarshalerType indicates that the field carries an ObjectMarshaler. ObjectMarshalerType + // ArrayMarshalerType indicates that the field carries an ArrayMarshaler. + ArrayMarshalerType // ReflectType indicates that the field carries an interface{}, which should // be serialized using reflection. ReflectType @@ -86,6 +88,8 @@ func (f Field) AddTo(enc ObjectEncoder) { enc.AddString(f.Key, f.Interface.(fmt.Stringer).String()) case ObjectMarshalerType: err = enc.AddObject(f.Key, f.Interface.(ObjectMarshaler)) + case ArrayMarshalerType: + err = enc.AddArray(f.Key, f.Interface.(ArrayMarshaler)) case ReflectType: err = enc.AddReflected(f.Key, f.Interface) case ErrorType: diff --git a/zapcore/json_encoder.go b/zapcore/json_encoder.go index 81321ef3d..c4504143f 100644 --- a/zapcore/json_encoder.go +++ b/zapcore/json_encoder.go @@ -90,7 +90,7 @@ func (enc *jsonEncoder) AddString(key, val string) { func (enc *jsonEncoder) AddBool(key string, val bool) { enc.addKey(key) - enc.bytes = strconv.AppendBool(enc.bytes, val) + enc.AppendBool(val) } func (enc *jsonEncoder) AddInt64(key string, val int64) { @@ -119,10 +119,12 @@ func (enc *jsonEncoder) AddFloat64(key string, val float64) { func (enc *jsonEncoder) AddObject(key string, obj ObjectMarshaler) error { enc.addKey(key) - enc.bytes = append(enc.bytes, '{') - err := obj.MarshalLogObject(enc) - enc.bytes = append(enc.bytes, '}') - return err + return enc.AppendObject(obj) +} + +func (enc *jsonEncoder) AddArray(key string, arr ArrayMarshaler) error { + enc.addKey(key) + return enc.AppendArray(arr) } func (enc *jsonEncoder) AddReflected(key string, obj interface{}) error { @@ -135,6 +137,27 @@ func (enc *jsonEncoder) AddReflected(key string, obj interface{}) error { return nil } +func (enc *jsonEncoder) AppendArray(arr ArrayMarshaler) error { + enc.separateElements() + enc.bytes = append(enc.bytes, '[') + err := arr.MarshalLogArray(enc) + enc.bytes = append(enc.bytes, ']') + return err +} + +func (enc *jsonEncoder) AppendObject(obj ObjectMarshaler) error { + enc.separateElements() + enc.bytes = append(enc.bytes, '{') + err := obj.MarshalLogObject(enc) + enc.bytes = append(enc.bytes, '}') + return err +} + +func (enc *jsonEncoder) AppendBool(val bool) { + enc.separateElements() + enc.bytes = strconv.AppendBool(enc.bytes, val) +} + func (enc *jsonEncoder) Clone() Encoder { clone := jsonPool.Get().(*jsonEncoder) clone.truncate() @@ -193,15 +216,25 @@ func (enc *jsonEncoder) free() { } func (enc *jsonEncoder) addKey(key string) { - last := len(enc.bytes) - 1 - if last >= 0 && enc.bytes[last] != '{' { - enc.bytes = append(enc.bytes, ',') - } + enc.separateElements() enc.bytes = append(enc.bytes, '"') enc.safeAddString(key) enc.bytes = append(enc.bytes, '"', ':') } +func (enc *jsonEncoder) separateElements() { + last := len(enc.bytes) - 1 + if last < 0 { + return + } + switch enc.bytes[last] { + case '{', '[', ':', ',': + return + default: + enc.bytes = append(enc.bytes, ',') + } +} + // safeAddString JSON-escapes a string and appends it to the internal buffer. // Unlike the standard library's encoder, it doesn't attempt to protect the // user from browser vulnerabilities or JSONP-related problems. diff --git a/zapcore/json_encoder_bench_test.go b/zapcore/json_encoder_bench_test.go index 7a129c5b1..9810c3184 100644 --- a/zapcore/json_encoder_bench_test.go +++ b/zapcore/json_encoder_bench_test.go @@ -63,8 +63,8 @@ func BenchmarkZapJSON(b *testing.B) { for pb.Next() { enc := newJSONEncoder(cfg) enc.AddString("str", "foo") - enc.AddInt64("int", 1) - enc.AddInt64("int64", 1) + enc.AddInt64("int64-1", 1) + enc.AddInt64("int64-2", 2) enc.AddFloat64("float64", 1.0) enc.AddString("string1", "\n") enc.AddString("string2", "💩") @@ -92,7 +92,7 @@ func BenchmarkStandardJSON(b *testing.B) { Time: time.Unix(0, 0), Fields: map[string]interface{}{ "str": "foo", - "int64-1": int(1), + "int64-1": int64(1), "int64-2": int64(1), "float64": float64(1.0), "string1": "\n", diff --git a/zapcore/json_encoder_test.go b/zapcore/json_encoder_test.go index 632911caa..188010323 100644 --- a/zapcore/json_encoder_test.go +++ b/zapcore/json_encoder_test.go @@ -31,11 +31,38 @@ import ( "github.com/stretchr/testify/assert" + "go.uber.org/zap/internal/multierror" "go.uber.org/zap/testutils" ) var epoch = time.Date(1970, time.January, 1, 0, 0, 0, 0, time.UTC) +// Nested Array- and ObjectMarshalers. +type turducken struct{} + +func (t turducken) MarshalLogObject(enc ObjectEncoder) error { + return enc.AddArray("ducks", ArrayMarshalerFunc(func(arr ArrayEncoder) error { + for i := 0; i < 2; i++ { + arr.AppendObject(ObjectMarshalerFunc(func(inner ObjectEncoder) error { + inner.AddString("in", "chicken") + return nil + })) + } + return nil + })) +} + +type turduckens int + +func (t turduckens) MarshalLogArray(enc ArrayEncoder) error { + var errs multierror.Error + tur := turducken{} + for i := 0; i < int(t); i++ { + errs = errs.Append(enc.AppendObject(tur)) + } + return errs.AsError() +} + func newJSONEncoder(cfg JSONConfig) *jsonEncoder { return NewJSONEncoder(cfg).(*jsonEncoder) } @@ -66,6 +93,14 @@ func (l loggable) MarshalLogObject(enc ObjectEncoder) error { return nil } +func (l loggable) MarshalLogArray(enc ArrayEncoder) error { + if !l.bool { + return errors.New("can't marshal") + } + enc.AppendBool(true) + return nil +} + func assertOutput(t testing.TB, desc string, expected string, f func(Encoder)) { withJSONEncoder(func(enc *jsonEncoder) { f(enc) @@ -84,7 +119,7 @@ func assertOutput(t testing.TB, desc string, expected string, f func(Encoder)) { }) } -func TestJSONEncoderFields(t *testing.T) { +func TestJSONEncoderObjectFields(t *testing.T) { tests := []struct { desc string expected string @@ -112,13 +147,41 @@ func TestJSONEncoderFields(t *testing.T) { {"float64", `"k":"+Inf"`, func(e Encoder) { e.AddFloat64("k", math.Inf(1)) }}, {"float64", `"k":"-Inf"`, func(e Encoder) { e.AddFloat64("k", math.Inf(-1)) }}, {"ObjectMarshaler", `"k":{"loggable":"yes"}`, func(e Encoder) { - assert.NoError(t, e.AddObject("k", loggable{true}), "Unexpected error calling MarshalObject.") + assert.NoError(t, e.AddObject("k", loggable{true}), "Unexpected error calling MarshalLogObject.") }}, {"ObjectMarshaler", `"k\\":{"loggable":"yes"}`, func(e Encoder) { - assert.NoError(t, e.AddObject(`k\`, loggable{true}), "Unexpected error calling MarshalObject.") + assert.NoError(t, e.AddObject(`k\`, loggable{true}), "Unexpected error calling MarshalLogObject.") }}, {"ObjectMarshaler", `"k":{}`, func(e Encoder) { - assert.Error(t, e.AddObject("k", loggable{false}), "Expected an error calling MarshalObject.") + assert.Error(t, e.AddObject("k", loggable{false}), "Expected an error calling MarshalLogObject.") + }}, + { + "ObjectMarshaler(ArrayMarshaler(ObjectMarshaler))", + `"turducken":{"ducks":[{"in":"chicken"},{"in":"chicken"}]}`, + func(e Encoder) { + assert.NoError( + t, + e.AddObject("turducken", turducken{}), + "Unexpected error calling MarshalLogObject with nested ObjectMarshalers and ArrayMarshalers.", + ) + }, + }, + { + "ArrayMarshaler(ObjectMarshaler(ArrayMarshaler(ObjectMarshaler)))", + `"turduckens":[{"ducks":[{"in":"chicken"},{"in":"chicken"}]},{"ducks":[{"in":"chicken"},{"in":"chicken"}]}]`, + func(e Encoder) { + assert.NoError( + t, + e.AddArray("turduckens", turduckens(2)), + "Unexpected error calling MarshalLogObject with nested ObjectMarshalers and ArrayMarshalers.", + ) + }, + }, + {"ArrayMarshaler", `"k\\":[true]`, func(e Encoder) { + assert.NoError(t, e.AddArray(`k\`, loggable{true}), "Unexpected error calling MarshalLogArray.") + }}, + {"ArrayMarshaler", `"k":[]`, func(e Encoder) { + assert.Error(t, e.AddArray("k", loggable{false}), "Expected an error calling MarshalLogArray.") }}, {"arbitrary object", `"k":{"loggable":"yes"}`, func(e Encoder) { assert.NoError(t, e.AddReflected("k", map[string]string{"loggable": "yes"}), "Unexpected error JSON-serializing a map.") @@ -136,6 +199,57 @@ func TestJSONEncoderFields(t *testing.T) { } } +func TestJSONEncoderArrayTypes(t *testing.T) { + tests := []struct { + desc string + f func(ArrayEncoder) error + expected string + shouldError bool + }{ + // arrays of ObjectMarshalers are covered by the turducken test above. + { + "arrays of arrays", + func(arr ArrayEncoder) error { + arr.AppendArray(ArrayMarshalerFunc(func(enc ArrayEncoder) error { + enc.AppendBool(true) + return nil + })) + arr.AppendArray(ArrayMarshalerFunc(func(enc ArrayEncoder) error { + enc.AppendBool(true) + return nil + })) + return nil + }, + `[[true],[true]]`, + false, + }, + { + "bools", + func(arr ArrayEncoder) error { + arr.AppendBool(true) + arr.AppendBool(false) + return nil + }, + `[true,false]`, + false, + }, + } + + for _, tt := range tests { + f := func(enc Encoder) error { + return enc.AddArray("array", ArrayMarshalerFunc(tt.f)) + } + assertOutput(t, tt.desc, `"array":`+tt.expected, func(enc Encoder) { + err := f(enc) + if tt.shouldError { + assert.Error(t, err, "Expected an error adding array to JSON encoder.") + } else { + assert.NoError(t, err, "Unexpected error adding array to JSON encoder.") + } + }) + } +} + func TestJSONWriteEntry(t *testing.T) { epoch := time.Unix(0, 0) withJSONEncoder(func(enc *jsonEncoder) { diff --git a/zapcore/marshaler.go b/zapcore/marshaler.go index 5618d7283..2627a653d 100644 --- a/zapcore/marshaler.go +++ b/zapcore/marshaler.go @@ -27,7 +27,7 @@ type ObjectMarshaler interface { MarshalLogObject(ObjectEncoder) error } -// ObjectMarshalerFunc is a type adapter that allows using a function as a +// ObjectMarshalerFunc is a type adapter that turns a function into an // ObjectMarshaler. type ObjectMarshalerFunc func(ObjectEncoder) error @@ -36,4 +36,18 @@ func (f ObjectMarshalerFunc) MarshalLogObject(enc ObjectEncoder) error { return f(enc) } -// TODO: Add LogArrayMarshaler and ArrayEncoder interfaces. +// ArrayMarshaler allows user-defined types to efficiently add themselves to the +// logging context, and to selectively omit information which shouldn't be +// included in logs (e.g., passwords). +type ArrayMarshaler interface { + MarshalLogArray(ArrayEncoder) error +} + +// ArrayMarshalerFunc is a type adapter that turns a function into an +// ArrayMarshaler. +type ArrayMarshalerFunc func(ArrayEncoder) error + +// MarshalLogArray calls the underlying function. +func (f ArrayMarshalerFunc) MarshalLogArray(enc ArrayEncoder) error { + return f(enc) +} diff --git a/zapcore/map_encoder.go b/zapcore/memory_encoder.go similarity index 63% rename from zapcore/map_encoder.go rename to zapcore/memory_encoder.go index ab20d96d2..adcc44ad6 100644 --- a/zapcore/map_encoder.go +++ b/zapcore/memory_encoder.go @@ -25,30 +25,62 @@ package zapcore // helpful in tests. type MapObjectEncoder map[string]interface{} -// AddBool adds the value under the specified key to the map. +// AddBool implements ObjectEncoder. func (m MapObjectEncoder) AddBool(k string, v bool) { m[k] = v } -// AddFloat64 adds the value under the specified key to the map. +// AddFloat64 implements ObjectEncoder. func (m MapObjectEncoder) AddFloat64(k string, v float64) { m[k] = v } -// AddInt64 adds the value under the specified key to the map. +// AddInt64 implements ObjectEncoder. func (m MapObjectEncoder) AddInt64(k string, v int64) { m[k] = v } -// AddUint64 adds the value under the specified key to the map. +// AddUint64 implements ObjectEncoder. func (m MapObjectEncoder) AddUint64(k string, v uint64) { m[k] = v } -// AddReflected adds the value under the specified key to the map. +// AddReflected implements ObjectEncoder. func (m MapObjectEncoder) AddReflected(k string, v interface{}) error { m[k] = v return nil } -// AddString adds the value under the specified key to the map. +// AddString implements ObjectEncoder. func (m MapObjectEncoder) AddString(k string, v string) { m[k] = v } -// AddObject adds the value under the specified key to the map. +// AddObject implements ObjectEncoder. func (m MapObjectEncoder) AddObject(k string, v ObjectMarshaler) error { newMap := make(MapObjectEncoder) m[k] = newMap return v.MarshalLogObject(newMap) } + +// AddArray implements ObjectEncoder. +func (m MapObjectEncoder) AddArray(key string, v ArrayMarshaler) error { + arr := &sliceArrayEncoder{} + err := v.MarshalLogArray(arr) + m[key] = arr.elems + return err +} + +// sliceArrayEncoder is an ArrayEncoder backed by a simple []interface{}. Like +// the MapObjectEncoder, it's not designed for production use. +type sliceArrayEncoder struct { + elems []interface{} +} + +func (s *sliceArrayEncoder) AppendArray(v ArrayMarshaler) error { + enc := &sliceArrayEncoder{} + err := v.MarshalLogArray(enc) + s.elems = append(s.elems, enc.elems) + return err +} + +func (s *sliceArrayEncoder) AppendObject(v ObjectMarshaler) error { + m := make(MapObjectEncoder) + err := v.MarshalLogObject(m) + s.elems = append(s.elems, m) + return err +} + +func (s *sliceArrayEncoder) AppendBool(v bool) { + s.elems = append(s.elems, v) +} diff --git a/zapcore/map_encoder_test.go b/zapcore/memory_encoder_test.go similarity index 66% rename from zapcore/map_encoder_test.go rename to zapcore/memory_encoder_test.go index 8be5e01cb..bc27aa93c 100644 --- a/zapcore/map_encoder_test.go +++ b/zapcore/memory_encoder_test.go @@ -27,22 +27,45 @@ import ( "github.com/stretchr/testify/assert" ) -func TestMapEncoderAdd(t *testing.T) { +func TestMapObjectEncoderAdd(t *testing.T) { arbitraryObj := map[string]interface{}{ "foo": "bar", "baz": 5, } enc := make(MapObjectEncoder) + // ObjectEncoder methods. enc.AddBool("b", true) enc.AddFloat64("f64", 1.56) enc.AddInt64("i64", math.MaxInt64) enc.AddUint64("uint64", 42) enc.AddString("s", "string") - assert.NoError(t, enc.AddReflected("reflect", arbitraryObj), "Expected AddReflected to succeed.") assert.NoError(t, enc.AddObject("object", loggable{true}), "Expected AddObject to succeed.") + // Array types. + assert.NoError(t, enc.AddArray("array", ArrayMarshalerFunc(func(arr ArrayEncoder) error { + arr.AppendBool(true) + arr.AppendBool(false) + arr.AppendBool(true) + return nil + })), "Expected AddArray to succeed.") + assert.NoError(t, enc.AddArray("arrays-of-arrays", ArrayMarshalerFunc(func(enc ArrayEncoder) error { + enc.AppendArray(ArrayMarshalerFunc(func(e ArrayEncoder) error { + e.AppendBool(true) + return nil + })) + return nil + })), "Expected AddArray to succeed.") + // Nested objects and arrays. + assert.NoError(t, enc.AddArray("turduckens", turduckens(2)), "Expected AddObject to succeed.") + assert.NoError(t, enc.AddObject("turducken", turducken{}), "Expected AddObject to succeed.") + wantTurducken := MapObjectEncoder{ + "ducks": []interface{}{ + MapObjectEncoder{"in": "chicken"}, + MapObjectEncoder{"in": "chicken"}, + }, + } want := MapObjectEncoder{ "b": true, "f64": 1.56, @@ -53,6 +76,10 @@ func TestMapEncoderAdd(t *testing.T) { "object": MapObjectEncoder{ "loggable": "yes", }, + "array": []interface{}{true, false, true}, + "arrays-of-arrays": []interface{}{[]interface{}{true}}, + "turducken": wantTurducken, + "turduckens": []interface{}{wantTurducken, wantTurducken}, } assert.Equal(t, want, enc, "Encoder's final state is unexpected.") } diff --git a/zapcore/tee.go b/zapcore/tee.go index bf8ed297a..1e2838795 100644 --- a/zapcore/tee.go +++ b/zapcore/tee.go @@ -20,6 +20,8 @@ package zapcore +import "go.uber.org/zap/internal/multierror" + // Tee creates a Facility that duplicates log entries into two or more // facilities; if you call it with less than two, you get back the one facility // you passed (or nil in the pathological case). @@ -62,11 +64,9 @@ func (mf multiFacility) Check(ent Entry, ce *CheckedEntry) *CheckedEntry { } func (mf multiFacility) Write(ent Entry, fields []Field) error { - var errs multiError + var errs multierror.Error for i := range mf { - if err := mf[i].Write(ent, fields); err != nil { - errs = append(errs, err) - } + errs = errs.Append(mf[i].Write(ent, fields)) } - return errs.asError() + return errs.AsError() } diff --git a/zapcore/write_syncer.go b/zapcore/write_syncer.go index 03269f670..a536cd4df 100644 --- a/zapcore/write_syncer.go +++ b/zapcore/write_syncer.go @@ -21,9 +21,10 @@ package zapcore import ( - "bytes" "io" "sync" + + "go.uber.org/zap/internal/multierror" ) // A WriteSyncer is an io.Writer that can also flush any buffered data. Note @@ -97,50 +98,24 @@ func MultiWriteSyncer(ws ...WriteSyncer) WriteSyncer { // the smallest number is returned even though Write() is called on // all of them. func (ws multiWriteSyncer) Write(p []byte) (int, error) { - var errs multiError + var errs multierror.Error nWritten := 0 for _, w := range ws { n, err := w.Write(p) - if err != nil { - errs = append(errs, err) - } + errs = errs.Append(err) if nWritten == 0 && n != 0 { nWritten = n } else if n < nWritten { nWritten = n } } - return nWritten, errs.asError() + return nWritten, errs.AsError() } func (ws multiWriteSyncer) Sync() error { - var errs multiError + var errs multierror.Error for _, w := range ws { - if err := w.Sync(); err != nil { - errs = append(errs, err) - } - } - return errs.asError() -} - -type multiError []error - -func (m multiError) asError() error { - switch len(m) { - case 0: - return nil - case 1: - return m[0] - default: - return m - } -} - -func (m multiError) Error() string { - sb := bytes.Buffer{} - for _, err := range m { - sb.WriteString(err.Error()) - sb.WriteString(" ") + errs = errs.Append(w.Sync()) } - return sb.String() + return errs.AsError() } diff --git a/zapcore/write_syncer_test.go b/zapcore/write_syncer_test.go index 2275b068b..b7ecd9cd4 100644 --- a/zapcore/write_syncer_test.go +++ b/zapcore/write_syncer_test.go @@ -115,13 +115,6 @@ func TestMultiWriteSyncerSync_NoErrorsOnDiscard(t *testing.T) { assert.NoError(t, ws.Sync(), "Expected error-free sync to /dev/null") } -func TestMultiError_WrapsStrings(t *testing.T) { - err := multiError{errors.New("battlestar"), errors.New("galactaca")} - assert.Error(t, err) - assert.Contains(t, err.Error(), "battlestar") - assert.Contains(t, err.Error(), "galactaca") -} - func TestMultiWriteSyncerSync_AllCalled(t *testing.T) { failed, second := &testutils.Buffer{}, &testutils.Buffer{}