Skip to content

Commit

Permalink
Merge branch 'bugfix/show-non-piechart-perfdata-not-as-piechart-6515'
Browse files Browse the repository at this point in the history 
fixes #6515
  • Loading branch information
Johannes Meyer committed Jul 14, 2014
2 parents b046023 + bacea36 commit a40feeb
Show file tree
Hide file tree
Showing 6 changed files with 184 additions and 100 deletions.
4 changes: 2 additions & 2 deletions modules/monitoring/application/clicommands/ListCommand.php
View file
Expand Up @@ -263,13 +263,13 @@ protected function renderStatusQuery($query)
try {
$pset = PerfdataSet::fromString($row->service_perfdata);
$perfs = array();
foreach ($pset as $perfName => $p) {
foreach ($pset as $p) {
if ($percent = $p->getPercentage()) {
if ($percent < 0 || $percent > 100) {
continue;
}
$perfs[] = ' '
. $perfName
. $p->getLabel()
. ': '
. $this->getPercentageSign($percent)
. ' '
Expand Down
33 changes: 19 additions & 14 deletions modules/monitoring/application/views/helpers/Perfdata.php
View file
Expand Up @@ -11,14 +11,18 @@ class Zend_View_Helper_Perfdata extends Zend_View_Helper_Abstract
{
public function perfdata($perfdataStr, $compact = false, $float = false)
{
$pset = PerfdataSet::fromString($perfdataStr)->asArray();
$onlyPieChartData = array_filter($pset, function ($e) { return $e->getPercentage() > 0; });
if ($compact) {
$onlyPieChartData = array_slice($onlyPieChartData, 0, 5);
} else {
$nonPieChartData = array_filter($pset, function ($e) { return $e->getPercentage() == 0; });
}

$result = '';
$table = array();
$pset = array_slice(PerfdataSet::fromString($perfdataStr)->asArray(), 0, ($compact ? 5 : null));
foreach ($pset as $label => $perfdata) {
if (!$perfdata->isPercentage() && $perfdata->getMaximumValue() === null) {
continue;
}
$pieChart = $this->createInlinePie($perfdata, $label, htmlspecialchars($label));
foreach ($onlyPieChartData as $perfdata) {
$pieChart = $this->createInlinePie($perfdata);
if ($compact) {
if (! $float) {
$result .= $pieChart->render();
Expand All @@ -27,22 +31,23 @@ public function perfdata($perfdataStr, $compact = false, $float = false)
}
} else {
if (! $perfdata->isPercentage()) {
// TODO: Should we trust sprintf-style placeholders in perfdata titles?
$pieChart->setTooltipFormat('{{label}}: {{formatted}} ({{percent}}%)');
}
$pieChart->setStyle('margin: 0.2em 0.5em 0.2em 0.5em;');
$table[] = '<tr><th>' . $pieChart->render()
. htmlspecialchars($label)
. htmlspecialchars($perfdata->getLabel())
. '</th><td> '
. htmlspecialchars($this->formatPerfdataValue($perfdata)) .
' </td></tr>';
}
}

// TODO: What if we have both? And should we trust sprintf-style placeholders in perfdata titles?
if (empty($table)) {
return $compact ? $result : $perfdataStr;
if ($compact) {
return $result;
} else {
return '<table class="perfdata">' . implode("\n", $table) . '</table>';
$pieCharts = empty($table) ? '' : '<table class="perfdata">' . implode("\n", $table) . '</table>';
return $pieCharts . "\n" . implode("<br>\n", $nonPieChartData);
}
}

Expand Down Expand Up @@ -81,10 +86,10 @@ protected function formatPerfdataValue(Perfdata $perfdata)
return $perfdata->getValue();
}

protected function createInlinePie(Perfdata $perfdata, $title, $label = '')
protected function createInlinePie(Perfdata $perfdata)
{
$pieChart = new InlinePie($this->calculatePieChartData($perfdata), $title);
$pieChart->setLabel($label);
$pieChart = new InlinePie($this->calculatePieChartData($perfdata), $perfdata->getLabel());
$pieChart->setLabel(htmlspecialchars($perfdata->getLabel()));
$pieChart->setHideEmptyLabel();

//$pieChart->setHeight(32)->setWidth(32);
Expand Down
70 changes: 51 additions & 19 deletions modules/monitoring/library/Monitoring/Plugin/Perfdata.php
View file
Expand Up @@ -4,7 +4,7 @@

namespace Icinga\Module\Monitoring\Plugin;

use Icinga\Exception\ProgrammingError;
use InvalidArgumentException;

class Perfdata
{
Expand All @@ -22,6 +22,13 @@ class Perfdata
*/
protected $unit;

/**
* The label
*
* @var string
*/
protected $label;

/**
* The value
*
Expand Down Expand Up @@ -58,13 +65,15 @@ class Perfdata
protected $criticalThreshold;

/**
* Create a new Perfdata object based on the given performance data value
* Create a new Perfdata object based on the given performance data label and value
*
* @param string $perfdataValue The value to parse
* @param string $label The perfdata label
* @param string $value The perfdata value
*/
protected function __construct($perfdataValue)
public function __construct($label, $value)
{
$this->perfdataValue = $perfdataValue;
$this->perfdataValue = $value;
$this->label = $label;
$this->parse();

if ($this->unit === '%') {
Expand All @@ -78,25 +87,30 @@ protected function __construct($perfdataValue)
}

/**
* Return a new Perfdata object based on the given performance data value
* Return a new Perfdata object based on the given performance data key=value pair
*
* @param string $perfdataValue The value to parse
* @param string $perfdata The key=value pair to parse
*
* @return Perfdata
*
* @throws ProgrammingError In case the given performance data value has no content
* @throws InvalidArgumentException In case the given performance data has no content or a invalid format
*/
public static function fromString($perfdataValue)
public static function fromString($perfdata)
{
if (empty($perfdataValue)) {
throw new ProgrammingError('Perfdata::fromString expects a string with content');
if (empty($perfdata)) {
throw new InvalidArgumentException('Perfdata::fromString expects a string with content');
} elseif (false === strpos($perfdata, '=')) {
throw new InvalidArgumentException(
'Perfdata::fromString expects a key=value formatted string. Got "' . $perfdata . '" instead'
);
}

return new static($perfdataValue);
list($label, $value) = explode('=', $perfdata, 2);
return new static(trim($label), trim($value));
}

/**
* Return whether this performance data value is a number
* Return whether this performance data's value is a number
*
* @return bool True in case it's a number, otherwise False
*/
Expand All @@ -106,7 +120,7 @@ public function isNumber()
}

/**
* Return whether this performance data value are seconds
* Return whether this performance data's value are seconds
*
* @return bool True in case it's seconds, otherwise False
*/
Expand All @@ -116,7 +130,7 @@ public function isSeconds()
}

/**
* Return whether this performance data value is in percentage
* Return whether this performance data's value is in percentage
*
* @return bool True in case it's in percentage, otherwise False
*/
Expand All @@ -126,7 +140,7 @@ public function isPercentage()
}

/**
* Return whether this performance data value is in bytes
* Return whether this performance data's value is in bytes
*
* @return bool True in case it's in bytes, otherwise False
*/
Expand All @@ -136,7 +150,7 @@ public function isBytes()
}

/**
* Return whether this performance data value is a counter
* Return whether this performance data's value is a counter
*
* @return bool True in case it's a counter, otherwise False
*/
Expand All @@ -145,6 +159,14 @@ public function isCounter()
return $this->unit === 'c';
}

/**
* Return this perfomance data's label
*/
public function getLabel()
{
return $this->label;
}

/**
* Return the value or null if it is unknown (U)
*
Expand Down Expand Up @@ -176,7 +198,7 @@ public function getPercentage()
}

/**
* Return this value's warning treshold or null if it is not available
* Return this performance data's warning treshold or null if it is not available
*
* @return null|string
*/
Expand All @@ -186,7 +208,7 @@ public function getWarningThreshold()
}

/**
* Return this value's critical treshold or null if it is not available
* Return this performance data's critical treshold or null if it is not available
*
* @return null|string
*/
Expand Down Expand Up @@ -215,6 +237,16 @@ public function getMaximumValue()
return $this->maxValue;
}

/**
* Return this performance data as string
*
* @return string
*/
public function __toString()
{
return sprintf(strpos($this->label, ' ') === false ? '%s=%s' : "'%s'=%s", $this->label, $this->perfdataValue);
}

/**
* Parse the current performance data value
*
Expand Down
2 changes: 1 addition & 1 deletion modules/monitoring/library/Monitoring/Plugin/PerfdataSet.php
View file
Expand Up @@ -85,7 +85,7 @@ protected function parse()
$value = trim($this->readUntil(' '));

if ($label && $value) {
$this->perfdata[$label] = Perfdata::fromString($value);
$this->perfdata[] = new Perfdata($label, $value);
}
}
}
Expand Down
40 changes: 20 additions & 20 deletions modules/monitoring/test/php/library/Monitoring/Plugin/PerfdataSetTest.php
View file
Expand Up @@ -17,64 +17,64 @@ class PerfdataSetTest extends BaseTestCase
public function testWhetherValidSimplePerfdataLabelsAreProperlyParsed()
{
$pset = PerfdataSetWithPublicData::fromString('key1=val1 key2=val2 key3 =val3');
$this->assertArrayHasKey(
$this->assertEquals(
'key1',
$pset->perfdata,
$pset->perfdata[0]->getLabel(),
'PerfdataSet does not correctly parse valid simple labels'
);
$this->assertArrayHasKey(
$this->assertEquals(
'key2',
$pset->perfdata,
$pset->perfdata[1]->getLabel(),
'PerfdataSet does not correctly parse valid simple labels'
);
$this->assertArrayHasKey(
$this->assertEquals(
'key3',
$pset->perfdata,
$pset->perfdata[2]->getLabel(),
'PerfdataSet does not correctly parse valid simple labels'
);
}

public function testWhetherNonQuotedPerfdataLablesWithSpacesAreProperlyParsed()
{
$pset = PerfdataSetWithPublicData::fromString('key 1=val1 key 1 + 1=val2');
$this->assertArrayHasKey(
$this->assertEquals(
'key 1',
$pset->perfdata,
$pset->perfdata[0]->getLabel(),
'PerfdataSet does not correctly parse non quoted labels with spaces'
);
$this->assertArrayHasKey(
$this->assertEquals(
'key 1 + 1',
$pset->perfdata,
$pset->perfdata[1]->getLabel(),
'PerfdataSet does not correctly parse non quoted labels with spaces'
);
}

public function testWhetherValidQuotedPerfdataLabelsAreProperlyParsed()
{
$pset = PerfdataSetWithPublicData::fromString('\'key 1\'=val1 "key 2"=val2');
$this->assertArrayHasKey(
$this->assertEquals(
'key 1',
$pset->perfdata,
$pset->perfdata[0]->getLabel(),
'PerfdataSet does not correctly parse valid quoted labels'
);
$this->assertArrayHasKey(
$this->assertEquals(
'key 2',
$pset->perfdata,
$pset->perfdata[1]->getLabel(),
'PerfdataSet does not correctly parse valid quoted labels'
);
}

public function testWhetherInvalidQuotedPerfdataLabelsAreProperlyParsed()
{
$pset = PerfdataSetWithPublicData::fromString('\'key 1=val1 key 2"=val2');
$this->assertArrayHasKey(
$this->assertEquals(
'key 1',
$pset->perfdata,
$pset->perfdata[0]->getLabel(),
'PerfdataSet does not correctly parse invalid quoted labels'
);
$this->assertArrayHasKey(
$this->assertEquals(
'key 2"',
$pset->perfdata,
$pset->perfdata[1]->getLabel(),
'PerfdataSet does not correctly parse invalid quoted labels'
);
}
Expand All @@ -85,8 +85,8 @@ public function testWhetherInvalidQuotedPerfdataLabelsAreProperlyParsed()
public function testWhetherAPerfdataSetIsIterable()
{
$pset = PerfdataSet::fromString('key=value');
foreach ($pset as $label => $value) {
$this->assertEquals('key', $label);
foreach ($pset as $p) {
$this->assertEquals('key', $p->getLabel());
return;
}

Expand Down

0 comments on commit a40feeb

Please sign in to comment.