8000 [#674, #675] Mitigate performance regression by filtering restricted … · ethervoid/client_python@c49e55a · GitHub
[go: up one dir, main page]
Skip to content

Commit c49e55a

Browse files
pavel-lexyrpavel-lexyr
authored
[prometheus#674, prometheus#675] Mitigate performance regression by filtering restricted registry collections (prometheus#680)
* Made restricted registries call collect() only on relevant collections. * Added a skip-if for a test that won't run on Python 2.7. * Moved yielding target_info out of the lock. * Fixed style and a race condition. Signed-off-by: Pavel <pavel@lexyr.com>
1 parent e92fa05 commit c49e55a

File tree

2 files changed

+43
-4
lines changed

2 files changed

+43
-4
lines changed

‎prometheus_client/registry.py

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -135,10 +135,21 @@ def __init__(self, names, registry):
135135
self._registry = registry
136136

137137
def collect(self):
138-
for metric in self._registry.collect():
139-
m = metric._restricted_metric(self._name_set)
140-
if m:
141-
yield m
138+
collectors = set()
139+
target_info_metric = None
140+
with self._registry._lock:
141+
if 'target_info' in self._name_set and self._registry._target_info:
142+
target_info_metric = self._registry._target_info_metric()
143+
for name in self._name_set:
144+
if name != 'target_info' and name in self._registry._names_to_collectors:
145+
collectors.add(self._registry._names_to_collectors[name])
146+
if target_info_metric:
147+
yield target_info_metric
148+
for collector in collectors:
149+
for metric in collector.collect():
150+
m = metric._restricted_metric(self._name_set)
151+
if m:
152+
yield m
142153

143154

144155
REGISTRY = CollectorRegistry(auto_describe=True)

‎tests/test_core.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
from __future__ import unicode_literals
22

33
from concurrent.futures import ThreadPoolExecutor
4+
import sys
45
import time
56

67
import pytest
@@ -838,6 +839,33 @@ def test_target_info_restricted_registry(self):
838839
m.samples = [Sample('target_info', {'foo': 'bar'}, 1)]
839840
self.assertEqual([m], list(registry.restricted_registry(['target_info']).collect()))
840841

842+
@unittest.skipIf(sys.version_info < (3, 3), "Test requires Python 3.3+.")
843+
def test_restricted_registry_does_not_call_extra(self):
844+
from unittest.mock import MagicMock
845+
registry = CollectorRegistry()
846+
mock_collector = MagicMock()
847+
mock_collector.describe.return_value = [Metric('foo', 'help', 'summary')]
848+
registry.register(mock_collector)
849+
Summary('s', 'help', registry=registry).observe(7)
850+
851+
m = Metric('s', 'help', 'summary')
852+
m.samples = [Sample('s_sum', {}, 7)]
853+
self.assertEqual([m], list(registry.restricted_registry(['s_sum']).collect()))
854+
mock_collector.collect.assert_not_called()
855+
856+
def test_restricted_registry_does_not_yield_while_locked(self):
857+
registry = CollectorRegistry(target_info={'foo': 'bar'})
858+
Summary('s', 'help', registry=registry).observe(7)
859+
860+
m = Metric('s', 'help', 'summary')
861+
m.samples = [Sample('s_sum', {}, 7)]
862+
self.assertEqual([m], list(registry.restricted_registry(['s_sum']).collect()))
863+
864+
m = Metric('target', 'Target metadata', 'info')
865+
m.samples = [Sample('target_info', {'foo': 'bar'}, 1)]
866+
for _ in registry.restricted_registry(['target_info', 's_sum']).collect():
867+
self.assertFalse(registry._lock.locked())
868+
841869

842870
if __name__ == '__main__':
843871
unittest.main()

0 commit comments

Comments
 (0)
0