Skip to content

Commit

Permalink
Add max_scale option (#3323)
Browse files Browse the repository at this point in the history
  • Loading branch information
ocelotl committed Jun 13, 2023
1 parent 92bfd08 commit 2f804fa
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 2 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -955,6 +965,7 @@ def _create_aggregation(
attributes,
start_time_unix_nano,
max_size=self._max_size,
max_scale=self._max_scale,
)


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
Expand Down Expand Up @@ -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)))
Expand Down

0 comments on commit 2f804fa

Please sign in to comment.