From 6de7c08d33dbd9529679fa32a067d70cca0da52a Mon Sep 17 00:00:00 2001 From: David Ashpole Date: Tue, 25 Jun 2024 19:07:31 +0000 Subject: [PATCH] move time.Now call into exemplar reservoir --- CHANGELOG.md | 1 + .../aggregate/exponential_histogram.go | 4 +- .../aggregate/exponential_histogram_test.go | 20 ++--- sdk/metric/internal/aggregate/histogram.go | 4 +- .../internal/aggregate/histogram_test.go | 32 ++++---- sdk/metric/internal/aggregate/lastvalue.go | 4 +- .../internal/aggregate/lastvalue_test.go | 80 +++++++++---------- sdk/metric/internal/aggregate/sum.go | 4 +- sdk/metric/internal/aggregate/sum_test.go | 76 +++++++++--------- sdk/metric/internal/exemplar/drop.go | 3 +- sdk/metric/internal/exemplar/filter.go | 5 +- sdk/metric/internal/exemplar/filter_test.go | 7 +- sdk/metric/internal/exemplar/hist.go | 5 +- sdk/metric/internal/exemplar/rand.go | 3 +- sdk/metric/internal/exemplar/rand_test.go | 2 +- sdk/metric/internal/exemplar/reservoir.go | 6 +- .../internal/exemplar/reservoir_test.go | 15 ++-- 17 files changed, 134 insertions(+), 137 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 41609fae5f5a..d6bd67a539b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Upgrade `go.opentelemetry.io/otel/semconv/v1.25.0` to `go.opentelemetry.io/otel/semconv/v1.26.0` in `go.opentelemetry.io/otel/sdk/resource`. (#5490) - Upgrade `go.opentelemetry.io/otel/semconv/v1.25.0` to `go.opentelemetry.io/otel/semconv/v1.26.0` in `go.opentelemetry.io/otel/sdk/trace`. (#5490) - Use non-generic functions in the `Start` method of `"go.opentelemetry.io/otel/sdk/trace".Trace` to reduce memory allocation. (#5497) +- Improve performance of metric instruments in `go.opentelemetry.io/otel/sdk/metric` by removing unnecessary calls to time.Now. (#5545) ### Fixed diff --git a/sdk/metric/internal/aggregate/exponential_histogram.go b/sdk/metric/internal/aggregate/exponential_histogram.go index 902074b5bfdc..766585608a6e 100644 --- a/sdk/metric/internal/aggregate/exponential_histogram.go +++ b/sdk/metric/internal/aggregate/exponential_histogram.go @@ -319,8 +319,6 @@ func (e *expoHistogram[N]) measure(ctx context.Context, value N, fltrAttr attrib return } - t := now() - e.valuesMu.Lock() defer e.valuesMu.Unlock() @@ -333,7 +331,7 @@ func (e *expoHistogram[N]) measure(ctx context.Context, value N, fltrAttr attrib e.values[attr.Equivalent()] = v } v.record(value) - v.res.Offer(ctx, t, exemplar.NewValue(value), droppedAttr) + v.res.Offer(ctx, exemplar.NewValue(value), droppedAttr) } func (e *expoHistogram[N]) delta(dest *metricdata.Aggregation) int { diff --git a/sdk/metric/internal/aggregate/exponential_histogram_test.go b/sdk/metric/internal/aggregate/exponential_histogram_test.go index 2ffd3ebf0bfc..8af8589d3a63 100644 --- a/sdk/metric/internal/aggregate/exponential_histogram_test.go +++ b/sdk/metric/internal/aggregate/exponential_histogram_test.go @@ -778,7 +778,7 @@ func testDeltaExpoHist[N int64 | float64]() func(t *testing.T) { { Attributes: fltrAlice, StartTime: y2kPlus(1), - Time: y2kPlus(9), + Time: y2kPlus(2), Count: 7, Min: metricdata.NewExtrema[N](-1), Max: metricdata.NewExtrema[N](16), @@ -832,8 +832,8 @@ func testDeltaExpoHist[N int64 | float64]() func(t *testing.T) { DataPoints: []metricdata.ExponentialHistogramDataPoint[N]{ { Attributes: fltrAlice, - StartTime: y2kPlus(10), - Time: y2kPlus(24), + StartTime: y2kPlus(3), + Time: y2kPlus(4), Count: 7, Min: metricdata.NewExtrema[N](-1), Max: metricdata.NewExtrema[N](16), @@ -850,8 +850,8 @@ func testDeltaExpoHist[N int64 | float64]() func(t *testing.T) { }, { Attributes: overflowSet, - StartTime: y2kPlus(10), - Time: y2kPlus(24), + StartTime: y2kPlus(3), + Time: y2kPlus(4), Count: 6, Min: metricdata.NewExtrema[N](1), Max: metricdata.NewExtrema[N](16), @@ -905,7 +905,7 @@ func testCumulativeExpoHist[N int64 | float64]() func(t *testing.T) { { Attributes: fltrAlice, StartTime: y2kPlus(0), - Time: y2kPlus(9), + Time: y2kPlus(2), Count: 7, Min: metricdata.NewExtrema[N](-1), Max: metricdata.NewExtrema[N](16), @@ -938,7 +938,7 @@ func testCumulativeExpoHist[N int64 | float64]() func(t *testing.T) { { Attributes: fltrAlice, StartTime: y2kPlus(0), - Time: y2kPlus(13), + Time: y2kPlus(3), Count: 10, Min: metricdata.NewExtrema[N](-1), Max: metricdata.NewExtrema[N](16), @@ -967,7 +967,7 @@ func testCumulativeExpoHist[N int64 | float64]() func(t *testing.T) { { Attributes: fltrAlice, StartTime: y2kPlus(0), - Time: y2kPlus(14), + Time: y2kPlus(4), Count: 10, Min: metricdata.NewExtrema[N](-1), Max: metricdata.NewExtrema[N](16), @@ -1004,7 +1004,7 @@ func testCumulativeExpoHist[N int64 | float64]() func(t *testing.T) { { Attributes: fltrAlice, StartTime: y2kPlus(0), - Time: y2kPlus(21), + Time: y2kPlus(5), Count: 10, Min: metricdata.NewExtrema[N](-1), Max: metricdata.NewExtrema[N](16), @@ -1022,7 +1022,7 @@ func testCumulativeExpoHist[N int64 | float64]() func(t *testing.T) { { Attributes: overflowSet, StartTime: y2kPlus(0), - Time: y2kPlus(21), + Time: y2kPlus(5), Count: 6, Min: metricdata.NewExtrema[N](1), Max: metricdata.NewExtrema[N](16), diff --git a/sdk/metric/internal/aggregate/histogram.go b/sdk/metric/internal/aggregate/histogram.go index 213baf50f53a..43b542d5c5a0 100644 --- a/sdk/metric/internal/aggregate/histogram.go +++ b/sdk/metric/internal/aggregate/histogram.go @@ -80,8 +80,6 @@ func (s *histValues[N]) measure(ctx context.Context, value N, fltrAttr attribute // (s.bounds[len(s.bounds)-1], +∞). idx := sort.SearchFloat64s(s.bounds, float64(value)) - t := now() - s.valuesMu.Lock() defer s.valuesMu.Unlock() @@ -106,7 +104,7 @@ func (s *histValues[N]) measure(ctx context.Context, value N, fltrAttr attribute if !s.noSum { b.sum(value) } - b.res.Offer(ctx, t, exemplar.NewValue(value), droppedAttr) + b.res.Offer(ctx, exemplar.NewValue(value), droppedAttr) } // newHistogram returns an Aggregator that summarizes a set of measurements as diff --git a/sdk/metric/internal/aggregate/histogram_test.go b/sdk/metric/internal/aggregate/histogram_test.go index 38ba1229eb29..cc9772da0d92 100644 --- a/sdk/metric/internal/aggregate/histogram_test.go +++ b/sdk/metric/internal/aggregate/histogram_test.go @@ -80,8 +80,8 @@ func testDeltaHist[N int64 | float64](c conf[N]) func(t *testing.T) { agg: metricdata.Histogram[N]{ Temporality: metricdata.DeltaTemporality, DataPoints: []metricdata.HistogramDataPoint[N]{ - c.hPt(fltrAlice, 2, 3, y2kPlus(1), y2kPlus(7)), - c.hPt(fltrBob, 10, 2, y2kPlus(1), y2kPlus(7)), + c.hPt(fltrAlice, 2, 3, y2kPlus(1), y2kPlus(2)), + c.hPt(fltrBob, 10, 2, y2kPlus(1), y2kPlus(2)), }, }, }, @@ -96,8 +96,8 @@ func testDeltaHist[N int64 | float64](c conf[N]) func(t *testing.T) { agg: metricdata.Histogram[N]{ Temporality: metricdata.DeltaTemporality, DataPoints: []metricdata.HistogramDataPoint[N]{ - c.hPt(fltrAlice, 10, 1, y2kPlus(7), y2kPlus(10)), - c.hPt(fltrBob, 3, 1, y2kPlus(7), y2kPlus(10)), + c.hPt(fltrAlice, 10, 1, y2kPlus(2), y2kPlus(3)), + c.hPt(fltrBob, 3, 1, y2kPlus(2), y2kPlus(3)), }, }, }, @@ -126,9 +126,9 @@ func testDeltaHist[N int64 | float64](c conf[N]) func(t *testing.T) { agg: metricdata.Histogram[N]{ Temporality: metricdata.DeltaTemporality, DataPoints: []metricdata.HistogramDataPoint[N]{ - c.hPt(fltrAlice, 1, 1, y2kPlus(11), y2kPlus(16)), - c.hPt(fltrBob, 1, 1, y2kPlus(11), y2kPlus(16)), - c.hPt(overflowSet, 1, 2, y2kPlus(11), y2kPlus(16)), + c.hPt(fltrAlice, 1, 1, y2kPlus(4), y2kPlus(5)), + c.hPt(fltrBob, 1, 1, y2kPlus(4), y2kPlus(5)), + c.hPt(overflowSet, 1, 2, y2kPlus(4), y2kPlus(5)), }, }, }, @@ -167,8 +167,8 @@ func testCumulativeHist[N int64 | float64](c conf[N]) func(t *testing.T) { agg: metricdata.Histogram[N]{ Temporality: metricdata.CumulativeTemporality, DataPoints: []metricdata.HistogramDataPoint[N]{ - c.hPt(fltrAlice, 2, 3, y2kPlus(0), y2kPlus(7)), - c.hPt(fltrBob, 10, 2, y2kPlus(0), y2kPlus(7)), + c.hPt(fltrAlice, 2, 3, y2kPlus(0), y2kPlus(2)), + c.hPt(fltrBob, 10, 2, y2kPlus(0), y2kPlus(2)), }, }, }, @@ -183,8 +183,8 @@ func testCumulativeHist[N int64 | float64](c conf[N]) func(t *testing.T) { agg: metricdata.Histogram[N]{ Temporality: metricdata.CumulativeTemporality, DataPoints: []metricdata.HistogramDataPoint[N]{ - c.hPt(fltrAlice, 2, 4, y2kPlus(0), y2kPlus(10)), - c.hPt(fltrBob, 10, 3, y2kPlus(0), y2kPlus(10)), + c.hPt(fltrAlice, 2, 4, y2kPlus(0), y2kPlus(3)), + c.hPt(fltrBob, 10, 3, y2kPlus(0), y2kPlus(3)), }, }, }, @@ -196,8 +196,8 @@ func testCumulativeHist[N int64 | float64](c conf[N]) func(t *testing.T) { agg: metricdata.Histogram[N]{ Temporality: metricdata.CumulativeTemporality, DataPoints: []metricdata.HistogramDataPoint[N]{ - c.hPt(fltrAlice, 2, 4, y2kPlus(0), y2kPlus(11)), - c.hPt(fltrBob, 10, 3, y2kPlus(0), y2kPlus(11)), + c.hPt(fltrAlice, 2, 4, y2kPlus(0), y2kPlus(4)), + c.hPt(fltrBob, 10, 3, y2kPlus(0), y2kPlus(4)), }, }, }, @@ -213,9 +213,9 @@ func testCumulativeHist[N int64 | float64](c conf[N]) func(t *testing.T) { agg: metricdata.Histogram[N]{ Temporality: metricdata.CumulativeTemporality, DataPoints: []metricdata.HistogramDataPoint[N]{ - c.hPt(fltrAlice, 2, 4, y2kPlus(0), y2kPlus(14)), - c.hPt(fltrBob, 10, 3, y2kPlus(0), y2kPlus(14)), - c.hPt(overflowSet, 1, 2, y2kPlus(0), y2kPlus(14)), + c.hPt(fltrAlice, 2, 4, y2kPlus(0), y2kPlus(5)), + c.hPt(fltrBob, 10, 3, y2kPlus(0), y2kPlus(5)), + c.hPt(overflowSet, 1, 2, y2kPlus(0), y2kPlus(5)), }, }, }, diff --git a/sdk/metric/internal/aggregate/lastvalue.go b/sdk/metric/internal/aggregate/lastvalue.go index 3b65e761e862..640d61e52f3b 100644 --- a/sdk/metric/internal/aggregate/lastvalue.go +++ b/sdk/metric/internal/aggregate/lastvalue.go @@ -40,8 +40,6 @@ type lastValue[N int64 | float64] struct { } func (s *lastValue[N]) measure(ctx context.Context, value N, fltrAttr attribute.Set, droppedAttr []attribute.KeyValue) { - t := now() - s.Lock() defer s.Unlock() @@ -53,7 +51,7 @@ func (s *lastValue[N]) measure(ctx context.Context, value N, fltrAttr attribute. d.attrs = attr d.value = value - d.res.Offer(ctx, t, exemplar.NewValue(value), droppedAttr) + d.res.Offer(ctx, exemplar.NewValue(value), droppedAttr) s.values[attr.Equivalent()] = d } diff --git a/sdk/metric/internal/aggregate/lastvalue_test.go b/sdk/metric/internal/aggregate/lastvalue_test.go index 1e4ca21c96ad..77e0d283ba0f 100644 --- a/sdk/metric/internal/aggregate/lastvalue_test.go +++ b/sdk/metric/internal/aggregate/lastvalue_test.go @@ -61,13 +61,13 @@ func testDeltaLastValue[N int64 | float64]() func(*testing.T) { { Attributes: fltrAlice, StartTime: y2kPlus(1), - Time: y2kPlus(7), + Time: y2kPlus(2), Value: 2, }, { Attributes: fltrBob, StartTime: y2kPlus(1), - Time: y2kPlus(7), + Time: y2kPlus(2), Value: -10, }, }, @@ -88,14 +88,14 @@ func testDeltaLastValue[N int64 | float64]() func(*testing.T) { DataPoints: []metricdata.DataPoint[N]{ { Attributes: fltrAlice, - StartTime: y2kPlus(8), - Time: y2kPlus(11), + StartTime: y2kPlus(3), + Time: y2kPlus(4), Value: 10, }, { Attributes: fltrBob, - StartTime: y2kPlus(8), - Time: y2kPlus(11), + StartTime: y2kPlus(3), + Time: y2kPlus(4), Value: 3, }, }, @@ -115,20 +115,20 @@ func testDeltaLastValue[N int64 | float64]() func(*testing.T) { DataPoints: []metricdata.DataPoint[N]{ { Attributes: fltrAlice, - StartTime: y2kPlus(11), - Time: y2kPlus(16), + StartTime: y2kPlus(4), + Time: y2kPlus(5), Value: 1, }, { Attributes: fltrBob, - StartTime: y2kPlus(11), - Time: y2kPlus(16), + StartTime: y2kPlus(4), + Time: y2kPlus(5), Value: 1, }, { Attributes: overflowSet, - StartTime: y2kPlus(11), - Time: y2kPlus(16), + StartTime: y2kPlus(4), + Time: y2kPlus(5), Value: 1, }, }, @@ -165,13 +165,13 @@ func testCumulativeLastValue[N int64 | float64]() func(*testing.T) { { Attributes: fltrAlice, StartTime: y2kPlus(0), - Time: y2kPlus(7), + Time: y2kPlus(2), Value: 2, }, { Attributes: fltrBob, StartTime: y2kPlus(0), - Time: y2kPlus(7), + Time: y2kPlus(2), Value: -10, }, }, @@ -187,13 +187,13 @@ func testCumulativeLastValue[N int64 | float64]() func(*testing.T) { { Attributes: fltrAlice, StartTime: y2kPlus(0), - Time: y2kPlus(8), + Time: y2kPlus(3), Value: 2, }, { Attributes: fltrBob, StartTime: y2kPlus(0), - Time: y2kPlus(8), + Time: y2kPlus(3), Value: -10, }, }, @@ -211,13 +211,13 @@ func testCumulativeLastValue[N int64 | float64]() func(*testing.T) { { Attributes: fltrAlice, StartTime: y2kPlus(0), - Time: y2kPlus(11), + Time: y2kPlus(4), Value: 10, }, { Attributes: fltrBob, StartTime: y2kPlus(0), - Time: y2kPlus(11), + Time: y2kPlus(4), Value: 3, }, }, @@ -238,19 +238,19 @@ func testCumulativeLastValue[N int64 | float64]() func(*testing.T) { { Attributes: fltrAlice, StartTime: y2kPlus(0), - Time: y2kPlus(16), + Time: y2kPlus(5), Value: 1, }, { Attributes: fltrBob, StartTime: y2kPlus(0), - Time: y2kPlus(16), + Time: y2kPlus(5), Value: 1, }, { Attributes: overflowSet, StartTime: y2kPlus(0), - Time: y2kPlus(16), + Time: y2kPlus(5), Value: 1, }, }, @@ -287,13 +287,13 @@ func testDeltaPrecomputedLastValue[N int64 | float64]() func(*testing.T) { { Attributes: fltrAlice, StartTime: y2kPlus(1), - Time: y2kPlus(7), + Time: y2kPlus(2), Value: 2, }, { Attributes: fltrBob, StartTime: y2kPlus(1), - Time: y2kPlus(7), + Time: y2kPlus(2), Value: -10, }, }, @@ -314,14 +314,14 @@ func testDeltaPrecomputedLastValue[N int64 | float64]() func(*testing.T) { DataPoints: []metricdata.DataPoint[N]{ { Attributes: fltrAlice, - StartTime: y2kPlus(8), - Time: y2kPlus(11), + StartTime: y2kPlus(3), + Time: y2kPlus(4), Value: 10, }, { Attributes: fltrBob, - StartTime: y2kPlus(8), - Time: y2kPlus(11), + StartTime: y2kPlus(3), + Time: y2kPlus(4), Value: 3, }, }, @@ -341,20 +341,20 @@ func testDeltaPrecomputedLastValue[N int64 | float64]() func(*testing.T) { DataPoints: []metricdata.DataPoint[N]{ { Attributes: fltrAlice, - StartTime: y2kPlus(11), - Time: y2kPlus(16), + StartTime: y2kPlus(4), + Time: y2kPlus(5), Value: 1, }, { Attributes: fltrBob, - StartTime: y2kPlus(11), - Time: y2kPlus(16), + StartTime: y2kPlus(4), + Time: y2kPlus(5), Value: 1, }, { Attributes: overflowSet, - StartTime: y2kPlus(11), - Time: y2kPlus(16), + StartTime: y2kPlus(4), + Time: y2kPlus(5), Value: 1, }, }, @@ -391,13 +391,13 @@ func testCumulativePrecomputedLastValue[N int64 | float64]() func(*testing.T) { { Attributes: fltrAlice, StartTime: y2kPlus(0), - Time: y2kPlus(7), + Time: y2kPlus(2), Value: 2, }, { Attributes: fltrBob, StartTime: y2kPlus(0), - Time: y2kPlus(7), + Time: y2kPlus(2), Value: -10, }, }, @@ -419,13 +419,13 @@ func testCumulativePrecomputedLastValue[N int64 | float64]() func(*testing.T) { { Attributes: fltrAlice, StartTime: y2kPlus(0), - Time: y2kPlus(11), + Time: y2kPlus(4), Value: 10, }, { Attributes: fltrBob, StartTime: y2kPlus(0), - Time: y2kPlus(11), + Time: y2kPlus(4), Value: 3, }, }, @@ -446,19 +446,19 @@ func testCumulativePrecomputedLastValue[N int64 | float64]() func(*testing.T) { { Attributes: fltrAlice, StartTime: y2kPlus(0), - Time: y2kPlus(16), + Time: y2kPlus(5), Value: 1, }, { Attributes: fltrBob, StartTime: y2kPlus(0), - Time: y2kPlus(16), + Time: y2kPlus(5), Value: 1, }, { Attributes: overflowSet, StartTime: y2kPlus(0), - Time: y2kPlus(16), + Time: y2kPlus(5), Value: 1, }, }, diff --git a/sdk/metric/internal/aggregate/sum.go b/sdk/metric/internal/aggregate/sum.go index babe76aba9b7..c9116a675dda 100644 --- a/sdk/metric/internal/aggregate/sum.go +++ b/sdk/metric/internal/aggregate/sum.go @@ -36,8 +36,6 @@ func newValueMap[N int64 | float64](limit int, r func() exemplar.Reservoir) *val } func (s *valueMap[N]) measure(ctx context.Context, value N, fltrAttr attribute.Set, droppedAttr []attribute.KeyValue) { - t := now() - s.Lock() defer s.Unlock() @@ -49,7 +47,7 @@ func (s *valueMap[N]) measure(ctx context.Context, value N, fltrAttr attribute.S v.attrs = attr v.n += value - v.res.Offer(ctx, t, exemplar.NewValue(value), droppedAttr) + v.res.Offer(ctx, exemplar.NewValue(value), droppedAttr) s.values[attr.Equivalent()] = v } diff --git a/sdk/metric/internal/aggregate/sum_test.go b/sdk/metric/internal/aggregate/sum_test.go index c20adaed500b..bb825e183757 100644 --- a/sdk/metric/internal/aggregate/sum_test.go +++ b/sdk/metric/internal/aggregate/sum_test.go @@ -75,13 +75,13 @@ func testDeltaSum[N int64 | float64]() func(t *testing.T) { { Attributes: fltrAlice, StartTime: y2kPlus(1), - Time: y2kPlus(7), + Time: y2kPlus(2), Value: 4, }, { Attributes: fltrBob, StartTime: y2kPlus(1), - Time: y2kPlus(7), + Time: y2kPlus(2), Value: -11, }, }, @@ -101,14 +101,14 @@ func testDeltaSum[N int64 | float64]() func(t *testing.T) { DataPoints: []metricdata.DataPoint[N]{ { Attributes: fltrAlice, - StartTime: y2kPlus(7), - Time: y2kPlus(10), + StartTime: y2kPlus(2), + Time: y2kPlus(3), Value: 10, }, { Attributes: fltrBob, - StartTime: y2kPlus(7), - Time: y2kPlus(10), + StartTime: y2kPlus(2), + Time: y2kPlus(3), Value: 3, }, }, @@ -143,20 +143,20 @@ func testDeltaSum[N int64 | float64]() func(t *testing.T) { DataPoints: []metricdata.DataPoint[N]{ { Attributes: fltrAlice, - StartTime: y2kPlus(11), - Time: y2kPlus(16), + StartTime: y2kPlus(4), + Time: y2kPlus(5), Value: 1, }, { Attributes: fltrBob, - StartTime: y2kPlus(11), - Time: y2kPlus(16), + StartTime: y2kPlus(4), + Time: y2kPlus(5), Value: 1, }, { Attributes: overflowSet, - StartTime: y2kPlus(11), - Time: y2kPlus(16), + StartTime: y2kPlus(4), + Time: y2kPlus(5), Value: 2, }, }, @@ -203,13 +203,13 @@ func testCumulativeSum[N int64 | float64]() func(t *testing.T) { { Attributes: fltrAlice, StartTime: y2kPlus(0), - Time: y2kPlus(7), + Time: y2kPlus(2), Value: 4, }, { Attributes: fltrBob, StartTime: y2kPlus(0), - Time: y2kPlus(7), + Time: y2kPlus(2), Value: -11, }, }, @@ -230,13 +230,13 @@ func testCumulativeSum[N int64 | float64]() func(t *testing.T) { { Attributes: fltrAlice, StartTime: y2kPlus(0), - Time: y2kPlus(10), + Time: y2kPlus(3), Value: 14, }, { Attributes: fltrBob, StartTime: y2kPlus(0), - Time: y2kPlus(10), + Time: y2kPlus(3), Value: -8, }, }, @@ -258,19 +258,19 @@ func testCumulativeSum[N int64 | float64]() func(t *testing.T) { { Attributes: fltrAlice, StartTime: y2kPlus(0), - Time: y2kPlus(13), + Time: y2kPlus(4), Value: 14, }, { Attributes: fltrBob, StartTime: y2kPlus(0), - Time: y2kPlus(13), + Time: y2kPlus(4), Value: -8, }, { Attributes: overflowSet, StartTime: y2kPlus(0), - Time: y2kPlus(13), + Time: y2kPlus(4), Value: 2, }, }, @@ -317,13 +317,13 @@ func testDeltaPrecomputedSum[N int64 | float64]() func(t *testing.T) { { Attributes: fltrAlice, StartTime: y2kPlus(1), - Time: y2kPlus(7), + Time: y2kPlus(2), Value: 4, }, { Attributes: fltrBob, StartTime: y2kPlus(1), - Time: y2kPlus(7), + Time: y2kPlus(2), Value: -11, }, }, @@ -344,14 +344,14 @@ func testDeltaPrecomputedSum[N int64 | float64]() func(t *testing.T) { DataPoints: []metricdata.DataPoint[N]{ { Attributes: fltrAlice, - StartTime: y2kPlus(7), - Time: y2kPlus(11), + StartTime: y2kPlus(2), + Time: y2kPlus(3), Value: 7, }, { Attributes: fltrBob, - StartTime: y2kPlus(7), - Time: y2kPlus(11), + StartTime: y2kPlus(2), + Time: y2kPlus(3), Value: 14, }, }, @@ -386,20 +386,20 @@ func testDeltaPrecomputedSum[N int64 | float64]() func(t *testing.T) { DataPoints: []metricdata.DataPoint[N]{ { Attributes: fltrAlice, - StartTime: y2kPlus(12), - Time: y2kPlus(17), + StartTime: y2kPlus(4), + Time: y2kPlus(5), Value: 1, }, { Attributes: fltrBob, - StartTime: y2kPlus(12), - Time: y2kPlus(17), + StartTime: y2kPlus(4), + Time: y2kPlus(5), Value: 1, }, { Attributes: overflowSet, - StartTime: y2kPlus(12), - Time: y2kPlus(17), + StartTime: y2kPlus(4), + Time: y2kPlus(5), Value: 2, }, }, @@ -446,13 +446,13 @@ func testCumulativePrecomputedSum[N int64 | float64]() func(t *testing.T) { { Attributes: fltrAlice, StartTime: y2kPlus(0), - Time: y2kPlus(7), + Time: y2kPlus(2), Value: 4, }, { Attributes: fltrBob, StartTime: y2kPlus(0), - Time: y2kPlus(7), + Time: y2kPlus(2), Value: -11, }, }, @@ -474,13 +474,13 @@ func testCumulativePrecomputedSum[N int64 | float64]() func(t *testing.T) { { Attributes: fltrAlice, StartTime: y2kPlus(0), - Time: y2kPlus(11), + Time: y2kPlus(3), Value: 11, }, { Attributes: fltrBob, StartTime: y2kPlus(0), - Time: y2kPlus(11), + Time: y2kPlus(3), Value: 3, }, }, @@ -516,19 +516,19 @@ func testCumulativePrecomputedSum[N int64 | float64]() func(t *testing.T) { { Attributes: fltrAlice, StartTime: y2kPlus(0), - Time: y2kPlus(17), + Time: y2kPlus(5), Value: 1, }, { Attributes: fltrBob, StartTime: y2kPlus(0), - Time: y2kPlus(17), + Time: y2kPlus(5), Value: 1, }, { Attributes: overflowSet, StartTime: y2kPlus(0), - Time: y2kPlus(17), + Time: y2kPlus(5), Value: 2, }, }, diff --git a/sdk/metric/internal/exemplar/drop.go b/sdk/metric/internal/exemplar/drop.go index bf21e45dfaf2..d8716a998653 100644 --- a/sdk/metric/internal/exemplar/drop.go +++ b/sdk/metric/internal/exemplar/drop.go @@ -5,7 +5,6 @@ package exemplar // import "go.opentelemetry.io/otel/sdk/metric/internal/exempla import ( "context" - "time" "go.opentelemetry.io/otel/attribute" ) @@ -16,7 +15,7 @@ func Drop() Reservoir { return &dropRes{} } type dropRes struct{} // Offer does nothing, all measurements offered will be dropped. -func (r *dropRes) Offer(context.Context, time.Time, Value, []attribute.KeyValue) {} +func (r *dropRes) Offer(context.Context, Value, []attribute.KeyValue) {} // Collect resets dest. No exemplars will ever be returned. func (r *dropRes) Collect(dest *[]Exemplar) { diff --git a/sdk/metric/internal/exemplar/filter.go b/sdk/metric/internal/exemplar/filter.go index d96aacc281aa..e3ddcf092698 100644 --- a/sdk/metric/internal/exemplar/filter.go +++ b/sdk/metric/internal/exemplar/filter.go @@ -5,7 +5,6 @@ package exemplar // import "go.opentelemetry.io/otel/sdk/metric/internal/exempla import ( "context" - "time" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" @@ -22,8 +21,8 @@ type filtered struct { Reservoir } -func (f filtered) Offer(ctx context.Context, t time.Time, n Value, a []attribute.KeyValue) { +func (f filtered) Offer(ctx context.Context, n Value, a []attribute.KeyValue) { if trace.SpanContextFromContext(ctx).IsSampled() { - f.Reservoir.Offer(ctx, t, n, a) + f.Reservoir.Offer(ctx, n, a) } } diff --git a/sdk/metric/internal/exemplar/filter_test.go b/sdk/metric/internal/exemplar/filter_test.go index eadcc667a8b1..c1e6e4c9fc01 100644 --- a/sdk/metric/internal/exemplar/filter_test.go +++ b/sdk/metric/internal/exemplar/filter_test.go @@ -6,7 +6,6 @@ package exemplar // import "go.opentelemetry.io/otel/sdk/metric/internal/exempla import ( "context" "testing" - "time" "github.com/stretchr/testify/assert" @@ -25,9 +24,9 @@ func testSampledFiltered[N int64 | float64](t *testing.T) { r := SampledFilter(under) ctx := context.Background() - r.Offer(ctx, staticTime, NewValue(N(0)), nil) + r.Offer(ctx, NewValue(N(0)), nil) assert.False(t, under.OfferCalled, "underlying Reservoir Offer called") - r.Offer(sample(ctx), staticTime, NewValue(N(0)), nil) + r.Offer(sample(ctx), NewValue(N(0)), nil) assert.True(t, under.OfferCalled, "underlying Reservoir Offer not called") r.Collect(nil) @@ -48,7 +47,7 @@ type res struct { CollectCalled bool } -func (r *res) Offer(context.Context, time.Time, Value, []attribute.KeyValue) { +func (r *res) Offer(context.Context, Value, []attribute.KeyValue) { r.OfferCalled = true } diff --git a/sdk/metric/internal/exemplar/hist.go b/sdk/metric/internal/exemplar/hist.go index a6ff86d02714..c284311729ec 100644 --- a/sdk/metric/internal/exemplar/hist.go +++ b/sdk/metric/internal/exemplar/hist.go @@ -7,7 +7,6 @@ import ( "context" "slices" "sort" - "time" "go.opentelemetry.io/otel/attribute" ) @@ -32,7 +31,7 @@ type histRes struct { bounds []float64 } -func (r *histRes) Offer(ctx context.Context, t time.Time, v Value, a []attribute.KeyValue) { +func (r *histRes) Offer(ctx context.Context, v Value, a []attribute.KeyValue) { var x float64 switch v.Type() { case Int64ValueType: @@ -42,5 +41,5 @@ func (r *histRes) Offer(ctx context.Context, t time.Time, v Value, a []attribute default: panic("unknown value type") } - r.store[sort.SearchFloat64s(r.bounds, x)] = newMeasurement(ctx, t, v, a) + r.store[sort.SearchFloat64s(r.bounds, x)] = newMeasurement(ctx, now(), v, a) } diff --git a/sdk/metric/internal/exemplar/rand.go b/sdk/metric/internal/exemplar/rand.go index 199a2608f718..666d5f150aa4 100644 --- a/sdk/metric/internal/exemplar/rand.go +++ b/sdk/metric/internal/exemplar/rand.go @@ -76,7 +76,7 @@ type randRes struct { w float64 } -func (r *randRes) Offer(ctx context.Context, t time.Time, n Value, a []attribute.KeyValue) { +func (r *randRes) Offer(ctx context.Context, n Value, a []attribute.KeyValue) { // The following algorithm is "Algorithm L" from Li, Kim-Hung (4 December // 1994). "Reservoir-Sampling Algorithms of Time Complexity // O(n(1+log(N/n)))". ACM Transactions on Mathematical Software. 20 (4): @@ -118,6 +118,7 @@ func (r *randRes) Offer(ctx context.Context, t time.Time, n Value, a []attribute // https://github.com/MrAlias/reservoir-sampling for a performance // comparison of reservoir sampling algorithms. + t := now() if int(r.count) < cap(r.store) { r.store[r.count] = newMeasurement(ctx, t, n, a) } else { diff --git a/sdk/metric/internal/exemplar/rand_test.go b/sdk/metric/internal/exemplar/rand_test.go index a4c42dcf72c7..79d31b3d39b6 100644 --- a/sdk/metric/internal/exemplar/rand_test.go +++ b/sdk/metric/internal/exemplar/rand_test.go @@ -37,7 +37,7 @@ func TestFixedSizeSamplingCorrectness(t *testing.T) { r := FixedSize(sampleSize) for _, value := range data { - r.Offer(context.Background(), staticTime, NewValue(value), nil) + r.Offer(context.Background(), NewValue(value), nil) } var sum float64 diff --git a/sdk/metric/internal/exemplar/reservoir.go b/sdk/metric/internal/exemplar/reservoir.go index 80fa59554f20..bcefc9fdfafd 100644 --- a/sdk/metric/internal/exemplar/reservoir.go +++ b/sdk/metric/internal/exemplar/reservoir.go @@ -23,10 +23,14 @@ type Reservoir interface { // The time t is the time when the measurement was made. The val and attr // parameters are the value and dropped (filtered) attributes of the // measurement respectively. - Offer(ctx context.Context, t time.Time, val Value, attr []attribute.KeyValue) + Offer(ctx context.Context, val Value, attr []attribute.KeyValue) // Collect returns all the held exemplars. // // The Reservoir state is preserved after this call. Collect(dest *[]Exemplar) } + +// now is used to return the current local time while allowing tests to +// override the default time.Now function. +var now = time.Now diff --git a/sdk/metric/internal/exemplar/reservoir_test.go b/sdk/metric/internal/exemplar/reservoir_test.go index b5fc5453d421..cedbbbf19863 100644 --- a/sdk/metric/internal/exemplar/reservoir_test.go +++ b/sdk/metric/internal/exemplar/reservoir_test.go @@ -22,6 +22,9 @@ type factory func(requstedCap int) (r Reservoir, actualCap int) func ReservoirTest[N int64 | float64](f factory) func(*testing.T) { return func(t *testing.T) { + // return staticTime from calls to now() during the test + now = func() time.Time { return staticTime } + defer func() { now = time.Now }() t.Helper() ctx := context.Background() @@ -42,7 +45,7 @@ func ReservoirTest[N int64 | float64](f factory) func(*testing.T) { }) ctx := trace.ContextWithSpanContext(ctx, sc) - r.Offer(ctx, staticTime, NewValue(N(10)), nil) + r.Offer(ctx, NewValue(N(10)), nil) var dest []Exemplar r.Collect(&dest) @@ -66,7 +69,7 @@ func ReservoirTest[N int64 | float64](f factory) func(*testing.T) { } adminTrue := attribute.Bool("admin", true) - r.Offer(ctx, staticTime, NewValue(N(10)), []attribute.KeyValue{adminTrue}) + r.Offer(ctx, NewValue(N(10)), []attribute.KeyValue{adminTrue}) var dest []Exemplar r.Collect(&dest) @@ -88,7 +91,7 @@ func ReservoirTest[N int64 | float64](f factory) func(*testing.T) { t.Skip("skipping, reservoir capacity less than 2:", n) } - r.Offer(ctx, staticTime, NewValue(N(10)), nil) + r.Offer(ctx, NewValue(N(10)), nil) var dest []Exemplar r.Collect(&dest) @@ -106,7 +109,7 @@ func ReservoirTest[N int64 | float64](f factory) func(*testing.T) { for i := 0; i < n+1; i++ { v := NewValue(N(i)) - r.Offer(ctx, staticTime, v, nil) + r.Offer(ctx, v, nil) } var dest []Exemplar @@ -116,7 +119,7 @@ func ReservoirTest[N int64 | float64](f factory) func(*testing.T) { // Ensure the collect reset also resets any counting state. for i := 0; i < n+1; i++ { v := NewValue(N(i)) - r.Offer(ctx, staticTime, v, nil) + r.Offer(ctx, v, nil) } dest = dest[:0] @@ -132,7 +135,7 @@ func ReservoirTest[N int64 | float64](f factory) func(*testing.T) { t.Skip("skipping, reservoir capacity greater than 0:", n) } - r.Offer(context.Background(), staticTime, NewValue(N(10)), nil) + r.Offer(context.Background(), NewValue(N(10)), nil) dest := []Exemplar{{}} // Should be reset to empty. r.Collect(&dest)