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.

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

Raise the correct exception if the metrics are not observable #666

Merged
merged 1 commit into from
Jun 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions prometheus_client/metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@ def _metric_init(self):

def inc(self, amount=1):
"""Increment counter by the given amount."""
self._raise_if_not_observable()
if amount < 0:
raise ValueError('Counters can only be incremented by non-negative amounts.')
self._value.inc(amount)
Expand Down Expand Up @@ -353,14 +354,17 @@ def _metric_init(self):

def inc(self, amount=1):
"""Increment gauge by the given amount."""
self._raise_if_not_observable()
self._value.inc(amount)

def dec(self, amount=1):
"""Decrement gauge by the given amount."""
self._raise_if_not_observable()
self._value.inc(-amount)

def set(self, value):
"""Set gauge to the given value."""
self._raise_if_not_observable()
self._value.set(float(value))

def set_to_current_time(self):
Expand Down Expand Up @@ -392,6 +396,8 @@ def set_function(self, f):
multiple threads. All other methods of the Gauge become NOOPs.
"""

self._raise_if_not_observable()

def samples(self):
return (('', {}, float(f())),)

Expand Down Expand Up @@ -450,6 +456,7 @@ def observe(self, amount):
https://prometheus.io/docs/practices/histograms/#count-and-sum-of-observations
for details.
"""
self._raise_if_not_observable()
self._count.inc(1)
self._sum.inc(amount)

Expand Down Expand Up @@ -567,6 +574,7 @@ def observe(self, amount):
https://prometheus.io/docs/practices/histograms/#count-and-sum-of-observations
for details.
"""
self._raise_if_not_observable()
self._sum.inc(amount)
for i, bound in enumerate(self._upper_bounds):
if amount <= bound:
Expand Down
75 changes: 55 additions & 20 deletions tests/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,21 @@
import unittest


def assert_not_observable(fn, *args, **kwargs):
"""
Assert that a function call falls with a ValueError exception containing
'missing label values'
"""

try:
fn(*args, **kwargs)
except ValueError as e:
assert 'missing label values' in str(e)
return

assert False, "Did not raise a 'missing label values' exception"


class TestCounter(unittest.TestCase):
def setUp(self):
self.registry = CollectorRegistry()
Expand All @@ -36,7 +51,6 @@ def test_repr(self):

def test_negative_increment_raises(self):
self.assertRaises(ValueError, self.counter.inc, -1)


def test_function_decorator(self):
@self.counter.count_exceptions(ValueError)
Expand Down Expand Up @@ -76,18 +90,21 @@ def test_block_decorator(self):

def test_count_exceptions_not_observable(self):
counter = Counter('counter', 'help', labelnames=('label',), registry=self.registry)
assert_not_observable(counter.count_exceptions)

try:
counter.count_exceptions()
except ValueError as e:
self.assertIn('missing label values', str(e))
def test_inc_not_observable(self):
""".inc() must fail if the counter is not observable."""

counter = Counter('counter', 'help', labelnames=('label',), registry=self.registry)
assert_not_observable(counter.inc)


class TestGauge(unittest.TestCase):
def setUp(self):
self.registry = CollectorRegistry()
self.gauge = Gauge('g', 'help', registry=self.registry)

self.gauge_with_label = Gauge('g2', 'help', labelnames=("label1",), registry=self.registry)

def test_repr(self):
self.assertEqual(repr(self.gauge), "prometheus_client.metrics.Gauge(g)")

Expand All @@ -100,6 +117,21 @@ def test_gauge(self):
self.gauge.set(9)
self.assertEqual(9, self.registry.get_sample_value('g'))

def test_inc_not_observable(self):
""".inc() must fail if the gauge is not observable."""

assert_not_observable(self.gauge_with_label.inc)

def test_dec_not_observable(self):
""".dec() must fail if the gauge is not observable."""

assert_not_observable(self.gauge_with_label.dec)

def test_set_not_observable(self):
""".set() must fail if the gauge is not observable."""

assert_not_observable(self.gauge_with_label.set, 1)

def test_inprogress_function_decorator(self):
self.assertEqual(0, self.registry.get_sample_value('g'))

Expand Down Expand Up @@ -127,6 +159,11 @@ def test_gauge_function(self):
x['a'] = None
self.assertEqual(1, self.registry.get_sample_value('g'))

def test_set_function_not_observable(self):
""".set_function() must fail if the gauge is not observable."""

assert_not_observable(self.gauge_with_label.set_function, lambda: 1)

def test_time_function_decorator(self):
self.assertEqual(0, self.registry.get_sample_value('g'))

Expand Down Expand Up @@ -167,25 +204,18 @@ def test_time_block_decorator(self):

def test_track_in_progress_not_observable(self):
g = Gauge('test', 'help', labelnames=('label',), registry=self.registry)

try:
g.track_inprogress()
except ValueError as e:
self.assertIn('missing label values', str(e))
assert_not_observable(g.track_inprogress)

def test_timer_not_observable(self):
g = Gauge('test', 'help', labelnames=('label',), registry=self.registry)

try:
g.time()
except ValueError as e:
self.assertIn('missing label values', str(e))
assert_not_observable(g.time)


class TestSummary(unittest.TestCase):
def setUp(self):
self.registry = CollectorRegistry()
self.summary = Summary('s', 'help', registry=self.registry)
self.summary_with_labels = Summary('s_with_labels', 'help', labelnames=("label1",), registry=self.registry)

def test_repr(self):
self.assertEqual(repr(self.summary), "prometheus_client.metrics.Summary(s)")
Expand All @@ -197,6 +227,10 @@ def test_summary(self):
self.assertEqual(1, self.registry.get_sample_value('s_count'))
self.assertEqual(10, self.registry.get_sample_value('s_sum'))

def test_summary_not_observable(self):
""".observe() must fail if the Summary is not observable."""
assert_not_observable(self.summary_with_labels.observe, 1)

def test_function_decorator(self):
self.assertEqual(0, self.registry.get_sample_value('s_count'))

Expand Down Expand Up @@ -267,10 +301,7 @@ def test_block_decorator(self):
def test_timer_not_observable(self):
s = Summary('test', 'help', labelnames=('label',), registry=self.registry)

try:
s.time()
except ValueError as e:
self.assertIn('missing label values', str(e))
assert_not_observable(s.time)


class TestHistogram(unittest.TestCase):
Expand Down Expand Up @@ -315,6 +346,10 @@ def test_histogram(self):
self.assertEqual(3, self.registry.get_sample_value('h_count'))
self.assertEqual(float("inf"), self.registry.get_sample_value('h_sum'))

def test_histogram_not_observable(self):
""".observe() must fail if the Summary is not observable."""
assert_not_observable(self.labels.observe, 1)

def test_setting_buckets(self):
h = Histogram('h', 'help', registry=None, buckets=[0, 1, 2])
self.assertEqual([0.0, 1.0, 2.0, float("inf")], h._upper_bounds)
Expand Down