ValueError: empty range for randrange() — data race in SimpleFixedSizeExemplarReservoir between offer() and collect()

[deepanshi-s]

Describe your environment

opentelemetry-sdk: 1.40.0
Python: 3.9 (reproduces reliably); does not reproduce on 3.12 due to GIL switch-interval differences
OS: Linux

What happened?

With the default trace_based exemplar filter, recording a metric inside a sampled span while a PeriodicExportingMetricReader runs its collection cycle intermittently crashes the recording thread:

File ".../sdk/metrics/_internal/exemplar/exemplar_reservoir.py", line 279, in _find_bucket_index
    index = randrange(0, self._measurements_seen)
ValueError: empty range for randrange() (0, 0, 0)

Root cause

SimpleFixedSizeExemplarReservoir._find_bucket_index is unsynchronized:

self._measurements_seen += 1
index = randrange(0, self._measurements_seen)

The exporter thread calls collect() → _reset(), setting _measurements_seen = 0 between those two lines. offer()/collect() on the reservoir share _measurements_seen with no lock, so the recording thread executes randrange(0, 0). In an application this exception propagates out of counter.add() and can kill the calling thread.

Steps to Reproduce

import threading
from opentelemetry.context import Context
from opentelemetry.sdk.metrics._internal.exemplar import (
    SimpleFixedSizeExemplarReservoir,
)

res = SimpleFixedSizeExemplarReservoir()
stop = threading.Event()

def offerer():
    ctx = Context()
    while not stop.is_set():
        res.offer(0, 0, {}, ctx)

def collector():
    while not stop.is_set():
        res.collect({})

t = threading.Thread(target=offerer)
c = threading.Thread(target=collector, daemon=True)
t.start()
c.start()
t.join()  # ValueError: empty range for randrange() (0, 0, 0)

Expected Result

Concurrent offer() and collect() on an exemplar reservoir should be safe. Metric recording (counter.add(), histogram.record()) must never raise as a result of a race with the SDK's own export thread.

Actual Result

The recording thread raises ValueError: empty range for randrange() (0, 0, 0) from inside _find_bucket_index. Because it originates in the metrics recording path, the exception propagates out of counter.add() into application code and crashes the calling thread.

Additional context

Guard the reservoir's offer / _find_bucket_index / collect / _reset with a lock, or make _find_bucket_index tolerant of _measurements_seen == 0.

Would you like to implement a fix?

No response

Tip

No response

[saisharan0103]

This appears to hit the unsynchronized _measurements_seen update in opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/exemplar/exemplar_reservoir.py, specifically SimpleFixedSizeExemplarReservoir._find_bucket_index. The key race is that collect() can reset the counter between incrementing it and calling randrange(0, self._measurements_seen), leaving the recording path with randrange(0, 0). For a PR, I would start by making offer() and collect() coordinate around the reservoir state reset and bucket selection.

[feiiiiii5]

I'd like to take this one if no one else is on it.

The root-cause analysis in the body is clear: SimpleFixedSizeExemplarReservoir._find_bucket_index reads/writes _measurements_seen without synchronization, and collect() → _reset() zeroes it between the increment and the randrange() call, producing randrange(0, 0).

For the fix direction, I'm planning to go with (1) a lock rather than (2) tolerating _measurements_seen == 0, for these reasons:

• A lock fixes the race for all the shared state, not just the randrange call. _reservoir_storage (the defaultdict[ExemplarBucket]) is also read in collect() while offer() may be inserting a new bucket — a defaultdict is not safe for concurrent read+write on CPython without the GIL guarantee, and even with the GIL the iteration in collect() can raise RuntimeError: dictionary changed size during iteration if a new bucket gets inserted mid-iteration. Just making _find_bucket_index tolerant of _measurements_seen == 0 would mask the crash but leave that underlying dict race intact.
• The tradeoff is taking a lock on every offer(). That's already the case for the SDK's aggregation paths (the _telemetry_metrics_adjuster / StreamAggregator use locks), so adding one here keeps the exemplar reservoir consistent with the rest of the metric collection pipeline. Lock acquisition cost is dwarfed by the work offer() already does (looking up trace.get_current_span, building attributes, etc).

I'll add a threading.Lock on FixedSizeExemplarReservoirABC, guard offer() / _find_bucket_index() (when called transitively from offer()) / collect() / _reset() with it, and add a regression test that mirrors the issue's repro (spawn an offer thread + a collector thread on Python 3.9, assert no ValueError after a bounded run). The existing exemplar_reservoir tests already cover the single-threaded happy path, so I'll keep those untouched.

If a maintainer prefers direction (2) — tolerate _measurements_seen == 0 and separately address the defaultdict race — I'm happy to switch. Just let me know.