Multiprocess environment variable in upper case (#624)

* Use upper case environment variable for multi process
* add test cases for deprecation warning on variable capitalization
* Remove the warning when both are used and rephrase the message to be
  more deprecation aware.

Signed-off-by: Marc-Aurèle Brothier <[email protected]>

Author: marcaurele

prometheus_client/multiprocess.py

@@ -4,6 +4,7 @@
 import glob
 import json
 import os
+import warnings
 
 from .metrics_core import Metric
 from .mmap_dict import MmapedDict
@@ -23,9 +24,13 @@ class MultiProcessCollector(object):
 
     def __init__(self, registry, path=None):
         if path is None:
-            path = os.environ.get('prometheus_multiproc_dir')
+            # This deprecation warning can go away in a few releases when removing the compatibility
+            if 'prometheus_multiproc_dir' in os.environ and 'PROMETHEUS_MULTIPROC_DIR' not in os.environ:
+                os.environ['PROMETHEUS_MULTIPROC_DIR'] = os.environ['prometheus_multiproc_dir']
+                warnings.warn("prometheus_multiproc_dir variable has been deprecated in favor of the upper case naming PROMETHEUS_MULTIPROC_DIR", DeprecationWarning)
+            path = os.environ.get('PROMETHEUS_MULTIPROC_DIR')
         if not path or not os.path.isdir(path):
-            raise ValueError('env prometheus_multiproc_dir is not set or not a directory')
+            raise ValueError('env PROMETHEUS_MULTIPROC_DIR is not set or not a directory')
         self._path = path
         if registry:
             registry.register(self)
@@ -66,7 +71,7 @@ def _parse_key(key):
                     # the file is missing
                     continue
                 raise
-            for key, value, pos in file_values:
+            for key, value, _ in file_values:
                 metric_name, name, labels, labels_key = _parse_key(key)
 
                 metric = metrics.get(metric_name)
@@ -152,7 +157,7 @@ def collect(self):
 def mark_process_dead(pid, path=None):
     """Do bookkeeping for when one process dies in a multi-process setup."""
     if path is None:
-        path = os.environ.get('prometheus_multiproc_dir')
+        path = os.environ.get('PROMETHEUS_MULTIPROC_DIR')
     for f in glob.glob(os.path.join(path, 'gauge_livesum_{0}.db'.format(pid))):
         os.remove(f)
     for f in glob.glob(os.path.join(path, 'gauge_liveall_{0}.db'.format(pid))):

prometheus_client/values.py

@@ -2,6 +2,7 @@
 
 import os
 from threading import Lock
+import warnings
 
 from .mmap_dict import mmap_key, MmapedDict
 
@@ -51,6 +52,10 @@ class MmapedValue(object):
 
         def __init__(self, typ, metric_name, name, labelnames, labelvalues, multiprocess_mode='', **kwargs):
             self._params = typ, metric_name, name, labelnames, labelvalues, multiprocess_mode
+            # This deprecation warning can go away in a few releases when removing the compatibility
+            if 'prometheus_multiproc_dir' in os.environ and 'PROMETHEUS_MULTIPROC_DIR' not in os.environ:
+                os.environ['PROMETHEUS_MULTIPROC_DIR'] = os.environ['prometheus_multiproc_dir']
+                warnings.warn("prometheus_multiproc_dir variable has been deprecated in favor of the upper case naming PROMETHEUS_MULTIPROC_DIR", DeprecationWarning)
             with lock:
                 self.__check_for_pid_change()
                 self.__reset()
@@ -64,7 +69,7 @@ def __reset(self):
                 file_prefix = typ
             if file_prefix not in files:
                 filename = os.path.join(
-                    os.environ['prometheus_multiproc_dir'],
+                    os.environ.get('PROMETHEUS_MULTIPROC_DIR'),
                     '{0}_{1}.db'.format(file_prefix, pid['value']))
 
                 files[file_prefix] = MmapedDict(filename)
@@ -108,7 +113,7 @@ def get_value_class():
     # This needs to be chosen before the first metric is constructed,
     # and as that may be in some arbitrary library the user/admin has
     # no control over we use an environment variable.
-    if 'prometheus_multiproc_dir' in os.environ:
+    if 'prometheus_multiproc_dir' in os.environ or 'PROMETHEUS_MULTIPROC_DIR' in os.environ:
         return MultiProcessValue()
     else:
         return MutexValue

tests/test_multiprocess.py

@@ -5,6 +5,7 @@
 import shutil
 import sys
 import tempfile
+import warnings
 
 from prometheus_client import mmap_dict, values
 from prometheus_client.core import (
@@ -13,7 +14,9 @@
 from prometheus_client.multiprocess import (
     mark_process_dead, MultiProcessCollector,
 )
-from prometheus_client.values import MultiProcessValue, MutexValue
+from prometheus_client.values import (
+    get_value_class, MultiProcessValue, MutexValue,
+)
 
 if sys.version_info < (2, 7):
     # We need the skip decorators from unittest2 on Python 2.6.
@@ -22,20 +25,44 @@
     import unittest
 
 
-class TestMultiProcess(unittest.TestCase):
+class TestMultiProcessDeprecation(unittest.TestCase):
     def setUp(self):
         self.tempdir = tempfile.mkdtemp()
+
+    def tearDown(self):
+        del os.environ['prometheus_multiproc_dir']
+        del os.environ['PROMETHEUS_MULTIPROC_DIR']
+        values.ValueClass = MutexValue
+        shutil.rmtree(self.tempdir)
+
+    def test_deprecation_warning(self):
         os.environ['prometheus_multiproc_dir'] = self.tempdir
+        with warnings.catch_warnings(record=True) as w:
+            values.ValueClass = get_value_class()
+            registry = CollectorRegistry()
+            collector = MultiProcessCollector(registry)
+            Counter('c', 'help', registry=None)
+
+            assert os.environ['PROMETHEUS_MULTIPROC_DIR'] == self.tempdir
+            assert len(w) == 1
+            assert issubclass(w[-1].category, DeprecationWarning)
+            assert "PROMETHEUS_MULTIPROC_DIR" in str(w[-1].message)
+
+
+class TestMultiProcess(unittest.TestCase):
+    def setUp(self):
+        self.tempdir = tempfile.mkdtemp()
+        os.environ['PROMETHEUS_MULTIPROC_DIR'] = self.tempdir
         values.ValueClass = MultiProcessValue(lambda: 123)
         self.registry = CollectorRegistry()
-        self.collector = MultiProcessCollector(self.registry, self.tempdir)
+        self.collector = MultiProcessCollector(self.registry)
 
     @property
     def _value_class(self):
         return
 
     def tearDown(self):
-        del os.environ['prometheus_multiproc_dir']
+        del os.environ['PROMETHEUS_MULTIPROC_DIR']
         shutil.rmtree(self.tempdir)
         values.ValueClass = MutexValue
 
@@ -80,7 +107,7 @@ def test_gauge_all(self):
         self.assertEqual(0, self.registry.get_sample_value('g', {'pid': '456'}))
         g1.set(1)
         g2.set(2)
-        mark_process_dead(123, os.environ['prometheus_multiproc_dir'])
+        mark_process_dead(123)
         self.assertEqual(1, self.registry.get_sample_value('g', {'pid': '123'}))
         self.assertEqual(2, self.registry.get_sample_value('g', {'pid': '456'}))
 
@@ -94,7 +121,7 @@ def test_gauge_liveall(self):
         g2.set(2)
         self.assertEqual(1, self.registry.get_sample_value('g', {'pid': '123'}))
         self.assertEqual(2, self.registry.get_sample_value('g', {'pid': '456'}))
-        mark_process_dead(123, os.environ['prometheus_multiproc_dir'])
+        mark_process_dead(123, os.environ['PROMETHEUS_MULTIPROC_DIR'])
         self.assertEqual(None, self.registry.get_sample_value('g', {'pid': '123'}))
         self.assertEqual(2, self.registry.get_sample_value('g', {'pid': '456'}))
 
@@ -124,7 +151,7 @@ def test_gauge_livesum(self):
         g1.set(1)
         g2.set(2)
         self.assertEqual(3, self.registry.get_sample_value('g'))
-        mark_process_dead(123, os.environ['prometheus_multiproc_dir'])
+        mark_process_dead(123, os.environ['PROMETHEUS_MULTIPROC_DIR'])
         self.assertEqual(2, self.registry.get_sample_value('g'))
 
     def test_namespace_subsystem(self):
@@ -151,7 +178,7 @@ def test_initialization_detects_pid_change(self):
         # can not inspect the files cache directly, as it's a closure, so we
         # check for the actual files themselves
         def files():
-            fs = os.listdir(os.environ['prometheus_multiproc_dir'])
+            fs = os.listdir(os.environ['PROMETHEUS_MULTIPROC_DIR'])
             fs.sort()
             return fs
 
@@ -240,7 +267,7 @@ def add_label(key, value):
         pid = 1
         h.labels(**labels).observe(5)
 
-        path = os.path.join(os.environ['prometheus_multiproc_dir'], '*.db')
+        path = os.path.join(os.environ['PROMETHEUS_MULTIPROC_DIR'], '*.db')
         files = glob.glob(path)
         metrics = dict(
             (m.name, m) for m in self.collector.merge(files, accumulate=False)