prometheus / prometheus/client_python
MultiProcessCollector._accumulate_metrics always drops timestamps
还没有人认领这个 Issue。
- 主要语言
- Python
- 星标
- 4.4k
- 派生
- 876
- 平均合并
- 8 天 4 小时
- 30 天内合并 PR
- 1
描述
Summary
MultiProcessCollector._accumulate_metrics drops timestamps for all exported metrics, regardless of whether the selected sample or multiproc_mode used or needs them. This causes metrics file compaction routines (to cleanup old DB files from long-running web worker processes, for example) to drop samples that they otherwise wouldn't.
Details
MultiProcessCollector.merge calls _read_metrics to read all metrics (including dupes) from the DB files, followed by _accumulate_metrics to dedupe these based on multiproc_mode.
https://github.com/prometheus/client_python/blob/master/prometheus_client/multiprocess.py#L35-L44
In the docstring, it explicitly calls out the use case this affects (compaction by writing back to the mmap files):
But if writing the merged data back to mmap files, use
accumulate=False to avoid compound accumulation.
_accumulate_metrics considers the timestamp when deciding which sample to keep:
https://github.com/prometheus/client_python/blob/master/prometheus_client/multiprocess.py#L97-L116
However, it does not include this timestamp in the recreated sample it returns:
https://github.com/prometheus/client_python/blob/master/prometheus_client/multiprocess.py#L153
Impact
We have an internal compaction tool that effectively just periodically calls the MultiProcessCollector.merge method periodically, wrapped with an flock, and given the prevalence of open issues such as #568 I suspect others may too. Without this, long-running gunicorn processes with worker rotation settings will accumulate large numbers of stale files that slow scrape times significantly. This compaction ignores live pids, only compacting DBs from dead ones.
This issue results in this compaction potentially dropping samples that would otherwise have been the newest timestamp, simply because they've been compacted from a dead pid. Consider the following:
t1. process 1 and 2 spawn and define gauge my_gauge with multiproc_mode="mostrecent"
t2. process 1 samples my_gauge with timestamp=time.time()
t3. process 2 samples my_gauge with timestamp=time.time()
t4. process 2 dies
t5. compaction calls MultiProcessCollector.merge as part of stale DB compaction.
t6. MultiProcessCollector._accumulate_metrics returns the process 2 sample without a timestamp, which is then written to the compacted DB
t7. both future compaction and regular metrics exposition (via a scrape or otherwise) now drop the process 2 sample despite it being newest
Proposed Solution
Since the collector already tracks the sample timestamp in a defaultdict(float) and collection considers 0.0 and None equivalent, I think it should be as simple as changing the exported sample to this (or the equivalent):
timestamped_samples = []
for (name, label), value in samples.items():
without_pid_key = (name, tuple(l for l in labels if l[0] != 'pid'))
timestamped_samples.append(
prometheus_client.samples.Sample(
name_, dict(labels), value, sample_timestamps[without_pid_key]
)
)
metric.samples = timestamped_samples
贡献指南
从这里开始
- 先读完整个 Issue,再读项目的贡献指南。
- 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
- Fork 仓库,在一个分支上完成修改。
- 提交 Pull Request,并在描述里引用这个 Issue 编号。
调研方向
从 prometheus_client/multiprocess.py 开始,沿着 _read_metrics 和 _accumulate_metrics 跟踪 MultiProcessCollector.merge。验证在每种多进程模式下如何跟踪所选 sample 的时间戳,以及如何写入重新创建的 sample。完成的标准是压缩保留后续选择和 exposition 所需的时间戳,同时不改变不使用时间戳的模式。
由索引模型根据 Issue 内容生成。
评估
- 技术栈
- python
- 领域
- observability-sre
- Issue 类型
- 缺陷
- 难度
- 3/5
- 预计耗时
- 1-2 天
- 活跃度
- 停滞
- 描述清晰度
- 描述清楚
- 新手友好度
- 45/100