Skip to content

refactor(prometheus): simplify _CustomCollector by replacing deque with Optional[MetricsData]#5258

Open
sridhar-3009 wants to merge 1 commit into
open-telemetry:mainfrom
sridhar-3009:refactor/prometheus-reader-simplify-deque
Open

refactor(prometheus): simplify _CustomCollector by replacing deque with Optional[MetricsData]#5258
sridhar-3009 wants to merge 1 commit into
open-telemetry:mainfrom
sridhar-3009:refactor/prometheus-reader-simplify-deque

Conversation

@sridhar-3009

Copy link
Copy Markdown

Summary

Fixes #2500 (referenced in PR #2321 discussion).

The internal _CustomCollector used a deque[MetricsData] to buffer metric data between the _receive_metrics → add_metrics_data call and the Prometheus collect() scrape. In practice the deque holds at most one item per scrape cycle because _callback() triggers exactly one _receive_metrics invocation before collect() drains the queue.

This PR simplifies the flow:

  • Replace deque[MetricsData] with MetricsData | None
  • add_metrics_data assigns directly instead of appending
  • collect() reads and clears the single field, replacing the while … popleft() loop
  • Remove the now-unused from collections import deque import

This also fixes a latent correctness issue: with the deque, if add_metrics_data were called multiple times between scrapes, collect() would yield duplicate metric family entries (because metric_family_id_metric_family accumulated across the loop but was yielded inside the loop).

Before

collect() → callback() → _receive_metrics() → add_metrics_data() → deque.append()
         → while deque: popleft() → translate → yield (inside loop, potential duplicates)

After

collect() → callback() → _receive_metrics() → add_metrics_data() → self._metrics_data = …
         → translate single MetricsData → yield once

Test plan

  • Existing Prometheus exporter tests pass
  • Manual scrape endpoint returns correct metrics
  • No metrics emitted when no data collected (metrics_data is None)

…th single MetricsData field

The callback → _receive_metrics → add_metrics_data → deque → yield chain
always held at most one MetricsData per collect cycle (the callback
triggers exactly one _receive_metrics call before collect drains the
queue). Replace the deque[MetricsData] with an Optional[MetricsData]
field and simplify collect() to consume it directly, removing the
while-loop, popleft(), len() check, and the now-unused `deque` import.

This fixes a latent bug where multiple items in the deque would have
caused collect() to yield duplicate Prometheus metric families.

Fixes open-telemetry#2500
@sridhar-3009 sridhar-3009 requested a review from a team as a code owner May 30, 2026 08:14
@linux-foundation-easycla

linux-foundation-easycla Bot commented May 30, 2026

Copy link
Copy Markdown

CLA Signed
The committers listed above are authorized under a signed CLA.

  • ✅ login: sridhar-3009 / name: Sai Sridhar (db88905)

@tammy-baylis-swi tammy-baylis-swi moved this to Ready for review in Python PR digest Jun 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Ready for review

Development

Successfully merging this pull request may close these issues.

simplify flow of prometheus metric reader

2 participants