diff --git a/CHANGELOG.md b/CHANGELOG.md index bbd3371593..3828829336 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/metrics/test_otlp_metrics_exporter.py b/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/metrics/test_otlp_metrics_exporter.py index b6d0d5b9b0..4a171b7986 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/metrics/test_otlp_metrics_exporter.py +++ b/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/metrics/test_otlp_metrics_exporter.py @@ -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, ), ), } diff --git a/exporter/opentelemetry-exporter-prometheus/tests/test_prometheus_exporter.py b/exporter/opentelemetry-exporter-prometheus/tests/test_prometheus_exporter.py index 092ed54481..445c9448ad 100644 --- a/exporter/opentelemetry-exporter-prometheus/tests/test_prometheus_exporter.py +++ b/exporter/opentelemetry-exporter-prometheus/tests/test_prometheus_exporter.py @@ -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}, ) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py index 8a61e78e72..c6ecff0cfa 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/aggregation.py @@ -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, ) @@ -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( @@ -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 diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/point.py b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/point.py index 8cc3a9bcd1..fcc127e949 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/point.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/_metrics/point.py @@ -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) @@ -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] diff --git a/opentelemetry-sdk/tests/metrics/test_aggregation.py b/opentelemetry-sdk/tests/metrics/test_aggregation.py index 7a00a8b321..99f3478267 100644 --- a/opentelemetry-sdk/tests/metrics/test_aggregation.py +++ b/opentelemetry-sdk/tests/metrics/test_aggregation.py @@ -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( @@ -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, @@ -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, @@ -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, ), ) diff --git a/opentelemetry-sdk/tests/metrics/test_point.py b/opentelemetry-sdk/tests/metrics/test_point.py index ff741115fc..0e5d99a726 100644 --- a/opentelemetry-sdk/tests/metrics/test_point.py +++ b/opentelemetry-sdk/tests/metrics/test_point.py @@ -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(), ) @@ -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(), )