Do not evaluate invalid performance data (#849)

fix #838
This commit is contained in:
Johannes Meyer 2023-09-05 16:56:28 +02:00 committed by GitHub
commit c626bc147d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 354 additions and 79 deletions

View file

@ -39,21 +39,21 @@ class PerfData
/**
* The value
*
* @var float
* @var ?float
*/
protected $value;
/**
* The minimum value
*
* @var float
* @var ?float
*/
protected $minValue;
/**
* The maximum value
*
* @var float
* @var ?float
*/
protected $maxValue;
@ -71,6 +71,27 @@ class PerfData
*/
protected $criticalThreshold;
/**
* The raw value
*
* @var ?string
*/
protected $rawValue;
/**
* The raw minimum value
*
* @var ?string
*/
protected $rawMinValue;
/**
* The raw maximum value
*
* @var ?string
*/
protected $rawMaxValue;
/**
* Create a new PerfData object based on the given performance data label and value
*
@ -312,7 +333,7 @@ class PerfData
*/
public function isVisualizable(): bool
{
return isset($this->minValue) && isset($this->maxValue) && isset($this->value);
return isset($this->minValue, $this->maxValue, $this->value) && $this->isValid();
}
/**
@ -428,25 +449,39 @@ class PerfData
$matches = array();
if (preg_match('@^(U|-?(?:\d+)?(?:\.\d+)?)([a-zA-TV-Z%°]{1,3})$@u', $parts[0], $matches)) {
$this->unit = $matches[2];
$this->value = $matches[1];
$value = $matches[1];
} else {
$this->value = $parts[0];
$value = $parts[0];
}
if (! is_numeric($this->value)) {
if (! is_numeric($value)) {
if ($value !== 'U') {
$this->rawValue = $parts[0];
}
$this->value = null;
} else {
$this->value = floatval($value);
}
switch (count($parts)) {
/* @noinspection PhpMissingBreakStatementInspection */
case 5:
if ($parts[4] !== '') {
$this->maxValue = $parts[4];
if (is_numeric($parts[4])) {
$this->maxValue = floatval($parts[4]);
} else {
$this->rawMaxValue = $parts[4];
}
}
/* @noinspection PhpMissingBreakStatementInspection */
case 4:
if ($parts[3] !== '') {
$this->minValue = $parts[3];
if (is_numeric($parts[3])) {
$this->minValue = floatval($parts[3]);
} else {
$this->rawMinValue = $parts[3];
}
}
/* @noinspection PhpMissingBreakStatementInspection */
case 3:
@ -509,6 +544,10 @@ class PerfData
}
if ($value instanceof ThresholdRange) {
if (! $value->isValid()) {
return (string) $value;
}
if ($value->getMin()) {
return (string) $value;
}
@ -578,18 +617,22 @@ class PerfData
public function toArray(): array
{
return array(
return [
'label' => $this->getLabel(),
'value' => $this->format($this->getValue()),
'min' => isset($this->minValue) && !$this->isPercentage()
? $this->format($this->minValue)
: '',
'max' => isset($this->maxValue) && !$this->isPercentage()
? $this->format($this->maxValue)
: '',
'warn' => $this->format($this->warningThreshold),
'crit' => $this->format($this->criticalThreshold)
);
'value' => isset($this->value) ? $this->format($this->value) : $this->rawValue,
'min' => (string) (
! $this->isPercentage()
? (isset($this->minValue) ? $this->format($this->minValue) : $this->rawMinValue)
: null
),
'max' => (string) (
! $this->isPercentage()
? (isset($this->maxValue) ? $this->format($this->maxValue) : $this->rawMaxValue)
: null
),
'warn' => $this->format($this->warningThreshold),
'crit' => $this->format($this->criticalThreshold)
];
}
/**
@ -643,4 +686,18 @@ class PerfData
return false;
}
/**
* Returns whether the performance data can be evaluated
*
* @return bool
*/
public function isValid(): bool
{
return ! isset($this->rawValue)
&& ! isset($this->rawMinValue)
&& ! isset($this->rawMaxValue)
&& $this->criticalThreshold->isValid()
&& $this->warningThreshold->isValid();
}
}

View file

@ -131,10 +131,9 @@ class PerfDataFormat
return sprintf('%0.2f d', $value / 86400);
}
protected static function formatForUnits($value, array &$units, int $base): string
protected static function formatForUnits(float $value, array &$units, int $base): string
{
$sign = '';
$value = (float) $value;
if ($value < 0) {
$value = abs($value);
$sign = '-';

View file

@ -37,6 +37,13 @@ class ThresholdRange
*/
protected $raw;
/**
* Whether the threshold range is valid
*
* @var bool
*/
protected $isValid = true;
/**
* Create a new instance based on a threshold range conforming to <https://nagios-plugins.org/doc/guidelines.html>
*
@ -61,6 +68,12 @@ class ThresholdRange
if (strpos($rawRange, ':') === false) {
$min = 0.0;
$max = trim($rawRange);
if (! is_numeric($max)) {
$range->isValid = false;
return $range;
}
$max = floatval(trim($rawRange));
} else {
list($min, $max) = explode(':', $rawRange, 2);
@ -75,9 +88,19 @@ class ThresholdRange
$min = null;
break;
default:
if (! is_numeric($min)) {
$range->isValid = false;
return $range;
}
$min = floatval($min);
}
if (! empty($max) && ! is_numeric($max)) {
$range->isValid = false;
return $range;
}
$max = empty($max) ? null : floatval($max);
}
@ -168,6 +191,16 @@ class ThresholdRange
));
}
/**
* Return whether the threshold range is valid
*
* @return bool
*/
public function isValid()
{
return $this->isValid;
}
/**
* Return the textual representation of $this, suitable for fromString()
*

View file

@ -12,9 +12,13 @@ use ipl\Html\HtmlElement;
use ipl\Html\HtmlString;
use ipl\Html\Table;
use ipl\Html\Text;
use ipl\I18n\Translation;
use ipl\Web\Widget\Icon;
class PerfDataTable extends Table
{
use Translation;
protected $defaultAttributes = [
'class' => 'performance-data-table collapsible',
'data-visible-rows' => 6
@ -58,7 +62,7 @@ class PerfDataTable extends Table
$containsSparkline = false;
foreach ($pieChartData as $perfdata) {
if ($perfdata->isVisualizable()) {
if ($perfdata->isVisualizable() || ! $perfdata->isValid()) {
$containsSparkline = true;
break;
}
@ -91,6 +95,19 @@ class PerfDataTable extends Table
HtmlString::create($perfdata->asInlinePie($this->color)->render()),
['class' => 'sparkline-col']
);
} elseif (! $perfdata->isValid()) {
$cols[] = Table::td(
new Icon(
'triangle-exclamation',
[
'title' => $this->translate(
'Evaluation failed. Performance data is invalid.'
),
'class' => ['invalid-perfdata']
]
),
['class' => 'sparkline-col']
);
} else {
$cols[] = Table::td('');
}

View file

@ -5840,46 +5840,6 @@ parameters:
count: 2
path: library/Icingadb/Util/PerfData.php
-
message: "#^Property Icinga\\\\Module\\\\Icingadb\\\\Util\\\\PerfData\\:\\:\\$maxValue \\(float\\) does not accept string\\.$#"
count: 1
path: library/Icingadb/Util/PerfData.php
-
message: "#^Property Icinga\\\\Module\\\\Icingadb\\\\Util\\\\PerfData\\:\\:\\$maxValue \\(float\\) in isset\\(\\) is not nullable\\.$#"
count: 2
path: library/Icingadb/Util/PerfData.php
-
message: "#^Property Icinga\\\\Module\\\\Icingadb\\\\Util\\\\PerfData\\:\\:\\$minValue \\(float\\) does not accept string\\.$#"
count: 1
path: library/Icingadb/Util/PerfData.php
-
message: "#^Property Icinga\\\\Module\\\\Icingadb\\\\Util\\\\PerfData\\:\\:\\$minValue \\(float\\) in isset\\(\\) is not nullable\\.$#"
count: 2
path: library/Icingadb/Util/PerfData.php
-
message: "#^Property Icinga\\\\Module\\\\Icingadb\\\\Util\\\\PerfData\\:\\:\\$value \\(float\\) does not accept null\\.$#"
count: 1
path: library/Icingadb/Util/PerfData.php
-
message: "#^Property Icinga\\\\Module\\\\Icingadb\\\\Util\\\\PerfData\\:\\:\\$value \\(float\\) does not accept string\\.$#"
count: 2
path: library/Icingadb/Util/PerfData.php
-
message: "#^Property Icinga\\\\Module\\\\Icingadb\\\\Util\\\\PerfData\\:\\:\\$value \\(float\\) in isset\\(\\) is not nullable\\.$#"
count: 1
path: library/Icingadb/Util/PerfData.php
-
message: "#^Result of && is always true\\.$#"
count: 2
path: library/Icingadb/Util/PerfData.php
-
message: "#^Method Icinga\\\\Module\\\\Icingadb\\\\Util\\\\PerfDataFormat\\:\\:ampereSeconds\\(\\) has parameter \\$value with no type specified\\.$#"
count: 1
@ -5905,11 +5865,6 @@ parameters:
count: 1
path: library/Icingadb/Util/PerfDataFormat.php
-
message: "#^Method Icinga\\\\Module\\\\Icingadb\\\\Util\\\\PerfDataFormat\\:\\:formatForUnits\\(\\) has parameter \\$value with no type specified\\.$#"
count: 1
path: library/Icingadb/Util/PerfDataFormat.php
-
message: "#^Method Icinga\\\\Module\\\\Icingadb\\\\Util\\\\PerfDataFormat\\:\\:grams\\(\\) has parameter \\$value with no type specified\\.$#"
count: 1

View file

@ -43,6 +43,11 @@
min-width: 1.75em;
width: 1.75em;
padding: 2/12em 0;
.invalid-perfdata {
font-size: 1.25em;
vertical-align: text-bottom;
color: @color-warning;
}
}
.inline-pie > svg {

View file

@ -93,7 +93,7 @@ class PerfdataSetTest extends TestCase
'PerfdataSet does not correctly parse invalid quoted labels'
);
$this->assertSame(
'2',
2.0,
$pset->perfdata[1]->getValue()
);
}

View file

@ -32,7 +32,7 @@ class PerfdataTest extends TestCase
'Perfdata::fromString does not properly parse performance data labels'
);
$this->assertSame(
'1234',
1234.0,
$p->getValue(),
'Perfdata::fromString does not properly parse performance data values'
);
@ -44,12 +44,12 @@ class PerfdataTest extends TestCase
public function testWhetherGetValueReturnsValidValues()
{
$this->assertSame(
'1337',
1337.0,
Perfdata::fromString('test=1337')->getValue(),
'Perfdata::getValue does not return correct values'
);
$this->assertSame(
'1337',
1337.0,
Perfdata::fromString('test=1337;;;;')->getValue(),
'Perfdata::getValue does not return correct values'
);
@ -61,12 +61,12 @@ class PerfdataTest extends TestCase
public function testWhetherDecimalValuesAreCorrectlyParsed()
{
$this->assertSame(
'1337.5',
1337.5,
Perfdata::fromString('test=1337.5')->getValue(),
'Perfdata objects do not parse decimal values correctly'
);
$this->assertSame(
'1337.5',
1337.5,
Perfdata::fromString('test=1337.5B')->getValue(),
'Perfdata objects do not parse decimal values correctly'
);
@ -250,12 +250,12 @@ class PerfdataTest extends TestCase
public function testWhetherGetMinimumValueReturnsCorrectValues()
{
$this->assertSame(
'1337',
1337.0,
Perfdata::fromString('test=1;;;1337')->getMinimumValue(),
'Perfdata::getMinimumValue does not return correct values'
);
$this->assertSame(
'1337.5',
1337.5,
Perfdata::fromString('test=1;;;1337.5')->getMinimumValue(),
'Perfdata::getMinimumValue does not return correct values'
);
@ -267,12 +267,12 @@ class PerfdataTest extends TestCase
public function testWhetherGetMaximumValueReturnsCorrectValues()
{
$this->assertSame(
'1337',
1337.0,
Perfdata::fromString('test=1;;;;1337')->getMaximumValue(),
'Perfdata::getMaximumValue does not return correct values'
);
$this->assertSame(
'1337.5',
1337.5,
Perfdata::fromString('test=1;;;;1337.5')->getMaximumValue(),
'Perfdata::getMaximumValue does not return correct values'
);
@ -373,7 +373,7 @@ class PerfdataTest extends TestCase
public function testWhetherPercentagesAreHandledCorrectly()
{
$this->assertSame(
'66',
66.0,
Perfdata::fromString('test=66%')->getPercentage(),
'Perfdata objects do not correctly handle native percentages'
);
@ -399,4 +399,193 @@ class PerfdataTest extends TestCase
'Perfdata objects do not ignore impossible min/max combinations when returning percentages'
);
}
public function testWhetherInvalidValueInPerfDataHandledCorrectly()
{
$p1 = Perfdata::fromString('test=2,0');
$this->assertFalse($p1->isValid());
$this->assertNull(
$p1->getValue(),
'Perfdata::getValue does not return null for invalid values'
);
$this->assertSame(
'2,0',
$p1->toArray()['value']
);
$p2 = Perfdata::fromString('test=i am not a value');
$this->assertFalse($p2->isValid());
$this->assertNull(
$p2->getValue(),
'Perfdata::getValue does not return null for invalid values'
);
$this->assertSame(
'i am not a value',
$p2->toArray()['value']
);
$p3 = Perfdata::fromString('test=');
$this->assertFalse($p3->isValid());
$this->assertNull(
$p3->getValue(),
'Perfdata::getValue does not return null for invalid values'
);
$this->assertSame(
'',
$p3->toArray()['value']
);
$p4 = Perfdata::fromString('test=-kW');
$this->assertFalse($p4->isValid());
$this->assertNull(
$p4->getValue(),
'Perfdata::getValue does not return null for invalid values'
);
$this->assertSame(
'-kW',
$p4->toArray()['value']
);
$p5 = Perfdata::fromString('test=kW');
$this->assertFalse($p5->isValid());
$this->assertNull(
$p5->getValue(),
'Perfdata::getValue does not return null for invalid values'
);
$this->assertSame(
'kW',
$p5->toArray()['value']
);
$p6 = Perfdata::fromString('test=-');
$this->assertFalse($p6->isValid());
$this->assertNull(
$p6->getValue(),
'Perfdata::getValue does not return null for invalid values'
);
$this->assertSame(
'-',
$p6->toArray()['value']
);
}
public function testWhetherInvalidMinInPerfDataHandledCorrectly()
{
$p1 = Perfdata::fromString('test=1;;;2,0');
$this->assertFalse($p1->isValid());
$this->assertNull(
$p1->getMinimumValue(),
'Perfdata::getMinimumValue does not return null for invalid min values'
);
$this->assertSame(
'2,0',
$p1->toArray()['min']
);
$p2 = Perfdata::fromString('test=1;;;foo');
$this->assertFalse($p2->isValid());
$this->assertNull(
$p2->getMinimumValue(),
'Perfdata::getMinimumValue does not return null for invalid min values'
);
$this->assertSame(
'foo',
$p2->toArray()['min']
);
}
public function testWhetherInvalidMaxInPerfDataHandledCorrectly()
{
$p1 = Perfdata::fromString('test=1;;;;2,0');
$this->assertFalse($p1->isValid());
$this->assertNull(
$p1->getMaximumValue(),
'Perfdata::getMaximumValue does not return null for invalid max values'
);
$this->assertSame(
'2,0',
$p1->toArray()['max']
);
$p2 = Perfdata::fromString('test=1;;;;foo');
$this->assertFalse($p2->isValid());
$this->assertNull(
$p2->getMaximumValue(),
'Perfdata::getMaximumValue does not return null for invalid max values'
);
$this->assertSame(
'foo',
$p2->toArray()['max']
);
}
public function testWhetherInvalidWarningThresholdInPerfDataHandledCorrectly()
{
$p1 = Perfdata::fromString('test=1;2,0:');
$this->assertFalse($p1->getWarningThreshold()->isValid());
$this->assertFalse($p1->isValid());
$this->assertSame(
'2,0:',
(string) $p1->getWarningThreshold()
);
$p2 = Perfdata::fromString('test=1;0:4,0');
$this->assertFalse($p2->getWarningThreshold()->isValid());
$this->assertFalse($p2->isValid());
$this->assertSame(
'0:4,0',
(string) $p2->getWarningThreshold()
);
$p3 = Perfdata::fromString('test=1;foo');
$this->assertFalse($p2->getWarningThreshold()->isValid());
$this->assertFalse($p3->isValid());
$this->assertSame(
'foo',
(string) $p3->getWarningThreshold()
);
$p4 = Perfdata::fromString('test=1;10@');
$this->assertFalse($p2->getWarningThreshold()->isValid());
$this->assertFalse($p4->isValid());
$this->assertSame(
'10@',
(string) $p4->getWarningThreshold()
);
}
public function testWhetherInvalidCriticalThresholdInPerfDataHandledCorrectly()
{
$p1 = Perfdata::fromString('test=1;;2,0:');
$this->assertFalse($p1->getCriticalThreshold()->isValid());
$this->assertFalse($p1->isValid());
$this->assertSame(
'2,0:',
(string) $p1->getCriticalThreshold()
);
$p2 = Perfdata::fromString('test=1;;0:4,0');
$this->assertFalse($p2->getCriticalThreshold()->isValid());
$this->assertFalse($p2->isValid());
$this->assertSame(
'0:4,0',
(string) $p2->getCriticalThreshold()
);
$p3 = Perfdata::fromString('test=1;;foo');
$this->assertFalse($p2->getCriticalThreshold()->isValid());
$this->assertFalse($p3->isValid());
$this->assertSame(
'foo',
(string) $p3->getCriticalThreshold()
);
$p4 = Perfdata::fromString('test=1;;10@');
$this->assertFalse($p2->getCriticalThreshold()->isValid());
$this->assertFalse($p4->isValid());
$this->assertSame(
'10@',
(string) $p4->getCriticalThreshold()
);
}
}

View file

@ -319,5 +319,25 @@ class ThresholdRangeTest extends TestCase
'foo',
(string) ThresholdRange::fromString('foo')
);
$this->assertSame(
'4,4:2,2',
(string) ThresholdRange::fromString('4,4:2,2')
);
}
public function testInvalidThresholdNotationsConsideredInValid()
{
$this->assertFalse(
ThresholdRange::fromString('10@')->isValid(),
'Invalid threshold notation 10@ considered as valid'
);
$this->assertFalse(
ThresholdRange::fromString('foo')->isValid(),
'Invalid threshold notation foo considered as valid'
);
$this->assertFalse(
ThresholdRange::fromString('4,4:2,2')->isValid(),
'Invalid threshold notation 4,4:2,2 considered as valid'
);
}
}