Skip to content

Commit 02093c7

Browse files
wph95peterxcli
authored andcommitted
[core][metric] improve histogram metrics midpoint calculation (ray-project#57948)
There’s a potential risk in the current midpoint calculation. It can be wrong when values are negative. Line 167: lower_bound + buckets[0] / 2.0 Line 171: (buckets[i] + buckets[i - 1]) / 2.0 I improved the formula and added a test to make sure it works. Signed-off-by: justwph <2732352+wph95@users.noreply.github.com> Signed-off-by: peterxcli <peterxcli@gmail.com>
1 parent 9595df0 commit 02093c7

File tree

2 files changed

+11
-1
lines changed

2 files changed

+11
-1
lines changed

python/ray/_private/telemetry/open_telemetry_metric_recorder.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ def register_histogram_metric(
164164
if i == 0:
165165
lower_bound = 0.0 if buckets[0] > 0 else buckets[0] * 2.0
166166
self._histogram_bucket_midpoints[name].append(
167-
lower_bound + buckets[0] / 2.0
167+
(lower_bound + buckets[0]) / 2.0
168168
)
169169
else:
170170
self._histogram_bucket_midpoints[name].append(

python/ray/tests/test_open_telemetry_metric_recorder.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,16 @@ def test_register_histogram_metric(
119119
)
120120
mock_logger_warning.assert_not_called()
121121

122+
mock_meter.create_histogram.return_value = NoOpHistogram(name="neg_histogram")
123+
recorder.register_histogram_metric(
124+
name="neg_histogram",
125+
description="Histogram with negative first boundary",
126+
buckets=[-5.0, 0.0, 10.0],
127+
)
128+
129+
mids = recorder.get_histogram_bucket_midpoints("neg_histogram")
130+
assert mids == pytest.approx([-7.5, -2.5, 5.0, 20.0])
131+
122132

123133
@patch("opentelemetry.metrics.set_meter_provider")
124134
@patch("opentelemetry.metrics.get_meter")

0 commit comments

Comments
 (0)