From 593a45b291bf6b11a3c307be8fd581b5e61b2ccd Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 16 Aug 2023 14:44:53 -0700 Subject: [PATCH 01/24] Add internal/exemplar pkg --- sdk/metric/internal/exemplar/drop.go | 38 +++++ sdk/metric/internal/exemplar/filter.go | 66 +++++++++ sdk/metric/internal/exemplar/fixed.go | 169 ++++++++++++++++++++++ sdk/metric/internal/exemplar/hist.go | 47 ++++++ sdk/metric/internal/exemplar/rand.go | 66 +++++++++ sdk/metric/internal/exemplar/reservoir.go | 45 ++++++ 6 files changed, 431 insertions(+) create mode 100644 sdk/metric/internal/exemplar/drop.go create mode 100644 sdk/metric/internal/exemplar/filter.go create mode 100644 sdk/metric/internal/exemplar/fixed.go create mode 100644 sdk/metric/internal/exemplar/hist.go create mode 100644 sdk/metric/internal/exemplar/rand.go create mode 100644 sdk/metric/internal/exemplar/reservoir.go diff --git a/sdk/metric/internal/exemplar/drop.go b/sdk/metric/internal/exemplar/drop.go new file mode 100644 index 00000000000..8f2054336b5 --- /dev/null +++ b/sdk/metric/internal/exemplar/drop.go @@ -0,0 +1,38 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package exemplar // import "go.opentelemetry.io/otel/sdk/metric/internal/exemplar" + +import ( + "context" + "time" + + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/sdk/metric/metricdata" +) + +// Drop returns a [Reservoir] that drops all measurements it is offered. +func Drop[N int64 | float64]() Reservoir[N] { return &dropRes[N]{} } + +type dropRes[N int64 | float64] struct{} + +func (r *dropRes[N]) Offer(context.Context, time.Time, N, attribute.Set) {} + +func (r *dropRes[N]) Collect(dest *[]metricdata.Exemplar[N], _ attribute.Set) { + *dest = (*dest)[:0] +} + +func (r *dropRes[N]) Flush(dest *[]metricdata.Exemplar[N], _ attribute.Set) { + *dest = (*dest)[:0] +} diff --git a/sdk/metric/internal/exemplar/filter.go b/sdk/metric/internal/exemplar/filter.go new file mode 100644 index 00000000000..ecc13f73057 --- /dev/null +++ b/sdk/metric/internal/exemplar/filter.go @@ -0,0 +1,66 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package exemplar // import "go.opentelemetry.io/otel/sdk/metric/internal/exemplar" + +import ( + "context" + "time" + + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/trace" +) + +// Filter determines filters measurements passed to a [Reservoir]. If true is +// return, the measurement will be considered for sampling. +// +// See [Filtered] for how to create a [Reservoir] that uses a Filter. +type Filter[N int64 | float64] func(context.Context, N, attribute.Set) bool + +// AlwaysSample is a Filter that always signals measurements should be +// considered for sampling by a [Reservoir]. +func AlwaysSample[N int64 | float64](context.Context, N, attribute.Set) bool { + return true +} + +// NeverSample is a Filter that always signals measurements should not be +// considered for sampling by a [Reservoir]. +func NeverSample[N int64 | float64](context.Context, N, attribute.Set) bool { + return false +} + +// TraceBasedSample is a Filter that signals measurements should be considered +// for sampling by a [Reservoir] if the ctx contains a +// [go.opentelemetry.io/otel/trace.SpanContext] that is sampled. +func TraceBasedSample[N int64 | float64](ctx context.Context, _ N, _ attribute.Set) bool { + return trace.SpanContextFromContext(ctx).IsSampled() +} + +// Filtered returns a [Reservoir] wrapping r that will only offer measurements +// to r if f returns true. +func Filtered[N int64 | float64](r Reservoir[N], f Filter[N]) Reservoir[N] { + return filtered[N]{Reservoir: r, Filter: f} +} + +type filtered[N int64 | float64] struct { + Reservoir[N] + + Filter Filter[N] +} + +func (f filtered[N]) Offer(ctx context.Context, t time.Time, n N, a attribute.Set) { + if f.Filter(ctx, n, a) { + f.Reservoir.Offer(ctx, t, n, a) + } +} diff --git a/sdk/metric/internal/exemplar/fixed.go b/sdk/metric/internal/exemplar/fixed.go new file mode 100644 index 00000000000..d7e760d4267 --- /dev/null +++ b/sdk/metric/internal/exemplar/fixed.go @@ -0,0 +1,169 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package exemplar // import "go.opentelemetry.io/otel/sdk/metric/internal/exemplar" + +import ( + "context" + "time" + + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/internal/global" + "go.opentelemetry.io/otel/sdk/metric/metricdata" + "go.opentelemetry.io/otel/trace" +) + +type fixedRes[N int64 | float64] struct { + // store are the measurements sampled. + // + // This does not use []metricdata.Exemplar because it potentially would + // require an allocation for trace and span IDs in the hot path of Offer. + store []measurement[N] +} + +func newFixedRes[N int64 | float64](n int) *fixedRes[N] { + return &fixedRes[N]{store: make([]measurement[N], n)} +} + +func (r *fixedRes[N]) Collect(dest *[]metricdata.Exemplar[N], attrs attribute.Set) { + *dest = reset(*dest, len(r.store), len(r.store)) + var n int + for _, m := range r.store { + if m.Empty() { + continue + } + + m.Exemplar(&(*dest)[n], attrs) + n++ + } + *dest = (*dest)[:n] +} + +func (r *fixedRes[N]) Flush(dest *[]metricdata.Exemplar[N], attrs attribute.Set) { + *dest = reset(*dest, len(r.store), len(r.store)) + var n int + for i, m := range r.store { + if m.Empty() { + continue + } + + m.Exemplar(&(*dest)[n], attrs) + n++ + + // Reset. + r.store[i] = measurement[N]{} + } + *dest = (*dest)[:n] +} + +// measurement is a measurement made by a telemetry system. +type measurement[N int64 | float64] struct { + Attributes attribute.Set + Time time.Time + Value N + SpanContext trace.SpanContext + + valid bool +} + +// newMeasurement returns a new non-empty Measurement. +func newMeasurement[N int64 | float64](ctx context.Context, ts time.Time, v N, measuredAttr attribute.Set) measurement[N] { + return measurement[N]{ + Attributes: measuredAttr, + Time: ts, + Value: v, + SpanContext: trace.SpanContextFromContext(ctx), + valid: true, + } +} + +// Empty returns false if m represents a measurement made by a telemetry +// system, otherwise it returns true when m is its zero-value. +func (m measurement[N]) Empty() bool { return !m.valid } + +// Exemplar returns m as a [metricdata.Exemplar]. +func (m measurement[N]) Exemplar(dest *metricdata.Exemplar[N], recorded attribute.Set) { + // Note: A more optimal solution would be to store the filtered attributes + // when the exemplar is recorded, instead of re-calculating here. That + // approach isn't implemented though because it contrary to the OTel + // specification definition of a Reservoir, which is defined to accept the + // complete set of measured attributes. + dropped(&dest.FilteredAttributes, m.Attributes, recorded) + dest.Time = m.Time + dest.Value = m.Value + + if m.SpanContext.HasTraceID() { + traceID := m.SpanContext.TraceID() + dest.TraceID = traceID[:] + } else { + dest.TraceID = dest.TraceID[:0] + } + + if m.SpanContext.HasSpanID() { + spanID := m.SpanContext.SpanID() + dest.SpanID = spanID[:] + } else { + dest.SpanID = dest.SpanID[:0] + } +} + +// dropped returns the attribute that were measured, but not included in the +// recorded attributes. +func dropped(dest *[]attribute.KeyValue, measured, recorded attribute.Set) { + measN := measured.Len() + recN := recorded.Len() + + n := measN - recN + switch { + case n < 0: + // recorded should only ever be the filtered set of measured. Abandon + // instead of panicking. + global.Warn( + "invalid measured attributes for exemplar, dropping", + "measured", measured, + "recorded", recorded, + ) + fallthrough + case n == 0: + // Nothing dropped. + *dest = (*dest)[:0] + return + } + *dest = reset(*dest, n, n) + + measIter := measured.Iter() + recIter := recorded.Iter() + + var i int + recIter.Next() + for measIter.Next() { + m := measIter.Attribute() + r := recIter.Attribute() + + if m == r { + recIter.Next() + continue + } + + (*dest)[i] = m + i++ + } +} + +func reset[T any](s []T, length, capacity int) []T { + if cap(s) < capacity { + return make([]T, length, capacity) + } + return s[:length] +} diff --git a/sdk/metric/internal/exemplar/hist.go b/sdk/metric/internal/exemplar/hist.go new file mode 100644 index 00000000000..a0ec197301a --- /dev/null +++ b/sdk/metric/internal/exemplar/hist.go @@ -0,0 +1,47 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package exemplar // import "go.opentelemetry.io/otel/sdk/metric/internal/exemplar" + +import ( + "context" + "sort" + "time" + + "go.opentelemetry.io/otel/attribute" +) + +// Histogram returns a [Reservoir] that samples the last measurement that falls +// within a histogram bucket. The histogram bucket upper-boundaries are define +// by bounds. +// +// The passed bounds will be sorted by this function. +func Histogram[N int64 | float64](bounds []float64) Reservoir[N] { + sort.Float64s(bounds) + return &histRes[N]{ + bounds: bounds, + fixedRes: newFixedRes[N](len(bounds) + 1), + } +} + +type histRes[N int64 | float64] struct { + *fixedRes[N] + + // bounds are bucket bounds in ascending order. + bounds []float64 +} + +func (r *histRes[N]) Offer(ctx context.Context, t time.Time, n N, a attribute.Set) { + r.store[sort.SearchFloat64s(r.bounds, float64(n))] = newMeasurement(ctx, t, n, a) +} diff --git a/sdk/metric/internal/exemplar/rand.go b/sdk/metric/internal/exemplar/rand.go new file mode 100644 index 00000000000..e8e7091df53 --- /dev/null +++ b/sdk/metric/internal/exemplar/rand.go @@ -0,0 +1,66 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package exemplar // import "go.opentelemetry.io/otel/sdk/metric/internal/exemplar" + +import ( + "context" + "math/rand" + "time" + + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/sdk/metric/metricdata" +) + +var rng = rand.New(rand.NewSource(time.Now().UnixNano())) + +// FixedSize returns a [Reservoir] that samples at most n exemplars. If there +// are n or less number of measurements made, the Reservoir will sample each +// one. If there are more than n number of measurements made, the Reservoir +// will then randomly sample all additional measurement with a decreasing +// probability. +func FixedSize[N int64 | float64](n int) Reservoir[N] { + return &randRes[N]{fixedRes: newFixedRes[N](n)} +} + +type randRes[N int64 | float64] struct { + *fixedRes[N] + + // count is the number of measurement seen. + count int64 +} + +func (r *randRes[N]) Offer(ctx context.Context, t time.Time, n N, a attribute.Set) { + // TODO: fix overflow error. + r.count++ + if int(r.count) <= cap(r.store) { + r.store[r.count-1] = newMeasurement(ctx, t, n, a) + return + } + + j := int(rng.Int63n(r.count)) + if j < cap(r.store) { + r.store[j] = newMeasurement(ctx, t, n, a) + } +} + +func (r *randRes[N]) Collect(dest *[]metricdata.Exemplar[N], attrs attribute.Set) { + r.fixedRes.Collect(dest, attrs) + r.count = 0 +} + +func (r *randRes[N]) Flush(dest *[]metricdata.Exemplar[N], attrs attribute.Set) { + r.fixedRes.Flush(dest, attrs) + r.count = 0 +} diff --git a/sdk/metric/internal/exemplar/reservoir.go b/sdk/metric/internal/exemplar/reservoir.go new file mode 100644 index 00000000000..8d9d3bb978f --- /dev/null +++ b/sdk/metric/internal/exemplar/reservoir.go @@ -0,0 +1,45 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package exemplar // import "go.opentelemetry.io/otel/sdk/metric/internal/exemplar" + +import ( + "context" + "time" + + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/sdk/metric/metricdata" +) + +// Reservoir holds the sampled exemplar of measurements made. +type Reservoir[N int64 | float64] interface { + // Offer accepts the parameters associated with a measurement. The + // parameters will be stored as an exemplar if the Reservoir decides to + // sample the measurement. + Offer(context.Context, time.Time, N, attribute.Set) + + // Collect returns all the held exemplars with each exemplars dropped + // attributes updated to include any attributes the Filter filters out. + // + // The Reservoir state is preserved after this call. See Flush to + // copy-and-clear instead. + Collect(dest *[]metricdata.Exemplar[N], attrs attribute.Set) + + // Flush returns all the held exemplars with each exemplars dropped + // attributes updated to include any attributes the Filter filters out. + // + // The Reservoir state is reset after this call. See Collect to preserve + // the state instead. + Flush(dest *[]metricdata.Exemplar[N], attrs attribute.Set) +} From c37db5702f3db4e2399dccac8e0e2a76d76cf530 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 16 Aug 2023 14:45:19 -0700 Subject: [PATCH 02/24] Update internal/aggregate to support exemplars --- sdk/metric/internal/aggregate/aggregate.go | 35 ++++++-- .../internal/aggregate/aggregate_test.go | 16 ++-- .../aggregate/exponential_histogram.go | 83 +++++++++++------- .../aggregate/exponential_histogram_test.go | 12 +-- sdk/metric/internal/aggregate/histogram.go | 66 ++++++++------ .../internal/aggregate/histogram_test.go | 19 ++-- sdk/metric/internal/aggregate/lastvalue.go | 45 +++++++--- sdk/metric/internal/aggregate/sum.go | 86 +++++++++++++------ 8 files changed, 237 insertions(+), 125 deletions(-) diff --git a/sdk/metric/internal/aggregate/aggregate.go b/sdk/metric/internal/aggregate/aggregate.go index 8dec14237b9..31e81d5d6b1 100644 --- a/sdk/metric/internal/aggregate/aggregate.go +++ b/sdk/metric/internal/aggregate/aggregate.go @@ -19,6 +19,7 @@ import ( "time" "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/sdk/metric/internal/exemplar" "go.opentelemetry.io/otel/sdk/metric/metricdata" ) @@ -44,17 +45,35 @@ type Builder[N int64 | float64] struct { // Filter is the attribute filter the aggregate function will use on the // input of measurements. Filter attribute.Filter + // ReservoirFunc is the factory function used by aggregate functions to + // create new exemplar reservoirs for a new seen attribute set. + // + // If this is not provided a default factory function that returns an + // exemplar.Drop reservoir will be used. + ReservoirFunc func() exemplar.Reservoir[N] } -func (b Builder[N]) filter(f Measure[N]) Measure[N] { +func (b Builder[N]) resFunc() func() exemplar.Reservoir[N] { + if b.ReservoirFunc != nil { + return b.ReservoirFunc + } + + return exemplar.Drop[N] +} + +type fltrMeasure[N int64 | float64] func(ctx context.Context, value N, origAttr, fltrAttr attribute.Set) + +func (b Builder[N]) filter(f fltrMeasure[N]) Measure[N] { if b.Filter != nil { fltr := b.Filter // Copy to make it immutable after assignment. return func(ctx context.Context, n N, a attribute.Set) { fAttr, _ := a.Filter(fltr) - f(ctx, n, fAttr) + f(ctx, n, a, fAttr) } } - return f + return func(ctx context.Context, n N, a attribute.Set) { + f(ctx, n, a, a) + } } // LastValue returns a last-value aggregate function input and output. @@ -63,7 +82,7 @@ func (b Builder[N]) filter(f Measure[N]) Measure[N] { func (b Builder[N]) LastValue() (Measure[N], ComputeAggregation) { // Delta temporality is the only temporality that makes semantic sense for // a last-value aggregate. - lv := newLastValue[N]() + lv := newLastValue[N](b.resFunc()) return b.filter(lv.measure), func(dest *metricdata.Aggregation) int { // Ignore if dest is not a metricdata.Gauge. The chance for memory @@ -79,7 +98,7 @@ func (b Builder[N]) LastValue() (Measure[N], ComputeAggregation) { // PrecomputedSum returns a sum aggregate function input and output. The // arguments passed to the input are expected to be the precomputed sum values. func (b Builder[N]) PrecomputedSum(monotonic bool) (Measure[N], ComputeAggregation) { - s := newPrecomputedSum[N](monotonic) + s := newPrecomputedSum[N](monotonic, b.resFunc()) switch b.Temporality { case metricdata.DeltaTemporality: return b.filter(s.measure), s.delta @@ -90,7 +109,7 @@ func (b Builder[N]) PrecomputedSum(monotonic bool) (Measure[N], ComputeAggregati // Sum returns a sum aggregate function input and output. func (b Builder[N]) Sum(monotonic bool) (Measure[N], ComputeAggregation) { - s := newSum[N](monotonic) + s := newSum[N](monotonic, b.resFunc()) switch b.Temporality { case metricdata.DeltaTemporality: return b.filter(s.measure), s.delta @@ -102,7 +121,7 @@ func (b Builder[N]) Sum(monotonic bool) (Measure[N], ComputeAggregation) { // ExplicitBucketHistogram returns a histogram aggregate function input and // output. func (b Builder[N]) ExplicitBucketHistogram(boundaries []float64, noMinMax, noSum bool) (Measure[N], ComputeAggregation) { - h := newHistogram[N](boundaries, noMinMax, noSum) + h := newHistogram[N](boundaries, noMinMax, noSum, b.resFunc()) switch b.Temporality { case metricdata.DeltaTemporality: return b.filter(h.measure), h.delta @@ -114,7 +133,7 @@ func (b Builder[N]) ExplicitBucketHistogram(boundaries []float64, noMinMax, noSu // ExponentialBucketHistogram returns a histogram aggregate function input and // output. func (b Builder[N]) ExponentialBucketHistogram(maxSize, maxScale int32, noMinMax, noSum bool) (Measure[N], ComputeAggregation) { - h := newExponentialHistogram[N](maxSize, maxScale, noMinMax, noSum) + h := newExponentialHistogram[N](maxSize, maxScale, noMinMax, noSum, b.resFunc()) switch b.Temporality { case metricdata.DeltaTemporality: return b.filter(h.measure), h.delta diff --git a/sdk/metric/internal/aggregate/aggregate_test.go b/sdk/metric/internal/aggregate/aggregate_test.go index 79031b40218..71153c4babe 100644 --- a/sdk/metric/internal/aggregate/aggregate_test.go +++ b/sdk/metric/internal/aggregate/aggregate_test.go @@ -23,6 +23,7 @@ import ( "github.com/stretchr/testify/assert" "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/sdk/metric/internal/exemplar" "go.opentelemetry.io/otel/sdk/metric/metricdata" "go.opentelemetry.io/otel/sdk/metric/metricdata/metricdatatest" ) @@ -55,6 +56,10 @@ var ( } ) +func dropExemplars[N int64 | float64]() exemplar.Reservoir[N] { + return exemplar.Drop[N]() +} + func TestBuilderFilter(t *testing.T) { t.Run("Int64", testBuilderFilter[int64]()) t.Run("Float64", testBuilderFilter[float64]()) @@ -65,20 +70,21 @@ func testBuilderFilter[N int64 | float64]() func(t *testing.T) { t.Helper() value, attr := N(1), alice - run := func(b Builder[N], wantA attribute.Set) func(*testing.T) { + run := func(b Builder[N], wantO, wantF attribute.Set) func(*testing.T) { return func(t *testing.T) { t.Helper() - meas := b.filter(func(_ context.Context, v N, a attribute.Set) { + meas := b.filter(func(_ context.Context, v N, o, f attribute.Set) { assert.Equal(t, value, v, "measured incorrect value") - assert.Equal(t, wantA, a, "measured incorrect attributes") + assert.Equal(t, wantO, o, "measured incorrect original attributes") + assert.Equal(t, wantF, f, "measured incorrect filtered attributes") }) meas(context.Background(), value, attr) } } - t.Run("NoFilter", run(Builder[N]{}, attr)) - t.Run("Filter", run(Builder[N]{Filter: attrFltr}, fltrAlice)) + t.Run("NoFilter", run(Builder[N]{}, attr, attr)) + t.Run("Filter", run(Builder[N]{Filter: attrFltr}, alice, fltrAlice)) } } diff --git a/sdk/metric/internal/aggregate/exponential_histogram.go b/sdk/metric/internal/aggregate/exponential_histogram.go index 368f0027ec3..c1ccdd2d8c5 100644 --- a/sdk/metric/internal/aggregate/exponential_histogram.go +++ b/sdk/metric/internal/aggregate/exponential_histogram.go @@ -23,6 +23,7 @@ import ( "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/sdk/metric/internal/exemplar" "go.opentelemetry.io/otel/sdk/metric/metricdata" ) @@ -40,6 +41,9 @@ const ( // expoHistogramDataPoint is a single data point in an exponential histogram. type expoHistogramDataPoint[N int64 | float64] struct { + attr attribute.Set + res exemplar.Reservoir[N] + count uint64 min N max N @@ -288,14 +292,15 @@ func (b *expoBuckets) downscale(delta int) { // newExponentialHistogram returns an Aggregator that summarizes a set of // measurements as an exponential histogram. Each histogram is scoped by attributes // and the aggregation cycle the measurements were made in. -func newExponentialHistogram[N int64 | float64](maxSize, maxScale int32, noMinMax, noSum bool) *expoHistogram[N] { +func newExponentialHistogram[N int64 | float64](maxSize, maxScale int32, noMinMax, noSum bool, r func() exemplar.Reservoir[N]) *expoHistogram[N] { return &expoHistogram[N]{ noSum: noSum, noMinMax: noMinMax, maxSize: int(maxSize), maxScale: int(maxScale), - values: make(map[attribute.Set]*expoHistogramDataPoint[N]), + newRes: r, + values: make(map[attribute.Distinct]*expoHistogramDataPoint[N]), start: now(), } @@ -309,27 +314,35 @@ type expoHistogram[N int64 | float64] struct { maxSize int maxScale int - values map[attribute.Set]*expoHistogramDataPoint[N] + newRes func() exemplar.Reservoir[N] + values map[attribute.Distinct]*expoHistogramDataPoint[N] valuesMu sync.Mutex start time.Time } -func (e *expoHistogram[N]) measure(_ context.Context, value N, attr attribute.Set) { +func (e *expoHistogram[N]) measure(ctx context.Context, value N, origAttr, fltrAttr attribute.Set) { // Ignore NaN and infinity. if math.IsInf(float64(value), 0) || math.IsNaN(float64(value)) { return } + t := now() + key := fltrAttr.Equivalent() + e.valuesMu.Lock() defer e.valuesMu.Unlock() - v, ok := e.values[attr] + v, ok := e.values[key] if !ok { v = newExpoHistogramDataPoint[N](e.maxSize, e.maxScale, e.noMinMax, e.noSum) - e.values[attr] = v + v.attr = fltrAttr + v.res = e.newRes() + + e.values[key] = v } v.record(value) + v.res.Offer(ctx, t, value, origAttr) } func (e *expoHistogram[N]) delta(dest *metricdata.Aggregation) int { @@ -347,31 +360,33 @@ func (e *expoHistogram[N]) delta(dest *metricdata.Aggregation) int { hDPts := reset(h.DataPoints, n, n) var i int - for a, b := range e.values { - hDPts[i].Attributes = a + for key, val := range e.values { + hDPts[i].Attributes = val.attr hDPts[i].StartTime = e.start hDPts[i].Time = t - hDPts[i].Count = b.count - hDPts[i].Scale = int32(b.scale) - hDPts[i].ZeroCount = b.zeroCount + hDPts[i].Count = val.count + hDPts[i].Scale = int32(val.scale) + hDPts[i].ZeroCount = val.zeroCount hDPts[i].ZeroThreshold = 0.0 - hDPts[i].PositiveBucket.Offset = int32(b.posBuckets.startBin) - hDPts[i].PositiveBucket.Counts = reset(hDPts[i].PositiveBucket.Counts, len(b.posBuckets.counts), len(b.posBuckets.counts)) - copy(hDPts[i].PositiveBucket.Counts, b.posBuckets.counts) + hDPts[i].PositiveBucket.Offset = int32(val.posBuckets.startBin) + hDPts[i].PositiveBucket.Counts = reset(hDPts[i].PositiveBucket.Counts, len(val.posBuckets.counts), len(val.posBuckets.counts)) + copy(hDPts[i].PositiveBucket.Counts, val.posBuckets.counts) - 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)) + hDPts[i].NegativeBucket.Offset = int32(val.negBuckets.startBin) + hDPts[i].NegativeBucket.Counts = reset(hDPts[i].NegativeBucket.Counts, len(val.negBuckets.counts), len(val.negBuckets.counts)) if !e.noSum { - hDPts[i].Sum = b.sum + hDPts[i].Sum = val.sum } if !e.noMinMax { - hDPts[i].Min = metricdata.NewExtrema(b.min) - hDPts[i].Max = metricdata.NewExtrema(b.max) + hDPts[i].Min = metricdata.NewExtrema(val.min) + hDPts[i].Max = metricdata.NewExtrema(val.max) } - delete(e.values, a) + val.res.Flush(&hDPts[i].Exemplars, val.attr) + + delete(e.values, key) i++ } e.start = t @@ -395,30 +410,32 @@ func (e *expoHistogram[N]) cumulative(dest *metricdata.Aggregation) int { hDPts := reset(h.DataPoints, n, n) var i int - for a, b := range e.values { - hDPts[i].Attributes = a + for _, val := range e.values { + hDPts[i].Attributes = val.attr hDPts[i].StartTime = e.start hDPts[i].Time = t - hDPts[i].Count = b.count - hDPts[i].Scale = int32(b.scale) - hDPts[i].ZeroCount = b.zeroCount + hDPts[i].Count = val.count + hDPts[i].Scale = int32(val.scale) + hDPts[i].ZeroCount = val.zeroCount hDPts[i].ZeroThreshold = 0.0 - hDPts[i].PositiveBucket.Offset = int32(b.posBuckets.startBin) - hDPts[i].PositiveBucket.Counts = reset(hDPts[i].PositiveBucket.Counts, len(b.posBuckets.counts), len(b.posBuckets.counts)) - copy(hDPts[i].PositiveBucket.Counts, b.posBuckets.counts) + hDPts[i].PositiveBucket.Offset = int32(val.posBuckets.startBin) + hDPts[i].PositiveBucket.Counts = reset(hDPts[i].PositiveBucket.Counts, len(val.posBuckets.counts), len(val.posBuckets.counts)) + copy(hDPts[i].PositiveBucket.Counts, val.posBuckets.counts) - 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)) + hDPts[i].NegativeBucket.Offset = int32(val.negBuckets.startBin) + hDPts[i].NegativeBucket.Counts = reset(hDPts[i].NegativeBucket.Counts, len(val.negBuckets.counts), len(val.negBuckets.counts)) if !e.noSum { - hDPts[i].Sum = b.sum + hDPts[i].Sum = val.sum } if !e.noMinMax { - hDPts[i].Min = metricdata.NewExtrema(b.min) - hDPts[i].Max = metricdata.NewExtrema(b.max) + hDPts[i].Min = metricdata.NewExtrema(val.min) + hDPts[i].Max = metricdata.NewExtrema(val.max) } + val.res.Collect(&hDPts[i].Exemplars, val.attr) + i++ // TODO (#3006): This will use an unbounded amount of memory if there // are unbounded number of attribute sets being aggregated. Attribute diff --git a/sdk/metric/internal/aggregate/exponential_histogram_test.go b/sdk/metric/internal/aggregate/exponential_histogram_test.go index cac734c9312..21eb38ebf03 100644 --- a/sdk/metric/internal/aggregate/exponential_histogram_test.go +++ b/sdk/metric/internal/aggregate/exponential_histogram_test.go @@ -188,11 +188,11 @@ func testExpoHistogramMinMaxSumInt64(t *testing.T) { restore := withHandler(t) defer restore() - h := newExponentialHistogram[int64](4, 20, false, false) + h := newExponentialHistogram[int64](4, 20, false, false, dropExemplars[int64]) for _, v := range tt.values { - h.measure(context.Background(), v, alice) + h.measure(context.Background(), v, alice, alice) } - dp := h.values[alice] + dp := h.values[alice.Equivalent()] assert.Equal(t, tt.expected.max, dp.max) assert.Equal(t, tt.expected.min, dp.min) @@ -230,11 +230,11 @@ func testExpoHistogramMinMaxSumFloat64(t *testing.T) { restore := withHandler(t) defer restore() - h := newExponentialHistogram[float64](4, 20, false, false) + h := newExponentialHistogram[float64](4, 20, false, false, dropExemplars[float64]) for _, v := range tt.values { - h.measure(context.Background(), v, alice) + h.measure(context.Background(), v, alice, alice) } - dp := h.values[alice] + dp := h.values[alice.Equivalent()] assert.Equal(t, tt.expected.max, dp.max) assert.Equal(t, tt.expected.min, dp.min) diff --git a/sdk/metric/internal/aggregate/histogram.go b/sdk/metric/internal/aggregate/histogram.go index 62ec51e1f5e..631d1111291 100644 --- a/sdk/metric/internal/aggregate/histogram.go +++ b/sdk/metric/internal/aggregate/histogram.go @@ -21,10 +21,14 @@ import ( "time" "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/sdk/metric/internal/exemplar" "go.opentelemetry.io/otel/sdk/metric/metricdata" ) type buckets[N int64 | float64] struct { + attr attribute.Set + res exemplar.Reservoir[N] + counts []uint64 count uint64 total N @@ -54,11 +58,12 @@ type histValues[N int64 | float64] struct { noSum bool bounds []float64 - values map[attribute.Set]*buckets[N] + newRes func() exemplar.Reservoir[N] + values map[attribute.Distinct]*buckets[N] valuesMu sync.Mutex } -func newHistValues[N int64 | float64](bounds []float64, noSum bool) *histValues[N] { +func newHistValues[N int64 | float64](bounds []float64, noSum bool, r func() exemplar.Reservoir[N]) *histValues[N] { // The responsibility of keeping all buckets correctly associated with the // 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 @@ -69,13 +74,14 @@ func newHistValues[N int64 | float64](bounds []float64, noSum bool) *histValues[ return &histValues[N]{ noSum: noSum, bounds: b, - values: make(map[attribute.Set]*buckets[N]), + newRes: r, + values: make(map[attribute.Distinct]*buckets[N]), } } // Aggregate records the measurement value, scoped by attr, and aggregates it // into a histogram. -func (s *histValues[N]) measure(_ context.Context, value N, attr attribute.Set) { +func (s *histValues[N]) measure(ctx context.Context, value N, origAttr, fltrAttr attribute.Set) { // This search will return an index in the range [0, len(s.bounds)], where // it will return len(s.bounds) if value is greater than the last element // of s.bounds. This aligns with the buckets in that the length of buckets @@ -83,10 +89,13 @@ func (s *histValues[N]) measure(_ context.Context, value N, attr attribute.Set) // (s.bounds[len(s.bounds)-1], +∞). idx := sort.SearchFloat64s(s.bounds, float64(value)) + t := now() + key := fltrAttr.Equivalent() + s.valuesMu.Lock() defer s.valuesMu.Unlock() - b, ok := s.values[attr] + b, ok := s.values[key] if !ok { // N+1 buckets. For example: // @@ -96,21 +105,25 @@ func (s *histValues[N]) measure(_ context.Context, value N, attr attribute.Set) // // buckets = (-∞, 0], (0, 5.0], (5.0, 10.0], (10.0, +∞) b = newBuckets[N](len(s.bounds) + 1) + b.attr = fltrAttr + b.res = s.newRes() + // Ensure min and max are recorded values (not zero), for new buckets. b.min, b.max = value, value - s.values[attr] = b + s.values[key] = b } b.bin(idx, value) if !s.noSum { b.sum(value) } + b.res.Offer(ctx, t, value, origAttr) } // newHistogram returns an Aggregator that summarizes a set of measurements as // an histogram. -func newHistogram[N int64 | float64](boundaries []float64, noMinMax, noSum bool) *histogram[N] { +func newHistogram[N int64 | float64](boundaries []float64, noMinMax, noSum bool, r func() exemplar.Reservoir[N]) *histogram[N] { return &histogram[N]{ - histValues: newHistValues[N](boundaries, noSum), + histValues: newHistValues[N](boundaries, noSum, r), noMinMax: noMinMax, start: now(), } @@ -144,25 +157,27 @@ func (s *histogram[N]) delta(dest *metricdata.Aggregation) int { hDPts := reset(h.DataPoints, n, n) var i int - for a, b := range s.values { - hDPts[i].Attributes = a + for key, val := range s.values { + hDPts[i].Attributes = val.attr hDPts[i].StartTime = s.start hDPts[i].Time = t - hDPts[i].Count = b.count + hDPts[i].Count = val.count hDPts[i].Bounds = bounds - hDPts[i].BucketCounts = b.counts + hDPts[i].BucketCounts = val.counts if !s.noSum { - hDPts[i].Sum = b.total + hDPts[i].Sum = val.total } if !s.noMinMax { - hDPts[i].Min = metricdata.NewExtrema(b.min) - hDPts[i].Max = metricdata.NewExtrema(b.max) + hDPts[i].Min = metricdata.NewExtrema(val.min) + hDPts[i].Max = metricdata.NewExtrema(val.max) } + val.res.Flush(&hDPts[i].Exemplars, val.attr) + // Unused attribute sets do not report. - delete(s.values, a) + delete(s.values, key) i++ } // The delta collection cycle resets. @@ -193,30 +208,33 @@ func (s *histogram[N]) cumulative(dest *metricdata.Aggregation) int { hDPts := reset(h.DataPoints, n, n) var i int - for a, b := range s.values { + for _, val := 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) + counts := make([]uint64, len(val.counts)) + copy(counts, val.counts) - hDPts[i].Attributes = a + hDPts[i].Attributes = val.attr hDPts[i].StartTime = s.start hDPts[i].Time = t - hDPts[i].Count = b.count + hDPts[i].Count = val.count hDPts[i].Bounds = bounds hDPts[i].BucketCounts = counts if !s.noSum { - hDPts[i].Sum = b.total + hDPts[i].Sum = val.total } if !s.noMinMax { - hDPts[i].Min = metricdata.NewExtrema(b.min) - hDPts[i].Max = metricdata.NewExtrema(b.max) + hDPts[i].Min = metricdata.NewExtrema(val.min) + hDPts[i].Max = metricdata.NewExtrema(val.max) } + + val.res.Collect(&hDPts[i].Exemplars, val.attr) + i++ // TODO (#3006): This will use an unbounded amount of memory if there // are unbounded number of attribute sets being aggregated. Attribute diff --git a/sdk/metric/internal/aggregate/histogram_test.go b/sdk/metric/internal/aggregate/histogram_test.go index ab44607e5f6..45d901f63bc 100644 --- a/sdk/metric/internal/aggregate/histogram_test.go +++ b/sdk/metric/internal/aggregate/histogram_test.go @@ -273,13 +273,13 @@ func TestHistogramImmutableBounds(t *testing.T) { cpB := make([]float64, len(b)) copy(cpB, b) - h := newHistogram[int64](b, false, false) + h := newHistogram[int64](b, false, false, dropExemplars[int64]) require.Equal(t, cpB, h.bounds) b[0] = 10 assert.Equal(t, cpB, h.bounds, "modifying the bounds argument should not change the bounds") - h.measure(context.Background(), 5, alice) + h.measure(context.Background(), 5, alice, alice) var data metricdata.Aggregation = metricdata.Histogram[int64]{} h.cumulative(&data) @@ -289,31 +289,32 @@ func TestHistogramImmutableBounds(t *testing.T) { } func TestCumulativeHistogramImutableCounts(t *testing.T) { - h := newHistogram[int64](bounds, noMinMax, false) - h.measure(context.Background(), 5, alice) + h := newHistogram[int64](bounds, noMinMax, false, dropExemplars[int64]) + h.measure(context.Background(), 5, alice, alice) var data metricdata.Aggregation = metricdata.Histogram[int64]{} h.cumulative(&data) hdp := data.(metricdata.Histogram[int64]).DataPoints[0] - require.Equal(t, hdp.BucketCounts, h.values[alice].counts) + key := alice.Equivalent() + require.Equal(t, hdp.BucketCounts, h.values[key].counts) cpCounts := make([]uint64, len(hdp.BucketCounts)) copy(cpCounts, hdp.BucketCounts) hdp.BucketCounts[0] = 10 - assert.Equal(t, cpCounts, h.values[alice].counts, "modifying the Aggregator bucket counts should not change the Aggregator") + assert.Equal(t, cpCounts, h.values[key].counts, "modifying the Aggregator bucket counts should not change the Aggregator") } func TestDeltaHistogramReset(t *testing.T) { t.Cleanup(mockTime(now)) - h := newHistogram[int64](bounds, noMinMax, false) + h := newHistogram[int64](bounds, noMinMax, false, dropExemplars[int64]) var data metricdata.Aggregation = metricdata.Histogram[int64]{} require.Equal(t, 0, h.delta(&data)) require.Len(t, data.(metricdata.Histogram[int64]).DataPoints, 0) - h.measure(context.Background(), 1, alice) + h.measure(context.Background(), 1, alice, alice) expect := metricdata.Histogram[int64]{Temporality: metricdata.DeltaTemporality} expect.DataPoints = []metricdata.HistogramDataPoint[int64]{hPointSummed[int64](alice, 1, 1)} @@ -326,7 +327,7 @@ func TestDeltaHistogramReset(t *testing.T) { assert.Len(t, data.(metricdata.Histogram[int64]).DataPoints, 0) // Aggregating another set should not affect the original (alice). - h.measure(context.Background(), 1, bob) + h.measure(context.Background(), 1, bob, bob) expect.DataPoints = []metricdata.HistogramDataPoint[int64]{hPointSummed[int64](bob, 1, 1)} h.delta(&data) metricdatatest.AssertAggregationsEqual(t, expect, data) diff --git a/sdk/metric/internal/aggregate/lastvalue.go b/sdk/metric/internal/aggregate/lastvalue.go index 6af2d606141..90d2a611ba6 100644 --- a/sdk/metric/internal/aggregate/lastvalue.go +++ b/sdk/metric/internal/aggregate/lastvalue.go @@ -20,31 +20,51 @@ import ( "time" "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/sdk/metric/internal/exemplar" "go.opentelemetry.io/otel/sdk/metric/metricdata" ) // datapoint is timestamped measurement data. type datapoint[N int64 | float64] struct { + attr attribute.Set timestamp time.Time value N + res exemplar.Reservoir[N] } -func newLastValue[N int64 | float64]() *lastValue[N] { - return &lastValue[N]{values: make(map[attribute.Set]datapoint[N])} +func newLastValue[N int64 | float64](r func() exemplar.Reservoir[N]) *lastValue[N] { + return &lastValue[N]{ + values: make(map[attribute.Distinct]datapoint[N]), + newRes: r, + } } // lastValue summarizes a set of measurements as the last one made. type lastValue[N int64 | float64] struct { sync.Mutex - values map[attribute.Set]datapoint[N] + values map[attribute.Distinct]datapoint[N] + newRes func() exemplar.Reservoir[N] } -func (s *lastValue[N]) measure(ctx context.Context, value N, attr attribute.Set) { - d := datapoint[N]{timestamp: now(), value: value} +func (s *lastValue[N]) measure(ctx context.Context, value N, origAttr, fltrAttr attribute.Set) { + t := now() + key := fltrAttr.Equivalent() + s.Lock() - s.values[attr] = d - s.Unlock() + defer s.Unlock() + + d, ok := s.values[key] + if !ok { + d.attr = fltrAttr + d.res = s.newRes() + } + + d.timestamp = t + d.value = value + d.res.Offer(ctx, t, value, origAttr) + + s.values[key] = d } func (s *lastValue[N]) computeAggregation(dest *[]metricdata.DataPoint[N]) { @@ -55,14 +75,15 @@ func (s *lastValue[N]) computeAggregation(dest *[]metricdata.DataPoint[N]) { *dest = reset(*dest, n, n) var i int - for a, v := range s.values { - (*dest)[i].Attributes = a + for key, val := range s.values { + (*dest)[i].Attributes = val.attr // The event time is the only meaningful timestamp, StartTime is // ignored. - (*dest)[i].Time = v.timestamp - (*dest)[i].Value = v.value + (*dest)[i].Time = val.timestamp + (*dest)[i].Value = val.value + val.res.Flush(&(*dest)[i].Exemplars, val.attr) // Do not report stale values. - delete(s.values, a) + delete(s.values, key) i++ } } diff --git a/sdk/metric/internal/aggregate/sum.go b/sdk/metric/internal/aggregate/sum.go index 1e52ff0d1e5..0c9321310c6 100644 --- a/sdk/metric/internal/aggregate/sum.go +++ b/sdk/metric/internal/aggregate/sum.go @@ -20,31 +20,57 @@ import ( "time" "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/sdk/metric/internal/exemplar" "go.opentelemetry.io/otel/sdk/metric/metricdata" ) // valueMap is the storage for sums. type valueMap[N int64 | float64] struct { sync.Mutex - values map[attribute.Set]N + values map[attribute.Distinct]struct { + attr attribute.Set + n N + res exemplar.Reservoir[N] + } + newRes func() exemplar.Reservoir[N] } -func newValueMap[N int64 | float64]() *valueMap[N] { - return &valueMap[N]{values: make(map[attribute.Set]N)} +func newValueMap[N int64 | float64](r func() exemplar.Reservoir[N]) *valueMap[N] { + return &valueMap[N]{ + values: make(map[attribute.Distinct]struct { + attr attribute.Set + n N + res exemplar.Reservoir[N] + }), + newRes: r, + } } -func (s *valueMap[N]) measure(_ context.Context, value N, attr attribute.Set) { +func (s *valueMap[N]) measure(ctx context.Context, value N, origAttr, fltrAttr attribute.Set) { + t := now() + key := fltrAttr.Equivalent() + s.Lock() - s.values[attr] += value - s.Unlock() + defer s.Unlock() + + v, ok := s.values[key] + if !ok { + v.attr = fltrAttr + v.res = s.newRes() + } + + v.n += value + v.res.Offer(ctx, t, value, origAttr) + + s.values[key] = v } // newSum returns an aggregator that summarizes a set of measurements as their // arithmetic sum. Each sum is scoped by attributes and the aggregation cycle // the measurements were made in. -func newSum[N int64 | float64](monotonic bool) *sum[N] { +func newSum[N int64 | float64](monotonic bool, r func() exemplar.Reservoir[N]) *sum[N] { return &sum[N]{ - valueMap: newValueMap[N](), + valueMap: newValueMap[N](r), monotonic: monotonic, start: now(), } @@ -74,13 +100,14 @@ func (s *sum[N]) delta(dest *metricdata.Aggregation) int { dPts := reset(sData.DataPoints, n, n) var i int - for attr, value := range s.values { - dPts[i].Attributes = attr + for key, val := range s.values { + dPts[i].Attributes = val.attr dPts[i].StartTime = s.start dPts[i].Time = t - dPts[i].Value = value + dPts[i].Value = val.n + val.res.Flush(&dPts[i].Exemplars, val.attr) // Do not report stale values. - delete(s.values, attr) + delete(s.values, key) i++ } // The delta collection cycle resets. @@ -108,11 +135,12 @@ func (s *sum[N]) cumulative(dest *metricdata.Aggregation) int { dPts := reset(sData.DataPoints, n, n) var i int - for attr, value := range s.values { - dPts[i].Attributes = attr + for _, val := range s.values { + dPts[i].Attributes = val.attr dPts[i].StartTime = s.start dPts[i].Time = t - dPts[i].Value = value + dPts[i].Value = val.n + val.res.Collect(&dPts[i].Exemplars, val.attr) // TODO (#3006): This will use an unbounded amount of memory if there // are unbounded number of attribute sets being aggregated. Attribute // sets that become "stale" need to be forgotten so this will not @@ -129,9 +157,9 @@ func (s *sum[N]) cumulative(dest *metricdata.Aggregation) int { // newPrecomputedSum returns an aggregator that summarizes a set of // observatrions as their arithmetic sum. Each sum is scoped by attributes and // the aggregation cycle the measurements were made in. -func newPrecomputedSum[N int64 | float64](monotonic bool) *precomputedSum[N] { +func newPrecomputedSum[N int64 | float64](monotonic bool, r func() exemplar.Reservoir[N]) *precomputedSum[N] { return &precomputedSum[N]{ - valueMap: newValueMap[N](), + valueMap: newValueMap[N](r), monotonic: monotonic, start: now(), } @@ -144,12 +172,12 @@ type precomputedSum[N int64 | float64] struct { monotonic bool start time.Time - reported map[attribute.Set]N + reported map[attribute.Distinct]N } func (s *precomputedSum[N]) delta(dest *metricdata.Aggregation) int { t := now() - newReported := make(map[attribute.Set]N) + newReported := make(map[attribute.Distinct]N) // If *dest is not a metricdata.Sum, memory reuse is missed. In that case, // use the zero-value sData and hope for better alignment next cycle. @@ -164,17 +192,18 @@ func (s *precomputedSum[N]) delta(dest *metricdata.Aggregation) int { dPts := reset(sData.DataPoints, n, n) var i int - for attr, value := range s.values { - delta := value - s.reported[attr] + for key, val := range s.values { + delta := val.n - s.reported[key] - dPts[i].Attributes = attr + dPts[i].Attributes = val.attr dPts[i].StartTime = s.start dPts[i].Time = t dPts[i].Value = delta + val.res.Flush(&dPts[i].Exemplars, val.attr) - newReported[attr] = value + newReported[key] = val.n // Unused attribute sets do not report. - delete(s.values, attr) + delete(s.values, key) i++ } // Unused attribute sets are forgotten. @@ -204,14 +233,15 @@ func (s *precomputedSum[N]) cumulative(dest *metricdata.Aggregation) int { dPts := reset(sData.DataPoints, n, n) var i int - for attr, value := range s.values { - dPts[i].Attributes = attr + for key, val := range s.values { + dPts[i].Attributes = val.attr dPts[i].StartTime = s.start dPts[i].Time = t - dPts[i].Value = value + dPts[i].Value = val.n + val.res.Collect(&dPts[i].Exemplars, val.attr) // Unused attribute sets do not report. - delete(s.values, attr) + delete(s.values, key) i++ } From becb3a0b08cfa7dd109244376f2751cc6d27a12f Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 16 Aug 2023 14:46:06 -0700 Subject: [PATCH 03/24] Add experimental feature functionality --- sdk/metric/experimental.go | 55 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 sdk/metric/experimental.go diff --git a/sdk/metric/experimental.go b/sdk/metric/experimental.go new file mode 100644 index 00000000000..ab5a367f68b --- /dev/null +++ b/sdk/metric/experimental.go @@ -0,0 +1,55 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package metric // import "go.opentelemetry.io/otel/sdk/metric" + +import ( + "os" + "strings" +) + +const xEnvKeyRoot = "OTEL_GO_X_" + +var ( + xExemplar = xFeature{ + envKeySuffix: "EXEMPLAR", + enablementVals: []string{"true"}, + } +) + +type xFeature struct { + // envKey is the environment variable key suffix the xFeature is stored at. + // It is assumed xEnvKeyRoot is the base of the environment variable key. + envKeySuffix string + // enablementVals are the case-insensitive comparison values that indicate + // the xFeature is enabled. + enablementVals []string +} + +// xEnabled returns if the xFeature is enabled. +func xEnabled(xf xFeature) bool { + key := xEnvKeyRoot + xf.envKeySuffix + vRaw, present := os.LookupEnv(key) + if !present { + return false + } + + v := strings.ToLower(vRaw) + for _, allowed := range xf.enablementVals { + if v == strings.ToLower(allowed) { + return true + } + } + return false +} From 141afc01f2eccc56ac03febb1b2bc18d7f63b92f Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 16 Aug 2023 14:46:46 -0700 Subject: [PATCH 04/24] Update metric SDK to support exemplars --- sdk/metric/exemplar.go | 73 ++++++++++++++++++++++++++++++++++++++++++ sdk/metric/go.mod | 2 +- sdk/metric/pipeline.go | 3 +- 3 files changed, 76 insertions(+), 2 deletions(-) create mode 100644 sdk/metric/exemplar.go diff --git a/sdk/metric/exemplar.go b/sdk/metric/exemplar.go new file mode 100644 index 00000000000..2d585e2ff9c --- /dev/null +++ b/sdk/metric/exemplar.go @@ -0,0 +1,73 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package metric // import "go.opentelemetry.io/otel/sdk/metric" + +import ( + "os" + + "go.opentelemetry.io/otel/sdk/metric/internal/exemplar" +) + +// reservoirFunc returns the appropriately configured exemplar reservoir +// creation func based on the passed InstrumentKind and user defined +// environment variables. +// +// Note: This will only return non-nil values when the experimental exemplar +// feature is enabled. +func reservoirFunc[N int64 | float64](agg Aggregation) func() exemplar.Reservoir[N] { + if !xEnabled(xExemplar) { + return nil + } + + // https://github.com/open-telemetry/opentelemetry-specification/blob/d4b241f451674e8f611bb589477680341006ad2b/specification/configuration/sdk-environment-variables.md#exemplar + const filterEnvKey = "OTEL_METRICS_EXEMPLAR_FILTER" + + var fltr exemplar.Filter[N] + switch os.Getenv(filterEnvKey) { + case "always_on": + fltr = exemplar.AlwaysSample[N] + case "always_off": + fltr = exemplar.NeverSample[N] + case "trace_based": + fallthrough + default: + fltr = exemplar.TraceBasedSample[N] + } + + // TODO: This is not defined by the specification, nor is the mechanism to + // configure it. + const defaultFixedSize = 1 + + // https://github.com/open-telemetry/opentelemetry-specification/blob/d4b241f451674e8f611bb589477680341006ad2b/specification/metrics/sdk.md#exemplar-defaults + resF := func() func() exemplar.Reservoir[N] { + a, ok := agg.(AggregationExplicitBucketHistogram) + if ok && len(a.Boundaries) > 1 { + cp := make([]float64, len(a.Boundaries)) + copy(cp, a.Boundaries) + return func() exemplar.Reservoir[N] { + bounds := cp + return exemplar.Histogram[N](bounds) + } + } + + return func() exemplar.Reservoir[N] { + return exemplar.FixedSize[N](defaultFixedSize) + } + }() + + return func() exemplar.Reservoir[N] { + return exemplar.Filtered[N](resF(), fltr) + } +} diff --git a/sdk/metric/go.mod b/sdk/metric/go.mod index f7f92f4ecc2..be0497b4135 100644 --- a/sdk/metric/go.mod +++ b/sdk/metric/go.mod @@ -9,12 +9,12 @@ require ( go.opentelemetry.io/otel v1.16.0 go.opentelemetry.io/otel/metric v1.16.0 go.opentelemetry.io/otel/sdk v1.16.0 + go.opentelemetry.io/otel/trace v1.16.0 ) require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - go.opentelemetry.io/otel/trace v1.16.0 // indirect golang.org/x/sys v0.11.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/sdk/metric/pipeline.go b/sdk/metric/pipeline.go index d76231cff7f..55a99ef2508 100644 --- a/sdk/metric/pipeline.go +++ b/sdk/metric/pipeline.go @@ -349,7 +349,8 @@ func (i *inserter[N]) cachedAggregator(scope instrumentation.Scope, kind Instrum normID := id.normalize() cv := i.aggregators.Lookup(normID, func() aggVal[N] { b := aggregate.Builder[N]{ - Temporality: i.pipeline.reader.temporality(kind), + Temporality: i.pipeline.reader.temporality(kind), + ReservoirFunc: reservoirFunc[N](stream.Aggregation), } if len(stream.AllowAttributeKeys) > 0 { b.Filter = stream.attributeFilter() From 778c62e90b2d18636de38a9ae3d435aa8e7f6c81 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 17 Aug 2023 09:17:10 -0700 Subject: [PATCH 05/24] Add exemplar pkg tests --- sdk/metric/internal/exemplar/drop_test.go | 29 +++ sdk/metric/internal/exemplar/filter_test.go | 136 +++++++++++++ sdk/metric/internal/exemplar/fixed_test.go | 46 +++++ sdk/metric/internal/exemplar/hist_test.go | 28 +++ sdk/metric/internal/exemplar/rand_test.go | 27 +++ .../internal/exemplar/reservoir_test.go | 188 ++++++++++++++++++ 6 files changed, 454 insertions(+) create mode 100644 sdk/metric/internal/exemplar/drop_test.go create mode 100644 sdk/metric/internal/exemplar/filter_test.go create mode 100644 sdk/metric/internal/exemplar/fixed_test.go create mode 100644 sdk/metric/internal/exemplar/hist_test.go create mode 100644 sdk/metric/internal/exemplar/rand_test.go create mode 100644 sdk/metric/internal/exemplar/reservoir_test.go diff --git a/sdk/metric/internal/exemplar/drop_test.go b/sdk/metric/internal/exemplar/drop_test.go new file mode 100644 index 00000000000..dc874502723 --- /dev/null +++ b/sdk/metric/internal/exemplar/drop_test.go @@ -0,0 +1,29 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package exemplar + +import ( + "testing" +) + +func TestDrop(t *testing.T) { + t.Run("Int64", testReservoir[int64](func(int) (Reservoir[int64], int) { + return Drop[int64](), 0 + })) + + t.Run("Float64", testReservoir[float64](func(int) (Reservoir[float64], int) { + return Drop[float64](), 0 + })) +} diff --git a/sdk/metric/internal/exemplar/filter_test.go b/sdk/metric/internal/exemplar/filter_test.go new file mode 100644 index 00000000000..c7a32b8dfa6 --- /dev/null +++ b/sdk/metric/internal/exemplar/filter_test.go @@ -0,0 +1,136 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package exemplar // import "go.opentelemetry.io/otel/sdk/metric/internal/exemplar" + +import ( + "context" + "math" + "testing" + "time" + + "github.com/stretchr/testify/assert" + + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/sdk/metric/metricdata" + "go.opentelemetry.io/otel/trace" +) + +func sample(parent context.Context) context.Context { + sc := trace.NewSpanContext(trace.SpanContextConfig{ + TraceID: trace.TraceID{0x01}, + SpanID: trace.SpanID{0x01}, + TraceFlags: trace.FlagsSampled, + }) + return trace.ContextWithSpanContext(parent, sc) +} + +func TestAlwaysSample(t *testing.T) { + t.Run("Int64", testAlwaysSample[int64]) + t.Run("Float64", testAlwaysSample[float64]) +} + +func testAlwaysSample[N int64 | float64](t *testing.T) { + ctx := context.Background() + + assert.True(t, AlwaysSample[N](ctx, 0, alice)) + assert.True(t, AlwaysSample[N](ctx, 0, fltrAlice)) + assert.True(t, AlwaysSample[N](sample(ctx), 0, alice)) + assert.True(t, AlwaysSample[N](ctx, math.MaxInt64, alice)) + assert.True(t, AlwaysSample[N](ctx, math.MinInt64, alice)) + assert.True(t, AlwaysSample[N](ctx, N(math.NaN()), alice)) + assert.True(t, AlwaysSample[N](ctx, N(math.Inf(-1)), alice)) + assert.True(t, AlwaysSample[N](ctx, N(math.Inf(1)), alice)) +} + +func TestNeverSample(t *testing.T) { + t.Run("Int64", testNeverSample[int64]) + t.Run("Float64", testNeverSample[float64]) +} + +func testNeverSample[N int64 | float64](t *testing.T) { + ctx := context.Background() + + assert.False(t, NeverSample[N](ctx, 0, alice)) + assert.False(t, NeverSample[N](ctx, 0, fltrAlice)) + assert.False(t, NeverSample[N](sample(ctx), 0, alice)) + assert.False(t, NeverSample[N](ctx, math.MaxInt64, alice)) + assert.False(t, NeverSample[N](ctx, math.MinInt64, alice)) + assert.False(t, NeverSample[N](ctx, N(math.NaN()), alice)) + assert.False(t, NeverSample[N](ctx, N(math.Inf(-1)), alice)) + assert.False(t, NeverSample[N](ctx, N(math.Inf(1)), alice)) +} + +func TestTraceBasedSample(t *testing.T) { + t.Run("Int64", testTraceBasedSample[int64]) + t.Run("Float64", testTraceBasedSample[float64]) +} + +func testTraceBasedSample[N int64 | float64](t *testing.T) { + ctx := context.Background() + + assert.False(t, TraceBasedSample[N](ctx, 0, alice)) + assert.False(t, TraceBasedSample[N](ctx, 0, fltrAlice)) + assert.True(t, TraceBasedSample[N](sample(ctx), 0, alice)) + assert.False(t, TraceBasedSample[N](ctx, math.MaxInt64, alice)) + assert.False(t, TraceBasedSample[N](ctx, math.MinInt64, alice)) + assert.False(t, TraceBasedSample[N](ctx, N(math.NaN()), alice)) + assert.False(t, TraceBasedSample[N](ctx, N(math.Inf(-1)), alice)) + assert.False(t, TraceBasedSample[N](ctx, N(math.Inf(1)), alice)) +} + +type res[N int64 | float64] struct { + OfferCalled bool + CollectCalled bool + FlushCalled bool +} + +func (r *res[N]) Offer(context.Context, time.Time, N, attribute.Set) { + r.OfferCalled = true +} + +func (r *res[N]) Collect(*[]metricdata.Exemplar[N], attribute.Set) { + r.CollectCalled = true +} + +func (r *res[N]) Flush(*[]metricdata.Exemplar[N], attribute.Set) { + r.FlushCalled = true +} + +func TestFilteredReservoir(t *testing.T) { + t.Run("Int64", testFilteredReservoir[int64]) + t.Run("Float64", testFilteredReservoir[float64]) +} + +func testFilteredReservoir[N int64 | float64](t *testing.T) { + under := &res[N]{} + + var called bool + fltr := func(context.Context, N, attribute.Set) bool { + called = true + return true + } + + r := Filtered[N](under, fltr) + + r.Offer(context.Background(), staticTime, 0, alice) + assert.True(t, called, "filter not called on Offer") + assert.True(t, under.OfferCalled, "underlying Reservoir Offer not called") + + r.Collect(nil, alice) + assert.True(t, under.CollectCalled, "underlying Reservoir Collect not called") + + r.Flush(nil, alice) + assert.True(t, under.FlushCalled, "underlying Reservoir Flush not called") +} diff --git a/sdk/metric/internal/exemplar/fixed_test.go b/sdk/metric/internal/exemplar/fixed_test.go new file mode 100644 index 00000000000..98a3dad8413 --- /dev/null +++ b/sdk/metric/internal/exemplar/fixed_test.go @@ -0,0 +1,46 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package exemplar // import "go.opentelemetry.io/otel/sdk/metric/internal/exemplar" + +import ( + "log" + "os" + "testing" + + "github.com/go-logr/logr" + "github.com/go-logr/logr/funcr" + "github.com/go-logr/stdr" + "github.com/stretchr/testify/assert" + + "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/attribute" +) + +func TestDroppedHandlesInvalidGracefully(t *testing.T) { + var msg string + t.Cleanup(func(orig logr.Logger) func() { + otel.SetLogger(funcr.New(func(_, args string) { + msg = args + }, funcr.Options{Verbosity: 20})) + return func() { otel.SetLogger(orig) } + }(stdr.New(log.New(os.Stderr, "", log.LstdFlags|log.Lshortfile)))) + + dest := []attribute.KeyValue{{}} + // measured < recorded is invalid. + dropped(&dest, fltrAlice, alice) + + assert.Contains(t, msg, "invalid measured attributes for exemplar, dropping") + assert.Len(t, dest, 0, "dest not reset") +} diff --git a/sdk/metric/internal/exemplar/hist_test.go b/sdk/metric/internal/exemplar/hist_test.go new file mode 100644 index 00000000000..bf6b766ad70 --- /dev/null +++ b/sdk/metric/internal/exemplar/hist_test.go @@ -0,0 +1,28 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package exemplar + +import "testing" + +func TestHist(t *testing.T) { + bounds := []float64{0, 100} + t.Run("Int64", testReservoir[int64](func(int) (Reservoir[int64], int) { + return Histogram[int64](bounds), len(bounds) + })) + + t.Run("Float64", testReservoir[float64](func(int) (Reservoir[float64], int) { + return Histogram[float64](bounds), len(bounds) + })) +} diff --git a/sdk/metric/internal/exemplar/rand_test.go b/sdk/metric/internal/exemplar/rand_test.go new file mode 100644 index 00000000000..5b3d9df9890 --- /dev/null +++ b/sdk/metric/internal/exemplar/rand_test.go @@ -0,0 +1,27 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package exemplar + +import "testing" + +func TestFixedSize(t *testing.T) { + t.Run("Int64", testReservoir[int64](func(n int) (Reservoir[int64], int) { + return FixedSize[int64](n), n + })) + + t.Run("Float64", testReservoir[float64](func(n int) (Reservoir[float64], int) { + return FixedSize[float64](n), n + })) +} diff --git a/sdk/metric/internal/exemplar/reservoir_test.go b/sdk/metric/internal/exemplar/reservoir_test.go new file mode 100644 index 00000000000..1f3f200d15a --- /dev/null +++ b/sdk/metric/internal/exemplar/reservoir_test.go @@ -0,0 +1,188 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package exemplar + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/sdk/metric/metricdata" + "go.opentelemetry.io/otel/trace" +) + +var ( + keyUser = "user" + userAlice = attribute.String(keyUser, "Alice") + adminTrue = attribute.Bool("admin", true) + + alice = attribute.NewSet(userAlice, adminTrue) + fltrAlice = attribute.NewSet(userAlice) + + // Sat Jan 01 2000 00:00:00 GMT+0000. + staticTime = time.Unix(946684800, 0) +) + +type factory[N int64 | float64] func(requstedCap int) (r Reservoir[N], actualCap int) + +func testReservoir[N int64 | float64](f factory[N]) func(*testing.T) { + return func(t *testing.T) { + t.Helper() + + ctx := context.Background() + + t.Run("CaptureSpanContext", func(t *testing.T) { + t.Helper() + + r, n := f(1) + if n < 1 { + t.Skip("skipping, reservoir capacity less than 1:", n) + } + + tID, sID := trace.TraceID{0x01}, trace.SpanID{0x01} + sc := trace.NewSpanContext(trace.SpanContextConfig{ + TraceID: tID, + SpanID: sID, + TraceFlags: trace.FlagsSampled, + }) + ctx := trace.ContextWithSpanContext(ctx, sc) + + r.Offer(ctx, staticTime, 10, alice) + + var dest []metricdata.Exemplar[N] + r.Collect(&dest, alice) + + want := metricdata.Exemplar[N]{ + Time: staticTime, + Value: 10, + SpanID: []byte(sID[:]), + TraceID: []byte(tID[:]), + } + require.Len(t, dest, 1, "number of collected exemplars") + assert.Equal(t, want, dest[0]) + }) + + t.Run("FilterAttributes", func(t *testing.T) { + t.Helper() + + r, n := f(1) + if n < 1 { + t.Skip("skipping, reservoir capacity less than 1:", n) + } + + r.Offer(ctx, staticTime, 10, alice) + + var dest []metricdata.Exemplar[N] + r.Collect(&dest, fltrAlice) + + want := metricdata.Exemplar[N]{ + FilteredAttributes: []attribute.KeyValue{adminTrue}, + Time: staticTime, + Value: 10, + } + require.Len(t, dest, 1, "number of collected exemplars") + assert.Equal(t, want, dest[0]) + }) + + t.Run("CollectDoesNotFlush", func(t *testing.T) { + t.Helper() + + r, n := f(1) + if n < 1 { + t.Skip("skipping, reservoir capacity less than 1:", n) + } + + r.Offer(ctx, staticTime, 10, alice) + + var dest []metricdata.Exemplar[N] + r.Collect(&dest, alice) + require.Len(t, dest, 1, "number of collected exemplars") + + dest = dest[:0] + r.Collect(&dest, alice) + assert.Len(t, dest, 1, "Collect flushed reservoir") + }) + + t.Run("FlushFlushes", func(t *testing.T) { + t.Helper() + + r, n := f(1) + if n < 1 { + t.Skip("skipping, reservoir capacity less than 1:", n) + } + + r.Offer(ctx, staticTime, 10, alice) + + var dest []metricdata.Exemplar[N] + r.Flush(&dest, alice) + require.Len(t, dest, 1, "number of flushed exemplars") + + r.Flush(&dest, alice) + assert.Len(t, dest, 0, "Flush did not flush reservoir") + }) + + t.Run("MultipleOffers", func(t *testing.T) { + t.Helper() + + r, n := f(3) + if n < 1 { + t.Skip("skipping, reservoir capacity less than 1:", n) + } + + for i := 0; i < n+1; i++ { + v := N(i) + r.Offer(ctx, staticTime, v, alice) + } + + var dest []metricdata.Exemplar[N] + r.Flush(&dest, alice) + assert.Len(t, dest, n, "multiple offers did not fill reservoir") + + // Ensure the flush reset also resets any couting state. + for i := 0; i < n+1; i++ { + v := N(2 * i) + r.Offer(ctx, staticTime, v, alice) + } + + dest = dest[:0] + r.Flush(&dest, alice) + assert.Len(t, dest, n, "internal count state not reset") + }) + + t.Run("DropAll", func(t *testing.T) { + t.Helper() + + r, n := f(0) + if n > 0 { + t.Skip("skipping, reservoir capacity greater than 0:", n) + } + + r.Offer(context.Background(), staticTime, 10, alice) + + dest := []metricdata.Exemplar[N]{{}} // Should be reset to empty. + r.Collect(&dest, alice) + assert.Len(t, dest, 0, "no exemplars should be collected") + + r.Offer(context.Background(), staticTime, 10, alice) + dest = []metricdata.Exemplar[N]{{}} // Should be reset to empty. + r.Flush(&dest, alice) + assert.Len(t, dest, 0, "no exemplars should be flushed") + }) + } +} From 99f6b14157347a333c369201ec8810bb86ed4583 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 18 Aug 2023 08:20:57 -0700 Subject: [PATCH 06/24] Use Algorithm L for rand res --- sdk/metric/internal/exemplar/rand.go | 58 ++++++++++++++++++----- sdk/metric/internal/exemplar/rand_test.go | 34 ++++++++++++- 2 files changed, 79 insertions(+), 13 deletions(-) diff --git a/sdk/metric/internal/exemplar/rand.go b/sdk/metric/internal/exemplar/rand.go index e8e7091df53..b2ad2c4fe38 100644 --- a/sdk/metric/internal/exemplar/rand.go +++ b/sdk/metric/internal/exemplar/rand.go @@ -16,6 +16,7 @@ package exemplar // import "go.opentelemetry.io/otel/sdk/metric/internal/exempla import ( "context" + "math" "math/rand" "time" @@ -31,7 +32,9 @@ var rng = rand.New(rand.NewSource(time.Now().UnixNano())) // will then randomly sample all additional measurement with a decreasing // probability. func FixedSize[N int64 | float64](n int) Reservoir[N] { - return &randRes[N]{fixedRes: newFixedRes[N](n)} + r := &randRes[N]{fixedRes: newFixedRes[N](n)} + r.reset() + return r } type randRes[N int64 | float64] struct { @@ -39,28 +42,59 @@ type randRes[N int64 | float64] struct { // count is the number of measurement seen. count int64 + // next is the next count that will store a measurement at a randon index + // once the reservoir has been filled. + next int64 + // w is the largest random number in a distribution that is used to compute + // the next next. + w float64 } func (r *randRes[N]) Offer(ctx context.Context, t time.Time, n N, a attribute.Set) { - // TODO: fix overflow error. - r.count++ - if int(r.count) <= cap(r.store) { - r.store[r.count-1] = newMeasurement(ctx, t, n, a) - return - } + // 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): + // 481–493 (https://dl.acm.org/doi/10.1145/198429.198435). + // + // It is used because of its balance of simplicity and performance. In + // particular it has an asymptotic runtime of O(k(1 + log(n/k)) where n is + // the number of measurements offered and k is the reservoir size. This is + // much more optimal for large measurement sets than the algorithm + // recommended by the OTel spcification ("Algorithm R" as described in + // Vitter, Jeffrey S. (1 March 1985). "Random sampling with a reservoir" + // (http://www.cs.umd.edu/~samir/498/vitter.pdf)) which has an asymptotic + // runtime of O(n). - j := int(rng.Int63n(r.count)) - if j < cap(r.store) { - r.store[j] = newMeasurement(ctx, t, n, a) + if int(r.count) < cap(r.store) { + r.store[r.count] = newMeasurement(ctx, t, n, a) + } else { + if r.count == r.next { + idx := int(rng.Int63n(int64(cap(r.store)))) + r.store[idx] = newMeasurement(ctx, t, n, a) + r.advance() + } } + r.count++ +} + +func (r *randRes[N]) reset() { + r.count = 0 + r.next = int64(cap(r.store)) + r.w = math.Exp(math.Log(rng.Float64()) / float64(cap(r.store))) + r.advance() +} + +func (r *randRes[N]) advance() { + r.w *= math.Exp(math.Log(rng.Float64()) / float64(cap(r.store))) + r.next += int64(math.Log(rng.Float64())/math.Log(1-r.w)) + 1 } func (r *randRes[N]) Collect(dest *[]metricdata.Exemplar[N], attrs attribute.Set) { r.fixedRes.Collect(dest, attrs) - r.count = 0 + r.reset() } func (r *randRes[N]) Flush(dest *[]metricdata.Exemplar[N], attrs attribute.Set) { r.fixedRes.Flush(dest, attrs) - r.count = 0 + r.reset() } diff --git a/sdk/metric/internal/exemplar/rand_test.go b/sdk/metric/internal/exemplar/rand_test.go index 5b3d9df9890..87a0d68ae13 100644 --- a/sdk/metric/internal/exemplar/rand_test.go +++ b/sdk/metric/internal/exemplar/rand_test.go @@ -14,7 +14,14 @@ package exemplar -import "testing" +import ( + "context" + "math" + "sort" + "testing" + + "github.com/stretchr/testify/assert" +) func TestFixedSize(t *testing.T) { t.Run("Int64", testReservoir[int64](func(n int) (Reservoir[int64], int) { @@ -25,3 +32,28 @@ func TestFixedSize(t *testing.T) { return FixedSize[float64](n), n })) } + +func TestFixedSizeSamplingCorrectness(t *testing.T) { + intensity := 0.1 + sampleSize := 1000 + + data := make([]float64, sampleSize*1000) + for i := range data { + data[i] = (-1.0 / intensity) * math.Log(rng.Float64()) + } + // Sort to avoid position bias. + sort.Float64s(data) + + r := FixedSize[float64](sampleSize) + for _, value := range data { + r.Offer(context.Background(), staticTime, value, alice) + } + + var sum float64 + for _, m := range r.(*randRes[float64]).store { + sum += m.Value + } + mean := sum / float64(sampleSize) + + assert.InDelta(t, 1/mean, intensity, 0.01) +} From 0486576cdcbe6e5a727c793ef685926735175a61 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 18 Aug 2023 08:55:31 -0700 Subject: [PATCH 07/24] Move experimental support to internal pkg --- sdk/metric/exemplar.go | 3 +- .../{experimental.go => internal/x/x.go} | 34 ++++++++++--------- 2 files changed, 20 insertions(+), 17 deletions(-) rename sdk/metric/{experimental.go => internal/x/x.go} (52%) diff --git a/sdk/metric/exemplar.go b/sdk/metric/exemplar.go index 2d585e2ff9c..8bbe9e44392 100644 --- a/sdk/metric/exemplar.go +++ b/sdk/metric/exemplar.go @@ -18,6 +18,7 @@ import ( "os" "go.opentelemetry.io/otel/sdk/metric/internal/exemplar" + "go.opentelemetry.io/otel/sdk/metric/internal/x" ) // reservoirFunc returns the appropriately configured exemplar reservoir @@ -27,7 +28,7 @@ import ( // Note: This will only return non-nil values when the experimental exemplar // feature is enabled. func reservoirFunc[N int64 | float64](agg Aggregation) func() exemplar.Reservoir[N] { - if !xEnabled(xExemplar) { + if !x.Enabled(x.Exemplars) { return nil } diff --git a/sdk/metric/experimental.go b/sdk/metric/internal/x/x.go similarity index 52% rename from sdk/metric/experimental.go rename to sdk/metric/internal/x/x.go index ab5a367f68b..50b6efa61ec 100644 --- a/sdk/metric/experimental.go +++ b/sdk/metric/internal/x/x.go @@ -12,41 +12,43 @@ // See the License for the specific language governing permissions and // limitations under the License. -package metric // import "go.opentelemetry.io/otel/sdk/metric" +// Package x contains support for OTel metric SDK experimental features. +package x // import "go.opentelemetry.io/otel/sdk/metric/internal/x" import ( "os" "strings" ) -const xEnvKeyRoot = "OTEL_GO_X_" +const EnvKeyRoot = "OTEL_GO_X_" var ( - xExemplar = xFeature{ - envKeySuffix: "EXEMPLAR", - enablementVals: []string{"true"}, + Exemplars = Feature{ + EnvKeySuffix: "EXEMPLAR", + EnablementVals: []string{"true"}, } ) -type xFeature struct { - // envKey is the environment variable key suffix the xFeature is stored at. - // It is assumed xEnvKeyRoot is the base of the environment variable key. - envKeySuffix string - // enablementVals are the case-insensitive comparison values that indicate - // the xFeature is enabled. - enablementVals []string +type Feature struct { + // EnvKeySuffix is the environment variable key suffix the xFeature is + // stored at. It is assumed EnvKeyRoot is the base of the environment + // variable key. + EnvKeySuffix string + // EnablementVals are the case-insensitive comparison values that indicate + // the Feature is enabled. + EnablementVals []string } -// xEnabled returns if the xFeature is enabled. -func xEnabled(xf xFeature) bool { - key := xEnvKeyRoot + xf.envKeySuffix +// Enabled returns if the Feature is enabled. +func Enabled(f Feature) bool { + key := EnvKeyRoot + f.EnvKeySuffix vRaw, present := os.LookupEnv(key) if !present { return false } v := strings.ToLower(vRaw) - for _, allowed := range xf.enablementVals { + for _, allowed := range f.EnablementVals { if v == strings.ToLower(allowed) { return true } From 1d571d5f927aaeaf57caf326e5748e831c340488 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 15 Dec 2023 11:02:11 -0800 Subject: [PATCH 08/24] Update default fixed bucket exemplar sizes --- sdk/metric/exemplar.go | 33 ++++++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/sdk/metric/exemplar.go b/sdk/metric/exemplar.go index 4002f901088..865ed3e5012 100644 --- a/sdk/metric/exemplar.go +++ b/sdk/metric/exemplar.go @@ -16,6 +16,7 @@ package metric // import "go.opentelemetry.io/otel/sdk/metric" import ( "os" + "runtime" "go.opentelemetry.io/otel/sdk/metric/internal/exemplar" "go.opentelemetry.io/otel/sdk/metric/internal/x" @@ -47,12 +48,10 @@ func reservoirFunc[N int64 | float64](agg Aggregation) func() exemplar.Reservoir fltr = exemplar.TraceBasedSample[N] } - // TODO: This is not defined by the specification, nor is the mechanism to - // configure it. - const defaultFixedSize = 1 - // https://github.com/open-telemetry/opentelemetry-specification/blob/d4b241f451674e8f611bb589477680341006ad2b/specification/metrics/sdk.md#exemplar-defaults resF := func() func() exemplar.Reservoir[N] { + // Explicit bucket histogram aggregation with more than 1 bucket will + // use AlignedHistogramBucketExemplarReservoir. a, ok := agg.(AggregationExplicitBucketHistogram) if ok && len(a.Boundaries) > 1 { cp := make([]float64, len(a.Boundaries)) @@ -63,8 +62,32 @@ func reservoirFunc[N int64 | float64](agg Aggregation) func() exemplar.Reservoir } } + var n int + if a, ok := agg.(AggregationBase2ExponentialHistogram); ok { + // Base2 Exponential Histogram Aggregation SHOULD use a + // SimpleFixedSizeExemplarReservoir with a reservoir equal to the + // smaller of the maximum number of buckets configured on the + // aggregation or twenty (e.g. min(20, max_buckets)). + n = int(a.MaxSize) + if n > 20 { + n = 20 + } + } else { + // https://github.com/open-telemetry/opentelemetry-specification/blob/e94af89e3d0c01de30127a0f423e912f6cda7bed/specification/metrics/sdk.md#simplefixedsizeexemplarreservoir + // This Exemplar reservoir MAY take a configuration parameter for + // the size of the reservoir. If no size configuration is + // provided, the default size MAY be the number of possible + // concurrent threads (e.g. numer of CPUs) to help reduce + // contention. Otherwise, a default size of 1 SHOULD be used. + n = runtime.NumCPU() + if n < 1 { + // Should never be the case, but be defensive. + n = 1 + } + } + return func() exemplar.Reservoir[N] { - return exemplar.FixedSize[N](defaultFixedSize) + return exemplar.FixedSize[N](n) } }() From ebe1807ed4ab4376604d89fea98d393a30201eda Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 17 Jan 2024 10:45:58 -0800 Subject: [PATCH 09/24] Revert name changes unrelated to PR --- .../aggregate/exponential_histogram.go | 58 +++++++++---------- sdk/metric/internal/aggregate/histogram.go | 36 ++++++------ sdk/metric/internal/aggregate/lastvalue.go | 12 ++-- sdk/metric/internal/aggregate/sum.go | 14 ++--- 4 files changed, 60 insertions(+), 60 deletions(-) diff --git a/sdk/metric/internal/aggregate/exponential_histogram.go b/sdk/metric/internal/aggregate/exponential_histogram.go index 650943656ad..d2acf457089 100644 --- a/sdk/metric/internal/aggregate/exponential_histogram.go +++ b/sdk/metric/internal/aggregate/exponential_histogram.go @@ -360,33 +360,33 @@ func (e *expoHistogram[N]) delta(dest *metricdata.Aggregation) int { hDPts := reset(h.DataPoints, n, n) var i int - for attr, val := range e.values { - hDPts[i].Attributes = attr + for a, b := range e.values { + hDPts[i].Attributes = a hDPts[i].StartTime = e.start hDPts[i].Time = t - hDPts[i].Count = val.count - hDPts[i].Scale = int32(val.scale) - hDPts[i].ZeroCount = val.zeroCount + hDPts[i].Count = b.count + hDPts[i].Scale = int32(b.scale) + hDPts[i].ZeroCount = b.zeroCount hDPts[i].ZeroThreshold = 0.0 - hDPts[i].PositiveBucket.Offset = int32(val.posBuckets.startBin) - hDPts[i].PositiveBucket.Counts = reset(hDPts[i].PositiveBucket.Counts, len(val.posBuckets.counts), len(val.posBuckets.counts)) - copy(hDPts[i].PositiveBucket.Counts, val.posBuckets.counts) + hDPts[i].PositiveBucket.Offset = int32(b.posBuckets.startBin) + hDPts[i].PositiveBucket.Counts = reset(hDPts[i].PositiveBucket.Counts, len(b.posBuckets.counts), len(b.posBuckets.counts)) + copy(hDPts[i].PositiveBucket.Counts, b.posBuckets.counts) - hDPts[i].NegativeBucket.Offset = int32(val.negBuckets.startBin) - hDPts[i].NegativeBucket.Counts = reset(hDPts[i].NegativeBucket.Counts, len(val.negBuckets.counts), len(val.negBuckets.counts)) + 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)) if !e.noSum { - hDPts[i].Sum = val.sum + hDPts[i].Sum = b.sum } if !e.noMinMax { - hDPts[i].Min = metricdata.NewExtrema(val.min) - hDPts[i].Max = metricdata.NewExtrema(val.max) + hDPts[i].Min = metricdata.NewExtrema(b.min) + hDPts[i].Max = metricdata.NewExtrema(b.max) } - val.res.Flush(&hDPts[i].Exemplars, attr) + b.res.Flush(&hDPts[i].Exemplars, a) - delete(e.values, attr) + delete(e.values, a) i++ } e.start = t @@ -410,31 +410,31 @@ func (e *expoHistogram[N]) cumulative(dest *metricdata.Aggregation) int { hDPts := reset(h.DataPoints, n, n) var i int - for attr, val := range e.values { - hDPts[i].Attributes = attr + for a, b := range e.values { + hDPts[i].Attributes = a hDPts[i].StartTime = e.start hDPts[i].Time = t - hDPts[i].Count = val.count - hDPts[i].Scale = int32(val.scale) - hDPts[i].ZeroCount = val.zeroCount + hDPts[i].Count = b.count + hDPts[i].Scale = int32(b.scale) + hDPts[i].ZeroCount = b.zeroCount hDPts[i].ZeroThreshold = 0.0 - hDPts[i].PositiveBucket.Offset = int32(val.posBuckets.startBin) - hDPts[i].PositiveBucket.Counts = reset(hDPts[i].PositiveBucket.Counts, len(val.posBuckets.counts), len(val.posBuckets.counts)) - copy(hDPts[i].PositiveBucket.Counts, val.posBuckets.counts) + hDPts[i].PositiveBucket.Offset = int32(b.posBuckets.startBin) + hDPts[i].PositiveBucket.Counts = reset(hDPts[i].PositiveBucket.Counts, len(b.posBuckets.counts), len(b.posBuckets.counts)) + copy(hDPts[i].PositiveBucket.Counts, b.posBuckets.counts) - hDPts[i].NegativeBucket.Offset = int32(val.negBuckets.startBin) - hDPts[i].NegativeBucket.Counts = reset(hDPts[i].NegativeBucket.Counts, len(val.negBuckets.counts), len(val.negBuckets.counts)) + 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)) if !e.noSum { - hDPts[i].Sum = val.sum + hDPts[i].Sum = b.sum } if !e.noMinMax { - hDPts[i].Min = metricdata.NewExtrema(val.min) - hDPts[i].Max = metricdata.NewExtrema(val.max) + hDPts[i].Min = metricdata.NewExtrema(b.min) + hDPts[i].Max = metricdata.NewExtrema(b.max) } - val.res.Collect(&hDPts[i].Exemplars, attr) + b.res.Collect(&hDPts[i].Exemplars, a) i++ // TODO (#3006): This will use an unbounded amount of memory if there diff --git a/sdk/metric/internal/aggregate/histogram.go b/sdk/metric/internal/aggregate/histogram.go index 798d12c36bb..68199b43a8c 100644 --- a/sdk/metric/internal/aggregate/histogram.go +++ b/sdk/metric/internal/aggregate/histogram.go @@ -157,27 +157,27 @@ func (s *histogram[N]) delta(dest *metricdata.Aggregation) int { hDPts := reset(h.DataPoints, n, n) var i int - for attr, val := range s.values { - hDPts[i].Attributes = attr + for a, b := range s.values { + hDPts[i].Attributes = a hDPts[i].StartTime = s.start hDPts[i].Time = t - hDPts[i].Count = val.count + hDPts[i].Count = b.count hDPts[i].Bounds = bounds - hDPts[i].BucketCounts = val.counts + hDPts[i].BucketCounts = b.counts if !s.noSum { - hDPts[i].Sum = val.total + hDPts[i].Sum = b.total } if !s.noMinMax { - hDPts[i].Min = metricdata.NewExtrema(val.min) - hDPts[i].Max = metricdata.NewExtrema(val.max) + hDPts[i].Min = metricdata.NewExtrema(b.min) + hDPts[i].Max = metricdata.NewExtrema(b.max) } - val.res.Flush(&hDPts[i].Exemplars, attr) + b.res.Flush(&hDPts[i].Exemplars, a) // Unused attribute sets do not report. - delete(s.values, attr) + delete(s.values, a) i++ } // The delta collection cycle resets. @@ -208,32 +208,32 @@ func (s *histogram[N]) cumulative(dest *metricdata.Aggregation) int { hDPts := reset(h.DataPoints, n, n) var i int - for attr, val := range s.values { + 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(val.counts)) - copy(counts, val.counts) + counts := make([]uint64, len(b.counts)) + copy(counts, b.counts) - hDPts[i].Attributes = attr + hDPts[i].Attributes = a hDPts[i].StartTime = s.start hDPts[i].Time = t - hDPts[i].Count = val.count + hDPts[i].Count = b.count hDPts[i].Bounds = bounds hDPts[i].BucketCounts = counts if !s.noSum { - hDPts[i].Sum = val.total + hDPts[i].Sum = b.total } if !s.noMinMax { - hDPts[i].Min = metricdata.NewExtrema(val.min) - hDPts[i].Max = metricdata.NewExtrema(val.max) + hDPts[i].Min = metricdata.NewExtrema(b.min) + hDPts[i].Max = metricdata.NewExtrema(b.max) } - val.res.Collect(&hDPts[i].Exemplars, attr) + b.res.Collect(&hDPts[i].Exemplars, a) i++ // TODO (#3006): This will use an unbounded amount of memory if there diff --git a/sdk/metric/internal/aggregate/lastvalue.go b/sdk/metric/internal/aggregate/lastvalue.go index 35edef1bdcf..0a1d95d1867 100644 --- a/sdk/metric/internal/aggregate/lastvalue.go +++ b/sdk/metric/internal/aggregate/lastvalue.go @@ -75,15 +75,15 @@ func (s *lastValue[N]) computeAggregation(dest *[]metricdata.DataPoint[N]) { *dest = reset(*dest, n, n) var i int - for attr, val := range s.values { - (*dest)[i].Attributes = attr + for a, v := range s.values { + (*dest)[i].Attributes = a // The event time is the only meaningful timestamp, StartTime is // ignored. - (*dest)[i].Time = val.timestamp - (*dest)[i].Value = val.value - val.res.Flush(&(*dest)[i].Exemplars, attr) + (*dest)[i].Time = v.timestamp + (*dest)[i].Value = v.value + v.res.Flush(&(*dest)[i].Exemplars, a) // Do not report stale values. - delete(s.values, attr) + delete(s.values, a) i++ } } diff --git a/sdk/metric/internal/aggregate/sum.go b/sdk/metric/internal/aggregate/sum.go index 7e80b0d899a..ca8aba1df6f 100644 --- a/sdk/metric/internal/aggregate/sum.go +++ b/sdk/metric/internal/aggregate/sum.go @@ -133,12 +133,12 @@ func (s *sum[N]) cumulative(dest *metricdata.Aggregation) int { dPts := reset(sData.DataPoints, n, n) var i int - for attr, val := range s.values { + for attr, value := range s.values { dPts[i].Attributes = attr dPts[i].StartTime = s.start dPts[i].Time = t - dPts[i].Value = val.n - val.res.Collect(&dPts[i].Exemplars, attr) + dPts[i].Value = value.n + value.res.Collect(&dPts[i].Exemplars, attr) // TODO (#3006): This will use an unbounded amount of memory if there // are unbounded number of attribute sets being aggregated. Attribute // sets that become "stale" need to be forgotten so this will not @@ -190,16 +190,16 @@ func (s *precomputedSum[N]) delta(dest *metricdata.Aggregation) int { dPts := reset(sData.DataPoints, n, n) var i int - for attr, val := range s.values { - delta := val.n - s.reported[attr] + for attr, value := range s.values { + delta := value.n - s.reported[attr] dPts[i].Attributes = attr dPts[i].StartTime = s.start dPts[i].Time = t dPts[i].Value = delta - val.res.Flush(&dPts[i].Exemplars, attr) + value.res.Flush(&dPts[i].Exemplars, attr) - newReported[attr] = val.n + newReported[attr] = value.n // Unused attribute sets do not report. delete(s.values, attr) i++ From e694fb84b49c9757091d6e165b6bd1c57cd57292 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 17 Jan 2024 11:29:37 -0800 Subject: [PATCH 10/24] Document Offer params in Reservoir iface --- sdk/metric/internal/exemplar/reservoir.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/sdk/metric/internal/exemplar/reservoir.go b/sdk/metric/internal/exemplar/reservoir.go index 8d9d3bb978f..7fa5b1c1577 100644 --- a/sdk/metric/internal/exemplar/reservoir.go +++ b/sdk/metric/internal/exemplar/reservoir.go @@ -27,7 +27,15 @@ type Reservoir[N int64 | float64] interface { // Offer accepts the parameters associated with a measurement. The // parameters will be stored as an exemplar if the Reservoir decides to // sample the measurement. - Offer(context.Context, time.Time, N, attribute.Set) + // + // The passed ctx needs to contain any baggage or span that were active + // when the measurement was made. This information may be used by the + // Reservoir in making a sampling decision. + // + // The time t is the time when the measurement was made. The val and attrs + // parameters are the value and complete (unfiltered) attribute set of the + // measurement respectively. + Offer(ctx context.Context, t time.Time, val N, attrs attribute.Set) // Collect returns all the held exemplars with each exemplars dropped // attributes updated to include any attributes the Filter filters out. From 623ed1beca6dbedc1227c0abd9edfa871c8c7c4d Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 17 Jan 2024 12:40:24 -0800 Subject: [PATCH 11/24] Add end-to-end benchmark for rand and hist --- sdk/metric/benchmark_test.go | 89 ++++++++++++++++++++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/sdk/metric/benchmark_test.go b/sdk/metric/benchmark_test.go index 90f88088630..9c5b6dc8a8b 100644 --- a/sdk/metric/benchmark_test.go +++ b/sdk/metric/benchmark_test.go @@ -16,6 +16,8 @@ package metric // import "go.opentelemetry.io/otel/sdk/metric" import ( "context" + "fmt" + "runtime" "strconv" "testing" @@ -24,6 +26,7 @@ import ( "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/sdk/metric/metricdata" + "go.opentelemetry.io/otel/trace" ) var viewBenchmarks = []struct { @@ -369,3 +372,89 @@ func benchCollectAttrs(setup func(attribute.Set) Reader) func(*testing.B) { b.Run("Attributes/10", run(setup(attribute.NewSet(attrs...)))) } } + +func BenchmarkExemplars(b *testing.B) { + sc := trace.NewSpanContext(trace.SpanContextConfig{ + SpanID: trace.SpanID{01}, + TraceID: trace.TraceID{01}, + TraceFlags: trace.FlagsSampled, + }) + ctx := trace.ContextWithSpanContext(context.Background(), sc) + + attr := attribute.NewSet( + attribute.String("user", "Alice"), + attribute.Bool("admin", true), + ) + + setup := func(name string) (metric.Meter, Reader) { + r := NewManualReader() + v := NewView(Instrument{Name: "*"}, Stream{ + AttributeFilter: func(kv attribute.KeyValue) bool { + return kv.Key == attribute.Key("user") + }, + }) + mp := NewMeterProvider(WithReader(r), WithView(v)) + return mp.Meter(name), r + } + nCPU := runtime.NumCPU() + + b.Setenv("OTEL_GO_X_EXEMPLAR", "true") + + name := fmt.Sprintf("Int64Counter/%d", nCPU) + b.Run(name, func(b *testing.B) { + m, r := setup("Int64Counter") + i, err := m.Int64Counter("int64-counter") + assert.NoError(b, err) + + rm := newRM(metricdata.Sum[int64]{ + DataPoints: []metricdata.DataPoint[int64]{ + {Exemplars: make([]metricdata.Exemplar[int64], 0, nCPU)}, + }, + }) + e := &(rm.ScopeMetrics[0].Metrics[0].Data.(metricdata.Sum[int64]).DataPoints[0].Exemplars) + + b.ReportAllocs() + b.ResetTimer() + for n := 0; n < b.N; n++ { + for j := 0; j < 2*nCPU; j++ { + i.Add(ctx, 1, metric.WithAttributeSet(attr)) + } + + r.Collect(ctx, rm) + assert.Len(b, *e, nCPU) + } + }) + + name = fmt.Sprintf("Int64Histogram/%d", nCPU) + b.Run(name, func(b *testing.B) { + m, r := setup("Int64Counter") + i, err := m.Int64Histogram("int64-histogram") + assert.NoError(b, err) + + rm := newRM(metricdata.Histogram[int64]{ + DataPoints: []metricdata.HistogramDataPoint[int64]{ + {Exemplars: make([]metricdata.Exemplar[int64], 0, 1)}, + }, + }) + e := &(rm.ScopeMetrics[0].Metrics[0].Data.(metricdata.Histogram[int64]).DataPoints[0].Exemplars) + + b.ReportAllocs() + b.ResetTimer() + for n := 0; n < b.N; n++ { + for j := 0; j < 2*nCPU; j++ { + i.Record(ctx, 1, metric.WithAttributeSet(attr)) + } + + r.Collect(ctx, rm) + assert.Len(b, *e, 1) + } + }) +} + +func newRM(a metricdata.Aggregation) *metricdata.ResourceMetrics { + return &metricdata.ResourceMetrics{ + ScopeMetrics: []metricdata.ScopeMetrics{ + {Metrics: []metricdata.Metrics{{Data: a}}}, + }, + } +} From 6a94b22dc0e901e6f11941005b99000b0b8167f2 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 17 Jan 2024 14:00:38 -0800 Subject: [PATCH 12/24] Move to recording dropped at Offer --- sdk/metric/internal/aggregate/aggregate.go | 8 +-- .../internal/aggregate/aggregate_test.go | 10 +-- .../aggregate/exponential_histogram.go | 8 +-- .../aggregate/exponential_histogram_test.go | 4 +- sdk/metric/internal/aggregate/histogram.go | 8 +-- .../internal/aggregate/histogram_test.go | 8 +-- sdk/metric/internal/aggregate/lastvalue.go | 6 +- sdk/metric/internal/aggregate/sum.go | 12 ++-- sdk/metric/internal/exemplar/drop.go | 6 +- sdk/metric/internal/exemplar/filter.go | 12 ++-- sdk/metric/internal/exemplar/filter_test.go | 45 ++++--------- sdk/metric/internal/exemplar/fixed.go | 67 +++---------------- sdk/metric/internal/exemplar/fixed_test.go | 46 ------------- sdk/metric/internal/exemplar/hist.go | 2 +- sdk/metric/internal/exemplar/rand.go | 10 +-- sdk/metric/internal/exemplar/rand_test.go | 2 +- sdk/metric/internal/exemplar/reservoir.go | 10 +-- .../internal/exemplar/reservoir_test.go | 41 +++++------- 18 files changed, 93 insertions(+), 212 deletions(-) delete mode 100644 sdk/metric/internal/exemplar/fixed_test.go diff --git a/sdk/metric/internal/aggregate/aggregate.go b/sdk/metric/internal/aggregate/aggregate.go index 3077aa69c54..4060a2f76d3 100644 --- a/sdk/metric/internal/aggregate/aggregate.go +++ b/sdk/metric/internal/aggregate/aggregate.go @@ -69,18 +69,18 @@ func (b Builder[N]) resFunc() func() exemplar.Reservoir[N] { return exemplar.Drop[N] } -type fltrMeasure[N int64 | float64] func(ctx context.Context, value N, origAttr, fltrAttr attribute.Set) +type fltrMeasure[N int64 | float64] func(ctx context.Context, value N, fltrAttr attribute.Set, droppedAttr []attribute.KeyValue) func (b Builder[N]) filter(f fltrMeasure[N]) Measure[N] { if b.Filter != nil { fltr := b.Filter // Copy to make it immutable after assignment. return func(ctx context.Context, n N, a attribute.Set) { - fAttr, _ := a.Filter(fltr) - f(ctx, n, a, fAttr) + fAttr, dropped := a.Filter(fltr) + f(ctx, n, fAttr, dropped) } } return func(ctx context.Context, n N, a attribute.Set) { - f(ctx, n, a, a) + f(ctx, n, a, nil) } } diff --git a/sdk/metric/internal/aggregate/aggregate_test.go b/sdk/metric/internal/aggregate/aggregate_test.go index a451b02e7c6..be568f14c04 100644 --- a/sdk/metric/internal/aggregate/aggregate_test.go +++ b/sdk/metric/internal/aggregate/aggregate_test.go @@ -74,21 +74,21 @@ func testBuilderFilter[N int64 | float64]() func(t *testing.T) { t.Helper() value, attr := N(1), alice - run := func(b Builder[N], wantO, wantF attribute.Set) func(*testing.T) { + run := func(b Builder[N], wantF attribute.Set, wantD []attribute.KeyValue) func(*testing.T) { return func(t *testing.T) { t.Helper() - meas := b.filter(func(_ context.Context, v N, o, f attribute.Set) { + meas := b.filter(func(_ context.Context, v N, f attribute.Set, d []attribute.KeyValue) { assert.Equal(t, value, v, "measured incorrect value") - assert.Equal(t, wantO, o, "measured incorrect original attributes") assert.Equal(t, wantF, f, "measured incorrect filtered attributes") + assert.ElementsMatch(t, wantD, d, "measured incorrect dropped attributes") }) meas(context.Background(), value, attr) } } - t.Run("NoFilter", run(Builder[N]{}, attr, attr)) - t.Run("Filter", run(Builder[N]{Filter: attrFltr}, alice, fltrAlice)) + t.Run("NoFilter", run(Builder[N]{}, attr, nil)) + t.Run("Filter", run(Builder[N]{Filter: attrFltr}, fltrAlice, []attribute.KeyValue{adminTrue})) } } diff --git a/sdk/metric/internal/aggregate/exponential_histogram.go b/sdk/metric/internal/aggregate/exponential_histogram.go index d2acf457089..0d1798d4185 100644 --- a/sdk/metric/internal/aggregate/exponential_histogram.go +++ b/sdk/metric/internal/aggregate/exponential_histogram.go @@ -322,7 +322,7 @@ type expoHistogram[N int64 | float64] struct { start time.Time } -func (e *expoHistogram[N]) measure(ctx context.Context, value N, origAttr, fltrAttr attribute.Set) { +func (e *expoHistogram[N]) measure(ctx context.Context, value N, fltrAttr attribute.Set, droppedAttr []attribute.KeyValue) { // Ignore NaN and infinity. if math.IsInf(float64(value), 0) || math.IsNaN(float64(value)) { return @@ -342,7 +342,7 @@ func (e *expoHistogram[N]) measure(ctx context.Context, value N, origAttr, fltrA e.values[attr] = v } v.record(value) - v.res.Offer(ctx, t, value, origAttr) + v.res.Offer(ctx, t, value, droppedAttr) } func (e *expoHistogram[N]) delta(dest *metricdata.Aggregation) int { @@ -384,7 +384,7 @@ func (e *expoHistogram[N]) delta(dest *metricdata.Aggregation) int { hDPts[i].Max = metricdata.NewExtrema(b.max) } - b.res.Flush(&hDPts[i].Exemplars, a) + b.res.Flush(&hDPts[i].Exemplars) delete(e.values, a) i++ @@ -434,7 +434,7 @@ func (e *expoHistogram[N]) cumulative(dest *metricdata.Aggregation) int { hDPts[i].Max = metricdata.NewExtrema(b.max) } - b.res.Collect(&hDPts[i].Exemplars, a) + b.res.Collect(&hDPts[i].Exemplars) i++ // TODO (#3006): This will use an unbounded amount of memory if there diff --git a/sdk/metric/internal/aggregate/exponential_histogram_test.go b/sdk/metric/internal/aggregate/exponential_histogram_test.go index 577d216d35e..3fed4897ee2 100644 --- a/sdk/metric/internal/aggregate/exponential_histogram_test.go +++ b/sdk/metric/internal/aggregate/exponential_histogram_test.go @@ -185,7 +185,7 @@ func testExpoHistogramMinMaxSumInt64(t *testing.T) { h := newExponentialHistogram[int64](4, 20, false, false, 0, dropExemplars[int64]) for _, v := range tt.values { - h.measure(context.Background(), v, alice, alice) + h.measure(context.Background(), v, alice, nil) } dp := h.values[alice] @@ -227,7 +227,7 @@ func testExpoHistogramMinMaxSumFloat64(t *testing.T) { h := newExponentialHistogram[float64](4, 20, false, false, 0, dropExemplars[float64]) for _, v := range tt.values { - h.measure(context.Background(), v, alice, alice) + h.measure(context.Background(), v, alice, nil) } dp := h.values[alice] diff --git a/sdk/metric/internal/aggregate/histogram.go b/sdk/metric/internal/aggregate/histogram.go index 68199b43a8c..0618639fb9e 100644 --- a/sdk/metric/internal/aggregate/histogram.go +++ b/sdk/metric/internal/aggregate/histogram.go @@ -82,7 +82,7 @@ func newHistValues[N int64 | float64](bounds []float64, noSum bool, limit int, r // Aggregate records the measurement value, scoped by attr, and aggregates it // into a histogram. -func (s *histValues[N]) measure(ctx context.Context, value N, origAttr, fltrAttr attribute.Set) { +func (s *histValues[N]) measure(ctx context.Context, value N, fltrAttr attribute.Set, droppedAttr []attribute.KeyValue) { // This search will return an index in the range [0, len(s.bounds)], where // it will return len(s.bounds) if value is greater than the last element // of s.bounds. This aligns with the buckets in that the length of buckets @@ -116,7 +116,7 @@ func (s *histValues[N]) measure(ctx context.Context, value N, origAttr, fltrAttr if !s.noSum { b.sum(value) } - b.res.Offer(ctx, t, value, origAttr) + b.res.Offer(ctx, t, value, droppedAttr) } // newHistogram returns an Aggregator that summarizes a set of measurements as @@ -174,7 +174,7 @@ func (s *histogram[N]) delta(dest *metricdata.Aggregation) int { hDPts[i].Max = metricdata.NewExtrema(b.max) } - b.res.Flush(&hDPts[i].Exemplars, a) + b.res.Flush(&hDPts[i].Exemplars) // Unused attribute sets do not report. delete(s.values, a) @@ -233,7 +233,7 @@ func (s *histogram[N]) cumulative(dest *metricdata.Aggregation) int { hDPts[i].Max = metricdata.NewExtrema(b.max) } - b.res.Collect(&hDPts[i].Exemplars, a) + b.res.Collect(&hDPts[i].Exemplars) i++ // TODO (#3006): This will use an unbounded amount of memory if there diff --git a/sdk/metric/internal/aggregate/histogram_test.go b/sdk/metric/internal/aggregate/histogram_test.go index e7916a4bce9..bd88b083492 100644 --- a/sdk/metric/internal/aggregate/histogram_test.go +++ b/sdk/metric/internal/aggregate/histogram_test.go @@ -319,7 +319,7 @@ func TestHistogramImmutableBounds(t *testing.T) { b[0] = 10 assert.Equal(t, cpB, h.bounds, "modifying the bounds argument should not change the bounds") - h.measure(context.Background(), 5, alice, alice) + h.measure(context.Background(), 5, alice, nil) var data metricdata.Aggregation = metricdata.Histogram[int64]{} h.cumulative(&data) @@ -330,7 +330,7 @@ func TestHistogramImmutableBounds(t *testing.T) { func TestCumulativeHistogramImutableCounts(t *testing.T) { h := newHistogram[int64](bounds, noMinMax, false, 0, dropExemplars[int64]) - h.measure(context.Background(), 5, alice, alice) + h.measure(context.Background(), 5, alice, nil) var data metricdata.Aggregation = metricdata.Histogram[int64]{} h.cumulative(&data) @@ -353,7 +353,7 @@ func TestDeltaHistogramReset(t *testing.T) { require.Equal(t, 0, h.delta(&data)) require.Len(t, data.(metricdata.Histogram[int64]).DataPoints, 0) - h.measure(context.Background(), 1, alice, alice) + h.measure(context.Background(), 1, alice, nil) expect := metricdata.Histogram[int64]{Temporality: metricdata.DeltaTemporality} expect.DataPoints = []metricdata.HistogramDataPoint[int64]{hPointSummed[int64](alice, 1, 1)} @@ -366,7 +366,7 @@ func TestDeltaHistogramReset(t *testing.T) { assert.Len(t, data.(metricdata.Histogram[int64]).DataPoints, 0) // Aggregating another set should not affect the original (alice). - h.measure(context.Background(), 1, bob, bob) + h.measure(context.Background(), 1, bob, nil) expect.DataPoints = []metricdata.HistogramDataPoint[int64]{hPointSummed[int64](bob, 1, 1)} h.delta(&data) metricdatatest.AssertAggregationsEqual(t, expect, data) diff --git a/sdk/metric/internal/aggregate/lastvalue.go b/sdk/metric/internal/aggregate/lastvalue.go index 0a1d95d1867..5d3c27a376c 100644 --- a/sdk/metric/internal/aggregate/lastvalue.go +++ b/sdk/metric/internal/aggregate/lastvalue.go @@ -48,7 +48,7 @@ type lastValue[N int64 | float64] struct { values map[attribute.Set]datapoint[N] } -func (s *lastValue[N]) measure(ctx context.Context, value N, origAttr, fltrAttr attribute.Set) { +func (s *lastValue[N]) measure(ctx context.Context, value N, fltrAttr attribute.Set, droppedAttr []attribute.KeyValue) { t := now() s.Lock() @@ -62,7 +62,7 @@ func (s *lastValue[N]) measure(ctx context.Context, value N, origAttr, fltrAttr d.timestamp = t d.value = value - d.res.Offer(ctx, t, value, origAttr) + d.res.Offer(ctx, t, value, droppedAttr) s.values[attr] = d } @@ -81,7 +81,7 @@ func (s *lastValue[N]) computeAggregation(dest *[]metricdata.DataPoint[N]) { // ignored. (*dest)[i].Time = v.timestamp (*dest)[i].Value = v.value - v.res.Flush(&(*dest)[i].Exemplars, a) + v.res.Flush(&(*dest)[i].Exemplars) // Do not report stale values. delete(s.values, a) i++ diff --git a/sdk/metric/internal/aggregate/sum.go b/sdk/metric/internal/aggregate/sum.go index ca8aba1df6f..35446e6a5e5 100644 --- a/sdk/metric/internal/aggregate/sum.go +++ b/sdk/metric/internal/aggregate/sum.go @@ -45,7 +45,7 @@ func newValueMap[N int64 | float64](limit int, r func() exemplar.Reservoir[N]) * } } -func (s *valueMap[N]) measure(ctx context.Context, value N, origAttr, fltrAttr attribute.Set) { +func (s *valueMap[N]) measure(ctx context.Context, value N, fltrAttr attribute.Set, droppedAttr []attribute.KeyValue) { t := now() s.Lock() @@ -58,7 +58,7 @@ func (s *valueMap[N]) measure(ctx context.Context, value N, origAttr, fltrAttr a } v.n += value - v.res.Offer(ctx, t, value, origAttr) + v.res.Offer(ctx, t, value, droppedAttr) s.values[attr] = v } @@ -103,7 +103,7 @@ func (s *sum[N]) delta(dest *metricdata.Aggregation) int { dPts[i].StartTime = s.start dPts[i].Time = t dPts[i].Value = val.n - val.res.Flush(&dPts[i].Exemplars, attr) + val.res.Flush(&dPts[i].Exemplars) // Do not report stale values. delete(s.values, attr) i++ @@ -138,7 +138,7 @@ func (s *sum[N]) cumulative(dest *metricdata.Aggregation) int { dPts[i].StartTime = s.start dPts[i].Time = t dPts[i].Value = value.n - value.res.Collect(&dPts[i].Exemplars, attr) + value.res.Collect(&dPts[i].Exemplars) // TODO (#3006): This will use an unbounded amount of memory if there // are unbounded number of attribute sets being aggregated. Attribute // sets that become "stale" need to be forgotten so this will not @@ -197,7 +197,7 @@ func (s *precomputedSum[N]) delta(dest *metricdata.Aggregation) int { dPts[i].StartTime = s.start dPts[i].Time = t dPts[i].Value = delta - value.res.Flush(&dPts[i].Exemplars, attr) + value.res.Flush(&dPts[i].Exemplars) newReported[attr] = value.n // Unused attribute sets do not report. @@ -236,7 +236,7 @@ func (s *precomputedSum[N]) cumulative(dest *metricdata.Aggregation) int { dPts[i].StartTime = s.start dPts[i].Time = t dPts[i].Value = val.n - val.res.Collect(&dPts[i].Exemplars, attr) + val.res.Collect(&dPts[i].Exemplars) // Unused attribute sets do not report. delete(s.values, attr) diff --git a/sdk/metric/internal/exemplar/drop.go b/sdk/metric/internal/exemplar/drop.go index 8f2054336b5..eed36a11bf1 100644 --- a/sdk/metric/internal/exemplar/drop.go +++ b/sdk/metric/internal/exemplar/drop.go @@ -27,12 +27,12 @@ func Drop[N int64 | float64]() Reservoir[N] { return &dropRes[N]{} } type dropRes[N int64 | float64] struct{} -func (r *dropRes[N]) Offer(context.Context, time.Time, N, attribute.Set) {} +func (r *dropRes[N]) Offer(context.Context, time.Time, N, []attribute.KeyValue) {} -func (r *dropRes[N]) Collect(dest *[]metricdata.Exemplar[N], _ attribute.Set) { +func (r *dropRes[N]) Collect(dest *[]metricdata.Exemplar[N]) { *dest = (*dest)[:0] } -func (r *dropRes[N]) Flush(dest *[]metricdata.Exemplar[N], _ attribute.Set) { +func (r *dropRes[N]) Flush(dest *[]metricdata.Exemplar[N]) { *dest = (*dest)[:0] } diff --git a/sdk/metric/internal/exemplar/filter.go b/sdk/metric/internal/exemplar/filter.go index ecc13f73057..898e3ad3641 100644 --- a/sdk/metric/internal/exemplar/filter.go +++ b/sdk/metric/internal/exemplar/filter.go @@ -26,24 +26,24 @@ import ( // return, the measurement will be considered for sampling. // // See [Filtered] for how to create a [Reservoir] that uses a Filter. -type Filter[N int64 | float64] func(context.Context, N, attribute.Set) bool +type Filter[N int64 | float64] func(context.Context) bool // AlwaysSample is a Filter that always signals measurements should be // considered for sampling by a [Reservoir]. -func AlwaysSample[N int64 | float64](context.Context, N, attribute.Set) bool { +func AlwaysSample[N int64 | float64](context.Context) bool { return true } // NeverSample is a Filter that always signals measurements should not be // considered for sampling by a [Reservoir]. -func NeverSample[N int64 | float64](context.Context, N, attribute.Set) bool { +func NeverSample[N int64 | float64](context.Context) bool { return false } // TraceBasedSample is a Filter that signals measurements should be considered // for sampling by a [Reservoir] if the ctx contains a // [go.opentelemetry.io/otel/trace.SpanContext] that is sampled. -func TraceBasedSample[N int64 | float64](ctx context.Context, _ N, _ attribute.Set) bool { +func TraceBasedSample[N int64 | float64](ctx context.Context) bool { return trace.SpanContextFromContext(ctx).IsSampled() } @@ -59,8 +59,8 @@ type filtered[N int64 | float64] struct { Filter Filter[N] } -func (f filtered[N]) Offer(ctx context.Context, t time.Time, n N, a attribute.Set) { - if f.Filter(ctx, n, a) { +func (f filtered[N]) Offer(ctx context.Context, t time.Time, n N, a []attribute.KeyValue) { + if f.Filter(ctx) { f.Reservoir.Offer(ctx, t, n, a) } } diff --git a/sdk/metric/internal/exemplar/filter_test.go b/sdk/metric/internal/exemplar/filter_test.go index c7a32b8dfa6..f8fa2f14b1c 100644 --- a/sdk/metric/internal/exemplar/filter_test.go +++ b/sdk/metric/internal/exemplar/filter_test.go @@ -16,7 +16,6 @@ package exemplar // import "go.opentelemetry.io/otel/sdk/metric/internal/exempla import ( "context" - "math" "testing" "time" @@ -44,14 +43,8 @@ func TestAlwaysSample(t *testing.T) { func testAlwaysSample[N int64 | float64](t *testing.T) { ctx := context.Background() - assert.True(t, AlwaysSample[N](ctx, 0, alice)) - assert.True(t, AlwaysSample[N](ctx, 0, fltrAlice)) - assert.True(t, AlwaysSample[N](sample(ctx), 0, alice)) - assert.True(t, AlwaysSample[N](ctx, math.MaxInt64, alice)) - assert.True(t, AlwaysSample[N](ctx, math.MinInt64, alice)) - assert.True(t, AlwaysSample[N](ctx, N(math.NaN()), alice)) - assert.True(t, AlwaysSample[N](ctx, N(math.Inf(-1)), alice)) - assert.True(t, AlwaysSample[N](ctx, N(math.Inf(1)), alice)) + assert.True(t, AlwaysSample[N](ctx)) + assert.True(t, AlwaysSample[N](sample(ctx))) } func TestNeverSample(t *testing.T) { @@ -62,14 +55,8 @@ func TestNeverSample(t *testing.T) { func testNeverSample[N int64 | float64](t *testing.T) { ctx := context.Background() - assert.False(t, NeverSample[N](ctx, 0, alice)) - assert.False(t, NeverSample[N](ctx, 0, fltrAlice)) - assert.False(t, NeverSample[N](sample(ctx), 0, alice)) - assert.False(t, NeverSample[N](ctx, math.MaxInt64, alice)) - assert.False(t, NeverSample[N](ctx, math.MinInt64, alice)) - assert.False(t, NeverSample[N](ctx, N(math.NaN()), alice)) - assert.False(t, NeverSample[N](ctx, N(math.Inf(-1)), alice)) - assert.False(t, NeverSample[N](ctx, N(math.Inf(1)), alice)) + assert.False(t, NeverSample[N](ctx)) + assert.False(t, NeverSample[N](sample(ctx))) } func TestTraceBasedSample(t *testing.T) { @@ -80,14 +67,8 @@ func TestTraceBasedSample(t *testing.T) { func testTraceBasedSample[N int64 | float64](t *testing.T) { ctx := context.Background() - assert.False(t, TraceBasedSample[N](ctx, 0, alice)) - assert.False(t, TraceBasedSample[N](ctx, 0, fltrAlice)) - assert.True(t, TraceBasedSample[N](sample(ctx), 0, alice)) - assert.False(t, TraceBasedSample[N](ctx, math.MaxInt64, alice)) - assert.False(t, TraceBasedSample[N](ctx, math.MinInt64, alice)) - assert.False(t, TraceBasedSample[N](ctx, N(math.NaN()), alice)) - assert.False(t, TraceBasedSample[N](ctx, N(math.Inf(-1)), alice)) - assert.False(t, TraceBasedSample[N](ctx, N(math.Inf(1)), alice)) + assert.False(t, TraceBasedSample[N](ctx)) + assert.True(t, TraceBasedSample[N](sample(ctx))) } type res[N int64 | float64] struct { @@ -96,15 +77,15 @@ type res[N int64 | float64] struct { FlushCalled bool } -func (r *res[N]) Offer(context.Context, time.Time, N, attribute.Set) { +func (r *res[N]) Offer(context.Context, time.Time, N, []attribute.KeyValue) { r.OfferCalled = true } -func (r *res[N]) Collect(*[]metricdata.Exemplar[N], attribute.Set) { +func (r *res[N]) Collect(*[]metricdata.Exemplar[N]) { r.CollectCalled = true } -func (r *res[N]) Flush(*[]metricdata.Exemplar[N], attribute.Set) { +func (r *res[N]) Flush(*[]metricdata.Exemplar[N]) { r.FlushCalled = true } @@ -117,20 +98,20 @@ func testFilteredReservoir[N int64 | float64](t *testing.T) { under := &res[N]{} var called bool - fltr := func(context.Context, N, attribute.Set) bool { + fltr := func(context.Context) bool { called = true return true } r := Filtered[N](under, fltr) - r.Offer(context.Background(), staticTime, 0, alice) + r.Offer(context.Background(), staticTime, 0, nil) assert.True(t, called, "filter not called on Offer") assert.True(t, under.OfferCalled, "underlying Reservoir Offer not called") - r.Collect(nil, alice) + r.Collect(nil) assert.True(t, under.CollectCalled, "underlying Reservoir Collect not called") - r.Flush(nil, alice) + r.Flush(nil) assert.True(t, under.FlushCalled, "underlying Reservoir Flush not called") } diff --git a/sdk/metric/internal/exemplar/fixed.go b/sdk/metric/internal/exemplar/fixed.go index d7e760d4267..9859b7472b4 100644 --- a/sdk/metric/internal/exemplar/fixed.go +++ b/sdk/metric/internal/exemplar/fixed.go @@ -19,7 +19,6 @@ import ( "time" "go.opentelemetry.io/otel/attribute" - "go.opentelemetry.io/otel/internal/global" "go.opentelemetry.io/otel/sdk/metric/metricdata" "go.opentelemetry.io/otel/trace" ) @@ -36,7 +35,7 @@ func newFixedRes[N int64 | float64](n int) *fixedRes[N] { return &fixedRes[N]{store: make([]measurement[N], n)} } -func (r *fixedRes[N]) Collect(dest *[]metricdata.Exemplar[N], attrs attribute.Set) { +func (r *fixedRes[N]) Collect(dest *[]metricdata.Exemplar[N]) { *dest = reset(*dest, len(r.store), len(r.store)) var n int for _, m := range r.store { @@ -44,13 +43,13 @@ func (r *fixedRes[N]) Collect(dest *[]metricdata.Exemplar[N], attrs attribute.Se continue } - m.Exemplar(&(*dest)[n], attrs) + m.Exemplar(&(*dest)[n]) n++ } *dest = (*dest)[:n] } -func (r *fixedRes[N]) Flush(dest *[]metricdata.Exemplar[N], attrs attribute.Set) { +func (r *fixedRes[N]) Flush(dest *[]metricdata.Exemplar[N]) { *dest = reset(*dest, len(r.store), len(r.store)) var n int for i, m := range r.store { @@ -58,7 +57,7 @@ func (r *fixedRes[N]) Flush(dest *[]metricdata.Exemplar[N], attrs attribute.Set) continue } - m.Exemplar(&(*dest)[n], attrs) + m.Exemplar(&(*dest)[n]) n++ // Reset. @@ -69,7 +68,7 @@ func (r *fixedRes[N]) Flush(dest *[]metricdata.Exemplar[N], attrs attribute.Set) // measurement is a measurement made by a telemetry system. type measurement[N int64 | float64] struct { - Attributes attribute.Set + Attributes []attribute.KeyValue Time time.Time Value N SpanContext trace.SpanContext @@ -78,9 +77,9 @@ type measurement[N int64 | float64] struct { } // newMeasurement returns a new non-empty Measurement. -func newMeasurement[N int64 | float64](ctx context.Context, ts time.Time, v N, measuredAttr attribute.Set) measurement[N] { +func newMeasurement[N int64 | float64](ctx context.Context, ts time.Time, v N, droppedAttr []attribute.KeyValue) measurement[N] { return measurement[N]{ - Attributes: measuredAttr, + Attributes: droppedAttr, Time: ts, Value: v, SpanContext: trace.SpanContextFromContext(ctx), @@ -93,13 +92,8 @@ func newMeasurement[N int64 | float64](ctx context.Context, ts time.Time, v N, m func (m measurement[N]) Empty() bool { return !m.valid } // Exemplar returns m as a [metricdata.Exemplar]. -func (m measurement[N]) Exemplar(dest *metricdata.Exemplar[N], recorded attribute.Set) { - // Note: A more optimal solution would be to store the filtered attributes - // when the exemplar is recorded, instead of re-calculating here. That - // approach isn't implemented though because it contrary to the OTel - // specification definition of a Reservoir, which is defined to accept the - // complete set of measured attributes. - dropped(&dest.FilteredAttributes, m.Attributes, recorded) +func (m measurement[N]) Exemplar(dest *metricdata.Exemplar[N]) { + dest.FilteredAttributes = m.Attributes dest.Time = m.Time dest.Value = m.Value @@ -118,49 +112,6 @@ func (m measurement[N]) Exemplar(dest *metricdata.Exemplar[N], recorded attribut } } -// dropped returns the attribute that were measured, but not included in the -// recorded attributes. -func dropped(dest *[]attribute.KeyValue, measured, recorded attribute.Set) { - measN := measured.Len() - recN := recorded.Len() - - n := measN - recN - switch { - case n < 0: - // recorded should only ever be the filtered set of measured. Abandon - // instead of panicking. - global.Warn( - "invalid measured attributes for exemplar, dropping", - "measured", measured, - "recorded", recorded, - ) - fallthrough - case n == 0: - // Nothing dropped. - *dest = (*dest)[:0] - return - } - *dest = reset(*dest, n, n) - - measIter := measured.Iter() - recIter := recorded.Iter() - - var i int - recIter.Next() - for measIter.Next() { - m := measIter.Attribute() - r := recIter.Attribute() - - if m == r { - recIter.Next() - continue - } - - (*dest)[i] = m - i++ - } -} - func reset[T any](s []T, length, capacity int) []T { if cap(s) < capacity { return make([]T, length, capacity) diff --git a/sdk/metric/internal/exemplar/fixed_test.go b/sdk/metric/internal/exemplar/fixed_test.go deleted file mode 100644 index 98a3dad8413..00000000000 --- a/sdk/metric/internal/exemplar/fixed_test.go +++ /dev/null @@ -1,46 +0,0 @@ -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package exemplar // import "go.opentelemetry.io/otel/sdk/metric/internal/exemplar" - -import ( - "log" - "os" - "testing" - - "github.com/go-logr/logr" - "github.com/go-logr/logr/funcr" - "github.com/go-logr/stdr" - "github.com/stretchr/testify/assert" - - "go.opentelemetry.io/otel" - "go.opentelemetry.io/otel/attribute" -) - -func TestDroppedHandlesInvalidGracefully(t *testing.T) { - var msg string - t.Cleanup(func(orig logr.Logger) func() { - otel.SetLogger(funcr.New(func(_, args string) { - msg = args - }, funcr.Options{Verbosity: 20})) - return func() { otel.SetLogger(orig) } - }(stdr.New(log.New(os.Stderr, "", log.LstdFlags|log.Lshortfile)))) - - dest := []attribute.KeyValue{{}} - // measured < recorded is invalid. - dropped(&dest, fltrAlice, alice) - - assert.Contains(t, msg, "invalid measured attributes for exemplar, dropping") - assert.Len(t, dest, 0, "dest not reset") -} diff --git a/sdk/metric/internal/exemplar/hist.go b/sdk/metric/internal/exemplar/hist.go index a0ec197301a..4b1e0645339 100644 --- a/sdk/metric/internal/exemplar/hist.go +++ b/sdk/metric/internal/exemplar/hist.go @@ -42,6 +42,6 @@ type histRes[N int64 | float64] struct { bounds []float64 } -func (r *histRes[N]) Offer(ctx context.Context, t time.Time, n N, a attribute.Set) { +func (r *histRes[N]) Offer(ctx context.Context, t time.Time, n N, a []attribute.KeyValue) { r.store[sort.SearchFloat64s(r.bounds, float64(n))] = newMeasurement(ctx, t, n, a) } diff --git a/sdk/metric/internal/exemplar/rand.go b/sdk/metric/internal/exemplar/rand.go index b2ad2c4fe38..12df46004dc 100644 --- a/sdk/metric/internal/exemplar/rand.go +++ b/sdk/metric/internal/exemplar/rand.go @@ -50,7 +50,7 @@ type randRes[N int64 | float64] struct { w float64 } -func (r *randRes[N]) Offer(ctx context.Context, t time.Time, n N, a attribute.Set) { +func (r *randRes[N]) Offer(ctx context.Context, t time.Time, n N, 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): @@ -89,12 +89,12 @@ func (r *randRes[N]) advance() { r.next += int64(math.Log(rng.Float64())/math.Log(1-r.w)) + 1 } -func (r *randRes[N]) Collect(dest *[]metricdata.Exemplar[N], attrs attribute.Set) { - r.fixedRes.Collect(dest, attrs) +func (r *randRes[N]) Collect(dest *[]metricdata.Exemplar[N]) { + r.fixedRes.Collect(dest) r.reset() } -func (r *randRes[N]) Flush(dest *[]metricdata.Exemplar[N], attrs attribute.Set) { - r.fixedRes.Flush(dest, attrs) +func (r *randRes[N]) Flush(dest *[]metricdata.Exemplar[N]) { + r.fixedRes.Flush(dest) r.reset() } diff --git a/sdk/metric/internal/exemplar/rand_test.go b/sdk/metric/internal/exemplar/rand_test.go index 87a0d68ae13..8f0df8ba43c 100644 --- a/sdk/metric/internal/exemplar/rand_test.go +++ b/sdk/metric/internal/exemplar/rand_test.go @@ -46,7 +46,7 @@ func TestFixedSizeSamplingCorrectness(t *testing.T) { r := FixedSize[float64](sampleSize) for _, value := range data { - r.Offer(context.Background(), staticTime, value, alice) + r.Offer(context.Background(), staticTime, value, nil) } var sum float64 diff --git a/sdk/metric/internal/exemplar/reservoir.go b/sdk/metric/internal/exemplar/reservoir.go index 7fa5b1c1577..302acee5527 100644 --- a/sdk/metric/internal/exemplar/reservoir.go +++ b/sdk/metric/internal/exemplar/reservoir.go @@ -32,22 +32,22 @@ type Reservoir[N int64 | float64] interface { // when the measurement was made. This information may be used by the // Reservoir in making a sampling decision. // - // The time t is the time when the measurement was made. The val and attrs - // parameters are the value and complete (unfiltered) attribute set of the + // 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 N, attrs attribute.Set) + Offer(ctx context.Context, t time.Time, val N, attr []attribute.KeyValue) // Collect returns all the held exemplars with each exemplars dropped // attributes updated to include any attributes the Filter filters out. // // The Reservoir state is preserved after this call. See Flush to // copy-and-clear instead. - Collect(dest *[]metricdata.Exemplar[N], attrs attribute.Set) + Collect(dest *[]metricdata.Exemplar[N]) // Flush returns all the held exemplars with each exemplars dropped // attributes updated to include any attributes the Filter filters out. // // The Reservoir state is reset after this call. See Collect to preserve // the state instead. - Flush(dest *[]metricdata.Exemplar[N], attrs attribute.Set) + Flush(dest *[]metricdata.Exemplar[N]) } diff --git a/sdk/metric/internal/exemplar/reservoir_test.go b/sdk/metric/internal/exemplar/reservoir_test.go index 1f3f200d15a..4ba169cacd7 100644 --- a/sdk/metric/internal/exemplar/reservoir_test.go +++ b/sdk/metric/internal/exemplar/reservoir_test.go @@ -28,13 +28,8 @@ import ( ) var ( - keyUser = "user" - userAlice = attribute.String(keyUser, "Alice") adminTrue = attribute.Bool("admin", true) - alice = attribute.NewSet(userAlice, adminTrue) - fltrAlice = attribute.NewSet(userAlice) - // Sat Jan 01 2000 00:00:00 GMT+0000. staticTime = time.Unix(946684800, 0) ) @@ -63,10 +58,10 @@ func testReservoir[N int64 | float64](f factory[N]) func(*testing.T) { }) ctx := trace.ContextWithSpanContext(ctx, sc) - r.Offer(ctx, staticTime, 10, alice) + r.Offer(ctx, staticTime, 10, nil) var dest []metricdata.Exemplar[N] - r.Collect(&dest, alice) + r.Collect(&dest) want := metricdata.Exemplar[N]{ Time: staticTime, @@ -86,10 +81,10 @@ func testReservoir[N int64 | float64](f factory[N]) func(*testing.T) { t.Skip("skipping, reservoir capacity less than 1:", n) } - r.Offer(ctx, staticTime, 10, alice) + r.Offer(ctx, staticTime, 10, []attribute.KeyValue{adminTrue}) var dest []metricdata.Exemplar[N] - r.Collect(&dest, fltrAlice) + r.Collect(&dest) want := metricdata.Exemplar[N]{ FilteredAttributes: []attribute.KeyValue{adminTrue}, @@ -108,14 +103,14 @@ func testReservoir[N int64 | float64](f factory[N]) func(*testing.T) { t.Skip("skipping, reservoir capacity less than 1:", n) } - r.Offer(ctx, staticTime, 10, alice) + r.Offer(ctx, staticTime, 10, nil) var dest []metricdata.Exemplar[N] - r.Collect(&dest, alice) + r.Collect(&dest) require.Len(t, dest, 1, "number of collected exemplars") dest = dest[:0] - r.Collect(&dest, alice) + r.Collect(&dest) assert.Len(t, dest, 1, "Collect flushed reservoir") }) @@ -127,13 +122,13 @@ func testReservoir[N int64 | float64](f factory[N]) func(*testing.T) { t.Skip("skipping, reservoir capacity less than 1:", n) } - r.Offer(ctx, staticTime, 10, alice) + r.Offer(ctx, staticTime, 10, nil) var dest []metricdata.Exemplar[N] - r.Flush(&dest, alice) + r.Flush(&dest) require.Len(t, dest, 1, "number of flushed exemplars") - r.Flush(&dest, alice) + r.Flush(&dest) assert.Len(t, dest, 0, "Flush did not flush reservoir") }) @@ -147,21 +142,21 @@ func testReservoir[N int64 | float64](f factory[N]) func(*testing.T) { for i := 0; i < n+1; i++ { v := N(i) - r.Offer(ctx, staticTime, v, alice) + r.Offer(ctx, staticTime, v, nil) } var dest []metricdata.Exemplar[N] - r.Flush(&dest, alice) + r.Flush(&dest) assert.Len(t, dest, n, "multiple offers did not fill reservoir") // Ensure the flush reset also resets any couting state. for i := 0; i < n+1; i++ { v := N(2 * i) - r.Offer(ctx, staticTime, v, alice) + r.Offer(ctx, staticTime, v, nil) } dest = dest[:0] - r.Flush(&dest, alice) + r.Flush(&dest) assert.Len(t, dest, n, "internal count state not reset") }) @@ -173,15 +168,15 @@ func testReservoir[N int64 | float64](f factory[N]) func(*testing.T) { t.Skip("skipping, reservoir capacity greater than 0:", n) } - r.Offer(context.Background(), staticTime, 10, alice) + r.Offer(context.Background(), staticTime, 10, nil) dest := []metricdata.Exemplar[N]{{}} // Should be reset to empty. - r.Collect(&dest, alice) + r.Collect(&dest) assert.Len(t, dest, 0, "no exemplars should be collected") - r.Offer(context.Background(), staticTime, 10, alice) + r.Offer(context.Background(), staticTime, 10, nil) dest = []metricdata.Exemplar[N]{{}} // Should be reset to empty. - r.Flush(&dest, alice) + r.Flush(&dest) assert.Len(t, dest, 0, "no exemplars should be flushed") }) } From c4d8381054cf3ca2558aeff438cb2ff6daab14b1 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 19 Jan 2024 09:45:11 -0800 Subject: [PATCH 13/24] Remove the unneeded exemplar Filter --- sdk/metric/exemplar.go | 37 +++++++------ sdk/metric/internal/exemplar/filter.go | 38 +++----------- sdk/metric/internal/exemplar/filter_test.go | 58 ++++----------------- 3 files changed, 33 insertions(+), 100 deletions(-) diff --git a/sdk/metric/exemplar.go b/sdk/metric/exemplar.go index 865ed3e5012..7938f897198 100644 --- a/sdk/metric/exemplar.go +++ b/sdk/metric/exemplar.go @@ -27,27 +27,13 @@ import ( // environment variables. // // Note: This will only return non-nil values when the experimental exemplar -// feature is enabled. +// feature is enabled and the OTEL_METRICS_EXEMPLAR_FILTER environment variable +// is not set to always_off. func reservoirFunc[N int64 | float64](agg Aggregation) func() exemplar.Reservoir[N] { if !x.Exemplars.Enabled() { return nil } - // https://github.com/open-telemetry/opentelemetry-specification/blob/d4b241f451674e8f611bb589477680341006ad2b/specification/configuration/sdk-environment-variables.md#exemplar - const filterEnvKey = "OTEL_METRICS_EXEMPLAR_FILTER" - - var fltr exemplar.Filter[N] - switch os.Getenv(filterEnvKey) { - case "always_on": - fltr = exemplar.AlwaysSample[N] - case "always_off": - fltr = exemplar.NeverSample[N] - case "trace_based": - fallthrough - default: - fltr = exemplar.TraceBasedSample[N] - } - // https://github.com/open-telemetry/opentelemetry-specification/blob/d4b241f451674e8f611bb589477680341006ad2b/specification/metrics/sdk.md#exemplar-defaults resF := func() func() exemplar.Reservoir[N] { // Explicit bucket histogram aggregation with more than 1 bucket will @@ -89,9 +75,22 @@ func reservoirFunc[N int64 | float64](agg Aggregation) func() exemplar.Reservoir return func() exemplar.Reservoir[N] { return exemplar.FixedSize[N](n) } - }() + } - return func() exemplar.Reservoir[N] { - return exemplar.Filtered[N](resF(), fltr) + // https://github.com/open-telemetry/opentelemetry-specification/blob/d4b241f451674e8f611bb589477680341006ad2b/specification/configuration/sdk-environment-variables.md#exemplar + const filterEnvKey = "OTEL_METRICS_EXEMPLAR_FILTER" + + switch os.Getenv(filterEnvKey) { + case "always_on": + return resF() + case "always_off": + return exemplar.Drop[N] + case "trace_based": + fallthrough + default: + newR := resF() + return func() exemplar.Reservoir[N] { + return exemplar.SampledFilter(newR()) + } } } diff --git a/sdk/metric/internal/exemplar/filter.go b/sdk/metric/internal/exemplar/filter.go index 898e3ad3641..4f5946fb966 100644 --- a/sdk/metric/internal/exemplar/filter.go +++ b/sdk/metric/internal/exemplar/filter.go @@ -22,45 +22,19 @@ import ( "go.opentelemetry.io/otel/trace" ) -// Filter determines filters measurements passed to a [Reservoir]. If true is -// return, the measurement will be considered for sampling. -// -// See [Filtered] for how to create a [Reservoir] that uses a Filter. -type Filter[N int64 | float64] func(context.Context) bool - -// AlwaysSample is a Filter that always signals measurements should be -// considered for sampling by a [Reservoir]. -func AlwaysSample[N int64 | float64](context.Context) bool { - return true -} - -// NeverSample is a Filter that always signals measurements should not be -// considered for sampling by a [Reservoir]. -func NeverSample[N int64 | float64](context.Context) bool { - return false -} - -// TraceBasedSample is a Filter that signals measurements should be considered -// for sampling by a [Reservoir] if the ctx contains a -// [go.opentelemetry.io/otel/trace.SpanContext] that is sampled. -func TraceBasedSample[N int64 | float64](ctx context.Context) bool { - return trace.SpanContextFromContext(ctx).IsSampled() -} - -// Filtered returns a [Reservoir] wrapping r that will only offer measurements -// to r if f returns true. -func Filtered[N int64 | float64](r Reservoir[N], f Filter[N]) Reservoir[N] { - return filtered[N]{Reservoir: r, Filter: f} +// SampledFilter returns a [Reservoir] wrapping r that will only offer measurements +// to r if the passed context associated with the measurement contains a sampled +// [go.opentelemetry.io/otel/trace.SpanContext]. +func SampledFilter[N int64 | float64](r Reservoir[N]) Reservoir[N] { + return filtered[N]{Reservoir: r} } type filtered[N int64 | float64] struct { Reservoir[N] - - Filter Filter[N] } func (f filtered[N]) Offer(ctx context.Context, t time.Time, n N, a []attribute.KeyValue) { - if f.Filter(ctx) { + if trace.SpanContextFromContext(ctx).IsSampled() { f.Reservoir.Offer(ctx, t, n, a) } } diff --git a/sdk/metric/internal/exemplar/filter_test.go b/sdk/metric/internal/exemplar/filter_test.go index f8fa2f14b1c..aede7a21523 100644 --- a/sdk/metric/internal/exemplar/filter_test.go +++ b/sdk/metric/internal/exemplar/filter_test.go @@ -35,42 +35,6 @@ func sample(parent context.Context) context.Context { return trace.ContextWithSpanContext(parent, sc) } -func TestAlwaysSample(t *testing.T) { - t.Run("Int64", testAlwaysSample[int64]) - t.Run("Float64", testAlwaysSample[float64]) -} - -func testAlwaysSample[N int64 | float64](t *testing.T) { - ctx := context.Background() - - assert.True(t, AlwaysSample[N](ctx)) - assert.True(t, AlwaysSample[N](sample(ctx))) -} - -func TestNeverSample(t *testing.T) { - t.Run("Int64", testNeverSample[int64]) - t.Run("Float64", testNeverSample[float64]) -} - -func testNeverSample[N int64 | float64](t *testing.T) { - ctx := context.Background() - - assert.False(t, NeverSample[N](ctx)) - assert.False(t, NeverSample[N](sample(ctx))) -} - -func TestTraceBasedSample(t *testing.T) { - t.Run("Int64", testTraceBasedSample[int64]) - t.Run("Float64", testTraceBasedSample[float64]) -} - -func testTraceBasedSample[N int64 | float64](t *testing.T) { - ctx := context.Background() - - assert.False(t, TraceBasedSample[N](ctx)) - assert.True(t, TraceBasedSample[N](sample(ctx))) -} - type res[N int64 | float64] struct { OfferCalled bool CollectCalled bool @@ -89,24 +53,20 @@ func (r *res[N]) Flush(*[]metricdata.Exemplar[N]) { r.FlushCalled = true } -func TestFilteredReservoir(t *testing.T) { - t.Run("Int64", testFilteredReservoir[int64]) - t.Run("Float64", testFilteredReservoir[float64]) +func TestSampledFilter(t *testing.T) { + t.Run("Int64", testSampledFiltered[int64]) + t.Run("Float64", testSampledFiltered[float64]) } -func testFilteredReservoir[N int64 | float64](t *testing.T) { +func testSampledFiltered[N int64 | float64](t *testing.T) { under := &res[N]{} - var called bool - fltr := func(context.Context) bool { - called = true - return true - } + r := SampledFilter[N](under) - r := Filtered[N](under, fltr) - - r.Offer(context.Background(), staticTime, 0, nil) - assert.True(t, called, "filter not called on Offer") + ctx := context.Background() + r.Offer(ctx, staticTime, 0, nil) + assert.False(t, under.OfferCalled, "underlying Reservoir Offer called") + r.Offer(sample(ctx), staticTime, 0, nil) assert.True(t, under.OfferCalled, "underlying Reservoir Offer not called") r.Collect(nil) From f77205fb8c8dda93c8b8bc5d19e512a11a27be04 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 19 Jan 2024 10:15:17 -0800 Subject: [PATCH 14/24] Comment drop --- sdk/metric/internal/exemplar/drop.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sdk/metric/internal/exemplar/drop.go b/sdk/metric/internal/exemplar/drop.go index eed36a11bf1..62a95b55f9a 100644 --- a/sdk/metric/internal/exemplar/drop.go +++ b/sdk/metric/internal/exemplar/drop.go @@ -27,12 +27,15 @@ func Drop[N int64 | float64]() Reservoir[N] { return &dropRes[N]{} } type dropRes[N int64 | float64] struct{} +// Offer does nothing, all measurements offered will be dropped. func (r *dropRes[N]) Offer(context.Context, time.Time, N, []attribute.KeyValue) {} +// Collect resets dest. No exemplars will ever be returned. func (r *dropRes[N]) Collect(dest *[]metricdata.Exemplar[N]) { *dest = (*dest)[:0] } +// Flush resets dest. No exemplars will ever be returned. func (r *dropRes[N]) Flush(dest *[]metricdata.Exemplar[N]) { *dest = (*dest)[:0] } From 11bace0e3a2005051fa649ca1621d5839d848509 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 19 Jan 2024 10:15:42 -0800 Subject: [PATCH 15/24] Reorder filter_test.go --- sdk/metric/internal/exemplar/filter_test.go | 46 ++++++++++----------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/sdk/metric/internal/exemplar/filter_test.go b/sdk/metric/internal/exemplar/filter_test.go index aede7a21523..ae1e276cb83 100644 --- a/sdk/metric/internal/exemplar/filter_test.go +++ b/sdk/metric/internal/exemplar/filter_test.go @@ -26,6 +26,29 @@ import ( "go.opentelemetry.io/otel/trace" ) +func TestSampledFilter(t *testing.T) { + t.Run("Int64", testSampledFiltered[int64]) + t.Run("Float64", testSampledFiltered[float64]) +} + +func testSampledFiltered[N int64 | float64](t *testing.T) { + under := &res[N]{} + + r := SampledFilter[N](under) + + ctx := context.Background() + r.Offer(ctx, staticTime, 0, nil) + assert.False(t, under.OfferCalled, "underlying Reservoir Offer called") + r.Offer(sample(ctx), staticTime, 0, nil) + assert.True(t, under.OfferCalled, "underlying Reservoir Offer not called") + + r.Collect(nil) + assert.True(t, under.CollectCalled, "underlying Reservoir Collect not called") + + r.Flush(nil) + assert.True(t, under.FlushCalled, "underlying Reservoir Flush not called") +} + func sample(parent context.Context) context.Context { sc := trace.NewSpanContext(trace.SpanContextConfig{ TraceID: trace.TraceID{0x01}, @@ -52,26 +75,3 @@ func (r *res[N]) Collect(*[]metricdata.Exemplar[N]) { func (r *res[N]) Flush(*[]metricdata.Exemplar[N]) { r.FlushCalled = true } - -func TestSampledFilter(t *testing.T) { - t.Run("Int64", testSampledFiltered[int64]) - t.Run("Float64", testSampledFiltered[float64]) -} - -func testSampledFiltered[N int64 | float64](t *testing.T) { - under := &res[N]{} - - r := SampledFilter[N](under) - - ctx := context.Background() - r.Offer(ctx, staticTime, 0, nil) - assert.False(t, under.OfferCalled, "underlying Reservoir Offer called") - r.Offer(sample(ctx), staticTime, 0, nil) - assert.True(t, under.OfferCalled, "underlying Reservoir Offer not called") - - r.Collect(nil) - assert.True(t, under.CollectCalled, "underlying Reservoir Collect not called") - - r.Flush(nil) - assert.True(t, under.FlushCalled, "underlying Reservoir Flush not called") -} From d2171e73549a9028cadab821870240d12c66a51d Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 19 Jan 2024 10:16:45 -0800 Subject: [PATCH 16/24] Rename measurement.Empty to Valid --- sdk/metric/internal/exemplar/fixed.go | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/sdk/metric/internal/exemplar/fixed.go b/sdk/metric/internal/exemplar/fixed.go index 9859b7472b4..45430a9013d 100644 --- a/sdk/metric/internal/exemplar/fixed.go +++ b/sdk/metric/internal/exemplar/fixed.go @@ -35,11 +35,15 @@ func newFixedRes[N int64 | float64](n int) *fixedRes[N] { return &fixedRes[N]{store: make([]measurement[N], n)} } +// Collect returns all the held exemplars. +// +// The Reservoir state is preserved after this call. See Flush to +// copy-and-clear instead. func (r *fixedRes[N]) Collect(dest *[]metricdata.Exemplar[N]) { *dest = reset(*dest, len(r.store), len(r.store)) var n int for _, m := range r.store { - if m.Empty() { + if !m.Valid() { continue } @@ -49,11 +53,15 @@ func (r *fixedRes[N]) Collect(dest *[]metricdata.Exemplar[N]) { *dest = (*dest)[:n] } +// Flush returns all the held exemplars. +// +// The Reservoir state is reset after this call. See Collect to preserve the +// state instead. func (r *fixedRes[N]) Flush(dest *[]metricdata.Exemplar[N]) { *dest = reset(*dest, len(r.store), len(r.store)) var n int for i, m := range r.store { - if m.Empty() { + if !m.Valid() { continue } @@ -87,9 +95,9 @@ func newMeasurement[N int64 | float64](ctx context.Context, ts time.Time, v N, d } } -// Empty returns false if m represents a measurement made by a telemetry -// system, otherwise it returns true when m is its zero-value. -func (m measurement[N]) Empty() bool { return !m.valid } +// Valid returns true if m represents a measurement made by a telemetry +// system (created with newMeasurement), otherwise it returns false. +func (m measurement[N]) Valid() bool { return m.valid } // Exemplar returns m as a [metricdata.Exemplar]. func (m measurement[N]) Exemplar(dest *metricdata.Exemplar[N]) { From 27bc3e7ce09dbe444e8ce0311ba118bc3604a242 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 19 Jan 2024 10:17:01 -0800 Subject: [PATCH 17/24] Doc fixed.go --- sdk/metric/internal/exemplar/fixed.go | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/sdk/metric/internal/exemplar/fixed.go b/sdk/metric/internal/exemplar/fixed.go index 45430a9013d..dbd3c7b2b6b 100644 --- a/sdk/metric/internal/exemplar/fixed.go +++ b/sdk/metric/internal/exemplar/fixed.go @@ -23,6 +23,7 @@ import ( "go.opentelemetry.io/otel/trace" ) +// fixedRes is the storage for [Reservoir] implementations. type fixedRes[N int64 | float64] struct { // store are the measurements sampled. // @@ -76,9 +77,13 @@ func (r *fixedRes[N]) Flush(dest *[]metricdata.Exemplar[N]) { // measurement is a measurement made by a telemetry system. type measurement[N int64 | float64] struct { - Attributes []attribute.KeyValue - Time time.Time - Value N + // FilteredAttributes are the attributes dropped during the measurement. + FilteredAttributes []attribute.KeyValue + // Time is the time when the measurement was made. + Time time.Time + // Value is the value of the measurement. + Value N + // SpanContext is the SpanContext active when a measurement was made. SpanContext trace.SpanContext valid bool @@ -87,11 +92,11 @@ type measurement[N int64 | float64] struct { // newMeasurement returns a new non-empty Measurement. func newMeasurement[N int64 | float64](ctx context.Context, ts time.Time, v N, droppedAttr []attribute.KeyValue) measurement[N] { return measurement[N]{ - Attributes: droppedAttr, - Time: ts, - Value: v, - SpanContext: trace.SpanContextFromContext(ctx), - valid: true, + FilteredAttributes: droppedAttr, + Time: ts, + Value: v, + SpanContext: trace.SpanContextFromContext(ctx), + valid: true, } } @@ -101,7 +106,7 @@ func (m measurement[N]) Valid() bool { return m.valid } // Exemplar returns m as a [metricdata.Exemplar]. func (m measurement[N]) Exemplar(dest *metricdata.Exemplar[N]) { - dest.FilteredAttributes = m.Attributes + dest.FilteredAttributes = m.FilteredAttributes dest.Time = m.Time dest.Value = m.Value From b044f348b0d353ed5538a9513596829dfda0d100 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 19 Jan 2024 10:17:30 -0800 Subject: [PATCH 18/24] Doc rand.go --- sdk/metric/internal/exemplar/rand.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/sdk/metric/internal/exemplar/rand.go b/sdk/metric/internal/exemplar/rand.go index 12df46004dc..0b6b932bec4 100644 --- a/sdk/metric/internal/exemplar/rand.go +++ b/sdk/metric/internal/exemplar/rand.go @@ -24,13 +24,16 @@ import ( "go.opentelemetry.io/otel/sdk/metric/metricdata" ) +// rng is used to make sampling decisions. +// +// Do not use crypto/rand. There is no reason for the decrease in performance +// given this is not a security sensitive decision. var rng = rand.New(rand.NewSource(time.Now().UnixNano())) // FixedSize returns a [Reservoir] that samples at most n exemplars. If there -// are n or less number of measurements made, the Reservoir will sample each -// one. If there are more than n number of measurements made, the Reservoir -// will then randomly sample all additional measurement with a decreasing -// probability. +// are n or less measurements made, the Reservoir will sample each one. If +// there are more than n, the Reservoir will then randomly sample all +// additional measurement with a decreasing probability. func FixedSize[N int64 | float64](n int) Reservoir[N] { r := &randRes[N]{fixedRes: newFixedRes[N](n)} r.reset() From b729954e29e7c9b864a7e857b2b73d99f58d87b0 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 19 Jan 2024 10:17:42 -0800 Subject: [PATCH 19/24] Fix doc for Reservoir --- sdk/metric/internal/exemplar/reservoir.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/sdk/metric/internal/exemplar/reservoir.go b/sdk/metric/internal/exemplar/reservoir.go index 302acee5527..b3654414ee7 100644 --- a/sdk/metric/internal/exemplar/reservoir.go +++ b/sdk/metric/internal/exemplar/reservoir.go @@ -37,15 +37,13 @@ type Reservoir[N int64 | float64] interface { // measurement respectively. Offer(ctx context.Context, t time.Time, val N, attr []attribute.KeyValue) - // Collect returns all the held exemplars with each exemplars dropped - // attributes updated to include any attributes the Filter filters out. + // Collect returns all the held exemplars. // // The Reservoir state is preserved after this call. See Flush to // copy-and-clear instead. Collect(dest *[]metricdata.Exemplar[N]) - // Flush returns all the held exemplars with each exemplars dropped - // attributes updated to include any attributes the Filter filters out. + // Flush returns all the held exemplars. // // The Reservoir state is reset after this call. See Collect to preserve // the state instead. From 6d2dfaa63e6208fe9c9335685b9f420ecf3ce6b7 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 19 Jan 2024 10:17:58 -0800 Subject: [PATCH 20/24] Move adminTrue to only use scope --- sdk/metric/internal/exemplar/reservoir_test.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/sdk/metric/internal/exemplar/reservoir_test.go b/sdk/metric/internal/exemplar/reservoir_test.go index 4ba169cacd7..3f0ad070603 100644 --- a/sdk/metric/internal/exemplar/reservoir_test.go +++ b/sdk/metric/internal/exemplar/reservoir_test.go @@ -27,12 +27,8 @@ import ( "go.opentelemetry.io/otel/trace" ) -var ( - adminTrue = attribute.Bool("admin", true) - - // Sat Jan 01 2000 00:00:00 GMT+0000. - staticTime = time.Unix(946684800, 0) -) +// Sat Jan 01 2000 00:00:00 GMT+0000. +var staticTime = time.Unix(946684800, 0) type factory[N int64 | float64] func(requstedCap int) (r Reservoir[N], actualCap int) @@ -81,6 +77,7 @@ func testReservoir[N int64 | float64](f factory[N]) func(*testing.T) { t.Skip("skipping, reservoir capacity less than 1:", n) } + adminTrue := attribute.Bool("admin", true) r.Offer(ctx, staticTime, 10, []attribute.KeyValue{adminTrue}) var dest []metricdata.Exemplar[N] From 3b4a1b58d0294173ed70f3534ee0700f4b73d84f Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 19 Jan 2024 10:30:46 -0800 Subject: [PATCH 21/24] Add internal/exemplar/doc.go --- sdk/metric/internal/exemplar/doc.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 sdk/metric/internal/exemplar/doc.go diff --git a/sdk/metric/internal/exemplar/doc.go b/sdk/metric/internal/exemplar/doc.go new file mode 100644 index 00000000000..3caeb542c57 --- /dev/null +++ b/sdk/metric/internal/exemplar/doc.go @@ -0,0 +1,17 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package exemplar provides an implementation of the OpenTelemetry exemplar +// reservoir to be used in metric collection pipelines. +package exemplar // import "go.opentelemetry.io/otel/sdk/metric/internal/exemplar" From 446038928fcf43c9040bf24e41533f408cd9c8c6 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 19 Jan 2024 10:34:57 -0800 Subject: [PATCH 22/24] Add link to res sampling comparison repo --- sdk/metric/internal/exemplar/rand.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sdk/metric/internal/exemplar/rand.go b/sdk/metric/internal/exemplar/rand.go index 0b6b932bec4..85c6c306576 100644 --- a/sdk/metric/internal/exemplar/rand.go +++ b/sdk/metric/internal/exemplar/rand.go @@ -67,6 +67,9 @@ func (r *randRes[N]) Offer(ctx context.Context, t time.Time, n N, a []attribute. // Vitter, Jeffrey S. (1 March 1985). "Random sampling with a reservoir" // (http://www.cs.umd.edu/~samir/498/vitter.pdf)) which has an asymptotic // runtime of O(n). + // + // See https://github.com/MrAlias/reservoir-sampling for a comparison of + // reservoir sampling algorithms (including performance benchmarks). if int(r.count) < cap(r.store) { r.store[r.count] = newMeasurement(ctx, t, n, a) From 925675f4040db7d572bee218c53fdfd084b1a644 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 19 Jan 2024 11:03:26 -0800 Subject: [PATCH 23/24] Rename fixedRes to storage --- sdk/metric/internal/exemplar/fixed.go | 12 ++++++------ sdk/metric/internal/exemplar/hist.go | 4 ++-- sdk/metric/internal/exemplar/rand.go | 4 ++-- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/sdk/metric/internal/exemplar/fixed.go b/sdk/metric/internal/exemplar/fixed.go index dbd3c7b2b6b..52c4ba13b74 100644 --- a/sdk/metric/internal/exemplar/fixed.go +++ b/sdk/metric/internal/exemplar/fixed.go @@ -23,8 +23,8 @@ import ( "go.opentelemetry.io/otel/trace" ) -// fixedRes is the storage for [Reservoir] implementations. -type fixedRes[N int64 | float64] struct { +// storage is an exemplar storage for [Reservoir] implementations. +type storage[N int64 | float64] struct { // store are the measurements sampled. // // This does not use []metricdata.Exemplar because it potentially would @@ -32,15 +32,15 @@ type fixedRes[N int64 | float64] struct { store []measurement[N] } -func newFixedRes[N int64 | float64](n int) *fixedRes[N] { - return &fixedRes[N]{store: make([]measurement[N], n)} +func newStorage[N int64 | float64](n int) *storage[N] { + return &storage[N]{store: make([]measurement[N], n)} } // Collect returns all the held exemplars. // // The Reservoir state is preserved after this call. See Flush to // copy-and-clear instead. -func (r *fixedRes[N]) Collect(dest *[]metricdata.Exemplar[N]) { +func (r *storage[N]) Collect(dest *[]metricdata.Exemplar[N]) { *dest = reset(*dest, len(r.store), len(r.store)) var n int for _, m := range r.store { @@ -58,7 +58,7 @@ func (r *fixedRes[N]) Collect(dest *[]metricdata.Exemplar[N]) { // // The Reservoir state is reset after this call. See Collect to preserve the // state instead. -func (r *fixedRes[N]) Flush(dest *[]metricdata.Exemplar[N]) { +func (r *storage[N]) Flush(dest *[]metricdata.Exemplar[N]) { *dest = reset(*dest, len(r.store), len(r.store)) var n int for i, m := range r.store { diff --git a/sdk/metric/internal/exemplar/hist.go b/sdk/metric/internal/exemplar/hist.go index 4b1e0645339..4761cdbd18f 100644 --- a/sdk/metric/internal/exemplar/hist.go +++ b/sdk/metric/internal/exemplar/hist.go @@ -31,12 +31,12 @@ func Histogram[N int64 | float64](bounds []float64) Reservoir[N] { sort.Float64s(bounds) return &histRes[N]{ bounds: bounds, - fixedRes: newFixedRes[N](len(bounds) + 1), + fixedRes: newStorage[N](len(bounds) + 1), } } type histRes[N int64 | float64] struct { - *fixedRes[N] + *storage[N] // bounds are bucket bounds in ascending order. bounds []float64 diff --git a/sdk/metric/internal/exemplar/rand.go b/sdk/metric/internal/exemplar/rand.go index 85c6c306576..5195397aa24 100644 --- a/sdk/metric/internal/exemplar/rand.go +++ b/sdk/metric/internal/exemplar/rand.go @@ -35,13 +35,13 @@ var rng = rand.New(rand.NewSource(time.Now().UnixNano())) // there are more than n, the Reservoir will then randomly sample all // additional measurement with a decreasing probability. func FixedSize[N int64 | float64](n int) Reservoir[N] { - r := &randRes[N]{fixedRes: newFixedRes[N](n)} + r := &randRes[N]{fixedRes: newStorage[N](n)} r.reset() return r } type randRes[N int64 | float64] struct { - *fixedRes[N] + *storage[N] // count is the number of measurement seen. count int64 From bb1c495b2e4d22fb7f231365d7bd523d715a5286 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 19 Jan 2024 11:10:35 -0800 Subject: [PATCH 24/24] Rename testReservoir to ReservoirTest --- sdk/metric/internal/exemplar/drop_test.go | 4 ++-- sdk/metric/internal/exemplar/hist_test.go | 4 ++-- sdk/metric/internal/exemplar/rand_test.go | 4 ++-- sdk/metric/internal/exemplar/reservoir_test.go | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/sdk/metric/internal/exemplar/drop_test.go b/sdk/metric/internal/exemplar/drop_test.go index dc874502723..5b02bf09437 100644 --- a/sdk/metric/internal/exemplar/drop_test.go +++ b/sdk/metric/internal/exemplar/drop_test.go @@ -19,11 +19,11 @@ import ( ) func TestDrop(t *testing.T) { - t.Run("Int64", testReservoir[int64](func(int) (Reservoir[int64], int) { + t.Run("Int64", ReservoirTest[int64](func(int) (Reservoir[int64], int) { return Drop[int64](), 0 })) - t.Run("Float64", testReservoir[float64](func(int) (Reservoir[float64], int) { + t.Run("Float64", ReservoirTest[float64](func(int) (Reservoir[float64], int) { return Drop[float64](), 0 })) } diff --git a/sdk/metric/internal/exemplar/hist_test.go b/sdk/metric/internal/exemplar/hist_test.go index bf6b766ad70..c85694db78c 100644 --- a/sdk/metric/internal/exemplar/hist_test.go +++ b/sdk/metric/internal/exemplar/hist_test.go @@ -18,11 +18,11 @@ import "testing" func TestHist(t *testing.T) { bounds := []float64{0, 100} - t.Run("Int64", testReservoir[int64](func(int) (Reservoir[int64], int) { + t.Run("Int64", ReservoirTest[int64](func(int) (Reservoir[int64], int) { return Histogram[int64](bounds), len(bounds) })) - t.Run("Float64", testReservoir[float64](func(int) (Reservoir[float64], int) { + t.Run("Float64", ReservoirTest[float64](func(int) (Reservoir[float64], int) { return Histogram[float64](bounds), len(bounds) })) } diff --git a/sdk/metric/internal/exemplar/rand_test.go b/sdk/metric/internal/exemplar/rand_test.go index 8f0df8ba43c..775679db547 100644 --- a/sdk/metric/internal/exemplar/rand_test.go +++ b/sdk/metric/internal/exemplar/rand_test.go @@ -24,11 +24,11 @@ import ( ) func TestFixedSize(t *testing.T) { - t.Run("Int64", testReservoir[int64](func(n int) (Reservoir[int64], int) { + t.Run("Int64", ReservoirTest[int64](func(n int) (Reservoir[int64], int) { return FixedSize[int64](n), n })) - t.Run("Float64", testReservoir[float64](func(n int) (Reservoir[float64], int) { + t.Run("Float64", ReservoirTest[float64](func(n int) (Reservoir[float64], int) { return FixedSize[float64](n), n })) } diff --git a/sdk/metric/internal/exemplar/reservoir_test.go b/sdk/metric/internal/exemplar/reservoir_test.go index 3f0ad070603..9ef8e964538 100644 --- a/sdk/metric/internal/exemplar/reservoir_test.go +++ b/sdk/metric/internal/exemplar/reservoir_test.go @@ -32,7 +32,7 @@ var staticTime = time.Unix(946684800, 0) type factory[N int64 | float64] func(requstedCap int) (r Reservoir[N], actualCap int) -func testReservoir[N int64 | float64](f factory[N]) func(*testing.T) { +func ReservoirTest[N int64 | float64](f factory[N]) func(*testing.T) { return func(t *testing.T) { t.Helper()