Repository: prometheus/client_python
Pull request #1077 (2024-12-03 13:54:04)

Check if labelvalues is in _metrics before deletion in MetricWrapperBase.remove()
Currently raises an error if labelvalues is not in _metrics. Since the labelvalues get removed anyways, there isn't really any point of this error, since we would want to assume that after running this function call labelvalues will not exist regardless of it existed beforehand or not.


Patch:
diff --git a/.circleci/config.yml b/.circleci/config.yml
index 2605a50..26109fc 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -75,7 +75,6 @@ workflows:
           matrix:
             parameters:
                python:
-                 - "3.8.18"
                  - "3.9.18"
                  - "3.10"
                  - "3.11"
@@ -89,4 +88,4 @@ workflows:
           matrix:
             parameters:
                python:
-                 - "3.8"
+                 - "3.9"
diff --git a/prometheus_client/metrics.py b/prometheus_client/metrics.py
index 4617586..b9f25ff 100644
--- a/prometheus_client/metrics.py
+++ b/prometheus_client/metrics.py
@@ -1,5 +1,5 @@
 import os
-from threading import RLock
+from threading import Lock
 import time
 import types
 from typing import (
@@ -120,7 +120,7 @@ class MetricWrapperBase(Collector):
 
         if self._is_parent():
             # Prepare the fields needed for child metrics.
-            self._lock = RLock()
+            self._lock = Lock()
             self._metrics: Dict[Sequence[str], T] = {}
 
         if self._is_observable():
@@ -200,7 +200,8 @@ class MetricWrapperBase(Collector):
             raise ValueError('Incorrect label count (expected %d, got %s)' % (len(self._labelnames), labelvalues))
         labelvalues = tuple(str(l) for l in labelvalues)
         with self._lock:
-            del self._metrics[labelvalues]
+            if labelvalues in self._metrics:
+                del self._metrics[labelvalues]
 
     def clear(self) -> None:
         """Remove all labelsets from the metric"""
@@ -673,7 +674,7 @@ class Info(MetricWrapperBase):
 
     def _metric_init(self):
         self._labelname_set = set(self._labelnames)
-        self._lock = RLock()
+        self._lock = Lock()
         self._value = {}
 
     def info(self, val: Dict[str, str]) -> None:
@@ -735,7 +736,7 @@ class Enum(MetricWrapperBase):
 
     def _metric_init(self) -> None:
         self._value = 0
-        self._lock = RLock()
+        self._lock = Lock()
 
     def state(self, state: str) -> None:
         """Set enum metric state."""
diff --git a/prometheus_client/registry.py b/prometheus_client/registry.py
index 4326b39..694e4bd 100644
--- a/prometheus_client/registry.py
+++ b/prometheus_client/registry.py
@@ -1,6 +1,6 @@
 from abc import ABC, abstractmethod
 import copy
-from threading import RLock
+from threading import Lock
 from typing import Dict, Iterable, List, Optional
 
 from .metrics_core import Metric
@@ -30,7 +30,7 @@ class CollectorRegistry(Collector):
         self._collector_to_names: Dict[Collector, List[str]] = {}
         self._names_to_collectors: Dict[str, Collector] = {}
         self._auto_describe = auto_describe
-        self._lock = RLock()
+        self._lock = Lock()
         self._target_info: Optional[Dict[str, str]] = {}
         self.set_target_info(target_info)
 
diff --git a/prometheus_client/values.py b/prometheus_client/values.py
index 05331f8..6ff85e3 100644
--- a/prometheus_client/values.py
+++ b/prometheus_client/values.py
@@ -1,5 +1,5 @@
 import os
-from threading import RLock
+from threading import Lock
 import warnings
 
 from .mmap_dict import mmap_key, MmapedDict
@@ -13,7 +13,7 @@ class MutexValue:
     def __init__(self, typ, metric_name, name, labelnames, labelvalues, help_text, **kwargs):
         self._value = 0.0
         self._exemplar = None
-        self._lock = RLock()
+        self._lock = Lock()
 
     def inc(self, amount):
         with self._lock:
@@ -50,7 +50,7 @@ def MultiProcessValue(process_identifier=os.getpid):
     # Use a single global lock when in multi-processing mode
     # as we presume this means there is no threading going on.
     # This avoids the need to also have mutexes in __MmapDict.
-    lock = RLock()
+    lock = Lock()
 
     class MmapedValue:
         """A float protected by a mutex backed by a per-process mmaped file."""
diff --git a/setup.py b/setup.py
index 438f643..1665ed7 100644
--- a/setup.py
+++ b/setup.py
@@ -30,7 +30,7 @@ setup(
         'twisted': ['twisted'],
     },
     test_suite="tests",
-    python_requires=">=3.8",
+    python_requires=">=3.9",
     classifiers=[
         "Development Status :: 4 - Beta",
         "Intended Audience :: Developers",
@@ -38,7 +38,6 @@ setup(
         "Intended Audience :: System Administrators",
         "Programming Language :: Python",
         "Programming Language :: Python :: 3",
-        "Programming Language :: Python :: 3.8",
         "Programming Language :: Python :: 3.9",
         "Programming Language :: Python :: 3.10",
         "Programming Language :: Python :: 3.11",
diff --git a/tox.ini b/tox.ini
index 6d6b756..354ad69 100644
--- a/tox.ini
+++ b/tox.ini
@@ -1,15 +1,15 @@
 [tox]
-envlist = coverage-clean,py{3.8,3.9,3.10,3.11,3.12,py3.8,3.9-nooptionals},coverage-report,flake8,isort,mypy
+envlist = coverage-clean,py{3.9,3.10,3.11,3.12,py3.9,3.9-nooptionals},coverage-report,flake8,isort,mypy
 
 [testenv]
 deps =
     coverage
     pytest
     attrs
-    {py3.8,pypy3.8}: twisted
-    py3.8: asgiref
-    # See https://github.com/django/asgiref/issues/393 for why we need to pin asgiref for pypy
-    pypy3.8: asgiref==3.6.0
+    {py3.9,pypy3.9}: twisted
+    # NOTE: Pinned due to https://github.com/prometheus/client_python/issues/1020
+    py3.9: asgiref==3.7
+    pypy3.9: asgiref==3.7
 commands = coverage run --parallel -m pytest {posargs}
 
 [testenv:py3.9-nooptionals]


Test patch:
diff --git a/tests/test_core.py b/tests/test_core.py
index 4e99ca3..f28c9ab 100644
--- a/tests/test_core.py
+++ b/tests/test_core.py
@@ -19,14 +19,6 @@ from prometheus_client.validation import (
 )
 
 
-def is_locked(lock):
-    "Tries to obtain a lock, returns True on success, False on failure."
-    locked = lock.acquire(blocking=False)
-    if locked:
-        lock.release()
-    return not locked
-
-
 def assert_not_observable(fn, *args, **kwargs):
     """
     Assert that a function call falls with a ValueError exception containing
@@ -1012,7 +1004,7 @@ class TestCollectorRegistry(unittest.TestCase):
         m = Metric('target', 'Target metadata', 'info')
         m.samples = [Sample('target_info', {'foo': 'bar'}, 1)]
         for _ in registry.restricted_registry(['target_info', 's_sum']).collect():
-            self.assertFalse(is_locked(registry._lock))
+            self.assertFalse(registry._lock.locked())
 
 
 if __name__ == '__main__':