Skip to content
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

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

Open
beorn7 opened this issue Jul 16, 2019 · 4 comments

Comments

@beorn7
Copy link
Member

beorn7 commented Jul 16, 2019

prometheus/prometheus#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
Copy link

lacion commented Jul 16, 2019

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
Copy link
Member Author

beorn7 commented Jul 16, 2019

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
Copy link

dingdayu commented Apr 7, 2020

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: gin-gonic/contrib#189

@beorn7
Copy link
Member Author

beorn7 commented Jun 2, 2021

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.

TwiN added a commit to TwiN/gatus that referenced this issue Jan 20, 2023
Fixes #406

Root cause seems to be that promhttp.Handler() has its own gzip compression prometheus/client_golang#622
ti-chi-bot pushed a commit to tikv/pd that referenced this issue Apr 20, 2023
ref prometheus/client_golang#622, ref #5895

Signed-off-by: Ryan Leung <rleungx@gmail.com>
rleungx added a commit to rleungx/pd that referenced this issue Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants