Skip to content

Commit 742593b

Browse files
chimchim89xrmx
andauthored
Fix logarithm mapping max scale comment and align with Go implementation (#4981)
Co-authored-by: Riccardo Magliocchetti <riccardo.magliocchetti@gmail.com>
1 parent 6929c69 commit 742593b

File tree

1 file changed

+6
-2
lines changed
  • opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/exponential_histogram/mapping

1 file changed

+6
-2
lines changed

opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/exponential_histogram/mapping/logarithm_mapping.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,12 @@ def _get_min_scale(self):
4747
return self._min_scale
4848

4949
def _get_max_scale(self):
50-
# FIXME The Go implementation uses a value of 20 here, find out the
51-
# right value for this implementation, more information here:
50+
# _max_scale is 20. The OpenTelemetry specification requires that
51+
# bucket indices fit within a signed 32-bit integer. At scale 20,
52+
# the maximum bucket index is ((MAX_NORMAL_EXPONENT + 1) << 20) - 1,
53+
# which fits within this range. At scale 21, the maximum bucket
54+
# index reaches the upper limit of a signed 32-bit integer, making
55+
# it difficult to test correctness. See:
5256
# https://github.com/lightstep/otel-launcher-go/blob/c9ca8483be067a39ab306b09060446e7fda65f35/lightstep/sdk/metric/aggregator/histogram/structure/README.md#mapping-function
5357
# https://github.com/open-telemetry/opentelemetry-go/blob/0e6f9c29c10d6078e8131418e1d1d166c7195d61/sdk/metric/aggregator/exponential/mapping/logarithm/logarithm.go#L32-L45
5458
return self._max_scale

0 commit comments

Comments
 (0)