-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Add support for TimeEncoding for Zap logger #10205
Conversation
encoding explicitly Update newLogger to read and interpret the selected option
|
// TimeEncoding controls the zap logger's time encoding | ||
// Permitted values are: | ||
// - "Epoch" | ||
// - "EpochMillis" | ||
// - "EpochNanos" | ||
// - "ISO8601" | ||
// - "RFC3339" | ||
// - "RFC3339Nano" | ||
// Leaving the value blank results in the default time encoding, which | ||
// depends on the message encoding (for JSON, it will be EpochNanos, | ||
// and for console, it will be ISO8601 | ||
TimeEncoding string `mapstructure:"time_encoding"` |
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.
We would prefer not to derive from the zap.Config. Is this there already? if yes, can you add a link to confirm the name?
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.
Apologies, but I'm not following this. I wasn't able to find any mechanism by which I can set the Zap logger's time encoding through the existing telemetry config, hence the new field. One can only indirectly influence it (ex, by choosing console as the format), but there is no way to use, say, RFC3339 time encoding along with JSON formatted messages.
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
Description
TimeEncoding
field underLogsConfig
to control Zap logger time encoding explicitlynewLogger
to read and interpret the selected optionLink to tracking issue
Fixes #10204
Testing
None yet (wasn't able to find existing tests for
newLogger
)Documentation
None yet (unsure where the docs for this live)