Skip to content

Commit

Permalink
Allow for negtive histogram buckets.
Browse files Browse the repository at this point in the history
Per OM discussions. I also extended this to _gsum,
where as it's a gauge we can allow a negative value
if there's a negative bucket.

Signed-off-by: Brian Brazil <brian.brazil@robustperception.io>
  • Loading branch information
brian-brazil committed Dec 19, 2019
1 parent 3ab0374 commit 668fd0c
Show file tree
Hide file tree
Showing 8 changed files with 86 additions and 14 deletions.
3 changes: 2 additions & 1 deletion prometheus_client/metrics.py
Expand Up @@ -566,7 +566,8 @@ def _child_samples(self):
acc += self._buckets[i].get()
samples.append(('_bucket', {'le': floatToGoString(bound)}, acc))
samples.append(('_count', {}, acc))
samples.append(('_sum', {}, self._sum.get()))
if self._upper_bounds[0] >= 0:
samples.append(('_sum', {}, self._sum.get()))
samples.append(('_created', {}, self._created))
return tuple(samples)

Expand Down
15 changes: 9 additions & 6 deletions prometheus_client/metrics_core.py
Expand Up @@ -183,8 +183,8 @@ class HistogramMetricFamily(Metric):

def __init__(self, name, documentation, buckets=None, sum_value=None, labels=None, unit=''):
Metric.__init__(self, name, documentation, 'histogram', unit)
if (sum_value is None) != (buckets is None):
raise ValueError('buckets and sum_value must be provided together.')
if sum_value is not None and buckets is None:
raise ValueError('sum value cannot be provided without buckets.')
if labels is not None and buckets is not None:
raise ValueError('Can only specify at most one of buckets and labels.')
if labels is None:
Expand Down Expand Up @@ -217,10 +217,13 @@ def add_metric(self, labels, buckets, sum_value, timestamp=None):
exemplar,
))
# +Inf is last and provides the count value.
self.samples.extend([
Sample(self.name + '_count', dict(zip(self._labelnames, labels)), buckets[-1][1], timestamp),
Sample(self.name + '_sum', dict(zip(self._labelnames, labels)), sum_value, timestamp),
])
self.samples.append(
Sample(self.name + '_count', dict(zip(self._labelnames, labels)), buckets[-1][1], timestamp))
# Don't iunclude sum if there's negative buckets.
if float(buckets[0][0]) >= 0 and sum_value is not None:
self.samples.append(
Sample(self.name + '_sum', dict(zip(self._labelnames, labels)), sum_value, timestamp))



class GaugeHistogramMetricFamily(Metric):
Expand Down
23 changes: 18 additions & 5 deletions prometheus_client/openmetrics/parser.py
Expand Up @@ -389,6 +389,10 @@ def do_checks():
raise ValueError("+Inf bucket missing: " + name)
if count is not None and value != count:
raise ValueError("Count does not match +Inf value: " + name)
if has_negative_buckets and has_sum:
raise ValueError("Cannot have _sum with negative buckets: " + name)
if not has_negative_buckets and has_negative_gsum:
raise ValueError("Cannot have negative _gsum with non-negative buckets: " + name)

for s in samples:
suffix = s.name[len(name):]
Expand All @@ -397,21 +401,31 @@ def do_checks():
if group is not None:
do_checks()
count = None
bucket = -1
bucket = None
has_negative_buckets = False
has_sum = False
has_negative_gsum = False
value = 0
group = g
timestamp = s.timestamp

if suffix == '_bucket':
b = float(s.labels['le'])
if b <= bucket:
if b < 0:
has_negative_buckets = True
if bucket is not None and b <= bucket:
raise ValueError("Buckets out of order: " + name)
if s.value < value:
raise ValueError("Bucket values out of order: " + name)
bucket = b
value = s.value
elif suffix in ['_count', '_gcount']:
count = s.value
elif suffix in ['_sum']:
has_sum = True
elif suffix in ['_gsum'] and s.value < 0:
has_negative_gsum = True

if group is not None:
do_checks()

Expand Down Expand Up @@ -529,7 +543,7 @@ def build_metric(name, documentation, typ, unit, samples):
if typ == 'stateset' and name not in sample.labels:
raise ValueError("Stateset missing label: " + line)
if (typ in ['histogram', 'gaugehistogram'] and name + '_bucket' == sample.name
and (float(sample.labels.get('le', -1)) < 0
and (sample.labels.get('le', "NaN") == "NaN"
or sample.labels['le'] != floatToGoString(sample.labels['le']))):
raise ValueError("Invalid le label: " + line)
if (typ == 'summary' and name == sample.name
Expand Down Expand Up @@ -567,8 +581,7 @@ def build_metric(name, documentation, typ, unit, samples):
if sample.name[len(name):] in ['_total', '_sum', '_count', '_bucket', '_gcount', '_gsum'] and math.isnan(
sample.value):
raise ValueError("Counter-like samples cannot be NaN: " + line)
if sample.name[len(name):] in ['_total', '_sum', '_count', '_bucket', '_gcount',
'_gsum'] and sample.value < 0:
if sample.name[len(name):] in ['_total', '_sum', '_count', '_bucket', '_gcount'] and sample.value < 0:
raise ValueError("Counter-like samples cannot be negative: " + line)
if sample.exemplar and not (
(typ in ['histogram', 'gaugehistogram'] and sample.name.endswith('_bucket'))
Expand Down
1 change: 1 addition & 0 deletions prometheus_client/utils.py
Expand Up @@ -2,6 +2,7 @@

INF = float("inf")
MINUS_INF = float("-inf")
NaN = float("NaN")


def floatToGoString(d):
Expand Down
28 changes: 28 additions & 0 deletions tests/openmetrics/test_exposition.py
Expand Up @@ -89,6 +89,22 @@ def test_histogram(self):
hh_sum 0.05
hh_created 123.456
# EOF
""", generate_latest(self.registry))

def test_histogram_negative_buckets(self):
s = Histogram('hh', 'A histogram', buckets=[-1, -0.5, 0, 0.5, 1], registry=self.registry)
s.observe(-0.5)
self.assertEqual(b"""# HELP hh A histogram
# TYPE hh histogram
hh_bucket{le="-1.0"} 0.0
hh_bucket{le="-0.5"} 1.0
hh_bucket{le="0.0"} 1.0
hh_bucket{le="0.5"} 1.0
hh_bucket{le="1.0"} 1.0
hh_bucket{le="+Inf"} 1.0
hh_count 1.0
hh_created 123.456
# EOF
""", generate_latest(self.registry))

def test_histogram_exemplar(self):
Expand Down Expand Up @@ -148,6 +164,18 @@ def test_gaugehistogram(self):
gh_gcount 5.0
gh_gsum 7.0
# EOF
""", generate_latest(self.registry))

def test_gaugehistogram_negative_buckets(self):
self.custom_collector(
GaugeHistogramMetricFamily('gh', 'help', buckets=[('-1.0', 4), ('+Inf', (5))], gsum_value=-7))
self.assertEqual(b"""# HELP gh help
# TYPE gh gaugehistogram
gh_bucket{le="-1.0"} 4.0
gh_bucket{le="+Inf"} 5.0
gh_gcount 5.0
gh_gsum -7.0
# EOF
""", generate_latest(self.registry))

def test_info(self):
Expand Down
27 changes: 27 additions & 0 deletions tests/openmetrics/test_parser.py
Expand Up @@ -122,6 +122,18 @@ def test_simple_histogram(self):
self.assertEqual([HistogramMetricFamily("a", "help", sum_value=2, buckets=[("1.0", 0.0), ("+Inf", 3.0)])],
list(families))

def test_negative_bucket_histogram(self):
families = text_string_to_metric_families("""# TYPE a histogram
# HELP a help
a_bucket{le="-1.0"} 0
a_bucket{le="1.0"} 1
a_bucket{le="+Inf"} 3
a_count 3
# EOF
""")
self.assertEqual([HistogramMetricFamily("a", "help", buckets=[("-1.0", 0.0), ("1.0", 1.0), ("+Inf", 3.0)])],
list(families))

def test_histogram_exemplars(self):
families = text_string_to_metric_families("""# TYPE a histogram
# HELP a help
Expand Down Expand Up @@ -150,6 +162,19 @@ def test_simple_gaugehistogram(self):
self.assertEqual([GaugeHistogramMetricFamily("a", "help", gsum_value=2, buckets=[("1.0", 0.0), ("+Inf", 3.0)])],
list(families))

def test_negative_bucket_gaugehistogram(self):
families = text_string_to_metric_families("""# TYPE a gaugehistogram
# HELP a help
a_bucket{le="-1.0"} 1
a_bucket{le="1.0"} 2
a_bucket{le="+Inf"} 3
a_gcount 3
a_gsum -5
# EOF
""")
self.assertEqual([GaugeHistogramMetricFamily("a", "help", gsum_value=-5, buckets=[("-1.0", 1.0), ("1.0", 2.0), ("+Inf", 3.0)])],
list(families))

def test_gaugehistogram_exemplars(self):
families = text_string_to_metric_families("""# TYPE a gaugehistogram
# HELP a help
Expand Down Expand Up @@ -689,6 +714,8 @@ def test_invalid_input(self):
('# TYPE a histogram\na_sum -1\n# EOF\n'),
('# TYPE a histogram\na_count -1\n# EOF\n'),
('# TYPE a histogram\na_bucket{le="+Inf"} -1\n# EOF\n'),
('# TYPE a histogram\na_bucket{le="-1.0"} 1\na_bucket{le="+Inf"} 2\na_sum -1\n# EOF\n'),
('# TYPE a histogram\na_bucket{le="-1.0"} 1\na_bucket{le="+Inf"} 2\na_sum 1\n# EOF\n'),
('# TYPE a gaugehistogram\na_bucket{le="+Inf"} NaN\n# EOF\n'),
('# TYPE a gaugehistogram\na_bucket{le="+Inf"} -1\na_gcount -1\n# EOF\n'),
('# TYPE a gaugehistogram\na_bucket{le="+Inf"} -1\n# EOF\n'),
Expand Down
2 changes: 1 addition & 1 deletion tests/test_core.py
Expand Up @@ -657,7 +657,7 @@ def test_bad_constructors(self):
self.assertRaises(ValueError, SummaryMetricFamily, 's', 'help', count_value=1, sum_value=1, labels=['a'])

self.assertRaises(ValueError, HistogramMetricFamily, 'h', 'help', sum_value=1)
self.assertRaises(ValueError, HistogramMetricFamily, 'h', 'help', buckets={})
self.assertRaises(KeyError, HistogramMetricFamily, 'h', 'help', buckets={})
self.assertRaises(ValueError, HistogramMetricFamily, 'h', 'help', sum_value=1, labels=['a'])
self.assertRaises(ValueError, HistogramMetricFamily, 'h', 'help', buckets={}, labels=['a'])
self.assertRaises(ValueError, HistogramMetricFamily, 'h', 'help', buckets={}, sum_value=1, labels=['a'])
Expand Down
1 change: 0 additions & 1 deletion tests/test_exposition.py
Expand Up @@ -386,7 +386,6 @@ def test_summary_metric_family(registry, count_value, sum_value, error):


@pytest.mark.parametrize('MetricFamily', [
core.HistogramMetricFamily,
core.GaugeHistogramMetricFamily,
])
@pytest.mark.parametrize('buckets,sum_value,error', [
Expand Down

0 comments on commit 668fd0c

Please sign in to comment.