Prometheus remote write endpoint #16609

ChrsMark · 2020-02-26T15:40:40Z

What does this PR do?

This PR implements Prometheus remote write as a new metricset.

Why is it important?

How to test this PR locally

Start metricbeat

Enable metricbeat Prometheus module
config:

- module: prometheus
  metricsets: ["remote_write"]
  host: "localhost"
  port: "9201"

start metricbeat and expect to see:

2020-02-27T17:49:14.724+0200	DEBUG	[module]	module/wrapper.go:189	Starting metricSetWrapper[module=prometheus, name=remote_write, host=]
2020-02-27T17:49:14.724+0200	INFO	remote_write/remote_write.go:92	Starting HTTP server on localhost:9201

Sending metrics from Prometheus

Use this forked project: https://github.com/ChrsMark/dockprom.

Download the project.
Add to `prometheus/prometheus.yml:

remote_write:
  - url: "http://host.docker.internal:9201/write"

docker-compose up

With that metrics should be collected by metricbeat and reported.

Extra: Test with https

Generate ssl keys:

openssl genrsa -out my_key.key 2048
openssl req -x509 -new -nodes -key my_key.key -sha256 -days 1825 -out my_key.pem

Configure Prometheus to use https for remote_write: (edit /prometheus/prometheus.yml of the above docker-compose project)

remote_write:
  - url: "https://host.docker.internal:9201/write"
    tls_config:
        cert_file: "/etc/prometheus/my_key.pem"
        key_file: "/etc/prometheus/my_key.key"
        insecure_skip_verify: true

Move my_key.key and my_key.pem under /prometheus folder in the docker-compose project.
Configure Metricbeat to start HTTPS server using my_key.key and my_key.pem.

- module: prometheus
  metricsets: ["remote_write"]
  host: "localhost"
  port: "9201"

  # Secure settings for the server using TLS/SSL:
  ssl.certificate: "/Users/some/pki/my_key.pem"
  ssl.key: "/Users/some/pki/my_key.key"

Related issues

Closes [Metricbeat] Prometheus Remote Endpoint #14983

Notes about changes

This PR changes HttpServer so as to accept handlerFunc from metricset on server initialisation. This makes it reusable from different metricsets and not only server metricset of http module. In order to confirm that server metricset of http module is not broken I run INTEGRATION_TESTS=1 BEAT_STRICT_PERMS=false nosetests ./module/http/test_http.py successfully which shows that functionality is preserved.

Signed-off-by: chrismark <[email protected]>

ChrsMark · 2020-02-28T14:09:26Z

For this we will need to vendor github.com/prometheus/prometheus/prompb. Would this be a problem for the go-modules-porting @kvch?

update: I guess I could wait for #15853 and make use of go modules.

kvch · 2020-02-28T15:00:38Z

If it is not urgent, please wait until go modules is merged. It is a huge effort to keep go-modules up to date, so I am pushing to get it in as soon as possible.

ChrsMark · 2020-02-28T15:06:12Z

If it is not urgent, please wait until go modules is merged. It is a huge effort to keep go-modules up to date, so I am pushing to get it in as soon as possible.

Cool! No problem on waiting!

Signed-off-by: chrismark <[email protected]>

ChrsMark · 2020-03-05T15:59:06Z

@exekias any feedback at this point would be more than welcome. It is becoming quite big but we can split it in smaller PRs like so:

first PR to add a working metricset
refactoring code to utilize http helpers
system tests

exekias

Left a few comments, I like where things are going!

exekias · 2020-03-05T16:02:46Z

metricbeat/module/prometheus/prometheus_pb2.py

@@ -0,0 +1,577 @@
+# -*- coding: utf-8 -*-


is this file needed?

It is required by the python tests: https://github.com/elastic/beats/pull/16609/files#diff-f0fbea18a4681afd920cb91b903d03d2R6

metricbeat/module/prometheus/_meta/config.yml

exekias · 2020-03-05T16:05:50Z

metricbeat/module/prometheus/remote_write/data.go

+	return p.labels.String()
+}
+
+func samplesToEvents(metrics model.Samples) map[string]mb.Event {


I understand Samples will always be untyped right? For example, we won't be able to differenciate a histogram metric from a gauge

Prometheus sends data with writerequest using this proto. Not sure if we can retrieve the type of the metric in an easy way 🤔

I guess this is going to be an issue with histograms? Should we do a best-effort guess if it includes metrics with some special labels and/or suffixes?

exekias · 2020-03-05T16:07:12Z

metricbeat/module/prometheus/remote_write/data.go

+		if !math.IsNaN(val) && !math.IsInf(val, 0) {
+			events = append(events, PromEvent{
+				data: common.MapStr{
+					name: val,
+				},
+				labels:    labels,
+				timestamp: metric.Timestamp.Time(),
+			})
+		}
+	}
+
+	// join metrics with same labels in a single event
+	eventList := map[string]mb.Event{}
+
+	for _, promEvent := range events {
+		labelsHash := promEvent.LabelsHash()
+		if _, ok := eventList[labelsHash]; !ok {
+			eventList[labelsHash] = mb.Event{
+				ModuleFields: common.MapStr{
+					"metrics": common.MapStr{},
+				},
+			}
+
+			// Add labels
+			if len(promEvent.labels) > 0 {
+				eventList[labelsHash].ModuleFields["labels"] = promEvent.labels
+			}
+		}
+
+		// Not checking anything here because we create these maps some lines before
+		e := eventList[labelsHash]
+		e.Timestamp = promEvent.timestamp
+		e.ModuleFields["metrics"].(common.MapStr).Update(promEvent.data)
+	}


this can probably be optimized, by storing events already indexed by label hash, so only an insert is needed

exekias · 2020-03-05T16:20:29Z

metricbeat/helper/server/http/http.go

+// was an error creating the MetricSet then an error will be returned. The
+// returned MetricSet must also implement either EventFetcher or EventsFetcher
+// (but not both).
+type HandlerFactory func(h *HttpServer) func(writer http.ResponseWriter, req *http.Request)


Do you really need a factory? The metricset is already initializing the server, so you could pass a private metricset.handleFunc which implements func(writer http.ResponseWriter, req *http.Request)

exekias · 2020-03-05T16:23:34Z

metricbeat/helper/server/http/http.go

+func (h *HttpEvent) SetEvent(event common.MapStr) {
+	h.event = event
+}


Having that you are now passing a custom handler when initializing (which I think is a good idea), do you need an HttpEvent construct anymore? This was used to pass data between a generic HTTP server and the metricset. But now the metricset is in control of the requests, and can directly build and send events to the output. That should make things simpler, WDYT?

Signed-off-by: chrismark <[email protected]>

ChrsMark · 2020-03-06T11:04:31Z

Note: In order to confirm that server metricset of http module is not broken I run INTEGRATION_TESTS=1 BEAT_STRICT_PERMS=false nosetests ./module/http/test_http.py successfully which shows that functionality is preserved.

Signed-off-by: chrismark <[email protected]>

metricbeat/module/prometheus/docker-compose.yml

metricbeat/helper/server/http/http_test.go

metricbeat/helper/server/http/http.go

metricbeat/module/http/server/data.go

metricbeat/module/http/server/server.go

metricbeat/module/prometheus/_meta/prometheus.yml

metricbeat/module/prometheus/test_prometheus.py

metricbeat/module/prometheus/remote_write/remote_write.go

Signed-off-by: chrismark <[email protected]>

metricbeat/helper/server/http/http.go

metricbeat/module/prometheus/remote_write/data.go

jsoriano · 2020-03-10T17:32:22Z

metricbeat/module/prometheus/remote_write/data.go

+	return p.labels.String()
+}
+
+func samplesToEvents(metrics model.Samples) map[string]mb.Event {


I guess this is going to be an issue with histograms? Should we do a best-effort guess if it includes metrics with some special labels and/or suffixes?

ChrsMark · 2020-03-10T17:32:44Z

Comments addressed. Let's see if CI is happy.

Signed-off-by: chrismark <[email protected]>

ChrsMark · 2020-03-11T09:15:49Z

@jsoriano regarding #16609 (comment), this is something we discussed with @exekias already and guessing would be the way to go when we will decide to adopt histograms for this metricset.

I think that comments have been addressed 🙂 .

jsoriano · 2020-03-11T16:01:03Z

metricbeat/module/prometheus/_meta/run.sh

+  else
+    # this works only on Mac envs
+    HOST_DOMAIN="host.docker.internal"
+    break


Why only breaking in this case?

We want to keep trying to reach host.docker.internal otherwise to fallback to 0.0.0.0.

(cherry picked from commit a6f406b)

WIP

4039278

ChrsMark added in progress Pull request is currently in progress. Metricbeat Metricbeat [zube]: In Progress Team:Integrations Label for the Integrations team Team:Platforms Label for the Integrations - Platforms team labels Feb 26, 2020

ChrsMark requested a review from a team as a code owner February 26, 2020 15:40

ChrsMark self-assigned this Feb 26, 2020

ChrsMark changed the title ~~[WIP] Prometheus remote endpoint~~ [WIP] Prometheus remote write endpoint Feb 27, 2020

ChrsMark added 4 commits February 27, 2020 16:26

remove remote_read functionality

a356dd7

Signed-off-by: chrismark <[email protected]>

make update

8cc5380

Signed-off-by: chrismark <[email protected]>

Add docs and fix config

0b35dcd

Signed-off-by: chrismark <[email protected]>

Add data.json

5bf8324

Signed-off-by: chrismark <[email protected]>

ChrsMark added 3 commits March 4, 2020 12:12

Merge remote-tracking branch 'upstream/master' into prom_remote_endpoint

da960f3

rebase

db3bad5

Signed-off-by: chrismark <[email protected]>

add dep using go mod

0adda8c

Signed-off-by: chrismark <[email protected]>

ChrsMark force-pushed the prom_remote_endpoint branch from fb85033 to 0adda8c Compare March 4, 2020 10:45

ChrsMark added 2 commits March 5, 2020 15:20

Add python system test

d51a12a

Signed-off-by: chrismark <[email protected]>

refactoring

58e41ca

Signed-off-by: chrismark <[email protected]>

exekias reviewed Mar 5, 2020

View reviewed changes

refactor handler functions

d052651

Signed-off-by: chrismark <[email protected]>

ChrsMark force-pushed the prom_remote_endpoint branch from 0cfcdbc to d052651 Compare March 6, 2020 09:07

ChrsMark changed the title ~~[WIP] Prometheus remote write endpoint~~ Prometheus remote write endpoint Mar 6, 2020

optimisations

fd89b1f

Signed-off-by: chrismark <[email protected]>

ChrsMark requested a review from exekias March 6, 2020 10:48

ChrsMark added 2 commits March 10, 2020 11:16

Merge remote-tracking branch 'upstream/master' into prom_remote_endpoint

001e4d8

bump up container version

18c6ce2

Signed-off-by: chrismark <[email protected]>

jsoriano reviewed Mar 10, 2020

View reviewed changes

ChrsMark added 2 commits March 10, 2020 17:29

review changes

eb755ac

Signed-off-by: chrismark <[email protected]>

fixes

841fc7c

Signed-off-by: chrismark <[email protected]>

jsoriano reviewed Mar 10, 2020

View reviewed changes

ChrsMark added 3 commits March 10, 2020 20:53

refactor NewHttpServer and NewHttpServerWithHandler

453e808

Signed-off-by: chrismark <[email protected]>

fix run.sh script

02c7902

Signed-off-by: chrismark <[email protected]>

remove leftover

6402466

Signed-off-by: chrismark <[email protected]>

Merge remote-tracking branch 'upstream/master' into prom_remote_endpoint

05bdecd

ChrsMark added test-plan Add this PR to be manual test plan and removed test-plan Add this PR to be manual test plan labels Mar 11, 2020

jsoriano approved these changes Mar 11, 2020

View reviewed changes

ChrsMark added needs_backport PR is waiting to be backported to other branches. v7.7.0 labels Mar 12, 2020

ChrsMark merged commit a6f406b into elastic:master Mar 12, 2020

zube bot added [zube]: Done and removed [zube]: In Progress labels Mar 12, 2020

ChrsMark mentioned this pull request Mar 12, 2020

Cherry-pick #16609 to 7.x: Prometheus remote write endpoint #16973

Merged

ChrsMark added a commit to ChrsMark/beats that referenced this pull request Mar 12, 2020

Prometheus remote write endpoint (elastic#16609)

bd05439

(cherry picked from commit a6f406b)

ChrsMark removed the needs_backport PR is waiting to be backported to other branches. label Mar 12, 2020

ChrsMark mentioned this pull request Mar 13, 2020

Improve Prometheus remote_write metricset docs #16997

Merged

ChrsMark added a commit that referenced this pull request Mar 16, 2020

Cherry-pick #16609 to 7.x: Prometheus remote write endpoint (#16973)

d8cd2ff

ChrsMark mentioned this pull request Mar 16, 2020

Cherry-pick #16997 to 7.x: Improve Prometheus remote_write metricset docs #17017

Merged

andresrc removed the [zube]: Done label Mar 20, 2020

exekias added the test-plan Add this PR to be manual test plan label Mar 27, 2020

andresrc added the test-plan-added This PR has been added to the test plan label Mar 31, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prometheus remote write endpoint #16609

Prometheus remote write endpoint #16609

ChrsMark commented Feb 26, 2020 •

edited

Loading

ChrsMark commented Feb 28, 2020 •

edited

Loading

kvch commented Feb 28, 2020

ChrsMark commented Feb 28, 2020

ChrsMark commented Mar 5, 2020

exekias left a comment

exekias Mar 5, 2020

ChrsMark Mar 6, 2020

exekias Mar 5, 2020

ChrsMark Mar 6, 2020

jsoriano Mar 10, 2020

exekias Mar 5, 2020

exekias Mar 5, 2020

exekias Mar 5, 2020

ChrsMark commented Mar 6, 2020 •

edited

Loading

jsoriano Mar 10, 2020

ChrsMark commented Mar 10, 2020

ChrsMark commented Mar 11, 2020

jsoriano Mar 11, 2020

ChrsMark Mar 11, 2020

Prometheus remote write endpoint #16609

Prometheus remote write endpoint #16609

Conversation

ChrsMark commented Feb 26, 2020 • edited Loading

What does this PR do?

Why is it important?

How to test this PR locally

Start metricbeat

Sending metrics from Prometheus

Extra: Test with https

Related issues

Notes about changes

ChrsMark commented Feb 28, 2020 • edited Loading

kvch commented Feb 28, 2020

ChrsMark commented Feb 28, 2020

ChrsMark commented Mar 5, 2020

exekias left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChrsMark commented Mar 6, 2020 • edited Loading

Choose a reason for hiding this comment

ChrsMark commented Mar 10, 2020

ChrsMark commented Mar 11, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChrsMark commented Feb 26, 2020 •

edited

Loading

ChrsMark commented Feb 28, 2020 •

edited

Loading

ChrsMark commented Mar 6, 2020 •

edited

Loading