Skip to content

Commit

Permalink
SugaredLogger: Turn error into zap.Error (#1185)
Browse files Browse the repository at this point in the history
  • Loading branch information
Craig Pastro authored Oct 15, 2022
1 parent 9137e0e commit 9b86a50
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 8 deletions.
24 changes: 20 additions & 4 deletions sugar.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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]))
Expand Down
27 changes: 23 additions & 4 deletions sugar_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
package zap

import (
"errors"
"testing"

"go.uber.org/zap/internal/exit"
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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...)
Expand Down

0 comments on commit 9b86a50

Please sign in to comment.