Make logger configurable

[andig]

Usage of log.Logger is private and hard-wired. Would be helpful to set a custom logger.

[vlastahajek]

@andig, thanks for the feedback. Such an item is already on our backlog. It could be in the next release.

[vlastahajek]

@andig, what would you like to achieve with a custom logger?
Override complete messages by reimplementing Debug, Warn, etc., functions?
Or just redirect output from stderr to file, etc.?

[andig]

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.

[vlastahajek]

Thanks for explaining your use case, understand.
Regarding what to log, I would like to check, whether are you aware of the ability to set debug level, or it doesn't suit your needs?

[andig]

Noticed that, perfect.

[andig]

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.

[andig]

Regarding the upcoming 1.4 api, I feel this could be a very simple change:

	go func() {
		for err := range writer.Errors() {
			m.log.ERROR.Println(err)
		}
	}()

Why have the writer.Errors return a channel of

• either structs containing error plus log level and
• have the err as promoted field so compatibility remains

Would be happy to try a PR if this is the direction you'd like.

[andig]

Actually, after looking at the source, the issue is at a deeper level. The Errors() interface will always return actual errors. What is missing is an abstraction of the logger to an interface and:

• the ability to set a custom logger
• allow setting logger to nil
• controlling instantiation of a default logger

[andig]

See #144 for a suggestion.

I've not happy with SetDebugLevel becoming part of the public API of the Logger interface (instead of client). Please let me know what you think- I'd propose to add an internal log adapter that translates SetDebugLevel into calls of the actual instantiated logger.

[andig]

Updated #144 as suggested. Ready for review, sorry for the noise.

[Volcore]

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").

[vlastahajek]

@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.
The best solution would be to reconfigure the load balancer to return a JSON error message for REST API paths: /apiv/v2/*, /health, /ready, /setup

[Volcore]

Thanks for solving this! ❤️