Allow statsd and custom logger_class

[bloodearnest]

Currently, if you configure statsd_host, you cannot customise the logger_class, gunicorn will just use hardcoded gunicorn.intrument.Statsd class.

https://github.com/benoitc/gunicorn/blob/master/gunicorn/config.py#L150

This is not great, as the user's configured logger class is silently swallowed up and not used.

Could we at least add a warning if the the user has configured logger_class and statsd_host, so at least we know what's happening? :)

A simple fix would be to always prefer the user configured logger_class? And maybe improve docs to mention the Statsd class in logger_class config?

[benoitc]

What do you mean by customize? Do you want to have both , ie. logging on file or std + statsd?

We should definitely make it clear when which Logging class is in use. What about printing that information on startup?

For the rest I don't know, I guess the answer will depends on the question above ;) I do think we should be able to mix different logging strategy, the current implementation should be then improved. Thoughts?

[bloodearnest]

I mean I want to configure logging_class to use my own implementation[1], but I also want to use the statsd functionality and config.

I inherited my custom class from gunicorn.instrument.Statsd, but I discovered my class was not being being used when I also configured statsd_host. Works fine if I don't configure statsd_host.

e.g. (for clarity, apologies if obvious)

This logs output is as I expect
gunicorn app --logger-class='myclass'

This log output as default gunicorn, myclass is not being used
gunicorn app --logger-class='myclass' --statsd_host=localhost:12345

A possible fix (unfinished):

#1188

Regards mixing different logging strategy, could we possibly merge the Statsd logger functionality into the base Logger class, this removing this switching logic all together? Some care would need to be taken to avoid a performance hit if statsd is not enabled, but I think that's probably doable.

I'd be happy to do the work if you think that's worth pursuing.

Thanks (for a prompt response, and for gunicorn in general :)

[1] FWIW, my motivations for providing a custom logger_class implementation are:

a) increase timestamp resolution to include msec on access logs (not possible AFAICS with out overriding Logger.now())

b) use a custom formatter on the error log, which is currently hardcoded to logging.Formatter(Logger.error_fmt, Logger.datefmt). This to add some structured logging tags, like a request id, if present.

Both of these could maybe be addressed by extending the logger class, if you think they're worth doing. Then I wouldn't need a custom Logger implementation at all :)

[bloodearnest]

I have submitted PR (including tests and docs) which always uses the user configured logger.

[bloodearnest]

Has enough feedback been provided?

[bloodearnest]

This was merged, thanks!