zap
zap copied to clipboard
Allow configurable stacktrace encoding
This PR adds a StacktraceEncoder
type to the EncoderConfig
struct
Users can override the EncodeStacktrace
field to configure how stacktraces are outputed This PR aims to resolve https://github.com/uber-go/zap/issues/514 by mirroring the behavior of the EncodeCaller
field
The EncodeStacktrace
field has been inserted as a required field, and has been added to NewDevelopmentConfig
and NewProductionConfig
as sane defaults however, as a required field it can cause a panic if a user is manually building their config without these helpers.
Codecov Report
Merging #1371 (cad62a8) into master (87577d8) will decrease coverage by
0.13%
. The diff coverage is84.12%
.
:exclamation: Current head cad62a8 differs from pull request most recent head d45920a. Consider uploading reports for the commit d45920a to get more accurate results
@@ Coverage Diff @@
## master #1371 +/- ##
==========================================
- Coverage 98.28% 98.15% -0.13%
==========================================
Files 53 53
Lines 3493 3527 +34
==========================================
+ Hits 3433 3462 +29
- Misses 50 55 +5
Partials 10 10
Files | Coverage Δ | |
---|---|---|
config.go | 100.00% <100.00%> (ø) |
|
zapcore/json_encoder.go | 100.00% <100.00%> (ø) |
|
zapcore/console_encoder.go | 97.05% <78.57%> (-2.95%) |
:arrow_down: |
zapcore/encoder.go | 83.45% <22.22%> (-4.24%) |
:arrow_down: |
... and 1 file with indirect coverage changes
:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more
Thank you for the clarification, I'm unsure how I missed the default catch for the nil cases. This has been added.
One other concern I had was with the console encoder; I'm not sure I can avoid calling getSliceEncoder()
a second time because of the split in output between structured fields and unstructured panic data. This felt ineffecient, but the code seems cleaner than the alternative
Related: We'll want to make a decision on https://github.com/uber-go/zap/issues/1373 before we release this because choices there affect the design for this.
@abhinav @arwineap hi! Any news about this PR? Responses are really appreciated :slightly_smiling_face:
I will work to rebase this with modern changes; but depending on any decision in https://github.com/uber-go/zap/issues/1373 we may try to send the entire stack object into the function instead of the string