Skip to content

Commit

Permalink
Include min/max information in Histogram point (#2581)
Browse files Browse the repository at this point in the history 
* hist

* changelog

* tests

* agg

Co-authored-by: Diego Hurtado <ocelotl@users.noreply.github.com>
  • Loading branch information
@lzchen @ocelotl
lzchen and ocelotl authored Apr 13, 2022
1 parent cb60b54 commit ee2a0f4
Show file tree
Hide file tree
Showing 7 changed files with 115 additions and 65 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
([#2547](https://github.com/open-telemetry/opentelemetry-python/pull/2547))
- Update otlp-proto-grpc and otlp-proto-http exporters to have more lax requirements for `backoff` lib
([#2575](https://github.com/open-telemetry/opentelemetry-python/pull/2575))
- Add min/max to histogram point
([#2581](https://github.com/open-telemetry/opentelemetry-python/pull/2581))

## [1.10.0-0.29b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.10.0-0.29b0) - 2022-03-10

Expand Down
10 changes: 6 additions & 4 deletions exporter/opentelemetry-exporter-otlp-proto-grpc/tests/metrics/test_otlp_metrics_exporter.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,12 +117,14 @@ def setUp(self):
"histogram": _generate_metric(
"histogram",
Histogram(
time_unix_nano=1641946016139533244,
start_time_unix_nano=1641946016139533244,
aggregation_temporality=AggregationTemporality.DELTA,
bucket_counts=[1, 4],
sum=67,
explicit_bounds=[10.0, 20.0],
aggregation_temporality=AggregationTemporality.DELTA,
max=18,
min=8,
start_time_unix_nano=1641946016139533244,
sum=67,
time_unix_nano=1641946016139533244,
),
),
}
Expand Down
10 changes: 6 additions & 4 deletions exporter/opentelemetry-exporter-prometheus/tests/test_prometheus_exporter.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,14 @@ def test_histogram_to_prometheus(self):
record = _generate_metric(
"test@name",
Histogram(
time_unix_nano=1641946016139533244,
start_time_unix_nano=1641946016139533244,
aggregation_temporality=AggregationTemporality.CUMULATIVE,
bucket_counts=[1, 3, 2],
sum=579.0,
explicit_bounds=[123.0, 456.0],
aggregation_temporality=AggregationTemporality.CUMULATIVE,
start_time_unix_nano=1641946016139533244,
max=457,
min=1,
sum=579.0,
time_unix_nano=1641946016139533244,
),
attributes={"histo": 1},
)
Expand Down
26 changes: 20 additions & 6 deletions opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -267,18 +267,24 @@ def collect(self) -> HistogramPoint:
value = self._bucket_counts
start_time_unix_nano = self._start_time_unix_nano
histogram_sum = self._sum
histogram_max = self._max
histogram_min = self._min

self._bucket_counts = self._get_empty_bucket_counts()
self._start_time_unix_nano = now + 1
self._sum = 0
self._min = inf
self._max = -inf

return HistogramPoint(
start_time_unix_nano=start_time_unix_nano,
time_unix_nano=now,
aggregation_temporality=AggregationTemporality.DELTA,
bucket_counts=tuple(value),
explicit_bounds=self._boundaries,
aggregation_temporality=AggregationTemporality.DELTA,
max=histogram_max,
min=histogram_min,
start_time_unix_nano=start_time_unix_nano,
sum=histogram_sum,
time_unix_nano=now,
)


Expand Down Expand Up @@ -374,9 +380,15 @@ def _convert_aggregation_temporality(
if current_point.aggregation_temporality is aggregation_temporality:
return current_point

max_ = current_point.max
min_ = current_point.min

if aggregation_temporality is AggregationTemporality.CUMULATIVE:
start_time_unix_nano = previous_point.start_time_unix_nano
sum_ = current_point.sum + previous_point.sum
# Only update min/max on delta -> cumulative
max_ = max(current_point.max, previous_point.max)
min_ = min(current_point.min, previous_point.min)
bucket_counts = [
curr_count + prev_count
for curr_count, prev_count in zip(
Expand All @@ -394,12 +406,14 @@ def _convert_aggregation_temporality(
]

return HistogramPoint(
start_time_unix_nano=start_time_unix_nano,
time_unix_nano=current_point.time_unix_nano,
aggregation_temporality=aggregation_temporality,
bucket_counts=bucket_counts,
explicit_bounds=current_point.explicit_bounds,
max=max_,
min=min_,
start_time_unix_nano=start_time_unix_nano,
sum=sum_,
aggregation_temporality=aggregation_temporality,
time_unix_nano=current_point.time_unix_nano,
)
return None

Expand Down
12 changes: 7 additions & 5 deletions opentelemetry-sdk/src/opentelemetry/sdk/_metrics/point.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,11 @@ class AggregationTemporality(IntEnum):

@dataclass(frozen=True)
class Sum:
aggregation_temporality: AggregationTemporality
is_monotonic: bool
start_time_unix_nano: int
time_unix_nano: int
value: Union[int, float]
aggregation_temporality: AggregationTemporality
is_monotonic: bool


@dataclass(frozen=True)
Expand All @@ -45,12 +45,14 @@ class Gauge:

@dataclass(frozen=True)
class Histogram:
start_time_unix_nano: int
time_unix_nano: int
aggregation_temporality: AggregationTemporality
bucket_counts: Sequence[int]
explicit_bounds: Sequence[float]
max: int
min: int
start_time_unix_nano: int
sum: Union[int, float]
aggregation_temporality: AggregationTemporality
time_unix_nano: int


PointT = Union[Sum, Gauge, Histogram]
Expand Down
104 changes: 65 additions & 39 deletions opentelemetry-sdk/tests/metrics/test_aggregation.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -624,12 +624,14 @@ class TestHistogramConvertAggregationTemporality(TestCase):
def test_previous_point_none(self):

current_point = HistogramPoint(
start_time_unix_nano=0,
time_unix_nano=1,
aggregation_temporality=AggregationTemporality.DELTA,
bucket_counts=[0, 2, 1, 2, 0],
explicit_bounds=[0, 5, 10, 25],
max=24,
min=2,
start_time_unix_nano=0,
sum=70,
aggregation_temporality=AggregationTemporality.DELTA,
time_unix_nano=1,
)

self.assertEqual(
Expand All @@ -648,42 +650,50 @@ def test_previous_point_non_cumulative(self):

_convert_aggregation_temporality(
HistogramPoint(
start_time_unix_nano=0,
time_unix_nano=1,
aggregation_temporality=AggregationTemporality.DELTA,
bucket_counts=[0, 2, 1, 2, 0],
explicit_bounds=[0, 5, 10, 25],
max=24,
min=2,
start_time_unix_nano=0,
sum=70,
aggregation_temporality=AggregationTemporality.DELTA,
time_unix_nano=1,
),
HistogramPoint(
start_time_unix_nano=1,
time_unix_nano=2,
aggregation_temporality=AggregationTemporality.DELTA,
bucket_counts=[0, 1, 3, 0, 0],
explicit_bounds=[0, 5, 10, 25],
max=9,
min=2,
start_time_unix_nano=1,
sum=35,
aggregation_temporality=AggregationTemporality.DELTA,
time_unix_nano=2,
),
AggregationTemporality.DELTA,
),

def test_same_aggregation_temporality_cumulative(self):
current_point = HistogramPoint(
start_time_unix_nano=0,
time_unix_nano=2,
aggregation_temporality=AggregationTemporality.CUMULATIVE,
bucket_counts=[0, 3, 4, 2, 0],
explicit_bounds=[0, 5, 10, 25],
max=24,
min=1,
start_time_unix_nano=0,
sum=105,
aggregation_temporality=AggregationTemporality.CUMULATIVE,
time_unix_nano=2,
)
self.assertEqual(
_convert_aggregation_temporality(
HistogramPoint(
start_time_unix_nano=0,
time_unix_nano=1,
aggregation_temporality=AggregationTemporality.CUMULATIVE,
bucket_counts=[0, 2, 1, 2, 0],
explicit_bounds=[0, 5, 10, 25],
max=24,
min=1,
start_time_unix_nano=0,
sum=70,
aggregation_temporality=AggregationTemporality.CUMULATIVE,
time_unix_nano=1,
),
current_point,
AggregationTemporality.CUMULATIVE,
Expand All @@ -693,23 +703,27 @@ def test_same_aggregation_temporality_cumulative(self):

def test_same_aggregation_temporality_delta(self):
current_point = HistogramPoint(
start_time_unix_nano=1,
time_unix_nano=2,
aggregation_temporality=AggregationTemporality.DELTA,
bucket_counts=[0, 1, 3, 0, 0],
explicit_bounds=[0, 5, 10, 25],
max=9,
min=1,
start_time_unix_nano=1,
sum=35,
aggregation_temporality=AggregationTemporality.DELTA,
time_unix_nano=2,
)

self.assertEqual(
_convert_aggregation_temporality(
HistogramPoint(
start_time_unix_nano=0,
time_unix_nano=2,
aggregation_temporality=AggregationTemporality.CUMULATIVE,
bucket_counts=[0, 3, 4, 2, 0],
explicit_bounds=[0, 5, 10, 25],
max=24,
min=1,
start_time_unix_nano=0,
sum=105,
aggregation_temporality=AggregationTemporality.CUMULATIVE,
time_unix_nano=2,
),
current_point,
AggregationTemporality.DELTA,
Expand All @@ -719,67 +733,79 @@ def test_same_aggregation_temporality_delta(self):

def test_aggregation_temporality_to_cumulative(self):
current_point = HistogramPoint(
start_time_unix_nano=1,
time_unix_nano=2,
aggregation_temporality=AggregationTemporality.DELTA,
bucket_counts=[0, 1, 3, 0, 0],
explicit_bounds=[0, 5, 10, 25],
max=24,
min=1,
start_time_unix_nano=1,
sum=35,
aggregation_temporality=AggregationTemporality.DELTA,
time_unix_nano=2,
)

self.assertEqual(
_convert_aggregation_temporality(
HistogramPoint(
start_time_unix_nano=0,
time_unix_nano=1,
aggregation_temporality=AggregationTemporality.CUMULATIVE,
bucket_counts=[0, 2, 1, 2, 0],
explicit_bounds=[0, 5, 10, 25],
max=24,
min=1,
start_time_unix_nano=0,
sum=70,
aggregation_temporality=AggregationTemporality.CUMULATIVE,
time_unix_nano=1,
),
current_point,
AggregationTemporality.CUMULATIVE,
),
HistogramPoint(
start_time_unix_nano=0,
time_unix_nano=2,
aggregation_temporality=AggregationTemporality.CUMULATIVE,
bucket_counts=[0, 3, 4, 2, 0],
explicit_bounds=[0, 5, 10, 25],
max=24,
min=1,
start_time_unix_nano=0,
sum=105,
aggregation_temporality=AggregationTemporality.CUMULATIVE,
time_unix_nano=2,
),
)

def test_aggregation_temporality_to_delta(self):
current_point = HistogramPoint(
start_time_unix_nano=0,
time_unix_nano=2,
aggregation_temporality=AggregationTemporality.CUMULATIVE,
bucket_counts=[0, 3, 4, 2, 0],
explicit_bounds=[0, 5, 10, 25],
max=22,
min=3,
start_time_unix_nano=0,
sum=105,
aggregation_temporality=AggregationTemporality.CUMULATIVE,
time_unix_nano=2,
)

self.assertEqual(
_convert_aggregation_temporality(
HistogramPoint(
start_time_unix_nano=0,
time_unix_nano=1,
aggregation_temporality=AggregationTemporality.CUMULATIVE,
bucket_counts=[0, 2, 1, 2, 0],
explicit_bounds=[0, 5, 10, 25],
max=24,
min=1,
start_time_unix_nano=0,
sum=70,
aggregation_temporality=AggregationTemporality.CUMULATIVE,
time_unix_nano=1,
),
current_point,
AggregationTemporality.DELTA,
),
HistogramPoint(
start_time_unix_nano=1,
time_unix_nano=2,
aggregation_temporality=AggregationTemporality.DELTA,
bucket_counts=[0, 1, 3, 0, 0],
explicit_bounds=[0, 5, 10, 25],
max=22,
min=3,
start_time_unix_nano=1,
sum=35,
aggregation_temporality=AggregationTemporality.DELTA,
time_unix_nano=2,
),
)

Expand Down
16 changes: 9 additions & 7 deletions opentelemetry-sdk/tests/metrics/test_point.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,15 @@ class TestDatapointToJSON(TestCase):
def test_sum(self):
point = _create_metric(
Sum(
aggregation_temporality=2,
is_monotonic=True,
start_time_unix_nano=10,
time_unix_nano=20,
value=9,
aggregation_temporality=2,
is_monotonic=True,
)
)
self.assertEqual(
'{"attributes": {"attr-key": "test-val"}, "description": "test-description", "instrumentation_info": "InstrumentationInfo(name, version, None)", "name": "test-name", "resource": "BoundedAttributes({\'resource-key\': \'resource-val\'}, maxlen=None)", "unit": "test-unit", "point": {"start_time_unix_nano": 10, "time_unix_nano": 20, "value": 9, "aggregation_temporality": 2, "is_monotonic": true}}',
'{"attributes": {"attr-key": "test-val"}, "description": "test-description", "instrumentation_info": "InstrumentationInfo(name, version, None)", "name": "test-name", "resource": "BoundedAttributes({\'resource-key\': \'resource-val\'}, maxlen=None)", "unit": "test-unit", "point": {"aggregation_temporality": 2, "is_monotonic": true, "start_time_unix_nano": 10, "time_unix_nano": 20, "value": 9}}',
point.to_json(),
)

Expand All @@ -59,16 +59,18 @@ def test_gauge(self):
def test_histogram(self):
point = _create_metric(
Histogram(
start_time_unix_nano=50,
time_unix_nano=60,
aggregation_temporality=1,
bucket_counts=[0, 0, 1, 0],
explicit_bounds=[0.1, 0.5, 0.9, 1],
aggregation_temporality=1,
max=0.8,
min=0.8,
start_time_unix_nano=50,
sum=0.8,
time_unix_nano=60,
)
)
self.maxDiff = None
self.assertEqual(
'{"attributes": {"attr-key": "test-val"}, "description": "test-description", "instrumentation_info": "InstrumentationInfo(name, version, None)", "name": "test-name", "resource": "BoundedAttributes({\'resource-key\': \'resource-val\'}, maxlen=None)", "unit": "test-unit", "point": {"start_time_unix_nano": 50, "time_unix_nano": 60, "bucket_counts": [0, 0, 1, 0], "explicit_bounds": [0.1, 0.5, 0.9, 1], "sum": 0.8, "aggregation_temporality": 1}}',
'{"attributes": {"attr-key": "test-val"}, "description": "test-description", "instrumentation_info": "InstrumentationInfo(name, version, None)", "name": "test-name", "resource": "BoundedAttributes({\'resource-key\': \'resource-val\'}, maxlen=None)", "unit": "test-unit", "point": {"aggregation_temporality": 1, "bucket_counts": [0, 0, 1, 0], "explicit_bounds": [0.1, 0.5, 0.9, 1], "max": 0.8, "min": 0.8, "start_time_unix_nano": 50, "sum": 0.8, "time_unix_nano": 60}}',
point.to_json(),
)

0 comments on commit ee2a0f4

Please sign in to comment.