Skip to content

Commit 6cb81a5

Browse files
committed
Validate Enum arguments before registering the collector
Enum.__init__ called super().__init__() -- which registers the collector in the CollectorRegistry -- before validating that states is non-empty and that the metric name does not overlap a label name. When either check failed, the ValueError was raised as expected, but a half-built Enum (whose _states was never assigned) had already been registered. That left the registry in a broken state: the name was permanently taken, so recreating the metric raised 'Duplicated timeseries', and any subsequent scrape crashed with AttributeError: 'Enum' object has no attribute '_states' when _child_samples iterated self._states. A realistic trigger is building the states list from configuration that turns out to be empty. Gauge and Histogram already validate before calling super().__init__(); this moves Enum's two guards ahead of registration to match, so a failed constructor leaves the registry untouched. Add test_failed_init_does_not_pollute_registry, which asserts that after two failed Enum constructions the name is still free, the metric can be created, and the registry scrapes cleanly. Signed-off-by: Sean Kim <skim8705@gmail.com>
1 parent a96f6f4 commit 6cb81a5

2 files changed

Lines changed: 17 additions & 4 deletions

File tree

prometheus_client/metrics.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -769,6 +769,10 @@ def __init__(self,
769769
_labelvalues: Optional[Sequence[str]] = None,
770770
states: Optional[Sequence[str]] = None,
771771
):
772+
if name in labelnames:
773+
raise ValueError(f'Overlapping labels for Enum metric: {name}')
774+
if not states:
775+
raise ValueError(f'No states provided for Enum metric: {name}')
772776
super().__init__(
773777
name=name,
774778
documentation=documentation,
@@ -779,10 +783,6 @@ def __init__(self,
779783
registry=registry,
780784
_labelvalues=_labelvalues,
781785
)
782-
if name in labelnames:
783-
raise ValueError(f'Overlapping labels for Enum metric: {name}')
784-
if not states:
785-
raise ValueError(f'No states provided for Enum metric: {name}')
786786
self._kwargs['states'] = self._states = states
787787

788788
def _metric_init(self) -> None:

tests/test_core.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -595,6 +595,19 @@ def test_overlapping_labels(self):
595595
with pytest.raises(ValueError):
596596
Enum('e', 'help', registry=None, labelnames=['e'])
597597

598+
def test_failed_init_does_not_pollute_registry(self):
599+
registry = CollectorRegistry()
600+
# A validation failure in __init__ must not leave a half-built collector
601+
# registered: otherwise the name stays permanently taken and any later
602+
# scrape of the registry crashes on the missing _states attribute.
603+
with pytest.raises(ValueError):
604+
Enum('task_state', 'help', states=None, registry=registry)
605+
with pytest.raises(ValueError):
606+
Enum('task_state', 'help', states=['a'], labelnames=['task_state'], registry=registry)
607+
# The name is still free, so a correct definition registers and scrapes.
608+
Enum('task_state', 'help', states=['a', 'b'], registry=registry)
609+
self.assertEqual(1, registry.get_sample_value('task_state', {'task_state': 'a'}))
610+
598611

599612
class TestMetricWrapper(unittest.TestCase):
600613
def setUp(self):

0 commit comments

Comments
 (0)