Openmetrics support using textparser by premendrasingh · Pull Request #27269 · elastic/beats

premendrasingh · 2021-08-06T05:44:06Z

Enhancement - Added openmetrics module to use Openmetrics parser.

What does this PR do?

This PR updated metricset in Openmetrics module. It handles metrics endpoints returning data with
Content-Type: application/openmetrics-text

Why is it important?

Many of our customers have data exposed in OpenMetrics format, but existing module piggy backs on Prometheus module which processes metrics only in expfmt, Openmetrics data is not handled properly. Hence the need for Openmetrics support also.

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.

How to test this PR locally

Unit tests in openmetrics/collector can be used to test the metrics locally.

Setup a http server to serve Openmetrics data with HTTP header Content-Type: application/openmetrics-text

Change metricset in metricbeat.yml to use openmetrics/collector as shown below

metricbeat.modules:
- module: openmetrics
  hosts: ["http://localhost:8080/metrics"]
  metricsets: ["collector"]

mergify · 2021-08-06T05:44:40Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b openmetrics-collector upstream/openmetrics-collector
git merge upstream/master
git push upstream openmetrics-collector

…-collector

elasticmachine · 2021-08-06T06:58:13Z

💚 Build Succeeded

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: 2021-11-16T10:34:06.284+0000
Duration: 47 min 41 sec
Commit: ff107b4

Test stats 🧪

Test	Results
Failed	0
Passed	708
Skipped	239
Total	947

💚 Flaky test report

Tests succeeded.

🤖 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!)

mergify · 2021-08-11T18:45:38Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b openmetrics-collector upstream/openmetrics-collector
git merge upstream/master
git push upstream openmetrics-collector

…-collector

mergify · 2021-08-12T02:26:43Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b openmetrics-collector upstream/openmetrics-collector
git merge upstream/master
git push upstream openmetrics-collector

…-collector

mergify · 2021-08-13T08:27:44Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b openmetrics-collector upstream/openmetrics-collector
git merge upstream/master
git push upstream openmetrics-collector

…-collector

elasticmachine · 2021-08-19T17:32:34Z

Pinging @elastic/integrations (Team:Integrations)

jsoriano · 2021-08-19T17:32:58Z

/test

…-collector

ChrsMark

Thank for this PR @premendrasingh , it is going into the right direction I think!

I left some comments inline, mostly about minors, but overall the patch looks good. My two main concenrs are:

I see a lot of code duplication between between this module and prometheus module around helper functionalities. Wondering if we could handle it better by introducing one common library?
I see we add new types with PR. Maybe this needs evaluation product-wise.

@exekias @jsoriano your feedback is more than welcome here :).

metricbeat/docs/modules/linux/memory.asciidoc

metricbeat/module/openmetrics/collector/collector_test.go

metricbeat/module/openmetrics/collector/data.go

ChrsMark · 2021-08-31T12:54:52Z

metricbeat/module/openmetrics/collector/collector.go

+	return m.skipFamilyName(*family.Name)
+}
+
+func (m *MetricSet) skipFamilyName(family string) bool {


I see/expect some replication of code between the 2 modules (prometheus and openmetircs) so maybe we can move those methods to a utility library when we refactor prometheus module too.

I think it will be better to have a separate PR for refactoring this into a utility library.

Yeap, doing this when we change prometheus module to use textparser or in a different follow-up sounds good.

metricbeat/module/openmetrics/_meta/fields.yml

ChrsMark · 2021-08-31T13:32:22Z

metricbeat/helper/openmetrics/openmetrics_test.go

+)
+
+const (
+	openMetricsTestSamples = `# TYPE first_metric gauge


Maybe also add a case with exemplars?

…-collector

premendrasingh

Thanks for the review comments.

metricbeat/docs/modules/linux/memory.asciidoc

premendrasingh · 2021-09-01T05:02:29Z

metricbeat/module/openmetrics/_meta/fields.yml

+        - name: type
+          type: keyword
+          description: >
+            metric type


changed to Metric

metricbeat/module/openmetrics/_meta/fields.yml

premendrasingh · 2021-09-01T05:04:52Z

metricbeat/module/openmetrics/collector/collector.go

+		if m.skipFamily(family) {
+			continue
+		}
+		promEvents := m.promEventsGen.GenerateOpenMetricsEvents(family)


Yes, you are right. Changed to openMetricsEvents

premendrasingh · 2021-09-01T05:05:18Z

metricbeat/module/openmetrics/collector/collector.go

+				eventList[promEvent.Type][labelsHash]["exemplar"] = promEvent.Exemplars
+			}
+			// Accumulate metrics in the event
+			eventList[promEvent.Type][labelsHash].DeepUpdate(promEvent.Data)


Reference - https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md#name

metricbeat/module/openmetrics/collector/collector_test.go

metricbeat/module/openmetrics/collector/data.go

jsoriano

I haven't looked into the details of how each metric type is handled, but overall this is looking good.

jsoriano · 2021-09-01T08:53:41Z

go.mod

+	github.com/Azure/go-autorest => github.com/Azure/go-autorest v14.2.0+incompatible
 	github.com/Microsoft/go-winio => github.com/bi-zone/go-winio v0.4.15
 	github.com/Shopify/sarama => github.com/elastic/sarama v1.19.1-0.20210120173147-5c8cb347d877
 	github.com/cucumber/godog => github.com/cucumber/godog v0.8.1


Many changes here seem unrelated.

Upstream rebase removed it.

Umnm, there are still many other changes in go.sum that look unrelated (for google could, oracle, azure...).

Cleaned up go.mod, go.sum. Please guide me if it is still not right.

metricbeat/helper/openmetrics/openmetrics.go

metricbeat/module/openmetrics/_meta/fields.yml

metricbeat/mb/testing/testdata.go

metricbeat/module/openmetrics/_meta/fields.yml

metricbeat/module/openmetrics/collector/collector_test.go

mergify · 2021-09-01T17:56:51Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b openmetrics-collector upstream/openmetrics-collector
git merge upstream/master
git push upstream openmetrics-collector

…-collector

…uslabels

jsoriano · 2021-09-02T16:39:57Z

/test

ChrsMark · 2021-11-03T20:04:28Z

@newly12 sorry for the delay here but upgrading the library got stuck a little bit but now it is merged (#28716) and we can move this one forward 🙂 .

…-collector

ChrsMark · 2021-11-15T10:31:57Z

/test

metricbeat/docs/modules/openmetrics/collector.asciidoc

ChrsMark · 2021-11-16T10:33:26Z

/test

Dismissing my old review.

ChrsMark

LGTM, thank you for contributing this one!

mergify · 2021-11-17T08:10:55Z

This pull request does not have a backport label. Could you fix it @premendrasingh? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

backport-v./d./d./d is the label to automatically backport to the 7./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

premendrasingh · 2021-11-17T08:24:08Z

Thanks a lot @ChrsMark

jsoriano · 2021-11-17T10:46:58Z

Happy to see this merged 🙂 Thanks @premendrasingh and @ChrsMark!

premendrasingh added 3 commits August 3, 2021 16:48

Openmetrics support using textparser

22fe440

Remove created, commented code, reformat.

a78259d

Update go.mod for Azure/go-autorest

8d16c93

botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Aug 6, 2021

Merge remote-tracking branch 'beats_upstream/master' into openmetrics…

d5157de

…-collector

Merge remote-tracking branch 'beats_upstream/master' into openmetrics…

d33b99b

…-collector

Merge remote-tracking branch 'beats_upstream/master' into openmetrics…

809126c

…-collector

Merge remote-tracking branch 'beats_upstream/master' into openmetrics…

5f098b8

…-collector

jsoriano added the Team:Integrations Label for the Integrations team label Aug 19, 2021

botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Aug 19, 2021

premendrasingh added 3 commits August 20, 2021 07:03

Merge github.com:premendrasingh/beats into openmetrics-collector

eba0802

Merge remote-tracking branch 'beats_upstream/master' into openmetrics…

9d66568

…-collector

Merge remote-tracking branch 'beats_upstream/master' into openmetrics…

828e75a

…-collector

ChrsMark mentioned this pull request Aug 31, 2021

Verify new Prometheus exposition formats are supported #24707

Closed

ChrsMark reviewed Aug 31, 2021

View reviewed changes

premendrasingh added 2 commits August 31, 2021 18:37

Merge remote-tracking branch 'beats_upstream/master' into openmetrics…

9ed8773

…-collector

Changes based on PR review

5c55db1

premendrasingh commented Sep 1, 2021

View reviewed changes

jsoriano reviewed Sep 1, 2021

View reviewed changes

premendrasingh added 2 commits September 1, 2021 22:00

Merge remote-tracking branch 'beats_upstream/master' into openmetrics…

1572d2a

…-collector

Remove ; and commented code, remove import, change import to promethe…

6427329

…uslabels

ChrsMark mentioned this pull request Oct 29, 2021

Upgrade prometheus library #28716

Merged

premendrasingh added 4 commits November 14, 2021 11:58

Merge remote-tracking branch 'beats_upstream/master' into openmetrics…

605a024

…-collector

Restore go.mod

4400daf

do make update

b80441a

Make fmt

24ad943

ChrsMark requested review from ChrsMark, MichaelKatsoulis and jsoriano November 15, 2021 23:48

ChrsMark reviewed Nov 15, 2021

View reviewed changes

metricbeat/docs/modules/openmetrics/collector.asciidoc Show resolved Hide resolved

premendrasingh added 2 commits November 15, 2021 17:05

Review comment to change to openmetrics

80fe40d

make update reverted openmetrics back to host

ff107b4

jsoriano removed their request for review November 16, 2021 11:08

ChrsMark approved these changes Nov 17, 2021

View reviewed changes

ChrsMark added v8.1.0 enhancement and removed v7.16.0 backport-v7.16.0 Automated backport with mergify Team:Automation Label for the Observability productivity team labels Nov 17, 2021

mergify bot added the backport-skip Skip notification from the automated backport with mergify label Nov 17, 2021

ChrsMark merged commit ddf8b47 into elastic:master Nov 17, 2021

ChrsMark mentioned this pull request Nov 29, 2022

Use textparse in prometheus module helper library #33865

Merged

ChrsMark mentioned this pull request Dec 13, 2022

Handle duplicated TYPE line for prometheus metrics #18813

Closed

tetianakravchenko mentioned this pull request Feb 8, 2023

[Metricbeat] Testing: Allow use of different directory to test data instead of the expected one #34467

Merged

6 tasks

Conversation

premendrasingh commented Aug 6, 2021 • edited by ChrsMark Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Why is it important?

Checklist

How to test this PR locally

Uh oh!

mergify bot commented Aug 6, 2021

Uh oh!

elasticmachine commented Aug 6, 2021 • edited by jenkins-beats-ci bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💚 Build Succeeded

Build stats

Test stats 🧪

💚 Flaky test report

🤖 GitHub comments

Uh oh!

mergify bot commented Aug 11, 2021

Uh oh!

mergify bot commented Aug 12, 2021

Uh oh!

mergify bot commented Aug 13, 2021

Uh oh!

elasticmachine commented Aug 19, 2021

Uh oh!

jsoriano commented Aug 19, 2021

Uh oh!

ChrsMark left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

premendrasingh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jsoriano left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mergify bot commented Sep 1, 2021

premendrasingh commented Aug 6, 2021 •

edited by ChrsMark

Loading

elasticmachine commented Aug 6, 2021 •

edited by jenkins-beats-ci bot

Loading