Skip to content

Add an option to disable errorVerbose #1487

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions config.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ type Config struct {
ErrorOutputPaths []string `json:"errorOutputPaths" yaml:"errorOutputPaths"`
// InitialFields is a collection of fields to add to the root logger.
InitialFields map[string]interface{} `json:"initialFields" yaml:"initialFields"`
// DisableErrorVerbose stops printing error verbose in the error log.
DisableErrorVerbose bool `json:"disableErrorVerbose" yaml:"disableErrorVerbose"`
}

// NewProductionEncoderConfig returns an opinionated EncoderConfig for
Expand Down Expand Up @@ -166,6 +168,7 @@ func NewProductionConfig() Config {
EncoderConfig: NewProductionEncoderConfig(),
OutputPaths: []string{"stderr"},
ErrorOutputPaths: []string{"stderr"},
DisableErrorVerbose: true,
}
}

Expand Down Expand Up @@ -232,6 +235,7 @@ func NewDevelopmentConfig() Config {
EncoderConfig: NewDevelopmentEncoderConfig(),
OutputPaths: []string{"stderr"},
ErrorOutputPaths: []string{"stderr"},
DisableErrorVerbose: true,
}
}

Expand Down Expand Up @@ -276,10 +280,15 @@ func (cfg Config) buildOptions(errSink zapcore.WriteSyncer) []Option {
if cfg.Development {
stackLevel = WarnLevel
}

if !cfg.DisableStacktrace {
opts = append(opts, AddStacktrace(stackLevel))
}

if cfg.DisableErrorVerbose {
opts = append(opts, DisableErrorVerbose())
}

if scfg := cfg.Sampling; scfg != nil {
opts = append(opts, WrapCore(func(core zapcore.Core) zapcore.Core {
var samplerOpts []zapcore.SamplerOption
Expand Down
5 changes: 5 additions & 0 deletions logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ type Logger struct {
callerSkip int

clock zapcore.Clock

DisableErrorVerbose bool
}

// New constructs a new Logger from the provided zapcore.Core and Options. If
Expand Down Expand Up @@ -260,6 +262,9 @@ func (log *Logger) Warn(msg string, fields ...Field) {
// at the log site, as well as any fields accumulated on the logger.
func (log *Logger) Error(msg string, fields ...Field) {
if ce := log.check(ErrorLevel, msg); ce != nil {
for i := range fields {
fields[i].DisableErrorVerbose = log.DisableErrorVerbose
}
ce.Write(fields...)
}
}
Expand Down
14 changes: 14 additions & 0 deletions options.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,3 +184,17 @@ func WithClock(clock zapcore.Clock) Option {
log.clock = clock
})
}

// WithDisableErrorVerbose configures the Logger to turn off the error verbose or not,
// depending on the value of disableErrorVerbose. This is a generalized form of DisableErrorVerbose.
func WithDisableErrorVerbose(disableErrorVerbose bool) Option {
return optionFunc(func(log *Logger) {
log.DisableErrorVerbose = disableErrorVerbose
})
}

// DisableErrorVerbose configures the Logger to turn off the error verbose.
// See also WithDisableErrorVerbose.
func DisableErrorVerbose() Option {
return WithDisableErrorVerbose(true)
}
8 changes: 6 additions & 2 deletions zapcore/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ import (
// ...
// ],
// }
func encodeError(key string, err error, enc ObjectEncoder) (retErr error) {
func encodeError(key string, err error, enc ObjectEncoder, opts ...bool) (retErr error) {
// Try to capture panics (from nil references or otherwise) when calling
// the Error() method
defer func() {
Expand All @@ -68,8 +68,12 @@ func encodeError(key string, err error, enc ObjectEncoder) (retErr error) {
case errorGroup:
return enc.AddArray(key+"Causes", errArray(e.Errors()))
case fmt.Formatter:
disableErrorVerbose := false
if len(opts) > 0 {
disableErrorVerbose = opts[0]
}
verbose := fmt.Sprintf("%+v", e)
if verbose != basic {
if !disableErrorVerbose && verbose != basic {
// This is a rich error type, like those produced by
// github.com/pkg/errors.
enc.AddString(key+"Verbose", verbose)
Expand Down
39 changes: 27 additions & 12 deletions zapcore/error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,20 +79,31 @@ func (e customMultierr) Errors() []error {

func TestErrorEncoding(t *testing.T) {
tests := []struct {
key string
iface any
want map[string]any
key string
iface any
disableErrorVerbose bool
want map[string]any
}{
{
key: "k",
iface: errTooManyUsers(2),
key: "k",
iface: errTooManyUsers(2),
disableErrorVerbose: true,
want: map[string]any{
"k": "2 too many users",
},
},
{
key: "k",
iface: errTooFewUsers(2),
key: "k",
iface: errTooFewUsers(2),
disableErrorVerbose: true,
want: map[string]any{
"k": "2 too few users",
},
},
{
key: "k",
iface: errTooFewUsers(2),
disableErrorVerbose: false,
want: map[string]any{
"k": "2 too few users",
"kVerbose": "verbose: 2 too few users",
Expand All @@ -105,6 +116,7 @@ func TestErrorEncoding(t *testing.T) {
errors.New("bar"),
errors.New("baz"),
),
disableErrorVerbose: true,
want: map[string]any{
"err": "foo; bar; baz",
"errCauses": []any{
Expand All @@ -115,8 +127,9 @@ func TestErrorEncoding(t *testing.T) {
},
},
{
key: "e",
iface: customMultierr{},
key: "e",
iface: customMultierr{},
disableErrorVerbose: true,
want: map[string]any{
"e": "great sadness",
"eCauses": []any{
Expand All @@ -132,8 +145,9 @@ func TestErrorEncoding(t *testing.T) {
},
},
{
key: "k",
iface: fmt.Errorf("failed: %w", errors.New("egad")),
key: "k",
iface: fmt.Errorf("failed: %w", errors.New("egad")),
disableErrorVerbose: true,
want: map[string]any{
"k": "failed: egad",
},
Expand All @@ -147,6 +161,7 @@ func TestErrorEncoding(t *testing.T) {
errors.New("baz"),
fmt.Errorf("world: %w", errors.New("qux")),
),
disableErrorVerbose: true,
want: map[string]any{
"error": "hello: foo; bar; baz; world: qux",
"errorCauses": []any{
Expand All @@ -162,7 +177,7 @@ func TestErrorEncoding(t *testing.T) {

for _, tt := range tests {
enc := NewMapObjectEncoder()
f := Field{Key: tt.key, Type: ErrorType, Interface: tt.iface}
f := Field{Key: tt.key, Type: ErrorType, Interface: tt.iface, DisableErrorVerbose: tt.disableErrorVerbose}
f.AddTo(enc)
assert.Equal(t, tt.want, enc.Fields, "Unexpected output from field %+v.", f)
}
Expand Down
13 changes: 7 additions & 6 deletions zapcore/field.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,11 +102,12 @@ const (
// context. Most fields are lazily marshaled, so it's inexpensive to add fields
// to disabled debug-level log statements.
type Field struct {
Key string
Type FieldType
Integer int64
String string
Interface interface{}
Key string
Type FieldType
Integer int64
String string
Interface interface{}
DisableErrorVerbose bool
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We avoid adding new struct fields to the Field type for performance reasons, as the struct is passed as a value.

We could add a new FieldType to indicate an error that should only have the message included, though that could have backwards compatibility concerns with some encoders.

Copy link
Author

@makotonakai makotonakai Feb 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@prashantv

I appreciate your feedback. I will create a new type with error field and DisableErrorVerbose field.

}

// AddTo exports a field through the ObjectEncoder interface. It's primarily
Expand Down Expand Up @@ -173,7 +174,7 @@ func (f Field) AddTo(enc ObjectEncoder) {
case StringerType:
err = encodeStringer(f.Key, f.Interface, enc)
case ErrorType:
err = encodeError(f.Key, f.Interface.(error), enc)
err = encodeError(f.Key, f.Interface.(error), enc, f.DisableErrorVerbose)
case SkipType:
break
default:
Expand Down