Infer types in Prometheus remote_write by ChrsMark · Pull Request #19944 · elastic/beats

ChrsMark · 2020-07-15T13:59:39Z

This PR add types for Prometheus metrics that are collected via remote_write metricset.
These metrics are in raw format when received so a heuristic approach is applied to extract types from metrics' name patterns.

Closes #17675.

TODOs

Implementation
Tests
Update docs

How to test this PR

Env setup

Use https://github.com/ChrsMark/docker-prometheus-playground for a ready to use prometheus env.
For remote write this config is needed: https://github.com/ChrsMark/docker-prometheus-playground/blob/master/prometheus/prometheus.yml#L10
If using a darwin machine the following should be enough:

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

On linux machines in order to access host from within the Prometheus container one can set network_mode: host in the docker-compose.yml and then host can be reach using 0.0.0.0.

Testing

Use oss version and test collector and remote_write (without types) metricsets for regressions.
Use basic version and test collector (with types) for regressions.
Use basic version and test that remote_write metricset:
a. uses types as expected.
b. can identify types based on input patterns: https://github.com/elastic/beats/pull/19944/files?file-filters%5B%5D=.asciidoc#diff-b69e84fad87eb2ae563115f43675a547R118

Signed-off-by: chrismark <chrismarkou92@gmail.com>

elasticmachine · 2020-07-17T14:22:09Z

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

ChrsMark · 2020-07-17T14:24:47Z

This one is ready for a first round of review. Implementation is complete along with unit tests to cover all the metrics processing chain from the heuristic approach of extracting types to grouping metrics with same labesets together.

Signed-off-by: chrismark <chrismarkou92@gmail.com>

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

Signed-off-by: chrismark <chrismarkou92@gmail.com>

…e_write

Signed-off-by: chrismark <chrismarkou92@gmail.com>

ChrsMark · 2020-07-22T12:20:31Z

jenkins run the tests please

…e_write

Signed-off-by: chrismark <chrismarkou92@gmail.com>

ChrsMark · 2020-07-27T12:18:16Z

@exekias I did some cleanups and it looks like this is ready for a final review so feel free to hit it once you get the chance.

exekias

I like how things are looking, I left a few minor comments!

metricbeat/helper/prometheus/prometheus.go

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

Signed-off-by: chrismark <chrismarkou92@gmail.com>

exekias

LGTM although lint errors are legit, please review

Signed-off-by: chrismark <chrismarkou92@gmail.com>

jsoriano

Nice feature and well documented 👍 I have added some small suggestions, nothing serious.

metricbeat/helper/prometheus/prometheus.go

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

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

jsoriano · 2020-07-28T16:48:10Z

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

+. `_count` suffix: the metric is of Counter type
+. `_bucket` suffix and `le` in labels: the metric is of Histogram type
+
+Everything else is handled as a Gauge.


Just curious, is there a reason to default to gauge and not to counter?

Hmm, we check if a metric is a counter or a histogram and if not then we conclude that is a gauge. This is the "heuristic". If we fall-back to counters then we will not have a way to identify gauges.

Yeah, I meant if there was a reason to add a heuristic to detect counters and handle everything else as gauges and not the other way around. Maybe it is more difficult to find a heuristic to detect gauges 🤔

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

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

Signed-off-by: chrismark <chrismarkou92@gmail.com>

jsoriano

Thanks for addressing the comments!

jsoriano · 2020-07-29T09:15:50Z

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

+. `_count` suffix: the metric is of Counter type
+. `_bucket` suffix and `le` in labels: the metric is of Histogram type
+
+Everything else is handled as a Gauge.


Yeah, I meant if there was a reason to add a heuristic to detect counters and handle everything else as gauges and not the other way around. Maybe it is more difficult to find a heuristic to detect gauges 🤔

jsoriano · 2020-07-29T09:19:18Z

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

+const (
+	counterType   = "counter_type"
+	histogramType = "histgram_type"
+	otherType     = "other_type"


Oh ok, it is not only about gauges 👍 Should we mention in some place that summaries are handled as gauges?

Signed-off-by: chrismark <chrismarkou92@gmail.com>

ChrsMark · 2020-07-29T10:41:41Z

Thanks for the review @jsoriano!

Regarding #19944 (comment), it is easier to identify counters and histograms, gauge does not follow a specific pattern. This approach was also defined at #19944 (comment).

I also added a note in the docs about Summary handling 🍻 .

(cherry picked from commit b797a7e)

…ne-2.0 * upstream/master: (29 commits) Add an explicit system test for processes on unix systems (elastic#20320) [Autodiscovery] Ignore ErrInputNotFinished errors in autodiscover config checks (elastic#20305) [CI] Update README.md with CI references (elastic#20316) Add ECK doc links to Heartbeat docs (elastic#20284) [Filebeat] Add export tests to x-pack/filebeat (elastic#20156) feat(ci): support building docker images for PRs (elastic#20323) Update system tests dependency (elastic#20287) [Libbeat] Log debug message if the Kibana dashboard can not be imported from the archive (elastic#12211) (elastic#20150) [Filebeat][Gsuite] Transform all dates to timestamp with processor (elastic#20308) Infer types in Prometheus remote_write (elastic#19944) Remove unnecessary restarts of metricsets while using Node autodiscover (elastic#19974) docs: update changelog on master branch (elastic#20259) feat(ci): support storing artifacts for PRs in separate dirs (elastic#20282) [CI] Change upstream reference (elastic#20296) [Filebeat] Updates to Suricata module (elastic#20220) [docs] Fix Windows download link for agent (elastic#20258) [docs] Rename release highlights to what's new (elastic#20255) fix: update the display name of the multibranch job (elastic#20265) [Elastic Agent] Add basic protocol to control Elastic Agent. (elastic#20146) Cisco ASA: Fix message 106100 (elastic#20245) ...

botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Jul 15, 2020

Infer types in remote_write

c0c6a0f

Signed-off-by: chrismark <chrismarkou92@gmail.com>

ChrsMark force-pushed the infer_types_remote_write branch from 1d4c919 to c0c6a0f Compare July 15, 2020 14:00

ChrsMark added the Team:Platforms Label for the Integrations - Platforms team label Jul 15, 2020

botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Jul 15, 2020

fmt update

456f909

Signed-off-by: chrismark <chrismarkou92@gmail.com>

andresrc added [zube]: Inbox [zube]: In Progress and removed [zube]: Inbox labels Jul 16, 2020

ChrsMark added 7 commits July 16, 2020 11:05

fmt update x-pack

ccd1857

Signed-off-by: chrismark <chrismarkou92@gmail.com>

fixes

3a8cf2e

Signed-off-by: chrismark <chrismarkou92@gmail.com>

Add unit tests for counter, gauge, summary metric types

063c269

Signed-off-by: chrismark <chrismarkou92@gmail.com>

Add histogram unit tests

273cc87

Signed-off-by: chrismark <chrismarkou92@gmail.com>

fmt update

7d2cd4d

Signed-off-by: chrismark <chrismarkou92@gmail.com>

Remove print

69803fd

Signed-off-by: chrismark <chrismarkou92@gmail.com>

Add docstrings

28c9512

Signed-off-by: chrismark <chrismarkou92@gmail.com>

ChrsMark requested review from exekias and jsoriano July 17, 2020 14:21

ChrsMark self-assigned this Jul 17, 2020

ChrsMark added [zube]: (7.10) Planned for release v7.10.0 and removed [zube]: (7.10) Planned for release labels Jul 17, 2020

ChrsMark marked this pull request as ready for review July 17, 2020 14:22

ChrsMark added the review label Jul 17, 2020

ChrsMark added 2 commits July 17, 2020 17:26

Remove blank line deletion

76e62c7

Signed-off-by: chrismark <chrismarkou92@gmail.com>

fmt x-pack code

f9895f7

Signed-off-by: chrismark <chrismarkou92@gmail.com>

exekias reviewed Jul 20, 2020

View reviewed changes

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

Fix leftover

6ce5987

Signed-off-by: chrismark <chrismarkou92@gmail.com>

ChrsMark added test-plan Add this PR to be manual test plan [zube]: In Review and removed [zube]: In Progress labels Jul 21, 2020

ChrsMark added 2 commits July 22, 2020 12:48

Merge remote-tracking branch 'upstream/master' into infer_types_remot…

3e4bd7a

…e_write

make update

6c340fd

Signed-off-by: chrismark <chrismarkou92@gmail.com>

ChrsMark added 2 commits July 27, 2020 10:32

Merge remote-tracking branch 'upstream/master' into infer_types_remot…

04e73db

…e_write

clean ups

eace032

Signed-off-by: chrismark <chrismarkou92@gmail.com>

exekias reviewed Jul 28, 2020

View reviewed changes

metricbeat/helper/prometheus/prometheus.go Show resolved Hide resolved

metricbeat/helper/prometheus/prometheus.go Show resolved Hide resolved

x-pack/metricbeat/module/prometheus/remote_write/data.go Outdated Show resolved Hide resolved

review changes

99705c0

Signed-off-by: chrismark <chrismarkou92@gmail.com>

exekias approved these changes Jul 28, 2020

View reviewed changes

fmt

e3fc125

Signed-off-by: chrismark <chrismarkou92@gmail.com>

jsoriano reviewed Jul 28, 2020

View reviewed changes

review changes

bdc4431

Signed-off-by: chrismark <chrismarkou92@gmail.com>

jsoriano approved these changes Jul 29, 2020

View reviewed changes

ChrsMark added 2 commits July 29, 2020 13:34

Add note about summary in the docs

b538355

Signed-off-by: chrismark <chrismarkou92@gmail.com>

fix

c3712e4

Signed-off-by: chrismark <chrismarkou92@gmail.com>

ChrsMark merged commit b797a7e into elastic:master Jul 29, 2020

zube bot added [zube]: Done and removed [zube]: In Review labels Jul 29, 2020

ChrsMark added a commit to ChrsMark/beats that referenced this pull request Jul 29, 2020

Infer types in Prometheus remote_write (elastic#19944)

823346b

(cherry picked from commit b797a7e)

ChrsMark mentioned this pull request Jul 29, 2020

Cherry-pick #19944 to 7.x: Infer types in Prometheus remote_write #20314

Merged

3 tasks

ChrsMark added a commit that referenced this pull request Jul 29, 2020

Infer types in Prometheus remote_write (#19944) (#20314)

f7c59d5

(cherry picked from commit b797a7e)

ChrsMark mentioned this pull request Aug 11, 2020

Update Prometheus package to leverage types in remote write elastic/integrations#243

Merged

2 tasks

andresrc added the test-plan-added This PR has been added to the test plan label Oct 3, 2020

melchiormoulin pushed a commit to melchiormoulin/beats that referenced this pull request Oct 14, 2020

Infer types in Prometheus remote_write (elastic#19944)

ef23981

zube bot removed the [zube]: Done label Oct 28, 2020

Conversation

ChrsMark commented Jul 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TODOs

How to test this PR

Env setup

Testing

Uh oh!

elasticmachine commented Jul 17, 2020

Uh oh!

ChrsMark commented Jul 17, 2020

Uh oh!

Uh oh!

ChrsMark commented Jul 22, 2020

Uh oh!

ChrsMark commented Jul 27, 2020

Uh oh!

exekias left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

exekias left a comment

Choose a reason for hiding this comment

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!

jsoriano Jul 28, 2020

Choose a reason for hiding this comment

Uh oh!

ChrsMark Jul 29, 2020

Choose a reason for hiding this comment

Uh oh!

jsoriano Jul 29, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jsoriano left a comment

Choose a reason for hiding this comment

Uh oh!

jsoriano Jul 29, 2020

Choose a reason for hiding this comment

Uh oh!

jsoriano Jul 29, 2020

Choose a reason for hiding this comment

Uh oh!

ChrsMark commented Jul 29, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ChrsMark commented Jul 15, 2020 •

edited

Loading