Use Elasticsearch histogram type to store Prometheus histograms by exekias · Pull Request #17061 · elastic/beats

exekias · 2020-03-17T16:35:29Z

What does this PR do?

Allow to store Prometheus histograms as Elasticsearch histograms when using the collector metricset. This change adds a new use_types feature flag to enable the new behavior. When enabled it will use a new schema for Prometheus metrics:

Store counters as prometheus.*.counter
Store gauges/untyped as prometheus.*.value
Store histograms as prometheus.*.histogram
Summaries use the previous types

Also, when using rate_counters, a new prometheus.*.rate field will be created for all counter metrics. It stores the increment of the counter since the last fetch.

For better results, it is recommended to use tdigest.compression: 1 when calculating percentiles:

A note on graph fidelity

This is a first stab at supporting Prometheus Histograms using Elasticsearch histograms. As algorithms and query semantics differ between these two projects, the resulting graphs are not exactly the same (while both are representing correct data).

We will keep working with the Elasticsearch team to provide better fidelity when compared to Prometheus way of graphing histograms. This will be a good start: elastic/elasticsearch#49452

Why is it important?

Histograms are a big part of Prometheus metrics, and being able to store and query them correctly is a great feature when dealing with these.

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

How to test this PR locally

This feature requires Elasticsearch 7.6 or later. Also Kibana support is not yet merged into master, I'm testing this in combination with elastic/kibana#59387.

Related issues

Closes #14843

elasticmachine · 2020-03-17T16:35:32Z

Pinging @elastic/integrations-platforms (Team:Platforms)

x-pack/metricbeat/module/prometheus/_meta/fields.yml

exekias · 2020-03-18T10:08:49Z

x-pack/metricbeat/module/prometheus/collector/histogram.go

@iverase FYI, I'm working on this now 🙂, in case you want to have a look

x-pack/metricbeat/module/prometheus/collector/histogram.go

This is useful when we want to have different behaviors between flavours

This should make room for using different Elasticsearch types depending on the Prometheus metric

Co-Authored-By: Chris Mark <chrismarkou92@gmail.com>

metricbeat/module/prometheus/collector/_meta/docs.asciidoc

jsoriano

It LGTM. I think we could use use the new implementation by default in openmetrics from the very beginning as we haven't released it yet, would it make sense?
Apart of that, some minor suggestions and nitpicking.

metricbeat/module/prometheus/collector/_meta/docs.asciidoc

metricbeat/module/prometheus/collector/collector.go

x-pack/metricbeat/module/openmetrics/collector/collector.go

metricbeat/module/prometheus/collector/collector.go

x-pack/metricbeat/module/prometheus/collector/counter.go

jsoriano · 2020-03-24T11:39:39Z

x-pack/metricbeat/module/prometheus/collector/data.go

+}
+
+func (g *typedGenerator) Start() {
+	cfgwarn.Beta("Prometheus 'use_types' setting is beta")


This would need to be parameterized for openmetrics if used also there.

x-pack/metricbeat/module/prometheus/collector/data.go

jsoriano · 2020-03-24T11:49:34Z

x-pack/metricbeat/module/prometheus/collector/histogram.go

+	var counts []uint64
+
+	// calculate centroids and rated counts
+	var lastUpper, prevUpper float64


Nit. Use prevLastUpper or secondLastUpper to know that it refers to the second last upper bound seen?

Suggested change

var lastUpper, prevUpper float64

var lastUpper, prevLastUpper float64

x-pack/metricbeat/module/prometheus/docker-compose.yml

Co-Authored-By: Jaime Soriano Pastor <jaime.soriano@elastic.co>

ChrsMark

LGTM!

…tic#17061) * Allow to override already defined metricsets This is useful when we want to have different behaviors between flavours * Add new Prometheus skeleton for basic types usage * Refactor Prometheus module to accommodate newer uses * Add typed schema to Prometheus module This should make room for using different Elasticsearch types depending on the Prometheus metric * Convert Prometheus histograms to ES histograms Co-authored-by: Chris Mark <chrismarkou92@gmail.com> Co-authored-by: Jaime Soriano Pastor <jaime.soriano@elastic.co> (cherry picked from commit 142b859)

…Prometheus histograms (#17227) * Use Elasticsearch histogram type to store Prometheus histograms (#17061) * Allow to override already defined metricsets This is useful when we want to have different behaviors between flavours * Add new Prometheus skeleton for basic types usage * Refactor Prometheus module to accommodate newer uses * Add typed schema to Prometheus module This should make room for using different Elasticsearch types depending on the Prometheus metric * Convert Prometheus histograms to ES histograms Co-authored-by: Chris Mark <chrismarkou92@gmail.com> Co-authored-by: Jaime Soriano Pastor <jaime.soriano@elastic.co> (cherry picked from commit 142b859) * make update

exekias added enhancement in progress Pull request is currently in progress. Metricbeat Metricbeat needs_backport PR is waiting to be backported to other branches. v7.7.0 Team:Platforms Label for the Integrations - Platforms team labels Mar 17, 2020

exekias changed the title ~~Prometheus histograms~~ Use Elasticsearch histogram type to store Prometheus histograms Mar 17, 2020

ChrsMark reviewed Mar 18, 2020

View reviewed changes

x-pack/metricbeat/module/prometheus/_meta/fields.yml Outdated Show resolved Hide resolved

exekias commented Mar 18, 2020

View reviewed changes

exekias force-pushed the prometheus-histograms branch from 9f813f7 to 1be82a6 Compare March 18, 2020 10:31

andresrc added [zube]: Inbox [zube]: In Progress and removed [zube]: Inbox labels Mar 18, 2020

ChrsMark reviewed Mar 19, 2020

View reviewed changes

x-pack/metricbeat/module/prometheus/collector/histogram.go Outdated Show resolved Hide resolved

Carlos Pérez-Aradros Herce and others added 14 commits March 21, 2020 23:16

Allow to override already defined metricsets

8758921

This is useful when we want to have different behaviors between flavours

Refactor Prometheus module to accommodate newer uses

0de2208

Add new Prometheus skeleton for basic types usage

fc72602

Add typed schema to Prometheus module

e9b2782

This should make room for using different Elasticsearch types depending on the Prometheus metric

Convert Prometheus histograms to ES histograms

06bdb1d

Disable sorting, it's not needed

8820af6

Handle counter cache start/stop

3fbac0b

Remove duplicated docs

d5486d5

Do config warnings on start (not config) time

cf92843

Rate counters

2f84fb1

Unify up event codepaths

e76e792

Update config options

85e9799

Update x-pack/metricbeat/module/prometheus/collector/histogram.go

a75753b

Co-Authored-By: Chris Mark <chrismarkou92@gmail.com>

Add prometheus.yml.disabled

f05f1ce

exekias force-pushed the prometheus-histograms branch from 41ddf1e to f05f1ce Compare March 21, 2020 22:16

Update changelog

78d1941

exekias force-pushed the prometheus-histograms branch from 58f42fe to 78d1941 Compare March 24, 2020 09:54

Update docs

174b4fa

ChrsMark reviewed Mar 24, 2020

View reviewed changes

metricbeat/module/prometheus/collector/_meta/docs.asciidoc Outdated Show resolved Hide resolved

jsoriano reviewed Mar 24, 2020

View reviewed changes

exekias and others added 8 commits March 24, 2020 14:21

Update metricbeat/module/prometheus/collector/_meta/docs.asciidoc

5716c4e

Co-Authored-By: Jaime Soriano Pastor <jaime.soriano@elastic.co>

Update metricbeat/module/prometheus/collector/collector.go

de1973f

Co-Authored-By: Jaime Soriano Pastor <jaime.soriano@elastic.co>

Update x-pack/metricbeat/module/prometheus/collector/data.go

d4bb333

Co-Authored-By: Jaime Soriano Pastor <jaime.soriano@elastic.co>

Merge branch 'master' into prometheus-histograms

adeb3c8

Update x-pack/metricbeat/module/prometheus/collector/counter.go

2325fc7

Co-Authored-By: Jaime Soriano Pastor <jaime.soriano@elastic.co>

Update metricbeat/module/prometheus/collector/_meta/docs.asciidoc

906876f

Co-Authored-By: Jaime Soriano Pastor <jaime.soriano@elastic.co>

Reuse prometheus docker-compose

7c89bf3

Add testdata

fc5e461

exekias removed the in progress Pull request is currently in progress. label Mar 24, 2020

ChrsMark approved these changes Mar 24, 2020

View reviewed changes

Carlos Pérez-Aradros Herce added 2 commits March 24, 2020 16:53

Fix tests

f4470cf

Fix test

fff3faf

exekias force-pushed the prometheus-histograms branch from b204a49 to fff3faf Compare March 24, 2020 18:21

exekias merged commit 142b859 into elastic:master Mar 24, 2020

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

exekias mentioned this pull request Mar 24, 2020

Cherry-pick #17061 to 7.x: Use Elasticsearch histogram type to store Prometheus histograms #17227

Merged

5 tasks

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

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

andresrc added test-plan-added This PR has been added to the test plan and removed [zube]: Done labels Mar 31, 2020

MichaelKatsoulis mentioned this pull request Jun 6, 2022

Kubernetes Apiserver metricset collects deprecated metrics #31834

Closed

MichaelKatsoulis mentioned this pull request Jun 21, 2022

Use Elasticsearch histogram type to store Prometheus histograms #32006

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use Elasticsearch histogram type to store Prometheus histograms#17061

Use Elasticsearch histogram type to store Prometheus histograms#17061
exekias merged 34 commits intoelastic:masterfrom
exekias:prometheus-histograms

exekias commented Mar 17, 2020 •

edited

Loading

Uh oh!

elasticmachine commented Mar 17, 2020

Uh oh!

Uh oh!

exekias Mar 18, 2020

Uh oh!

Uh oh!

Uh oh!

jsoriano left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jsoriano Mar 24, 2020

Uh oh!

Uh oh!

jsoriano Mar 24, 2020

Uh oh!

Uh oh!

ChrsMark left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

	var lastUpper, prevUpper float64
	var lastUpper, prevLastUpper float64

Conversation

exekias commented Mar 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

A note on graph fidelity

Why is it important?

Checklist

How to test this PR locally

Related issues

Uh oh!

elasticmachine commented Mar 17, 2020

Uh oh!

Uh oh!

exekias Mar 18, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jsoriano left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jsoriano Mar 24, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jsoriano Mar 24, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ChrsMark left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

exekias commented Mar 17, 2020 •

edited

Loading