From cee1215fd3826944a9e4325b614f007759092331 Mon Sep 17 00:00:00 2001 From: Ben Timby Date: Tue, 5 Mar 2024 11:59:32 -0500 Subject: [PATCH 1/6] Use re-entrant lock. Signed-off-by: Ben Timby --- prometheus_client/metrics.py | 8 ++++---- prometheus_client/registry.py | 4 ++-- prometheus_client/values.py | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/prometheus_client/metrics.py b/prometheus_client/metrics.py index af512115..3bda92c4 100644 --- a/prometheus_client/metrics.py +++ b/prometheus_client/metrics.py @@ -1,5 +1,5 @@ import os -from threading import Lock +from threading import RLock import time import types from typing import ( @@ -144,7 +144,7 @@ def __init__(self: T, if self._is_parent(): # Prepare the fields needed for child metrics. - self._lock = Lock() + self._lock = RLock() self._metrics: Dict[Sequence[str], T] = {} if self._is_observable(): @@ -697,7 +697,7 @@ class Info(MetricWrapperBase): def _metric_init(self): self._labelname_set = set(self._labelnames) - self._lock = Lock() + self._lock = RLock() self._value = {} def info(self, val: Dict[str, str]) -> None: @@ -759,7 +759,7 @@ def __init__(self, def _metric_init(self) -> None: self._value = 0 - self._lock = Lock() + self._lock = RLock() def state(self, state: str) -> None: """Set enum metric state.""" diff --git a/prometheus_client/registry.py b/prometheus_client/registry.py index 694e4bd8..4326b39a 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 Lock +from threading import RLock from typing import Dict, Iterable, List, Optional from .metrics_core import Metric @@ -30,7 +30,7 @@ def __init__(self, auto_describe: bool = False, target_info: Optional[Dict[str, self._collector_to_names: Dict[Collector, List[str]] = {} self._names_to_collectors: Dict[str, Collector] = {} self._auto_describe = auto_describe - self._lock = Lock() + self._lock = RLock() 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 6ff85e3b..bcc66f72 100644 --- a/prometheus_client/values.py +++ b/prometheus_client/values.py @@ -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 = Lock() + self._lock = RLock() 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 = Lock() + lock = RLock() class MmapedValue: """A float protected by a mutex backed by a per-process mmaped file.""" From df30ee674d33308cbe6e1e590471dc19ebb134db Mon Sep 17 00:00:00 2001 From: Ben Timby Date: Tue, 5 Mar 2024 12:04:11 -0500 Subject: [PATCH 2/6] Fix import. Signed-off-by: Ben Timby --- prometheus_client/values.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prometheus_client/values.py b/prometheus_client/values.py index bcc66f72..05331f82 100644 --- a/prometheus_client/values.py +++ b/prometheus_client/values.py @@ -1,5 +1,5 @@ import os -from threading import Lock +from threading import RLock import warnings from .mmap_dict import mmap_key, MmapedDict From 045a3c0071da7f336d7f4476df4e73aaa6fe5766 Mon Sep 17 00:00:00 2001 From: Ben Timby Date: Tue, 5 Mar 2024 15:59:34 -0500 Subject: [PATCH 3/6] Use custom Lock class that provides locked() method (for tests). Signed-off-by: Ben Timby --- prometheus_client/metrics.py | 9 ++++----- prometheus_client/registry.py | 4 ++-- prometheus_client/utils.py | 6 ++++++ prometheus_client/values.py | 6 +++--- 4 files changed, 15 insertions(+), 10 deletions(-) diff --git a/prometheus_client/metrics.py b/prometheus_client/metrics.py index 3bda92c4..684cf1ae 100644 --- a/prometheus_client/metrics.py +++ b/prometheus_client/metrics.py @@ -1,5 +1,4 @@ import os -from threading import RLock import time import types from typing import ( @@ -16,7 +15,7 @@ ) from .registry import Collector, CollectorRegistry, REGISTRY from .samples import Exemplar, Sample -from .utils import floatToGoString, INF +from .utils import floatToGoString, INF, Lock T = TypeVar('T', bound='MetricWrapperBase') F = TypeVar("F", bound=Callable[..., Any]) @@ -144,7 +143,7 @@ def __init__(self: T, 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(): @@ -697,7 +696,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: @@ -759,7 +758,7 @@ def __init__(self, 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 4326b39a..ab002445 100644 --- a/prometheus_client/registry.py +++ b/prometheus_client/registry.py @@ -1,9 +1,9 @@ from abc import ABC, abstractmethod import copy -from threading import RLock from typing import Dict, Iterable, List, Optional from .metrics_core import Metric +from .utils import Lock # Ideally this would be a Protocol, but Protocols are only available in Python >= 3.8. @@ -30,7 +30,7 @@ def __init__(self, auto_describe: bool = False, target_info: Optional[Dict[str, 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/utils.py b/prometheus_client/utils.py index 0d2b0948..567f62d0 100644 --- a/prometheus_client/utils.py +++ b/prometheus_client/utils.py @@ -1,4 +1,5 @@ import math +from threading import _PyRLock INF = float("inf") MINUS_INF = float("-inf") @@ -22,3 +23,8 @@ def floatToGoString(d): mantissa = f'{s[0]}.{s[1:dot]}{s[dot + 1:]}'.rstrip('0.') return f'{mantissa}e+0{dot - 1}' return s + + +class Lock(_PyRLock): + def locked(self): + return bool(self._count) diff --git a/prometheus_client/values.py b/prometheus_client/values.py index 05331f82..0dfe8621 100644 --- a/prometheus_client/values.py +++ b/prometheus_client/values.py @@ -1,8 +1,8 @@ import os -from threading import RLock import warnings from .mmap_dict import mmap_key, MmapedDict +from .utils import Lock class MutexValue: @@ -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.""" From af452a23ecaf6511decce2cc7d603e28f1624f81 Mon Sep 17 00:00:00 2001 From: Ben Timby Date: Tue, 5 Mar 2024 16:09:47 -0500 Subject: [PATCH 4/6] Ignore MyPy error (attribute does indeed exist). Signed-off-by: Ben Timby --- prometheus_client/utils.py | 1 + 1 file changed, 1 insertion(+) diff --git a/prometheus_client/utils.py b/prometheus_client/utils.py index 567f62d0..66ddaea9 100644 --- a/prometheus_client/utils.py +++ b/prometheus_client/utils.py @@ -1,4 +1,5 @@ import math +# type: ignore[attr-defined] from threading import _PyRLock INF = float("inf") From f8f919198f4091b2d5f43c0d70155b8dec3a9aa7 Mon Sep 17 00:00:00 2001 From: Ben Timby Date: Tue, 5 Mar 2024 16:12:21 -0500 Subject: [PATCH 5/6] Let's try this again. Signed-off-by: Ben Timby --- prometheus_client/utils.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/prometheus_client/utils.py b/prometheus_client/utils.py index 66ddaea9..51dea014 100644 --- a/prometheus_client/utils.py +++ b/prometheus_client/utils.py @@ -1,6 +1,5 @@ import math -# type: ignore[attr-defined] -from threading import _PyRLock +from threading import _PyRLock # type: ignore[attr-defined] INF = float("inf") MINUS_INF = float("-inf") From 529022ece2bae9dc55c458a76d3876767973f0f8 Mon Sep 17 00:00:00 2001 From: Ben Timby Date: Fri, 2 Aug 2024 13:21:42 -0400 Subject: [PATCH 6/6] Alternative implementation. Signed-off-by: Ben Timby --- prometheus_client/metrics.py | 9 +++++---- prometheus_client/registry.py | 4 ++-- prometheus_client/utils.py | 6 ------ prometheus_client/values.py | 6 +++--- tests/test_core.py | 10 +++++++++- 5 files changed, 19 insertions(+), 16 deletions(-) diff --git a/prometheus_client/metrics.py b/prometheus_client/metrics.py index 684cf1ae..3bda92c4 100644 --- a/prometheus_client/metrics.py +++ b/prometheus_client/metrics.py @@ -1,4 +1,5 @@ import os +from threading import RLock import time import types from typing import ( @@ -15,7 +16,7 @@ ) from .registry import Collector, CollectorRegistry, REGISTRY from .samples import Exemplar, Sample -from .utils import floatToGoString, INF, Lock +from .utils import floatToGoString, INF T = TypeVar('T', bound='MetricWrapperBase') F = TypeVar("F", bound=Callable[..., Any]) @@ -143,7 +144,7 @@ def __init__(self: T, if self._is_parent(): # Prepare the fields needed for child metrics. - self._lock = Lock() + self._lock = RLock() self._metrics: Dict[Sequence[str], T] = {} if self._is_observable(): @@ -696,7 +697,7 @@ class Info(MetricWrapperBase): def _metric_init(self): self._labelname_set = set(self._labelnames) - self._lock = Lock() + self._lock = RLock() self._value = {} def info(self, val: Dict[str, str]) -> None: @@ -758,7 +759,7 @@ def __init__(self, def _metric_init(self) -> None: self._value = 0 - self._lock = Lock() + self._lock = RLock() def state(self, state: str) -> None: """Set enum metric state.""" diff --git a/prometheus_client/registry.py b/prometheus_client/registry.py index ab002445..4326b39a 100644 --- a/prometheus_client/registry.py +++ b/prometheus_client/registry.py @@ -1,9 +1,9 @@ from abc import ABC, abstractmethod import copy +from threading import RLock from typing import Dict, Iterable, List, Optional from .metrics_core import Metric -from .utils import Lock # Ideally this would be a Protocol, but Protocols are only available in Python >= 3.8. @@ -30,7 +30,7 @@ def __init__(self, auto_describe: bool = False, target_info: Optional[Dict[str, self._collector_to_names: Dict[Collector, List[str]] = {} self._names_to_collectors: Dict[str, Collector] = {} self._auto_describe = auto_describe - self._lock = Lock() + self._lock = RLock() self._target_info: Optional[Dict[str, str]] = {} self.set_target_info(target_info) diff --git a/prometheus_client/utils.py b/prometheus_client/utils.py index 51dea014..0d2b0948 100644 --- a/prometheus_client/utils.py +++ b/prometheus_client/utils.py @@ -1,5 +1,4 @@ import math -from threading import _PyRLock # type: ignore[attr-defined] INF = float("inf") MINUS_INF = float("-inf") @@ -23,8 +22,3 @@ def floatToGoString(d): mantissa = f'{s[0]}.{s[1:dot]}{s[dot + 1:]}'.rstrip('0.') return f'{mantissa}e+0{dot - 1}' return s - - -class Lock(_PyRLock): - def locked(self): - return bool(self._count) diff --git a/prometheus_client/values.py b/prometheus_client/values.py index 0dfe8621..05331f82 100644 --- a/prometheus_client/values.py +++ b/prometheus_client/values.py @@ -1,8 +1,8 @@ import os +from threading import RLock import warnings from .mmap_dict import mmap_key, MmapedDict -from .utils import Lock class MutexValue: @@ -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 = Lock() + self._lock = RLock() 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 = Lock() + lock = RLock() class MmapedValue: """A float protected by a mutex backed by a per-process mmaped file.""" diff --git a/tests/test_core.py b/tests/test_core.py index 8a54a02d..f80fb882 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -16,6 +16,14 @@ from prometheus_client.metrics import _get_use_created +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 @@ -963,7 +971,7 @@ def test_restricted_registry_does_not_yield_while_locked(self): 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(registry._lock.locked()) + self.assertFalse(is_locked(registry._lock)) if __name__ == '__main__':