Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Proposing a way to customize aggregations and percentils for histograms #3238

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Proposing a way to customize aggregations and percentils for histograms #3238

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 17 additions & 6 deletions aggregator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,17 +232,28 @@ def __init__(self, formatter, name, tags, hostname, device_name, extra_config=No
self.name = name
self.count = 0
self.samples = []
self.aggregates = extra_config['aggregates'] if\
extra_config is not None and extra_config.get('aggregates') is not None\
else DEFAULT_HISTOGRAM_AGGREGATES
self.percentiles = extra_config['percentiles'] if\
extra_config is not None and extra_config.get('percentiles') is not None\
else DEFAULT_HISTOGRAM_PERCENTILES
self.aggregates = self._extract_relevant_config(extra_config, 'aggregates',
DEFAULT_HISTOGRAM_AGGREGATES)
self.percentiles = self._extract_relevant_config(extra_config, 'percentiles',
DEFAULT_HISTOGRAM_PERCENTILES)
self.tags = tags
self.hostname = hostname
self.device_name = device_name
self.last_sample_time = None

def _extract_relevant_config(self, extra_config, key, default):
if extra_config is None or extra_config.get(key) is None:
return default

# let's try and see if we have a regex matching our name
for regex, config in extra_config[key]:
if regex is None:
default = config
elif regex.search(self.name):
return config

return default

def sample(self, value, sample_rate, timestamp=None):
self.count += int(1 / sample_rate)
self.samples.append(value)
Expand Down
93 changes: 60 additions & 33 deletions config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import platform
import re
from socket import gaierror, gethostbyname
import sre_constants
import string
import sys
import traceback
Expand Down Expand Up @@ -244,52 +245,78 @@ def get_default_bind_host():


def get_histogram_aggregates(configstr=None):
if configstr is None:
return None
return _get_histogram_config(configstr, 'aggregate', _parse_histogram_aggregates_values)

try:
vals = configstr.split(',')
valid_values = ['min', 'max', 'median', 'avg', 'sum', 'count']
result = []
def _parse_histogram_aggregates_values(vals):
result = []
valid_values = ['min', 'max', 'median', 'avg', 'sum', 'count']

for val in vals:
val = val.strip()
if val not in valid_values:
log.warning("Ignored histogram aggregate {0}, invalid".format(val))
continue
else:
result.append(val)
except Exception:
log.exception("Error when parsing histogram aggregates, skipping")
return None
for val in vals:
val = val.strip()
if val not in valid_values:
log.warning("Ignored histogram aggregate {0}, invalid".format(val))
continue
else:
result.append(val)

return result


def get_histogram_percentiles(configstr=None):
return _get_histogram_config(configstr, 'percentile', _parse_histogram_percentiles_values)

def _parse_histogram_percentiles_values(vals):
result = []

for val in vals:
try:
val = val.strip()
floatval = float(val)
if floatval <= 0 or floatval >= 1:
raise ValueError
if len(val) > 4:
log.warning("Histogram percentiles are rounded to 2 digits: {0} rounded"
.format(floatval))
result.append(float(val[0:4]))
except ValueError:
log.warning("Bad histogram percentile value {0}, must be float in ]0;1[, skipping"
.format(val))

return result

def _get_histogram_config(configstr, type_str, parse_callback):
if configstr is None:
return None

result = []
try:
vals = configstr.split(',')
for val in vals:
try:
val = val.strip()
floatval = float(val)
if floatval <= 0 or floatval >= 1:
raise ValueError
if len(val) > 4:
log.warning("Histogram percentiles are rounded to 2 digits: {0} rounded"
.format(floatval))
result.append(float(val[0:4]))
except ValueError:
log.warning("Bad histogram percentile value {0}, must be float in ]0;1[, skipping"
.format(val))
return _parse_histogram_config(configstr, type_str, parse_callback)
except Exception:
log.exception("Error when parsing histogram percentiles, skipping")
log.exception('Error when parsing histogram {0}s, skipping'.format(type_str))
return None

def _parse_histogram_config(configstr, type_str, parse_callback):
groups = configstr.split(';')

result = []
for group in groups:
group_config = group.rsplit(':', 1)
if len(group_config) == 1:
# general config
result.append((None, parse_callback(group.split(','))))
elif len(group_config) == 2:
# only applies to metrics matching a given regex
regex = group_config[0].strip()
if not regex:
log.warning('Ignoring empty regex for histogram {0}s'.format(type_str))
continue
try:
compiled_regex = re.compile(regex)
except sre_constants.error as exception:
log.warning('Ignoring invalid regex {0} for histogram {1}s: {2}'
.format(regex, type_str, exception.message))
continue

result.append((compiled_regex, parse_callback(group_config[1].split(','))))

return result


Expand Down
18 changes: 9 additions & 9 deletions tests/core/test_aggregator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ def test_histogram_normalization(self):
stats = MetricsAggregator(
'myhost',
interval=10,
histogram_aggregates=DEFAULT_HISTOGRAM_AGGREGATES+['min']
histogram_aggregates=[(None, DEFAULT_HISTOGRAM_AGGREGATES+['min'])]
)
for i in range(5):
stats.submit_packets('h1:1|h')
Expand Down Expand Up @@ -342,7 +342,7 @@ def test_histogram(self):
# The min is not enabled by default
stats = MetricsAggregator(
'myhost',
histogram_aggregates=DEFAULT_HISTOGRAM_AGGREGATES+['min']
histogram_aggregates=[(None, DEFAULT_HISTOGRAM_AGGREGATES+['min'])]
)

# Sample all numbers between 1-100 many times. This
Expand Down Expand Up @@ -377,7 +377,7 @@ def test_sampled_histogram(self):
# The min is not enabled by default
stats = MetricsAggregator(
'myhost',
histogram_aggregates=DEFAULT_HISTOGRAM_AGGREGATES+['min']
histogram_aggregates=[(None, DEFAULT_HISTOGRAM_AGGREGATES+['min'])]
)
stats.submit_packets('sampled.hist:5|h|@0.5')

Expand Down Expand Up @@ -410,13 +410,13 @@ def test_monokey_batching_notags(self):
# The min is not enabled by default
stats = MetricsAggregator(
'host',
histogram_aggregates=DEFAULT_HISTOGRAM_AGGREGATES+['min']
histogram_aggregates=[(None, DEFAULT_HISTOGRAM_AGGREGATES+['min'])]
)
stats.submit_packets('test_hist:0.3|ms:2.5|ms|@0.5:3|ms')

stats_ref = MetricsAggregator(
'host',
histogram_aggregates=DEFAULT_HISTOGRAM_AGGREGATES+['min']
histogram_aggregates=[(None, DEFAULT_HISTOGRAM_AGGREGATES+['min'])]
)
packets = [
'test_hist:0.3|ms',
Expand Down Expand Up @@ -458,13 +458,13 @@ def test_monokey_batching_withtags_with_sampling(self):
# The min is not enabled by default
stats = MetricsAggregator(
'host',
histogram_aggregates=DEFAULT_HISTOGRAM_AGGREGATES+['min']
histogram_aggregates=[(None, DEFAULT_HISTOGRAM_AGGREGATES+['min'])]
)
stats.submit_packets('test_metric:1.5|c|#tag1:one,tag2:two:2.3|g|#tag3:three:3|g:42|h|#tag1:12,tag42:42|@0.22')

stats_ref = MetricsAggregator(
'host',
histogram_aggregates=DEFAULT_HISTOGRAM_AGGREGATES+['min']
histogram_aggregates=[(None, DEFAULT_HISTOGRAM_AGGREGATES+['min'])]
)
packets = [
'test_metric:1.5|c|#tag1:one,tag2:two',
Expand Down Expand Up @@ -517,7 +517,7 @@ def test_metrics_expiry(self):
'myhost',
interval=ag_interval,
expiry_seconds=expiry,
histogram_aggregates=DEFAULT_HISTOGRAM_AGGREGATES+['min']
histogram_aggregates=[(None, DEFAULT_HISTOGRAM_AGGREGATES+['min'])]
)
stats.submit_packets('test.counter:123|c')
stats.submit_packets('test.gauge:55|g')
Expand Down Expand Up @@ -779,7 +779,7 @@ def test_recent_point_threshold(self):
stats = MetricsAggregator(
'myhost',
recent_point_threshold=threshold,
histogram_aggregates=DEFAULT_HISTOGRAM_AGGREGATES+['min']
histogram_aggregates=[(None, DEFAULT_HISTOGRAM_AGGREGATES+['min'])]
)
timestamp_beyond_threshold = time.time() - threshold*2
timestamp_within_threshold = time.time() - threshold/2
Expand Down
14 changes: 7 additions & 7 deletions tests/core/test_bucket_aggregator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ def test_histogram_normalization(self):
ag_interval = 10
# The min is not enabled by default
stats = MetricsBucketAggregator('myhost', interval=ag_interval,
histogram_aggregates=DEFAULT_HISTOGRAM_AGGREGATES+['min'])
histogram_aggregates=[(None, DEFAULT_HISTOGRAM_AGGREGATES+['min'])])
for i in range(5):
stats.submit_packets('h1:1|h')
for i in range(20):
Expand Down Expand Up @@ -551,7 +551,7 @@ def test_histogram(self):
ag_interval = self.interval
# The min is not enabled by default
stats = MetricsBucketAggregator('myhost', interval=ag_interval,
histogram_aggregates=DEFAULT_HISTOGRAM_AGGREGATES+['min'])
histogram_aggregates=[(None, DEFAULT_HISTOGRAM_AGGREGATES+['min'])])
self.wait_for_bucket_boundary(ag_interval)

# Sample all numbers between 1-100 many times. This
Expand Down Expand Up @@ -589,7 +589,7 @@ def test_sampled_histogram(self):
stats = MetricsBucketAggregator(
'myhost',
interval=self.interval,
histogram_aggregates=DEFAULT_HISTOGRAM_AGGREGATES+['min']
histogram_aggregates=[(None, DEFAULT_HISTOGRAM_AGGREGATES+['min'])]
)
stats.submit_packets('sampled.hist:5|h|@0.5')

Expand All @@ -607,7 +607,7 @@ def test_histogram_buckets(self):
ag_interval = 1
# The min is not enabled by default
stats = MetricsBucketAggregator('myhost', interval=ag_interval,
histogram_aggregates=DEFAULT_HISTOGRAM_AGGREGATES+['min'])
histogram_aggregates=[(None, DEFAULT_HISTOGRAM_AGGREGATES+['min'])])

# Sample all numbers between 1-100 many times. This
# means our percentiles should be relatively close to themselves.
Expand Down Expand Up @@ -665,7 +665,7 @@ def test_histogram_flush_during_bucket(self):
ag_interval = 1
# The min is not enabled by default
stats = MetricsBucketAggregator('myhost', interval=ag_interval,
histogram_aggregates=DEFAULT_HISTOGRAM_AGGREGATES+['min'])
histogram_aggregates=[(None, DEFAULT_HISTOGRAM_AGGREGATES+['min'])])

# Sample all numbers between 1-100 many times. This
# means our percentiles should be relatively close to themselves.
Expand Down Expand Up @@ -768,7 +768,7 @@ def test_metrics_expiry(self):
# The min is not enabled by default
stats = MetricsBucketAggregator('myhost', interval=ag_interval,
expiry_seconds=expiry,
histogram_aggregates=DEFAULT_HISTOGRAM_AGGREGATES+['min'])
histogram_aggregates=[(None, DEFAULT_HISTOGRAM_AGGREGATES+['min'])])
stats.submit_packets('test.counter:123|c')
stats.submit_packets('test.gauge:55|g')
stats.submit_packets('test.set:44|s')
Expand Down Expand Up @@ -974,7 +974,7 @@ def test_recent_point_threshold(self):
'myhost',
recent_point_threshold=threshold,
interval=ag_interval,
histogram_aggregates=DEFAULT_HISTOGRAM_AGGREGATES+['min']
histogram_aggregates=[(None, DEFAULT_HISTOGRAM_AGGREGATES+['min'])]
)
timestamp_beyond_threshold = time.time() - threshold*2

Expand Down
Loading