-
Notifications
You must be signed in to change notification settings - Fork 117
Make logger configurable #104
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
Comments
@andig, thanks for the feedback. Such an item is already on our backlog. It could be in the next release. |
@andig, what would you like to achieve with a custom logger? |
Ideally I would like to chose what to log (info, debug etc). There is no common go standard for doing so afaik. So at least I would be able to set the logger instance (using Logger interface). I could then at least chose to set my TRACE logger instance. |
Thanks for explaining your use case, understand. |
Noticed that, perfect. |
I‘m using https://github.com/spf13/jwalterweatherman for my logging which is of course opinionated. One thing I cannot do with the influx log level is to differentiate which log level the client actually logs to when supplying a custom logger. That is still acceptable for time being. |
Regarding the upcoming 1.4 api, I feel this could be a very simple change:
Why have the
Would be happy to try a PR if this is the direction you'd like. |
Actually, after looking at the source, the issue is at a deeper level. The
|
See #144 for a suggestion. I've not happy with |
Updated #144 as suggested. Ready for review, sorry for the noise. |
I'd like to chime in here. I'd love to see the PR #144 make it through. Our influx server is down briefly every day as it's running on preemptive VMs, and that creates a lot of log spam on the writer side, which currently can not be disabled (as we can't set the log level to -1 or disable error logging in any way). It even took us a while to figure out where the error logs are coming from, as they're unmarked and just said "[E] Write error" with some generic html error (as the load balancer was waiting for influx to come back), spammed over multiple lines, see below: So, another side note is that the default errors should probably include some form of description of the source of the error (eg. "Influx write error" instead of "Write error"). |
@Volcore, the missing prefix will be added in the next release. There will also be an option to turn off logging, however, errors should be logged, because these are sign of sth not working ok. |
Thanks for solving this! ❤️ |
Usage of
log.Logger
is private and hard-wired. Would be helpful to set a custom logger.The text was updated successfully, but these errors were encountered: