Merge pull request #3451 from Icinga/feature/interpret-performance-data-threshold-ranges-8194

Interpret performance data threshold ranges

modules/monitoring/library/Monitoring/Plugin/Perfdata.php

@@ -61,18 +61,14 @@ class Perfdata
     /**
      * The WARNING threshold
      *
-     * TODO: Should be parsed Range-Object instead of string
+     * @var ThresholdRange
-     *
-     * @var string
      */
     protected $warningThreshold;
 
     /**
      * The CRITICAL threshold
      *
-     * TODO: Should be parsed Range-Object instead of string
+     * @var ThresholdRange
-     *
-     * @var string
      */
     protected $criticalThreshold;
 
@@ -96,6 +92,15 @@ class Perfdata
                 $this->maxValue = 100.0;
             }
         }
+
+        $warn = $this->warningThreshold->getMax();
+        if ($warn !== null) {
+            $crit = $this->criticalThreshold->getMax();
+            if ($crit !== null && $warn > $crit) {
+                $this->warningThreshold->setInverted();
+                $this->criticalThreshold->setInverted();
+            }
+        }
     }
 
     /**
@@ -111,7 +116,7 @@ class Perfdata
     {
         if (empty($perfdata)) {
             throw new InvalidArgumentException('Perfdata::fromString expects a string with content');
-        } elseif (false === strpos($perfdata, '=')) {
+        } elseif (strpos($perfdata, '=') === false) {
             throw new InvalidArgumentException(
                 'Perfdata::fromString expects a key=value formatted string. Got "' . $perfdata . '" instead'
             );
@@ -211,8 +216,8 @@ class Perfdata
         }
 
         if ($this->maxValue !== null) {
-            $minValue = $this->minValue !== null ? $this->minValue : 0;
+            $minValue = $this->minValue !== null ? $this->minValue : 0.0;
-            if ((float) ($this->maxValue - $minValue) === 0.0) {
+            if ($this->maxValue == $minValue) {
                 return null;
             }
 
@@ -223,9 +228,9 @@ class Perfdata
     }
 
     /**
-     * Return this performance data's warning treshold or null if it is not available
+     * Return this performance data's warning treshold
      *
-     * @return  null|string
+     * @return  ThresholdRange
      */
     public function getWarningThreshold()
     {
@@ -233,9 +238,9 @@ class Perfdata
     }
 
     /**
-     * Return this performance data's critical treshold or null if it is not available
+     * Return this performance data's critical treshold
      *
-     * @return  null|string
+     * @return  ThresholdRange
      */
     public function getCriticalThreshold()
     {
@@ -302,10 +307,23 @@ class Perfdata
                 }
             /* @noinspection PhpMissingBreakStatementInspection */
             case 3:
-                $this->criticalThreshold = trim($parts[2]) ? trim($parts[2]) : null;
+                $this->criticalThreshold = self::convert(
+                    ThresholdRange::fromString(trim($parts[2])),
+                    $this->unit
+                );
                 // Fallthrough
             case 2:
-                $this->warningThreshold = trim($parts[1]) ? trim($parts[1]) : null;
+                $this->warningThreshold = self::convert(
+                    ThresholdRange::fromString(trim($parts[1])),
+                    $this->unit
+                );
+        }
+
+        if ($this->warningThreshold === null) {
+            $this->warningThreshold = new ThresholdRange();
+        }
+        if ($this->criticalThreshold === null) {
+            $this->criticalThreshold = new ThresholdRange();
         }
     }
 
@@ -319,6 +337,22 @@ class Perfdata
      */
     protected static function convert($value, $fromUnit = null)
     {
+        if ($value instanceof ThresholdRange) {
+            $value = clone $value;
+
+            $min = $value->getMin();
+            if ($min !== null) {
+                $value->setMin(self::convert($min, $fromUnit));
+            }
+
+            $max = $value->getMax();
+            if ($max !== null) {
+                $value->setMax(self::convert($max, $fromUnit));
+            }
+
+            return $value;
+        }
+
         if (is_numeric($value)) {
             switch ($fromUnit) {
                 case 'us':
@@ -343,52 +377,21 @@ class Perfdata
     {
         $rawValue = $this->getValue();
         $minValue = $this->getMinimumValue() !== null ? $this->getMinimumValue() : 0;
-        $maxValue = $this->getMaximumValue();
         $usedValue = ($rawValue - $minValue);
-        $unusedValue = ($maxValue - $minValue) - $usedValue;
 
-        $warningThreshold = $this->convert($this->warningThreshold, $this->unit);
-        $criticalThreshold = $this->convert($this->criticalThreshold, $this->unit);
-
-        $gray = $unusedValue;
         $green = $orange = $red = 0;
 
-        $pieState = self::PERFDATA_OK;
+        if ($this->criticalThreshold->contains($rawValue)) {
-        if ($warningThreshold > $criticalThreshold) {
+            if ($this->warningThreshold->contains($rawValue)) {
-            // inverted threshold parsing OK > warning > critical
+                $green = $usedValue;
-            if (isset($warningThreshold) && $this->value <= $warningThreshold) {
+            } else {
-                $pieState = self::PERFDATA_WARNING;
+                $orange = $usedValue;
-            }
-            if (isset($criticalThreshold) && $this->value <= $criticalThreshold) {
-                $pieState = self::PERFDATA_CRITICAL;
             }
         } else {
-            // TODO: Use standard perfdata range format to decide the state #8194
+            $red = $usedValue;
-
-            // regular threshold parsing  OK < warning < critical
-            if (isset($warningThreshold) && $rawValue > $warningThreshold) {
-                $pieState = self::PERFDATA_WARNING;
-            }
-            if (isset($criticalThreshold) && $rawValue > $criticalThreshold) {
-                $pieState = self::PERFDATA_CRITICAL;
-            }
         }
 
-        switch ($pieState) {
+        return array($green, $orange, $red, ($this->getMaximumValue() - $minValue) - $usedValue);
-            case self::PERFDATA_OK:
-                $green = $usedValue;
-                break;
-
-            case self::PERFDATA_CRITICAL:
-                $red = $usedValue;
-                break;
-
-            case self::PERFDATA_WARNING:
-                $orange = $usedValue;
-                break;
-        }
-
-        return array($green, $orange, $red, $gray);
     }
 
 
@@ -413,6 +416,15 @@ class Perfdata
      */
     protected function format($value)
     {
+        if ($value instanceof ThresholdRange) {
+            if ($value->getMin()) {
+                return (string) $value;
+            }
+
+            $max = $value->getMax();
+            return $max === null ? '' : $this->format($max);
+        }
+
         if ($this->isPercentage()) {
             return (string)$value . '%';
         }
@@ -444,7 +456,7 @@ class Perfdata
 
     public function toArray()
     {
-        $parts = array(
+        return array(
             'label' => $this->getLabel(),
             'value' => $this->format($this->getvalue()),
             'min' => isset($this->minValue) && !$this->isPercentage()
@@ -453,14 +465,9 @@ class Perfdata
             'max' => isset($this->maxValue) && !$this->isPercentage()
                 ? $this->format($this->maxValue)
                 : '',
-            'warn' => isset($this->warningThreshold)
+            'warn' => $this->format($this->warningThreshold),
-                ? $this->format(self::convert($this->warningThreshold, $this->unit))
+            'crit' => $this->format($this->criticalThreshold)
-                : '',
-            'crit' => isset($this->criticalThreshold)
-                ? $this->format(self::convert($this->criticalThreshold, $this->unit))
-                : ''
         );
-        return $parts;
     }
 
     /**
@@ -476,13 +483,11 @@ class Perfdata
             return Service::STATE_UNKNOWN;
         }
 
-        if (! ($this->criticalThreshold === null
+        if (! $this->criticalThreshold->contains($this->value)) {
-            || $this->value < $this->criticalThreshold)) {
             return Service::STATE_CRITICAL;
         }
 
-        if (! ($this->warningThreshold === null
+        if (! $this->warningThreshold->contains($this->value)) {
-            || $this->value < $this->warningThreshold)) {
             return Service::STATE_WARNING;
         }
 

modules/monitoring/library/Monitoring/Plugin/ThresholdRange.php

@@ -0,0 +1,179 @@
+<?php
+/* Icinga Web 2 | (c) 2016 Icinga Development Team | GPLv2+ */
+
+namespace Icinga\Module\Monitoring\Plugin;
+
+/**
+ * The warning/critical threshold of a measured value
+ */
+class ThresholdRange
+{
+    /**
+     * The smallest value inside the range (null stands for -∞)
+     *
+     * @var float|null
+     */
+    protected $min;
+
+    /**
+     * The biggest value inside the range (null stands for ∞)
+     *
+     * @var float|null
+     */
+    protected $max;
+
+    /**
+     * Whether to invert the result of contains()
+     *
+     * @var bool
+     */
+    protected $inverted = false;
+
+    /**
+     * The unmodified range as passed to fromString()
+     *
+     * @var string
+     */
+    protected $raw;
+
+    /**
+     * Create a new instance based on a threshold range conforming to <https://nagios-plugins.org/doc/guidelines.html>
+     *
+     * @param   string  $rawRange
+     *
+     * @return  ThresholdRange
+     */
+    public static function fromString($rawRange)
+    {
+        $range = new static();
+        $range->raw = $rawRange;
+
+        if ($rawRange == '') {
+            return $range;
+        }
+
+        $rawRange = ltrim($rawRange);
+        if (substr($rawRange, 0, 1) === '@') {
+            $range->setInverted();
+            $rawRange = substr($rawRange, 1);
+        }
+
+        if (strpos($rawRange, ':') === false) {
+            $min = 0.0;
+            $max = floatval(trim($rawRange));
+        } else {
+            list($min, $max) = explode(':', $rawRange, 2);
+            $min = trim($min);
+            $max = trim($max);
+
+            switch ($min) {
+                case '':
+                    $min = 0.0;
+                    break;
+                case '~':
+                    $min = null;
+                    break;
+                default:
+                    $min = floatval($min);
+            }
+
+            $max = empty($max) ? null : floatval($max);
+        }
+
+        return $range->setMin($min)
+            ->setMax($max);
+    }
+
+    /**
+     * Set the smallest value inside the range (null stands for -∞)
+     *
+     * @param   float|null  $min
+     *
+     * @return  $this
+     */
+    public function setMin($min)
+    {
+        $this->min = $min;
+        return $this;
+    }
+
+    /**
+     * Get the smallest value inside the range (null stands for -∞)
+     *
+     * @return  float|null
+     */
+    public function getMin()
+    {
+        return $this->min;
+    }
+
+    /**
+     * Set the biggest value inside the range (null stands for ∞)
+     *
+     * @param   float|null  $max
+     *
+     * @return  $this
+     */
+    public function setMax($max)
+    {
+        $this->max = $max;
+        return $this;
+    }
+
+    /**
+     * Get the biggest value inside the range (null stands for ∞)
+     *
+     * @return  float|null
+     */
+    public function getMax()
+    {
+        return $this->max;
+    }
+
+    /**
+     * Set whether to invert the result of contains()
+     *
+     * @param   bool    $inverted
+     *
+     * @return  $this
+     */
+    public function setInverted($inverted = true)
+    {
+        $this->inverted = $inverted;
+        return $this;
+    }
+
+    /**
+     * Get whether to invert the result of contains()
+     *
+     * @return  bool
+     */
+    public function isInverted()
+    {
+        return $this->inverted;
+    }
+
+    /**
+     * Return whether $value is inside $this
+     *
+     * @param   float   $value
+     *
+     * @return  bool
+     */
+    public function contains($value)
+    {
+        return (bool) ($this->inverted ^ (
+            ($this->min === null || $this->min <= $value) && ($this->max === null || $this->max >= $value)
+        ));
+    }
+
+    /**
+     * Return the textual representation of $this, suitable for fromString()
+     *
+     * @return  string
+     */
+    public function __toString()
+    {
+        return (string) $this->raw;
+    }
+}

modules/monitoring/test/php/library/Monitoring/Plugin/PerfdataTest.php

@@ -189,12 +189,12 @@ class PerfdataTest extends BaseTestCase
     /**
      * @depends testWhetherFromStringParsesAGivenStringCorrectly
      */
-    public function testWhetherMissingValuesAreReturnedAsNull()
+    public function testWhetherMissingValuesAreProperlyHandled()
     {
         $perfdata = Perfdata::fromString('test=1;;3;5');
-        $this->assertNull(
+        $this->assertEmpty(
-            $perfdata->getWarningThreshold(),
+            (string) $perfdata->getWarningThreshold(),
-            'Perfdata objects do not return null for missing warning tresholds'
+            'Perfdata objects do not correctly identify omitted warning tresholds'
         );
         $this->assertNull(
             $perfdata->getMaximumValue(),

modules/monitoring/test/php/library/Monitoring/Plugin/ThresholdRangeTest.php

@@ -0,0 +1,322 @@
+<?php
+/* Icinga Web 2 | (c) 2018 Icinga Development Team | GPLv2+ */
+
+namespace Tests\Icinga\Module\Monitoring\Plugin;
+
+use Icinga\Test\BaseTestCase;
+use Icinga\Module\Monitoring\Plugin\ThresholdRange;
+
+class ThresholdRangeTest extends BaseTestCase
+{
+    public function testFromStringProperlyParsesDoubleExclusiveRanges()
+    {
+        $outside0And10 = ThresholdRange::fromString('10');
+        $this->assertEquals(
+            0,
+            $outside0And10->getMin(),
+            'ThresholdRange::fromString() does not identify zero as default minimum for double exclusive ranges'
+        );
+        $this->assertEquals(
+            10,
+            $outside0And10->getMax(),
+            'ThresholdRange::fromString() does not identify ten as explicit maximum for double exclusive ranges'
+        );
+        $this->assertFalse(
+            $outside0And10->isInverted(),
+            'ThresholdRange::fromString() identifies double exclusive ranges as inclusive'
+        );
+
+        $outside10And20 = ThresholdRange::fromString('10:20');
+        $this->assertEquals(
+            10,
+            $outside10And20->getMin(),
+            'ThresholdRange::fromString() does not identify ten as explicit minimum for double exclusive ranges'
+        );
+        $this->assertEquals(
+            20,
+            $outside10And20->getMax(),
+            'ThresholdRange::fromString() does not identify twenty as explicit maximum for double exclusive ranges'
+        );
+        $this->assertFalse(
+            $outside10And20->isInverted(),
+            'ThresholdRange::fromString() identifies double exclusive ranges as inclusive'
+        );
+    }
+
+    /**
+     * @depends testFromStringProperlyParsesDoubleExclusiveRanges
+     */
+    public function testContainsCorrectlyEvaluatesDoubleExclusiveRanges()
+    {
+        $outside0And10 = ThresholdRange::fromString('10');
+        $this->assertFalse(
+            $outside0And10->contains(-1),
+            'ThresholdRange::contains() identifies negative values as greater than or equal to zero'
+        );
+        $this->assertFalse(
+            $outside0And10->contains(11),
+            'ThresholdRange::contains() identifies eleven as smaller than or equal to ten'
+        );
+        $this->assertTrue(
+            $outside0And10->contains(10),
+            'ThresholdRange::contains() identifies 10 as outside the range 0..10'
+        );
+
+        $outside10And20 = ThresholdRange::fromString('10:20');
+        $this->assertFalse(
+            $outside10And20->contains(9),
+            'ThresholdRange::contains() identifies nine as greater than or equal to 10'
+        );
+        $this->assertFalse(
+            $outside10And20->contains(21),
+            'ThresholdRange::contains() identifies twenty-one as smaller than or equal to twenty'
+        );
+        $this->assertTrue(
+            $outside10And20->contains(20),
+            'ThresholdRange::contains() identifies 20 as outside the range 10..20'
+        );
+    }
+
+    public function testFromStringProperlyParsesSingleExclusiveRanges()
+    {
+        $smallerThan10 = ThresholdRange::fromString('10:');
+        $this->assertEquals(
+            10,
+            $smallerThan10->getMin(),
+            'ThresholdRange::fromString() does not identify ten as explicit minimum for single exclusive ranges'
+        );
+        $this->assertNull(
+            $smallerThan10->getMax(),
+            'ThresholdRange::fromString() does not identify infinity as default maximum for single exclusive ranges'
+        );
+        $this->assertFalse(
+            $smallerThan10->isInverted(),
+            'ThresholdRange::fromString() identifies single exclusive ranges as inclusive'
+        );
+
+        $greaterThan10 = ThresholdRange::fromString('~:10');
+        $this->assertNull(
+            $greaterThan10->getMin(),
+            'ThresholdRange::fromString() does not identify infinity as explicit minimum for single exclusive ranges'
+        );
+        $this->assertEquals(
+            10,
+            $greaterThan10->getMax(),
+            'ThresholdRange::fromString() does not identify ten as explicit maximum for single exclusive ranges'
+        );
+        $this->assertFalse(
+            $greaterThan10->isInverted(),
+            'ThresholdRange::fromString() identifies single exclusive ranges as inclusive'
+        );
+    }
+
+    /**
+     * @depends testFromStringProperlyParsesSingleExclusiveRanges
+     */
+    public function testContainsCorrectlyEvaluatesSingleExclusiveRanges()
+    {
+        $smallerThan10 = ThresholdRange::fromString('10:');
+        $this->assertFalse(
+            $smallerThan10->contains(9),
+            'ThresholdRange::contains() identifies nine as greater than or equal to ten'
+        );
+        $this->assertTrue(
+            $smallerThan10->contains(PHP_INT_MAX),
+            'ThresholdRange::contains() identifies infinity as outside the range 10..~'
+        );
+
+        $greaterThan10 = ThresholdRange::fromString('~:10');
+        $this->assertFalse(
+            $greaterThan10->contains(11),
+            'ThresholdRange::contains() identifies eleven as smaller than or equal to ten'
+        );
+        $this->assertTrue(
+            $greaterThan10->contains(~PHP_INT_MAX),
+            'ThresholdRange::contains() identifies negative infinity as outside the range ~..10'
+        );
+    }
+
+    public function testFromStringProperlyParsesInclusiveRanges()
+    {
+        $inside0And10 = ThresholdRange::fromString('@10');
+        $this->assertEquals(
+            0,
+            $inside0And10->getMin(),
+            'ThresholdRange::fromString() does not identify zero as default minimum for inclusive ranges'
+        );
+        $this->assertEquals(
+            10,
+            $inside0And10->getMax(),
+            'ThresholdRange::fromString() does not identify ten as explicit maximum for inclusive ranges'
+        );
+        $this->assertTrue(
+            $inside0And10->isInverted(),
+            'ThresholdRange::fromString() identifies inclusive ranges as double exclusive'
+        );
+
+        $inside10And20 = ThresholdRange::fromString('@10:20');
+        $this->assertEquals(
+            10,
+            $inside10And20->getMin(),
+            'ThresholdRange::fromString() does not identify ten as explicit minimum for inclusive ranges'
+        );
+        $this->assertEquals(
+            20,
+            $inside10And20->getMax(),
+            'ThresholdRange::fromString() does not identify twenty as explicit maximum for inclusive ranges'
+        );
+        $this->assertTrue(
+            $inside10And20->isInverted(),
+            'ThresholdRange::fromString() identifies inclusive ranges as double exclusive'
+        );
+
+        $greaterThan10 = ThresholdRange::fromString('@10:');
+        $this->assertEquals(
+            10,
+            $greaterThan10->getMin(),
+            'ThresholdRange::fromString() does not identify ten as explicit minimum for inclusive ranges'
+        );
+        $this->assertNull(
+            $greaterThan10->getMax(),
+            'ThresholdRange::fromString() does not identify infinity as default maximum for inclusive ranges'
+        );
+        $this->assertTrue(
+            $greaterThan10->isInverted(),
+            'ThresholdRange::fromString() identifies inclusive ranges as single exclusive'
+        );
+
+        $smallerThan10 = ThresholdRange::fromString('@~:10');
+        $this->assertNull(
+            $smallerThan10->getMin(),
+            'ThresholdRange::fromString() does not identify infinity as explicit minimum for inclusive ranges'
+        );
+        $this->assertEquals(
+            10,
+            $smallerThan10->getMax(),
+            'ThresholdRange::fromString() does not identify ten as explicit maximum for inclusive ranges'
+        );
+        $this->assertTrue(
+            $smallerThan10->isInverted(),
+            'ThresholdRange::fromString() identifies inclusive ranges as single exclusive'
+        );
+    }
+
+    /**
+     * @depends testFromStringProperlyParsesInclusiveRanges
+     */
+    public function testContainsCorrectlyEvaluatesInclusiveRanges()
+    {
+        $inside0And10 = ThresholdRange::fromString('@10');
+        $this->assertFalse(
+            $inside0And10->contains(10),
+            'ThresholdRange::contains() identifies ten as greater than ten'
+        );
+        $this->assertTrue(
+            $inside0And10->contains(11),
+            'ThresholdRange::contains() identifies eleven as smaller than or equal to ten'
+        );
+        $this->assertTrue(
+            $inside0And10->contains(-1),
+            'ThresholdRange::contains() identifies negative values as greater than or equal to zero'
+        );
+
+        $inside10And20 = ThresholdRange::fromString('@10:20');
+        $this->assertFalse(
+            $inside10And20->contains(20),
+            'ThresholdRange::contains() identifies twenty as greater than twenty'
+        );
+        $this->assertTrue(
+            $inside10And20->contains(21),
+            'ThresholdRange::contains() identifies twenty-one as smaller than or equal to twenty'
+        );
+        $this->assertTrue(
+            $inside10And20->contains(9),
+            'ThresholdRange::contains() identifies nine as greater than or equal to ten'
+        );
+
+        $greaterThan10 = ThresholdRange::fromString('@10:');
+        $this->assertFalse(
+            $greaterThan10->contains(PHP_INT_MAX),
+            'ThresholdRange::contains() identifies infinity as smaller than ten'
+        );
+        $this->assertTrue(
+            $greaterThan10->contains(9),
+            'ThresholdRange::contains() identifies nine as greater than or equal to ten'
+        );
+
+        $smallerThan10 = ThresholdRange::fromString('@~:10');
+        $this->assertFalse(
+            $smallerThan10->contains(~PHP_INT_MAX),
+            'ThresholdRange::contains() identifies negative infinity as greater than ten'
+        );
+        $this->assertTrue(
+            $smallerThan10->contains(11),
+            'ThresholdRange::contains() identifies eleven as smaller than or equal to ten'
+        );
+    }
+
+    public function testFromStringProperlyParsesEmptyThresholds()
+    {
+        $emptyThreshold = ThresholdRange::fromString('');
+        $this->assertNull(
+            $emptyThreshold->getMin(),
+            'ThresholdRange::fromString() does not identify negative infinity as implicit minimum for empty strings'
+        );
+        $this->assertNull(
+            $emptyThreshold->getMax(),
+            'ThresholdRange::fromString() does not identify infinity as implicit maximum for empty strings'
+        );
+        $this->assertFalse(
+            $emptyThreshold->isInverted(),
+            'ThresholdRange::fromString() identifies empty strings as inclusive ranges rather than exclusive'
+        );
+    }
+
+    /**
+     * @depends testFromStringProperlyParsesEmptyThresholds
+     */
+    public function testContainsEvaluatesEverythingToTrueForEmptyThresholds()
+    {
+        $emptyThreshold = ThresholdRange::fromString('');
+        $this->assertTrue(
+            $emptyThreshold->contains(0),
+            'ThresholdRange::contains() does not identify zero as valid without any threshold'
+        );
+        $this->assertTrue(
+            $emptyThreshold->contains(10),
+            'ThresholdRange::contains() does not identify ten as valid without any threshold'
+        );
+        $this->assertTrue(
+            $emptyThreshold->contains(PHP_INT_MAX),
+            'ThresholdRange::contains() does not identify infinity as valid without any threshold'
+        );
+        $this->assertTrue(
+            $emptyThreshold->contains(~PHP_INT_MAX),
+            'ThresholdRange::contains() does not identify negative infinity as valid without any threshold'
+        );
+    }
+
+    public function testInvalidThresholdNotationsAreRenderedAsIs()
+    {
+        $this->assertEquals(
+            ':',
+            (string) ThresholdRange::fromString(':')
+        );
+        $this->assertEquals(
+            '~:',
+            (string) ThresholdRange::fromString('~:')
+        );
+        $this->assertEquals(
+            '20:10',
+            (string) ThresholdRange::fromString('20:10')
+        );
+        $this->assertEquals(
+            '10@',
+            (string) ThresholdRange::fromString('10@')
+        );
+        $this->assertEquals(
+            'foo',
+            (string) ThresholdRange::fromString('foo')
+        );
+    }
+}