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
Added new Log Enconder Config #2927
Added new Log Enconder Config #2927
Conversation
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
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.
Some of these can be set via existing flags: https://pkg.go.dev/sigs.k8s.io/controller-runtime/pkg/log/zap#Options.BindFlags. If we need to add more, can we use the existing naming convention?
I'm aware of these |
I think you can use |
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
pflag.StringVar(&encodeMessageKey, "zap-message-key", "message", "The message key to be used in the customized Log Encoder") | ||
pflag.StringVar(&encodeLevelKey, "zap-level-key", "level", "The level key to be used in the customized Log Encoder") | ||
pflag.StringVar(&encodeTimeKey, "zap-time-key", "timestamp", "The time key to be used in the customized Log Encoder") | ||
pflag.StringVar(&encodeLevelFormat, "zap-level-format", "uppercase", "The level format to be used in the customized Log Encoder") |
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.
Are the defaults here the same as we currently have? That is, if we don't set any of these, there's no change in behaviour?
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, but we can't compare it with the current log because we use the console
encoder by default, while these changes will be effective when we set the zap-encoder=json
.
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.
LGTM, modulo the lint failure.
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
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.
@yuriolisa can you also please update some docs too for this change? This would assist with the feedback we got in #873
Signed-off-by: Yuri Sa <yurimsa@gmail.com>
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.
🚀 🎉
Description:
Added 5 new parameters to enable a customized JSON log Encoder:
--enable-customized-log-encoder
= It will enable the feature to set customizedkeys
andformats
to the logs, otherwise, the operator will use the zap-devel config.--log-message-key
= It will replace the Key of the Message field (e.g. "msg")--log-level-key
= It will replace the Key of the Level field (e.g. "severity")--log-time-key
= It will replace the Key of the Time field (e.g. "timestamp")--log-time-format
= It will replace the Format of the Time field (e.g. "epoch", "epochmillis", "epochnano", "iso8601", "rfc3339" or "rfc3339nano")--log-level-format
= It will replace the Format of the Level field (e.g. "uppercase", "lowercase")Link to tracking Issue(s):
Testing:
Documentation: