Skip to content

Commit 9b5b470

Browse files
committed
Add max_scale option
Fixes #3130
1 parent 46757ff commit 9b5b470

File tree

3 files changed

+53
-2
lines changed

3 files changed

+53
-2
lines changed

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
66
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
77

88
## Unreleased
9+
10+
- Add max_scale option to Exponential Bucket Histogram Aggregation [#3323](https://github.com/open-telemetry/opentelemetry-python/pull/3323))
911
- 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))
12+
1013
## Version 1.18.0/0.39b0 (2023-05-04)
1114

1215
- Select histogram aggregation with an environment variable

opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/aggregation.py

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,7 @@ def __init__(
386386
# See the derivation here:
387387
# https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/sdk.md#exponential-bucket-histogram-aggregation)
388388
max_size: int = 160,
389+
max_scale: int = 20,
389390
):
390391
super().__init__(attributes)
391392
# max_size is the maximum capacity of the positive and negative
@@ -403,6 +404,7 @@ def __init__(
403404
)
404405

405406
self._max_size = max_size
407+
self._max_scale = max_scale
406408

407409
# _sum is the sum of all the values aggregated by this aggregator.
408410
self._sum = 0
@@ -428,7 +430,14 @@ def __init__(
428430

429431
# _mapping corresponds to the current scale, is shared by both the
430432
# positive and negative buckets.
431-
self._mapping = LogarithmMapping(LogarithmMapping._max_scale)
433+
434+
if self._max_scale > 20:
435+
_logger.warning(
436+
"max_scale is set to %s which is "
437+
"larger than the recommended value of 20",
438+
self._max_scale,
439+
)
440+
self._mapping = LogarithmMapping(self._max_scale)
432441

433442
self._instrument_temporality = AggregationTemporality.DELTA
434443
self._start_time_unix_nano = start_time_unix_nano
@@ -941,9 +950,10 @@ class ExponentialBucketHistogramAggregation(Aggregation):
941950
def __init__(
942951
self,
943952
max_size: int = 160,
953+
max_scale: int = 20,
944954
):
945-
946955
self._max_size = max_size
956+
self._max_scale = max_scale
947957

948958
def _create_aggregation(
949959
self,
@@ -955,6 +965,7 @@ def _create_aggregation(
955965
attributes,
956966
start_time_unix_nano,
957967
max_size=self._max_size,
968+
max_scale=self._max_scale,
958969
)
959970

960971

opentelemetry-sdk/tests/metrics/exponential_histogram/test_exponential_bucket_histogram_aggregation.py

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
# limitations under the License.
1414

1515
from itertools import permutations
16+
from logging import WARNING
1617
from math import ldexp
1718
from sys import float_info
1819
from types import MethodType
@@ -37,6 +38,9 @@
3738
LogarithmMapping,
3839
)
3940
from opentelemetry.sdk.metrics._internal.measurement import Measurement
41+
from opentelemetry.sdk.metrics.view import (
42+
ExponentialBucketHistogramAggregation,
43+
)
4044

4145

4246
def get_counts(buckets: Buckets) -> int:
@@ -77,6 +81,39 @@ def swap(
7781

7882

7983
class TestExponentialBucketHistogramAggregation(TestCase):
84+
@patch("opentelemetry.sdk.metrics._internal.aggregation.LogarithmMapping")
85+
def test_create_aggregation(self, mock_logarithm_mapping):
86+
exponential_bucket_histogram_aggregation = (
87+
ExponentialBucketHistogramAggregation()
88+
)._create_aggregation(Mock(), Mock(), Mock())
89+
90+
self.assertEqual(
91+
exponential_bucket_histogram_aggregation._max_scale, 20
92+
)
93+
94+
mock_logarithm_mapping.assert_called_with(20)
95+
96+
exponential_bucket_histogram_aggregation = (
97+
ExponentialBucketHistogramAggregation(max_scale=10)
98+
)._create_aggregation(Mock(), Mock(), Mock())
99+
100+
self.assertEqual(
101+
exponential_bucket_histogram_aggregation._max_scale, 10
102+
)
103+
104+
mock_logarithm_mapping.assert_called_with(10)
105+
106+
with self.assertLogs(level=WARNING):
107+
exponential_bucket_histogram_aggregation = (
108+
ExponentialBucketHistogramAggregation(max_scale=100)
109+
)._create_aggregation(Mock(), Mock(), Mock())
110+
111+
self.assertEqual(
112+
exponential_bucket_histogram_aggregation._max_scale, 100
113+
)
114+
115+
mock_logarithm_mapping.assert_called_with(100)
116+
80117
def assertInEpsilon(self, first, second, epsilon):
81118
self.assertLessEqual(first, (second * (1 + epsilon)))
82119
self.assertGreaterEqual(first, (second * (1 - epsilon)))

0 commit comments

Comments
 (0)