Reject infinite performance data values #10077
Conversation
lib/base/perfdatavalue.cpp
Outdated
| @@ -259,6 +259,10 @@ PerfdataValue::Ptr PerfdataValue::Parse(const String& perfdata) | |||
|
|
|||
| double value = Convert::ToDouble(tokens[0].SubStr(0, pos)); | |||
|
|
|||
| if (value == INFINITY || value == - INFINITY) { | |||
There was a problem hiding this comment.
There are special functions for such checks like std::isinf(). A related function like std::isfinite() or std::isnormal() might be even better suited here as it excludes more edge cases (I haven't checked if subnormal numbers would be a problem, that seems to be the only difference between these two functions).
There was a problem hiding this comment.
Please excuse my C. Those functions do seem more appropriate and sane to use.
I will change that accordingly. Thank you for the tip
RincewindsHat
force-pushed
the
reject_invalid_perfdata
branch
from
310efe0 to
b8c6899
Compare
|
@julianbrost should be fine now |
|
@julianbrost @oxzi ping |
Some fault monitoring plugins may return "inf" or "-inf" as values due to a failure to initialize or other errors. This patch introduces a check on whether the parse value is infinite (or negative infinite) and rejects the data point if that is the case. The reasoning here is: There is no possible way a value of "inf" is ever a true measuring or even useful. Furthermore, when passed to the performance data writers, it may be rejected by the backend and lead to further complications.
oxzi
force-pushed
the
reject_invalid_perfdata
branch
from
b8c6899 to
e738119
Compare
There was a problem hiding this comment.
First things first: Thanks a lot for this Pull Request and please excuse the delay on our side.
I have taken a closer look and tested it in a variety of setups. After rebasing it on the current master, I have added a test case for inf values which would succeed on the master branch, but raises an exception with your changes. (That's good.)
Then I built myself a test setup with an InfluxDB 2 registered as an Influxdb2Writer at the Icinga 2 and a CheckCommand sometimes dropping inf performance data.
CheckCommand definition
object CheckCommand "random fortune" {
import "plugin-check-command"
command = [ "/bin/bash", "-c", "/usr/games/fortune; echo \"|foo=`echo -e \"inf\n23\n42\" | shuf | head -n1`\"; exit $$0" ]
arguments += {
"(no key)" = {
skip_key = true
value = {{
var rand = Math.random() * 1000
if (host.last_check == -1 || host.state == 0) {
if (rand > 995) {
return 2
} else {
return 0
}
} else {
if (rand > 900) {
return 0
} else {
return 2
}
}
}}
}
}
}
With an Icinga 2 built from the master branch, this would always result in invalid data sent to the InfluxDB 2 as the valid data points are sent bundled together with invalid ones. Effectively no data will be stored in the InfluxDB 2 as the whole POST request will be discarded.
However, when building Icinga 2 based on this branch, the invalid data points are being removed and only the valid ones are sent. Meanwhile, Icinga 2 logs something like the following:
[2025-01-09 10:55:20 +0000] warning/Influxdb2Writer: Ignoring invalid perfdata for checkable 'dummy
-929!random fortune' and command 'random fortune' with value: foo=inf
Context:
(0) Processing check result for 'dummy-929!random fortune'
Some fault monitoring plugins may return "inf" or "-inf" as values due to a failure to initialize or other errors.
This patch introduces a check on whether the parse value is infinite (or negative infinite) and rejects the data point if that is the case.
The reasoning here is: There is no possible way a value of "inf" is ever a true measuring or even useful. Furthermore, when passed to the performance data writers, it may be rejected by the backend and lead to further complications.
fixes #10073