Skip to content

Commit 0485de2

Browse files
dashpoleXSAMMrAlias
authored
Move time.Now call into exemplar reservoir to improve performance (#5545)
Part of addressing #5542. ### Motivation This removes the `time.Now()` call from filtered-out Exemplars by only invoking `time.Now()` after the filtering decision is made. This improvement is especially noticeable for measurements without any attributes. ``` goos: linux goarch: amd64 pkg: go.opentelemetry.io/otel/sdk/metric cpu: AMD EPYC 7B12 │ old.txt │ new.txt │ │ sec/op │ sec/op vs base │ SyncMeasure/NoView/Int64Counter/Attributes/0-24 158.20n ± 4% 99.83n ± 1% -36.90% (p=0.000 n=10) SyncMeasure/NoView/Int64Counter/Attributes/1-24 333.3n ± 4% 274.8n ± 1% -17.55% (p=0.000 n=10) SyncMeasure/NoView/Int64Counter/Attributes/10-24 1.640µ ± 1% 1.600µ ± 1% -2.41% (p=0.000 n=10) SyncMeasure/NoView/Float64Counter/Attributes/0-24 159.0n ± 3% 101.3n ± 0% -36.27% (p=0.000 n=10) SyncMeasure/NoView/Float64Counter/Attributes/1-24 340.0n ± 2% 272.0n ± 1% -20.00% (p=0.000 n=10) SyncMeasure/NoView/Float64Counter/Attributes/10-24 1.661µ ± 1% 1.597µ ± 0% -3.85% (p=0.000 n=10) SyncMeasure/NoView/Int64UpDownCounter/Attributes/0-24 159.8n ± 1% 103.1n ± 0% -35.50% (p=0.000 n=10) SyncMeasure/NoView/Int64UpDownCounter/Attributes/1-24 339.5n ± 1% 273.1n ± 0% -19.57% (p=0.000 n=10) SyncMeasure/NoView/Int64UpDownCounter/Attributes/10-24 1.656µ ± 0% 1.589µ ± 0% -4.05% (p=0.000 n=10) SyncMeasure/NoView/Float64UpDownCounter/Attributes/0-24 159.3n ± 2% 100.8n ± 0% -36.74% (p=0.000 n=10) SyncMeasure/NoView/Float64UpDownCounter/Attributes/1-24 337.9n ± 2% 271.8n ± 1% -19.55% (p=0.000 n=10) SyncMeasure/NoView/Float64UpDownCounter/Attributes/10-24 1.657µ ± 0% 1.593µ ± 1% -3.83% (p=0.000 n=10) SyncMeasure/NoView/Int64Histogram/Attributes/0-24 144.65n ± 4% 89.38n ± 0% -38.21% (p=0.000 n=10) SyncMeasure/NoView/Int64Histogram/Attributes/1-24 235.7n ± 2% 183.5n ± 0% -22.15% (p=0.000 n=10) SyncMeasure/NoView/Int64Histogram/Attributes/10-24 900.8n ± 1% 836.8n ± 0% -7.10% (p=0.000 n=10) SyncMeasure/NoView/Float64Histogram/Attributes/0-24 145.60n ± 5% 93.48n ± 1% -35.80% (p=0.000 n=10) SyncMeasure/NoView/Float64Histogram/Attributes/1-24 240.9n ± 1% 183.0n ± 0% -24.06% (p=0.000 n=10) SyncMeasure/NoView/Float64Histogram/Attributes/10-24 905.6n ± 1% 826.3n ± 0% -8.76% (p=0.000 n=10) SyncMeasure/DropView/Int64Counter/Attributes/0-24 20.33n ± 0% 20.35n ± 0% ~ (p=0.302 n=10) SyncMeasure/DropView/Int64Counter/Attributes/1-24 26.46n ± 0% 26.45n ± 1% ~ (p=0.868 n=10) SyncMeasure/DropView/Int64Counter/Attributes/10-24 26.50n ± 0% 26.47n ± 0% ~ (p=0.208 n=10) SyncMeasure/DropView/Float64Counter/Attributes/0-24 20.34n ± 1% 20.27n ± 0% -0.34% (p=0.009 n=10) SyncMeasure/DropView/Float64Counter/Attributes/1-24 26.55n ± 0% 26.60n ± 1% ~ (p=0.109 n=10) SyncMeasure/DropView/Float64Counter/Attributes/10-24 26.59n ± 1% 26.57n ± 1% ~ (p=0.926 n=10) SyncMeasure/DropView/Int64UpDownCounter/Attributes/0-24 20.38n ± 1% 20.38n ± 0% ~ (p=0.725 n=10) SyncMeasure/DropView/Int64UpDownCounter/Attributes/1-24 26.39n ± 0% 26.44n ± 0% ~ (p=0.238 n=10) SyncMeasure/DropView/Int64UpDownCounter/Attributes/10-24 26.52n ± 0% 26.42n ± 0% -0.36% (p=0.049 n=10) SyncMeasure/DropView/Float64UpDownCounter/Attributes/0-24 20.30n ± 0% 20.25n ± 0% ~ (p=0.196 n=10) SyncMeasure/DropView/Float64UpDownCounter/Attributes/1-24 26.57n ± 0% 26.54n ± 1% ~ (p=0.540 n=10) SyncMeasure/DropView/Float64UpDownCounter/Attributes/10-24 26.57n ± 0% 26.51n ± 1% ~ (p=0.643 n=10) SyncMeasure/DropView/Int64Histogram/Attributes/0-24 20.37n ± 0% 20.36n ± 1% ~ (p=1.000 n=10) SyncMeasure/DropView/Int64Histogram/Attributes/1-24 26.41n ± 0% 26.50n ± 0% +0.32% (p=0.007 n=10) SyncMeasure/DropView/Int64Histogram/Attributes/10-24 26.44n ± 0% 26.55n ± 1% +0.42% (p=0.012 n=10) SyncMeasure/DropView/Float64Histogram/Attributes/0-24 20.30n ± 0% 20.45n ± 0% +0.74% (p=0.000 n=10) SyncMeasure/DropView/Float64Histogram/Attributes/1-24 26.52n ± 0% 26.48n ± 0% ~ (p=0.127 n=10) SyncMeasure/DropView/Float64Histogram/Attributes/10-24 26.55n ± 0% 26.48n ± 0% -0.26% (p=0.002 n=10) SyncMeasure/AttrFilterView/Int64Counter/Attributes/0-24 170.5n ± 2% 110.8n ± 0% -35.03% (p=0.000 n=10) SyncMeasure/AttrFilterView/Int64Counter/Attributes/1-24 402.5n ± 1% 331.5n ± 1% -17.64% (p=0.000 n=10) SyncMeasure/AttrFilterView/Int64Counter/Attributes/10-24 1.363µ ± 1% 1.281µ ± 1% -6.02% (p=0.000 n=10) SyncMeasure/AttrFilterView/Float64Counter/Attributes/0-24 170.6n ± 1% 111.5n ± 1% -34.64% (p=0.000 n=10) SyncMeasure/AttrFilterView/Float64Counter/Attributes/1-24 397.1n ± 1% 335.9n ± 0% -15.41% (p=0.000 n=10) SyncMeasure/AttrFilterView/Float64Counter/Attributes/10-24 1.371µ ± 1% 1.279µ ± 1% -6.71% (p=0.000 n=10) SyncMeasure/AttrFilterView/Int64UpDownCounter/Attributes/0-24 170.1n ± 1% 112.2n ± 0% -34.09% (p=0.000 n=10) SyncMeasure/AttrFilterView/Int64UpDownCounter/Attributes/1-24 397.5n ± 1% 330.2n ± 0% -16.93% (p=0.000 n=10) SyncMeasure/AttrFilterView/Int64UpDownCounter/Attributes/10-24 1.371µ ± 1% 1.289µ ± 1% -5.95% (p=0.000 n=10) SyncMeasure/AttrFilterView/Float64UpDownCounter/Attributes/0-24 171.4n ± 2% 112.9n ± 0% -34.13% (p=0.000 n=10) SyncMeasure/AttrFilterView/Float64UpDownCounter/Attributes/1-24 397.0n ± 3% 336.4n ± 0% -15.24% (p=0.000 n=10) SyncMeasure/AttrFilterView/Float64UpDownCounter/Attributes/10-24 1.383µ ± 1% 1.305µ ± 1% -5.61% (p=0.000 n=10) SyncMeasure/AttrFilterView/Int64Histogram/Attributes/0-24 157.30n ± 2% 98.58n ± 1% -37.33% (p=0.000 n=6+10) ``` ### Changes * Introduce `exemplar.Filter`, which is a filter function based on the context. It will not be user-facing, so we can always add other parameters later if needed. * Introduce `exemplar.FilteredReservoir`, which is similar to a reservoir, except it does not receive a timestamp. It gets the current time after the filter decision has been made. It uses generics to avoid the call to exemplar.NewValue(), since it is internal-only. * The `exemplar.Reservoir` is left as-is, so that it can be made public when exemplars are stable. It still includes a timestamp argument. * Unit tests are updated to expect a much lower number of calls to time.Now * `exemplar.Drop` is now an `exemplar.FilteredReservoir` instead of a `Reservoir`, since we don't need a Reservoir to store things in if the measurement is always dropped. Co-authored-by: Sam Xie <[email protected]> Co-authored-by: Tyler Yahn <[email protected]>
1 parent 1cefb17 commit 0485de2

18 files changed

+263
-231
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
3030
- 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)
3131
- 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)
3232
- Use non-generic functions in the `Start` method of `"go.opentelemetry.io/otel/sdk/trace".Trace` to reduce memory allocation. (#5497)
33+
- Improve performance of metric instruments in `go.opentelemetry.io/otel/sdk/metric` by removing unnecessary calls to time.Now. (#5545)
3334

3435
### Fixed
3536

‎sdk/metric/exemplar.go‎

Lines changed: 44 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -19,67 +19,63 @@ import (
1919
// Note: This will only return non-nil values when the experimental exemplar
2020
// feature is enabled and the OTEL_METRICS_EXEMPLAR_FILTER environment variable
2121
// is not set to always_off.
22-
func reservoirFunc(agg Aggregation) func() exemplar.Reservoir {
22+
func reservoirFunc[N int64 | float64](agg Aggregation) func() exemplar.FilteredReservoir[N] {
2323
if !x.Exemplars.Enabled() {
2424
return nil
2525
}
26-
27-
// https://github.com/open-telemetry/opentelemetry-specification/blob/d4b241f451674e8f611bb589477680341006ad2b/specification/metrics/sdk.md#exemplar-defaults
28-
resF := func() func() exemplar.Reservoir {
29-
// Explicit bucket histogram aggregation with more than 1 bucket will
30-
// use AlignedHistogramBucketExemplarReservoir.
31-
a, ok := agg.(AggregationExplicitBucketHistogram)
32-
if ok && len(a.Boundaries) > 0 {
33-
cp := slices.Clone(a.Boundaries)
34-
return func() exemplar.Reservoir {
35-
bounds := cp
36-
return exemplar.Histogram(bounds)
37-
}
38-
}
39-
40-
var n int
41-
if a, ok := agg.(AggregationBase2ExponentialHistogram); ok {
42-
// Base2 Exponential Histogram Aggregation SHOULD use a
43-
// SimpleFixedSizeExemplarReservoir with a reservoir equal to the
44-
// smaller of the maximum number of buckets configured on the
45-
// aggregation or twenty (e.g. min(20, max_buckets)).
46-
n = int(a.MaxSize)
47-
if n > 20 {
48-
n = 20
49-
}
50-
} else {
51-
// https://github.com/open-telemetry/opentelemetry-specification/blob/e94af89e3d0c01de30127a0f423e912f6cda7bed/specification/metrics/sdk.md#simplefixedsizeexemplarreservoir
52-
// This Exemplar reservoir MAY take a configuration parameter for
53-
// the size of the reservoir. If no size configuration is
54-
// provided, the default size MAY be the number of possible
55-
// concurrent threads (e.g. number of CPUs) to help reduce
56-
// contention. Otherwise, a default size of 1 SHOULD be used.
57-
n = runtime.NumCPU()
58-
if n < 1 {
59-
// Should never be the case, but be defensive.
60-
n = 1
61-
}
62-
}
63-
64-
return func() exemplar.Reservoir {
65-
return exemplar.FixedSize(n)
66-
}
67-
}
68-
6926
// https://github.com/open-telemetry/opentelemetry-specification/blob/d4b241f451674e8f611bb589477680341006ad2b/specification/configuration/sdk-environment-variables.md#exemplar
7027
const filterEnvKey = "OTEL_METRICS_EXEMPLAR_FILTER"
7128

29+
var filter exemplar.Filter
30+
7231
switch os.Getenv(filterEnvKey) {
7332
case "always_on":
74-
return resF()
33+
filter = exemplar.AlwaysOnFilter
7534
case "always_off":
7635
return exemplar.Drop
7736
case "trace_based":
7837
fallthrough
7938
default:
80-
newR := resF()
81-
return func() exemplar.Reservoir {
82-
return exemplar.SampledFilter(newR())
39+
filter = exemplar.SampledFilter
40+
}
41+
42+
// https://github.com/open-telemetry/opentelemetry-specification/blob/d4b241f451674e8f611bb589477680341006ad2b/specification/metrics/sdk.md#exemplar-defaults
43+
// Explicit bucket histogram aggregation with more than 1 bucket will
44+
// use AlignedHistogramBucketExemplarReservoir.
45+
a, ok := agg.(AggregationExplicitBucketHistogram)
46+
if ok && len(a.Boundaries) > 0 {
47+
cp := slices.Clone(a.Boundaries)
48+
return func() exemplar.FilteredReservoir[N] {
49+
bounds := cp
50+
return exemplar.NewFilteredReservoir[N](filter, exemplar.Histogram(bounds))
8351
}
8452
}
53+
54+
var n int
55+
if a, ok := agg.(AggregationBase2ExponentialHistogram); ok {
56+
// Base2 Exponential Histogram Aggregation SHOULD use a
57+
// SimpleFixedSizeExemplarReservoir with a reservoir equal to the
58+
// smaller of the maximum number of buckets configured on the
59+
// aggregation or twenty (e.g. min(20, max_buckets)).
60+
n = int(a.MaxSize)
61+
if n > 20 {
62+
n = 20
63+
}
64+
} else {
65+
// https://github.com/open-telemetry/opentelemetry-specification/blob/e94af89e3d0c01de30127a0f423e912f6cda7bed/specification/metrics/sdk.md#simplefixedsizeexemplarreservoir
66+
// This Exemplar reservoir MAY take a configuration parameter for
67+
// the size of the reservoir. If no size configuration is
68+
// provided, the default size MAY be the number of possible
69+
// concurrent threads (e.g. number of CPUs) to help reduce
70+
// contention. Otherwise, a default size of 1 SHOULD be used.
71+
n = runtime.NumCPU()
72+
if n < 1 {
73+
// Should never be the case, but be defensive.
74+
n = 1
75+
}
76+
}
77+
78+
return func() exemplar.FilteredReservoir[N] {
79+
return exemplar.NewFilteredReservoir[N](filter, exemplar.FixedSize(n))
80+
}
8581
}

‎sdk/metric/internal/aggregate/aggregate.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ type Builder[N int64 | float64] struct {
3939
//
4040
// If this is not provided a default factory function that returns an
4141
// exemplar.Drop reservoir will be used.
42-
ReservoirFunc func() exemplar.Reservoir
42+
ReservoirFunc func() exemplar.FilteredReservoir[N]
4343
// AggregationLimit is the cardinality limit of measurement attributes. Any
4444
// measurement for new attributes once the limit has been reached will be
4545
// aggregated into a single aggregate for the "otel.metric.overflow"
@@ -50,7 +50,7 @@ type Builder[N int64 | float64] struct {
5050
AggregationLimit int
5151
}
5252

53-
func (b Builder[N]) resFunc() func() exemplar.Reservoir {
53+
func (b Builder[N]) resFunc() func() exemplar.FilteredReservoir[N] {
5454
if b.ReservoirFunc != nil {
5555
return b.ReservoirFunc
5656
}

‎sdk/metric/internal/aggregate/aggregate_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,8 @@ func (c *clock) Register() (unregister func()) {
7373
return func() { now = orig }
7474
}
7575

76-
func dropExemplars[N int64 | float64]() exemplar.Reservoir {
77-
return exemplar.Drop()
76+
func dropExemplars[N int64 | float64]() exemplar.FilteredReservoir[N] {
77+
return exemplar.Drop[N]()
7878
}
7979

8080
func TestBuilderFilter(t *testing.T) {

‎sdk/metric/internal/aggregate/exponential_histogram.go‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ const (
3131
// expoHistogramDataPoint is a single data point in an exponential histogram.
3232
type expoHistogramDataPoint[N int64 | float64] struct {
3333
attrs attribute.Set
34-
res exemplar.Reservoir
34+
res exemplar.FilteredReservoir[N]
3535

3636
count uint64
3737
min N
@@ -282,7 +282,7 @@ func (b *expoBuckets) downscale(delta int) {
282282
// newExponentialHistogram returns an Aggregator that summarizes a set of
283283
// measurements as an exponential histogram. Each histogram is scoped by attributes
284284
// and the aggregation cycle the measurements were made in.
285-
func newExponentialHistogram[N int64 | float64](maxSize, maxScale int32, noMinMax, noSum bool, limit int, r func() exemplar.Reservoir) *expoHistogram[N] {
285+
func newExponentialHistogram[N int64 | float64](maxSize, maxScale int32, noMinMax, noSum bool, limit int, r func() exemplar.FilteredReservoir[N]) *expoHistogram[N] {
286286
return &expoHistogram[N]{
287287
noSum: noSum,
288288
noMinMax: noMinMax,
@@ -305,7 +305,7 @@ type expoHistogram[N int64 | float64] struct {
305305
maxSize int
306306
maxScale int
307307

308-
newRes func() exemplar.Reservoir
308+
newRes func() exemplar.FilteredReservoir[N]
309309
limit limiter[*expoHistogramDataPoint[N]]
310310
values map[attribute.Distinct]*expoHistogramDataPoint[N]
311311
valuesMu sync.Mutex
@@ -319,8 +319,6 @@ func (e *expoHistogram[N]) measure(ctx context.Context, value N, fltrAttr attrib
319319
return
320320
}
321321

322-
t := now()
323-
324322
e.valuesMu.Lock()
325323
defer e.valuesMu.Unlock()
326324

@@ -333,7 +331,7 @@ func (e *expoHistogram[N]) measure(ctx context.Context, value N, fltrAttr attrib
333331
e.values[attr.Equivalent()] = v
334332
}
335333
v.record(value)
336-
v.res.Offer(ctx, t, exemplar.NewValue(value), droppedAttr)
334+
v.res.Offer(ctx, value, droppedAttr)
337335
}
338336

339337
func (e *expoHistogram[N]) delta(dest *metricdata.Aggregation) int {

‎sdk/metric/internal/aggregate/exponential_histogram_test.go‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -778,7 +778,7 @@ func testDeltaExpoHist[N int64 | float64]() func(t *testing.T) {
778778
{
779779
Attributes: fltrAlice,
780780
StartTime: y2kPlus(1),
781-
Time: y2kPlus(9),
781+
Time: y2kPlus(2),
782782
Count: 7,
783783
Min: metricdata.NewExtrema[N](-1),
784784
Max: metricdata.NewExtrema[N](16),
@@ -832,8 +832,8 @@ func testDeltaExpoHist[N int64 | float64]() func(t *testing.T) {
832832
DataPoints: []metricdata.ExponentialHistogramDataPoint[N]{
833833
{
834834
Attributes: fltrAlice,
835-
StartTime: y2kPlus(10),
836-
Time: y2kPlus(24),
835+
StartTime: y2kPlus(3),
836+
Time: y2kPlus(4),
837837
Count: 7,
838838
Min: metricdata.NewExtrema[N](-1),
839839
Max: metricdata.NewExtrema[N](16),
@@ -850,8 +850,8 @@ func testDeltaExpoHist[N int64 | float64]() func(t *testing.T) {
850850
},
851851
{
852852
Attributes: overflowSet,
853-
StartTime: y2kPlus(10),
854-
Time: y2kPlus(24),
853+
StartTime: y2kPlus(3),
854+
Time: y2kPlus(4),
855855
Count: 6,
856856
Min: metricdata.NewExtrema[N](1),
857857
Max: metricdata.NewExtrema[N](16),
@@ -905,7 +905,7 @@ func testCumulativeExpoHist[N int64 | float64]() func(t *testing.T) {
905905
{
906906
Attributes: fltrAlice,
907907
StartTime: y2kPlus(0),
908-
Time: y2kPlus(9),
908+
Time: y2kPlus(2),
909909
Count: 7,
910910
Min: metricdata.NewExtrema[N](-1),
911911
Max: metricdata.NewExtrema[N](16),
@@ -938,7 +938,7 @@ func testCumulativeExpoHist[N int64 | float64]() func(t *testing.T) {
938938
{
939939
Attributes: fltrAlice,
940940
StartTime: y2kPlus(0),
941-
Time: y2kPlus(13),
941+
Time: y2kPlus(3),
942942
Count: 10,
943943
Min: metricdata.NewExtrema[N](-1),
944944
Max: metricdata.NewExtrema[N](16),
@@ -967,7 +967,7 @@ func testCumulativeExpoHist[N int64 | float64]() func(t *testing.T) {
967967
{
968968
Attributes: fltrAlice,
969969
StartTime: y2kPlus(0),
970-
Time: y2kPlus(14),
970+
Time: y2kPlus(4),
971971
Count: 10,
972972
Min: metricdata.NewExtrema[N](-1),
973973
Max: metricdata.NewExtrema[N](16),
@@ -1004,7 +1004,7 @@ func testCumulativeExpoHist[N int64 | float64]() func(t *testing.T) {
10041004
{
10051005
Attributes: fltrAlice,
10061006
StartTime: y2kPlus(0),
1007-
Time: y2kPlus(21),
1007+
Time: y2kPlus(5),
10081008
Count: 10,
10091009
Min: metricdata.NewExtrema[N](-1),
10101010
Max: metricdata.NewExtrema[N](16),
@@ -1022,7 +1022,7 @@ func testCumulativeExpoHist[N int64 | float64]() func(t *testing.T) {
10221022
{
10231023
Attributes: overflowSet,
10241024
StartTime: y2kPlus(0),
1025-
Time: y2kPlus(21),
1025+
Time: y2kPlus(5),
10261026
Count: 6,
10271027
Min: metricdata.NewExtrema[N](1),
10281028
Max: metricdata.NewExtrema[N](16),

‎sdk/metric/internal/aggregate/histogram.go‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import (
1717

1818
type buckets[N int64 | float64] struct {
1919
attrs attribute.Set
20-
res exemplar.Reservoir
20+
res exemplar.FilteredReservoir[N]
2121

2222
counts []uint64
2323
count uint64
@@ -48,13 +48,13 @@ type histValues[N int64 | float64] struct {
4848
noSum bool
4949
bounds []float64
5050

51-
newRes func() exemplar.Reservoir
51+
newRes func() exemplar.FilteredReservoir[N]
5252
limit limiter[*buckets[N]]
5353
values map[attribute.Distinct]*buckets[N]
5454
valuesMu sync.Mutex
5555
}
5656

57-
func newHistValues[N int64 | float64](bounds []float64, noSum bool, limit int, r func() exemplar.Reservoir) *histValues[N] {
57+
func newHistValues[N int64 | float64](bounds []float64, noSum bool, limit int, r func() exemplar.FilteredReservoir[N]) *histValues[N] {
5858
// The responsibility of keeping all buckets correctly associated with the
5959
// passed boundaries is ultimately this type's responsibility. Make a copy
6060
// here so we can always guarantee this. Or, in the case of failure, have
@@ -80,8 +80,6 @@ func (s *histValues[N]) measure(ctx context.Context, value N, fltrAttr attribute
8080
// (s.bounds[len(s.bounds)-1], +∞).
8181
idx := sort.SearchFloat64s(s.bounds, float64(value))
8282

83-
t := now()
84-
8583
s.valuesMu.Lock()
8684
defer s.valuesMu.Unlock()
8785

@@ -106,12 +104,12 @@ func (s *histValues[N]) measure(ctx context.Context, value N, fltrAttr attribute
106104
if !s.noSum {
107105
b.sum(value)
108106
}
109-
b.res.Offer(ctx, t, exemplar.NewValue(value), droppedAttr)
107+
b.res.Offer(ctx, value, droppedAttr)
110108
}
111109

112110
// newHistogram returns an Aggregator that summarizes a set of measurements as
113111
// an histogram.
114-
func newHistogram[N int64 | float64](boundaries []float64, noMinMax, noSum bool, limit int, r func() exemplar.Reservoir) *histogram[N] {
112+
func newHistogram[N int64 | float64](boundaries []float64, noMinMax, noSum bool, limit int, r func() exemplar.FilteredReservoir[N]) *histogram[N] {
115113
return &histogram[N]{
116114
histValues: newHistValues[N](boundaries, noSum, limit, r),
117115
noMinMax: noMinMax,

‎sdk/metric/internal/aggregate/histogram_test.go‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,8 @@ func testDeltaHist[N int64 | float64](c conf[N]) func(t *testing.T) {
8080
agg: metricdata.Histogram[N]{
8181
Temporality: metricdata.DeltaTemporality,
8282
DataPoints: []metricdata.HistogramDataPoint[N]{
83-
c.hPt(fltrAlice, 2, 3, y2kPlus(1), y2kPlus(7)),
84-
c.hPt(fltrBob, 10, 2, y2kPlus(1), y2kPlus(7)),
83+
c.hPt(fltrAlice, 2, 3, y2kPlus(1), y2kPlus(2)),
84+
c.hPt(fltrBob, 10, 2, y2kPlus(1), y2kPlus(2)),
8585
},
8686
},
8787
},
@@ -96,8 +96,8 @@ func testDeltaHist[N int64 | float64](c conf[N]) func(t *testing.T) {
9696
agg: metricdata.Histogram[N]{
9797
Temporality: metricdata.DeltaTemporality,
9898
DataPoints: []metricdata.HistogramDataPoint[N]{
99-
c.hPt(fltrAlice, 10, 1, y2kPlus(7), y2kPlus(10)),
100-
c.hPt(fltrBob, 3, 1, y2kPlus(7), y2kPlus(10)),
99+
c.hPt(fltrAlice, 10, 1, y2kPlus(2), y2kPlus(3)),
100+
c.hPt(fltrBob, 3, 1, y2kPlus(2), y2kPlus(3)),
101101
},
102102
},
103103
},
@@ -126,9 +126,9 @@ func testDeltaHist[N int64 | float64](c conf[N]) func(t *testing.T) {
126126
agg: metricdata.Histogram[N]{
127127
Temporality: metricdata.DeltaTemporality,
128128
DataPoints: []metricdata.HistogramDataPoint[N]{
129-
c.hPt(fltrAlice, 1, 1, y2kPlus(11), y2kPlus(16)),
130-
c.hPt(fltrBob, 1, 1, y2kPlus(11), y2kPlus(16)),
131-
c.hPt(overflowSet, 1, 2, y2kPlus(11), y2kPlus(16)),
129+
c.hPt(fltrAlice, 1, 1, y2kPlus(4), y2kPlus(5)),
130+
c.hPt(fltrBob, 1, 1, y2kPlus(4), y2kPlus(5)),
131+
c.hPt(overflowSet, 1, 2, y2kPlus(4), y2kPlus(5)),
132132
},
133133
},
134134
},
@@ -167,8 +167,8 @@ func testCumulativeHist[N int64 | float64](c conf[N]) func(t *testing.T) {
167167
agg: metricdata.Histogram[N]{
168168
Temporality: metricdata.CumulativeTemporality,
169169
DataPoints: []metricdata.HistogramDataPoint[N]{
170-
c.hPt(fltrAlice, 2, 3, y2kPlus(0), y2kPlus(7)),
171-
c.hPt(fltrBob, 10, 2, y2kPlus(0), y2kPlus(7)),
170+
c.hPt(fltrAlice, 2, 3, y2kPlus(0), y2kPlus(2)),
171+
c.hPt(fltrBob, 10, 2, y2kPlus(0), y2kPlus(2)),
172172
},
173173
},
174174
},
@@ -183,8 +183,8 @@ func testCumulativeHist[N int64 | float64](c conf[N]) func(t *testing.T) {
183183
agg: metricdata.Histogram[N]{
184184
Temporality: metricdata.CumulativeTemporality,
185185
DataPoints: []metricdata.HistogramDataPoint[N]{
186-
c.hPt(fltrAlice, 2, 4, y2kPlus(0), y2kPlus(10)),
187-
c.hPt(fltrBob, 10, 3, y2kPlus(0), y2kPlus(10)),
186+
c.hPt(fltrAlice, 2, 4, y2kPlus(0), y2kPlus(3)),
187+
c.hPt(fltrBob, 10, 3, y2kPlus(0), y2kPlus(3)),
188188
},
189189
},
190190
},
@@ -196,8 +196,8 @@ func testCumulativeHist[N int64 | float64](c conf[N]) func(t *testing.T) {
196196
agg: metricdata.Histogram[N]{
197197
Temporality: metricdata.CumulativeTemporality,
198198
DataPoints: []metricdata.HistogramDataPoint[N]{
199-
c.hPt(fltrAlice, 2, 4, y2kPlus(0), y2kPlus(11)),
200-
c.hPt(fltrBob, 10, 3, y2kPlus(0), y2kPlus(11)),
199+
c.hPt(fltrAlice, 2, 4, y2kPlus(0), y2kPlus(4)),
200+
c.hPt(fltrBob, 10, 3, y2kPlus(0), y2kPlus(4)),
201201
},
202202
},
203203
},
@@ -213,9 +213,9 @@ func testCumulativeHist[N int64 | float64](c conf[N]) func(t *testing.T) {
213213
agg: metricdata.Histogram[N]{
214214
Temporality: metricdata.CumulativeTemporality,
215215
DataPoints: []metricdata.HistogramDataPoint[N]{
216-
c.hPt(fltrAlice, 2, 4, y2kPlus(0), y2kPlus(14)),
217-
c.hPt(fltrBob, 10, 3, y2kPlus(0), y2kPlus(14)),
218-
c.hPt(overflowSet, 1, 2, y2kPlus(0), y2kPlus(14)),
216+
c.hPt(fltrAlice, 2, 4, y2kPlus(0), y2kPlus(5)),
217+
c.hPt(fltrBob, 10, 3, y2kPlus(0), y2kPlus(5)),
218+
c.hPt(overflowSet, 1, 2, y2kPlus(0), y2kPlus(5)),
219219
},
220220
},
221221
},

0 commit comments

Comments
 (0)