From 48931d93962c81131515daabf4ef02dbfe0a1e78 Mon Sep 17 00:00:00 2001 From: David Ashpole Date: Fri, 18 Oct 2024 19:38:16 +0000 Subject: [PATCH] use newNoopReservoir instead of nil --- sdk/metric/internal/aggregate/aggregate.go | 3 - .../internal/aggregate/aggregate_test.go | 10 ++- .../aggregate/exponential_histogram_test.go | 38 ++++++---- .../internal/aggregate/histogram_test.go | 38 ++++++---- .../internal/aggregate/lastvalue_test.go | 44 ++++++----- sdk/metric/internal/aggregate/sum_test.go | 76 +++++++++++-------- 6 files changed, 121 insertions(+), 88 deletions(-) diff --git a/sdk/metric/internal/aggregate/aggregate.go b/sdk/metric/internal/aggregate/aggregate.go index f821e0dcad5..bc7fdbdafeb 100644 --- a/sdk/metric/internal/aggregate/aggregate.go +++ b/sdk/metric/internal/aggregate/aggregate.go @@ -52,9 +52,6 @@ type Builder[N int64 | float64] struct { func (b Builder[N]) resFunc() func(attribute.Set) *filteredExemplarReservoir[N] { return func(attrs attribute.Set) *filteredExemplarReservoir[N] { - if b.ExemplarReservoirProvider == nil { - return newFilteredExemplarReservoir[N](b.ExemplarFilter, exemplar.NewFixedSizeReservoir(0)) - } return newFilteredExemplarReservoir[N](b.ExemplarFilter, b.ExemplarReservoirProvider(attrs)) } } diff --git a/sdk/metric/internal/aggregate/aggregate_test.go b/sdk/metric/internal/aggregate/aggregate_test.go index 59d72d37e08..135994065d0 100644 --- a/sdk/metric/internal/aggregate/aggregate_test.go +++ b/sdk/metric/internal/aggregate/aggregate_test.go @@ -73,8 +73,12 @@ func (c *clock) Register() (unregister func()) { return func() { now = orig } } +func newNoopReservoir(attribute.Set) exemplar.Reservoir { + return exemplar.NewFixedSizeReservoir(0) +} + func dropExemplars[N int64 | float64](attr attribute.Set) *filteredExemplarReservoir[N] { - return newFilteredExemplarReservoir[N](exemplar.AlwaysOffFilter, exemplar.NewFixedSizeReservoir(0)) + return newFilteredExemplarReservoir[N](exemplar.AlwaysOffFilter, newNoopReservoir(attr)) } func TestBuilderFilter(t *testing.T) { @@ -100,8 +104,8 @@ func testBuilderFilter[N int64 | float64]() func(t *testing.T) { } } - t.Run("NoFilter", run(Builder[N]{ExemplarFilter: exemplar.AlwaysOffFilter}, attr, nil)) - t.Run("Filter", run(Builder[N]{ExemplarFilter: exemplar.AlwaysOffFilter, Filter: attrFltr}, fltrAlice, []attribute.KeyValue{adminTrue})) + t.Run("NoFilter", run(Builder[N]{ExemplarFilter: exemplar.AlwaysOffFilter, ExemplarReservoirProvider: newNoopReservoir}, attr, nil)) + t.Run("Filter", run(Builder[N]{ExemplarFilter: exemplar.AlwaysOffFilter, ExemplarReservoirProvider: newNoopReservoir, Filter: attrFltr}, fltrAlice, []attribute.KeyValue{adminTrue})) } } diff --git a/sdk/metric/internal/aggregate/exponential_histogram_test.go b/sdk/metric/internal/aggregate/exponential_histogram_test.go index deb66ccacf1..5a8f17bdbc1 100644 --- a/sdk/metric/internal/aggregate/exponential_histogram_test.go +++ b/sdk/metric/internal/aggregate/exponential_histogram_test.go @@ -683,26 +683,30 @@ func BenchmarkExponentialHistogram(b *testing.B) { b.Run("Int64/Cumulative", benchmarkAggregate(func() (Measure[int64], ComputeAggregation) { return Builder[int64]{ - Temporality: metricdata.CumulativeTemporality, - ExemplarFilter: exemplar.AlwaysOffFilter, + Temporality: metricdata.CumulativeTemporality, + ExemplarFilter: exemplar.AlwaysOffFilter, + ExemplarReservoirProvider: newNoopReservoir, }.ExponentialBucketHistogram(maxSize, maxScale, noMinMax, noSum) })) b.Run("Int64/Delta", benchmarkAggregate(func() (Measure[int64], ComputeAggregation) { return Builder[int64]{ - Temporality: metricdata.DeltaTemporality, - ExemplarFilter: exemplar.AlwaysOffFilter, + Temporality: metricdata.DeltaTemporality, + ExemplarFilter: exemplar.AlwaysOffFilter, + ExemplarReservoirProvider: newNoopReservoir, }.ExponentialBucketHistogram(maxSize, maxScale, noMinMax, noSum) })) b.Run("Float64/Cumulative", benchmarkAggregate(func() (Measure[float64], ComputeAggregation) { return Builder[float64]{ - Temporality: metricdata.CumulativeTemporality, - ExemplarFilter: exemplar.AlwaysOffFilter, + Temporality: metricdata.CumulativeTemporality, + ExemplarFilter: exemplar.AlwaysOffFilter, + ExemplarReservoirProvider: newNoopReservoir, }.ExponentialBucketHistogram(maxSize, maxScale, noMinMax, noSum) })) b.Run("Float64/Delta", benchmarkAggregate(func() (Measure[float64], ComputeAggregation) { return Builder[float64]{ - Temporality: metricdata.DeltaTemporality, - ExemplarFilter: exemplar.AlwaysOffFilter, + Temporality: metricdata.DeltaTemporality, + ExemplarFilter: exemplar.AlwaysOffFilter, + ExemplarReservoirProvider: newNoopReservoir, }.ExponentialBucketHistogram(maxSize, maxScale, noMinMax, noSum) })) } @@ -749,10 +753,11 @@ func TestExponentialHistogramAggregation(t *testing.T) { func testDeltaExpoHist[N int64 | float64]() func(t *testing.T) { in, out := Builder[N]{ - Temporality: metricdata.DeltaTemporality, - Filter: attrFltr, - AggregationLimit: 2, - ExemplarFilter: exemplar.AlwaysOffFilter, + Temporality: metricdata.DeltaTemporality, + Filter: attrFltr, + AggregationLimit: 2, + ExemplarFilter: exemplar.AlwaysOffFilter, + ExemplarReservoirProvider: newNoopReservoir, }.ExponentialBucketHistogram(4, 20, false, false) ctx := context.Background() return test[N](in, out, []teststep[N]{ @@ -877,10 +882,11 @@ func testDeltaExpoHist[N int64 | float64]() func(t *testing.T) { func testCumulativeExpoHist[N int64 | float64]() func(t *testing.T) { in, out := Builder[N]{ - Temporality: metricdata.CumulativeTemporality, - Filter: attrFltr, - AggregationLimit: 2, - ExemplarFilter: exemplar.AlwaysOffFilter, + Temporality: metricdata.CumulativeTemporality, + Filter: attrFltr, + AggregationLimit: 2, + ExemplarFilter: exemplar.AlwaysOffFilter, + ExemplarReservoirProvider: newNoopReservoir, }.ExponentialBucketHistogram(4, 20, false, false) ctx := context.Background() return test[N](in, out, []teststep[N]{ diff --git a/sdk/metric/internal/aggregate/histogram_test.go b/sdk/metric/internal/aggregate/histogram_test.go index 769fc07fa11..158c1b005dc 100644 --- a/sdk/metric/internal/aggregate/histogram_test.go +++ b/sdk/metric/internal/aggregate/histogram_test.go @@ -52,10 +52,11 @@ type conf[N int64 | float64] struct { func testDeltaHist[N int64 | float64](c conf[N]) func(t *testing.T) { in, out := Builder[N]{ - Temporality: metricdata.DeltaTemporality, - Filter: attrFltr, - AggregationLimit: 3, - ExemplarFilter: exemplar.AlwaysOffFilter, + Temporality: metricdata.DeltaTemporality, + Filter: attrFltr, + AggregationLimit: 3, + ExemplarFilter: exemplar.AlwaysOffFilter, + ExemplarReservoirProvider: newNoopReservoir, }.ExplicitBucketHistogram(bounds, noMinMax, c.noSum) ctx := context.Background() return test[N](in, out, []teststep[N]{ @@ -140,10 +141,11 @@ func testDeltaHist[N int64 | float64](c conf[N]) func(t *testing.T) { func testCumulativeHist[N int64 | float64](c conf[N]) func(t *testing.T) { in, out := Builder[N]{ - Temporality: metricdata.CumulativeTemporality, - Filter: attrFltr, - AggregationLimit: 3, - ExemplarFilter: exemplar.AlwaysOffFilter, + Temporality: metricdata.CumulativeTemporality, + Filter: attrFltr, + AggregationLimit: 3, + ExemplarFilter: exemplar.AlwaysOffFilter, + ExemplarReservoirProvider: newNoopReservoir, }.ExplicitBucketHistogram(bounds, noMinMax, c.noSum) ctx := context.Background() return test[N](in, out, []teststep[N]{ @@ -378,26 +380,30 @@ func TestDeltaHistogramReset(t *testing.T) { func BenchmarkHistogram(b *testing.B) { b.Run("Int64/Cumulative", benchmarkAggregate(func() (Measure[int64], ComputeAggregation) { return Builder[int64]{ - Temporality: metricdata.CumulativeTemporality, - ExemplarFilter: exemplar.AlwaysOffFilter, + Temporality: metricdata.CumulativeTemporality, + ExemplarFilter: exemplar.AlwaysOffFilter, + ExemplarReservoirProvider: newNoopReservoir, }.ExplicitBucketHistogram(bounds, noMinMax, false) })) b.Run("Int64/Delta", benchmarkAggregate(func() (Measure[int64], ComputeAggregation) { return Builder[int64]{ - Temporality: metricdata.DeltaTemporality, - ExemplarFilter: exemplar.AlwaysOffFilter, + Temporality: metricdata.DeltaTemporality, + ExemplarFilter: exemplar.AlwaysOffFilter, + ExemplarReservoirProvider: newNoopReservoir, }.ExplicitBucketHistogram(bounds, noMinMax, false) })) b.Run("Float64/Cumulative", benchmarkAggregate(func() (Measure[float64], ComputeAggregation) { return Builder[float64]{ - Temporality: metricdata.CumulativeTemporality, - ExemplarFilter: exemplar.AlwaysOffFilter, + Temporality: metricdata.CumulativeTemporality, + ExemplarFilter: exemplar.AlwaysOffFilter, + ExemplarReservoirProvider: newNoopReservoir, }.ExplicitBucketHistogram(bounds, noMinMax, false) })) b.Run("Float64/Delta", benchmarkAggregate(func() (Measure[float64], ComputeAggregation) { return Builder[float64]{ - Temporality: metricdata.DeltaTemporality, - ExemplarFilter: exemplar.AlwaysOffFilter, + Temporality: metricdata.DeltaTemporality, + ExemplarFilter: exemplar.AlwaysOffFilter, + ExemplarReservoirProvider: newNoopReservoir, }.ExplicitBucketHistogram(bounds, noMinMax, false) })) } diff --git a/sdk/metric/internal/aggregate/lastvalue_test.go b/sdk/metric/internal/aggregate/lastvalue_test.go index 349ed337c50..7bba4666f0e 100644 --- a/sdk/metric/internal/aggregate/lastvalue_test.go +++ b/sdk/metric/internal/aggregate/lastvalue_test.go @@ -37,10 +37,11 @@ func TestLastValue(t *testing.T) { func testDeltaLastValue[N int64 | float64]() func(*testing.T) { in, out := Builder[N]{ - Temporality: metricdata.DeltaTemporality, - Filter: attrFltr, - AggregationLimit: 3, - ExemplarFilter: exemplar.AlwaysOffFilter, + Temporality: metricdata.DeltaTemporality, + Filter: attrFltr, + AggregationLimit: 3, + ExemplarFilter: exemplar.AlwaysOffFilter, + ExemplarReservoirProvider: newNoopReservoir, }.LastValue() ctx := context.Background() return test[N](in, out, []teststep[N]{ @@ -142,10 +143,11 @@ func testDeltaLastValue[N int64 | float64]() func(*testing.T) { func testCumulativeLastValue[N int64 | float64]() func(*testing.T) { in, out := Builder[N]{ - Temporality: metricdata.CumulativeTemporality, - Filter: attrFltr, - AggregationLimit: 3, - ExemplarFilter: exemplar.AlwaysOffFilter, + Temporality: metricdata.CumulativeTemporality, + Filter: attrFltr, + AggregationLimit: 3, + ExemplarFilter: exemplar.AlwaysOffFilter, + ExemplarReservoirProvider: newNoopReservoir, }.LastValue() ctx := context.Background() return test[N](in, out, []teststep[N]{ @@ -265,10 +267,11 @@ func testCumulativeLastValue[N int64 | float64]() func(*testing.T) { func testDeltaPrecomputedLastValue[N int64 | float64]() func(*testing.T) { in, out := Builder[N]{ - Temporality: metricdata.DeltaTemporality, - Filter: attrFltr, - AggregationLimit: 3, - ExemplarFilter: exemplar.AlwaysOffFilter, + Temporality: metricdata.DeltaTemporality, + Filter: attrFltr, + AggregationLimit: 3, + ExemplarFilter: exemplar.AlwaysOffFilter, + ExemplarReservoirProvider: newNoopReservoir, }.PrecomputedLastValue() ctx := context.Background() return test[N](in, out, []teststep[N]{ @@ -370,10 +373,11 @@ func testDeltaPrecomputedLastValue[N int64 | float64]() func(*testing.T) { func testCumulativePrecomputedLastValue[N int64 | float64]() func(*testing.T) { in, out := Builder[N]{ - Temporality: metricdata.CumulativeTemporality, - Filter: attrFltr, - AggregationLimit: 3, - ExemplarFilter: exemplar.AlwaysOffFilter, + Temporality: metricdata.CumulativeTemporality, + Filter: attrFltr, + AggregationLimit: 3, + ExemplarFilter: exemplar.AlwaysOffFilter, + ExemplarReservoirProvider: newNoopReservoir, }.PrecomputedLastValue() ctx := context.Background() return test[N](in, out, []teststep[N]{ @@ -475,7 +479,11 @@ func testCumulativePrecomputedLastValue[N int64 | float64]() func(*testing.T) { func BenchmarkLastValue(b *testing.B) { b.Run("Int64", benchmarkAggregate(Builder[int64]{ - ExemplarFilter: exemplar.AlwaysOffFilter}.PrecomputedLastValue)) + ExemplarFilter: exemplar.AlwaysOffFilter, + ExemplarReservoirProvider: newNoopReservoir, + }.PrecomputedLastValue)) b.Run("Float64", benchmarkAggregate(Builder[float64]{ - ExemplarFilter: exemplar.AlwaysOffFilter}.PrecomputedLastValue)) + ExemplarFilter: exemplar.AlwaysOffFilter, + ExemplarReservoirProvider: newNoopReservoir, + }.PrecomputedLastValue)) } diff --git a/sdk/metric/internal/aggregate/sum_test.go b/sdk/metric/internal/aggregate/sum_test.go index 524af5d0407..fcfd5c539ce 100644 --- a/sdk/metric/internal/aggregate/sum_test.go +++ b/sdk/metric/internal/aggregate/sum_test.go @@ -42,10 +42,11 @@ func TestSum(t *testing.T) { func testDeltaSum[N int64 | float64]() func(t *testing.T) { mono := false in, out := Builder[N]{ - Temporality: metricdata.DeltaTemporality, - Filter: attrFltr, - AggregationLimit: 3, - ExemplarFilter: exemplar.AlwaysOffFilter, + Temporality: metricdata.DeltaTemporality, + Filter: attrFltr, + AggregationLimit: 3, + ExemplarFilter: exemplar.AlwaysOffFilter, + ExemplarReservoirProvider: newNoopReservoir, }.Sum(mono) ctx := context.Background() return test[N](in, out, []teststep[N]{ @@ -171,10 +172,11 @@ func testDeltaSum[N int64 | float64]() func(t *testing.T) { func testCumulativeSum[N int64 | float64]() func(t *testing.T) { mono := false in, out := Builder[N]{ - Temporality: metricdata.CumulativeTemporality, - Filter: attrFltr, - AggregationLimit: 3, - ExemplarFilter: exemplar.AlwaysOffFilter, + Temporality: metricdata.CumulativeTemporality, + Filter: attrFltr, + AggregationLimit: 3, + ExemplarFilter: exemplar.AlwaysOffFilter, + ExemplarReservoirProvider: newNoopReservoir, }.Sum(mono) ctx := context.Background() return test[N](in, out, []teststep[N]{ @@ -286,10 +288,11 @@ func testCumulativeSum[N int64 | float64]() func(t *testing.T) { func testDeltaPrecomputedSum[N int64 | float64]() func(t *testing.T) { mono := false in, out := Builder[N]{ - Temporality: metricdata.DeltaTemporality, - Filter: attrFltr, - AggregationLimit: 3, - ExemplarFilter: exemplar.AlwaysOffFilter, + Temporality: metricdata.DeltaTemporality, + Filter: attrFltr, + AggregationLimit: 3, + ExemplarFilter: exemplar.AlwaysOffFilter, + ExemplarReservoirProvider: newNoopReservoir, }.PrecomputedSum(mono) ctx := context.Background() return test[N](in, out, []teststep[N]{ @@ -416,10 +419,11 @@ func testDeltaPrecomputedSum[N int64 | float64]() func(t *testing.T) { func testCumulativePrecomputedSum[N int64 | float64]() func(t *testing.T) { mono := false in, out := Builder[N]{ - Temporality: metricdata.CumulativeTemporality, - Filter: attrFltr, - AggregationLimit: 3, - ExemplarFilter: exemplar.AlwaysOffFilter, + Temporality: metricdata.CumulativeTemporality, + Filter: attrFltr, + AggregationLimit: 3, + ExemplarFilter: exemplar.AlwaysOffFilter, + ExemplarReservoirProvider: newNoopReservoir, }.PrecomputedSum(mono) ctx := context.Background() return test[N](in, out, []teststep[N]{ @@ -549,51 +553,59 @@ func BenchmarkSum(b *testing.B) { // performance, therefore, only monotonic=false is benchmarked here. b.Run("Int64/Cumulative", benchmarkAggregate(func() (Measure[int64], ComputeAggregation) { return Builder[int64]{ - Temporality: metricdata.CumulativeTemporality, - ExemplarFilter: exemplar.AlwaysOffFilter, + Temporality: metricdata.CumulativeTemporality, + ExemplarFilter: exemplar.AlwaysOffFilter, + ExemplarReservoirProvider: newNoopReservoir, }.Sum(false) })) b.Run("Int64/Delta", benchmarkAggregate(func() (Measure[int64], ComputeAggregation) { return Builder[int64]{ - Temporality: metricdata.DeltaTemporality, - ExemplarFilter: exemplar.AlwaysOffFilter, + Temporality: metricdata.DeltaTemporality, + ExemplarFilter: exemplar.AlwaysOffFilter, + ExemplarReservoirProvider: newNoopReservoir, }.Sum(false) })) b.Run("Float64/Cumulative", benchmarkAggregate(func() (Measure[float64], ComputeAggregation) { return Builder[float64]{ - Temporality: metricdata.CumulativeTemporality, - ExemplarFilter: exemplar.AlwaysOffFilter, + Temporality: metricdata.CumulativeTemporality, + ExemplarFilter: exemplar.AlwaysOffFilter, + ExemplarReservoirProvider: newNoopReservoir, }.Sum(false) })) b.Run("Float64/Delta", benchmarkAggregate(func() (Measure[float64], ComputeAggregation) { return Builder[float64]{ - Temporality: metricdata.DeltaTemporality, - ExemplarFilter: exemplar.AlwaysOffFilter, + Temporality: metricdata.DeltaTemporality, + ExemplarFilter: exemplar.AlwaysOffFilter, + ExemplarReservoirProvider: newNoopReservoir, }.Sum(false) })) b.Run("Precomputed/Int64/Cumulative", benchmarkAggregate(func() (Measure[int64], ComputeAggregation) { return Builder[int64]{ - Temporality: metricdata.CumulativeTemporality, - ExemplarFilter: exemplar.AlwaysOffFilter, + Temporality: metricdata.CumulativeTemporality, + ExemplarFilter: exemplar.AlwaysOffFilter, + ExemplarReservoirProvider: newNoopReservoir, }.PrecomputedSum(false) })) b.Run("Precomputed/Int64/Delta", benchmarkAggregate(func() (Measure[int64], ComputeAggregation) { return Builder[int64]{ - Temporality: metricdata.DeltaTemporality, - ExemplarFilter: exemplar.AlwaysOffFilter, + Temporality: metricdata.DeltaTemporality, + ExemplarFilter: exemplar.AlwaysOffFilter, + ExemplarReservoirProvider: newNoopReservoir, }.PrecomputedSum(false) })) b.Run("Precomputed/Float64/Cumulative", benchmarkAggregate(func() (Measure[float64], ComputeAggregation) { return Builder[float64]{ - Temporality: metricdata.CumulativeTemporality, - ExemplarFilter: exemplar.AlwaysOffFilter, + Temporality: metricdata.CumulativeTemporality, + ExemplarFilter: exemplar.AlwaysOffFilter, + ExemplarReservoirProvider: newNoopReservoir, }.PrecomputedSum(false) })) b.Run("Precomputed/Float64/Delta", benchmarkAggregate(func() (Measure[float64], ComputeAggregation) { return Builder[float64]{ - Temporality: metricdata.DeltaTemporality, - ExemplarFilter: exemplar.AlwaysOffFilter, + Temporality: metricdata.DeltaTemporality, + ExemplarFilter: exemplar.AlwaysOffFilter, + ExemplarReservoirProvider: newNoopReservoir, }.PrecomputedSum(false) })) }