From b302227390754a8d3c3619168d5a52c7c023a84f Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Mon, 26 Feb 2024 23:00:29 -0800 Subject: [PATCH] Utilize the new slices package in sdk/metric (#4982) --- sdk/metric/aggregation.go | 5 ++-- sdk/metric/exemplar.go | 4 ++-- sdk/metric/internal/aggregate/histogram.go | 28 ++++++++++------------ sdk/metric/internal/exemplar/hist.go | 3 ++- sdk/metric/internal/exemplar/rand_test.go | 4 ++-- 5 files changed, 20 insertions(+), 24 deletions(-) diff --git a/sdk/metric/aggregation.go b/sdk/metric/aggregation.go index faddbb0b6..35d79bf25 100644 --- a/sdk/metric/aggregation.go +++ b/sdk/metric/aggregation.go @@ -17,6 +17,7 @@ package metric // import "go.opentelemetry.io/otel/sdk/metric" import ( "errors" "fmt" + "slices" ) // errAgg is wrapped by misconfigured aggregations. @@ -141,10 +142,8 @@ func (h AggregationExplicitBucketHistogram) err() error { // copy returns a deep copy of h. func (h AggregationExplicitBucketHistogram) copy() Aggregation { - b := make([]float64, len(h.Boundaries)) - copy(b, h.Boundaries) return AggregationExplicitBucketHistogram{ - Boundaries: b, + Boundaries: slices.Clone(h.Boundaries), NoMinMax: h.NoMinMax, } } diff --git a/sdk/metric/exemplar.go b/sdk/metric/exemplar.go index 3f1ce9f1d..c16a60d63 100644 --- a/sdk/metric/exemplar.go +++ b/sdk/metric/exemplar.go @@ -17,6 +17,7 @@ package metric // import "go.opentelemetry.io/otel/sdk/metric" import ( "os" "runtime" + "slices" "go.opentelemetry.io/otel/sdk/metric/internal/exemplar" "go.opentelemetry.io/otel/sdk/metric/internal/x" @@ -40,8 +41,7 @@ func reservoirFunc[N int64 | float64](agg Aggregation) func() exemplar.Reservoir // use AlignedHistogramBucketExemplarReservoir. a, ok := agg.(AggregationExplicitBucketHistogram) if ok && len(a.Boundaries) > 0 { - cp := make([]float64, len(a.Boundaries)) - copy(cp, a.Boundaries) + cp := slices.Clone(a.Boundaries) return func() exemplar.Reservoir[N] { bounds := cp return exemplar.Histogram[N](bounds) diff --git a/sdk/metric/internal/aggregate/histogram.go b/sdk/metric/internal/aggregate/histogram.go index a9a4706bf..d695ad64c 100644 --- a/sdk/metric/internal/aggregate/histogram.go +++ b/sdk/metric/internal/aggregate/histogram.go @@ -16,6 +16,7 @@ package aggregate // import "go.opentelemetry.io/otel/sdk/metric/internal/aggreg import ( "context" + "slices" "sort" "sync" "time" @@ -68,9 +69,8 @@ func newHistValues[N int64 | float64](bounds []float64, noSum bool, limit int, r // passed boundaries is ultimately this type's responsibility. Make a copy // here so we can always guarantee this. Or, in the case of failure, have // complete control over the fix. - b := make([]float64, len(bounds)) - copy(b, bounds) - sort.Float64s(b) + b := slices.Clone(bounds) + slices.Sort(b) return &histValues[N]{ noSum: noSum, bounds: b, @@ -150,8 +150,7 @@ func (s *histogram[N]) delta(dest *metricdata.Aggregation) int { defer s.valuesMu.Unlock() // Do not allow modification of our copy of bounds. - bounds := make([]float64, len(s.bounds)) - copy(bounds, s.bounds) + bounds := slices.Clone(s.bounds) n := len(s.values) hDPts := reset(h.DataPoints, n, n) @@ -201,28 +200,25 @@ func (s *histogram[N]) cumulative(dest *metricdata.Aggregation) int { defer s.valuesMu.Unlock() // Do not allow modification of our copy of bounds. - bounds := make([]float64, len(s.bounds)) - copy(bounds, s.bounds) + bounds := slices.Clone(s.bounds) n := len(s.values) hDPts := reset(h.DataPoints, n, n) var i int for a, b := range s.values { - // The HistogramDataPoint field values returned need to be copies of - // the buckets value as we will keep updating them. - // - // TODO (#3047): Making copies for bounds and counts incurs a large - // memory allocation footprint. Alternatives should be explored. - counts := make([]uint64, len(b.counts)) - copy(counts, b.counts) - hDPts[i].Attributes = a hDPts[i].StartTime = s.start hDPts[i].Time = t hDPts[i].Count = b.count hDPts[i].Bounds = bounds - hDPts[i].BucketCounts = counts + + // The HistogramDataPoint field values returned need to be copies of + // the buckets value as we will keep updating them. + // + // TODO (#3047): Making copies for bounds and counts incurs a large + // memory allocation footprint. Alternatives should be explored. + hDPts[i].BucketCounts = slices.Clone(b.counts) if !s.noSum { hDPts[i].Sum = b.total diff --git a/sdk/metric/internal/exemplar/hist.go b/sdk/metric/internal/exemplar/hist.go index 6f4fe5524..2c1005398 100644 --- a/sdk/metric/internal/exemplar/hist.go +++ b/sdk/metric/internal/exemplar/hist.go @@ -16,6 +16,7 @@ package exemplar // import "go.opentelemetry.io/otel/sdk/metric/internal/exempla import ( "context" + "slices" "sort" "time" @@ -28,7 +29,7 @@ import ( // // The passed bounds will be sorted by this function. func Histogram[N int64 | float64](bounds []float64) Reservoir[N] { - sort.Float64s(bounds) + slices.Sort(bounds) return &histRes[N]{ bounds: bounds, storage: newStorage[N](len(bounds) + 1), diff --git a/sdk/metric/internal/exemplar/rand_test.go b/sdk/metric/internal/exemplar/rand_test.go index 9de7a7058..0ed7c7745 100644 --- a/sdk/metric/internal/exemplar/rand_test.go +++ b/sdk/metric/internal/exemplar/rand_test.go @@ -17,7 +17,7 @@ package exemplar import ( "context" "math" - "sort" + "slices" "testing" "github.com/stretchr/testify/assert" @@ -43,7 +43,7 @@ func TestFixedSizeSamplingCorrectness(t *testing.T) { data[i] = (-1.0 / intensity) * math.Log(random()) } // Sort to test position bias. - sort.Float64s(data) + slices.Sort(data) r := FixedSize[float64](sampleSize) for _, value := range data {