From 7fc85cadbd9a664b1464cf13c3937c9c91d5e3b2 Mon Sep 17 00:00:00 2001 From: Josh MacDonald Date: Fri, 18 Dec 2020 01:26:17 -0800 Subject: [PATCH 01/15] Remove quantile definition --- sdk/export/metric/aggregation/aggregation.go | 31 +-- sdk/go.mod | 1 - sdk/go.sum | 1 + sdk/metric/aggregator/array/array.go | 129 ++-------- sdk/metric/aggregator/array/array_test.go | 96 ++------ sdk/metric/aggregator/ddsketch/ddsketch.go | 155 ------------ .../aggregator/ddsketch/ddsketch_test.go | 221 ------------------ sdk/metric/correct_test.go | 2 +- sdk/metric/processor/basic/basic_test.go | 1 - sdk/metric/processor/processortest/test.go | 6 - sdk/metric/selector/simple/simple.go | 32 +-- sdk/metric/selector/simple/simple_test.go | 7 - 12 files changed, 53 insertions(+), 629 deletions(-) delete mode 100644 sdk/metric/aggregator/ddsketch/ddsketch.go delete mode 100644 sdk/metric/aggregator/ddsketch/ddsketch_test.go diff --git a/sdk/export/metric/aggregation/aggregation.go b/sdk/export/metric/aggregation/aggregation.go index 7f76b0baeb9..b62a36144c3 100644 --- a/sdk/export/metric/aggregation/aggregation.go +++ b/sdk/export/metric/aggregation/aggregation.go @@ -58,13 +58,6 @@ type ( Max() (number.Number, error) } - // Quantile returns an exact or estimated quantile over the - // set of values that were aggregated. - Quantile interface { - Aggregation - Quantile(float64) (number.Number, error) - } - // LastValue returns the latest value that was aggregated. LastValue interface { Aggregation @@ -74,7 +67,17 @@ type ( // Points returns the raw set of values that were aggregated. Points interface { Aggregation - Points() ([]number.Number, error) + Points() (Samples, error) + } + + // Samples is a set of samples. They implement the + // sort.Interface, sorting by arrival time. + Samples []Sample + + // Sample is a point + Sample struct { + number.Number + time.Time } // Buckets represents histogram buckets boundaries and counts. @@ -108,17 +111,6 @@ type ( Sum() (number.Number, error) Count() (int64, error) } - - // Distribution supports the Min, Max, Sum, Count, and Quantile - // interfaces. - Distribution interface { - Aggregation - Min() (number.Number, error) - Max() (number.Number, error) - Sum() (number.Number, error) - Count() (int64, error) - Quantile(float64) (number.Number, error) - } ) type ( @@ -143,7 +135,6 @@ const ( MinMaxSumCountKind Kind = "MinMaxSumCount" HistogramKind Kind = "Histogram" LastValueKind Kind = "Lastvalue" - SketchKind Kind = "Sketch" ExactKind Kind = "Exact" ) diff --git a/sdk/go.mod b/sdk/go.mod index ea3b45b2fd0..fe5d3cb79b3 100644 --- a/sdk/go.mod +++ b/sdk/go.mod @@ -5,7 +5,6 @@ go 1.14 replace go.opentelemetry.io/otel => ../ require ( - github.com/DataDog/sketches-go v0.0.1 github.com/benbjohnson/clock v1.0.3 github.com/google/go-cmp v0.5.4 github.com/google/gofuzz v1.1.0 // indirect diff --git a/sdk/go.sum b/sdk/go.sum index ae3c891c195..6bd1abfe0af 100644 --- a/sdk/go.sum +++ b/sdk/go.sum @@ -13,6 +13,7 @@ github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZN github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd0= github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +go.opentelemetry.io v0.1.0 h1:EANZoRCOP+A3faIlw/iN6YEWoYb1vleZRKm1EvH8T48= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= diff --git a/sdk/metric/aggregator/array/array.go b/sdk/metric/aggregator/array/array.go index 1ac1d82a5f1..ebaf078279c 100644 --- a/sdk/metric/aggregator/array/array.go +++ b/sdk/metric/aggregator/array/array.go @@ -16,10 +16,8 @@ package array // import "go.opentelemetry.io/otel/sdk/metric/aggregator/array" import ( "context" - "math" - "sort" "sync" - "unsafe" + "time" "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/metric/number" @@ -32,18 +30,14 @@ type ( // Aggregator aggregates events that form a distribution, keeping // an array with the exact set of values. Aggregator struct { - lock sync.Mutex - sum number.Number - points points + lock sync.Mutex + samples aggregation.Samples } - - points []number.Number ) var _ export.Aggregator = &Aggregator{} -var _ aggregation.MinMaxSumCount = &Aggregator{} -var _ aggregation.Distribution = &Aggregator{} var _ aggregation.Points = &Aggregator{} +var _ aggregation.Count = &Aggregator{} // New returns a new array aggregator, which aggregates recorded // measurements by storing them in an array. This type uses a mutex @@ -62,35 +56,14 @@ func (c *Aggregator) Kind() aggregation.Kind { return aggregation.ExactKind } -// Sum returns the sum of values in the checkpoint. -func (c *Aggregator) Sum() (number.Number, error) { - return c.sum, nil -} - // Count returns the number of values in the checkpoint. func (c *Aggregator) Count() (int64, error) { - return int64(len(c.points)), nil -} - -// Max returns the maximum value in the checkpoint. -func (c *Aggregator) Max() (number.Number, error) { - return c.points.Quantile(1) -} - -// Min returns the mininum value in the checkpoint. -func (c *Aggregator) Min() (number.Number, error) { - return c.points.Quantile(0) -} - -// Quantile returns the estimated quantile of data in the checkpoint. -// It is an error if `q` is less than 0 or greated than 1. -func (c *Aggregator) Quantile(q float64) (number.Number, error) { - return c.points.Quantile(q) + return int64(len(c.samples)), nil } // Points returns access to the raw data set. -func (c *Aggregator) Points() ([]number.Number, error) { - return c.points, nil +func (c *Aggregator) Points() (aggregation.Samples, error) { + return c.samples, nil } // SynchronizedMove saves the current state to oa and resets the current state to @@ -103,21 +76,13 @@ func (c *Aggregator) SynchronizedMove(oa export.Aggregator, desc *metric.Descrip } c.lock.Lock() - if o != nil { - o.points = c.points - o.sum = c.sum - } - c.points = nil - c.sum = 0 - c.lock.Unlock() + defer c.lock.Unlock() - // TODO: This sort should be done lazily, only when quantiles - // are requested. The SDK specification says you can use this - // aggregator to simply list values in the order they were - // received as an alternative to requesting quantile information. if o != nil { - o.sort(desc.NumberKind()) + o.samples = c.samples } + c.samples = nil + return nil } @@ -125,9 +90,12 @@ func (c *Aggregator) SynchronizedMove(oa export.Aggregator, desc *metric.Descrip // Update takes a lock to prevent concurrent Update() and SynchronizedMove() // calls. func (c *Aggregator) Update(_ context.Context, number number.Number, desc *metric.Descriptor) error { + now := time.Now() c.lock.Lock() - c.points = append(c.points, number) - c.sum.AddNumber(desc.NumberKind(), number) + c.samples = append(c.samples, aggregation.Sample{ + Number: number, + Time: now, + }) c.lock.Unlock() return nil @@ -140,34 +108,15 @@ func (c *Aggregator) Merge(oa export.Aggregator, desc *metric.Descriptor) error return aggregator.NewInconsistentAggregatorError(c, oa) } - // Note: Current assumption is that `o` was checkpointed, - // therefore is already sorted. See the TODO above, since - // this is an open question. - c.sum.AddNumber(desc.NumberKind(), o.sum) - c.points = combine(c.points, o.points, desc.NumberKind()) + c.samples = combine(c.samples, o.samples) return nil } -func (c *Aggregator) sort(kind number.Kind) { - switch kind { - case number.Float64Kind: - sort.Float64s(*(*[]float64)(unsafe.Pointer(&c.points))) - - case number.Int64Kind: - sort.Sort(&c.points) - - default: - // NOTE: This can't happen because the SDK doesn't - // support uint64-kind metric instruments. - panic("Impossible case") - } -} - -func combine(a, b points, kind number.Kind) points { - result := make(points, 0, len(a)+len(b)) +func combine(a, b aggregation.Samples) aggregation.Samples { + result := make(aggregation.Samples, 0, len(a)+len(b)) for len(a) != 0 && len(b) != 0 { - if a[0].CompareNumber(kind, b[0]) < 0 { + if a[0].Time.Before(b[0].Time) { result = append(result, a[0]) a = a[1:] } else { @@ -179,41 +128,3 @@ func combine(a, b points, kind number.Kind) points { result = append(result, b...) return result } - -func (p *points) Len() int { - return len(*p) -} - -func (p *points) Less(i, j int) bool { - // Note this is specialized for int64, because float64 is - // handled by `sort.Float64s` and uint64 numbers never appear - // in this data. - return int64((*p)[i]) < int64((*p)[j]) -} - -func (p *points) Swap(i, j int) { - (*p)[i], (*p)[j] = (*p)[j], (*p)[i] -} - -// Quantile returns the least X such that Pr(x=q, where X is an -// element of the data set. This uses the "Nearest-Rank" definition -// of a quantile. -func (p *points) Quantile(q float64) (number.Number, error) { - if len(*p) == 0 { - return 0, aggregation.ErrNoData - } - - if q < 0 || q > 1 { - return 0, aggregation.ErrInvalidQuantile - } - - if q == 0 || len(*p) == 1 { - return (*p)[0], nil - } else if q == 1 { - return (*p)[len(*p)-1], nil - } - - position := float64(len(*p)-1) * q - ceil := int(math.Ceil(position)) - return (*p)[ceil], nil -} diff --git a/sdk/metric/aggregator/array/array_test.go b/sdk/metric/aggregator/array/array_test.go index e9ba40b7001..6c3dfd3686d 100644 --- a/sdk/metric/aggregator/array/array_test.go +++ b/sdk/metric/aggregator/array/array_test.go @@ -15,7 +15,6 @@ package array import ( - "errors" "fmt" "math" "testing" @@ -34,23 +33,13 @@ type updateTest struct { } func checkZero(t *testing.T, agg *Aggregator, desc *metric.Descriptor) { - kind := desc.NumberKind() - - sum, err := agg.Sum() - require.NoError(t, err) - require.Equal(t, kind.Zero(), sum) - count, err := agg.Count() require.NoError(t, err) require.Equal(t, int64(0), count) - max, err := agg.Max() - require.True(t, errors.Is(err, aggregation.ErrNoData)) - require.Equal(t, kind.Zero(), max) - - min, err := agg.Min() - require.True(t, errors.Is(err, aggregation.ErrNoData)) - require.Equal(t, kind.Zero(), min) + pts, err := agg.Points() + require.NoError(t, err) + require.Equal(t, nil, pts) } func new2() (_, _ *Aggregator) { @@ -63,6 +52,14 @@ func new4() (_, _, _, _ *Aggregator) { return &alloc[0], &alloc[1], &alloc[2], &alloc[3] } +func sumOf(samples aggregation.Samples, k number.Kind) number.Number { + var n number.Number + for _, s := range samples { + n.AddNumber(k, s.Number) + } + return n +} + func (ut *updateTest) run(t *testing.T, profile aggregatortest.Profile) { descriptor := aggregatortest.NewAggregatorTest(metric.ValueRecorderInstrumentKind, profile.NumberKind) agg, ckpt := new2() @@ -86,8 +83,9 @@ func (ut *updateTest) run(t *testing.T, profile aggregatortest.Profile) { all.Sort() - sum, err := ckpt.Sum() + pts, err := ckpt.Points() require.Nil(t, err) + sum := sumOf(pts, profile.NumberKind) allSum := all.Sum() require.InEpsilon(t, (&allSum).CoerceToFloat64(profile.NumberKind), @@ -97,18 +95,6 @@ func (ut *updateTest) run(t *testing.T, profile aggregatortest.Profile) { count, err := ckpt.Count() require.Nil(t, err) require.Equal(t, all.Count(), count, "Same count") - - min, err := ckpt.Min() - require.Nil(t, err) - require.Equal(t, all.Min(), min, "Same min") - - max, err := ckpt.Max() - require.Nil(t, err) - require.Equal(t, all.Max(), max, "Same max") - - qx, err := ckpt.Quantile(0.5) - require.Nil(t, err) - require.Equal(t, all.Median(), qx, "Same median") } func TestArrayUpdate(t *testing.T) { @@ -166,9 +152,11 @@ func (mt *mergeTest) run(t *testing.T, profile aggregatortest.Profile) { all.Sort() - sum, err := ckpt1.Sum() + pts, err := ckpt1.Points() require.Nil(t, err) + allSum := all.Sum() + sum := sumOf(pts, profile.NumberKind) require.InEpsilon(t, (&allSum).CoerceToFloat64(profile.NumberKind), sum.CoerceToFloat64(profile.NumberKind), @@ -177,18 +165,6 @@ func (mt *mergeTest) run(t *testing.T, profile aggregatortest.Profile) { count, err := ckpt1.Count() require.Nil(t, err) require.Equal(t, all.Count(), count, "Same count - absolute") - - min, err := ckpt1.Min() - require.Nil(t, err) - require.Equal(t, all.Min(), min, "Same min - absolute") - - max, err := ckpt1.Max() - require.Nil(t, err) - require.Equal(t, all.Max(), max, "Same max - absolute") - - qx, err := ckpt1.Quantile(0.5) - require.Nil(t, err) - require.Equal(t, all.Median(), qx, "Same median - absolute") } func TestArrayMerge(t *testing.T) { @@ -215,18 +191,6 @@ func TestArrayErrors(t *testing.T) { aggregatortest.RunProfiles(t, func(t *testing.T, profile aggregatortest.Profile) { agg, ckpt := new2() - _, err := ckpt.Max() - require.Error(t, err) - require.Equal(t, err, aggregation.ErrNoData) - - _, err = ckpt.Min() - require.Error(t, err) - require.Equal(t, err, aggregation.ErrNoData) - - _, err = ckpt.Quantile(0.1) - require.Error(t, err) - require.Equal(t, err, aggregation.ErrNoData) - descriptor := aggregatortest.NewAggregatorTest(metric.ValueRecorderInstrumentKind, profile.NumberKind) aggregatortest.CheckedUpdate(t, agg, number.Number(0), descriptor) @@ -239,18 +203,6 @@ func TestArrayErrors(t *testing.T) { count, err := ckpt.Count() require.Equal(t, int64(1), count, "NaN value was not counted") require.Nil(t, err) - - num, err := ckpt.Quantile(0) - require.Nil(t, err) - require.Equal(t, num, number.Number(0)) - - _, err = ckpt.Quantile(-0.0001) - require.Error(t, err) - require.True(t, errors.Is(err, aggregation.ErrInvalidQuantile)) - - _, err = agg.Quantile(1.0001) - require.Error(t, err) - require.True(t, errors.Is(err, aggregation.ErrNoData)) }) } @@ -302,27 +254,17 @@ func TestArrayFloat64(t *testing.T) { all.Sort() - sum, err := ckpt.Sum() + pts, err := ckpt.Points() require.Nil(t, err) + allSum := all.Sum() + sum := sumOf(pts, number.Float64Kind) require.InEpsilon(t, (&allSum).AsFloat64(), sum.AsFloat64(), 0.0000001, "Same sum") count, err := ckpt.Count() require.Equal(t, all.Count(), count, "Same count") require.Nil(t, err) - min, err := ckpt.Min() - require.Nil(t, err) - require.Equal(t, all.Min(), min, "Same min") - - max, err := ckpt.Max() - require.Nil(t, err) - require.Equal(t, all.Max(), max, "Same max") - - qx, err := ckpt.Quantile(0.5) - require.Nil(t, err) - require.Equal(t, all.Median(), qx, "Same median") - po, err := ckpt.Points() require.Nil(t, err) require.Equal(t, all.Len(), len(po), "Points() must have same length of updates") diff --git a/sdk/metric/aggregator/ddsketch/ddsketch.go b/sdk/metric/aggregator/ddsketch/ddsketch.go deleted file mode 100644 index cbd97a776cf..00000000000 --- a/sdk/metric/aggregator/ddsketch/ddsketch.go +++ /dev/null @@ -1,155 +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 ddsketch // import "go.opentelemetry.io/otel/sdk/metric/aggregator/ddsketch" - -import ( - "context" - "math" - "sync" - - sdk "github.com/DataDog/sketches-go/ddsketch" - - "go.opentelemetry.io/otel/metric" - "go.opentelemetry.io/otel/metric/number" - export "go.opentelemetry.io/otel/sdk/export/metric" - "go.opentelemetry.io/otel/sdk/export/metric/aggregation" - "go.opentelemetry.io/otel/sdk/metric/aggregator" -) - -// Config is an alias for the underlying DDSketch config object. -type Config = sdk.Config - -// Aggregator aggregates events into a distribution. -type Aggregator struct { - lock sync.Mutex - cfg *Config - kind number.Kind - sketch *sdk.DDSketch -} - -var _ export.Aggregator = &Aggregator{} -var _ aggregation.MinMaxSumCount = &Aggregator{} -var _ aggregation.Distribution = &Aggregator{} - -// New returns a new DDSketch aggregator. -func New(cnt int, desc *metric.Descriptor, cfg *Config) []Aggregator { - if cfg == nil { - cfg = NewDefaultConfig() - } - aggs := make([]Aggregator, cnt) - for i := range aggs { - aggs[i] = Aggregator{ - cfg: cfg, - kind: desc.NumberKind(), - sketch: sdk.NewDDSketch(cfg), - } - } - return aggs -} - -// Aggregation returns an interface for reading the state of this aggregator. -func (c *Aggregator) Aggregation() aggregation.Aggregation { - return c -} - -// Kind returns aggregation.SketchKind. -func (c *Aggregator) Kind() aggregation.Kind { - return aggregation.SketchKind -} - -// NewDefaultConfig returns a new, default DDSketch config. -func NewDefaultConfig() *Config { - return sdk.NewDefaultConfig() -} - -// Sum returns the sum of values in the checkpoint. -func (c *Aggregator) Sum() (number.Number, error) { - return c.toNumber(c.sketch.Sum()), nil -} - -// Count returns the number of values in the checkpoint. -func (c *Aggregator) Count() (int64, error) { - return c.sketch.Count(), nil -} - -// Max returns the maximum value in the checkpoint. -func (c *Aggregator) Max() (number.Number, error) { - return c.Quantile(1) -} - -// Min returns the minimum value in the checkpoint. -func (c *Aggregator) Min() (number.Number, error) { - return c.Quantile(0) -} - -// Quantile returns the estimated quantile of data in the checkpoint. -// It is an error if `q` is less than 0 or greated than 1. -func (c *Aggregator) Quantile(q float64) (number.Number, error) { - if c.sketch.Count() == 0 { - return 0, aggregation.ErrNoData - } - f := c.sketch.Quantile(q) - if math.IsNaN(f) { - return 0, aggregation.ErrInvalidQuantile - } - return c.toNumber(f), nil -} - -func (c *Aggregator) toNumber(f float64) number.Number { - if c.kind == number.Float64Kind { - return number.NewFloat64Number(f) - } - return number.NewInt64Number(int64(f)) -} - -// SynchronizedMove saves the current state into oa and resets the current state to -// a new sketch, taking a lock to prevent concurrent Update() calls. -func (c *Aggregator) SynchronizedMove(oa export.Aggregator, _ *metric.Descriptor) error { - o, _ := oa.(*Aggregator) - - if oa != nil && o == nil { - return aggregator.NewInconsistentAggregatorError(c, oa) - } - - replace := sdk.NewDDSketch(c.cfg) - c.lock.Lock() - if o != nil { - o.sketch = c.sketch - } - c.sketch = replace - c.lock.Unlock() - - return nil -} - -// Update adds the recorded measurement to the current data set. -// Update takes a lock to prevent concurrent Update() and SynchronizedMove() -// calls. -func (c *Aggregator) Update(_ context.Context, number number.Number, desc *metric.Descriptor) error { - c.lock.Lock() - defer c.lock.Unlock() - c.sketch.Add(number.CoerceToFloat64(desc.NumberKind())) - return nil -} - -// Merge combines two sketches into one. -func (c *Aggregator) Merge(oa export.Aggregator, d *metric.Descriptor) error { - o, _ := oa.(*Aggregator) - if o == nil { - return aggregator.NewInconsistentAggregatorError(c, oa) - } - - c.sketch.Merge(o.sketch) - return nil -} diff --git a/sdk/metric/aggregator/ddsketch/ddsketch_test.go b/sdk/metric/aggregator/ddsketch/ddsketch_test.go deleted file mode 100644 index d05ab7b1ad8..00000000000 --- a/sdk/metric/aggregator/ddsketch/ddsketch_test.go +++ /dev/null @@ -1,221 +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 ddsketch - -import ( - "errors" - "fmt" - "testing" - - "github.com/stretchr/testify/require" - - "go.opentelemetry.io/otel/metric" - export "go.opentelemetry.io/otel/sdk/export/metric" - "go.opentelemetry.io/otel/sdk/export/metric/aggregation" - "go.opentelemetry.io/otel/sdk/metric/aggregator/aggregatortest" -) - -const count = 1000 - -type updateTest struct { -} - -func new2(desc *metric.Descriptor) (_, _ *Aggregator) { - alloc := New(2, desc, NewDefaultConfig()) - return &alloc[0], &alloc[1] -} - -func new4(desc *metric.Descriptor) (_, _, _, _ *Aggregator) { - alloc := New(4, desc, NewDefaultConfig()) - return &alloc[0], &alloc[1], &alloc[2], &alloc[3] -} - -func checkZero(t *testing.T, agg *Aggregator, desc *metric.Descriptor) { - kind := desc.NumberKind() - - sum, err := agg.Sum() - require.NoError(t, err) - require.Equal(t, kind.Zero(), sum) - - count, err := agg.Count() - require.NoError(t, err) - require.Equal(t, int64(0), count) - - max, err := agg.Max() - require.True(t, errors.Is(err, aggregation.ErrNoData)) - require.Equal(t, kind.Zero(), max) - - median, err := agg.Quantile(0.5) - require.True(t, errors.Is(err, aggregation.ErrNoData)) - require.Equal(t, kind.Zero(), median) - - min, err := agg.Min() - require.True(t, errors.Is(err, aggregation.ErrNoData)) - require.Equal(t, kind.Zero(), min) -} - -func (ut *updateTest) run(t *testing.T, profile aggregatortest.Profile) { - descriptor := aggregatortest.NewAggregatorTest(metric.ValueRecorderInstrumentKind, profile.NumberKind) - agg, ckpt := new2(descriptor) - - all := aggregatortest.NewNumbers(profile.NumberKind) - for i := 0; i < count; i++ { - x := profile.Random(+1) - all.Append(x) - aggregatortest.CheckedUpdate(t, agg, x, descriptor) - - y := profile.Random(-1) - all.Append(y) - aggregatortest.CheckedUpdate(t, agg, y, descriptor) - } - - err := agg.SynchronizedMove(ckpt, descriptor) - require.NoError(t, err) - - checkZero(t, agg, descriptor) - - all.Sort() - - sum, err := ckpt.Sum() - require.Nil(t, err) - allSum := all.Sum() - require.InDelta(t, - (&allSum).CoerceToFloat64(profile.NumberKind), - sum.CoerceToFloat64(profile.NumberKind), - 1, - "Same sum") - - count, err := ckpt.Count() - require.Equal(t, all.Count(), count, "Same count") - require.Nil(t, err) - - max, err := ckpt.Max() - require.Nil(t, err) - require.Equal(t, - all.Max(), - max, - "Same max") - - median, err := ckpt.Quantile(0.5) - require.Nil(t, err) - allMedian := all.Median() - require.InDelta(t, - (&allMedian).CoerceToFloat64(profile.NumberKind), - median.CoerceToFloat64(profile.NumberKind), - 10, - "Same median") -} - -func TestDDSketchUpdate(t *testing.T) { - ut := updateTest{} - aggregatortest.RunProfiles(t, ut.run) -} - -type mergeTest struct { - absolute bool -} - -func (mt *mergeTest) run(t *testing.T, profile aggregatortest.Profile) { - descriptor := aggregatortest.NewAggregatorTest(metric.ValueRecorderInstrumentKind, profile.NumberKind) - - agg1, agg2, ckpt1, ckpt2 := new4(descriptor) - - all := aggregatortest.NewNumbers(profile.NumberKind) - for i := 0; i < count; i++ { - x := profile.Random(+1) - all.Append(x) - aggregatortest.CheckedUpdate(t, agg1, x, descriptor) - - if !mt.absolute { - y := profile.Random(-1) - all.Append(y) - aggregatortest.CheckedUpdate(t, agg1, y, descriptor) - } - } - - for i := 0; i < count; i++ { - x := profile.Random(+1) - all.Append(x) - aggregatortest.CheckedUpdate(t, agg2, x, descriptor) - - if !mt.absolute { - y := profile.Random(-1) - all.Append(y) - aggregatortest.CheckedUpdate(t, agg2, y, descriptor) - } - } - - require.NoError(t, agg1.SynchronizedMove(ckpt1, descriptor)) - require.NoError(t, agg2.SynchronizedMove(ckpt2, descriptor)) - - checkZero(t, agg1, descriptor) - checkZero(t, agg1, descriptor) - - aggregatortest.CheckedMerge(t, ckpt1, ckpt2, descriptor) - - all.Sort() - - aggSum, err := ckpt1.Sum() - require.Nil(t, err) - allSum := all.Sum() - require.InDelta(t, - (&allSum).CoerceToFloat64(profile.NumberKind), - aggSum.CoerceToFloat64(profile.NumberKind), - 1, - "Same sum") - - count, err := ckpt1.Count() - require.Equal(t, all.Count(), count, "Same count") - require.Nil(t, err) - - max, err := ckpt1.Max() - require.Nil(t, err) - require.Equal(t, - all.Max(), - max, - "Same max") - - median, err := ckpt1.Quantile(0.5) - require.Nil(t, err) - allMedian := all.Median() - require.InDelta(t, - (&allMedian).CoerceToFloat64(profile.NumberKind), - median.CoerceToFloat64(profile.NumberKind), - 10, - "Same median") -} - -func TestDDSketchMerge(t *testing.T) { - // Test absolute and non-absolute - for _, absolute := range []bool{false, true} { - t.Run(fmt.Sprint("Absolute=", absolute), func(t *testing.T) { - mt := mergeTest{ - absolute: absolute, - } - // Test integer and floating point - aggregatortest.RunProfiles(t, mt.run) - }) - } -} - -func TestSynchronizedMoveReset(t *testing.T) { - aggregatortest.SynchronizedMoveResetTest( - t, - metric.ValueRecorderInstrumentKind, - func(desc *metric.Descriptor) export.Aggregator { - return &New(1, desc, NewDefaultConfig())[0] - }, - ) -} diff --git a/sdk/metric/correct_test.go b/sdk/metric/correct_test.go index 5ccba31af82..f54603a56f5 100644 --- a/sdk/metric/correct_test.go +++ b/sdk/metric/correct_test.go @@ -167,7 +167,7 @@ func TestInputRangeValueRecorder(t *testing.T) { processor.accumulations = nil checkpointed = sdk.Collect(ctx) - count, err := processor.accumulations[0].Aggregator().(aggregation.Distribution).Count() + count, err := processor.accumulations[0].Aggregator().(aggregation.Count).Count() require.Equal(t, int64(2), count) require.Equal(t, 1, checkpointed) require.Nil(t, testHandler.Flush()) diff --git a/sdk/metric/processor/basic/basic_test.go b/sdk/metric/processor/basic/basic_test.go index 016de9677bd..11b59717ea5 100644 --- a/sdk/metric/processor/basic/basic_test.go +++ b/sdk/metric/processor/basic/basic_test.go @@ -77,7 +77,6 @@ func TestProcessor(t *testing.T) { {kind: aggregation.HistogramKind}, {kind: aggregation.LastValueKind}, {kind: aggregation.ExactKind}, - {kind: aggregation.SketchKind}, } { t.Run(ac.kind.String(), func(t *testing.T) { testProcessor( diff --git a/sdk/metric/processor/processortest/test.go b/sdk/metric/processor/processortest/test.go index 56d5de154f6..4f0f9b529b9 100644 --- a/sdk/metric/processor/processortest/test.go +++ b/sdk/metric/processor/processortest/test.go @@ -26,7 +26,6 @@ import ( export "go.opentelemetry.io/otel/sdk/export/metric" "go.opentelemetry.io/otel/sdk/export/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/aggregator/array" - "go.opentelemetry.io/otel/sdk/metric/aggregator/ddsketch" "go.opentelemetry.io/otel/sdk/metric/aggregator/histogram" "go.opentelemetry.io/otel/sdk/metric/aggregator/lastvalue" "go.opentelemetry.io/otel/sdk/metric/aggregator/minmaxsumcount" @@ -183,11 +182,6 @@ func (testAggregatorSelector) AggregatorFor(desc *metric.Descriptor, aggPtrs ... for i := range aggPtrs { *aggPtrs[i] = &aggs[i] } - case strings.HasSuffix(desc.Name(), ".sketch"): - aggs := ddsketch.New(len(aggPtrs), desc, ddsketch.NewDefaultConfig()) - for i := range aggPtrs { - *aggPtrs[i] = &aggs[i] - } case strings.HasSuffix(desc.Name(), ".histogram"): aggs := histogram.New(len(aggPtrs), desc, nil) for i := range aggPtrs { diff --git a/sdk/metric/selector/simple/simple.go b/sdk/metric/selector/simple/simple.go index 529dce1386e..de852e919c2 100644 --- a/sdk/metric/selector/simple/simple.go +++ b/sdk/metric/selector/simple/simple.go @@ -18,7 +18,6 @@ import ( "go.opentelemetry.io/otel/metric" export "go.opentelemetry.io/otel/sdk/export/metric" "go.opentelemetry.io/otel/sdk/metric/aggregator/array" - "go.opentelemetry.io/otel/sdk/metric/aggregator/ddsketch" "go.opentelemetry.io/otel/sdk/metric/aggregator/histogram" "go.opentelemetry.io/otel/sdk/metric/aggregator/lastvalue" "go.opentelemetry.io/otel/sdk/metric/aggregator/minmaxsumcount" @@ -28,17 +27,13 @@ import ( type ( selectorInexpensive struct{} selectorExact struct{} - selectorSketch struct { - config *ddsketch.Config - } - selectorHistogram struct { + selectorHistogram struct { boundaries []float64 } ) var ( _ export.AggregatorSelector = selectorInexpensive{} - _ export.AggregatorSelector = selectorSketch{} _ export.AggregatorSelector = selectorExact{} _ export.AggregatorSelector = selectorHistogram{} ) @@ -52,17 +47,6 @@ func NewWithInexpensiveDistribution() export.AggregatorSelector { return selectorInexpensive{} } -// NewWithSketchDistribution returns a simple aggregation selector that -// uses counter, ddsketch, and ddsketch aggregators for the three -// kinds of metric. This selector uses more cpu and memory than the -// NewWithInexpensiveDistribution because it uses one DDSketch per distinct -// instrument and label set. -func NewWithSketchDistribution(config *ddsketch.Config) export.AggregatorSelector { - return selectorSketch{ - config: config, - } -} - // NewWithExactDistribution returns a simple aggregation selector that uses // counter, array, and array aggregators for the three kinds of metric. // This selector uses more memory than the NewWithSketchDistribution @@ -108,20 +92,6 @@ func (selectorInexpensive) AggregatorFor(descriptor *metric.Descriptor, aggPtrs } } -func (s selectorSketch) AggregatorFor(descriptor *metric.Descriptor, aggPtrs ...*export.Aggregator) { - switch descriptor.InstrumentKind() { - case metric.ValueObserverInstrumentKind: - lastValueAggs(aggPtrs) - case metric.ValueRecorderInstrumentKind: - aggs := ddsketch.New(len(aggPtrs), descriptor, s.config) - for i := range aggPtrs { - *aggPtrs[i] = &aggs[i] - } - default: - sumAggs(aggPtrs) - } -} - func (selectorExact) AggregatorFor(descriptor *metric.Descriptor, aggPtrs ...*export.Aggregator) { switch descriptor.InstrumentKind() { case metric.ValueObserverInstrumentKind: diff --git a/sdk/metric/selector/simple/simple_test.go b/sdk/metric/selector/simple/simple_test.go index 6b279c62aaa..68084dc3a76 100644 --- a/sdk/metric/selector/simple/simple_test.go +++ b/sdk/metric/selector/simple/simple_test.go @@ -23,7 +23,6 @@ import ( "go.opentelemetry.io/otel/metric/number" export "go.opentelemetry.io/otel/sdk/export/metric" "go.opentelemetry.io/otel/sdk/metric/aggregator/array" - "go.opentelemetry.io/otel/sdk/metric/aggregator/ddsketch" "go.opentelemetry.io/otel/sdk/metric/aggregator/histogram" "go.opentelemetry.io/otel/sdk/metric/aggregator/lastvalue" "go.opentelemetry.io/otel/sdk/metric/aggregator/minmaxsumcount" @@ -60,12 +59,6 @@ func TestInexpensiveDistribution(t *testing.T) { testFixedSelectors(t, inex) } -func TestSketchDistribution(t *testing.T) { - sk := simple.NewWithSketchDistribution(ddsketch.NewDefaultConfig()) - require.IsType(t, (*ddsketch.Aggregator)(nil), oneAgg(sk, &testValueRecorderDesc)) - testFixedSelectors(t, sk) -} - func TestExactDistribution(t *testing.T) { ex := simple.NewWithExactDistribution() require.IsType(t, (*array.Aggregator)(nil), oneAgg(ex, &testValueRecorderDesc)) From fe4466aa25c02753890836270df9ccc93fcf0b4a Mon Sep 17 00:00:00 2001 From: Josh MacDonald Date: Fri, 18 Dec 2020 13:53:38 -0800 Subject: [PATCH 02/15] Complete removal of Quantile-related function --- example/basic/go.sum | 4 -- example/basic/main.go | 1 - example/jaeger/go.sum | 2 - example/namedtracer/go.sum | 4 -- example/opencensus/go.sum | 4 -- example/otel-collector/go.sum | 4 -- example/prometheus/go.sum | 4 -- example/zipkin/go.sum | 2 - exporters/metric/prometheus/go.sum | 4 -- exporters/metric/prometheus/prometheus.go | 59 +++----------------- exporters/otlp/go.sum | 4 -- exporters/otlp/internal/transform/metric.go | 14 ++--- exporters/stdout/config.go | 28 ---------- exporters/stdout/example_test.go | 1 - exporters/stdout/exporter.go | 2 +- exporters/stdout/go.sum | 4 -- exporters/stdout/metric.go | 23 -------- exporters/stdout/metric_test.go | 31 +--------- exporters/trace/jaeger/go.sum | 2 - exporters/trace/zipkin/go.sum | 2 - sdk/export/metric/aggregation/aggregation.go | 1 - sdk/go.mod | 1 - sdk/go.sum | 5 -- sdk/metric/aggregator/aggregatortest/test.go | 12 ---- sdk/metric/aggregator/array/array_test.go | 9 +-- sdk/metric/benchmark_test.go | 18 ------ sdk/metric/processor/processortest/test.go | 28 ++++++---- 27 files changed, 43 insertions(+), 230 deletions(-) diff --git a/example/basic/go.sum b/example/basic/go.sum index ae3c891c195..ea37d8353b5 100644 --- a/example/basic/go.sum +++ b/example/basic/go.sum @@ -1,13 +1,9 @@ -github.com/DataDog/sketches-go v0.0.1 h1:RtG+76WKgZuz6FIaGsjoPePmadDBkuD/KC6+ZWu78b8= -github.com/DataDog/sketches-go v0.0.1/go.mod h1:Q5DbzQ+3AkgGwymQO7aZFNP7ns2lZKGtvRBzRXfdi60= github.com/benbjohnson/clock v1.0.3 h1:vkLuvpK4fmtSCuo60+yC63p7y0BmQ8gm5ZXGuBCJyXg= github.com/benbjohnson/clock v1.0.3/go.mod h1:bGMdMPoPVvcYyt1gHDf4J2KE153Yf9BuiUKYMaxlTDM= github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/google/go-cmp v0.5.4 h1:L8R9j+yAqZuZjsqh/z+F1NCffTKKLShY6zXTItVIZ8M= github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= -github.com/google/gofuzz v1.1.0 h1:Hsa8mG0dQ46ij8Sl2AYJDUv1oA9/d6Vk+3LG99Oe02g= -github.com/google/gofuzz v1.1.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= diff --git a/example/basic/main.go b/example/basic/main.go index 20193dad43c..865caedb8a5 100644 --- a/example/basic/main.go +++ b/example/basic/main.go @@ -40,7 +40,6 @@ var ( func main() { exporter, err := stdout.NewExporter([]stdout.Option{ - stdout.WithQuantiles([]float64{0.5, 0.9, 0.99}), stdout.WithPrettyPrint(), }...) if err != nil { diff --git a/example/jaeger/go.sum b/example/jaeger/go.sum index b0390ec48dc..7ce2567aa2d 100644 --- a/example/jaeger/go.sum +++ b/example/jaeger/go.sum @@ -34,7 +34,6 @@ cloud.google.com/go/storage v1.10.0/go.mod h1:FLPqc6j+Ki4BU591ie1oL6qBQGu2Bl/tZ9 dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo= -github.com/DataDog/sketches-go v0.0.1/go.mod h1:Q5DbzQ+3AkgGwymQO7aZFNP7ns2lZKGtvRBzRXfdi60= github.com/apache/thrift v0.13.0 h1:5hryIiq9gtn+MiLVn0wP37kb/uTeRZgN08WoCsAhIhI= github.com/apache/thrift v0.13.0/go.mod h1:cp2SuWMxlEZw2r+iP2GNCdIi4C1qmUzdZFSVb+bacwQ= github.com/benbjohnson/clock v1.0.3/go.mod h1:bGMdMPoPVvcYyt1gHDf4J2KE153Yf9BuiUKYMaxlTDM= @@ -93,7 +92,6 @@ github.com/google/go-cmp v0.5.2 h1:X2ev0eStA3AbceY54o37/0PQ/UWqKEiiO2dKL5OPaFM= github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.4 h1:L8R9j+yAqZuZjsqh/z+F1NCffTKKLShY6zXTItVIZ8M= github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= -github.com/google/gofuzz v1.1.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/google/martian v2.1.0+incompatible/go.mod h1:9I4somxYTbIHy5NJKHRl3wXiIaQGbYVAs8BPL6v8lEs= github.com/google/martian/v3 v3.0.0/go.mod h1:y5Zk1BBys9G+gd6Jrk0W3cC1+ELVxBWuIGO+w/tUAp0= github.com/google/martian/v3 v3.1.0/go.mod h1:y5Zk1BBys9G+gd6Jrk0W3cC1+ELVxBWuIGO+w/tUAp0= diff --git a/example/namedtracer/go.sum b/example/namedtracer/go.sum index ae3c891c195..ea37d8353b5 100644 --- a/example/namedtracer/go.sum +++ b/example/namedtracer/go.sum @@ -1,13 +1,9 @@ -github.com/DataDog/sketches-go v0.0.1 h1:RtG+76WKgZuz6FIaGsjoPePmadDBkuD/KC6+ZWu78b8= -github.com/DataDog/sketches-go v0.0.1/go.mod h1:Q5DbzQ+3AkgGwymQO7aZFNP7ns2lZKGtvRBzRXfdi60= github.com/benbjohnson/clock v1.0.3 h1:vkLuvpK4fmtSCuo60+yC63p7y0BmQ8gm5ZXGuBCJyXg= github.com/benbjohnson/clock v1.0.3/go.mod h1:bGMdMPoPVvcYyt1gHDf4J2KE153Yf9BuiUKYMaxlTDM= github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/google/go-cmp v0.5.4 h1:L8R9j+yAqZuZjsqh/z+F1NCffTKKLShY6zXTItVIZ8M= github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= -github.com/google/gofuzz v1.1.0 h1:Hsa8mG0dQ46ij8Sl2AYJDUv1oA9/d6Vk+3LG99Oe02g= -github.com/google/gofuzz v1.1.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= diff --git a/example/opencensus/go.sum b/example/opencensus/go.sum index 0f352e86792..46f12405462 100644 --- a/example/opencensus/go.sum +++ b/example/opencensus/go.sum @@ -1,7 +1,5 @@ cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= -github.com/DataDog/sketches-go v0.0.1 h1:RtG+76WKgZuz6FIaGsjoPePmadDBkuD/KC6+ZWu78b8= -github.com/DataDog/sketches-go v0.0.1/go.mod h1:Q5DbzQ+3AkgGwymQO7aZFNP7ns2lZKGtvRBzRXfdi60= github.com/benbjohnson/clock v1.0.3 h1:vkLuvpK4fmtSCuo60+yC63p7y0BmQ8gm5ZXGuBCJyXg= github.com/benbjohnson/clock v1.0.3/go.mod h1:bGMdMPoPVvcYyt1gHDf4J2KE153Yf9BuiUKYMaxlTDM= github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw= @@ -16,8 +14,6 @@ github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5y github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU= github.com/google/go-cmp v0.5.4 h1:L8R9j+yAqZuZjsqh/z+F1NCffTKKLShY6zXTItVIZ8M= github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= -github.com/google/gofuzz v1.1.0 h1:Hsa8mG0dQ46ij8Sl2AYJDUv1oA9/d6Vk+3LG99Oe02g= -github.com/google/gofuzz v1.1.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= diff --git a/example/otel-collector/go.sum b/example/otel-collector/go.sum index b9d0e686625..717f650c750 100644 --- a/example/otel-collector/go.sum +++ b/example/otel-collector/go.sum @@ -1,7 +1,5 @@ cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= -github.com/DataDog/sketches-go v0.0.1 h1:RtG+76WKgZuz6FIaGsjoPePmadDBkuD/KC6+ZWu78b8= -github.com/DataDog/sketches-go v0.0.1/go.mod h1:Q5DbzQ+3AkgGwymQO7aZFNP7ns2lZKGtvRBzRXfdi60= github.com/benbjohnson/clock v1.0.3 h1:vkLuvpK4fmtSCuo60+yC63p7y0BmQ8gm5ZXGuBCJyXg= github.com/benbjohnson/clock v1.0.3/go.mod h1:bGMdMPoPVvcYyt1gHDf4J2KE153Yf9BuiUKYMaxlTDM= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= @@ -34,8 +32,6 @@ github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/ github.com/google/go-cmp v0.5.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.4 h1:L8R9j+yAqZuZjsqh/z+F1NCffTKKLShY6zXTItVIZ8M= github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= -github.com/google/gofuzz v1.1.0 h1:Hsa8mG0dQ46ij8Sl2AYJDUv1oA9/d6Vk+3LG99Oe02g= -github.com/google/gofuzz v1.1.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/google/uuid v1.1.2/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/kisielk/errcheck v1.2.0/go.mod h1:/BMXB+zMLi60iA8Vv6Ksmxu/1UDYcXs4uQLJ+jE2L00= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= diff --git a/example/prometheus/go.sum b/example/prometheus/go.sum index d6cca95ee74..066a588984c 100644 --- a/example/prometheus/go.sum +++ b/example/prometheus/go.sum @@ -1,8 +1,6 @@ cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= cloud.google.com/go v0.34.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= -github.com/DataDog/sketches-go v0.0.1 h1:RtG+76WKgZuz6FIaGsjoPePmadDBkuD/KC6+ZWu78b8= -github.com/DataDog/sketches-go v0.0.1/go.mod h1:Q5DbzQ+3AkgGwymQO7aZFNP7ns2lZKGtvRBzRXfdi60= github.com/Knetic/govaluate v3.0.1-0.20171022003610-9aa49832a739+incompatible/go.mod h1:r7JcOSlj0wfOMncg0iLm8Leh48TZaKVeNIfJntJ2wa0= github.com/Shopify/sarama v1.19.0/go.mod h1:FVkBWblsNy7DGZRfXLU0O9RCGt5g3g3yEuWXgklEdEo= github.com/Shopify/toxiproxy v2.1.4+incompatible/go.mod h1:OXgGpZ6Cli1/URJOF1DMxUHB2q5Ap20/P/eIdh4G0pI= @@ -98,8 +96,6 @@ github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/ github.com/google/go-cmp v0.5.4 h1:L8R9j+yAqZuZjsqh/z+F1NCffTKKLShY6zXTItVIZ8M= github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= -github.com/google/gofuzz v1.1.0 h1:Hsa8mG0dQ46ij8Sl2AYJDUv1oA9/d6Vk+3LG99Oe02g= -github.com/google/gofuzz v1.1.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI= github.com/google/uuid v1.0.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/gopherjs/gopherjs v0.0.0-20181017120253-0766667cb4d1/go.mod h1:wJfORRmW1u3UXTncJ5qlYoELFm8eSnnEO6hX4iZ3EWY= diff --git a/example/zipkin/go.sum b/example/zipkin/go.sum index d226056d1ae..eb8129cdb9b 100644 --- a/example/zipkin/go.sum +++ b/example/zipkin/go.sum @@ -1,6 +1,5 @@ cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= -github.com/DataDog/sketches-go v0.0.1/go.mod h1:Q5DbzQ+3AkgGwymQO7aZFNP7ns2lZKGtvRBzRXfdi60= github.com/Shopify/sarama v1.19.0/go.mod h1:FVkBWblsNy7DGZRfXLU0O9RCGt5g3g3yEuWXgklEdEo= github.com/Shopify/toxiproxy v2.1.4+incompatible/go.mod h1:OXgGpZ6Cli1/URJOF1DMxUHB2q5Ap20/P/eIdh4G0pI= github.com/benbjohnson/clock v1.0.3/go.mod h1:bGMdMPoPVvcYyt1gHDf4J2KE153Yf9BuiUKYMaxlTDM= @@ -40,7 +39,6 @@ github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/ github.com/google/go-cmp v0.5.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.4 h1:L8R9j+yAqZuZjsqh/z+F1NCffTKKLShY6zXTItVIZ8M= github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= -github.com/google/gofuzz v1.1.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/gorilla/context v1.1.1/go.mod h1:kBGZzfjB9CEq2AlWe17Uuf7NDRt0dE0s8S51q0aT7Yg= github.com/gorilla/mux v1.6.2/go.mod h1:1lud6UwP+6orDFRuTfBEV8e9/aOM/c4fVVCaMa2zaAs= github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU= diff --git a/exporters/metric/prometheus/go.sum b/exporters/metric/prometheus/go.sum index e4e972a7a59..11c0cd26545 100644 --- a/exporters/metric/prometheus/go.sum +++ b/exporters/metric/prometheus/go.sum @@ -1,8 +1,6 @@ cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= cloud.google.com/go v0.34.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= -github.com/DataDog/sketches-go v0.0.1 h1:RtG+76WKgZuz6FIaGsjoPePmadDBkuD/KC6+ZWu78b8= -github.com/DataDog/sketches-go v0.0.1/go.mod h1:Q5DbzQ+3AkgGwymQO7aZFNP7ns2lZKGtvRBzRXfdi60= github.com/Knetic/govaluate v3.0.1-0.20171022003610-9aa49832a739+incompatible/go.mod h1:r7JcOSlj0wfOMncg0iLm8Leh48TZaKVeNIfJntJ2wa0= github.com/Shopify/sarama v1.19.0/go.mod h1:FVkBWblsNy7DGZRfXLU0O9RCGt5g3g3yEuWXgklEdEo= github.com/Shopify/toxiproxy v2.1.4+incompatible/go.mod h1:OXgGpZ6Cli1/URJOF1DMxUHB2q5Ap20/P/eIdh4G0pI= @@ -99,8 +97,6 @@ github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/ github.com/google/go-cmp v0.5.4 h1:L8R9j+yAqZuZjsqh/z+F1NCffTKKLShY6zXTItVIZ8M= github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= -github.com/google/gofuzz v1.1.0 h1:Hsa8mG0dQ46ij8Sl2AYJDUv1oA9/d6Vk+3LG99Oe02g= -github.com/google/gofuzz v1.1.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI= github.com/google/uuid v1.0.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/gopherjs/gopherjs v0.0.0-20181017120253-0766667cb4d1/go.mod h1:wJfORRmW1u3UXTncJ5qlYoELFm8eSnnEO6hX4iZ3EWY= diff --git a/exporters/metric/prometheus/prometheus.go b/exporters/metric/prometheus/prometheus.go index a12a8ff3c48..2f5845028c7 100644 --- a/exporters/metric/prometheus/prometheus.go +++ b/exporters/metric/prometheus/prometheus.go @@ -50,10 +50,13 @@ type Exporter struct { lock sync.RWMutex controller *pull.Controller - defaultSummaryQuantiles []float64 defaultHistogramBoundaries []float64 } +// ErrUnsupportedAggregator is returned for unrepresentable aggregator +// types (e.g., array). +var ErrUnsupportedAggregator = fmt.Errorf("unsupported aggregator type") + var _ http.Handler = &Exporter{} // Config is a set of configs for the tally reporter. @@ -76,10 +79,6 @@ type Config struct { // If not specified the Registry will be used as default. Gatherer prometheus.Gatherer - // DefaultSummaryQuantiles is the default summary quantiles - // to use. Use nil to specify the system-default summary quantiles. - DefaultSummaryQuantiles []float64 - // DefaultHistogramBoundaries defines the default histogram bucket // boundaries. DefaultHistogramBoundaries []float64 @@ -104,7 +103,6 @@ func NewExportPipeline(config Config, options ...pull.Option) (*Exporter, error) handler: promhttp.HandlerFor(config.Gatherer, promhttp.HandlerOpts{}), registerer: config.Registerer, gatherer: config.Gatherer, - defaultSummaryQuantiles: config.DefaultSummaryQuantiles, defaultHistogramBoundaries: config.DefaultHistogramBoundaries, } @@ -167,15 +165,12 @@ func (e *Exporter) Controller() *pull.Controller { return e.controller } +// ExportKindFor implements ExportKindSelector. func (e *Exporter) ExportKindFor(desc *metric.Descriptor, kind aggregation.Kind) export.ExportKind { - // NOTE: Summary values should use Delta aggregation, then be - // combined into a sliding window, see the TODO below. - // NOTE: Prometheus also supports a "GaugeDelta" exposition format, - // which is expressed as a delta histogram. Need to understand if this - // should be a default behavior for ValueRecorder/ValueObserver. return export.CumulativeExportKindSelector().ExportKindFor(desc, kind) } +// ServeHTTP implements http.Handler. func (e *Exporter) ServeHTTP(w http.ResponseWriter, r *http.Request) { e.handler.ServeHTTP(w, r) } @@ -187,6 +182,7 @@ type collector struct { var _ prometheus.Collector = (*collector)(nil) +// Describe implements prometheus.Collector. func (c *collector) Describe(ch chan<- *prometheus.Desc) { c.exp.lock.RLock() defer c.exp.lock.RUnlock() @@ -226,18 +222,6 @@ func (c *collector) Collect(ch chan<- prometheus.Metric) { if err := c.exportHistogram(ch, hist, numberKind, desc, labels); err != nil { return fmt.Errorf("exporting histogram: %w", err) } - } else if dist, ok := agg.(aggregation.Distribution); ok { - // TODO: summaries values are never being resetted. - // As measurements are recorded, new records starts to have less impact on these summaries. - // We should implement an solution that is similar to the Prometheus Clients - // using a rolling window for summaries could be a solution. - // - // References: - // https://www.robustperception.io/how-does-a-prometheus-summary-work - // https://github.com/prometheus/client_golang/blob/fa4aa9000d2863904891d193dea354d23f3d712a/prometheus/summary.go#L135 - if err := c.exportSummary(ch, dist, numberKind, desc, labels); err != nil { - return fmt.Errorf("exporting summary: %w", err) - } } else if sum, ok := agg.(aggregation.Sum); ok && instrumentKind.Monotonic() { if err := c.exportMonotonicCounter(ch, sum, numberKind, desc, labels); err != nil { return fmt.Errorf("exporting monotonic counter: %w", err) @@ -250,6 +234,8 @@ func (c *collector) Collect(ch chan<- prometheus.Metric) { if err := c.exportLastValue(ch, lastValue, numberKind, desc, labels); err != nil { return fmt.Errorf("exporting last value: %w", err) } + } else { + return fmt.Errorf("%w: %s", ErrUnsupportedAggregator, agg.Kind()) } return nil }) @@ -303,33 +289,6 @@ func (c *collector) exportMonotonicCounter(ch chan<- prometheus.Metric, sum aggr return nil } -func (c *collector) exportSummary(ch chan<- prometheus.Metric, dist aggregation.Distribution, kind number.Kind, desc *prometheus.Desc, labels []string) error { - count, err := dist.Count() - if err != nil { - return fmt.Errorf("error retrieving count: %w", err) - } - - var sum number.Number - sum, err = dist.Sum() - if err != nil { - return fmt.Errorf("error retrieving distribution sum: %w", err) - } - - quantiles := make(map[float64]float64) - for _, quantile := range c.exp.defaultSummaryQuantiles { - q, _ := dist.Quantile(quantile) - quantiles[quantile] = q.CoerceToFloat64(kind) - } - - m, err := prometheus.NewConstSummary(desc, uint64(count), sum.CoerceToFloat64(kind), quantiles, labels...) - if err != nil { - return fmt.Errorf("error creating constant summary: %w", err) - } - - ch <- m - return nil -} - func (c *collector) exportHistogram(ch chan<- prometheus.Metric, hist aggregation.Histogram, kind number.Kind, desc *prometheus.Desc, labels []string) error { buckets, err := hist.Histogram() if err != nil { diff --git a/exporters/otlp/go.sum b/exporters/otlp/go.sum index b9d0e686625..717f650c750 100644 --- a/exporters/otlp/go.sum +++ b/exporters/otlp/go.sum @@ -1,7 +1,5 @@ cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= -github.com/DataDog/sketches-go v0.0.1 h1:RtG+76WKgZuz6FIaGsjoPePmadDBkuD/KC6+ZWu78b8= -github.com/DataDog/sketches-go v0.0.1/go.mod h1:Q5DbzQ+3AkgGwymQO7aZFNP7ns2lZKGtvRBzRXfdi60= github.com/benbjohnson/clock v1.0.3 h1:vkLuvpK4fmtSCuo60+yC63p7y0BmQ8gm5ZXGuBCJyXg= github.com/benbjohnson/clock v1.0.3/go.mod h1:bGMdMPoPVvcYyt1gHDf4J2KE153Yf9BuiUKYMaxlTDM= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= @@ -34,8 +32,6 @@ github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/ github.com/google/go-cmp v0.5.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.4 h1:L8R9j+yAqZuZjsqh/z+F1NCffTKKLShY6zXTItVIZ8M= github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= -github.com/google/gofuzz v1.1.0 h1:Hsa8mG0dQ46ij8Sl2AYJDUv1oA9/d6Vk+3LG99Oe02g= -github.com/google/gofuzz v1.1.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/google/uuid v1.1.2/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/kisielk/errcheck v1.2.0/go.mod h1:/BMXB+zMLi60iA8Vv6Ksmxu/1UDYcXs4uQLJ+jE2L00= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= diff --git a/exporters/otlp/internal/transform/metric.go b/exporters/otlp/internal/transform/metric.go index ad0432a6391..1a7503496e9 100644 --- a/exporters/otlp/internal/transform/metric.go +++ b/exporters/otlp/internal/transform/metric.go @@ -306,7 +306,7 @@ func Record(exportSelector export.ExportKindSelector, r export.Record) (*metricp } } -func gaugeArray(record export.Record, points []number.Number) (*metricpb.Metric, error) { +func gaugeArray(record export.Record, samples aggregation.Samples) (*metricpb.Metric, error) { desc := record.Descriptor() m := &metricpb.Metric{ Name: desc.Name(), @@ -314,15 +314,15 @@ func gaugeArray(record export.Record, points []number.Number) (*metricpb.Metric, Unit: string(desc.Unit()), } - switch n := desc.NumberKind(); n { + switch nk := desc.NumberKind(); nk { case number.Int64Kind: var pts []*metricpb.IntDataPoint - for _, p := range points { + for _, s := range samples { pts = append(pts, &metricpb.IntDataPoint{ Labels: nil, StartTimeUnixNano: toNanos(record.StartTime()), TimeUnixNano: toNanos(record.EndTime()), - Value: p.CoerceToInt64(n), + Value: s.Number.CoerceToInt64(nk), }) } m.Data = &metricpb.Metric_IntGauge{ @@ -333,12 +333,12 @@ func gaugeArray(record export.Record, points []number.Number) (*metricpb.Metric, case number.Float64Kind: var pts []*metricpb.DoubleDataPoint - for _, p := range points { + for _, s := range samples { pts = append(pts, &metricpb.DoubleDataPoint{ Labels: nil, StartTimeUnixNano: toNanos(record.StartTime()), TimeUnixNano: toNanos(record.EndTime()), - Value: p.CoerceToFloat64(n), + Value: s.Number.CoerceToFloat64(nk), }) } m.Data = &metricpb.Metric_DoubleGauge{ @@ -348,7 +348,7 @@ func gaugeArray(record export.Record, points []number.Number) (*metricpb.Metric, } default: - return nil, fmt.Errorf("%w: %v", ErrUnknownValueType, n) + return nil, fmt.Errorf("%w: %v", ErrUnknownValueType, nk) } return m, nil diff --git a/exporters/stdout/config.go b/exporters/stdout/config.go index 47ba053462e..4bf90b36e94 100644 --- a/exporters/stdout/config.go +++ b/exporters/stdout/config.go @@ -19,14 +19,12 @@ import ( "os" "go.opentelemetry.io/otel/label" - "go.opentelemetry.io/otel/sdk/export/metric/aggregation" ) var ( defaultWriter = os.Stdout defaultPrettyPrint = false defaultTimestamps = true - defaultQuantiles = []float64{0.5, 0.9, 0.99} defaultLabelEncoder = label.DefaultEncoder() defaultDisableTraceExport = false defaultDisableMetricExport = false @@ -45,15 +43,6 @@ type Config struct { // true. Timestamps bool - // Quantiles are the desired aggregation quantiles for distribution - // summaries, used when the configured aggregator supports - // quantiles. - // - // Note: this exporter is meant as a demonstration; a real - // exporter may wish to configure quantiles on a per-metric - // basis. - Quantiles []float64 - // LabelEncoder encodes the labels. LabelEncoder label.Encoder @@ -70,7 +59,6 @@ func NewConfig(options ...Option) (Config, error) { Writer: defaultWriter, PrettyPrint: defaultPrettyPrint, Timestamps: defaultTimestamps, - Quantiles: defaultQuantiles, LabelEncoder: defaultLabelEncoder, DisableTraceExport: defaultDisableTraceExport, DisableMetricExport: defaultDisableMetricExport, @@ -79,11 +67,6 @@ func NewConfig(options ...Option) (Config, error) { opt.Apply(&config) } - for _, q := range config.Quantiles { - if q < 0 || q > 1 { - return config, aggregation.ErrInvalidQuantile - } - } return config, nil } @@ -128,17 +111,6 @@ func (o timestampsOption) Apply(config *Config) { config.Timestamps = bool(o) } -// WithQuantiles sets the quantile values to export. -func WithQuantiles(quantiles []float64) Option { - return quantilesOption(quantiles) -} - -type quantilesOption []float64 - -func (o quantilesOption) Apply(config *Config) { - config.Quantiles = []float64(o) -} - // WithLabelEncoder sets the label encoder used in export. func WithLabelEncoder(enc label.Encoder) Option { return labelEncoderOption{enc} diff --git a/exporters/stdout/example_test.go b/exporters/stdout/example_test.go index e537f71e4d2..15e25558772 100644 --- a/exporters/stdout/example_test.go +++ b/exporters/stdout/example_test.go @@ -77,7 +77,6 @@ func multiply(ctx context.Context, x, y int64) int64 { func Example() { exportOpts := []stdout.Option{ - stdout.WithQuantiles([]float64{0.5}), stdout.WithPrettyPrint(), } // Registers both a trace and meter Provider globally. diff --git a/exporters/stdout/exporter.go b/exporters/stdout/exporter.go index 21a64c022cd..5f6b5102012 100644 --- a/exporters/stdout/exporter.go +++ b/exporters/stdout/exporter.go @@ -59,7 +59,7 @@ func NewExportPipeline(exportOpts []Option, pushOpts []push.Option) (trace.Trace tp := sdktrace.NewTracerProvider(sdktrace.WithBatcher(exporter)) pusher := push.New( basic.New( - simple.NewWithExactDistribution(), + simple.NewWithInexpensiveDistribution(), exporter, ), exporter, diff --git a/exporters/stdout/go.sum b/exporters/stdout/go.sum index ae3c891c195..ea37d8353b5 100644 --- a/exporters/stdout/go.sum +++ b/exporters/stdout/go.sum @@ -1,13 +1,9 @@ -github.com/DataDog/sketches-go v0.0.1 h1:RtG+76WKgZuz6FIaGsjoPePmadDBkuD/KC6+ZWu78b8= -github.com/DataDog/sketches-go v0.0.1/go.mod h1:Q5DbzQ+3AkgGwymQO7aZFNP7ns2lZKGtvRBzRXfdi60= github.com/benbjohnson/clock v1.0.3 h1:vkLuvpK4fmtSCuo60+yC63p7y0BmQ8gm5ZXGuBCJyXg= github.com/benbjohnson/clock v1.0.3/go.mod h1:bGMdMPoPVvcYyt1gHDf4J2KE153Yf9BuiUKYMaxlTDM= github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/google/go-cmp v0.5.4 h1:L8R9j+yAqZuZjsqh/z+F1NCffTKKLShY6zXTItVIZ8M= github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= -github.com/google/gofuzz v1.1.0 h1:Hsa8mG0dQ46ij8Sl2AYJDUv1oA9/d6Vk+3LG99Oe02g= -github.com/google/gofuzz v1.1.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= diff --git a/exporters/stdout/metric.go b/exporters/stdout/metric.go index 725d3abf795..9b183a0022d 100644 --- a/exporters/stdout/metric.go +++ b/exporters/stdout/metric.go @@ -41,17 +41,10 @@ type line struct { Count interface{} `json:"Count,omitempty"` LastValue interface{} `json:"Last,omitempty"` - Quantiles []quantile `json:"Quantiles,omitempty"` - // Note: this is a pointer because omitempty doesn't work when time.IsZero() Timestamp *time.Time `json:"Timestamp,omitempty"` } -type quantile struct { - Quantile interface{} `json:"Quantile"` - Value interface{} `json:"Value"` -} - func (e *metricExporter) ExportKindFor(desc *metric.Descriptor, kind aggregation.Kind) exportmetric.ExportKind { return exportmetric.StatelessExportKindSelector().ExportKindFor(desc, kind) } @@ -106,22 +99,6 @@ func (e *metricExporter) Export(_ context.Context, checkpointSet exportmetric.Ch return err } expose.Min = min.AsInterface(kind) - - if dist, ok := agg.(aggregation.Distribution); ok && len(e.config.Quantiles) != 0 { - summary := make([]quantile, len(e.config.Quantiles)) - expose.Quantiles = summary - - for i, q := range e.config.Quantiles { - value, err := dist.Quantile(q) - if err != nil { - return err - } - summary[i] = quantile{ - Quantile: q, - Value: value.AsInterface(kind), - } - } - } } else if lv, ok := agg.(aggregation.LastValue); ok { value, timestamp, err := lv.LastValue() if err != nil { diff --git a/exporters/stdout/metric_test.go b/exporters/stdout/metric_test.go index 07378fbb4d9..d5ebcfca864 100644 --- a/exporters/stdout/metric_test.go +++ b/exporters/stdout/metric_test.go @@ -31,11 +31,8 @@ import ( "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/metric/number" export "go.opentelemetry.io/otel/sdk/export/metric" - "go.opentelemetry.io/otel/sdk/export/metric/aggregation" "go.opentelemetry.io/otel/sdk/export/metric/metrictest" "go.opentelemetry.io/otel/sdk/metric/aggregator/aggregatortest" - "go.opentelemetry.io/otel/sdk/metric/aggregator/array" - "go.opentelemetry.io/otel/sdk/metric/aggregator/ddsketch" "go.opentelemetry.io/otel/sdk/metric/aggregator/lastvalue" "go.opentelemetry.io/otel/sdk/metric/aggregator/minmaxsumcount" "go.opentelemetry.io/otel/sdk/metric/aggregator/sum" @@ -78,14 +75,6 @@ func (fix testFixture) Export(checkpointSet export.CheckpointSet) { } } -func TestStdoutInvalidQuantile(t *testing.T) { - _, err := stdout.NewExporter( - stdout.WithQuantiles([]float64{1.1, 0.9}), - ) - require.Error(t, err, "Invalid quantile error expected") - require.Equal(t, aggregation.ErrInvalidQuantile, err) -} - func TestStdoutTimestamp(t *testing.T) { var buf bytes.Buffer exporter, err := stdout.NewExporter( @@ -197,7 +186,7 @@ func TestStdoutValueRecorderFormat(t *testing.T) { checkpointSet := metrictest.NewCheckpointSet(testResource) desc := metric.NewDescriptor("test.name", metric.ValueRecorderInstrumentKind, number.Float64Kind) - aagg, ckpt := metrictest.Unslice2(array.New(2)) + aagg, ckpt := metrictest.Unslice2(minmaxsumcount.New(2, &desc)) for i := 0; i < 1000; i++ { aggregatortest.CheckedUpdate(fix.t, aagg, number.NewFloat64Number(float64(i)+0.5), &desc) @@ -215,21 +204,7 @@ func TestStdoutValueRecorderFormat(t *testing.T) { "Min": 0.5, "Max": 999.5, "Sum": 500000, - "Count": 1000, - "Quantiles": [ - { - "Quantile": 0.5, - "Value": 500.5 - }, - { - "Quantile": 0.9, - "Value": 900.5 - }, - { - "Quantile": 0.99, - "Value": 990.5 - } - ] + "Count": 1000 } ]`, fix.Output()) } @@ -255,7 +230,7 @@ func TestStdoutNoData(t *testing.T) { }) } - runTwoAggs(metrictest.Unslice2(ddsketch.New(2, &desc, ddsketch.NewDefaultConfig()))) + runTwoAggs(metrictest.Unslice2(lastvalue.New(2))) runTwoAggs(metrictest.Unslice2(minmaxsumcount.New(2, &desc))) } diff --git a/exporters/trace/jaeger/go.sum b/exporters/trace/jaeger/go.sum index 86fff357acf..d2ca3a19062 100644 --- a/exporters/trace/jaeger/go.sum +++ b/exporters/trace/jaeger/go.sum @@ -34,7 +34,6 @@ cloud.google.com/go/storage v1.10.0/go.mod h1:FLPqc6j+Ki4BU591ie1oL6qBQGu2Bl/tZ9 dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo= -github.com/DataDog/sketches-go v0.0.1/go.mod h1:Q5DbzQ+3AkgGwymQO7aZFNP7ns2lZKGtvRBzRXfdi60= github.com/apache/thrift v0.13.0 h1:5hryIiq9gtn+MiLVn0wP37kb/uTeRZgN08WoCsAhIhI= github.com/apache/thrift v0.13.0/go.mod h1:cp2SuWMxlEZw2r+iP2GNCdIi4C1qmUzdZFSVb+bacwQ= github.com/benbjohnson/clock v1.0.3/go.mod h1:bGMdMPoPVvcYyt1gHDf4J2KE153Yf9BuiUKYMaxlTDM= @@ -93,7 +92,6 @@ github.com/google/go-cmp v0.5.2 h1:X2ev0eStA3AbceY54o37/0PQ/UWqKEiiO2dKL5OPaFM= github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.4 h1:L8R9j+yAqZuZjsqh/z+F1NCffTKKLShY6zXTItVIZ8M= github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= -github.com/google/gofuzz v1.1.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/google/martian v2.1.0+incompatible/go.mod h1:9I4somxYTbIHy5NJKHRl3wXiIaQGbYVAs8BPL6v8lEs= github.com/google/martian/v3 v3.0.0/go.mod h1:y5Zk1BBys9G+gd6Jrk0W3cC1+ELVxBWuIGO+w/tUAp0= github.com/google/martian/v3 v3.1.0/go.mod h1:y5Zk1BBys9G+gd6Jrk0W3cC1+ELVxBWuIGO+w/tUAp0= diff --git a/exporters/trace/zipkin/go.sum b/exporters/trace/zipkin/go.sum index 4bffaf98aa9..7887d24cc6d 100644 --- a/exporters/trace/zipkin/go.sum +++ b/exporters/trace/zipkin/go.sum @@ -1,6 +1,5 @@ cloud.google.com/go v0.26.0/go.mod h1:aQUYkXzVsufM+DwF1aE+0xfcU+56JwCaLick0ClmMTw= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= -github.com/DataDog/sketches-go v0.0.1/go.mod h1:Q5DbzQ+3AkgGwymQO7aZFNP7ns2lZKGtvRBzRXfdi60= github.com/Shopify/sarama v1.19.0/go.mod h1:FVkBWblsNy7DGZRfXLU0O9RCGt5g3g3yEuWXgklEdEo= github.com/Shopify/toxiproxy v2.1.4+incompatible/go.mod h1:OXgGpZ6Cli1/URJOF1DMxUHB2q5Ap20/P/eIdh4G0pI= github.com/benbjohnson/clock v1.0.3/go.mod h1:bGMdMPoPVvcYyt1gHDf4J2KE153Yf9BuiUKYMaxlTDM= @@ -40,7 +39,6 @@ github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/ github.com/google/go-cmp v0.5.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.4 h1:L8R9j+yAqZuZjsqh/z+F1NCffTKKLShY6zXTItVIZ8M= github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= -github.com/google/gofuzz v1.1.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/gorilla/context v1.1.1/go.mod h1:kBGZzfjB9CEq2AlWe17Uuf7NDRt0dE0s8S51q0aT7Yg= github.com/gorilla/mux v1.6.2/go.mod h1:1lud6UwP+6orDFRuTfBEV8e9/aOM/c4fVVCaMa2zaAs= github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU= diff --git a/sdk/export/metric/aggregation/aggregation.go b/sdk/export/metric/aggregation/aggregation.go index b62a36144c3..3d10fbbbd3f 100644 --- a/sdk/export/metric/aggregation/aggregation.go +++ b/sdk/export/metric/aggregation/aggregation.go @@ -139,7 +139,6 @@ const ( ) var ( - ErrInvalidQuantile = fmt.Errorf("the requested quantile is out of range") ErrNegativeInput = fmt.Errorf("negative value is out of range for this instrument") ErrNaNInput = fmt.Errorf("NaN value is an invalid input") ErrInconsistentType = fmt.Errorf("inconsistent aggregator types") diff --git a/sdk/go.mod b/sdk/go.mod index fe5d3cb79b3..ec78c08ab99 100644 --- a/sdk/go.mod +++ b/sdk/go.mod @@ -7,7 +7,6 @@ replace go.opentelemetry.io/otel => ../ require ( github.com/benbjohnson/clock v1.0.3 github.com/google/go-cmp v0.5.4 - github.com/google/gofuzz v1.1.0 // indirect github.com/stretchr/testify v1.6.1 go.opentelemetry.io/otel v0.15.0 ) diff --git a/sdk/go.sum b/sdk/go.sum index 6bd1abfe0af..ea37d8353b5 100644 --- a/sdk/go.sum +++ b/sdk/go.sum @@ -1,19 +1,14 @@ -github.com/DataDog/sketches-go v0.0.1 h1:RtG+76WKgZuz6FIaGsjoPePmadDBkuD/KC6+ZWu78b8= -github.com/DataDog/sketches-go v0.0.1/go.mod h1:Q5DbzQ+3AkgGwymQO7aZFNP7ns2lZKGtvRBzRXfdi60= github.com/benbjohnson/clock v1.0.3 h1:vkLuvpK4fmtSCuo60+yC63p7y0BmQ8gm5ZXGuBCJyXg= github.com/benbjohnson/clock v1.0.3/go.mod h1:bGMdMPoPVvcYyt1gHDf4J2KE153Yf9BuiUKYMaxlTDM= github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/google/go-cmp v0.5.4 h1:L8R9j+yAqZuZjsqh/z+F1NCffTKKLShY6zXTItVIZ8M= github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= -github.com/google/gofuzz v1.1.0 h1:Hsa8mG0dQ46ij8Sl2AYJDUv1oA9/d6Vk+3LG99Oe02g= -github.com/google/gofuzz v1.1.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd0= github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= -go.opentelemetry.io v0.1.0 h1:EANZoRCOP+A3faIlw/iN6YEWoYb1vleZRKm1EvH8T48= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= diff --git a/sdk/metric/aggregator/aggregatortest/test.go b/sdk/metric/aggregator/aggregatortest/test.go index 3a965551256..286668c85ec 100644 --- a/sdk/metric/aggregator/aggregatortest/test.go +++ b/sdk/metric/aggregator/aggregatortest/test.go @@ -92,8 +92,6 @@ func TestMain(m *testing.M) { os.Exit(m.Run()) } -// TODO: Expose Numbers in api/metric for sorting support - type Numbers struct { // numbers has to be aligned for 64-bit atomic operations. numbers []number.Number @@ -146,16 +144,6 @@ func (n *Numbers) Max() number.Number { return n.numbers[len(n.numbers)-1] } -// Median() is an alias for Quantile(0.5). -func (n *Numbers) Median() number.Number { - // Note that len(n.numbers) is 1 greater than the max element - // index, so dividing by two rounds up. This gives the - // intended definition for Quantile() in tests, which is to - // return the smallest element that is at or above the - // specified quantile. - return n.numbers[len(n.numbers)/2] -} - func (n *Numbers) Points() []number.Number { return n.numbers } diff --git a/sdk/metric/aggregator/array/array_test.go b/sdk/metric/aggregator/array/array_test.go index 6c3dfd3686d..ad076f4ee53 100644 --- a/sdk/metric/aggregator/array/array_test.go +++ b/sdk/metric/aggregator/array/array_test.go @@ -39,7 +39,7 @@ func checkZero(t *testing.T, agg *Aggregator, desc *metric.Descriptor) { pts, err := agg.Points() require.NoError(t, err) - require.Equal(t, nil, pts) + require.Equal(t, 0, len(pts)) } func new2() (_, _ *Aggregator) { @@ -252,8 +252,6 @@ func TestArrayFloat64(t *testing.T) { require.NoError(t, agg.SynchronizedMove(ckpt, descriptor)) - all.Sort() - pts, err := ckpt.Points() require.Nil(t, err) @@ -269,7 +267,10 @@ func TestArrayFloat64(t *testing.T) { require.Nil(t, err) require.Equal(t, all.Len(), len(po), "Points() must have same length of updates") for i := 0; i < len(po); i++ { - require.Equal(t, all.Points()[i], po[i], "Wrong point at position %d", i) + require.Equal(t, all.Points()[i], po[i].Number, "Wrong point at position %d", i) + if i > 0 { + require.True(t, po[i-1].Time.Before(po[i].Time)) + } } } diff --git a/sdk/metric/benchmark_test.go b/sdk/metric/benchmark_test.go index bb32bb34cdb..b7fda0b1e25 100644 --- a/sdk/metric/benchmark_test.go +++ b/sdk/metric/benchmark_test.go @@ -458,24 +458,6 @@ func BenchmarkFloat64MaxSumCountHandleAdd(b *testing.B) { benchmarkFloat64ValueRecorderHandleAdd(b, "float64.minmaxsumcount") } -// DDSketch - -func BenchmarkInt64DDSketchAdd(b *testing.B) { - benchmarkInt64ValueRecorderAdd(b, "int64.sketch") -} - -func BenchmarkInt64DDSketchHandleAdd(b *testing.B) { - benchmarkInt64ValueRecorderHandleAdd(b, "int64.sketch") -} - -func BenchmarkFloat64DDSketchAdd(b *testing.B) { - benchmarkFloat64ValueRecorderAdd(b, "float64.sketch") -} - -func BenchmarkFloat64DDSketchHandleAdd(b *testing.B) { - benchmarkFloat64ValueRecorderHandleAdd(b, "float64.sketch") -} - // Array func BenchmarkInt64ArrayAdd(b *testing.B) { diff --git a/sdk/metric/processor/processortest/test.go b/sdk/metric/processor/processortest/test.go index 4f0f9b529b9..5e3e38bf983 100644 --- a/sdk/metric/processor/processortest/test.go +++ b/sdk/metric/processor/processortest/test.go @@ -23,6 +23,7 @@ import ( "go.opentelemetry.io/otel/label" "go.opentelemetry.io/otel/metric" + "go.opentelemetry.io/otel/metric/number" export "go.opentelemetry.io/otel/sdk/export/metric" "go.opentelemetry.io/otel/sdk/export/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/aggregator/array" @@ -254,21 +255,28 @@ func (o *Output) AddRecord(rec export.Record) error { func (o *Output) Map() map[string]float64 { r := make(map[string]float64) err := o.ForEach(export.StatelessExportKindSelector(), func(record export.Record) error { - for key, value := range o.m { - encoded := value.labels.Encoded(o.labelEncoder) - rencoded := value.resource.Encoded(o.labelEncoder) - number := 0.0 - if s, ok := value.aggregator.(aggregation.Sum); ok { + for key, entry := range o.m { + encoded := entry.labels.Encoded(o.labelEncoder) + rencoded := entry.resource.Encoded(o.labelEncoder) + value := 0.0 + if s, ok := entry.aggregator.(aggregation.Sum); ok { sum, _ := s.Sum() - number = sum.CoerceToFloat64(key.desc.NumberKind()) - } else if l, ok := value.aggregator.(aggregation.LastValue); ok { + value = sum.CoerceToFloat64(key.desc.NumberKind()) + } else if l, ok := entry.aggregator.(aggregation.LastValue); ok { last, _, _ := l.LastValue() - number = last.CoerceToFloat64(key.desc.NumberKind()) + value = last.CoerceToFloat64(key.desc.NumberKind()) + } else if l, ok := entry.aggregator.(aggregation.Points); ok { + pts, _ := l.Points() + var sum number.Number + for _, s := range pts { + sum.AddNumber(key.desc.NumberKind(), s.Number) + } + value = sum.CoerceToFloat64(key.desc.NumberKind()) } else { - panic(fmt.Sprintf("Unhandled aggregator type: %T", value.aggregator)) + panic(fmt.Sprintf("Unhandled aggregator type: %T", entry.aggregator)) } name := fmt.Sprint(key.desc.Name(), "/", encoded, "/", rencoded) - r[name] = number + r[name] = value } return nil }) From 2efe643af675ef54a3f733162b1ff9f18c2b1fc4 Mon Sep 17 00:00:00 2001 From: Josh MacDonald Date: Fri, 18 Dec 2020 14:02:13 -0800 Subject: [PATCH 03/15] Comment in selector/simple/simple.go --- sdk/metric/selector/simple/simple.go | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/sdk/metric/selector/simple/simple.go b/sdk/metric/selector/simple/simple.go index de852e919c2..100a41e1bfa 100644 --- a/sdk/metric/selector/simple/simple.go +++ b/sdk/metric/selector/simple/simple.go @@ -38,28 +38,27 @@ var ( _ export.AggregatorSelector = selectorHistogram{} ) -// NewWithInexpensiveDistribution returns a simple aggregation selector -// that uses counter, minmaxsumcount and minmaxsumcount aggregators -// for the three kinds of metric. This selector is faster and uses -// less memory than the others because minmaxsumcount does not -// aggregate quantile information. +// NewWithInexpensiveDistribution returns a simple aggregator selector +// that uses minmaxsumcount aggregators for `ValueRecorder` +// instruments. This selector is faster and uses less memory than the +// others in this package because minmaxsumcount aggregators maintain +// the least information about the distribution among these choices. func NewWithInexpensiveDistribution() export.AggregatorSelector { return selectorInexpensive{} } -// NewWithExactDistribution returns a simple aggregation selector that uses -// counter, array, and array aggregators for the three kinds of metric. -// This selector uses more memory than the NewWithSketchDistribution -// because it aggregates an array of all values, therefore is able to -// compute exact quantiles. +// NewWithExactDistribution returns a simple aggregator selector that +// uses exact aggregators for `ValueRecorder` instruments. This +// selector uses more memory than the others in this package because +// exact aggregators maintain the most information about the +// distribution among these choices. func NewWithExactDistribution() export.AggregatorSelector { return selectorExact{} } -// NewWithHistogramDistribution returns a simple aggregation selector that uses counter, -// histogram, and histogram aggregators for the three kinds of metric. This -// selector uses more memory than the NewWithInexpensiveDistribution because it -// uses a counter per bucket. +// NewWithHistogramDistribution returns a simple aggregator selector +// that uses histogram aggregators for `ValueRecorder` instruments. +// This selector is a good default choice for most metric exporters. func NewWithHistogramDistribution(boundaries []float64) export.AggregatorSelector { return selectorHistogram{boundaries: boundaries} } From 8169eb273e97923723bc5fa746ebadec499a4835 Mon Sep 17 00:00:00 2001 From: Josh MacDonald Date: Fri, 18 Dec 2020 14:03:08 -0800 Subject: [PATCH 04/15] Remove 'quantile' in mmsc --- sdk/metric/aggregator/minmaxsumcount/mmsc.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sdk/metric/aggregator/minmaxsumcount/mmsc.go b/sdk/metric/aggregator/minmaxsumcount/mmsc.go index 66a888a59d0..3cdeaf03b52 100644 --- a/sdk/metric/aggregator/minmaxsumcount/mmsc.go +++ b/sdk/metric/aggregator/minmaxsumcount/mmsc.go @@ -46,8 +46,7 @@ var _ export.Aggregator = &Aggregator{} var _ aggregation.MinMaxSumCount = &Aggregator{} // New returns a new aggregator for computing the min, max, sum, and -// count. It does not compute quantile information other than Min and -// Max. +// count. // // This type uses a mutex for Update() and SynchronizedMove() concurrency. func New(cnt int, desc *metric.Descriptor) []Aggregator { From 59e68243b84c0a4a4aaddfcef3c877079b7f0f61 Mon Sep 17 00:00:00 2001 From: Josh MacDonald Date: Fri, 18 Dec 2020 14:07:08 -0800 Subject: [PATCH 05/15] Rename array->exact --- exporters/otlp/go.sum | 1 + exporters/otlp/internal/transform/metric_test.go | 2 +- sdk/go.sum | 1 + .../aggregator/{array/array.go => exact/exact.go} | 4 ++-- .../{array/array_test.go => exact/exact_test.go} | 10 +++++----- sdk/metric/processor/processortest/test.go | 4 ++-- sdk/metric/selector/simple/simple.go | 4 ++-- sdk/metric/selector/simple/simple_test.go | 4 ++-- 8 files changed, 16 insertions(+), 14 deletions(-) rename sdk/metric/aggregator/{array/array.go => exact/exact.go} (96%) rename sdk/metric/aggregator/{array/array_test.go => exact/exact_test.go} (97%) diff --git a/exporters/otlp/go.sum b/exporters/otlp/go.sum index 717f650c750..e85523dacb5 100644 --- a/exporters/otlp/go.sum +++ b/exporters/otlp/go.sum @@ -42,6 +42,7 @@ github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+ github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA= github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd0= github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +go.opentelemetry.io v0.1.0 h1:EANZoRCOP+A3faIlw/iN6YEWoYb1vleZRKm1EvH8T48= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= diff --git a/exporters/otlp/internal/transform/metric_test.go b/exporters/otlp/internal/transform/metric_test.go index f6fa64e58f3..3e1911d52aa 100644 --- a/exporters/otlp/internal/transform/metric_test.go +++ b/exporters/otlp/internal/transform/metric_test.go @@ -32,7 +32,7 @@ import ( export "go.opentelemetry.io/otel/sdk/export/metric" "go.opentelemetry.io/otel/sdk/export/metric/aggregation" "go.opentelemetry.io/otel/sdk/export/metric/metrictest" - arrAgg "go.opentelemetry.io/otel/sdk/metric/aggregator/array" + arrAgg "go.opentelemetry.io/otel/sdk/metric/aggregator/exact" "go.opentelemetry.io/otel/sdk/metric/aggregator/lastvalue" lvAgg "go.opentelemetry.io/otel/sdk/metric/aggregator/lastvalue" "go.opentelemetry.io/otel/sdk/metric/aggregator/minmaxsumcount" diff --git a/sdk/go.sum b/sdk/go.sum index ea37d8353b5..df3278c9738 100644 --- a/sdk/go.sum +++ b/sdk/go.sum @@ -9,6 +9,7 @@ github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZN github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd0= github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +go.opentelemetry.io v0.1.0 h1:EANZoRCOP+A3faIlw/iN6YEWoYb1vleZRKm1EvH8T48= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= diff --git a/sdk/metric/aggregator/array/array.go b/sdk/metric/aggregator/exact/exact.go similarity index 96% rename from sdk/metric/aggregator/array/array.go rename to sdk/metric/aggregator/exact/exact.go index ebaf078279c..c781ae5d02b 100644 --- a/sdk/metric/aggregator/array/array.go +++ b/sdk/metric/aggregator/exact/exact.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package array // import "go.opentelemetry.io/otel/sdk/metric/aggregator/array" +package exact // import "go.opentelemetry.io/otel/sdk/metric/aggregator/exact" import ( "context" @@ -39,7 +39,7 @@ var _ export.Aggregator = &Aggregator{} var _ aggregation.Points = &Aggregator{} var _ aggregation.Count = &Aggregator{} -// New returns a new array aggregator, which aggregates recorded +// New returns a new exact aggregator, which aggregates recorded // measurements by storing them in an array. This type uses a mutex // for Update() and SynchronizedMove() concurrency. func New(cnt int) []Aggregator { diff --git a/sdk/metric/aggregator/array/array_test.go b/sdk/metric/aggregator/exact/exact_test.go similarity index 97% rename from sdk/metric/aggregator/array/array_test.go rename to sdk/metric/aggregator/exact/exact_test.go index ad076f4ee53..0b15ecdef20 100644 --- a/sdk/metric/aggregator/array/array_test.go +++ b/sdk/metric/aggregator/exact/exact_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package array +package exact import ( "fmt" @@ -97,7 +97,7 @@ func (ut *updateTest) run(t *testing.T, profile aggregatortest.Profile) { require.Equal(t, all.Count(), count, "Same count") } -func TestArrayUpdate(t *testing.T) { +func TestExactUpdate(t *testing.T) { // Test with an odd an even number of measurements for count := 999; count <= 1000; count++ { t.Run(fmt.Sprint("Odd=", count%2 == 1), func(t *testing.T) { @@ -167,7 +167,7 @@ func (mt *mergeTest) run(t *testing.T, profile aggregatortest.Profile) { require.Equal(t, all.Count(), count, "Same count - absolute") } -func TestArrayMerge(t *testing.T) { +func TestExactMerge(t *testing.T) { // Test with an odd an even number of measurements for count := 999; count <= 1000; count++ { t.Run(fmt.Sprint("Odd=", count%2 == 1), func(t *testing.T) { @@ -187,7 +187,7 @@ func TestArrayMerge(t *testing.T) { } } -func TestArrayErrors(t *testing.T) { +func TestExactErrors(t *testing.T) { aggregatortest.RunProfiles(t, func(t *testing.T, profile aggregatortest.Profile) { agg, ckpt := new2() @@ -206,7 +206,7 @@ func TestArrayErrors(t *testing.T) { }) } -func TestArrayFloat64(t *testing.T) { +func TestExactFloat64(t *testing.T) { descriptor := aggregatortest.NewAggregatorTest(metric.ValueRecorderInstrumentKind, number.Float64Kind) fpsf := func(sign int) []float64 { diff --git a/sdk/metric/processor/processortest/test.go b/sdk/metric/processor/processortest/test.go index 5e3e38bf983..17141a8de9b 100644 --- a/sdk/metric/processor/processortest/test.go +++ b/sdk/metric/processor/processortest/test.go @@ -26,7 +26,7 @@ import ( "go.opentelemetry.io/otel/metric/number" export "go.opentelemetry.io/otel/sdk/export/metric" "go.opentelemetry.io/otel/sdk/export/metric/aggregation" - "go.opentelemetry.io/otel/sdk/metric/aggregator/array" + "go.opentelemetry.io/otel/sdk/metric/aggregator/exact" "go.opentelemetry.io/otel/sdk/metric/aggregator/histogram" "go.opentelemetry.io/otel/sdk/metric/aggregator/lastvalue" "go.opentelemetry.io/otel/sdk/metric/aggregator/minmaxsumcount" @@ -189,7 +189,7 @@ func (testAggregatorSelector) AggregatorFor(desc *metric.Descriptor, aggPtrs ... *aggPtrs[i] = &aggs[i] } case strings.HasSuffix(desc.Name(), ".exact"): - aggs := array.New(len(aggPtrs)) + aggs := exact.New(len(aggPtrs)) for i := range aggPtrs { *aggPtrs[i] = &aggs[i] } diff --git a/sdk/metric/selector/simple/simple.go b/sdk/metric/selector/simple/simple.go index 100a41e1bfa..3d1d055a7f5 100644 --- a/sdk/metric/selector/simple/simple.go +++ b/sdk/metric/selector/simple/simple.go @@ -17,7 +17,7 @@ package simple // import "go.opentelemetry.io/otel/sdk/metric/selector/simple" import ( "go.opentelemetry.io/otel/metric" export "go.opentelemetry.io/otel/sdk/export/metric" - "go.opentelemetry.io/otel/sdk/metric/aggregator/array" + "go.opentelemetry.io/otel/sdk/metric/aggregator/exact" "go.opentelemetry.io/otel/sdk/metric/aggregator/histogram" "go.opentelemetry.io/otel/sdk/metric/aggregator/lastvalue" "go.opentelemetry.io/otel/sdk/metric/aggregator/minmaxsumcount" @@ -96,7 +96,7 @@ func (selectorExact) AggregatorFor(descriptor *metric.Descriptor, aggPtrs ...*ex case metric.ValueObserverInstrumentKind: lastValueAggs(aggPtrs) case metric.ValueRecorderInstrumentKind: - aggs := array.New(len(aggPtrs)) + aggs := exact.New(len(aggPtrs)) for i := range aggPtrs { *aggPtrs[i] = &aggs[i] } diff --git a/sdk/metric/selector/simple/simple_test.go b/sdk/metric/selector/simple/simple_test.go index 68084dc3a76..6bec06f3481 100644 --- a/sdk/metric/selector/simple/simple_test.go +++ b/sdk/metric/selector/simple/simple_test.go @@ -22,7 +22,7 @@ import ( "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/metric/number" export "go.opentelemetry.io/otel/sdk/export/metric" - "go.opentelemetry.io/otel/sdk/metric/aggregator/array" + "go.opentelemetry.io/otel/sdk/metric/aggregator/exact" "go.opentelemetry.io/otel/sdk/metric/aggregator/histogram" "go.opentelemetry.io/otel/sdk/metric/aggregator/lastvalue" "go.opentelemetry.io/otel/sdk/metric/aggregator/minmaxsumcount" @@ -61,7 +61,7 @@ func TestInexpensiveDistribution(t *testing.T) { func TestExactDistribution(t *testing.T) { ex := simple.NewWithExactDistribution() - require.IsType(t, (*array.Aggregator)(nil), oneAgg(ex, &testValueRecorderDesc)) + require.IsType(t, (*exact.Aggregator)(nil), oneAgg(ex, &testValueRecorderDesc)) testFixedSelectors(t, ex) } From 78ad879236f2d9ceed340dcf123c2dd17e9d556d Mon Sep 17 00:00:00 2001 From: Josh MacDonald Date: Fri, 18 Dec 2020 14:14:01 -0800 Subject: [PATCH 06/15] Update changelog --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 05853e9372c..eb8505f03d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,9 +19,14 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Improve span duration accuracy. (#1360) - Migrated CI/CD from CircleCI to GitHub Actions (#1382) - Remove duplicate checkout from GitHub Actions workflow (#1407) +- Metric `array` aggregator renamed `exact` inline with the name of its `aggregation.Kind` () +- Metric stdout exporter uses MinMaxSumCount aggregator for ValueRecorder instrument () + ### Removed - Remove `errUninitializedSpan` as its only usage is now obsolete. (#1360) +- Remove Metric export functionality related to quantiles and summary data points: this is not specified () +- Remove DDSketch metric aggregator; our intention is to re-introduce this as an option of the histogram aggregator after [new OTLP histogram data types](https://github.com/open-telemetry/opentelemetry-proto/pull/226) are released () ## [0.15.0] - 2020-12-10 From 390f975ad93c17e03e772d4e9d1a4d62f5261335 Mon Sep 17 00:00:00 2001 From: Josh MacDonald Date: Fri, 18 Dec 2020 14:37:28 -0800 Subject: [PATCH 07/15] Add PR number --- CHANGELOG.md | 9 +++++---- exporters/metric/prometheus/prometheus.go | 5 ++++- exporters/otlp/go.sum | 1 - sdk/go.sum | 1 - 4 files changed, 9 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eb8505f03d7..41cd70ddf53 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,14 +19,15 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Improve span duration accuracy. (#1360) - Migrated CI/CD from CircleCI to GitHub Actions (#1382) - Remove duplicate checkout from GitHub Actions workflow (#1407) -- Metric `array` aggregator renamed `exact` inline with the name of its `aggregation.Kind` () -- Metric stdout exporter uses MinMaxSumCount aggregator for ValueRecorder instrument () +- Metric `array` aggregator renamed `exact` to match its `aggregation.Kind` (#1412) +- Metric `exact` aggregator includes per-point timestamps (#1412) +- Metric stdout exporter uses MinMaxSumCount aggregator for ValueRecorder instruments (#1412) ### Removed - Remove `errUninitializedSpan` as its only usage is now obsolete. (#1360) -- Remove Metric export functionality related to quantiles and summary data points: this is not specified () -- Remove DDSketch metric aggregator; our intention is to re-introduce this as an option of the histogram aggregator after [new OTLP histogram data types](https://github.com/open-telemetry/opentelemetry-proto/pull/226) are released () +- Remove Metric export functionality related to quantiles and summary data points: this is not specified (#1412) +- Remove DDSketch metric aggregator; our intention is to re-introduce this as an option of the histogram aggregator after [new OTLP histogram data types](https://github.com/open-telemetry/opentelemetry-proto/pull/226) are released (#1412) ## [0.15.0] - 2020-12-10 diff --git a/exporters/metric/prometheus/prometheus.go b/exporters/metric/prometheus/prometheus.go index 2f5845028c7..5ed0e778bed 100644 --- a/exporters/metric/prometheus/prometheus.go +++ b/exporters/metric/prometheus/prometheus.go @@ -14,6 +14,9 @@ package prometheus // import "go.opentelemetry.io/otel/exporters/metric/prometheus" +// Note that this package does not support a way to export Prometheus +// Summary data points, removed in PR#1412. + import ( "context" "fmt" @@ -54,7 +57,7 @@ type Exporter struct { } // ErrUnsupportedAggregator is returned for unrepresentable aggregator -// types (e.g., array). +// types (e.g., exact). var ErrUnsupportedAggregator = fmt.Errorf("unsupported aggregator type") var _ http.Handler = &Exporter{} diff --git a/exporters/otlp/go.sum b/exporters/otlp/go.sum index e85523dacb5..717f650c750 100644 --- a/exporters/otlp/go.sum +++ b/exporters/otlp/go.sum @@ -42,7 +42,6 @@ github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+ github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA= github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd0= github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= -go.opentelemetry.io v0.1.0 h1:EANZoRCOP+A3faIlw/iN6YEWoYb1vleZRKm1EvH8T48= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= diff --git a/sdk/go.sum b/sdk/go.sum index df3278c9738..ea37d8353b5 100644 --- a/sdk/go.sum +++ b/sdk/go.sum @@ -9,7 +9,6 @@ github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZN github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd0= github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= -go.opentelemetry.io v0.1.0 h1:EANZoRCOP+A3faIlw/iN6YEWoYb1vleZRKm1EvH8T48= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= From 9fdb323d9dbf6eb37af1ac0cfae2ee6b7ec2b4dd Mon Sep 17 00:00:00 2001 From: Josh MacDonald Date: Fri, 18 Dec 2020 14:42:00 -0800 Subject: [PATCH 08/15] Rename exact benchmark --- sdk/metric/benchmark_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/sdk/metric/benchmark_test.go b/sdk/metric/benchmark_test.go index b7fda0b1e25..576d5c546a9 100644 --- a/sdk/metric/benchmark_test.go +++ b/sdk/metric/benchmark_test.go @@ -458,21 +458,21 @@ func BenchmarkFloat64MaxSumCountHandleAdd(b *testing.B) { benchmarkFloat64ValueRecorderHandleAdd(b, "float64.minmaxsumcount") } -// Array +// Exact -func BenchmarkInt64ArrayAdd(b *testing.B) { +func BenchmarkInt64ExactAdd(b *testing.B) { benchmarkInt64ValueRecorderAdd(b, "int64.exact") } -func BenchmarkInt64ArrayHandleAdd(b *testing.B) { +func BenchmarkInt64ExactHandleAdd(b *testing.B) { benchmarkInt64ValueRecorderHandleAdd(b, "int64.exact") } -func BenchmarkFloat64ArrayAdd(b *testing.B) { +func BenchmarkFloat64ExactAdd(b *testing.B) { benchmarkFloat64ValueRecorderAdd(b, "float64.exact") } -func BenchmarkFloat64ArrayHandleAdd(b *testing.B) { +func BenchmarkFloat64ExactHandleAdd(b *testing.B) { benchmarkFloat64ValueRecorderHandleAdd(b, "float64.exact") } From 628850db7c45ad3bc19994cf8bfb1972f8f147dd Mon Sep 17 00:00:00 2001 From: Josh MacDonald Date: Fri, 18 Dec 2020 14:46:48 -0800 Subject: [PATCH 09/15] New test for exact timestamps --- sdk/metric/aggregator/exact/exact_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/sdk/metric/aggregator/exact/exact_test.go b/sdk/metric/aggregator/exact/exact_test.go index 0b15ecdef20..acd229aeae1 100644 --- a/sdk/metric/aggregator/exact/exact_test.go +++ b/sdk/metric/aggregator/exact/exact_test.go @@ -18,6 +18,7 @@ import ( "fmt" "math" "testing" + "time" "github.com/stretchr/testify/require" @@ -240,6 +241,8 @@ func TestExactFloat64(t *testing.T) { agg, ckpt := new2() + startTime := time.Now() + for _, f := range fpsf(1) { all.Append(number.NewFloat64Number(f)) aggregatortest.CheckedUpdate(t, agg, number.NewFloat64Number(f), descriptor) @@ -250,6 +253,8 @@ func TestExactFloat64(t *testing.T) { aggregatortest.CheckedUpdate(t, agg, number.NewFloat64Number(f), descriptor) } + endTime := time.Now() + require.NoError(t, agg.SynchronizedMove(ckpt, descriptor)) pts, err := ckpt.Points() @@ -272,6 +277,8 @@ func TestExactFloat64(t *testing.T) { require.True(t, po[i-1].Time.Before(po[i].Time)) } } + require.True(t, po[0].Time.After(startTime)) + require.True(t, po[len(po)-1].Time.Before(endTime)) } func TestSynchronizedMoveReset(t *testing.T) { From c11fa6d9f8fdae29b485305b6caf0439771af4af Mon Sep 17 00:00:00 2001 From: Josh MacDonald Date: Fri, 18 Dec 2020 15:01:26 -0800 Subject: [PATCH 10/15] Add timestamp tests --- sdk/export/metric/aggregation/aggregation.go | 11 +++++++---- sdk/metric/aggregator/exact/exact_test.go | 12 ++++++++++-- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/sdk/export/metric/aggregation/aggregation.go b/sdk/export/metric/aggregation/aggregation.go index 3d10fbbbd3f..00451954c81 100644 --- a/sdk/export/metric/aggregation/aggregation.go +++ b/sdk/export/metric/aggregation/aggregation.go @@ -64,17 +64,20 @@ type ( LastValue() (number.Number, time.Time, error) } - // Points returns the raw set of values that were aggregated. + // Points returns the raw values that were aggregated. Points interface { Aggregation + + // Points returns samples in the order they were + // recorded. Points are approximately ordered by + // timestamp, but this is not guaranteed. Points() (Samples, error) } - // Samples is a set of samples. They implement the - // sort.Interface, sorting by arrival time. + // Samples is a list of Samples. Samples []Sample - // Sample is a point + // Sample is a raw data point, consisting of a number and value. Sample struct { number.Number time.Time diff --git a/sdk/metric/aggregator/exact/exact_test.go b/sdk/metric/aggregator/exact/exact_test.go index acd229aeae1..acce5df2131 100644 --- a/sdk/metric/aggregator/exact/exact_test.go +++ b/sdk/metric/aggregator/exact/exact_test.go @@ -151,11 +151,18 @@ func (mt *mergeTest) run(t *testing.T, profile aggregatortest.Profile) { aggregatortest.CheckedMerge(t, ckpt1, ckpt2, descriptor) - all.Sort() - pts, err := ckpt1.Points() require.Nil(t, err) + received := aggregatortest.NewNumbers(profile.NumberKind) + for i, s := range pts { + received.Append(s.Number) + + if i > 0 { + require.True(t, pts[i-1].Time.Before(pts[i].Time)) + } + } + allSum := all.Sum() sum := sumOf(pts, profile.NumberKind) require.InEpsilon(t, @@ -166,6 +173,7 @@ func (mt *mergeTest) run(t *testing.T, profile aggregatortest.Profile) { count, err := ckpt1.Count() require.Nil(t, err) require.Equal(t, all.Count(), count, "Same count - absolute") + require.Equal(t, all, received, "Same ordered contents") } func TestExactMerge(t *testing.T) { From e68abf48a646a69632f729af7fb52f78e1b31c7e Mon Sep 17 00:00:00 2001 From: Josh MacDonald Date: Fri, 18 Dec 2020 15:29:53 -0800 Subject: [PATCH 11/15] More test --- sdk/metric/aggregator/exact/exact_test.go | 57 +++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/sdk/metric/aggregator/exact/exact_test.go b/sdk/metric/aggregator/exact/exact_test.go index acce5df2131..b94c11b8e6f 100644 --- a/sdk/metric/aggregator/exact/exact_test.go +++ b/sdk/metric/aggregator/exact/exact_test.go @@ -298,3 +298,60 @@ func TestSynchronizedMoveReset(t *testing.T) { }, ) } + +func TestMergeBehavior(t *testing.T) { + aggregatortest.RunProfiles(t, func(t *testing.T, profile aggregatortest.Profile) { + for _, forward := range []bool{false, true} { + t.Run(fmt.Sprint("Forward=", forward), func(t *testing.T) { + descriptor := aggregatortest.NewAggregatorTest(metric.ValueRecorderInstrumentKind, profile.NumberKind) + agg1, agg2, ckpt, _ := new4() + + all := aggregatortest.NewNumbers(profile.NumberKind) + + for i := 0; i < 100; i++ { + x1 := profile.Random(+1) + all.Append(x1) + aggregatortest.CheckedUpdate(t, agg1, x1, descriptor) + } + + for i := 0; i < 100; i++ { + x2 := profile.Random(+1) + all.Append(x2) + aggregatortest.CheckedUpdate(t, agg2, x2, descriptor) + } + + if forward { + aggregatortest.CheckedMerge(t, ckpt, agg1, descriptor) + aggregatortest.CheckedMerge(t, ckpt, agg2, descriptor) + } else { + aggregatortest.CheckedMerge(t, ckpt, agg2, descriptor) + aggregatortest.CheckedMerge(t, ckpt, agg1, descriptor) + } + + pts, err := ckpt.Points() + require.Nil(t, err) + + received := aggregatortest.NewNumbers(profile.NumberKind) + for i, s := range pts { + received.Append(s.Number) + + if i > 0 { + require.True(t, pts[i-1].Time.Before(pts[i].Time)) + } + } + + allSum := all.Sum() + sum := sumOf(pts, profile.NumberKind) + require.InEpsilon(t, + (&allSum).CoerceToFloat64(profile.NumberKind), + sum.CoerceToFloat64(profile.NumberKind), + 0.0000001, + "Same sum - absolute") + count, err := ckpt.Count() + require.Nil(t, err) + require.Equal(t, all.Count(), count, "Same count - absolute") + require.Equal(t, all, received, "Same ordered contents") + }) + } + }) +} From 864b7c6a1c9dada551caf5ba5bed6a8fcccdc5f9 Mon Sep 17 00:00:00 2001 From: Josh MacDonald Date: Tue, 22 Dec 2020 21:37:09 -0800 Subject: [PATCH 12/15] From feedback --- sdk/metric/aggregator/exact/exact.go | 2 +- sdk/metric/aggregator/exact/exact_test.go | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/sdk/metric/aggregator/exact/exact.go b/sdk/metric/aggregator/exact/exact.go index c781ae5d02b..9507001b6bc 100644 --- a/sdk/metric/aggregator/exact/exact.go +++ b/sdk/metric/aggregator/exact/exact.go @@ -92,11 +92,11 @@ func (c *Aggregator) SynchronizedMove(oa export.Aggregator, desc *metric.Descrip func (c *Aggregator) Update(_ context.Context, number number.Number, desc *metric.Descriptor) error { now := time.Now() c.lock.Lock() + defer c.lock.Unlock() c.samples = append(c.samples, aggregation.Sample{ Number: number, Time: now, }) - c.lock.Unlock() return nil } diff --git a/sdk/metric/aggregator/exact/exact_test.go b/sdk/metric/aggregator/exact/exact_test.go index b94c11b8e6f..c28fb30e78c 100644 --- a/sdk/metric/aggregator/exact/exact_test.go +++ b/sdk/metric/aggregator/exact/exact_test.go @@ -89,7 +89,7 @@ func (ut *updateTest) run(t *testing.T, profile aggregatortest.Profile) { sum := sumOf(pts, profile.NumberKind) allSum := all.Sum() require.InEpsilon(t, - (&allSum).CoerceToFloat64(profile.NumberKind), + allSum.CoerceToFloat64(profile.NumberKind), sum.CoerceToFloat64(profile.NumberKind), 0.0000001, "Same sum") @@ -166,7 +166,7 @@ func (mt *mergeTest) run(t *testing.T, profile aggregatortest.Profile) { allSum := all.Sum() sum := sumOf(pts, profile.NumberKind) require.InEpsilon(t, - (&allSum).CoerceToFloat64(profile.NumberKind), + allSum.CoerceToFloat64(profile.NumberKind), sum.CoerceToFloat64(profile.NumberKind), 0.0000001, "Same sum - absolute") @@ -270,7 +270,7 @@ func TestExactFloat64(t *testing.T) { allSum := all.Sum() sum := sumOf(pts, number.Float64Kind) - require.InEpsilon(t, (&allSum).AsFloat64(), sum.AsFloat64(), 0.0000001, "Same sum") + require.InEpsilon(t, allSum.AsFloat64(), sum.AsFloat64(), 0.0000001, "Same sum") count, err := ckpt.Count() require.Equal(t, all.Count(), count, "Same count") @@ -329,7 +329,7 @@ func TestMergeBehavior(t *testing.T) { } pts, err := ckpt.Points() - require.Nil(t, err) + require.NoError(t, err) received := aggregatortest.NewNumbers(profile.NumberKind) for i, s := range pts { @@ -343,12 +343,12 @@ func TestMergeBehavior(t *testing.T) { allSum := all.Sum() sum := sumOf(pts, profile.NumberKind) require.InEpsilon(t, - (&allSum).CoerceToFloat64(profile.NumberKind), + allSum.CoerceToFloat64(profile.NumberKind), sum.CoerceToFloat64(profile.NumberKind), 0.0000001, "Same sum - absolute") count, err := ckpt.Count() - require.Nil(t, err) + require.NoError(t, err) require.Equal(t, all.Count(), count, "Same count - absolute") require.Equal(t, all, received, "Same ordered contents") }) From eeff5ecf0df0f3a83b173fffe84da1bebfbf8f31 Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Mon, 28 Dec 2020 21:42:28 -0800 Subject: [PATCH 13/15] Apply suggestions from code review Co-authored-by: Tyler Yahn --- sdk/metric/aggregator/exact/exact.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/metric/aggregator/exact/exact.go b/sdk/metric/aggregator/exact/exact.go index 9507001b6bc..7d623109fef 100644 --- a/sdk/metric/aggregator/exact/exact.go +++ b/sdk/metric/aggregator/exact/exact.go @@ -39,7 +39,7 @@ var _ export.Aggregator = &Aggregator{} var _ aggregation.Points = &Aggregator{} var _ aggregation.Count = &Aggregator{} -// New returns a new exact aggregator, which aggregates recorded +// New returns cnt many new exact aggregators, which aggregate recorded // measurements by storing them in an array. This type uses a mutex // for Update() and SynchronizedMove() concurrency. func New(cnt int) []Aggregator { From c42b69eeb0d4b3e9af12f6d181eada8d97f2b1bf Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Mon, 28 Dec 2020 22:08:52 -0800 Subject: [PATCH 14/15] Samples->Points --- exporters/otlp/internal/transform/metric.go | 2 +- sdk/export/metric/aggregation/aggregation.go | 11 ++++------- sdk/metric/aggregator/exact/exact.go | 10 +++++----- sdk/metric/aggregator/exact/exact_test.go | 2 +- 4 files changed, 11 insertions(+), 14 deletions(-) diff --git a/exporters/otlp/internal/transform/metric.go b/exporters/otlp/internal/transform/metric.go index 1a7503496e9..97d7cda2482 100644 --- a/exporters/otlp/internal/transform/metric.go +++ b/exporters/otlp/internal/transform/metric.go @@ -306,7 +306,7 @@ func Record(exportSelector export.ExportKindSelector, r export.Record) (*metricp } } -func gaugeArray(record export.Record, samples aggregation.Samples) (*metricpb.Metric, error) { +func gaugeArray(record export.Record, samples []aggregation.Point) (*metricpb.Metric, error) { desc := record.Descriptor() m := &metricpb.Metric{ Name: desc.Name(), diff --git a/sdk/export/metric/aggregation/aggregation.go b/sdk/export/metric/aggregation/aggregation.go index 00451954c81..776e4a3f8fc 100644 --- a/sdk/export/metric/aggregation/aggregation.go +++ b/sdk/export/metric/aggregation/aggregation.go @@ -68,17 +68,14 @@ type ( Points interface { Aggregation - // Points returns samples in the order they were + // Points returns points in the order they were // recorded. Points are approximately ordered by // timestamp, but this is not guaranteed. - Points() (Samples, error) + Points() ([]Point, error) } - // Samples is a list of Samples. - Samples []Sample - - // Sample is a raw data point, consisting of a number and value. - Sample struct { + // Point is a raw data point, consisting of a number and value. + Point struct { number.Number time.Time } diff --git a/sdk/metric/aggregator/exact/exact.go b/sdk/metric/aggregator/exact/exact.go index 7d623109fef..22505950cc9 100644 --- a/sdk/metric/aggregator/exact/exact.go +++ b/sdk/metric/aggregator/exact/exact.go @@ -31,7 +31,7 @@ type ( // an array with the exact set of values. Aggregator struct { lock sync.Mutex - samples aggregation.Samples + samples []aggregation.Point } ) @@ -62,7 +62,7 @@ func (c *Aggregator) Count() (int64, error) { } // Points returns access to the raw data set. -func (c *Aggregator) Points() (aggregation.Samples, error) { +func (c *Aggregator) Points() ([]aggregation.Point, error) { return c.samples, nil } @@ -93,7 +93,7 @@ func (c *Aggregator) Update(_ context.Context, number number.Number, desc *metri now := time.Now() c.lock.Lock() defer c.lock.Unlock() - c.samples = append(c.samples, aggregation.Sample{ + c.samples = append(c.samples, aggregation.Point{ Number: number, Time: now, }) @@ -112,8 +112,8 @@ func (c *Aggregator) Merge(oa export.Aggregator, desc *metric.Descriptor) error return nil } -func combine(a, b aggregation.Samples) aggregation.Samples { - result := make(aggregation.Samples, 0, len(a)+len(b)) +func combine(a, b []aggregation.Point) []aggregation.Point { + result := make([]aggregation.Point, 0, len(a)+len(b)) for len(a) != 0 && len(b) != 0 { if a[0].Time.Before(b[0].Time) { diff --git a/sdk/metric/aggregator/exact/exact_test.go b/sdk/metric/aggregator/exact/exact_test.go index c28fb30e78c..d7ff631be8e 100644 --- a/sdk/metric/aggregator/exact/exact_test.go +++ b/sdk/metric/aggregator/exact/exact_test.go @@ -53,7 +53,7 @@ func new4() (_, _, _, _ *Aggregator) { return &alloc[0], &alloc[1], &alloc[2], &alloc[3] } -func sumOf(samples aggregation.Samples, k number.Kind) number.Number { +func sumOf(samples []aggregation.Point, k number.Kind) number.Number { var n number.Number for _, s := range samples { n.AddNumber(k, s.Number) From 0903a89bcc8e0cad9b1e09130a896bdab6582f1b Mon Sep 17 00:00:00 2001 From: Joshua MacDonald Date: Tue, 12 Jan 2021 01:22:32 -0800 Subject: [PATCH 15/15] Rename --- exporters/otlp/internal/transform/metric.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/exporters/otlp/internal/transform/metric.go b/exporters/otlp/internal/transform/metric.go index 404bab28b4d..ca913457950 100644 --- a/exporters/otlp/internal/transform/metric.go +++ b/exporters/otlp/internal/transform/metric.go @@ -306,7 +306,7 @@ func Record(exportSelector export.ExportKindSelector, r export.Record) (*metricp } } -func gaugeArray(record export.Record, samples []aggregation.Point) (*metricpb.Metric, error) { +func gaugeArray(record export.Record, points []aggregation.Point) (*metricpb.Metric, error) { desc := record.Descriptor() m := &metricpb.Metric{ Name: desc.Name(), @@ -317,7 +317,7 @@ func gaugeArray(record export.Record, samples []aggregation.Point) (*metricpb.Me switch nk := desc.NumberKind(); nk { case number.Int64Kind: var pts []*metricpb.IntDataPoint - for _, s := range samples { + for _, s := range points { pts = append(pts, &metricpb.IntDataPoint{ Labels: nil, StartTimeUnixNano: toNanos(record.StartTime()), @@ -333,7 +333,7 @@ func gaugeArray(record export.Record, samples []aggregation.Point) (*metricpb.Me case number.Float64Kind: var pts []*metricpb.DoubleDataPoint - for _, s := range samples { + for _, s := range points { pts = append(pts, &metricpb.DoubleDataPoint{ Labels: nil, StartTimeUnixNano: toNanos(record.StartTime()),