Add sum, livemin, and livemax multiprocess modes for Gauges
#794
Conversation
Signed-off-by: Josh Karpel <josh.karpel@gmail.com>
|
Thanks! I should have some time next week to give this a review. |
README.md
Outdated
| - 'min': Return a single timeseries that is the minimum of the values of all processes, alive or dead. | ||
| - 'livemin': Return a single timeseries that is the minimum of the values of alive processes. |
There was a problem hiding this comment.
since the support is comprehensive with this change, wonder if we should simplify this to be something like:
- 'all': Default. Return a timeseries per process (alive or dead)
- 'sum': Return a single timeseries that is the some of the values of all processes (alive or dead).
- 'max': Return a single timeseries that is the maximum of the values of all processes (alive or dead).
- 'min': Return a single timeseries that is the minimum of the values of all processes (alive or dead).
Prepend 'live' to the beginning of the mode to return the same result but only considering living processes
- eg: 'liveall, 'livesum', 'livemax', 'livemin'
prometheus_client/multiprocess.py
Outdated
| @@ -63,7 +64,7 @@ def _parse_key(key): | |||
| try: | |||
| file_values = MmapedDict.read_all_values_from_file(f) | |||
| except FileNotFoundError: | |||
| if typ == 'gauge' and parts[1] in ('liveall', 'livesum'): | |||
| if typ == 'gauge' and 'live' in parts[1]: | |||
prometheus_client/multiprocess.py
Outdated
| os.remove(f) | ||
| for f in glob.glob(os.path.join(path, f'gauge_liveall_{pid}.db')): | ||
| os.remove(f) | ||
| for mode in {m for m in Gauge._MULTIPROC_MODES if 'live' in m}: |
There was a problem hiding this comment.
if we agree on the previous one then probably would want to make the same change here: m.startswith('live')
Signed-off-by: Josh Karpel <josh.karpel@gmail.com>
|
Some personal stuff came up and it is looking unlikely I am going to be able to review this PR this week. Just wanted to let you know that I have not forgotten about it! |
No worries, and no rush! Hope everything turns out ok! |
Signed-off-by: Josh Karpel <josh.karpel@gmail.com>
My pleasure! Thanks for reviewing! |
Hi!
I work on a multiprocess Flask application and we recently found ourselves wanting the equivalent of the
maxmultiprocessing mode, but only for live processes. I came here to poke around the code and also noticed this recent issue asking for asummode #793 , so it seems like there's some interest in more multiprocess modes.If I understand correctly, the core change needed to support this is actually around the deletion of the files when a process dies - files for
live*metrics should be deleted, others should not. So I added the new modes with the same collection logic as their non-live counterparts and expanded the check inmark_process_dead()to automatically include any metric withliveas part of its multiprocess mode name (this might be excessively fancy, but it seemed useful to reduce potential future changes).I also added tests to cover these new modes based on the existing tests and updated the
README.mdto describe the new modes.