Skip to content
Permalink
Browse files

the aggregator name can now match regular carbon naming (#544)


added Aggregator.from_name that supports both bg and carbon names
added unittests on MetricMetadata.from_string_dict
  • Loading branch information
rbizos committed Mar 25, 2020
1 parent 921fe2e commit db3894741e82f99fc764c9643cb3ac2391ccfdb2
Showing with 81 additions and 42 deletions.
  1. +17 −7 biggraphite/metric.py
  2. +64 −35 tests/test_metric.py
@@ -305,28 +305,38 @@ def _aggregate_total(self, values, counts, values_newest_first):

@classmethod
def from_carbon_name(cls, name):
"""Make an instance from a carbon-like name."""
"""Make an Aggregator instance from a carbon-like name (sum, min, max ...)."""
if not name:
return None
try:
return cls(name)
except ValueError:
raise InvalidArgumentError("Unknown carbon aggregation: %s" % name)

for agg in cls:
if agg.value is name:
return agg

raise InvalidArgumentError("Unknown carbon aggregation: %s" % name)

def carbon_name(self):
"""Returns the carbon name of this aggregator."""
return self.value

@classmethod
def from_config_name(cls, name):
"""Make an instance from a BigGraphite name."""
"""Make an Aggregator instance from a BigGraphite name (total, minimum ...)."""
if not name:
return None
try:
return cls[name]
except KeyError:
raise InvalidArgumentError("Unknown BG aggregator: %s" % name)

@classmethod
def from_name(cls, name):
"""Make an Aggregator instance from either BigGraphite name or graphite name."""
try:
return cls.from_config_name(name)
except InvalidArgumentError:
return cls.from_carbon_name(name)

@staticmethod
def __sum_and_count(values, counts):
total = 0.0
@@ -710,7 +720,7 @@ def from_string_dict(cls, d):
return cls.create()

return cls.create(
aggregator=Aggregator.from_config_name(d.get("aggregator")),
aggregator=Aggregator.from_name(d.get("aggregator")),
retention=Retention.from_string(d.get("retention")),
carbon_xfilesfactor=float(d.get("carbon_xfilesfactor")),
)
@@ -68,58 +68,62 @@ def test_create_should_define_default_values_when_no_parameters_given(self):

def test_metadata_object_should_be_the_same_when_created_with_same_parameters(self):
"""MetricMetadata.create() should always return the same MetricMetadata instance."""
metadata1 = bg_metric.MetricMetadata.create(self.DEFAULT_AGGREGATOR,
self.DEFAULT_RETENTION,
self.DEFAULT_XFACTOR)
metadata2 = bg_metric.MetricMetadata.create(self.DEFAULT_AGGREGATOR,
self.DEFAULT_RETENTION,
self.DEFAULT_XFACTOR)
metadata1 = bg_metric.MetricMetadata.create(
self.DEFAULT_AGGREGATOR, self.DEFAULT_RETENTION, self.DEFAULT_XFACTOR
)
metadata2 = bg_metric.MetricMetadata.create(
self.DEFAULT_AGGREGATOR, self.DEFAULT_RETENTION, self.DEFAULT_XFACTOR
)
self.assertIs(metadata1, metadata2)

# the returned instance should't depend on the order of the parameters
metadata3 = bg_metric.MetricMetadata.create(carbon_xfilesfactor=self.DEFAULT_XFACTOR,
aggregator=self.DEFAULT_AGGREGATOR,
retention=self.DEFAULT_RETENTION)
metadata3 = bg_metric.MetricMetadata.create(
carbon_xfilesfactor=self.DEFAULT_XFACTOR,
aggregator=self.DEFAULT_AGGREGATOR,
retention=self.DEFAULT_RETENTION,
)
self.assertIs(metadata1, metadata3)

def test_xfilesfactor_should_have_no_impact_on_uniqueness(self):
"""Check that carbon_xfilesfactor has no influence on uniqueness."""
metadata1 = bg_metric.MetricMetadata.create(self.DEFAULT_AGGREGATOR,
self.DEFAULT_RETENTION,
self.DEFAULT_XFACTOR)
metadata2 = bg_metric.MetricMetadata.create(self.DEFAULT_AGGREGATOR,
self.DEFAULT_RETENTION,
self.DEFAULT_XFACTOR - 0.1)
metadata1 = bg_metric.MetricMetadata.create(
self.DEFAULT_AGGREGATOR, self.DEFAULT_RETENTION, self.DEFAULT_XFACTOR
)
metadata2 = bg_metric.MetricMetadata.create(
self.DEFAULT_AGGREGATOR, self.DEFAULT_RETENTION, self.DEFAULT_XFACTOR - 0.1
)

self.assertIsNot(metadata1, metadata2)

def test_aggregator_should_have_no_impact_on_uniqueness(self):
"""Check that aggregator has no influence on uniqueness."""
metadata1 = bg_metric.MetricMetadata.create(self.DEFAULT_AGGREGATOR,
self.DEFAULT_RETENTION,
self.DEFAULT_XFACTOR)
metadata2 = bg_metric.MetricMetadata.create(bg_metric.Aggregator.total,
self.DEFAULT_RETENTION,
self.DEFAULT_XFACTOR)
metadata1 = bg_metric.MetricMetadata.create(
self.DEFAULT_AGGREGATOR, self.DEFAULT_RETENTION, self.DEFAULT_XFACTOR
)
metadata2 = bg_metric.MetricMetadata.create(
bg_metric.Aggregator.total, self.DEFAULT_RETENTION, self.DEFAULT_XFACTOR
)

self.assertIsNot(metadata1, metadata2)

def test_retention_should_have_no_impact_on_uniqueness(self):
"""Check that retention has no influence on uniqueness."""
metadata1 = bg_metric.MetricMetadata.create(self.DEFAULT_AGGREGATOR,
self.DEFAULT_RETENTION,
self.DEFAULT_XFACTOR)
metadata2 = bg_metric.MetricMetadata.create(self.DEFAULT_AGGREGATOR,
self.TEST_RETENTION,
self.DEFAULT_XFACTOR)
metadata1 = bg_metric.MetricMetadata.create(
self.DEFAULT_AGGREGATOR, self.DEFAULT_RETENTION, self.DEFAULT_XFACTOR
)
metadata2 = bg_metric.MetricMetadata.create(
self.DEFAULT_AGGREGATOR, self.TEST_RETENTION, self.DEFAULT_XFACTOR
)

self.assertIsNot(metadata1, metadata2)

def test_metadata_object_should_be_deleted_when_there_is_no_more_references_on_it(self):
def test_metadata_object_should_be_deleted_when_there_is_no_more_references_on_it(
self,
):
"""Check that a metadata are properly cleaned-up when no one holds a reference on it."""
metadata = bg_metric.MetricMetadata.create(self.DEFAULT_AGGREGATOR,
self.TEST_RETENTION,
self.DEFAULT_XFACTOR)
metadata = bg_metric.MetricMetadata.create(
self.DEFAULT_AGGREGATOR, self.TEST_RETENTION, self.DEFAULT_XFACTOR
)

metadata_weak = weakref.ref(metadata)
self.assertIsNotNone(metadata_weak())
@@ -132,19 +136,44 @@ def test_metadata_object_should_be_deleted_when_there_is_no_more_references_on_i
def test_total_metadata_object_count_should_be_reported_by_prometheus_client(self):
"""Test the gauge reporting the number of metadata hold in the internal dictionnary."""
def get_metadata_instances_count():
return prometheus_client.REGISTRY.get_sample_value('bg_metadata_instances_count')
return prometheus_client.REGISTRY.get_sample_value(
"bg_metadata_instances_count"
)

# make sure previously allocated metadata are cleaned up before starting the test
gc.collect()

# allocate a new metadata object
metadata_count_before = get_metadata_instances_count()
metadata = bg_metric.MetricMetadata.create(self.DEFAULT_AGGREGATOR,
self.TEST_RETENTION,
self.DEFAULT_XFACTOR)
metadata = bg_metric.MetricMetadata.create(
self.DEFAULT_AGGREGATOR, self.TEST_RETENTION, self.DEFAULT_XFACTOR
)
self.assertEqual(get_metadata_instances_count() - metadata_count_before, 1)

# delete the reference on the metadata object
del metadata
gc.collect()
self.assertEqual(get_metadata_instances_count(), metadata_count_before)

def test_from_dict_string(self):
parameters = (
(
{
"aggregator": "sum",
"retention": "86400*1s:10080*60s",
"carbon_xfilesfactor": 0.5,
},
bg_metric.Aggregator.total,
),
(
{
"aggregator": "total",
"retention": "86400*1s:10080*60s",
"carbon_xfilesfactor": 0.5,
},
bg_metric.Aggregator.total,
),
)
for parameter in parameters:
metadata = bg_metric.MetricMetadata.from_string_dict(parameter[0])
self.assertEqual(metadata.aggregator, parameter[1])

0 comments on commit db38947

Please sign in to comment.
You can’t perform that action at this time.