From 32bdee1264319412fa6edcf924d2f60c3d94dbfd Mon Sep 17 00:00:00 2001 From: Dolf Starreveld Date: Sat, 26 Mar 2016 17:16:04 -0700 Subject: [PATCH 1/4] 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/4] 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/4] 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() From 7a756e9a239cd6a147d79cad93579a7289e2ccee Mon Sep 17 00:00:00 2001 From: Dolf Starreveld Date: Tue, 29 Mar 2016 01:12:00 -0700 Subject: [PATCH 4/4] Various fixes in perfdata handling Previous change for handling defaults for min/max in perfdata without % was incorrect. We should distinguish between several use cases. The first is that when reading min/max for this case, the values should reflect defaults even if no values where specified. The second is that when these values were not specified, they should also not be reflected in the string representation. This was accomplished through two shadow variables. The handling of quotes in metric names was incorrect. As written "outer quotes" which would be necessitated if the metric name contains at least one space, are not stripped resulting in the "name" property incorrectly still containing these quotes. They should be stripped and re-attached in the string representation only. Secondly all single quotes are stripped. This is wrong, as the specification states that single quotes in the name are to be represented by two successive single quotes in perfdata. Stripping would remove all trace of the quotes. The fix is to replace double-single-quotes with single quotes, after first stripping just the outer quotes, if presen. A related problem is that none of the input transformations on the name are "undone" in the string representation. This makes a re-parse of the string representation result in a non-identical result. The fix was to re-attach the reverse transformations. Several test cases were added to catch the above scenarios. --- shinken/misc/perfdata.py | 31 ++++---- test/test_parse_perfdata.py | 142 +++++++++++++++++++++++++++-------- test/test_string_perfdata.py | 12 +++ 3 files changed, 140 insertions(+), 45 deletions(-) diff --git a/shinken/misc/perfdata.py b/shinken/misc/perfdata.py index cb9e7e43d1..9ed4c2261e 100755 --- a/shinken/misc/perfdata.py +++ b/shinken/misc/perfdata.py @@ -34,6 +34,8 @@ '^([^=]+)=([\d\.\-\+eE]+)([\w\/%]*)' ';?([\d\.\-\+eE:~@]+)?;?([\d\.\-\+eE:~@]+)?;?([\d\.\-\+eE]+)?;?([\d\.\-\+eE]+)?;?\s*' ) +quote_pattern = re.compile("^'(.+)'$") +replace1_pattern = re.compile('\1') # If we can return an int or a float, or None @@ -54,32 +56,31 @@ def __init__(self, s): # print "Analysis string", s r = metric_pattern.match(s) if r: - # Get the name but remove all ' in it - self.name = r.group(1).replace("'", "") + # Strip outer quotes, and replace double quotes with single + self.name = re.sub(r"^'(.+)'$", r'\1', r.group(1)).replace("''", "'") self.value = guess_int_or_float(r.group(2)) self.uom = r.group(3) self.warning = guess_int_or_float(r.group(4)) 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 + self.min = self.min_supplied = guess_int_or_float(r.group(6)) + self.max = self.max_supplied = guess_int_or_float(r.group(7)) # print 'Name', self.name # print "Value", self.value # print "Res", r # print r.groups() if self.uom == '%': - 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 + self.min = 0 if self.min is None else self.min + self.max = 100 if self.max is None else self.max def __str__(self): - 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] + # Restore double quotes in nae + name = self.name.replace("'", "''") + # Quote whole name if it contains a space + if " " in name: + name = "'" + name + "'" + min = self.min_supplied + max = self.max_supplied + components = ["%s=%s%s" % (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)) diff --git a/test/test_parse_perfdata.py b/test/test_parse_perfdata.py index 033a42f9dd..18933dd66c 100644 --- a/test/test_parse_perfdata.py +++ b/test/test_parse_perfdata.py @@ -45,16 +45,6 @@ def test_parsing_perfdata(self): self.assertEqual(0, m.min) self.assertEqual(1982, m.max) - s = 'ramused=90%;85;95;;' - m = Metric(s) - self.assertEqual('ramused', m.name) - self.assertEqual(90, m.value) - self.assertEqual('%', m.uom) - self.assertEqual(85, m.warning) - self.assertEqual(95, m.critical) - self.assertEqual(0, m.min) - self.assertEqual(100, m.max) - s = 'ramused=1009MB;;;0;1982 swapused=540MB;;;; memused=90%' p = PerfDatas(s) p.metrics @@ -78,26 +68,6 @@ def test_parsing_perfdata(self): self.assertEqual(3, len(p)) - s = "'Physical Memory Used'=12085620736Bytes; 'Physical Memory Utilisation'=94%;80;90;" - p = PerfDatas(s) - p.metrics - m = p['Physical Memory Used'] - self.assertEqual('Physical Memory Used', m.name) - self.assertEqual(12085620736, m.value) - self.assertEqual('Bytes', m.uom) - self.assertIs(None, m.warning) - self.assertIs(None, m.critical) - self.assertIs(None, m.min) - self.assertIs(None, m.max) - - m = p['Physical Memory Utilisation'] - self.assertEqual('Physical Memory Utilisation', m.name) - self.assertEqual(94, m.value) - self.assertEqual('%', m.uom) - self.assertEqual(80, m.warning) - self.assertEqual(90, m.critical) - self.assertEqual(0, m.min) - self.assertEqual(100, m.max) s = "'C: Space'=35.07GB; 'C: Utilisation'=87.7%;90;95;" p = PerfDatas(s) @@ -147,6 +117,118 @@ def test_parsing_perfdata(self): p = PerfDatas(s) self.assertEqual(len(p), 0) + def test_parsing_perfdata_percentages(self): + # Test default min and max automatically supplied + s = 'ramused=90%;85;95;;' + m = Metric(s) + self.assertEqual('ramused', m.name) + self.assertEqual(90, m.value) + self.assertEqual('%', m.uom) + self.assertEqual(85, m.warning) + self.assertEqual(95, m.critical) + self.assertEqual(0, m.min) + self.assertEqual(100, m.max) + # + # Test non-default min and max are handled + s = 'ramused=90%;85;95;10;95' + m = Metric(s) + self.assertEqual('ramused', m.name) + self.assertEqual(90, m.value) + self.assertEqual('%', m.uom) + self.assertEqual(85, m.warning) + self.assertEqual(95, m.critical) + self.assertEqual(10, m.min) + self.assertEqual(95, m.max) + # + # Test non-default min and max as floats are handled + s = 'ramused=90%;85;95;10.5;95.3' + m = Metric(s) + self.assertEqual('ramused', m.name) + self.assertEqual(90, m.value) + self.assertEqual('%', m.uom) + self.assertEqual(85, m.warning) + self.assertEqual(95, m.critical) + self.assertEqual(10.5, m.min) + self.assertEqual(95.3, m.max) + + def test_parsing_perfdata_spaces(self): + s = "'Physical Memory Used'=12085620736Bytes; 'Physical Memory Utilisation'=94%;80;90;" + p = PerfDatas(s) + p.metrics + m = p['Physical Memory Used'] + self.assertEqual('Physical Memory Used', m.name) + self.assertEqual(12085620736, m.value) + self.assertEqual('Bytes', m.uom) + self.assertIs(None, m.warning) + self.assertIs(None, m.critical) + self.assertIs(None, m.min) + self.assertIs(None, m.max) + + m = p['Physical Memory Utilisation'] + self.assertEqual('Physical Memory Utilisation', m.name) + self.assertEqual(94, m.value) + self.assertEqual('%', m.uom) + self.assertEqual(80, m.warning) + self.assertEqual(90, m.critical) + self.assertEqual(0, m.min) + self.assertEqual(100, m.max) + + def test_parsing_perfdata_quotes(self): + # Make sure outer quuotes get stripped, with or without space in name + s = "'ramused'=1009MB;;;0;1982 'ram is used'=1009MB;;;0;1982" + p = PerfDatas(s) + m = p['ramused'] + self.assertEqual('ramused', m.name) + self.assertEqual(1009, m.value) + self.assertEqual('MB', m.uom) + self.assertEqual(None, m.warning) + self.assertEqual(None, m.critical) + self.assertEqual(0, m.min) + self.assertEqual(1982, m.max) + + m = p['ram is used'] + self.assertEqual('ram is used', m.name) + self.assertEqual(1009, m.value) + self.assertEqual('MB', m.uom) + self.assertEqual(None, m.warning) + self.assertEqual(None, m.critical) + self.assertEqual(0, m.min) + self.assertEqual(1982, m.max) + + # Confirm double quotes replace by single + s = "ram''used=1009MB;;;0;1982" + p = PerfDatas(s) + m = p["ram'used"] + self.assertEqual("ram'used", m.name) + self.assertEqual(1009, m.value) + self.assertEqual('MB', m.uom) + self.assertEqual(None, m.warning) + self.assertEqual(None, m.critical) + self.assertEqual(0, m.min) + self.assertEqual(1982, m.max) + + s = "ram''used''=1009MB;;;0;1982" + p = PerfDatas(s) + m = p["ram'used'"] + self.assertEqual("ram'used'", m.name) + self.assertEqual(1009, m.value) + self.assertEqual('MB', m.uom) + self.assertEqual(None, m.warning) + self.assertEqual(None, m.critical) + self.assertEqual(0, m.min) + self.assertEqual(1982, m.max) + + s = "ram'''used''=1009MB;;;0;1982" + p = PerfDatas(s) + m = p["ram''used'"] + self.assertEqual("ram''used'", m.name) + self.assertEqual(1009, m.value) + self.assertEqual('MB', m.uom) + self.assertEqual(None, m.warning) + self.assertEqual(None, m.critical) + self.assertEqual(0, m.min) + self.assertEqual(1982, m.max) + if __name__ == '__main__': unittest.main() diff --git a/test/test_string_perfdata.py b/test/test_string_perfdata.py index 03a1e0cdca..629138ed50 100644 --- a/test/test_string_perfdata.py +++ b/test/test_string_perfdata.py @@ -77,6 +77,18 @@ def test_string_percent_minmax_echo(self): 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'))) + def test_string_quoted_names(self): + self.assertEqual("ram''used''=10", str(Metric("ram''used''=10"))) + self.assertEqual("ram''used=10", str(Metric("ram''used=10"))) + # Outer quotes present but not necessary should be stripped on format + self.assertEqual("ram''used=10", str(Metric("'ram''used'=10"))) + # But not so if there is also a space + self.assertEqual("'ram was ''used'=10", str(Metric("'ram was ''used'=10"))) + + # String missing required outer quotes, should come back quoted + # This is debatable as it basically is an invalid metric/perfdata + self.assertEqual("'ram ''used'''=10", str(Metric("ram ''used''=10"))) + if __name__ == '__main__': unittest.main()