From aaa4440fd31715084da66f2f9c24de579131ad94 Mon Sep 17 00:00:00 2001 From: Kaiyan Sheng Date: Thu, 30 Nov 2017 14:50:46 -0700 Subject: [PATCH] Use monasca-common to validate metrics Change-Id: I2f52cef518c492cf73ff8310d343363f2863b390 --- monasca_agent/common/aggregator.py | 99 +++--------------------------- requirements.txt | 1 + tests/test_aggregator.py | 72 +++++++++------------- 3 files changed, 36 insertions(+), 136 deletions(-) diff --git a/monasca_agent/common/aggregator.py b/monasca_agent/common/aggregator.py index e0afc2c8..59232341 100644 --- a/monasca_agent/common/aggregator.py +++ b/monasca_agent/common/aggregator.py @@ -1,13 +1,11 @@ -# (C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP +# (C) Copyright 2015-2017 Hewlett Packard Enterprise Development LP """ Aggregation classes used by the collector and statsd to batch messages sent to the forwarder. """ import json import logging -import re from time import time -import monasca_agent.common.metrics as metrics_pkg - +import monasca_common.validation.metrics as metric_validator log = logging.getLogger(__name__) @@ -18,33 +16,7 @@ log = logging.getLogger(__name__) # does not support submitting values for the past, and all values get # submitted for the timestamp passed into the flush() function. RECENT_POINT_THRESHOLD_DEFAULT = 3600 -VALUE_META_MAX_NUMBER = 16 VALUE_META_VALUE_MAX_LENGTH = 2048 -VALUE_META_NAME_MAX_LENGTH = 255 - -invalid_chars = "<>={}(),\"\\\\;&" -restricted_dimension_chars = re.compile('[' + invalid_chars + ']') -restricted_name_chars = re.compile('[' + invalid_chars + ' ' + ']') - - -class InvalidMetricName(Exception): - pass - - -class InvalidDimensionKey(Exception): - pass - - -class InvalidDimensionValue(Exception): - pass - - -class InvalidValue(Exception): - pass - - -class InvalidValueMeta(Exception): - pass class MetricsAggregator(object): @@ -94,76 +66,19 @@ class MetricsAggregator(object): return 0 return round(float(self.count) / interval, 2) - def _valid_value_meta(self, value_meta, name, dimensions): - if len(value_meta) > VALUE_META_MAX_NUMBER: - msg = "Too many valueMeta entries {0}, limit is {1}: {2} -> {3} valueMeta {4}" - log.error(msg.format(len(value_meta), VALUE_META_MAX_NUMBER, name, dimensions, value_meta)) - return False - for key, value in value_meta.items(): - if not key: - log.error("valueMeta name cannot be empty: {0} -> {1}".format(name, dimensions)) - return False - if len(key) > VALUE_META_NAME_MAX_LENGTH: - msg = "valueMeta name {0} must be {1} characters or less: {2} -> {3}" - log.error(msg.format(key, VALUE_META_NAME_MAX_LENGTH, name, dimensions)) - return False - - try: - if get_value_meta_overage(value_meta): - msg = "valueMeta name value combinations must be {0} characters or less: {1} -> {2} valueMeta {3}" - log.error(msg.format(VALUE_META_VALUE_MAX_LENGTH, name, dimensions, value_meta)) - return False - except Exception: - log.error("Unable to serialize valueMeta into JSON: {2} -> {3}".format(name, dimensions)) - return False - - return True - def submit_metric(self, name, value, metric_class, dimensions=None, delegated_tenant=None, hostname=None, device_name=None, value_meta=None, timestamp=None, sample_rate=1): + # validate dimensions, name, value and value meta if dimensions: - for k, v in dimensions.items(): - if not isinstance(k, (str, unicode)): - log.error("invalid dimension key {0} must be a string: {1} -> {2}".format(k, name, dimensions)) - raise InvalidDimensionKey - if len(k) > 255 or len(k) < 1: - log.error("invalid length for dimension key {0}: {1} -> {2}".format(k, name, dimensions)) - raise InvalidDimensionKey - if restricted_dimension_chars.search(k) or re.match('^_', k): - log.error("invalid characters in dimension key {0}: {1} -> {2}".format(k, name, dimensions)) - raise InvalidDimensionKey + metric_validator.validate_dimensions(dimensions) - if not isinstance(v, (str, unicode)): - log.error("invalid dimension value {0} for key {1} must be a string: {2} -> {3}".format(v, k, name, - dimensions)) - raise InvalidDimensionValue - if len(v) > 255 or len(v) < 1: - log.error("invalid length dimension value {0} for key {1}: {2} -> {3}".format(v, k, name, - dimensions)) - raise InvalidDimensionValue - if restricted_dimension_chars.search(v): - log.error("invalid characters in dimension value {0} for key {1}: {2} -> {3}".format(v, k, name, - dimensions)) - raise InvalidDimensionValue + metric_validator.validate_name(name) - if not isinstance(name, (str, unicode)): - log.error("invalid metric name must be a string: {0} -> {1}".format(name, dimensions)) - raise InvalidMetricName - if len(name) > 255 or len(name) < 1: - log.error("invalid length for metric name: {0} -> {1}".format(name, dimensions)) - raise InvalidMetricName - if restricted_name_chars.search(name): - log.error("invalid characters in metric name: {0} -> {1}".format(name, dimensions)) - raise InvalidMetricName - - if not isinstance(value, (int, long, float)): - log.error("invalid value {0} is not of number type for metric {1}".format(value, name)) - raise InvalidValue + metric_validator.validate_value(value) if value_meta: - if not self._valid_value_meta(value_meta, name, dimensions): - raise InvalidValueMeta + metric_validator.validate_value_meta(value_meta) hostname_to_post = self.get_hostname_to_post(hostname) diff --git a/requirements.txt b/requirements.txt index e35bf83b..abf50ec3 100644 --- a/requirements.txt +++ b/requirements.txt @@ -27,3 +27,4 @@ futures>=3.0.0;python_version=='2.7' or python_version=='2.6' # BSD # https://github.com/eventlet/eventlet/issues/401 is resolved eventlet!=0.18.3,!=0.20.1,<0.21.0,>=0.18.2 # MIT keystoneauth1>=3.2.0 # Apache-2.0 +monasca-common>=1.4.0 # Apache-2.0 diff --git a/tests/test_aggregator.py b/tests/test_aggregator.py index ed5fa1fb..1c8ea3f0 100644 --- a/tests/test_aggregator.py +++ b/tests/test_aggregator.py @@ -1,16 +1,18 @@ -# (C) Copyright 2015-2016 Hewlett Packard Enterprise Development Company LP +# (C) Copyright 2015-2017 Hewlett Packard Enterprise Development Company LP import unittest import monasca_agent.common.aggregator as aggregator import monasca_agent.common.metrics as metrics_pkg +import monasca_common.validation.metrics as metric_validator + # a few valid characters to test valid_name_chars = ".'_-" -invalid_name_chars = " <>={}(),\"\\\\;&" +invalid_name_chars = metric_validator.INVALID_CHARS # a few valid characters to test valid_dimension_chars = " .'_-" -invalid_dimension_chars = "<>={}(),\"\\\\;&" +invalid_dimension_chars = metric_validator.INVALID_CHARS class TestMetricsAggregator(unittest.TestCase): @@ -80,7 +82,7 @@ class TestMetricsAggregator(unittest.TestCase): 5, dimensions=dimensions, value_meta=value_meta, - exception=aggregator.InvalidMetricName) + exception=metric_validator.InvalidMetricName) def testInvalidMetricNameEmpty(self): dimensions = {'A': 'B', 'B': 'C', 'D': 'E'} @@ -89,7 +91,7 @@ class TestMetricsAggregator(unittest.TestCase): 5, dimensions=dimensions, value_meta=value_meta, - exception=aggregator.InvalidMetricName) + exception=metric_validator.InvalidMetricName) def testInvalidMetricNameNonStr(self): dimensions = {'A': 'B', 'B': 'C', 'D': 'E'} @@ -98,7 +100,7 @@ class TestMetricsAggregator(unittest.TestCase): 5, dimensions=dimensions, value_meta=value_meta, - exception=aggregator.InvalidMetricName) + exception=metric_validator.InvalidMetricName) def testInvalidMetricRestrictedCharacters(self): dimensions = {'A': 'B', 'B': 'C', 'D': 'E'} @@ -107,7 +109,7 @@ class TestMetricsAggregator(unittest.TestCase): 5, dimensions=dimensions, value_meta=value_meta, - exception=aggregator.InvalidMetricName) + exception=metric_validator.InvalidMetricName) def testInvalidDimensionEmptyKey(self): dimensions = {'A': 'B', '': 'C', 'D': 'E'} @@ -116,7 +118,7 @@ class TestMetricsAggregator(unittest.TestCase): 5, dimensions=dimensions, value_meta=value_meta, - exception=aggregator.InvalidDimensionKey) + exception=metric_validator.InvalidDimensionKey) def testInvalidDimensionEmptyValue(self): dimensions = {'A': 'B', 'B': 'C', 'D': ''} @@ -125,7 +127,7 @@ class TestMetricsAggregator(unittest.TestCase): 5, dimensions=dimensions, value_meta=value_meta, - exception=aggregator.InvalidDimensionValue) + exception=metric_validator.InvalidDimensionValue) def testInvalidDimensionNonStrKey(self): dimensions = {'A': 'B', 4: 'C', 'D': 'E'} @@ -134,7 +136,7 @@ class TestMetricsAggregator(unittest.TestCase): 5, dimensions=dimensions, value_meta=value_meta, - exception=aggregator.InvalidDimensionKey) + exception=metric_validator.InvalidDimensionKey) def testInvalidDimensionNonStrValue(self): dimensions = {'A': 13.3, 'B': 'C', 'D': 'E'} @@ -143,7 +145,7 @@ class TestMetricsAggregator(unittest.TestCase): 5, dimensions=dimensions, value_meta=value_meta, - exception=aggregator.InvalidDimensionValue) + exception=metric_validator.InvalidDimensionValue) def testInvalidDimensionKeyLength(self): dimensions = {'A'*256: 'B', 'B': 'C', 'D': 'E'} @@ -153,7 +155,7 @@ class TestMetricsAggregator(unittest.TestCase): 5, dimensions=dimensions, value_meta=value_meta, - exception=aggregator.InvalidDimensionKey) + exception=metric_validator.InvalidDimensionKey) def testInvalidDimensionValueLength(self): dimensions = {'A': 'B', 'B': 'C'*256, 'D': 'E'} @@ -162,7 +164,7 @@ class TestMetricsAggregator(unittest.TestCase): 5, dimensions=dimensions, value_meta=value_meta, - exception=aggregator.InvalidDimensionValue) + exception=metric_validator.InvalidDimensionValue) def testInvalidDimensionKeyRestrictedCharacters(self): dimensions = {'A': 'B', 'B': 'C', '(D)': 'E'} @@ -171,7 +173,7 @@ class TestMetricsAggregator(unittest.TestCase): 5, dimensions=dimensions, value_meta=value_meta, - exception=aggregator.InvalidDimensionKey) + exception=metric_validator.InvalidDimensionKey) def testInvalidDimensionValueRestrictedCharacters(self): dimensions = {'A': 'B;', 'B': 'C', 'D': 'E'} @@ -180,7 +182,7 @@ class TestMetricsAggregator(unittest.TestCase): 5, dimensions=dimensions, value_meta=value_meta, - exception=aggregator.InvalidDimensionValue) + exception=metric_validator.InvalidDimensionValue) def testInvalidDimensionKeyLeadingUnderscore(self): dimensions = {'_A': 'B', 'B': 'C', 'D': 'E'} @@ -189,7 +191,7 @@ class TestMetricsAggregator(unittest.TestCase): 5, dimensions=dimensions, value_meta=value_meta, - exception=aggregator.InvalidDimensionKey) + exception=metric_validator.InvalidDimensionKey) def testInvalidValue(self): dimensions = {'A': 'B', 'B': 'C', 'D': 'E'} @@ -198,7 +200,7 @@ class TestMetricsAggregator(unittest.TestCase): "value", dimensions=dimensions, value_meta=value_meta, - exception=aggregator.InvalidValue) + exception=metric_validator.InvalidValue) def testValidNameChars(self): for c in valid_name_chars: @@ -209,7 +211,7 @@ class TestMetricsAggregator(unittest.TestCase): for c in invalid_name_chars: self.submit_metric('test{}counter'.format(c), 2, dimensions={"test-key": "test-value"}, - exception=aggregator.InvalidMetricName) + exception=metric_validator.InvalidMetricName) def testValidDimensionChars(self): for c in valid_dimension_chars: @@ -220,10 +222,10 @@ class TestMetricsAggregator(unittest.TestCase): for c in invalid_dimension_chars: self.submit_metric('test-counter', 2, dimensions={'test{}key'.format(c): 'test-value'}, - exception=aggregator.InvalidDimensionKey) + exception=metric_validator.InvalidDimensionKey) self.submit_metric('test-counter', 2, dimensions={'test-key': 'test{}value'.format(c)}, - exception=aggregator.InvalidDimensionValue) + exception=metric_validator.InvalidDimensionValue) def testTooManyValueMeta(self): dimensions = {'A': 'B', 'B': 'C', 'D': 'E'} @@ -234,7 +236,7 @@ class TestMetricsAggregator(unittest.TestCase): 2, dimensions=dimensions, value_meta=value_meta, - exception=aggregator.InvalidValueMeta) + exception=metric_validator.InvalidValueMeta) def testEmptyValueMetaKey(self): dimensions = {'A': 'B', 'B': 'C', 'D': 'E'} @@ -243,21 +245,12 @@ class TestMetricsAggregator(unittest.TestCase): 2, dimensions=dimensions, value_meta=value_meta, - exception=aggregator.InvalidValueMeta) - - def testEmptyValueMetaKey(self): - dimensions = {'A': 'B', 'B': 'C', 'D': 'E'} - value_meta = {'': 'BBB'} - self.submit_metric("Foo", - 2, - dimensions=dimensions, - value_meta=value_meta, - exception=aggregator.InvalidValueMeta) + exception=metric_validator.InvalidValueMeta) def testTooLongValueMetaKey(self): dimensions = {'A': 'B', 'B': 'C', 'D': 'E'} key = "K" - for i in range(0, aggregator.VALUE_META_NAME_MAX_LENGTH): + for i in range(0, metric_validator.VALUE_META_NAME_MAX_LENGTH): key = "{}{}".format(key, "1") value_meta = {key: 'BBB'} print(key) @@ -265,22 +258,13 @@ class TestMetricsAggregator(unittest.TestCase): 2, dimensions=dimensions, value_meta=value_meta, - exception=aggregator.InvalidValueMeta) - - def testEmptyValueMetaKey(self): - dimensions = {'A': 'B', 'B': 'C', 'D': 'E'} - value_meta = {'': 'BBB'} - self.submit_metric("Foo", - 2, - dimensions=dimensions, - value_meta=value_meta, - exception=aggregator.InvalidValueMeta) + exception=metric_validator.InvalidValueMeta) def testTooLargeValueMeta(self): dimensions = {'A': 'B', 'B': 'C', 'D': 'E'} value_meta_value = "" num_value_meta = 10 - for i in range(0, aggregator.VALUE_META_VALUE_MAX_LENGTH/num_value_meta): + for i in range(0, metric_validator.VALUE_META_VALUE_MAX_LENGTH/num_value_meta): value_meta_value = '{}{}'.format(value_meta_value, '1') value_meta = {} @@ -290,4 +274,4 @@ class TestMetricsAggregator(unittest.TestCase): 2, dimensions=dimensions, value_meta=value_meta, - exception=aggregator.InvalidValueMeta) + exception=metric_validator.InvalidValueMeta)