From 242d23a1815d00f228e6767e4baea9019fefe50a Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Wed, 31 Jan 2024 14:07:40 -0800 Subject: [PATCH] Remove the Flush method from Exemplar (#4873) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Robert Pająk --- sdk/metric/internal/exemplar/drop.go | 5 -- sdk/metric/internal/exemplar/filter_test.go | 8 --- sdk/metric/internal/exemplar/rand.go | 5 -- sdk/metric/internal/exemplar/reservoir.go | 9 +--- .../internal/exemplar/reservoir_test.go | 50 ++----------------- sdk/metric/internal/exemplar/storage.go | 24 +-------- 6 files changed, 6 insertions(+), 95 deletions(-) diff --git a/sdk/metric/internal/exemplar/drop.go b/sdk/metric/internal/exemplar/drop.go index 62a95b55f9a..39bf37b9e94 100644 --- a/sdk/metric/internal/exemplar/drop.go +++ b/sdk/metric/internal/exemplar/drop.go @@ -34,8 +34,3 @@ func (r *dropRes[N]) Offer(context.Context, time.Time, N, []attribute.KeyValue) 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] -} diff --git a/sdk/metric/internal/exemplar/filter_test.go b/sdk/metric/internal/exemplar/filter_test.go index ae1e276cb83..e32d03c2352 100644 --- a/sdk/metric/internal/exemplar/filter_test.go +++ b/sdk/metric/internal/exemplar/filter_test.go @@ -44,9 +44,6 @@ func testSampledFiltered[N int64 | float64](t *testing.T) { 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 { @@ -61,7 +58,6 @@ func sample(parent context.Context) context.Context { type res[N int64 | float64] struct { OfferCalled bool CollectCalled bool - FlushCalled bool } func (r *res[N]) Offer(context.Context, time.Time, N, []attribute.KeyValue) { @@ -71,7 +67,3 @@ func (r *res[N]) Offer(context.Context, time.Time, N, []attribute.KeyValue) { func (r *res[N]) Collect(*[]metricdata.Exemplar[N]) { r.CollectCalled = true } - -func (r *res[N]) Flush(*[]metricdata.Exemplar[N]) { - r.FlushCalled = true -} diff --git a/sdk/metric/internal/exemplar/rand.go b/sdk/metric/internal/exemplar/rand.go index aa2c2f64047..7f9fda5b489 100644 --- a/sdk/metric/internal/exemplar/rand.go +++ b/sdk/metric/internal/exemplar/rand.go @@ -193,8 +193,3 @@ func (r *randRes[N]) Collect(dest *[]metricdata.Exemplar[N]) { // measurements that are made over the older collection cycle ones. r.reset() } - -func (r *randRes[N]) Flush(dest *[]metricdata.Exemplar[N]) { - r.storage.Flush(dest) - r.reset() -} diff --git a/sdk/metric/internal/exemplar/reservoir.go b/sdk/metric/internal/exemplar/reservoir.go index b3654414ee7..7d5276a3411 100644 --- a/sdk/metric/internal/exemplar/reservoir.go +++ b/sdk/metric/internal/exemplar/reservoir.go @@ -39,13 +39,6 @@ type Reservoir[N int64 | float64] interface { // Collect returns all the held exemplars. // - // The Reservoir state is preserved after this call. See Flush to - // copy-and-clear instead. + // The Reservoir state is preserved after this call. Collect(dest *[]metricdata.Exemplar[N]) - - // Flush returns all the held exemplars. - // - // The Reservoir state is reset after this call. See Collect to preserve - // the state instead. - Flush(dest *[]metricdata.Exemplar[N]) } diff --git a/sdk/metric/internal/exemplar/reservoir_test.go b/sdk/metric/internal/exemplar/reservoir_test.go index 41c90b7fed7..65c179dc106 100644 --- a/sdk/metric/internal/exemplar/reservoir_test.go +++ b/sdk/metric/internal/exemplar/reservoir_test.go @@ -92,25 +92,6 @@ func ReservoirTest[N int64 | float64](f factory[N]) func(*testing.T) { 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, nil) - - var dest []metricdata.Exemplar[N] - r.Collect(&dest) - require.Len(t, dest, 1, "number of collected exemplars") - - dest = dest[:0] - r.Collect(&dest) - assert.Len(t, dest, 1, "Collect flushed reservoir") - }) - t.Run("CollectLessThanN", func(t *testing.T) { t.Helper() @@ -127,24 +108,6 @@ func ReservoirTest[N int64 | float64](f factory[N]) func(*testing.T) { require.Len(t, dest, 1, "number of collected exemplars") }) - 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, nil) - - var dest []metricdata.Exemplar[N] - r.Flush(&dest) - require.Len(t, dest, 1, "number of flushed exemplars") - - r.Flush(&dest) - assert.Len(t, dest, 0, "Flush did not flush reservoir") - }) - t.Run("MultipleOffers", func(t *testing.T) { t.Helper() @@ -159,17 +122,17 @@ func ReservoirTest[N int64 | float64](f factory[N]) func(*testing.T) { } var dest []metricdata.Exemplar[N] - r.Flush(&dest) + r.Collect(&dest) assert.Len(t, dest, n, "multiple offers did not fill reservoir") - // Ensure the flush reset also resets any couting state. + // Ensure the collect reset also resets any couting state. for i := 0; i < n+1; i++ { - v := N(2 * i) + v := N(i) r.Offer(ctx, staticTime, v, nil) } dest = dest[:0] - r.Flush(&dest) + r.Collect(&dest) assert.Len(t, dest, n, "internal count state not reset") }) @@ -186,11 +149,6 @@ func ReservoirTest[N int64 | float64](f factory[N]) func(*testing.T) { dest := []metricdata.Exemplar[N]{{}} // Should be reset to empty. r.Collect(&dest) assert.Len(t, dest, 0, "no exemplars should be collected") - - r.Offer(context.Background(), staticTime, 10, nil) - dest = []metricdata.Exemplar[N]{{}} // Should be reset to empty. - r.Flush(&dest) - assert.Len(t, dest, 0, "no exemplars should be flushed") }) } } diff --git a/sdk/metric/internal/exemplar/storage.go b/sdk/metric/internal/exemplar/storage.go index 460b8196d06..e2c2b90a351 100644 --- a/sdk/metric/internal/exemplar/storage.go +++ b/sdk/metric/internal/exemplar/storage.go @@ -38,8 +38,7 @@ func newStorage[N int64 | float64](n int) *storage[N] { // Collect returns all the held exemplars. // -// The Reservoir state is preserved after this call. See Flush to -// copy-and-clear instead. +// The Reservoir state is preserved after this call. func (r *storage[N]) Collect(dest *[]metricdata.Exemplar[N]) { *dest = reset(*dest, len(r.store), len(r.store)) var n int @@ -54,27 +53,6 @@ func (r *storage[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 *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 { - if !m.valid { - continue - } - - m.Exemplar(&(*dest)[n]) - 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 { // FilteredAttributes are the attributes dropped during the measurement.