From 159a96d81f7dd3fa5140a4f322ab04fbcee00e8f Mon Sep 17 00:00:00 2001 From: Mario de Frutos Date: Mon, 13 Sep 2021 10:20:17 +0200 Subject: [PATCH] Create multiprocess files with process uuid In order to be able to clean-up the process files once it's stopped or destroyed, we need to identify those files with an UUID. If not, those files are going to be processed and collected and will keep growing until we restart the server or master process to perform the cleanup of the metrics directory This change uses the psutil library that gives a hash based on PID+Process creation time in order to differentiate two processes with the same PID but that are different This change fixes https://github.com/prometheus/client_python/issues/568 Signed-off-by: Mario de Frutos --- prometheus_client/mmap_dict.py | 7 +++++++ prometheus_client/multiprocess.py | 23 ++++++++++++++++++----- prometheus_client/utils.py | 16 ++++++++++++++++ prometheus_client/values.py | 13 +++++++++---- setup.py | 3 +++ 5 files changed, 53 insertions(+), 9 deletions(-) diff --git a/prometheus_client/mmap_dict.py b/prometheus_client/mmap_dict.py index 615d0033..18f67aa6 100644 --- a/prometheus_client/mmap_dict.py +++ b/prometheus_client/mmap_dict.py @@ -134,6 +134,13 @@ def close(self): self._m.close() self._m = None self._f.close() + try: + # Given that we're using uuid for the process we can safely + # remove the file because is not going to be shared by other process + os.remove(self._f.name) + except OSError: + # In windows if the file is in use raises an error + pass self._f = None diff --git a/prometheus_client/multiprocess.py b/prometheus_client/multiprocess.py index 03e3f4d5..abcda82b 100644 --- a/prometheus_client/multiprocess.py +++ b/prometheus_client/multiprocess.py @@ -6,10 +6,12 @@ import os import warnings +import psutil + from .metrics_core import Metric from .mmap_dict import MmapedDict from .samples import Sample -from .utils import floatToGoString +from .utils import floatToGoString, get_process_data try: # Python3 FileNotFoundError @@ -158,7 +160,18 @@ 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', 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))): - os.remove(f) + try: + process = psutil.Process(pid) + types = ["counter", "histogram", "summary", "gauge_min", "gauge_max", "gauge_livesum", "gauge_liveall"] + for typ in types: + for f in glob.glob(os.path.join(path, '{0}_{1}_{2}.db'.format(typ, pid, process.__hash__()))): + os.remove(f) + except (psutil.NoSuchProcess, psutil.AccessDenied): + # We couldn't found the dead process, no further action required because with the uuid of the process there is + # no much we can do + pass + except OSError: + # Error deleting some of the files, for example in windows if they're in use they'll raise this error + # Maybe we can rename them as _defunk if needed to do a future cleanup + pass + diff --git a/prometheus_client/utils.py b/prometheus_client/utils.py index 1ff77c3c..e1850468 100644 --- a/prometheus_client/utils.py +++ b/prometheus_client/utils.py @@ -1,4 +1,7 @@ import math +import sys + +import psutil INF = float("inf") MINUS_INF = float("-inf") @@ -22,3 +25,16 @@ def floatToGoString(d): mantissa = '{0}.{1}{2}'.format(s[0], s[1:dot], s[dot + 1:]).rstrip('0.') return '{0}e+0{1}'.format(mantissa, dot - 1) return s + +def get_process_data(pid): + try: + process = psutil.Process(pid) + return process.__hash__() + except psutil.NoSuchProcess: + print('Calling process {0} is not running'.format(pid), file=sys.stderr) + raise + except psutil.AccessDenied: + print('Not enough permissions to access process {0}'.format(pid), file=sys.stderr) + raise + + diff --git a/prometheus_client/values.py b/prometheus_client/values.py index bccb38e9..f4f385bd 100644 --- a/prometheus_client/values.py +++ b/prometheus_client/values.py @@ -4,7 +4,9 @@ from threading import Lock import warnings + from .mmap_dict import mmap_key, MmapedDict +from .utils import get_process_data class MutexValue(object): @@ -39,7 +41,9 @@ def MultiProcessValue(process_identifier=os.getpid): """ files = {} values = [] - pid = {'value': process_identifier()} + pid = process_identifier() + pdata = {'id': pid, 'uuid': get_process_data(pid)} + # 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. @@ -70,7 +74,7 @@ def __reset(self): if file_prefix not in files: filename = os.path.join( os.environ.get('PROMETHEUS_MULTIPROC_DIR'), - '{0}_{1}.db'.format(file_prefix, pid['value'])) + '{0}_{1}_{2}.db'.format(file_prefix, pdata['id'], pdata['uuid'])) files[file_prefix] = MmapedDict(filename) self._file = files[file_prefix] @@ -79,8 +83,9 @@ def __reset(self): def __check_for_pid_change(self): actual_pid = process_identifier() - if pid['value'] != actual_pid: - pid['value'] = actual_pid + if pid['id'] != actual_pid: + pid['id'] = actual_pid + pid['uuid'] = get_process_data(actual_pid) # There has been a fork(), reset all the values. for f in files.values(): f.close() diff --git a/setup.py b/setup.py index 0469f935..f72a2e97 100644 --- a/setup.py +++ b/setup.py @@ -27,6 +27,9 @@ 'prometheus_client.openmetrics', 'prometheus_client.twisted', ], + install_requires=[ + 'psutil' + ], extras_require={ 'twisted': ['twisted'], },