Ignore prometheus metrics when their values are NaN or Inf #12084

kaiyan-sheng · 2019-05-06T22:58:38Z

When prometheus report metrics with value NaN or +Inf or -Inf, metricbeat will fail with error like "Failed to serialize the event: unsupported float value: NaN". This PR is to add the logic to ignore prometheus metrics with NaN/Inf value in collector metricset.

This PR also added a metrics-with-naninf.plain test data with NaN/Inf as metric value. From the output metrics-with-naninf.plain-expected.json you can see, the metrics with NaN/Inf value are not there.

closes #10849

metricbeat/module/prometheus/collector/_meta/testdata/histogram.plain-expected.json

kaiyan-sheng · 2019-05-07T02:54:16Z

Changelog will be added later.

exekias

I'm guessing Prometheus helper is also affected: https://github.com/elastic/beats/blob/master/metricbeat/helper/prometheus/prometheus.go

metricbeat/module/prometheus/collector/collector.go

metricbeat/module/prometheus/collector/_meta/testdata/histogram.plain-expected.json

kaiyan-sheng · 2019-05-07T20:58:42Z

I'm guessing Prometheus helper is also affected: https://github.com/elastic/beats/blob/master/metricbeat/helper/prometheus/prometheus.go

@exekias Yes you are right, I just pushed a new commit with changes in the prometheus helper. Thanks!

jsoriano

Thanks @kaiyan-sheng for investigating this issue.

If I understand well these changes, they are only handling NaN/Inf values in histograms and summaries, but there most of the values are integers, where these values are in principle not possible. The prometheus library seems to be parsing them as floats and then converting them to integers, this is why you see them as the value of uint64(math.NaN()). This value, even if incorrect, would be a number that can be handled by libbeat outputs and ES.

The real problem is with floats, they can be NaN/Inf, and these values cannot be handled as numbers by libbeat outputs or ES, so events containing them are dropped. Gauges, Counters and Sums in Histograms and Summaries are floats, we should handle these values on these types of metrics.

metricbeat/helper/prometheus/prometheus_test.go

kaiyan-sheng · 2019-05-09T00:49:54Z

@jsoriano I don't know how I got myself into this histogram hole and ignored all the other real problem 🤣 Thanks for the explanation (and digging me out of the histogram hole )! I added the part for Gauge/Counter/Summary and removed the histogram change. Please let me know if this PR is going the right direction now... 😃

Reviewing again

jsoriano

Thanks, this is looking better now 🙂

We could still check that the values in the histograms are fine too, at least the sum. I see you do it in the module, could you do it too in the helper? And it'd be good to have test cases with all the types that can have this problem.

metricbeat/helper/prometheus/prometheus.go

metricbeat/helper/prometheus/prometheus_test.go

metricbeat/module/prometheus/collector/data.go

metricbeat/module/prometheus/collector/_meta/testdata/gauge-with-naninf.plain

jsoriano

I only wonder if we should also check for these values in the histogram sum just in case, for the rest it LGTM.

metricbeat/helper/prometheus/metric.go

metricbeat/module/prometheus/collector/data.go

…aN or Inf (#12084) (#12295) * Ignore prometheus metrics when their values are NaN or Inf (#12084) * Ignore prometheus metrics when their values are NaN or Inf * Avoid NaN/Inf in prometheus helper * Add checks on Gauge, Summary and Counter * Add NaN/Inf check on histogram values (cherry picked from commit 9244477) * Fix changelog

…aN or Inf (#12084) (#12286) * Ignore prometheus metrics when their values are NaN or Inf (#12084) * Ignore prometheus metrics when their values are NaN or Inf * Avoid NaN/Inf in prometheus helper * Add checks on Gauge, Summary and Counter * Add NaN/Inf check on histogram values (cherry picked from commit 9244477) * Fix changelog

…12296) * Ignore prometheus metrics when their values are NaN or Inf * Avoid NaN/Inf in prometheus helper * Add checks on Gauge, Summary and Counter * Add NaN/Inf check on histogram values (cherry picked from commit 9244477)

yxyx520 · 2019-07-17T09:27:33Z

with the version v7.2.0, I still get an error "2019-07-17T09:26:44.695Z ERROR elasticsearch/client.go:394 Failed to encode event: unsupported float value: NaN"

yxyx520 · 2019-07-17T09:31:16Z

@kaiyan-sheng any suggestions ?

charkadiusz · 2019-08-14T13:51:42Z

@kaiyan-sheng, same here - v 7.3.0 and am getting

ERROR	elasticsearch/client.go:402	Failed to encode event: unsupported float value: NaN

in the prometheus collector metricset when scraping /federate endpoint on prometheus in a kubernetes cluster.
This is quite problematic as I'm getting flooded with those error logs. Any advice?

…aN or Inf (elastic#12084) (elastic#12295) * Ignore prometheus metrics when their values are NaN or Inf (elastic#12084) * Ignore prometheus metrics when their values are NaN or Inf * Avoid NaN/Inf in prometheus helper * Add checks on Gauge, Summary and Counter * Add NaN/Inf check on histogram values (cherry picked from commit 6810e31) * Fix changelog

…2084) (elastic#12296) * Ignore prometheus metrics when their values are NaN or Inf * Avoid NaN/Inf in prometheus helper * Add checks on Gauge, Summary and Counter * Add NaN/Inf check on histogram values (cherry picked from commit 6810e31)

…aN or Inf (elastic#12084) (elastic#12286) * Ignore prometheus metrics when their values are NaN or Inf (elastic#12084) * Ignore prometheus metrics when their values are NaN or Inf * Avoid NaN/Inf in prometheus helper * Add checks on Gauge, Summary and Counter * Add NaN/Inf check on histogram values (cherry picked from commit 6810e31) * Fix changelog

kaiyan-sheng requested a review from a team as a code owner May 6, 2019 22:58

kaiyan-sheng commented May 6, 2019

View reviewed changes

metricbeat/module/prometheus/collector/_meta/testdata/histogram.plain-expected.json Outdated Show resolved Hide resolved

kaiyan-sheng self-assigned this May 6, 2019

kaiyan-sheng added the Team:Integrations Label for the Integrations team label May 6, 2019

kaiyan-sheng added needs_backport PR is waiting to be backported to other branches. review labels May 7, 2019

exekias reviewed May 7, 2019

View reviewed changes

metricbeat/module/prometheus/collector/collector.go Outdated Show resolved Hide resolved

metricbeat/module/prometheus/collector/_meta/testdata/histogram.plain-expected.json Outdated Show resolved Hide resolved

kaiyan-sheng requested a review from a team as a code owner May 7, 2019 20:56

jsoriano previously requested changes May 8, 2019

View reviewed changes

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

kaiyan-sheng added in progress Pull request is currently in progress. and removed review labels May 8, 2019

jsoriano reviewed May 9, 2019

View reviewed changes

kaiyan-sheng added [zube]: In Review and removed in progress Pull request is currently in progress. labels May 10, 2019

kaiyan-sheng mentioned this pull request May 16, 2019

Handling non-finite float values #9212

Closed

jsoriano reviewed May 21, 2019

View reviewed changes

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

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

jsoriano approved these changes May 24, 2019

View reviewed changes

metricbeat/module/prometheus/collector/data.go Outdated Show resolved Hide resolved

kaiyan-sheng added 9 commits May 24, 2019 11:28

Ignore prometheus metrics when their values are NaN or Inf

b3c6363

Avoid NaN/Inf in prometheus helper

25a0788

Add changelog

11aaaa7

Remove duplicated lines in prometheus_test.go

790e9e2

run make fmt

3569ed3

Add checks on Gauge, Summary and Counter

f7c9910

Remove NaN checks for histogram

0ef9650

Remove library thats imported but not used

5516c4e

Run make fmt

f42e7ba

kaiyan-sheng added v7.1.0 v7.2.0 labels May 24, 2019

kaiyan-sheng merged commit 9244477 into elastic:master May 24, 2019

kaiyan-sheng deleted the nan_prometheus branch May 24, 2019 19:54

zube bot added [zube]: Done and removed [zube]: In Review labels May 24, 2019

kaiyan-sheng removed the needs_backport PR is waiting to be backported to other branches. label May 28, 2019

andresrc removed the [zube]: Done label May 28, 2019

jsoriano added v7.1.2 v7.0.2 and removed v7.0.0 v7.1.0 labels May 29, 2019

jsoriano mentioned this pull request Sep 20, 2019

Prometheus untyped metrics with NaN value generate incorrect events #13750

Closed

michalpristas mentioned this pull request Jan 17, 2020

Events with encoding errors produce misbehaving retries with some outputs #11732

Open

jsoriano mentioned this pull request Nov 16, 2020

Handle NaNs in Cloud Foundry metrics #22599

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ignore prometheus metrics when their values are NaN or Inf #12084

Ignore prometheus metrics when their values are NaN or Inf #12084

kaiyan-sheng commented May 6, 2019 •

edited

Loading

kaiyan-sheng commented May 7, 2019

exekias left a comment

kaiyan-sheng commented May 7, 2019

jsoriano left a comment

kaiyan-sheng commented May 9, 2019

jsoriano left a comment

jsoriano left a comment

yxyx520 commented Jul 17, 2019

yxyx520 commented Jul 17, 2019

charkadiusz commented Aug 14, 2019

Ignore prometheus metrics when their values are NaN or Inf #12084

Ignore prometheus metrics when their values are NaN or Inf #12084

Conversation

kaiyan-sheng commented May 6, 2019 • edited Loading

kaiyan-sheng commented May 7, 2019

exekias left a comment

Choose a reason for hiding this comment

kaiyan-sheng commented May 7, 2019

jsoriano left a comment

Choose a reason for hiding this comment

kaiyan-sheng commented May 9, 2019

jsoriano left a comment

Choose a reason for hiding this comment

jsoriano left a comment

Choose a reason for hiding this comment

yxyx520 commented Jul 17, 2019

yxyx520 commented Jul 17, 2019

charkadiusz commented Aug 14, 2019

kaiyan-sheng commented May 6, 2019 •

edited

Loading