From 27e495d6f94a4236e90180e75765e46559a9af7d Mon Sep 17 00:00:00 2001 From: Erica Yin Date: Thu, 22 Feb 2024 16:51:41 -0500 Subject: [PATCH] Fix output exponential histogram negative buckets (#4956) * Fix output exponential histogram buckets * Update CHANGELOG --- CHANGELOG.md | 1 + .../aggregate/exponential_histogram.go | 2 + .../aggregate/exponential_histogram_test.go | 63 +++++++++++++------ 3 files changed, 48 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d64910dac2..7d8df3e9b04 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ The next release will require at least [Go 1.21]. ### Fixed - Fix registration of multiple callbacks when using the global meter provider from `go.opentelemetry.io/otel`. (#4945) +- Fix negative buckets in output of exponential histograms. (#4956) ## [1.23.1] 2024-02-07 diff --git a/sdk/metric/internal/aggregate/exponential_histogram.go b/sdk/metric/internal/aggregate/exponential_histogram.go index 660b86071ae..4139a6d1560 100644 --- a/sdk/metric/internal/aggregate/exponential_histogram.go +++ b/sdk/metric/internal/aggregate/exponential_histogram.go @@ -375,6 +375,7 @@ func (e *expoHistogram[N]) delta(dest *metricdata.Aggregation) int { hDPts[i].NegativeBucket.Offset = int32(b.negBuckets.startBin) hDPts[i].NegativeBucket.Counts = reset(hDPts[i].NegativeBucket.Counts, len(b.negBuckets.counts), len(b.negBuckets.counts)) + copy(hDPts[i].NegativeBucket.Counts, b.negBuckets.counts) if !e.noSum { hDPts[i].Sum = b.sum @@ -425,6 +426,7 @@ func (e *expoHistogram[N]) cumulative(dest *metricdata.Aggregation) int { hDPts[i].NegativeBucket.Offset = int32(b.negBuckets.startBin) hDPts[i].NegativeBucket.Counts = reset(hDPts[i].NegativeBucket.Counts, len(b.negBuckets.counts), len(b.negBuckets.counts)) + copy(hDPts[i].NegativeBucket.Counts, b.negBuckets.counts) if !e.noSum { hDPts[i].Sum = b.sum diff --git a/sdk/metric/internal/aggregate/exponential_histogram_test.go b/sdk/metric/internal/aggregate/exponential_histogram_test.go index 3fed4897ee2..4773ee79845 100644 --- a/sdk/metric/internal/aggregate/exponential_histogram_test.go +++ b/sdk/metric/internal/aggregate/exponential_histogram_test.go @@ -771,6 +771,7 @@ func testDeltaExpoHist[N int64 | float64]() func(t *testing.T) { {ctx, 2, alice}, {ctx, 16, alice}, {ctx, 1, alice}, + {ctx, -1, alice}, }, expect: output{ n: 1, @@ -781,15 +782,19 @@ func testDeltaExpoHist[N int64 | float64]() func(t *testing.T) { Attributes: fltrAlice, StartTime: staticTime, Time: staticTime, - Count: 6, - Min: metricdata.NewExtrema[N](1), + Count: 7, + Min: metricdata.NewExtrema[N](-1), Max: metricdata.NewExtrema[N](16), - Sum: 31, + Sum: 30, Scale: -1, PositiveBucket: metricdata.ExponentialBucket{ Offset: -1, Counts: []uint64{1, 4, 1}, }, + NegativeBucket: metricdata.ExponentialBucket{ + Offset: -1, + Counts: []uint64{1}, + }, }, }, }, @@ -821,6 +826,7 @@ func testDeltaExpoHist[N int64 | float64]() func(t *testing.T) { {ctx, 2, carol}, {ctx, 16, carol}, {ctx, 1, dave}, + {ctx, -1, alice}, }, expect: output{ n: 2, @@ -831,15 +837,19 @@ func testDeltaExpoHist[N int64 | float64]() func(t *testing.T) { Attributes: fltrAlice, StartTime: staticTime, Time: staticTime, - Count: 6, - Min: metricdata.NewExtrema[N](1), + Count: 7, + Min: metricdata.NewExtrema[N](-1), Max: metricdata.NewExtrema[N](16), - Sum: 31, + Sum: 30, Scale: -1, PositiveBucket: metricdata.ExponentialBucket{ Offset: -1, Counts: []uint64{1, 4, 1}, }, + NegativeBucket: metricdata.ExponentialBucket{ + Offset: -1, + Counts: []uint64{1}, + }, }, { Attributes: overflowSet, @@ -888,6 +898,7 @@ func testCumulativeExpoHist[N int64 | float64]() func(t *testing.T) { {ctx, 2, alice}, {ctx, 16, alice}, {ctx, 1, alice}, + {ctx, -1, alice}, }, expect: output{ n: 1, @@ -898,15 +909,19 @@ func testCumulativeExpoHist[N int64 | float64]() func(t *testing.T) { Attributes: fltrAlice, StartTime: staticTime, Time: staticTime, - Count: 6, - Min: metricdata.NewExtrema[N](1), + Count: 7, + Min: metricdata.NewExtrema[N](-1), Max: metricdata.NewExtrema[N](16), - Sum: 31, + Sum: 30, Scale: -1, PositiveBucket: metricdata.ExponentialBucket{ Offset: -1, Counts: []uint64{1, 4, 1}, }, + NegativeBucket: metricdata.ExponentialBucket{ + Offset: -1, + Counts: []uint64{1}, + }, }, }, }, @@ -927,15 +942,19 @@ func testCumulativeExpoHist[N int64 | float64]() func(t *testing.T) { Attributes: fltrAlice, StartTime: staticTime, Time: staticTime, - Count: 9, - Min: metricdata.NewExtrema[N](1), + Count: 10, + Min: metricdata.NewExtrema[N](-1), Max: metricdata.NewExtrema[N](16), - Sum: 44, + Sum: 43, Scale: -1, PositiveBucket: metricdata.ExponentialBucket{ Offset: -1, Counts: []uint64{1, 6, 2}, }, + NegativeBucket: metricdata.ExponentialBucket{ + Offset: -1, + Counts: []uint64{1}, + }, }, }, }, @@ -952,15 +971,19 @@ func testCumulativeExpoHist[N int64 | float64]() func(t *testing.T) { Attributes: fltrAlice, StartTime: staticTime, Time: staticTime, - Count: 9, - Min: metricdata.NewExtrema[N](1), + Count: 10, + Min: metricdata.NewExtrema[N](-1), Max: metricdata.NewExtrema[N](16), - Sum: 44, + Sum: 43, Scale: -1, PositiveBucket: metricdata.ExponentialBucket{ Offset: -1, Counts: []uint64{1, 6, 2}, }, + NegativeBucket: metricdata.ExponentialBucket{ + Offset: -1, + Counts: []uint64{1}, + }, }, }, }, @@ -985,15 +1008,19 @@ func testCumulativeExpoHist[N int64 | float64]() func(t *testing.T) { Attributes: fltrAlice, StartTime: staticTime, Time: staticTime, - Count: 9, - Min: metricdata.NewExtrema[N](1), + Count: 10, + Min: metricdata.NewExtrema[N](-1), Max: metricdata.NewExtrema[N](16), - Sum: 44, + Sum: 43, Scale: -1, PositiveBucket: metricdata.ExponentialBucket{ Offset: -1, Counts: []uint64{1, 6, 2}, }, + NegativeBucket: metricdata.ExponentialBucket{ + Offset: -1, + Counts: []uint64{1}, + }, }, { Attributes: overflowSet,