gunicorn
gunicorn copied to clipboard
benoitc
Separate instrumentation and logging
Issues like #1405 demonstrate that having statsd instrumentation built on top of the logging handlers means instrumentation and logging are coupled. Some have found this coupling surprising.
I would like to see proposals for ways to separate these functions.
This may be as simple as making the statsd Logger into a handler instead, so it can be invoked no matter which logger is being used.
Hello @tilgovi
I made decision for the PR above after considering:
- making instrumentation a log handler, which I didn't feel natural as handler.emit() gets LogRecord created as argument (not necessary for gauge/counter-type and not sufficient for access-kind histogram)
- making instrumentation a mixin class in a logger class, which still means the coupling between logger and instrumentation, and also makes sub-classing it necessary for custom logger. (ambiguous config structure)
This fix makes instrumentation directly from config setting no matter what logger is being loaded. I believe this config structure makes it easy to add more instrumentation in the future as well.
Do you think it's something workable? I'm open to any other idea as well :) Thanks!
I think it's a very reasonable approach. Thank you for laying out your thoughts so well, for taking care of backward compatibility, and for adding configuration tests. I have a few comments on the PR, but I'll wait and make them as a review on the PR issue if we agree this is the way to go.
@benoitc @berkerpeksag how do you feel about this?
i would lik to create a project (in github sense) for it and collect the ticket there.
About the implementation, I would like to simply implement some hooks that can be used by any implementation to add their own logic. And then support by default opentelemetry. Thoughts?
I would love to see some solution to this implemented. The statsD metrics that are exported right now are useful, but there is a lot of other information that I would like to be able to see. It would be good if I could easily write some code/config to be able to do that.
As of now, the PR of my proposal technically can handle any number of custom instrumentation classes mixed into logger class. Though statsD is only available at moment (I didn't wanna change the existing config flag of statsd_host), it sort of works as "hooks" to add any instrument class with methods matching with logger.
However, though it can support multiple custom instrumentation classes, this approach still means coupling between logger and instrumentation since instrumentation methods getting mixed into logger methods. So maybe a decision can be made on whether we completely separate the way to call instrumentation from logger, and for that, maybe we can get started from what log data and metrics data would eventually be provided by gunicorn? As of now for statsD instrumentation looks sufficient to be coupled with logger to me.
