Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Allow configurable stacktrace encoding #1371

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Allow configurable stacktrace encoding #1371

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
456073a
Allow configurable stacktrace encoding
arwineap Oct 9, 2023
ebc0c4f
Prevent unconfigured stacktraceEncoder from panicing
arwineap Oct 10, 2023
a0dbf50
Adding ability to unmarshal text to a StacktraceEncoder for configs
arwineap Oct 10, 2023
57f3398
removing unneeded nil check
arwineap Oct 10, 2023
cad62a8
Fix nil and noop EncodeStacktraces
arwineap Oct 10, 2023
449be6b
Update zapcore/encoder.go
arwineap Oct 11, 2023
d45920a
Update zapcore/encoder.go
arwineap Oct 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
50 changes: 26 additions & 24 deletions config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,18 +123,19 @@ type Config struct {
// cfg.EncodeTime = zapcore.ISO8601TimeEncoder
func NewProductionEncoderConfig() zapcore.EncoderConfig {
return zapcore.EncoderConfig{
TimeKey: "ts",
LevelKey: "level",
NameKey: "logger",
CallerKey: "caller",
FunctionKey: zapcore.OmitKey,
MessageKey: "msg",
StacktraceKey: "stacktrace",
LineEnding: zapcore.DefaultLineEnding,
EncodeLevel: zapcore.LowercaseLevelEncoder,
EncodeTime: zapcore.EpochTimeEncoder,
EncodeDuration: zapcore.SecondsDurationEncoder,
EncodeCaller: zapcore.ShortCallerEncoder,
TimeKey: "ts",
LevelKey: "level",
NameKey: "logger",
CallerKey: "caller",
FunctionKey: zapcore.OmitKey,
MessageKey: "msg",
StacktraceKey: "stacktrace",
LineEnding: zapcore.DefaultLineEnding,
EncodeLevel: zapcore.LowercaseLevelEncoder,
EncodeTime: zapcore.EpochTimeEncoder,
EncodeDuration: zapcore.SecondsDurationEncoder,
EncodeCaller: zapcore.ShortCallerEncoder,
EncodeStacktrace: zapcore.FullStacktraceEncoder,
}
}

Expand Down Expand Up @@ -200,18 +201,19 @@ func NewProductionConfig() Config {
func NewDevelopmentEncoderConfig() zapcore.EncoderConfig {
return zapcore.EncoderConfig{
// Keys can be anything except the empty string.
TimeKey: "T",
LevelKey: "L",
NameKey: "N",
CallerKey: "C",
FunctionKey: zapcore.OmitKey,
MessageKey: "M",
StacktraceKey: "S",
LineEnding: zapcore.DefaultLineEnding,
EncodeLevel: zapcore.CapitalLevelEncoder,
EncodeTime: zapcore.ISO8601TimeEncoder,
EncodeDuration: zapcore.StringDurationEncoder,
EncodeCaller: zapcore.ShortCallerEncoder,
TimeKey: "T",
LevelKey: "L",
NameKey: "N",
CallerKey: "C",
FunctionKey: zapcore.OmitKey,
MessageKey: "M",
StacktraceKey: "S",
LineEnding: zapcore.DefaultLineEnding,
EncodeLevel: zapcore.CapitalLevelEncoder,
EncodeTime: zapcore.ISO8601TimeEncoder,
EncodeDuration: zapcore.StringDurationEncoder,
EncodeCaller: zapcore.ShortCallerEncoder,
EncodeStacktrace: zapcore.FullStacktraceEncoder,
}
}

Expand Down
18 changes: 16 additions & 2 deletions zapcore/console_encoder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@
arr.AppendString(ent.Caller.Function)
}
}

for i := range arr.elems {
if i > 0 {
line.AppendString(c.ConsoleSeparator)
Expand All @@ -119,10 +120,23 @@
c.writeContext(line, fields)

// If there's no stacktrace key, honor that; this allows users to force
// single-line output.
// single-line output by avoiding printing the stacktrace.
if ent.Stack != "" && c.StacktraceKey != "" {
line.AppendByte('\n')
line.AppendString(ent.Stack)

arr = getSliceEncoder()
stacktraceEncoder := c.EncodeStacktrace
if stacktraceEncoder == nil {
stacktraceEncoder = FullStacktraceEncoder
}
stacktraceEncoder(ent.Stack, arr)
for i := range arr.elems {
if i > 0 {
line.AppendString(c.ConsoleSeparator)
}

Check warning on line 136 in zapcore/console_encoder.go

View check run for this annotation

Codecov / codecov/patch

zapcore/console_encoder.go#L135-L136 

Added lines #L135 - L136 were not covered by tests
fmt.Fprint(line, arr.elems[i])
}
Comment on lines +133 to +138
Copy link
Collaborator

Choose a reason for hiding this comment

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

The contract for stacktraceEncoder is that it makes at most one Append* call.
So we don't need the full arr.elems handling here. It can just be:

Suggested change
for i := range arr.elems {
if i > 0 {
line.AppendString(c.ConsoleSeparator)
}
fmt.Fprint(line, arr.elems[i])
}
if len(arr.elems) > 0 {
fmt.Fprint(line, arr.elems[0])
} else {
// User-provided encoder was a no-op.
// Fall back to full encoder.
line.AppendString(ent.Stack)
}

Copy link
Author

Choose a reason for hiding this comment

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

The first AppendString in this call is actually a way to add the ConsoleSeparator between each line; it could probably be integrated into the Fprint to reduce branching here

putSliceEncoder(arr)
}

line.AppendString(c.LineEnding)
Expand Down
31 changes: 27 additions & 4 deletions zapcore/encoder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,28 @@
return nil
}

// A StacktraceEncoder serializes a stacktrace
//
// This function must make exactly one call
// to a PrimitiveArrayEncoder's Append* method.
type StacktraceEncoder func(string, PrimitiveArrayEncoder)

// FullStacktraceEncoder passes down the full stacktrace as a string to the enc
arwineap marked this conversation as resolved.
Show resolved Hide resolved
func FullStacktraceEncoder(stacktrace string, enc PrimitiveArrayEncoder) {
enc.AppendString(stacktrace)
}

// UnmarshalText unmarshals text to a StacktraceEncoder. Currently, it will only default to FullStacktraceEncoder.
arwineap marked this conversation as resolved.
Show resolved Hide resolved
func (e *StacktraceEncoder) UnmarshalText(text []byte) error {
switch string(text) {
case "full":
*e = FullStacktraceEncoder
default:
*e = FullStacktraceEncoder

Check warning on line 290 in zapcore/encoder.go

View check run for this annotation

Codecov / codecov/patch

zapcore/encoder.go#L285-L290 

Added lines #L285 - L290 were not covered by tests
}
return nil

Check warning on line 292 in zapcore/encoder.go

View check run for this annotation

Codecov / codecov/patch

zapcore/encoder.go#L292 

Added line #L292 was not covered by tests
}

// A CallerEncoder serializes an EntryCaller to a primitive type.
//
// This function must make exactly one call
Expand Down Expand Up @@ -343,10 +365,11 @@
// Configure the primitive representations of common complex types. For
// example, some users may want all time.Times serialized as floating-point
// seconds since epoch, while others may prefer ISO8601 strings.
EncodeLevel LevelEncoder `json:"levelEncoder" yaml:"levelEncoder"`
EncodeTime TimeEncoder `json:"timeEncoder" yaml:"timeEncoder"`
EncodeDuration DurationEncoder `json:"durationEncoder" yaml:"durationEncoder"`
EncodeCaller CallerEncoder `json:"callerEncoder" yaml:"callerEncoder"`
EncodeLevel LevelEncoder `json:"levelEncoder" yaml:"levelEncoder"`
EncodeTime TimeEncoder `json:"timeEncoder" yaml:"timeEncoder"`
EncodeDuration DurationEncoder `json:"durationEncoder" yaml:"durationEncoder"`
EncodeCaller CallerEncoder `json:"callerEncoder" yaml:"callerEncoder"`
EncodeStacktrace StacktraceEncoder `json:"stacktraceEncoder" yaml:"stacktraceEncoder"`
arwineap marked this conversation as resolved.
Show resolved Hide resolved
// Unlike the other primitive type encoders, EncodeName is optional. The
// zero value falls back to FullNameEncoder.
EncodeName NameEncoder `json:"nameEncoder" yaml:"nameEncoder"`
Expand Down