prometheus / prometheus/client_python

MultiProcessCollector._accumulate_metrics always drops timestamps

Aperta
#1,056 1 commento 0 reazioni 0 assegnatari Vedi su GitHub

Nessuno ha ancora preso questa issue.

Lingua principale
Python
Stelle
4.4k
Fork
876
Merge medio
8g 4h
PR unite (30g)
1

Descrizione

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

Guida per i contributori

Apri la guida per i contributori

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Direzione di ricerca

Inizia in prometheus_client/multiprocess.py, seguendo MultiProcessCollector.merge attraverso _read_metrics e _accumulate_metrics. Verifica come viene tenuto traccia del timestamp del sample selezionato per ciascuna modalità multiprocesso e come viene scritto il sample ricreato. Il lavoro è completato quando la compattazione preserva il timestamp necessario per la selezione e l’esposizione successive, senza modificare le modalità che non utilizzano timestamp.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Valutazione

Stack tecnologico
python
Ambito
observability-sre
Tipo di issue
Bug
Difficoltà
3/5
Tempo stimato
1-2 giorni
Stato di attività
Ferma
Chiarezza
Specificata chiaramente
Idoneità per principianti
45/100

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.