diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ce6fc4251..0f10920db7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## Unreleased + +- Add max_scale option to Exponential Bucket Histogram Aggregation [#3323](https://github.com/open-telemetry/opentelemetry-python/pull/3323)) - Use BoundedAttributes instead of raw dict to extract attributes from LogRecord and Support dropped_attributes_count in LogRecord ([#3310](https://github.com/open-telemetry/opentelemetry-python/pull/3310)) + ## Version 1.18.0/0.39b0 (2023-05-04) - Select histogram aggregation with an environment variable diff --git a/README.md b/README.md index ce1bf2c59d..e3923e8fe5 100644 --- a/README.md +++ b/README.md @@ -99,6 +99,7 @@ Meeting notes are available as a public [Google doc](https://docs.google.com/doc Approvers ([@open-telemetry/python-approvers](https://github.com/orgs/open-telemetry/teams/python-approvers)): - [Aaron Abbott](https://github.com/aabmass), Google +- [Jeremy Voss](https://github.com/jeremydvoss), Microsoft - [Sanket Mehta](https://github.com/sanketmehta28), Cisco Emeritus Approvers @@ -106,7 +107,7 @@ Emeritus Approvers - [Ashutosh Goel](https://github.com/ashu658), Cisco - [Carlos Alberto Cortez](https://github.com/carlosalberto), Lightstep - [Christian Neumüller](https://github.com/Oberon00), Dynatrace -- [Hector Hernandez](https://github.com/hectorhdzg), Microsoft +- [Héctor Hernández](https://github.com/hectorhdzg), Microsoft - [Mauricio Vásquez](https://github.com/mauriciovasquezbernal), Kinvolk - [Nathaniel Ruiz Nowell](https://github.com/NathanielRN), AWS - [Tahir H. Butt](https://github.com/majorgreys) DataDog diff --git a/opentelemetry-api/pyproject.toml b/opentelemetry-api/pyproject.toml index 5b2ac54af1..dcc9a2f168 100644 --- a/opentelemetry-api/pyproject.toml +++ b/opentelemetry-api/pyproject.toml @@ -29,7 +29,7 @@ dependencies = [ "setuptools >= 16.0", # FIXME This should be able to be removed after 3.12 is released if there is a reliable API # in importlib.metadata. - "importlib-metadata ~= 6.0.0", + "importlib-metadata ~= 6.0", ] dynamic = [ "version", diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py index 7312df1aa7..ae21db907d 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py @@ -386,6 +386,7 @@ def __init__( # See the derivation here: # https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#exponential-bucket-histogram-aggregation) max_size: int = 160, + max_scale: int = 20, ): super().__init__(attributes) # max_size is the maximum capacity of the positive and negative @@ -403,6 +404,7 @@ def __init__( ) self._max_size = max_size + self._max_scale = max_scale # _sum is the sum of all the values aggregated by this aggregator. self._sum = 0 @@ -428,7 +430,14 @@ def __init__( # _mapping corresponds to the current scale, is shared by both the # positive and negative buckets. - self._mapping = LogarithmMapping(LogarithmMapping._max_scale) + + if self._max_scale > 20: + _logger.warning( + "max_scale is set to %s which is " + "larger than the recommended value of 20", + self._max_scale, + ) + self._mapping = LogarithmMapping(self._max_scale) self._instrument_temporality = AggregationTemporality.DELTA self._start_time_unix_nano = start_time_unix_nano @@ -941,9 +950,10 @@ class ExponentialBucketHistogramAggregation(Aggregation): def __init__( self, max_size: int = 160, + max_scale: int = 20, ): - self._max_size = max_size + self._max_scale = max_scale def _create_aggregation( self, @@ -955,6 +965,7 @@ def _create_aggregation( attributes, start_time_unix_nano, max_size=self._max_size, + max_scale=self._max_scale, ) diff --git a/opentelemetry-sdk/tests/metrics/exponential_histogram/test_exponential_bucket_histogram_aggregation.py b/opentelemetry-sdk/tests/metrics/exponential_histogram/test_exponential_bucket_histogram_aggregation.py index 65a437bc6d..9bea75e426 100644 --- a/opentelemetry-sdk/tests/metrics/exponential_histogram/test_exponential_bucket_histogram_aggregation.py +++ b/opentelemetry-sdk/tests/metrics/exponential_histogram/test_exponential_bucket_histogram_aggregation.py @@ -13,6 +13,7 @@ # limitations under the License. from itertools import permutations +from logging import WARNING from math import ldexp from sys import float_info from types import MethodType @@ -37,6 +38,9 @@ LogarithmMapping, ) from opentelemetry.sdk.metrics._internal.measurement import Measurement +from opentelemetry.sdk.metrics.view import ( + ExponentialBucketHistogramAggregation, +) def get_counts(buckets: Buckets) -> int: @@ -77,6 +81,39 @@ def swap( class TestExponentialBucketHistogramAggregation(TestCase): + @patch("opentelemetry.sdk.metrics._internal.aggregation.LogarithmMapping") + def test_create_aggregation(self, mock_logarithm_mapping): + exponential_bucket_histogram_aggregation = ( + ExponentialBucketHistogramAggregation() + )._create_aggregation(Mock(), Mock(), Mock()) + + self.assertEqual( + exponential_bucket_histogram_aggregation._max_scale, 20 + ) + + mock_logarithm_mapping.assert_called_with(20) + + exponential_bucket_histogram_aggregation = ( + ExponentialBucketHistogramAggregation(max_scale=10) + )._create_aggregation(Mock(), Mock(), Mock()) + + self.assertEqual( + exponential_bucket_histogram_aggregation._max_scale, 10 + ) + + mock_logarithm_mapping.assert_called_with(10) + + with self.assertLogs(level=WARNING): + exponential_bucket_histogram_aggregation = ( + ExponentialBucketHistogramAggregation(max_scale=100) + )._create_aggregation(Mock(), Mock(), Mock()) + + self.assertEqual( + exponential_bucket_histogram_aggregation._max_scale, 100 + ) + + mock_logarithm_mapping.assert_called_with(100) + def assertInEpsilon(self, first, second, epsilon): self.assertLessEqual(first, (second * (1 + epsilon))) self.assertGreaterEqual(first, (second * (1 - epsilon)))