Adds option to emit json structured logs#245
Conversation
ikatson
left a comment
There was a problem hiding this comment.
Needs a fix, at least to unbreak the desktop app.
b3ca2e0 to
dd95c17
Compare
|
Thanks for the feedback and pointers, I'm moving this back to a draft for now while I fight with tracing_subscriber |
|
Also please don't change stderr to stdout for logs. If you really need it for some reason, make it a parameter, but keep the current behavior. |
|
I'm misunderstanding where the stdout log stream(s) are coming from, I'm seeing duplicated logs, some with human and some with json. I guess need to revisit all the branches of the logging recorder and see what I missed |
6b9c193 to
2ad7128
Compare
a13f938 to
3583c6e
Compare
| let log_layer: Box<dyn Layer<_> + Send + Sync> = if opts.log_file_json { | ||
| Box::new( | ||
| fmt::layer() | ||
| .with_ansi(false) |
There was a problem hiding this comment.
this makes log file contain ANSI escape codes which makes it hard to grep and use. Those escape codes are intended for terminal use only.
PR LGTM otherwise, I'd merge if not for this
This introduces two new options that affect logging output, one that turns on json-formatted logs for the log file option and one for the stdout stream.
I think I've managed to not change things too much, lmk what you think