From 32bdee1264319412fa6edcf924d2f60c3d94dbfd Mon Sep 17 00:00:00 2001 From: Dolf Starreveld Date: Sat, 26 Mar 2016 17:16:04 -0700 Subject: [PATCH 1/3] Fixed omission of min or max from graphs if set to 0 String version of Metric does not include min or max values, even if they were present in the string passed to the constructor. String version of Metric does not include values for warn, crit, min, max if they were specified as 0. This happens because their inclusion in the string representation of Metric was conditional on their boolean value (which is false for 0). Changed to check for None instead. String version of Metric did not collapse None values into empty semi-colon delimited sections, causing input of ;;1 to result in ;1 on conversion to string. --- shinken/misc/perfdata.py | 11 +++--- test/test_string_perfdata.py | 66 ++++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 6 deletions(-) create mode 100644 test/test_string_perfdata.py diff --git a/shinken/misc/perfdata.py b/shinken/misc/perfdata.py index 6e7a4c7dbb..5f51f5fc99 100755 --- a/shinken/misc/perfdata.py +++ b/shinken/misc/perfdata.py @@ -71,12 +71,11 @@ def __init__(self, s): self.max = 100 def __str__(self): - s = "%s=%s%s" % (self.name, self.value, self.uom) - if self.warning: - s = s + ";%s" % (self.warning) - if self.critical: - s = s + ";%s" % (self.critical) - return s + components = [ "%s=%s%s" % (self.name, self.value, self.uom), + self.warning, self.critical, self.min, self.max ] + while components[-1] is None: + components.pop() + return ";".join(map(lambda v: "" if v is None else str(v), components)) class PerfDatas: diff --git a/test/test_string_perfdata.py b/test/test_string_perfdata.py new file mode 100644 index 0000000000..f5cdfcca15 --- /dev/null +++ b/test/test_string_perfdata.py @@ -0,0 +1,66 @@ +#!/usr/bin/env python +# -*- coding: utf-8 -*- +# Copyright (C) 2009-2014: +# Gabes Jean, naparuba@gmail.com +# Gerhard Lausser, Gerhard.Lausser@consol.de +# +# This file is part of Shinken. +# +# Shinken is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Shinken is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with Shinken. If not, see . + +# +# This file is used to test reading and processing of config files +# + +from shinken_test import * +from shinken.misc.perfdata import Metric, PerfDatas + + +class TestStringPerfdata(ShinkenTest): + # Uncomment this is you want to use a specific configuration + # for your test + #def setUp(self): + # self.setup_with_file('etc/shinken_parse_perfdata.cfg') + + def test_string_all_four(self): + self.assertEqual('ramused=1009MB;1;2;3;4', str(Metric('ramused=1009MB;1;2;3;4'))) + + def test_string_drop_empty_from_end(self): + self.assertEqual('ramused=1009MB', str(Metric('ramused=1009MB'))) + self.assertEqual('ramused=1009MB;1', str(Metric('ramused=1009MB;1'))) + self.assertEqual('ramused=1009MB;1;2', str(Metric('ramused=1009MB;1;2'))) + self.assertEqual('ramused=1009MB;1;2;3', str(Metric('ramused=1009MB;1;2;3'))) + self.assertEqual('ramused=1009MB;1;2;3;4', str(Metric('ramused=1009MB;1;2;3;4'))) + + def test_string_empty_for_None(self): + self.assertEqual('ramused=1009MB', str(Metric('ramused=1009MB;'))) + self.assertEqual('ramused=1009MB', str(Metric('ramused=1009MB;;'))) + self.assertEqual('ramused=1009MB', str(Metric('ramused=1009MB;;;'))) + self.assertEqual('ramused=1009MB', str(Metric('ramused=1009MB;;;;'))) + + self.assertEqual('ramused=1009MB;;2', str(Metric('ramused=1009MB;;2'))) + self.assertEqual('ramused=1009MB;;;3', str(Metric('ramused=1009MB;;;3'))) + self.assertEqual('ramused=1009MB;;;;4', str(Metric('ramused=1009MB;;;;4'))) + + self.assertEqual('ramused=1009MB;;2;;4', str(Metric('ramused=1009MB;;2;;4'))) + + def test_string_zero_preserved(self): + self.assertEqual('ramused=1009MB;0', str(Metric('ramused=1009MB;0'))) + self.assertEqual('ramused=1009MB;;0', str(Metric('ramused=1009MB;;0'))) + self.assertEqual('ramused=1009MB;;;0', str(Metric('ramused=1009MB;;;0'))) + self.assertEqual('ramused=1009MB;;;;0', str(Metric('ramused=1009MB;;;;0'))) + +if __name__ == '__main__': + unittest.main() + From 17d7ce7b0680791e7462167fdbc707d6286a0182 Mon Sep 17 00:00:00 2001 From: Dolf Starreveld Date: Sat, 26 Mar 2016 21:03:45 -0700 Subject: [PATCH 2/3] Fixed for pep8 complaints --- shinken/misc/perfdata.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/shinken/misc/perfdata.py b/shinken/misc/perfdata.py index 5f51f5fc99..3d771ab6df 100755 --- a/shinken/misc/perfdata.py +++ b/shinken/misc/perfdata.py @@ -71,10 +71,9 @@ def __init__(self, s): self.max = 100 def __str__(self): - components = [ "%s=%s%s" % (self.name, self.value, self.uom), - self.warning, self.critical, self.min, self.max ] + components = ["%s=%s%s" % (self.name, self.value, self.uom), self.warning, self.critical, self.min, self.max] while components[-1] is None: - components.pop() + components.pop() return ";".join(map(lambda v: "" if v is None else str(v), components)) From 114854a35e50f5be9ee16a8f760e7c121e0ca6ae Mon Sep 17 00:00:00 2001 From: Dolf Starreveld Date: Mon, 28 Mar 2016 20:43:30 -0700 Subject: [PATCH 3/3] Fix handling of min and max values for percentages When to uom is % min and max were always forced to 0. This causes graphs to always have 0 and 100 for min and max, even if other values were supplied. This is changed to only force the default values if no other values had been set. In addition, when converting a Metric to a string, those same min and max values were always included. If they are actually 0 and 100 that is not necessary, and they are now left out, but only if not explicitly specified on input. These changes allow control over the appearance of min and max lines in graphs, regardless of their desired values. If the value is specified, even when equal to a default, they will be output and thus graphed. If defaults were generated, they will not be graphed. This leaves the semantics of the perfdata unchanged. --- shinken/misc/perfdata.py | 15 ++++++++++++--- test/test_string_perfdata.py | 16 ++++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/shinken/misc/perfdata.py b/shinken/misc/perfdata.py index 3d771ab6df..cb9e7e43d1 100755 --- a/shinken/misc/perfdata.py +++ b/shinken/misc/perfdata.py @@ -62,16 +62,24 @@ def __init__(self, s): self.critical = guess_int_or_float(r.group(5)) self.min = guess_int_or_float(r.group(6)) self.max = guess_int_or_float(r.group(7)) + self.min_specified = self.min is not None + self.max_specified = self.max is not None # print 'Name', self.name # print "Value", self.value # print "Res", r # print r.groups() if self.uom == '%': - self.min = 0 - self.max = 100 + self.min = self.min if self.min_specified is not None else 0 + self.max = self.max if self.max_specified is not None else 100 def __str__(self): - components = ["%s=%s%s" % (self.name, self.value, self.uom), self.warning, self.critical, self.min, self.max] + min = self.min if self.min_specified else None + max = self.max if self.max_specified else None + prefix = "min=%s, max=%s" % (min, max) + if self.uom == '%': + min = None if (self.min == 0) and not self.min_specified else min + max = None if (self.max == 100) and not self.max_specified else max + components = ["%s=%s%s" % (self.name, self.value, self.uom), self.warning, self.critical, min, max] while components[-1] is None: components.pop() return ";".join(map(lambda v: "" if v is None else str(v), components)) @@ -99,3 +107,4 @@ def __getitem__(self, key): def __contains__(self, key): return key in self.metrics + diff --git a/test/test_string_perfdata.py b/test/test_string_perfdata.py index f5cdfcca15..03a1e0cdca 100644 --- a/test/test_string_perfdata.py +++ b/test/test_string_perfdata.py @@ -61,6 +61,22 @@ def test_string_zero_preserved(self): self.assertEqual('ramused=1009MB;;;0', str(Metric('ramused=1009MB;;;0'))) self.assertEqual('ramused=1009MB;;;;0', str(Metric('ramused=1009MB;;;;0'))) + def test_string_percent_minmaxdefault_0_100(self): + # If not specified, defaults of 0 and 100 for min/max should not come back + self.assertEqual('utilization=80%;90;95', str(Metric('utilization=80%;90;95'))) + self.assertEqual('utilization=80%;90;95;;', str(Metric('utilization=80%;90;95')) + ";;") + + def test_string_percent_minmax_echo(self): + # Defined values of min max should come back always, even if defaults + self.assertEqual('utilization=80%;50;75;0;100', str(Metric('utilization=80%;50;75;0;100'))) + self.assertEqual('utilization=80%;50;75;0', str(Metric('utilization=80%;50;75;0'))) + self.assertEqual('utilization=80%;50;75;;100', str(Metric('utilization=80%;50;75;;100'))) + + # Same tests with non-default values + self.assertEqual('utilization=80%;50;75;85;95', str(Metric('utilization=80%;50;75;85;95'))) + self.assertEqual('utilization=80%;50;75;85', str(Metric('utilization=80%;50;75;85'))) + self.assertEqual('utilization=80%;50;75;;95', str(Metric('utilization=80%;50;75;;95'))) + if __name__ == '__main__': unittest.main()