Add Field.Equals and use it in the observer (#444)

Use reflection (sparingly) to safely compare Fields in the observer. Fixes #443.

zapcore/field.go

@@ -21,8 +21,10 @@
 package zapcore
 
 import (
+	"bytes"
 	"fmt"
 	"math"
+	"reflect"
 	"time"
 )
 
@@ -182,6 +184,26 @@ func (f Field) AddTo(enc ObjectEncoder) {
 	}
 }
 
+// Equals returns whether two fields are equal. For non-primitive types such as
+// errors, marshalers, or reflect types, it uses reflect.DeepEqual.
+func (f Field) Equals(other Field) bool {
+	if f.Type != other.Type {
+		return false
+	}
+	if f.Key != other.Key {
+		return false
+	}
+
+	switch f.Type {
+	case BinaryType, ByteStringType:
+		return bytes.Equal(f.Interface.([]byte), other.Interface.([]byte))
+	case ArrayMarshalerType, ObjectMarshalerType, ErrorType, ReflectType:
+		return reflect.DeepEqual(f.Interface, other.Interface)
+	default:
+		return f == other
+	}
+}
+
 func addFields(enc ObjectEncoder, fields []Field) {
 	for i := range fields {
 		fields[i].AddTo(enc)

zapcore/field_test.go

@@ -28,6 +28,8 @@ import (
 	"testing"
 	"time"
 
+	"go.uber.org/zap"
+
 	richErrors "github.com/pkg/errors"
 	"github.com/stretchr/testify/assert"
 
@@ -139,6 +141,8 @@ func TestFields(t *testing.T) {
 
 		delete(enc.Fields, "k")
 		assert.Equal(t, 0, len(enc.Fields), "Unexpected extra fields present.")
+
+		assert.True(t, f.Equals(f), "Field does not equal itself")
 	}
 }
 
@@ -159,3 +163,101 @@ func TestRichErrorSupport(t *testing.T) {
 	assert.Regexp(t, `failed`, serialized, "Expected error annotation to be present.")
 	assert.Regexp(t, `TestRichErrorSupport`, serialized, "Expected calling function to be present in stacktrace.")
 }
+
+func TestEquals(t *testing.T) {
+	tests := []struct {
+		a, b Field
+		want bool
+	}{
+		{
+			a:    zap.Int16("a", 1),
+			b:    zap.Int32("a", 1),
+			want: false,
+		},
+		{
+			a:    zap.String("k", "a"),
+			b:    zap.String("k", "a"),
+			want: true,
+		},
+		{
+			a:    zap.String("k", "a"),
+			b:    zap.String("k2", "a"),
+			want: false,
+		},
+		{
+			a:    zap.String("k", "a"),
+			b:    zap.String("k", "b"),
+			want: false,
+		},
+		{
+			a:    zap.Time("k", time.Unix(1000, 1000)),
+			b:    zap.Time("k", time.Unix(1000, 1000)),
+			want: true,
+		},
+		{
+			a:    zap.Time("k", time.Unix(1000, 1000).In(time.UTC)),
+			b:    zap.Time("k", time.Unix(1000, 1000).In(time.FixedZone("TEST", -8))),
+			want: false,
+		},
+		{
+			a:    zap.Time("k", time.Unix(1000, 1000)),
+			b:    zap.Time("k", time.Unix(1000, 2000)),
+			want: false,
+		},
+		{
+			a:    zap.Binary("k", []byte{1, 2}),
+			b:    zap.Binary("k", []byte{1, 2}),
+			want: true,
+		},
+		{
+			a:    zap.Binary("k", []byte{1, 2}),
+			b:    zap.Binary("k", []byte{1, 3}),
+			want: false,
+		},
+		{
+			a:    zap.ByteString("k", []byte("abc")),
+			b:    zap.ByteString("k", []byte("abc")),
+			want: true,
+		},
+		{
+			a:    zap.ByteString("k", []byte("abc")),
+			b:    zap.ByteString("k", []byte("abd")),
+			want: false,
+		},
+		{
+			a:    zap.Ints("k", []int{1, 2}),
+			b:    zap.Ints("k", []int{1, 2}),
+			want: true,
+		},
+		{
+			a:    zap.Ints("k", []int{1, 2}),
+			b:    zap.Ints("k", []int{1, 3}),
+			want: false,
+		},
+		{
+			a:    zap.Object("k", users(10)),
+			b:    zap.Object("k", users(10)),
+			want: true,
+		},
+		{
+			a:    zap.Object("k", users(10)),
+			b:    zap.Object("k", users(20)),
+			want: false,
+		},
+		{
+			a:    zap.Any("k", map[string]string{"a": "b"}),
+			b:    zap.Any("k", map[string]string{"a": "b"}),
+			want: true,
+		},
+		{
+			a:    zap.Any("k", map[string]string{"a": "b"}),
+			b:    zap.Any("k", map[string]string{"a": "d"}),
+			want: false,
+		},
+	}
+
+	for _, tt := range tests {
+		assert.Equal(t, tt.want, tt.a.Equals(tt.b), "a.Equals(b) a: %#v b: %#v", tt.a, tt.b)
+		assert.Equal(t, tt.want, tt.b.Equals(tt.a), "b.Equals(a) a: %#v b: %#v", tt.a, tt.b)
+	}
+}

zaptest/observer/observer.go

@@ -99,7 +99,7 @@ func (o *ObservedLogs) FilterMessageSnippet(snippet string) *ObservedLogs {
 func (o *ObservedLogs) FilterField(field zapcore.Field) *ObservedLogs {
 	return o.filter(func(e LoggedEntry) bool {
 		for _, ctxField := range e.Context {
-			if ctxField == field {
+			if ctxField.Equals(field) {
 				return true
 			}
 		}

zaptest/observer/observer_test.go

@@ -141,6 +141,14 @@ func TestFilters(t *testing.T) {
 			Entry:   zapcore.Entry{Level: zap.InfoLevel, Message: "msg 1"},
 			Context: []zapcore.Field{zap.Int("a", 1), zap.Namespace("ns")},
 		},
+		{
+			Entry:   zapcore.Entry{Level: zap.InfoLevel, Message: "any map"},
+			Context: []zapcore.Field{zap.Any("map", map[string]string{"a": "b"})},
+		},
+		{
+			Entry:   zapcore.Entry{Level: zap.InfoLevel, Message: "any slice"},
+			Context: []zapcore.Field{zap.Any("slice", []string{"a"})},
+		},
 	}
 
 	logger, sink := New(zap.InfoLevel)
@@ -188,6 +196,16 @@ func TestFilters(t *testing.T) {
 			filtered: sink.FilterMessageSnippet("a").FilterField(zap.Int("b", 2)),
 			want:     logs[1:2],
 		},
+		{
+			msg:      "filter for map",
+			filtered: sink.FilterField(zap.Any("map", map[string]string{"a": "b"})),
+			want:     logs[5:6],
+		},
+		{
+			msg:      "filter for slice",
+			filtered: sink.FilterField(zap.Any("slice", []string{"a"})),
+			want:     logs[6:7],
+		},
 	}
 
 	for _, tt := range tests {