-
Notifications
You must be signed in to change notification settings - Fork 171
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
Use lower logging levels? #3071
Comments
Hi, in general we agree that not much should be logged on ERROR, WARN, and not even that much on INFO. I think the rough guideline is that ERROR is a serious issue, potentially rendering the entire iroh So this makes my question to you: which message are you seeing specifically? Maybe we have bugs to fix, or maybe we did get the log level wrong (there certainly are still those cases about). Also in the interim, are you aware of the EnvFilter directives which allow tweaking log levels per crate and module? |
The core guidance IMO is that it all needs to be considered from the end user perspective. If Iroh is hitting condition that means that the application is likely screwed and needs immediate user/administrator attention than it might deserve an That's another reason why libraries should not need to log all that much anyway - they should be returning errors to the app and let it handle it. Sometimes it is even worthwhile to expose something like a broadcast channel where the application can register to handle events (like errors) as it see fits. This is useful for exposing things from background threads etc. that can't simply return errors upwards to the higher level code. BTW. I wish
Sure I am. I have spent quite a bit of time, effort and thought on Rust logging, e.g. being an author of It is a workable bandaid, yes, but very crude. Typically applications end up maintaining some long list of muted dependencies, and having dependency simply muted like that is less than ideal. Over last few years I've reported similar issues to many libraries, trying to keep such list as short as possible. :) And then putting anything in |
We do have some places where things can be problems, bugs either in use or -more common- implementation, where the problem is not fatal and we can not error out to the user. The nature of all the things the Endpoint needs to do under the hood makes this unfortunately reasonably common. In those cases we opt for error or warning logs so that we have a chance of getting bug reports etc. One point were we might digress from your vision is that we are fairly verbose on debug level. And this is generally essential for debugging user problems. I expect as the library matures that these lower logging levels will slowly reduce. I still think we essentially agree with the use of logging though. So please, if you have specific log messages that are showing up too much do point them out so we can figure out if there are some bugs or whether we need to tweak the logging. |
Debug is a fair game. Ideally, it would be showing things relevant in debugging possible application's issues related to Iroh, but ... debug is debug. All logging messages are fair in love and debugging or something like that. Usually in apps I default to INFO, as the assumption is this is what the user should be informed about. Anything below that is much less of a problem anyway, as it only shows up when something non-default is being done.
Will do. It will take some time, as I'm slowly going through my app work. |
I started playing with Iroh and I see it logs things on WARN etc. levels which by default is getting in a way of debugging my own code.
My philosophy of logging levels is that since they (somewhat unfortunately) are typically globally shared, they should be used with an end to end point of view.
I'd expect Iroh will always be used as a lower level infrastructure layer, and thus what's happening inside it will always be some kind of trace information, maybe debug for most important iroh logging statements, and not application level "information", "warning" or "error" which should be reserved for applications.
The text was updated successfully, but these errors were encountered: