From 65adfd88ecb983005c9f735d207e11d63216ca00 Mon Sep 17 00:00:00 2001 From: hn8 <10730886+hn8@users.noreply.github.com> Date: Thu, 9 Sep 2021 00:00:06 +0800 Subject: [PATCH] Make Fields method accept both map and slice (#352) --- benchmark_test.go | 40 ++++++++++++------------- binary_test.go | 72 +++++++++++++++++++++++++++++++++++++++++++++ context.go | 6 ++-- event.go | 6 ++-- fields.go | 37 ++++++++++++++++++----- log_example_test.go | 64 ++++++++++++++++++++++++++++++++++++++++ log_test.go | 64 ++++++++++++++++++++++++++++++++++++++-- 7 files changed, 255 insertions(+), 34 deletions(-) diff --git a/benchmark_test.go b/benchmark_test.go index 7fc93e2d..9aa41008 100644 --- a/benchmark_test.go +++ b/benchmark_test.go @@ -148,16 +148,16 @@ func BenchmarkLogFieldType(b *testing.B) { {"a", "a", 0}, } objects := []obj{ - obj{"a", "a", 0}, - obj{"a", "a", 0}, - obj{"a", "a", 0}, - obj{"a", "a", 0}, - obj{"a", "a", 0}, - obj{"a", "a", 0}, - obj{"a", "a", 0}, - obj{"a", "a", 0}, - obj{"a", "a", 0}, - obj{"a", "a", 0}, + {"a", "a", 0}, + {"a", "a", 0}, + {"a", "a", 0}, + {"a", "a", 0}, + {"a", "a", 0}, + {"a", "a", 0}, + {"a", "a", 0}, + {"a", "a", 0}, + {"a", "a", 0}, + {"a", "a", 0}, } errs := []error{errors.New("a"), errors.New("b"), errors.New("c"), errors.New("d"), errors.New("e")} types := map[string]func(e *Event) *Event{ @@ -272,16 +272,16 @@ func BenchmarkContextFieldType(b *testing.B) { {"a", "a", 0}, } objects := []obj{ - obj{"a", "a", 0}, - obj{"a", "a", 0}, - obj{"a", "a", 0}, - obj{"a", "a", 0}, - obj{"a", "a", 0}, - obj{"a", "a", 0}, - obj{"a", "a", 0}, - obj{"a", "a", 0}, - obj{"a", "a", 0}, - obj{"a", "a", 0}, + {"a", "a", 0}, + {"a", "a", 0}, + {"a", "a", 0}, + {"a", "a", 0}, + {"a", "a", 0}, + {"a", "a", 0}, + {"a", "a", 0}, + {"a", "a", 0}, + {"a", "a", 0}, + {"a", "a", 0}, } errs := []error{errors.New("a"), errors.New("b"), errors.New("c"), errors.New("d"), errors.New("e")} types := map[string]func(c Context) Context{ diff --git a/binary_test.go b/binary_test.go index 79cfa9cc..b4882d7b 100644 --- a/binary_test.go +++ b/binary_test.go @@ -364,6 +364,42 @@ func ExampleEvent_Durs() { // Output: {"foo":"bar","durs":[10000,20000],"message":"hello world"} } +func ExampleEvent_Fields_map() { + fields := map[string]interface{}{ + "bar": "baz", + "n": 1, + } + + dst := bytes.Buffer{} + log := New(&dst) + + log.Log(). + Str("foo", "bar"). + Fields(fields). + Msg("hello world") + + fmt.Println(decodeIfBinaryToString(dst.Bytes())) + // Output: {"foo":"bar","bar":"baz","n":1,"message":"hello world"} +} + +func ExampleEvent_Fields_slice() { + fields := []interface{}{ + "bar", "baz", + "n", 1, + } + + dst := bytes.Buffer{} + log := New(&dst) + + log.Log(). + Str("foo", "bar"). + Fields(fields). + Msg("hello world") + + fmt.Println(decodeIfBinaryToString(dst.Bytes())) + // Output: {"foo":"bar","bar":"baz","n":1,"message":"hello world"} +} + func ExampleContext_Dict() { dst := bytes.Buffer{} log := New(&dst).With(). @@ -510,3 +546,39 @@ func ExampleContext_Durs() { fmt.Println(decodeIfBinaryToString(dst.Bytes())) // Output: {"foo":"bar","durs":[10000,20000],"message":"hello world"} } + +func ExampleContext_Fields_map() { + fields := map[string]interface{}{ + "bar": "baz", + "n": 1, + } + + dst := bytes.Buffer{} + log := New(&dst).With(). + Str("foo", "bar"). + Fields(fields). + Logger() + + log.Log().Msg("hello world") + + fmt.Println(decodeIfBinaryToString(dst.Bytes())) + // Output: {"foo":"bar","bar":"baz","n":1,"message":"hello world"} +} + +func ExampleContext_Fields_slice() { + fields := []interface{}{ + "bar", "baz", + "n", 1, + } + + dst := bytes.Buffer{} + log := New(&dst).With(). + Str("foo", "bar"). + Fields(fields). + Logger() + + log.Log().Msg("hello world") + + fmt.Println(decodeIfBinaryToString(dst.Bytes())) + // Output: {"foo":"bar","bar":"baz","n":1,"message":"hello world"} +} diff --git a/context.go b/context.go index 7cdd8cc2..f398e319 100644 --- a/context.go +++ b/context.go @@ -18,8 +18,10 @@ func (c Context) Logger() Logger { return c.l } -// Fields is a helper function to use a map to set fields using type assertion. -func (c Context) Fields(fields map[string]interface{}) Context { +// Fields is a helper function to use a map or slice to set fields using type assertion. +// Only map[string]interface{} and []interface{} are accepted. []interface{} must +// alternate string keys and arbitrary values, and extraneous ones are ignored. +func (c Context) Fields(fields interface{}) Context { c.l.context = appendFields(c.l.context, fields) return c } diff --git a/event.go b/event.go index 5a6d2a3e..6aaa618e 100644 --- a/event.go +++ b/event.go @@ -148,8 +148,10 @@ func (e *Event) msg(msg string) { } } -// Fields is a helper function to use a map to set fields using type assertion. -func (e *Event) Fields(fields map[string]interface{}) *Event { +// Fields is a helper function to use a map or slice to set fields using type assertion. +// Only map[string]interface{} and []interface{} are accepted. []interface{} must +// alternate string keys and arbitrary values, and extraneous ones are ignored. +func (e *Event) Fields(fields interface{}) *Event { if e == nil { return e } diff --git a/fields.go b/fields.go index 907b3f04..c1eb5ce7 100644 --- a/fields.go +++ b/fields.go @@ -12,15 +12,36 @@ func isNilValue(i interface{}) bool { return (*[2]uintptr)(unsafe.Pointer(&i))[1] == 0 } -func appendFields(dst []byte, fields map[string]interface{}) []byte { - keys := make([]string, 0, len(fields)) - for key := range fields { - keys = append(keys, key) +func appendFields(dst []byte, fields interface{}) []byte { + switch fields := fields.(type) { + case []interface{}: + if n := len(fields); n&0x1 == 1 { // odd number + fields = fields[:n-1] + } + dst = appendFieldList(dst, fields) + case map[string]interface{}: + keys := make([]string, 0, len(fields)) + for key := range fields { + keys = append(keys, key) + } + sort.Strings(keys) + kv := make([]interface{}, 2) + for _, key := range keys { + kv[0], kv[1] = key, fields[key] + dst = appendFieldList(dst, kv) + } } - sort.Strings(keys) - for _, key := range keys { - dst = enc.AppendKey(dst, key) - val := fields[key] + return dst +} + +func appendFieldList(dst []byte, kvList []interface{}) []byte { + for i, n := 0, len(kvList); i < n; i += 2 { + key, val := kvList[i], kvList[i+1] + if key, ok := key.(string); ok { + dst = enc.AppendKey(dst, key) + } else { + continue + } if val, ok := val.(LogObjectMarshaler); ok { e := newEvent(nil, 0) e.buf = e.buf[:0] diff --git a/log_example_test.go b/log_example_test.go index 71fd217d..a41fdbb4 100644 --- a/log_example_test.go +++ b/log_example_test.go @@ -335,6 +335,38 @@ func ExampleEvent_Durs() { // Output: {"foo":"bar","durs":[10000,20000],"message":"hello world"} } +func ExampleEvent_Fields_map() { + fields := map[string]interface{}{ + "bar": "baz", + "n": 1, + } + + log := zerolog.New(os.Stdout) + + log.Log(). + Str("foo", "bar"). + Fields(fields). + Msg("hello world") + + // Output: {"foo":"bar","bar":"baz","n":1,"message":"hello world"} +} + +func ExampleEvent_Fields_slice() { + fields := []interface{}{ + "bar", "baz", + "n", 1, + } + + log := zerolog.New(os.Stdout) + + log.Log(). + Str("foo", "bar"). + Fields(fields). + Msg("hello world") + + // Output: {"foo":"bar","bar":"baz","n":1,"message":"hello world"} +} + func ExampleContext_Dict() { log := zerolog.New(os.Stdout).With(). Str("foo", "bar"). @@ -484,3 +516,35 @@ func ExampleContext_MacAddr() { // Output: {"hostMAC":"00:14:22:01:23:45","message":"hello world"} } + +func ExampleContext_Fields_map() { + fields := map[string]interface{}{ + "bar": "baz", + "n": 1, + } + + log := zerolog.New(os.Stdout).With(). + Str("foo", "bar"). + Fields(fields). + Logger() + + log.Log().Msg("hello world") + + // Output: {"foo":"bar","bar":"baz","n":1,"message":"hello world"} +} + +func ExampleContext_Fields_slice() { + fields := []interface{}{ + "bar", "baz", + "n", 1, + } + + log := zerolog.New(os.Stdout).With(). + Str("foo", "bar"). + Fields(fields). + Logger() + + log.Log().Msg("hello world") + + // Output: {"foo":"bar","bar":"baz","n":1,"message":"hello world"} +} diff --git a/log_test.go b/log_test.go index e22e9269..8f49c8fe 100644 --- a/log_test.go +++ b/log_test.go @@ -246,6 +246,66 @@ func TestFieldsMapNilPnt(t *testing.T) { } } +func TestFieldsSlice(t *testing.T) { + out := &bytes.Buffer{} + log := New(out) + log.Log().Fields([]interface{}{ + "nil", nil, + "string", "foo", + "bytes", []byte("bar"), + "error", errors.New("some error"), + "bool", true, + "int", int(1), + "int8", int8(2), + "int16", int16(3), + "int32", int32(4), + "int64", int64(5), + "uint", uint(6), + "uint8", uint8(7), + "uint16", uint16(8), + "uint32", uint32(9), + "uint64", uint64(10), + "float32", float32(11), + "float64", float64(12), + "ipv6", net.IP{0x20, 0x01, 0x0d, 0xb8, 0x85, 0xa3, 0x00, 0x00, 0x00, 0x00, 0x8a, 0x2e, 0x03, 0x70, 0x73, 0x34}, + "dur", 1 * time.Second, + "time", time.Time{}, + "obj", obj{"a", "b", 1}, + }).Msg("") + if got, want := decodeIfBinaryToString(out.Bytes()), `{"nil":null,"string":"foo","bytes":"bar","error":"some error","bool":true,"int":1,"int8":2,"int16":3,"int32":4,"int64":5,"uint":6,"uint8":7,"uint16":8,"uint32":9,"uint64":10,"float32":11,"float64":12,"ipv6":"2001:db8:85a3::8a2e:370:7334","dur":1000,"time":"0001-01-01T00:00:00Z","obj":{"Pub":"a","Tag":"b","priv":1}}`+"\n"; got != want { + t.Errorf("invalid log output:\ngot: %v\nwant: %v", got, want) + } +} + +func TestFieldsSliceExtraneous(t *testing.T) { + out := &bytes.Buffer{} + log := New(out) + log.Log().Fields([]interface{}{ + "string", "foo", + "error", errors.New("some error"), + 32, "valueForNonStringKey", + "bool", true, + "int", int(1), + "keyWithoutValue", + }).Msg("") + if got, want := decodeIfBinaryToString(out.Bytes()), `{"string":"foo","error":"some error","bool":true,"int":1}`+"\n"; got != want { + t.Errorf("invalid log output:\ngot: %v\nwant: %v", got, want) + } +} + +func TestFieldsNotMapSlice(t *testing.T) { + out := &bytes.Buffer{} + log := New(out) + log.Log(). + Fields(obj{"a", "b", 1}). + Fields("string"). + Fields(1). + Msg("") + if got, want := decodeIfBinaryToString(out.Bytes()), `{}`+"\n"; got != want { + t.Errorf("invalid log output:\ngot: %v\nwant: %v", got, want) + } +} + func TestFields(t *testing.T) { out := &bytes.Buffer{} log := New(out) @@ -336,7 +396,7 @@ func TestFieldsArraySingleElement(t *testing.T) { Floats32("float32", []float32{11}). Floats64("float64", []float64{12}). Durs("dur", []time.Duration{1 * time.Second}). - Times("time", []time.Time{time.Time{}}). + Times("time", []time.Time{{}}). Msg("") if got, want := decodeIfBinaryToString(out.Bytes()), `{"string":["foo"],"err":["some error"],"bool":[true],"int":[1],"int8":[2],"int16":[3],"int32":[4],"int64":[5],"uint":[6],"uint8":[7],"uint16":[8],"uint32":[9],"uint64":[10],"float32":[11],"float64":[12],"dur":[1000],"time":["0001-01-01T00:00:00Z"]}`+"\n"; got != want { t.Errorf("invalid log output:\ngot: %v\nwant: %v", got, want) @@ -363,7 +423,7 @@ func TestFieldsArrayMultipleElement(t *testing.T) { Floats32("float32", []float32{11, 0}). Floats64("float64", []float64{12, 0}). Durs("dur", []time.Duration{1 * time.Second, 0}). - Times("time", []time.Time{time.Time{}, time.Time{}}). + Times("time", []time.Time{{}, {}}). Msg("") if got, want := decodeIfBinaryToString(out.Bytes()), `{"string":["foo","bar"],"err":["some error",null],"bool":[true,false],"int":[1,0],"int8":[2,0],"int16":[3,0],"int32":[4,0],"int64":[5,0],"uint":[6,0],"uint8":[7,0],"uint16":[8,0],"uint32":[9,0],"uint64":[10,0],"float32":[11,0],"float64":[12,0],"dur":[1000,0],"time":["0001-01-01T00:00:00Z","0001-01-01T00:00:00Z"]}`+"\n"; got != want { t.Errorf("invalid log output:\ngot: %v\nwant: %v", got, want)