Initial support for arrays (uber-go#239)

* Add an internal multierror package

We'll need a multierror implementation in both zap and zapcore to
support arrays. Clean up the existing implementation a bit and promote
it to its own internal package.

* Add support for logging arrays

Add basic support for logging arrays. To limit the scope of this commit,
it introduces only arrays of ObjectMarshalers, arrays of arrays, and
arrays of bools. It should, however, clarify how I intend to support
arrays of other concrete types.

Of note, I opted not to have `zap.Bools` return a `Field`. Instead, it
returns a `zapcore.ArrayMarshaler`. This lets us build arrays of
heterogenous arrays. For most users, this will be papered over by the
upcoming `zap.Any(key string, val interface{}) zapcore.Field` function,
which will basically be a huge type switch.

* Simpler struct literals in tests

* Update some comments

* Add some simple benchmarks

```
BenchmarkBoolsArrayMarshaler-4   	 1000000	      1028 ns/op	 1088 B/op	       3 allocs/op
BenchmarkBoolsReflect-4          	  500000	      3293 ns/op	 2280 B/op	       8 allocs/op
```

* Cleanups from CR

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.

zapcore/object_encoder.go → 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
 }

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)
+	}
+}

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}
 }

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
+}

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.")
+}

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

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()
 		}

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: