istio / ztunnel

The `ztunnel` component of ambient mesh
Apache License 2.0
306 stars 101 forks source link

metrics endpoint sends content-type: application/openmetrics-text #1356

Open craigbox opened 2 weeks ago

craigbox commented 2 weeks ago

Could ztunnel serve /metrics as text/plain if the user hasn't explicitly sent an Accept: header for application/openmetrics-text? Without, the browser will download the file, rather than showing it as plain text on the screen.

https://github.com/spring-projects/spring-boot/issues/28446 is an example of this pattern being fixed in Spring Boot.

(A waypoint, OTOH, will return text/plain and work as expected)

howardjohn commented 2 weeks ago

Envoy: content-type: text/plain; charset=UTF-8 Istio-agent: Content-Type: text/plain; version=0.0.4; charset=utf-8 Ztunnel: content-type: application/openmetrics-text;charset=utf-8;version=1.0.0

So yeah this looks right && there is a bug in istio-agent as well putting version when it shouldn't

ilrudie commented 2 weeks ago

This definitely is fixable, just need to decide on the correct behavior. Based on the above from John, do we care to ever have the openmetrics-text content type or just always say its plain text?

howardjohn commented 2 weeks ago

We need to say it openmetrics-text when prometheus requests it I believe

On Mon, Nov 4, 2024 at 7:43 AM Ian Rudie @.***> wrote:

This definitely is fixable, just need to decide on the correct behavior. Based on the above from John, do we care to ever have the openmetrics-text content type or just always say its plain text?

— Reply to this email directly, view it on GitHub https://github.com/istio/ztunnel/issues/1356#issuecomment-2455053289, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEYGXNAXFCEZ5462ETFKZLZ66B2JAVCNFSM6AAAAABRDGGDQGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINJVGA2TGMRYHE . You are receiving this because you commented.Message ID: @.***>

howardjohn commented 2 weeks ago

Just to be clear, the envoy/istio-proxy ones were when i did NOT send the 'accept' header at all. Prometheus will - I didn't test how they behave then, but its definitely some non-plaintext content

On Mon, Nov 4, 2024 at 7:52 AM John Howard @.***> wrote:

We need to say it openmetrics-text when prometheus requests it I believe

On Mon, Nov 4, 2024 at 7:43 AM Ian Rudie @.***> wrote:

This definitely is fixable, just need to decide on the correct behavior. Based on the above from John, do we care to ever have the openmetrics-text content type or just always say its plain text?

— Reply to this email directly, view it on GitHub https://github.com/istio/ztunnel/issues/1356#issuecomment-2455053289, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEYGXNAXFCEZ5462ETFKZLZ66B2JAVCNFSM6AAAAABRDGGDQGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINJVGA2TGMRYHE . You are receiving this because you commented.Message ID: @.***>

ilrudie commented 2 weeks ago

In testing, near as I can tell envoy ignores the accept header entirely. You have 2 different content-types being returned which are based on query params, not headers. If you include ?format=json you get 'application/json' and for everything else you get 'text/plain'. This assumes you are querying the internal admin interface which has more options. If you use 15090/stats/prometheus then it always uses 'text/plain' even if you request json formatting.

howardjohn commented 2 weeks ago

Well that might break with prometheus 3.0! https://github.com/prometheus/prometheus/pull/15136

Note we don't always hit envoy directly but rather proxy through istio-agent, so that one may matter less

ilrudie commented 2 weeks ago

Interesting. I think envoy behavior is probably fine for this. They call out that text/plain is the content-type expected for Prom parser and Envoy specifically supports Prom format, but does not support OpenMetrics (which is similar but stricter from what I can gather).

howardjohn commented 2 weeks ago

Ztunnel is more strictly openmetrics fwiw

ilrudie commented 2 weeks ago

Perfect. For ztunnel we can have the "if you request openmetrics with Accept you get openmetrics back" behavior then. I have some code in the works but it is not pushed anywhere yet.