From 5c890aa565dcd21dde9ad367d5b9c774b0558de7 Mon Sep 17 00:00:00 2001 From: raviks789 <33730024+raviks789@users.noreply.github.com> Date: Wed, 23 Aug 2023 14:54:15 +0200 Subject: [PATCH] Do not evaluate invalid performance data --- library/Icingadb/Util/PerfData.php | 83 ++++++++++++++++--- library/Icingadb/Util/ThresholdRange.php | 13 +++ .../Icingadb/Widget/Detail/PerfDataTable.php | 20 ++++- public/css/widget/performance-data-table.less | 5 ++ 4 files changed, 109 insertions(+), 12 deletions(-) diff --git a/library/Icingadb/Util/PerfData.php b/library/Icingadb/Util/PerfData.php index eccafc544..00ae47f4d 100644 --- a/library/Icingadb/Util/PerfData.php +++ b/library/Icingadb/Util/PerfData.php @@ -4,6 +4,7 @@ namespace Icinga\Module\Icingadb\Util; +use Exception; use Icinga\Module\Icingadb\Common\ServiceStates; use Icinga\Web\Widget\Chart\InlinePie; use InvalidArgumentException; @@ -60,17 +61,24 @@ class PerfData /** * The WARNING threshold * - * @var ThresholdRange + * @var ThresholdRange|string */ protected $warningThreshold; /** * The CRITICAL threshold * - * @var ThresholdRange + * @var ThresholdRange|string */ protected $criticalThreshold; + /** + * Whether the performance data is invalid + * + * @var bool + */ + protected $isValid; + /** * Create a new PerfData object based on the given performance data label and value * @@ -92,12 +100,14 @@ public function __construct(string $label, string $value) } } - $warn = $this->warningThreshold->getMax(); - if ($warn !== null) { - $crit = $this->criticalThreshold->getMax(); - if ($crit !== null && $warn > $crit) { - $this->warningThreshold->setInverted(); - $this->criticalThreshold->setInverted(); + if ($this->isValid) { + $warn = $this->warningThreshold->getMax(); + if ($warn !== null) { + $crit = $this->criticalThreshold->getMax(); + if ($crit !== null && $warn > $crit) { + $this->warningThreshold->setInverted(); + $this->criticalThreshold->setInverted(); + } } } } @@ -312,7 +322,7 @@ public function isCounter(): bool */ public function isVisualizable(): bool { - return isset($this->minValue) && isset($this->maxValue) && isset($this->value); + return isset($this->minValue, $this->maxValue, $this->value) && $this->isValid(); } /** @@ -416,6 +426,34 @@ public function __toString() return $this->formatLabel(); } + /** + * Checks if the performance data can be evaluated + */ + protected function validate() + { + if (is_string($this->warningThreshold) || is_string($this->criticalThreshold)) { + $this->isValid = false; + } else { + $warnMin = $this->warningThreshold->getMin(); + $warnMax = $this->warningThreshold->getMax(); + $criticalMin = $this->criticalThreshold->getMin(); + $criticalMax = $this->criticalThreshold->getMax(); + + switch (true) { + case $this->minValue !== null && ! is_numeric($this->minValue): + case $this->maxValue !== null && ! is_numeric($this->maxValue): + case $criticalMin !== null && ! is_numeric($criticalMin): + case $criticalMax !== null && ! is_numeric($criticalMax): + case $warnMin !== null & ! is_numeric($warnMin): + case $warnMax !== null & ! is_numeric($warnMax): + $this->isValid = false; + break; + default: + $this->isValid = true; + } + } + } + /** * Parse the current performance data value * @@ -450,10 +488,19 @@ protected function parse() } /* @noinspection PhpMissingBreakStatementInspection */ case 3: - $this->criticalThreshold = ThresholdRange::fromString(trim($parts[2])); + try { + $this->criticalThreshold = ThresholdRange::fromString(trim($parts[2])); + } catch (Exception $e) { + $this->criticalThreshold = trim($parts[2]); + } + // Fallthrough case 2: - $this->warningThreshold = ThresholdRange::fromString(trim($parts[1])); + try { + $this->warningThreshold = ThresholdRange::fromString(trim($parts[1])); + } catch (Exception $e) { + $this->warningThreshold = trim($parts[1]); + } } if ($this->warningThreshold === null) { @@ -643,4 +690,18 @@ public function worseThan(PerfData $rhs): bool return false; } + + /** + * Returns whether the performance data could be evaluated + * + * @return bool + */ + public function isValid(): bool + { + if ($this->isValid === null) { + $this->validate(); + } + + return $this->isValid; + } } diff --git a/library/Icingadb/Util/ThresholdRange.php b/library/Icingadb/Util/ThresholdRange.php index fefed75ba..824eb9942 100644 --- a/library/Icingadb/Util/ThresholdRange.php +++ b/library/Icingadb/Util/ThresholdRange.php @@ -4,6 +4,8 @@ namespace Icinga\Module\Icingadb\Util; +use Exception; + /** * The warning/critical threshold of a measured value */ @@ -43,6 +45,8 @@ class ThresholdRange * @param string $rawRange * * @return ThresholdRange + * + * @throws Exception */ public static function fromString(string $rawRange): self { @@ -61,6 +65,11 @@ public static function fromString(string $rawRange): self if (strpos($rawRange, ':') === false) { $min = 0.0; + $max = trim($rawRange); + if (! is_numeric($max)) { + throw new Exception(sprintf('Threshold range contains invalid max %s', $max)); + } + $max = floatval(trim($rawRange)); } else { list($min, $max) = explode(':', $rawRange, 2); @@ -75,6 +84,10 @@ public static function fromString(string $rawRange): self $min = null; break; default: + if (! is_numeric($min)) { + throw new Exception(sprintf('Threshold range contains invalid min %s', $min)); + } + $min = floatval($min); } diff --git a/library/Icingadb/Widget/Detail/PerfDataTable.php b/library/Icingadb/Widget/Detail/PerfDataTable.php index 4e0308965..99c87a71a 100644 --- a/library/Icingadb/Widget/Detail/PerfDataTable.php +++ b/library/Icingadb/Widget/Detail/PerfDataTable.php @@ -8,13 +8,18 @@ use Icinga\Module\Icingadb\Util\PerfDataSet; use Icinga\Module\Icingadb\Widget\EmptyState; use ipl\Html\Attributes; +use ipl\Html\HtmlDocument; 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; + /** @var bool Whether the table contains a sparkline column */ protected $containsSparkline = false; @@ -64,7 +69,7 @@ public function assemble() ] ); foreach ($pieChartData as $perfdata) { - if ($perfdata->isVisualizable()) { + if ($perfdata->isVisualizable() || ! $perfdata->isValid()) { $columns[''] = ''; $this->containsSparkline = true; } @@ -108,6 +113,19 @@ public function assemble() 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(''); } diff --git a/public/css/widget/performance-data-table.less b/public/css/widget/performance-data-table.less index a1a7b6ee7..26c18c832 100644 --- a/public/css/widget/performance-data-table.less +++ b/public/css/widget/performance-data-table.less @@ -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 {