prometheus / client_golang

Prometheus instrumentation library for Go applications
https://pkg.go.dev/github.com/prometheus/client_golang
Apache License 2.0
5.35k stars 1.17k forks source link

promhttp: Investigate interaction of the metrics handler with certain middlewares #622

Open beorn7 opened 5 years ago

beorn7 commented 5 years ago

https://github.com/prometheus/prometheus/issues/5085 demonstrated problems if the compression middleware from the echo framework was used, see https://github.com/labstack/echo/blob/master/middleware/compress.go .

The promhttp.Handler his its own gzip handling. Perhaps it's inevitable to then not work with other middlewares handling compression (which then should be clearly documented). But perhaps there is also something middleware-unfriendly that could be fixed in promhttp.Handler.

Purpose of this issue is to investigate how promhttp.Handler interacts with certain middlewares (echo framework, but also others, e.g. the NYTimes gzip handler, and also find out if problems happen with other, non-compressing middlewares). In v1, we might be able to fix problems, if there are any in the handler. For v2, we might consider separating out compression if there are established middlewares handling compression nicely. (On the other hand, the result could as well be that it is impossible to handle compression as a middleware without problems in certain situations, and thus handling it in the current way might be just right.)

lacion commented 5 years ago

is there any reason why the prom HTTP handler does its own GZIP? i find it a bit odd this happens in the middleware and not later on the chain where it should be handled?

it seems to be pretty common to have a middleware dedicated to gzipping or even a reverse proxy (nginx) doing that.

maybe the right solution is either to not gzip on the metrics handler or leave as a configurable option (default on to keep current behavior)?

beorn7 commented 5 years ago

The behavior is configurable via https://godoc.org/github.com/prometheus/client_golang/prometheus/promhttp#HandlerOpts .

The gzipping in the client library is very old. It might very well be that no common middleware for gzipping existed back then. One might even argue that even now it is not common enough to have it in the standard library. That's where my suspicion is coming from that there is perhaps no clean solution to do gzipping as a middleware.

The reverse proxy setup is something you can always do if you want. I'm just afraid that many would consider that one moving piece too many. The Prometheus project has already received a lot of criticism for generally not doing TLS and leaving it to reverse proxies to be set up by the users. (In fact, the project is now working in including TLS in its various HTTP servers.) And TLS is not even something you could do on the handler level.

dingdayu commented 4 years ago

try:

// Prometheus register prometheus api
func Prometheus(c *gin.Context) {
    // register promhttp.HandlerOpts DisableCompression
    promhttp.InstrumentMetricHandler(prometheus.DefaultRegisterer, promhttp.HandlerFor(prometheus.DefaultGatherer, promhttp.HandlerOpts{
        DisableCompression: true,
    })).ServeHTTP(c.Writer, c.Request)
    //promhttp.Handler().ServeHTTP(c.Writer, c.Request)
}

at: https://github.com/gin-gonic/contrib/pull/189

beorn7 commented 3 years ago

I mostly assigned this to myself as a “default” because I used to be the maintainer of this repo. Therefore, I'm un-assigning this from myself no. New maintainers @bwplotka & @kakkoyun, please deal with this as you see fit.