Skip to content

Commit

Permalink
Revert "Fix explicit bucket histogram aggregation"
Browse files Browse the repository at this point in the history
This reverts commit f1c6683.
  • Loading branch information
ocelotl committed Feb 7, 2024
1 parent 4bd0961 commit fc5aca6
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 313 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -398,12 +398,8 @@ def __init__(
record_min_max: bool = True,
):
super().__init__(attributes)

self._start_time_unix_nano = start_time_unix_nano
self._boundaries = tuple(boundaries)
self._record_min_max = record_min_max

self._current_value = None
self._bucket_counts = self._get_empty_bucket_counts()
self._min = inf
self._max = -inf
self._sum = 0
Expand All @@ -420,19 +416,15 @@ def _get_empty_bucket_counts(self) -> List[int]:

def aggregate(self, measurement: Measurement) -> None:

with self._lock:
value = measurement.value

if self._current_value is None:
self._current_value = self._get_empty_bucket_counts()
if self._record_min_max:
self._min = min(self._min, value)
self._max = max(self._max, value)

value = measurement.value
self._sum += value

if self._record_min_max:
self._min = min(self._min, value)
self._max = max(self._max, value)

self._current_value[bisect_left(self._boundaries, value)] += 1
self._sum = self._sum + value
self._bucket_counts[bisect_left(self._boundaries, value)] += 1

def collect(
self,
Expand All @@ -443,8 +435,14 @@ def collect(
Atomically return a point for the current value of the metric.
"""
with self._lock:
if not any(self._bucket_counts):
return None

if aggregation_temporality is AggregationTemporality.DELTA:
bucket_counts = self._bucket_counts
start_time_unix_nano = self._start_time_unix_nano
sum_ = self._sum
max_ = self._max
min_ = self._min

self._bucket_counts = self._get_empty_bucket_counts()
self._start_time_unix_nano = collection_start_nano
Expand Down Expand Up @@ -489,39 +487,31 @@ def collect(
current_point.bucket_counts,
self._previous_point.bucket_counts,
)

self._current_value = None
self._min = inf
self._max = -inf
self._sum = 0
self._previous_collection_start_nano = collection_start_nano

if current_value is None:
return None

return HistogramDataPoint(
attributes=self._attributes,
start_time_unix_nano=previous_collection_start_nano,
time_unix_nano=collection_start_nano,
count=sum(current_value),
sum=sum_,
bucket_counts=tuple(current_value),
explicit_bounds=self._boundaries,
min=min_,
max=max_,
]
else:
start_time_unix_nano = self._previous_point.time_unix_nano
sum_ = current_point.sum - self._previous_point.sum
bucket_counts = [
curr_count - prev_count
for curr_count, prev_count in zip(
current_point.bucket_counts,
self._previous_point.bucket_counts,
)
]

return HistogramDataPoint(
attributes=self._attributes,
start_time_unix_nano=self._start_time_unix_nano,
time_unix_nano=collection_start_nano,
count=sum(self._current_value),
sum=self._sum,
bucket_counts=tuple(self._current_value),
explicit_bounds=self._boundaries,
min=self._min,
max=self._max,
)
current_point = HistogramDataPoint(
attributes=self._attributes,
start_time_unix_nano=start_time_unix_nano,
time_unix_nano=current_point.time_unix_nano,
count=sum(bucket_counts),
sum=sum_,
bucket_counts=tuple(bucket_counts),
explicit_bounds=current_point.explicit_bounds,
min=min_,
max=max_,
)
self._previous_point = current_point
return current_point


# pylint: disable=protected-access
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -65,26 +65,17 @@ def test_histogram_counter_collection(self):

metric_data = in_memory_metric_reader.get_metrics_data()

# FIXME ExplicitBucketHistogramAggregation is resetting counts to zero
# even if aggregation temporality is cumulative.
self.assertEqual(
len(metric_data.resource_metrics[0].scope_metrics[0].metrics), 2
len(metric_data.resource_metrics[0].scope_metrics[0].metrics), 1
)

self.assertEqual(
(
metric_data.resource_metrics[0]
.scope_metrics[0]
.metrics[0]
.data.data_points[0]
.bucket_counts
),
(0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0),
)
self.assertEqual(
(
metric_data.resource_metrics[0]
.scope_metrics[0]
.metrics[1]
.data.data_points[0]
.value
),
1,
Expand Down
Loading

0 comments on commit fc5aca6

Please sign in to comment.