From 9b86a50a3e27e0e12ccb6b47288de27df7fd3d5b Mon Sep 17 00:00:00 2001 From: Craig Pastro Date: Sat, 15 Oct 2022 09:13:25 -0700 Subject: [PATCH] SugaredLogger: Turn error into zap.Error (#1185) --- sugar.go | 24 ++++++++++++++++++++---- sugar_test.go | 27 +++++++++++++++++++++++---- 2 files changed, 43 insertions(+), 8 deletions(-) diff --git a/sugar.go b/sugar.go index e42b694ed..ac387b3e4 100644 --- a/sugar.go +++ b/sugar.go @@ -31,6 +31,7 @@ import ( const ( _oddNumberErrMsg = "Ignored key without a value." _nonStringKeyErrMsg = "Ignored key-value pairs with non-string keys." + _multipleErrMsg = "Multiple errors without a key." ) // A SugaredLogger wraps the base Logger functionality in a slower, but less @@ -336,10 +337,13 @@ func (s *SugaredLogger) sweetenFields(args []interface{}) []Field { return nil } - // Allocate enough space for the worst case; if users pass only structured - // fields, we shouldn't penalize them with extra allocations. - fields := make([]Field, 0, len(args)) - var invalid invalidPairs + var ( + // Allocate enough space for the worst case; if users pass only structured + // fields, we shouldn't penalize them with extra allocations. + fields = make([]Field, 0, len(args)) + invalid invalidPairs + seenError bool + ) for i := 0; i < len(args); { // This is a strongly-typed field. Consume it and move on. @@ -349,6 +353,18 @@ func (s *SugaredLogger) sweetenFields(args []interface{}) []Field { continue } + // If it is an error, consume it and move on. + if err, ok := args[i].(error); ok { + if !seenError { + seenError = true + fields = append(fields, Error(err)) + } else { + s.base.Error(_multipleErrMsg, Error(err)) + } + i++ + continue + } + // Make sure this element isn't a dangling key. if i == len(args)-1 { s.base.Error(_oddNumberErrMsg, Any("ignored", args[i])) diff --git a/sugar_test.go b/sugar_test.go index 9c067cbab..602c4326a 100644 --- a/sugar_test.go +++ b/sugar_test.go @@ -21,6 +21,7 @@ package zap import ( + "errors" "testing" "go.uber.org/zap/internal/exit" @@ -46,6 +47,12 @@ func TestSugarWith(t *testing.T) { Context: []Field{Array("invalid", invalidPairs(pairs))}, } } + ignoredError := func(err error) observer.LoggedEntry { + return observer.LoggedEntry{ + Entry: zapcore.Entry{Level: ErrorLevel, Message: _multipleErrMsg}, + Context: []Field{Error(err)}, + } + } tests := []struct { desc string @@ -122,6 +129,15 @@ func TestSugarWith(t *testing.T) { nonString(invalidPair{2, true, "bar"}, invalidPair{5, 42, "reversed"}), }, }, + { + desc: "multiple errors", + args: []interface{}{errors.New("first"), errors.New("second"), errors.New("third")}, + expected: []Field{Error(errors.New("first"))}, + errLogs: []observer.LoggedEntry{ + ignoredError(errors.New("second")), + ignoredError(errors.New("third")), + }, + }, } for _, tt := range tests { @@ -198,10 +214,13 @@ func TestSugarStructuredLogging(t *testing.T) { } // Common to all test cases. - context := []interface{}{"foo", "bar"} - extra := []interface{}{"baz", false} - expectedFields := []Field{String("foo", "bar"), Bool("baz", false)} - + var ( + err = errors.New("qux") + context = []interface{}{"foo", "bar"} + extra = []interface{}{err, "baz", false} + expectedFields = []Field{String("foo", "bar"), Error(err), Bool("baz", false)} + ) + for _, tt := range tests { withSugar(t, DebugLevel, nil, func(logger *SugaredLogger, logs *observer.ObservedLogs) { logger.With(context...).Debugw(tt.msg, extra...)