Ignore parser errors from unsupported metrics types on Prometheus client and continue parsing until EOF is reached. #37357

gsantoro · 2023-12-08T13:33:46Z

Proposed commit message

Ignore parser errors from unsupported metrics types on Prometheus client and continue parsing until EOF is reached.

For example Prometheus client with default text/plain format doesn't support info metrics and it will generate an error at https://github.com/prometheus/prometheus/blob/965e603fa792bca0900ac76eb45ae84c81af1cdf/model/textparse/promparse.go#L319.

Currently that error breaks the parsing and it ignores any remaining metrics in the list. With this change instead we will correctly parse all the supported metrics types and ignore the unsupported ones.

Metrics of type info are correctly supported by OpenMetrics format. KSM supports OpenMetrics text format as well but in Beats the helper is hardcoded to use only plain text format at

beats/metricbeat/helper/prometheus/prometheus.go

Line 34 in 8b18c36

const acceptHeader = `text/plain;version=0.0.4;q=0.5,*/*;q=0.1`

.

Checklist

My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have made corresponding change to the default configuration files
I have added tests that prove my fix is effective or that my feature works
I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Author's Checklist

[ ]

How to test this PR locally

See related issue #37198 on how to test these changes

Related issues

Closes Missing datasets that are relying on kube-state-metrics in case custom resource state metrics are enabled #37198

Use cases

Screenshots

Logs

mergify · 2023-12-08T13:34:22Z

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @gsantoro? 🙏.
For such, you'll need to label your PR with:

The upcoming major version of the Elastic Stack
The upcoming minor version of the Elastic Stack (if you're not pushing a breaking change)

To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

backport-v8./d.0 is the label to automatically backport to the 8./d branch. /d is the digit

elasticmachine · 2023-12-08T13:35:20Z

Pinging @elastic/obs-ds-hosted-services (Team:obs-ds-hosted-services)

elasticmachine · 2023-12-08T13:39:55Z

❕ Build Aborted

There is a new build on-going so the previous on-going builds have been aborted.

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2023-12-08T13:34:07.227+0000
Duration: 6 min 42 sec

Steps errors

Expand to view the steps failures

`Error signal`

Took 0 min 0 sec . View more details here
Description: tar: step failed with error null

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.
/package : Generate the packages and run the E2E tests.
/beats-tester : Run the installation tests with beats-tester.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

tetianakravchenko · 2023-12-08T13:51:51Z

metricbeat/helper/prometheus/textparse.go

+			// any `info` metrics are not supported, and then there will be ignored here.
+			// if acceptHeader in the prometheus client `Accept: application/openmetrics-text; version=0.0.1`
+			// any `info` metrics are supported, and then there will be parsed here.
+			continue


can we add a debug log for such cases as well?

usually I would agree with you but logging in that class would be a non-trivial change. You would need to pass the prometheus logger to this method if you didn't want to create a new logger.

I'm not sure about it

elasticmachine · 2023-12-08T14:45:57Z

💔 Build Failed

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Duration: 70 min 23 sec

Pipeline error

This error is likely related to the pipeline itself. Click here
and then you will see the error (either incorrect syntax or an invalid configuration).

❕ Flaky test report

No test was executed to be analysed.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.
/package : Generate the packages and run the E2E tests.
/beats-tester : Run the installation tests with beats-tester.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

tetianakravchenko

please add the CHANGELOG record

elasticmachine · 2023-12-11T12:31:35Z

💔 Tests Failed

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2023-12-11T11:30:43.198+0000
Duration: 61 min 43 sec

Test stats 🧪

Test	Results
Failed	15
Passed	4872
Skipped	397
Total	5284