client_java icon indicating copy to clipboard operation
client_java copied to clipboard

HTTP server responds with metrics on any url

Open robinp-tw opened this issue 5 years ago • 12 comments
trafficstars

It seems client_java/simpleclient_httpserver/src/main/java/io/prometheus/client/exporter/HTTPServer.java creates context mappings for / and /metrics, which according to https://docs.oracle.com/javase/8/docs/jre/api/net/httpserver/spec/com/sun/net/httpserver/HttpServer.html#mapping_description means that - without further filtering - it just responds to any url with the metrics (even /foo/bar).

This results in a great scraping load on the server if it is hit by, for example, security scanners.

robinp-tw avatar May 12 '20 09:05 robinp-tw

I don't consider this to be a problem, if something can get to you over HTTP there's many ways it can cause a DOS.

brian-brazil avatar May 12 '20 09:05 brian-brazil

I agree with the sentiment, but the binding of the HTTP handler at https://github.com/prometheus/client_java/blob/master/simpleclient_httpserver/src/main/java/io/prometheus/client/exporter/HTTPServer.java#L165 is a bit misleading.

Binding explicitly to both / and /metrics, the naive reader would expect that it is only those two paths that the handler responds to. But those are rather path roots, and the handler will respond to any path under those roots.

So the /metrics is redundant. How about either fixing the handler by checking the actual path, or by dropping the /metrics binding to make it explicit that the server will reply with the metrics regardless of the actual path?

robinp-tw avatar May 22 '20 09:05 robinp-tw

Yeah, I can see that code as confusing - it confused me anyway.

brian-brazil avatar May 22 '20 10:05 brian-brazil

We just discovered this issue. So to address this issue, I have two questions: Is it necessary to keep the metrics exposed to the root path /? Is it possible to make this metrics path something configurable so that we can customize it?

qinghui-xu avatar Oct 05 '20 15:10 qinghui-xu

The Prometheus default is /metrics, so I see no need to make it configurable. Providing the metrics when the users visits the bare endpoint is also useful.

brian-brazil avatar Oct 05 '20 17:10 brian-brazil

In our case, we use JavaAgent export metrics without Intrusion into the application code. It will be helpful if the endpoint is configurable when the application has already used /metrics。

remones avatar Dec 16 '20 17:12 remones

That is unrelated, the code we're talking about here has complete ownership of the port - there are no other endpoints.

brian-brazil avatar Dec 16 '20 17:12 brian-brazil

This is working as designed. I propose we should close this issue.

dhoard avatar Sep 15 '21 12:09 dhoard

We want to serve some other things at another endpoint , so how can we do that? This client serves the same thing at all endpoints . Please help. It should take in a custom input for endpoint and serve metrics at that endpoint only.

zlearner1 avatar Nov 10 '22 15:11 zlearner1

@brian-brazil @fstab can you please make the endpoint a parameter that can be passed to HTTPServer's constructor? and not expose metrics at all endpoints? This is actually a serious bug and hinders us from exposing other things at other endpoints. We may have a default endpoint as '/metrics' but it should be customizable. Even if someone wants to serve metrics at multiple endpoints, the parameter should be taking an array of endpoints. I hope this makes sense. Looking forward to your help. Thanks.

zlearner1 avatar Nov 11 '22 05:11 zlearner1

@zlearner1 The HTTPServer is not meant to be a generic HTTP server for any/all applications.

If you are using com.sun.net.httpserver.HttpServer for your application, you can use HTTPServer.HTTPMetricHandler and add it as a handler on a specific path to your instance of com.sun.net.httpserver.HttpServer via code.

If you are using an application server, you should use the Servlet in simpleclient_servlet or simpleclient_servlet_jakarta and bind it to a specific path via Servlet configuration.

In either scenario, you control the path for metrics. This doesn't prevent your other applications from using other paths.

dhoard avatar Nov 11 '22 15:11 dhoard

I think it makes sense to expose metrics on only one path, defaulting to /metrics. However, it would be a breaking change.

We can do breaking changes with rel 1.0 because of the major version jump. So instead of implementing this now in a backwards-compatible way, it might be better to do it with 1.0 in a breaking way.

fstab avatar Nov 11 '22 20:11 fstab