Verify new Prometheus exposition formats are supported

[ChrsMark]

Recently there have been reported issues with new openmetrics types. We need to investigate how new formats will be supported and maybe update our prometheus library (expfmt) (or move to a new one) so as to support all generic cases and eliminate parsing errors.

• New openmetrics types giving problems new openmetrics types are not supported for prometheus module #24554
• Move out of expfmt? Evaluate using textparse

Something interesting is that at #17291 the header was added to work in general for all cases since these are the headers used by prometheus upstream too.

cc: @exekias @jsoriano @newly12 @masci

[elasticmachine]

Pinging @elastic/integrations (Team:Integrations)

[ChrsMark]

I did some investigation based on the issue reported at #24554. I used instrumenting-java-for-prometheus repo mentioned at #24554 (comment) so as to mime a prom exporter with the new type of metrics.

1. I see that return headers are set to version 1.0.0 for openmetrics:

curl -X HEAD -i http://localhost:8081/metrics -H "Accept: application/openmetrics-text ; version=0.0.1" 
Warning: Setting custom HTTP method to HEAD with -X/--request may not work the 
Warning: way you want. Consider using -I/--head instead.
HTTP/1.1 200 OK
Connection: keep-alive
Content-Type: application/openmetrics-text; version=1.0.0;charset=utf-8
Content-Length: 6644
Date: Tue, 23 Mar 2021 13:35:11 GMT

Despite the fact that I explicitly set them to 0.0.1. Maybe it falls back to 1.0.0 by default.

2. Openmetrics Exposition Format seems to have diverged from Prometheus Exposition Format. See https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md.
3. Based on the above we cannot support new openmetrics format since expfmt does not support decoding it yet: https://github.com/prometheus/common/blob/4240322d2138024beddb57544851d77a08afe19c/expfmt/decode.go#L52. Only ProtoType and TextType are supported. Also https://github.com/prometheus/common/blob/4240322d2138024beddb57544851d77a08afe19c/expfmt/decode.go#L75.
4. We fail to detect an unknown format at

   beats/metricbeat/helper/prometheus/prometheus.go

   Line 101 in fbaeefe

   if format == "" {

   since we compare to "" and not to unknown (https://github.com/prometheus/common/blob/90d71d7138448baa26007aa224ee917eea49f897/expfmt/expfmt.go#L30)

Taking the above into consideration having application/openmetrics-text inside the headers at

beats/metricbeat/helper/prometheus/prometheus.go

Line 38 in c685997

const acceptHeader = `application/openmetrics-text; version=0.0.1,text/plain;version=0.0.4;q=0.5,*/*;q=0.1`

does not make any sense, we can remove it. Despite we could support openmetrics with openmetrics module so far we cannot from now on since prometheus and openmetrics formats have diverged.

If we want to support openmetrics from now on we need to update our parsing helper and use
textparse and according to the response headers create an OpenMetricsParser or a PromParser.

[jsoriano]

I see that return headers are set to version 1.0.0 for openmetrics:

The Java client seems to check only that application/opnemetrics-text is included: https://github.com/prometheus/client_java/blob/29dfd0a2bf9b93282764d31447b843a1d36d4f10/simpleclient_common/src/main/java/io/prometheus/client/exporter/common/TextFormat.java#L38, it doesn't check the version.

If for version < 1.0.0 this should provide the old format, would this be a bug in this library?

In any case updating the parser sounds as the way to go.

[vjsamuel]

this is a big bug that we are running into at this time. given that this is only required for open metrics module, we should ideally only set this header for the same instead of doing it across all prometheus like modules. once we have moved parser implementations, we can make this the default. thoughts?

[exekias]

I see openmetrics introduced a few new metric types, we will need to think about how we will map them into ES documents, I see that at least we need to think about what to do with info, gaugehistogram and stateset

[ChrsMark]

There is an open PR related to this: #26308

[ChrsMark]

New PR: #27269

[ChrsMark]

So to summarize the open items here:

1. The issue with duplicated types reported at Verify new Prometheus exposition formats are supported #24707 should be fixed by Use textparse in prometheus module helper library #33865. This needs to be verified with tests and also verify that Type duplication is expected in Prometheus metrics however based on Handle duplicated TYPE line for prometheus metrics #18813 (comment) we need to verify if Prometheus can parse such endpoints and we should follow accordingly. @gizas if that's the case this is quite important so we could move forward and make the switch of the library with Use textparse in prometheus module helper library #33865.
2. We also need to cover the new Openmetric types in ES (investigate if doable) as mentioned at Verify new Prometheus exposition formats are supported #24707 (comment). This could covered at Create Openmetrics package integrations#628 as mentioned in its description.

[ChrsMark]

So to summarize the open items here:

1. The issue with duplicated types reported at Verify new Prometheus exposition formats are supported #24707 should be fixed by Use textparse in prometheus module helper library #33865. This needs to be verified with tests and also verify that Type duplication is expected in Prometheus metrics however based on Handle duplicated TYPE line for prometheus metrics #18813 (comment) we need to verify if Prometheus can parse such endpoints and we should follow accordingly. @gizas if that's the case this is quite important so we could move forward and make the switch of the library with Use textparse in prometheus module helper library #33865.

The respective PR was merged: #33865

2. We also need to cover the new Openmetric types in ES (investigate if doable) as mentioned at Verify new Prometheus exposition formats are supported #24707 (comment). This could covered at Create Openmetrics package integrations#628 as mentioned in its description.

It will be addressed at elastic/integrations#628.

Since both items are covered we can close this issue by now (cc: @gizas).