Fix deadlock on gcCollector (#371)

* Use gc.stats for gcCollector metrics

Signed-off-by: dmitry.kryukov <[email protected]>

Author: Krukov

prometheus_client/gc_collector.py

@@ -3,79 +3,44 @@
 from __future__ import unicode_literals
 
 import gc
-import os
-import time
 
-from .metrics import Histogram
+from .metrics_core import CounterMetricFamily
 from .registry import REGISTRY
 
 
 class GCCollector(object):
     """Collector for Garbage collection statistics."""
 
-    def __init__(self, registry=REGISTRY, gc=gc):
-        # To work around the deadlock issue described in
-        # https://github.com/prometheus/client_python/issues/322,
-        # the GC collector is always disabled in multiprocess mode.
-        if 'prometheus_multiproc_dir' in os.environ:
+    def __init__(self, registry=REGISTRY):
+        if not hasattr(gc, 'get_stats'):
             return
+        registry.register(self)
 
-        if not hasattr(gc, 'callbacks'):
-            return
-
-        collected = Histogram(
-            'python_gc_collected_objects',
+    def collect(self):
+        collected = CounterMetricFamily(
+            'python_gc_objects_collected',
             'Objects collected during gc',
-            ['generation'],
-            buckets=[500, 1000, 5000, 10000, 50000],
-            registry=registry
+            labels=['generation'],
         )
-
-        uncollectable = Histogram(
-            'python_gc_uncollectable_objects',
+        uncollectable = CounterMetricFamily(
+            'python_gc_objects_uncollectable',
             'Uncollectable object found during GC',
-            ['generation'],
-            buckets=[500, 1000, 5000, 10000, 50000],
-            registry=registry
+            labels=['generation'],
         )
 
-        latency = Histogram(
-            'python_gc_duration_seconds',
-            'Time spent in garbage collection',
-            ['generation'],
-            registry=registry
+        collections = CounterMetricFamily(
+            'python_gc_collections',
+            'Number of times this generation was collected',
+            labels=['generation'],
         )
 
-        times = {}
-
-        # Avoid _cb() being called re-entrantly
-        # by setting this flag and clearing it once
-        # the callback operation is complete.
-        # See https://github.com/prometheus/client_python/issues/322#issuecomment-438021132
-        self.gc_cb_active = False
-
-        def _cb(phase, info):
-            try:
-                if self.gc_cb_active:
-                    return
-                self.gc_cb_active = True
-
-                gen = info['generation']
-
-                if phase == 'start':
-                    times[gen] = time.time()
-
-                if phase == 'stop':
-                    delta = time.time() - times[gen]
-                    latency.labels(gen).observe(delta)
-                    if 'collected' in info:
-                        collected.labels(gen).observe(info['collected'])
-                    if 'uncollectable' in info:
-                        uncollectable.labels(gen).observe(info['uncollectable'])
-            finally:
-                self.gc_cb_active = False
+        for generation, stat in enumerate(gc.get_stats()):
+            generation = str(generation)
+            collected.add_metric([generation], value=stat['collected'])
+            uncollectable.add_metric([generation], value=stat['uncollectable'])
+            collections.add_metric([generation], value=stat['collections'])
 
-        gc.callbacks.append(_cb)
+        return [collected, uncollectable, collections]
 
 
 GC_COLLECTOR = GCCollector()

tests/test_gc_collector.py

@@ -1,54 +1,62 @@
 from __future__ import unicode_literals
 
-import unittest
+import gc
+import sys
+
+if sys.version_info < (2, 7):
+    # We need the skip decorators from unittest2 on Python 2.6.
+    import unittest2 as unittest
+else:
+    import unittest
 
 from prometheus_client import CollectorRegistry, GCCollector
 
 
+@unittest.skipIf(sys.version_info < (3, 4), "Test requires Python 3.4 +")
 class TestGCCollector(unittest.TestCase):
     def setUp(self):
+        gc.disable()
+        gc.collect()
         self.registry = CollectorRegistry()
-        self.gc = _MockGC()
 
     def test_working(self):
-        collector = GCCollector(registry=self.registry, gc=self.gc)
-        self.gc.start_gc({'generation': 0})
-        self.gc.stop_gc({'generation': 0, 'collected': 10, 'uncollectable': 2})
-
-        self.assertEqual(1,
-                         self.registry.get_sample_value(
-                             'python_gc_duration_seconds_count',
-                             labels={"generation": "0"}))
-
-        self.assertEqual(1,
-                         self.registry.get_sample_value(
-                             'python_gc_collected_objects_count',
-                             labels={"generation": "0"}))
-
-        self.assertEqual(1,
-                         self.registry.get_sample_value(
-                             'python_gc_uncollectable_objects_count',
-                             labels={"generation": "0"}))
-
-        self.assertEqual(10,
-                         self.registry.get_sample_value(
-                             'python_gc_collected_objects_sum',
-                             labels={"generation": "0"}))
 
-        self.assertEqual(2,
+        GCCollector(registry=self.registry)
+        self.registry.collect()
+        before = self.registry.get_sample_value('python_gc_objects_collected_total',
+                                                labels={"generation": "0"})
+
+        #  add targets for gc
+        a = []
+        a.append(a)
+        del a
+        b = []
+        b.append(b)
+        del b
+
+        gc.collect(0)
+        self.registry.collect()
+
+        after = self.registry.get_sample_value('python_gc_objects_collected_total',
+                                               labels={"generation": "0"})
+        self.assertEqual(2, after - before)
+        self.assertEqual(0,
                          self.registry.get_sample_value(
-                             'python_gc_uncollectable_objects_sum',
+                             'python_gc_objects_uncollectable_total',
                              labels={"generation": "0"}))
 
+    def test_empty(self):
 
-class _MockGC(object):
-    def __init__(self):
-        self.callbacks = []
+        GCCollector(registry=self.registry)
+        self.registry.collect()
+        before = self.registry.get_sample_value('python_gc_objects_collected_total',
+                                                labels={"generation": "0"})
+        gc.collect(0)
+        self.registry.collect()
 
-    def start_gc(self, info):
-        for cb in self.callbacks:
-            cb('start', info)
+        after = self.registry.get_sample_value('python_gc_objects_collected_total',
+                                               labels={"generation": "0"})
+        self.assertEqual(0, after - before)
 
-    def stop_gc(self, info):
-        for cb in self.callbacks:
-            cb('stop', info)
+    def tearDown(self):
+        gc.enable()