Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upConsider changing the default enabled log levels #147
Comments
alexcrichton
added
the
help wanted
label
May 19, 2017
This comment has been minimized.
This comment has been minimized.
lucab
commented
Jun 1, 2017
|
Bump for this for the missing offline comments. Also, has build-time stubbing of disabled levels (like slog does) been discussed? Is it in scope here? |
This comment has been minimized.
This comment has been minimized.
|
Oops, sorry! You can already statically disable levels actually: https://github.com/rust-lang-nursery/log/blob/master/Cargo.toml#L23-L35. The question here is over defaults. I feel pretty strongly that we shouldn't disable any level of logging by default. The whole point of logging is that you have bunch of diagnostic information available to you when you need it. You normally don't need to look at debug or trace level logging, but when you do it's invaluable! For example, Hyper's trace-level logging helped track down a bug in its chunked request body parser in a running server. If, for example, trace logging were disabled by default, you'd have several problems. If I ran into that Hyper problem above, I'd have to ship a new custom build of the server with trace logging enabled to actually be able to get the information I need. If the issue you're looking into is one of the annoying transient ones that no-one knows how to reproduce but pops up in production every once in a while, it's really important to be able to look very closely at whatever box it shows up on immediately! I could also override the default to enable trace logging by default, but then I potentially have other issues - if people become too used to trace logging being turned off, you may run into issues where those log events are extremely expensive or even broken! Rustc has some of those issues - the old log framework disabled debug logging in prod builds, and didn't evaluate log arguments if the log event wasn't enabled and it turned out that several would panic if actually evaluated. In addition, there is a fast path for logging levels that are entirely disabled at runtime. If you don't have any trace-level logging enabled in your logger configuration, the cost of a |
This comment has been minimized.
This comment has been minimized.
lucab
commented
Jun 1, 2017
|
Thanks for the feedback! It looks like my knowledge around here is wrong/stale, so I'm paging @dpc for some informed comment. |
This comment has been minimized.
This comment has been minimized.
dpc
commented
Jun 1, 2017
|
I understand @sfackler points, and I think they make sense. I also don't feel strongly about default logging level in As a library writer, it is convenient to have a logging level for your disposal, that you don't have to feel guilty about using, and comes handy when you are testing and debugging your library while you are working on it, change it etc.. It is not optimal if
There are some differences between Also, because in slog you do compose how the different pices of your software route their logging messages, it is possible to modify in-flight eg. lowering logging levels of your dependencies. Even if they do overuse trace, or debug, and you turned it on, they still wouldn't get logged and their overhead would be minimal. So in slog, if there's something useful to know in production, no matter how minor it might be from global perspective you can make it an So I don't think |
This comment has been minimized.
This comment has been minimized.
lucab
commented
Jun 1, 2017
|
For reference, I ended up here today because of such discussions happening around TLS libraries and logging: |
This comment has been minimized.
This comment has been minimized.
|
I think a concur that all log levels should be compiled in by default. Would be quite surprising for a naive user to deploy an app, go to turn on verbose logs and discover they don't exist. |
This comment has been minimized.
This comment has been minimized.
|
Probably about time to decide one way or the other here. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
I'm personally on board with "compile everything in by default" |
This comment has been minimized.
This comment has been minimized.
|
Compiling all by default sounds good to me too. Are we happy to close this or would you like to keep it open to solicit some more feedback? |
This comment has been minimized.
This comment has been minimized.
|
I think we're good to go. |
alexcrichton commentedMay 19, 2017
Right now I believe all log levels are enabled by default in all builds. @sfackler provided some good rationale for this during the evaluation, @sfackler want to copy that down here as well?
In any case would be worth discussing before a 1.0 release!