8000 Allow for negtive histogram buckets. (#496) · pathcl/client_python@87d08de · GitHub
[go: up one dir, main page]
Skip to content

Commit 87d08de

Browse files
brian-brazilbrian-brazil
authored
Allow for negtive histogram buckets. (prometheus#496)
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>
1 parent 3ab0374 commit 87d08de

File tree

8 files changed

+86
-14
lines changed

8 files changed

+86
-14
lines changed

‎prometheus_client/metrics.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -566,7 +566,8 @@ def _child_samples(self):
566566
acc += self._buckets[i].get()
567567
samples.append(('_bucket', {'le': floatToGoString(bound)}, acc))
568568
samples.append(('_count', {}, acc))
569-
samples.append(('_sum', {}, self._sum.get()))
569+
if self._upper_bounds[0] >= 0:
570+
samples.append(('_sum', {}, self._sum.get()))
570571
samples.append(('_created', {}, self._created))
571572
return tuple(samples)
572573

‎prometheus_client/metrics_core.py

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -183,8 +183,8 @@ class HistogramMetricFamily(Metric):
183183

184184
def __init__(self, name, documentation, buckets=None, sum_value=None, labels=None, unit=''):
185185
Metric.__init__(self, name, documentation, 'histogram', unit)
186-
if (sum_value is None) != (buckets is None):
187-
raise ValueError('buckets and sum_value must be provided together.')
186+
if sum_value is not None and buckets is None:
187+
raise ValueError('sum value cannot be provided without buckets.')
188188
if labels is not None and buckets is not None:
189189
raise ValueError('Can only specify at most one of buckets and labels.')
190190
if labels is None:
@@ -217,10 +217,13 @@ def add_metric(self, labels, buckets, sum_value, timestamp=None):
217217
exemplar,
218218
))
219219
# +Inf is last and provides the count value.
220-
self.samples.extend([
221-
Sample(self.name + '_count', dict(zip(self._labelnames, labels)), buckets[-1][1], timestamp),
222-
Sample(self.name + '_sum', dict(zip(self._labelnames, labels)), sum_value, timestamp),
223-
])
220+
self.samples.append(
221+
Sample(self.name + '_count', dict(zip(self._labelnames, labels)), buckets[-1][1], timestamp))
222+
# Don't iunclude sum if there's negative buckets.
223+
if float(buckets[0][0]) >= 0 and sum_value is not None:
224+
self.samples.append(
225+
Sample(self.name + '_sum', dict(zip(self._labelnames, labels)), sum_value, timestamp))
226+
224227

225228

226229
class GaugeHistogramMetricFamily(Metric):

‎prometheus_client/openmetrics/parser.py

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,10 @@ def do_checks():
389389
raise ValueError("+Inf bucket missing: " + name)
390390
if count is not None and value != count:
391391
raise ValueError("Count does not match +Inf value: " + name)
392+
if has_negative_buckets and has_sum:
393+
raise ValueError("Cannot have _sum with negative buckets: " + name)
394+
if not has_negative_buckets and has_negative_gsum:
395+
raise ValueError("Cannot have negative _gsum with non-negative buckets: " + name)
392396

393397
for s in samples:
394398
suffix = s.name[len(name):]
@@ -397,21 +401,31 @@ def do_checks():
397401
if group is not None:
398402
do_checks()
399403
count = None
400-
bucket = -1
404+
bucket = None
405+
has_negative_buckets = False
406+
has_sum = False
407+
has_negative_gsum = False
401408
value = 0
402409
group = g
403410
timestamp = s.timestamp
404411

405412
if suffix == '_bucket':
406413
b = float(s.labels['le'])
407-
if b <= bucket:
414+
if b < 0:
415+
has_negative_buckets = True
416+
if bucket is not None and b <= bucket:
408417
raise ValueError("Buckets out of order: " + name)
409418
if s.value < value:
410419
raise ValueError("Bucket values out of order: " + name)
411420
bucket = b
412421
value = s.value
413422
elif suffix in ['_count', '_gcount']:
414423
count = s.value
424+
elif suffix in ['_sum']:
425+
has_sum = True
426+
elif suffix in ['_gsum'] and s.value < 0:
427+
has_negative_gsum = True
428+
415429
if group is not None:
416430
do_checks()
417431

@@ -529,7 +543,7 @@ def build_metric(name, documentation, typ, unit, samples):
529543
if typ == 'stateset' and name not in sample.labels:
530544
raise ValueError("Stateset missing label: " + line)
531545
if (typ in ['histogram', 'gaugehistogram'] and name + '_bucket' == sample.name
532-
and (float(sample.labels.get('le', -1)) < 0
546+
and (sample.labels.get('le', "NaN") == "NaN"
533547
or sample.labels['le'] != floatToGoString(sample.labels['le']))):
534548
raise ValueError("Invalid le label: " + line)
535549
if (typ == 'summary' and name == sample.name
@@ -567,8 +581,7 @@ def build_metric(name, documentation, typ, unit, samples):
567581
if sample.name[len(name):] in ['_total', '_sum', '_count', '_bucket', '_gcount', '_gsum'] and math.isnan(
568582
sample.value):
569583
raise ValueError("Counter-like samples cannot be NaN: " + line)
570-
if sample.name[len(name):] in ['_total', '_sum', '_count', '_bucket', '_gcount',
571-
'_gsum'] and sample.value < 0:
584+
if sample.name[len(name):] in ['_total', '_sum', '_count', '_bucket', '_gcount'] and sample.value < 0:
572585
raise ValueError("Counter-like samples cannot be negative: " + line)
573586
if sample.exemplar and not (
574587
(typ in ['histogram', 'gaugehistogram'] and sample.name.endswith('_bucket'))

‎prometheus_client/utils.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
INF = float("inf")
44
MINUS_INF = float("-inf")
5+
NaN = float("NaN")
56

67

78
def floatToGoString(d):

‎tests/openmetrics/test_exposition.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,22 @@ def test_histogram(self):
8989
hh_sum 0.05
9090
hh_created 123.456
9191
# EOF
92+
""", generate_latest(self.registry))
93+
94+
def test_histogram_negative_buckets(self):
95+
s = Histogram('hh', 'A histogram', buckets=[-1, -0.5, 0, 0.5, 1], registry=self.registry)
96+
s.observe(-0.5)
97+
self.assertEqual(b"""# HELP hh A histogram
98+
# TYPE hh histogram
99+
hh_bucket{le="-1.0"} 0.0
100+
hh_bucket{le="-0.5"} 1.0
101+
hh_bucket{le="0.0"} 1.0
102+
hh_bucket{le="0.5"} 1.0
103+
hh_bucket{le="1.0"} 1.0
104+
hh_bucket{le="+Inf"} 1.0
105+
hh_count 1.0
106+
hh_created 123.456
107+
# EOF
92108
""", generate_latest(self.registry))
93109

94110
def test_histogram_exemplar(self):
@@ -148,6 +164,18 @@ def test_gaugehistogram(self):
148164
gh_gcount 5.0
149165
gh_gsum 7.0
150166
# EOF
167+
""", generate_latest(self.registry))
168+
169+
def test_gaugehistogram_negative_buckets(self):
170+
self.custom_collector(
171+
GaugeHistogramMetricFamily('gh', 'help', buckets=[('-1.0', 4), ('+Inf', (5))], gsum_value=-7))
172+
self.assertEqual(b"""# HELP gh help
173+
# TYPE gh gaugehistogram
174+
gh_bucket{le="-1.0"} 4.0
175+
gh_bucket{le="+Inf"} 5.0
176+
gh_gcount 5.0
177+
gh_gsum -7.0
178+
# EOF
151179
""", generate_latest(self.registry))
152180

153181
def test_info(self):

‎tests/openmetrics/test_parser.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,18 @@ def test_simple_histogram(self):
122122
self.assertEqual([HistogramMetricFamily("a", "help", sum_value=2, buckets=[("1.0", 0.0), ("+Inf", 3.0)])],
123123
list(families))
124124

125+
def test_negative_bucket_histogram(self):
126+
families = text_string_to_metric_families("""# TYPE a histogram
127+
# HELP a help
128+
a_bucket{le="-1.0"} 0
129+
a_bucket{le="1.0"} 1
130+
a_bucket{le="+Inf"} 3
131+
a_count 3
132+
# EOF
133+
""")
134+
self.assertEqual([HistogramMetricFamily("a", "help", buckets=[("-1.0", 0.0), ("1.0", 1.0), ("+Inf", 3.0)])],
135+
list(families))
136+
125137
def test_histogram_exemplars(self):
126138
families = text_string_to_metric_families("""# TYPE a histogram
127139
# HELP a help
@@ -150,6 +162,19 @@ def test_simple_gaugehistogram(self):
150162
self.assertEqual([GaugeHistogramMetricFamily("a", "help", gsum_value=2, buckets=[("1.0", 0.0), ("+Inf", 3.0)])],
151163
list(families))
152164

165+
def test_negative_bucket_gaugehistogram(self):
166+
families = text_string_to_metric_families("""# TYPE a gaugehistogram
167+
# HELP a help
168+
a_bucket{le="-1.0"} 1
169+
a_bucket{le="1.0"} 2
170+
a_bucket{le="+Inf"} 3
171+
a_gcount 3
172+
a_gsum -5
173+
# EOF
174+
""")
175+
self.assertEqual([GaugeHistogramMetricFamily("a", "help", gsum_value=-5, buckets=[("-1.0", 1.0), ("1.0", 2.0), ("+Inf", 3.0)])],
176+
list(families))
177+
153178
def test_gaugehistogram_exemplars(self):
154179
families = text_string_to_metric_families("""# TYPE a gaugehistogram
155180
# HELP a help
@@ -689,6 +714,8 @@ def test_invalid_input(self):
689714
('# TYPE a histogram\na_sum -1\n# EOF\n'),
690715
('# TYPE a histogram\na_count -1\n# EOF\n'),
691716
('# TYPE a histogram\na_bucket{le="+Inf"} -1\n# EOF\n'),
717+
('# TYPE a histogram\na_bucket{le="-1.0"} 1\na_bucket{le="+Inf"} 2\na_sum -1\n# EOF\n'),
718+
('# TYPE a histogram\na_bucket{le="-1.0"} 1\na_bucket{le="+Inf"} 2\na_sum 1\n# EOF\n'),
692719
('# TYPE a gaugehistogram\na_bucket{le="+Inf"} NaN\n# EOF\n'),
693720
('# TYPE a gaugehistogram\na_bucket{le="+Inf"} -1\na_gcount -1\n# EOF\n'),
694721
('# TYPE a gaugehistogram\na_bucket{le="+Inf"} -1\n# EOF\n'),

‎tests/test_core.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -657,7 +657,7 @@ def test_bad_constructors(self):
657657
self.assertRaises(ValueError, SummaryMetricFamily, 's', 'help', count_value=1, sum_value=1, labels=['a'])
658658

659659
self.assertRaises(ValueError, HistogramMetricFamily, 'h', 'help', sum_value=1)
660-
self.assertRaises(ValueError, HistogramMetricFamily, 'h', 'help', buckets={})
660+
self.assertRaises(KeyError, HistogramMetricFamily, 'h', 'help', buckets={})
661661
self.assertRaises(ValueError, HistogramMetricFamily, 'h', 'help', sum_value=1, labels=['a'])
662662
self.assertRaises(ValueError, HistogramMetricFamily, 'h', 'help', buckets={}, labels=['a'])
663663
self.assertRaises(ValueError, HistogramMetricFamily, 'h', 'help', buckets={}, sum_value=1, labels=['a'])

‎tests/test_exposition.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,6 @@ def test_summary_metric_family(registry, count_value, sum_value, error):
386386

387387

388388
@pytest.mark.parametrize('MetricFamily', [
389-
core.HistogramMetricFamily,
390389
core.GaugeHistogramMetricFamily,
391390
])
392391
@pytest.mark.parametrize('buckets,sum_value,error', [

0 commit comments

Comments
 (0)
0