feat: file and advanced logging configuration for tracing #1993
Conversation
shadowsocks69420
force-pushed
the
tracing-appender-v2
branch
from
938c5ef to
5af2060
Compare
| /// Default logger log level, [0, 3] | ||
| /// Default log level for all writers, [0, 3] | ||
| pub level: u32, | ||
| /// Default logger format configuration | ||
| /// Default format configuration for all writers | ||
| pub format: LogFormatConfig, |
There was a problem hiding this comment.
My original plan was to remove these fields from the struct while keeping the configuration format backward-compatible. E.g.: create a stdout writer using these values if no writers are configured, but ignore them otherwise (because it should be configured on a per-writer basis).
However, having to support the command line option prevented me from doing so because I figured that the command line option has to be handled in a somewhat sensible way and not ignored. But it is hard to reason about when there can be multiple writers. E.g.: which writer should the command line option apply to?
I'd still like to remove them if possible, let me know if you have opinion on this,
| pub format: LogFormatConfig, | ||
| /// Logging configuration file path | ||
| /// Log writers configuration | ||
| pub writers: Vec<LogWriterConfig>, |
There was a problem hiding this comment.
I used the term writers as requested in your comment #1990 (comment)
However, please note that in tracing the term writer refers to the MakeWriter trait which only handles dispatching a formatted buffer to an io::Write. What each LogWriterConfig here corresponds to is a "subscriber" or "layer" in tracing-speak, or an "appender" in log4rs-speak.
|
@zonyitoo This is ready for review. Waiting for your feedback on this. |
|
I know. But I was quite busy these days. |
This is the second attempt of #1992, as requested in #1990 (comment)
Compare to the first attempt, this iteration enables more flexibility for logging configuration, supporting logging to stdout and multiple files simultaneously, which was a limitation in the previous version.
This new PR is intended as an alternative to the first iteration, feel free to close the other once decision is made.
I removed
SS*Configstructs as the amount of code duplication involved to deserialize the newLogWriterConfigenum is too much. And I don't really get the benefit of having them in the first place. I can see it makes sense for server configurations for backward compatibility reasons, but service configurations don't carry the same baggage and, in most cases, can be easily worked around by utilizing existingserde_derivefeatures.I've provided extra tests to ensure the change to the deserialization logic won't break existing configurations.