-
Notifications
You must be signed in to change notification settings - Fork 324
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
[log] optimize console log format #3499
Conversation
One note: Line 195 in c75411c
|
Could this format of logs be indexed? |
see comment in #3476, we need to achieve:
replace with zerolog does not seem a good solution to achieve both |
zap |
Codecov Report
@@ Coverage Diff @@
## master #3499 +/- ##
==========================================
- Coverage 75.43% 74.35% -1.08%
==========================================
Files 247 259 +12
Lines 22845 23536 +691
==========================================
+ Hits 17233 17501 +268
- Misses 4685 5107 +422
- Partials 927 928 +1
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
the log file is json format. colored printing in cmmandline. see the picture above |
Because of ELK, the log file format can't change. I'll try to find the humanly-printed setting by zap console encoder. |
ping @dustinxie @CoderZhi @Liuhaai - pls take another look |
pkg/log/log.go
Outdated
var buildOpts []zap.Option | ||
buildOpts = append(buildOpts, coreOpt) | ||
buildOpts = append(buildOpts, opts...) | ||
logger, err := cfg.Zap.Build(buildOpts...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cfg.Zap.Build(coreOpt, opts)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no. build error because type of opt
is []zap.Option,
pkg/log/log.go
Outdated
zapcore.NewConsoleEncoder(consoleCfg.EncoderConfig), | ||
zapcore.Lock(os.Stderr), | ||
zap.InfoLevel)) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so looks like add a new console encoder to process the output in human-friendly format?
i assume you already run locally to verify the log format in the file has remained the same?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, add console encoder. I run in test-net, log format is the same with older
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
Thanks for making this one @huof6829 @dustinxie |
Description
Please include a summary of the changes and the related issue. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes #3476
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Test Configuration:
Checklist: