From c69d82781431addab4415df67adcceb65bbe3ca5 Mon Sep 17 00:00:00 2001 From: hanshuo Date: Fri, 6 Nov 2020 15:04:09 +1100 Subject: [PATCH 1/9] support exact kind in OTLP metrics exporter --- exporters/otlp/internal/transform/metric.go | 63 +++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/exporters/otlp/internal/transform/metric.go b/exporters/otlp/internal/transform/metric.go index 1bcffcf19c6..a2321f9fe9b 100644 --- a/exporters/otlp/internal/transform/metric.go +++ b/exporters/otlp/internal/transform/metric.go @@ -289,11 +289,74 @@ func Record(r export.Record) (*metricpb.Metric, error) { } return gauge(r, value, time.Time{}, tm) + case aggregation.ExactKind: + e, ok := agg.(aggregation.Points) + if !ok { + return nil, fmt.Errorf("%w: %T", ErrIncompatibleAgg, agg) + } + pts, err := e.Points() + if err != nil { + return nil, err + } + + return array(r, pts) + default: return nil, fmt.Errorf("%w: %T", ErrUnimplementedAgg, agg) } } +func array(record export.Record, points []otel.Number) (*metricpb.Metric, error) { + desc := record.Descriptor() + m := &metricpb.Metric{ + Name: desc.Name(), + Description: desc.Description(), + Unit: string(desc.Unit()), + } + + start := record.StartTime() + end := record.EndTime() + + switch n := desc.NumberKind(); n { + case otel.Int64NumberKind: + var pts []*metricpb.IntDataPoint + for _, p := range points { + pts = append(pts, &metricpb.IntDataPoint{ + Labels: nil, + StartTimeUnixNano: toNanos(start), + TimeUnixNano: toNanos(end), + Value: p.CoerceToInt64(n), + }) + } + m.Data = &metricpb.Metric_IntGauge{ + IntGauge: &metricpb.IntGauge{ + DataPoints: pts, + }, + } + + case otel.Float64NumberKind: + var pts []*metricpb.DoubleDataPoint + for _, p := range points { + pts = append(pts, &metricpb.DoubleDataPoint{ + Labels: nil, + StartTimeUnixNano: toNanos(start), + TimeUnixNano: toNanos(end), + Value: p.CoerceToFloat64(n), + }) + } + m.Data = &metricpb.Metric_DoubleGauge{ + DoubleGauge: &metricpb.DoubleGauge{ + DataPoints: pts, + }, + } + + default: + return nil, fmt.Errorf("%w: %v", ErrUnknownValueType, n) + } + + return m, nil +} + func gauge(record export.Record, num otel.Number, start, end time.Time) (*metricpb.Metric, error) { desc := record.Descriptor() labels := record.Labels() From 96cb3ed828dfc029a5b1d911874ced83e5ab320a Mon Sep 17 00:00:00 2001 From: hanshuo Date: Fri, 6 Nov 2020 15:18:20 +1100 Subject: [PATCH 2/9] add change log --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 587619124ab..d78b42897d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - The `go.opentelemetry.io/otel/api/global` packages global TextMapPropagator now delegates functionality to a globally set delegate for all previously returned propagators. (#1258) - Fix condition in `label.Any`. (#1299) +- Fix missing handler for `ExactKind` aggregator in OTLP metrics transformer (#1309) ## [0.13.0] - 2020-10-08 From 96968dcd2b36b8446896e3c2354d7e44e87faec2 Mon Sep 17 00:00:00 2001 From: hanshuo Date: Fri, 6 Nov 2020 15:32:09 +1100 Subject: [PATCH 3/9] rename function --- exporters/otlp/internal/transform/metric.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/exporters/otlp/internal/transform/metric.go b/exporters/otlp/internal/transform/metric.go index a2321f9fe9b..9e82e90f00e 100644 --- a/exporters/otlp/internal/transform/metric.go +++ b/exporters/otlp/internal/transform/metric.go @@ -299,14 +299,14 @@ func Record(r export.Record) (*metricpb.Metric, error) { return nil, err } - return array(r, pts) + return gaugeArray(r, pts) default: return nil, fmt.Errorf("%w: %T", ErrUnimplementedAgg, agg) } } -func array(record export.Record, points []otel.Number) (*metricpb.Metric, error) { +func gaugeArray(record export.Record, points []otel.Number) (*metricpb.Metric, error) { desc := record.Descriptor() m := &metricpb.Metric{ Name: desc.Name(), From 6122735d8ea9bc95046db17013fd135a550786e9 Mon Sep 17 00:00:00 2001 From: Hanshuo Tan Date: Tue, 10 Nov 2020 13:26:14 +1100 Subject: [PATCH 4/9] inline start time and end time variables --- exporters/otlp/internal/transform/metric.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/exporters/otlp/internal/transform/metric.go b/exporters/otlp/internal/transform/metric.go index 9e82e90f00e..585d5d3f92c 100644 --- a/exporters/otlp/internal/transform/metric.go +++ b/exporters/otlp/internal/transform/metric.go @@ -314,17 +314,14 @@ func gaugeArray(record export.Record, points []otel.Number) (*metricpb.Metric, e Unit: string(desc.Unit()), } - start := record.StartTime() - end := record.EndTime() - switch n := desc.NumberKind(); n { case otel.Int64NumberKind: var pts []*metricpb.IntDataPoint for _, p := range points { pts = append(pts, &metricpb.IntDataPoint{ Labels: nil, - StartTimeUnixNano: toNanos(start), - TimeUnixNano: toNanos(end), + StartTimeUnixNano: toNanos(record.StartTime()), + TimeUnixNano: toNanos(record.EndTime()), Value: p.CoerceToInt64(n), }) } @@ -339,8 +336,8 @@ func gaugeArray(record export.Record, points []otel.Number) (*metricpb.Metric, e for _, p := range points { pts = append(pts, &metricpb.DoubleDataPoint{ Labels: nil, - StartTimeUnixNano: toNanos(start), - TimeUnixNano: toNanos(end), + StartTimeUnixNano: toNanos(record.StartTime()), + TimeUnixNano: toNanos(record.EndTime()), Value: p.CoerceToFloat64(n), }) } From 6448b1fa62d69154e00964e86c50697549307170 Mon Sep 17 00:00:00 2001 From: Hanshuo Tan Date: Tue, 10 Nov 2020 13:46:38 +1100 Subject: [PATCH 5/9] fix test --- exporters/otlp/internal/transform/metric_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/exporters/otlp/internal/transform/metric_test.go b/exporters/otlp/internal/transform/metric_test.go index 1d1bdb5b978..dbc51c394e5 100644 --- a/exporters/otlp/internal/transform/metric_test.go +++ b/exporters/otlp/internal/transform/metric_test.go @@ -32,7 +32,6 @@ 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" - "go.opentelemetry.io/otel/sdk/metric/aggregator/array" "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" @@ -346,11 +345,11 @@ func TestRecordAggregatorIncompatibleErrors(t *testing.T) { require.Nil(t, mpb) require.True(t, errors.Is(err, ErrIncompatibleAgg)) - mpb, err = makeMpb(aggregation.ExactKind, &array.New(1)[0]) + mpb, err = makeMpb(aggregation.ExactKind, &lastvalue.New(1)[0]) require.Error(t, err) require.Nil(t, mpb) - require.True(t, errors.Is(err, ErrUnimplementedAgg)) + require.True(t, errors.Is(err, ErrIncompatibleAgg)) } func TestRecordAggregatorUnexpectedErrors(t *testing.T) { From 84cb15634ff18ef0fb6426bb89c675ab0ad72940 Mon Sep 17 00:00:00 2001 From: Hanshuo Tan Date: Tue, 10 Nov 2020 18:19:57 +1100 Subject: [PATCH 6/9] add test for exact int data points --- .../otlp/internal/transform/metric_test.go | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/exporters/otlp/internal/transform/metric_test.go b/exporters/otlp/internal/transform/metric_test.go index dbc51c394e5..350bba5693a 100644 --- a/exporters/otlp/internal/transform/metric_test.go +++ b/exporters/otlp/internal/transform/metric_test.go @@ -32,6 +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" "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" @@ -231,6 +232,32 @@ func TestLastValueIntDataPoints(t *testing.T) { } } +func TestExactIntDataPoints(t *testing.T) { + desc := otel.NewDescriptor("", otel.ValueObserverInstrumentKind, otel.Int64NumberKind) + labels := label.NewSet() + e, ckpt := metrictest.Unslice2(arrAgg.New(2)) + assert.NoError(t, e.Update(context.Background(), otel.Number(100), &desc)) + require.NoError(t, e.SynchronizedMove(ckpt, &desc)) + record := export.NewRecord(&desc, &labels, nil, ckpt.Aggregation(), intervalStart, intervalEnd) + p, ok := ckpt.(aggregation.Points) + require.True(t, ok, "ckpt is not an aggregation.Points: %T", ckpt) + pts, err := p.Points() + require.NoError(t, err) + + if m, err := gaugeArray(record, pts); assert.NoError(t, err) { + assert.Equal(t, []*metricpb.IntDataPoint{{ + Value: 100, + StartTimeUnixNano: toNanos(intervalStart), + TimeUnixNano: toNanos(intervalEnd), + }}, m.GetIntGauge().DataPoints) + assert.Nil(t, m.GetIntHistogram()) + assert.Nil(t, m.GetIntSum()) + assert.Nil(t, m.GetDoubleGauge()) + assert.Nil(t, m.GetDoubleHistogram()) + assert.Nil(t, m.GetDoubleSum()) + } +} + func TestSumErrUnknownValueType(t *testing.T) { desc := otel.NewDescriptor("", otel.ValueRecorderInstrumentKind, otel.NumberKind(-1)) labels := label.NewSet() From 3ddf29caa108976a1ec78839e780f4337df5eee1 Mon Sep 17 00:00:00 2001 From: Hanshuo Tan Date: Tue, 10 Nov 2020 18:22:50 +1100 Subject: [PATCH 7/9] add test for exact float data points --- .../otlp/internal/transform/metric_test.go | 28 ++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/exporters/otlp/internal/transform/metric_test.go b/exporters/otlp/internal/transform/metric_test.go index 350bba5693a..d9541605431 100644 --- a/exporters/otlp/internal/transform/metric_test.go +++ b/exporters/otlp/internal/transform/metric_test.go @@ -233,7 +233,7 @@ func TestLastValueIntDataPoints(t *testing.T) { } func TestExactIntDataPoints(t *testing.T) { - desc := otel.NewDescriptor("", otel.ValueObserverInstrumentKind, otel.Int64NumberKind) + desc := otel.NewDescriptor("", otel.ValueRecorderInstrumentKind, otel.Int64NumberKind) labels := label.NewSet() e, ckpt := metrictest.Unslice2(arrAgg.New(2)) assert.NoError(t, e.Update(context.Background(), otel.Number(100), &desc)) @@ -258,6 +258,32 @@ func TestExactIntDataPoints(t *testing.T) { } } +func TestExactFloatDataPoints(t *testing.T) { + desc := otel.NewDescriptor("", otel.ValueRecorderInstrumentKind, otel.Float64NumberKind) + labels := label.NewSet() + e, ckpt := metrictest.Unslice2(arrAgg.New(2)) + assert.NoError(t, e.Update(context.Background(), otel.NewFloat64Number(100), &desc)) + require.NoError(t, e.SynchronizedMove(ckpt, &desc)) + record := export.NewRecord(&desc, &labels, nil, ckpt.Aggregation(), intervalStart, intervalEnd) + p, ok := ckpt.(aggregation.Points) + require.True(t, ok, "ckpt is not an aggregation.Points: %T", ckpt) + pts, err := p.Points() + require.NoError(t, err) + + if m, err := gaugeArray(record, pts); assert.NoError(t, err) { + assert.Equal(t, []*metricpb.DoubleDataPoint{{ + Value: 100, + StartTimeUnixNano: toNanos(intervalStart), + TimeUnixNano: toNanos(intervalEnd), + }}, m.GetDoubleGauge().DataPoints) + assert.Nil(t, m.GetIntHistogram()) + assert.Nil(t, m.GetIntSum()) + assert.Nil(t, m.GetIntGauge()) + assert.Nil(t, m.GetDoubleHistogram()) + assert.Nil(t, m.GetDoubleSum()) + } +} + func TestSumErrUnknownValueType(t *testing.T) { desc := otel.NewDescriptor("", otel.ValueRecorderInstrumentKind, otel.NumberKind(-1)) labels := label.NewSet() From 84f931f652d6e3dc6363ff4a77434c8d6b2e22da Mon Sep 17 00:00:00 2001 From: Hanshuo Tan Date: Thu, 12 Nov 2020 15:04:27 +1100 Subject: [PATCH 8/9] use newly introduced number package for numbers according to upstream change --- exporters/otlp/internal/transform/metric.go | 4 ++-- exporters/otlp/internal/transform/metric_test.go | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/exporters/otlp/internal/transform/metric.go b/exporters/otlp/internal/transform/metric.go index e8f5d7a93ce..ad0432a6391 100644 --- a/exporters/otlp/internal/transform/metric.go +++ b/exporters/otlp/internal/transform/metric.go @@ -315,7 +315,7 @@ func gaugeArray(record export.Record, points []number.Number) (*metricpb.Metric, } switch n := desc.NumberKind(); n { - case otel.Int64NumberKind: + case number.Int64Kind: var pts []*metricpb.IntDataPoint for _, p := range points { pts = append(pts, &metricpb.IntDataPoint{ @@ -331,7 +331,7 @@ func gaugeArray(record export.Record, points []number.Number) (*metricpb.Metric, }, } - case otel.Float64NumberKind: + case number.Float64Kind: var pts []*metricpb.DoubleDataPoint for _, p := range points { pts = append(pts, &metricpb.DoubleDataPoint{ diff --git a/exporters/otlp/internal/transform/metric_test.go b/exporters/otlp/internal/transform/metric_test.go index 014ffe39994..1de8017dc69 100644 --- a/exporters/otlp/internal/transform/metric_test.go +++ b/exporters/otlp/internal/transform/metric_test.go @@ -245,10 +245,10 @@ func TestLastValueIntDataPoints(t *testing.T) { } func TestExactIntDataPoints(t *testing.T) { - desc := otel.NewDescriptor("", otel.ValueRecorderInstrumentKind, otel.Int64NumberKind) + desc := otel.NewDescriptor("", otel.ValueRecorderInstrumentKind, number.Int64Kind) labels := label.NewSet() e, ckpt := metrictest.Unslice2(arrAgg.New(2)) - assert.NoError(t, e.Update(context.Background(), otel.Number(100), &desc)) + assert.NoError(t, e.Update(context.Background(), number.Number(100), &desc)) require.NoError(t, e.SynchronizedMove(ckpt, &desc)) record := export.NewRecord(&desc, &labels, nil, ckpt.Aggregation(), intervalStart, intervalEnd) p, ok := ckpt.(aggregation.Points) @@ -271,10 +271,10 @@ func TestExactIntDataPoints(t *testing.T) { } func TestExactFloatDataPoints(t *testing.T) { - desc := otel.NewDescriptor("", otel.ValueRecorderInstrumentKind, otel.Float64NumberKind) + desc := otel.NewDescriptor("", otel.ValueRecorderInstrumentKind, number.Float64Kind) labels := label.NewSet() e, ckpt := metrictest.Unslice2(arrAgg.New(2)) - assert.NoError(t, e.Update(context.Background(), otel.NewFloat64Number(100), &desc)) + assert.NoError(t, e.Update(context.Background(), number.NewFloat64Number(100), &desc)) require.NoError(t, e.SynchronizedMove(ckpt, &desc)) record := export.NewRecord(&desc, &labels, nil, ckpt.Aggregation(), intervalStart, intervalEnd) p, ok := ckpt.(aggregation.Points) From 910bbd5726d67e28dad919af04bc64e0028a62ac Mon Sep 17 00:00:00 2001 From: Hanshuo Tan Date: Wed, 18 Nov 2020 11:11:14 +1100 Subject: [PATCH 9/9] fix package ref --- exporters/otlp/internal/transform/metric_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/exporters/otlp/internal/transform/metric_test.go b/exporters/otlp/internal/transform/metric_test.go index 69cf1075266..f6fa64e58f3 100644 --- a/exporters/otlp/internal/transform/metric_test.go +++ b/exporters/otlp/internal/transform/metric_test.go @@ -244,7 +244,7 @@ func TestLastValueIntDataPoints(t *testing.T) { } func TestExactIntDataPoints(t *testing.T) { - desc := otel.NewDescriptor("", otel.ValueRecorderInstrumentKind, number.Int64Kind) + desc := metric.NewDescriptor("", metric.ValueRecorderInstrumentKind, number.Int64Kind) labels := label.NewSet() e, ckpt := metrictest.Unslice2(arrAgg.New(2)) assert.NoError(t, e.Update(context.Background(), number.Number(100), &desc)) @@ -270,7 +270,7 @@ func TestExactIntDataPoints(t *testing.T) { } func TestExactFloatDataPoints(t *testing.T) { - desc := otel.NewDescriptor("", otel.ValueRecorderInstrumentKind, number.Float64Kind) + desc := metric.NewDescriptor("", metric.ValueRecorderInstrumentKind, number.Float64Kind) labels := label.NewSet() e, ckpt := metrictest.Unslice2(arrAgg.New(2)) assert.NoError(t, e.Update(context.Background(), number.NewFloat64Number(100), &desc))