diff --git a/.changelog/5462.fixed b/.changelog/5462.fixed new file mode 100644 index 00000000000..280c3181e45 --- /dev/null +++ b/.changelog/5462.fixed @@ -0,0 +1 @@ +`opentelemetry-sdk`: fill every bucket of `SimpleFixedSizeExemplarReservoir` before random sampling diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/exemplar/exemplar_reservoir.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/exemplar/exemplar_reservoir.py index afe2dcba38a..dd0a956c9dd 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/exemplar/exemplar_reservoir.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/exemplar/exemplar_reservoir.py @@ -255,10 +255,11 @@ def _find_bucket_index( attributes: Attributes, context: Context, ) -> int: - self._measurements_seen += 1 if self._measurements_seen < self._size: + self._measurements_seen += 1 return self._measurements_seen - 1 + self._measurements_seen += 1 index = randrange(0, self._measurements_seen) if index < self._size: return index diff --git a/opentelemetry-sdk/tests/metrics/test_exemplarreservoir.py b/opentelemetry-sdk/tests/metrics/test_exemplarreservoir.py index 9c5bf69bdb3..7078bd41424 100644 --- a/opentelemetry-sdk/tests/metrics/test_exemplarreservoir.py +++ b/opentelemetry-sdk/tests/metrics/test_exemplarreservoir.py @@ -63,6 +63,28 @@ def test_filter_attributes(self): self.assertIn("key1", exemplars[0].filtered_attributes) self.assertNotIn("key2", exemplars[0].filtered_attributes) + def test_fills_every_bucket_before_sampling(self): + # The first `size` measurements must each land in their own bucket, + # otherwise the reservoir never reaches its configured capacity. + for size in (1, 2, 4, 10): + with self.subTest(size=size): + reservoir = SimpleFixedSizeExemplarReservoir(size) + + for value in range(size): + reservoir.offer( + float(value), + time_ns(), + {"attribute": "value"}, + Context(), + ) + + exemplars = reservoir.collect({}) + self.assertEqual(len(exemplars), size) + self.assertEqual( + sorted(exemplar.value for exemplar in exemplars), + [float(value) for value in range(size)], + ) + def test_reset_after_collection(self): reservoir = SimpleFixedSizeExemplarReservoir(4)