From f00488577f2a145250629cdc5c5e8a748d2fd2fe Mon Sep 17 00:00:00 2001 From: Bogdan Drutu Date: Wed, 12 May 2021 16:47:34 -0700 Subject: [PATCH] Remove internaldata.MetricsData, same APIs as for traces Signed-off-by: Bogdan Drutu --- exporter/opencensusexporter/opencensus.go | 24 +++-- .../prometheusexporter/prometheus_test.go | 8 +- .../ocmetrics/opencensus.go | 19 ++-- .../ocmetrics/opencensus_test.go | 14 +-- .../internal/transaction.go | 6 +- .../internal/transaction_test.go | 12 ++- .../metrics_receiver_test.go | 75 ++++++++-------- translator/internaldata/consumerdata.go | 29 ------- translator/internaldata/metrics_to_oc.go | 33 +++---- translator/internaldata/metrics_to_oc_test.go | 87 ++++++++----------- translator/internaldata/oc_testdata_test.go | 29 ++++--- translator/internaldata/oc_to_metrics.go | 44 +++------- translator/internaldata/oc_to_metrics_test.go | 72 ++++++--------- 13 files changed, 178 insertions(+), 274 deletions(-) delete mode 100644 translator/internaldata/consumerdata.go diff --git a/exporter/opencensusexporter/opencensus.go b/exporter/opencensusexporter/opencensus.go index b487eee66b2..ac6387b36ae 100644 --- a/exporter/opencensusexporter/opencensus.go +++ b/exporter/opencensusexporter/opencensus.go @@ -207,23 +207,19 @@ func (oce *ocExporter) pushMetricsData(_ context.Context, md pdata.Metrics) erro } } - ocmds := internaldata.MetricsToOC(md) - for _, ocmd := range ocmds { + rms := md.ResourceMetrics() + for i := 0; i < rms.Len(); i++ { + ocReq := agentmetricspb.ExportMetricsServiceRequest{} + ocReq.Node, ocReq.Resource, ocReq.Metrics = internaldata.ResourceMetricsToOC(rms.At(i)) + // This is a hack because OC protocol expects a Node for the initial message. - node := ocmd.Node - if node == nil { - node = &commonpb.Node{} - } - resource := ocmd.Resource - if resource == nil { - resource = &resourcepb.Resource{} + if ocReq.Node == nil { + ocReq.Node = &commonpb.Node{} } - req := &agentmetricspb.ExportMetricsServiceRequest{ - Metrics: ocmd.Metrics, - Resource: resource, - Node: node, + if ocReq.Resource == nil { + ocReq.Resource = &resourcepb.Resource{} } - if err := mClient.msec.Send(req); err != nil { + if err := mClient.msec.Send(&ocReq); err != nil { // Error received, cancel the context used to create the RPC to free all resources, // put back nil to keep the number of workers constant. mClient.cancel() diff --git a/exporter/prometheusexporter/prometheus_test.go b/exporter/prometheusexporter/prometheus_test.go index db54cff12a9..965d3b4bcba 100644 --- a/exporter/prometheusexporter/prometheus_test.go +++ b/exporter/prometheusexporter/prometheus_test.go @@ -130,11 +130,11 @@ func TestPrometheusExporter_endToEnd(t *testing.T) { require.NoError(t, exp.Start(context.Background(), componenttest.NewNopHost())) // Should accumulate multiple metrics - md := internaldata.OCToMetrics(internaldata.MetricsData{Metrics: metricBuilder(128, "metric_1_")}) + md := internaldata.OCToMetrics(nil, nil, metricBuilder(128, "metric_1_")) assert.NoError(t, exp.ConsumeMetrics(context.Background(), md)) for delta := 0; delta <= 20; delta += 10 { - md := internaldata.OCToMetrics(internaldata.MetricsData{Metrics: metricBuilder(int64(delta), "metric_2_")}) + md := internaldata.OCToMetrics(nil, nil, metricBuilder(int64(delta), "metric_2_")) assert.NoError(t, exp.ConsumeMetrics(context.Background(), md)) res, err1 := http.Get("http://localhost:7777/metrics") @@ -208,11 +208,11 @@ func TestPrometheusExporter_endToEndWithTimestamps(t *testing.T) { // Should accumulate multiple metrics - md := internaldata.OCToMetrics(internaldata.MetricsData{Metrics: metricBuilder(128, "metric_1_")}) + md := internaldata.OCToMetrics(nil, nil, metricBuilder(128, "metric_1_")) assert.NoError(t, exp.ConsumeMetrics(context.Background(), md)) for delta := 0; delta <= 20; delta += 10 { - md := internaldata.OCToMetrics(internaldata.MetricsData{Metrics: metricBuilder(int64(delta), "metric_2_")}) + md := internaldata.OCToMetrics(nil, nil, metricBuilder(int64(delta), "metric_2_")) assert.NoError(t, exp.ConsumeMetrics(context.Background(), md)) res, err1 := http.Get("http://localhost:7777/metrics") diff --git a/receiver/opencensusreceiver/ocmetrics/opencensus.go b/receiver/opencensusreceiver/ocmetrics/opencensus.go index 304de57a2af..b169e91f904 100644 --- a/receiver/opencensusreceiver/ocmetrics/opencensus.go +++ b/receiver/opencensusreceiver/ocmetrics/opencensus.go @@ -21,6 +21,7 @@ import ( commonpb "github.com/census-instrumentation/opencensus-proto/gen-go/agent/common/v1" agentmetricspb "github.com/census-instrumentation/opencensus-proto/gen-go/agent/metrics/v1" + ocmetrics "github.com/census-instrumentation/opencensus-proto/gen-go/metrics/v1" resourcepb "github.com/census-instrumentation/opencensus-proto/gen-go/resource/v1" "go.opentelemetry.io/collector/component/componenterror" @@ -116,36 +117,28 @@ func (ocr *Receiver) processReceivedMsg( resource = recv.Resource } - md := internaldata.MetricsData{ - Node: lastNonNilNode, - Resource: resource, - Metrics: recv.Metrics, - } - - err := ocr.sendToNextConsumer(longLivedRPCCtx, md) + err := ocr.sendToNextConsumer(longLivedRPCCtx, lastNonNilNode, resource, recv.Metrics) return lastNonNilNode, resource, err } -func (ocr *Receiver) sendToNextConsumer(longLivedRPCCtx context.Context, md internaldata.MetricsData) error { +func (ocr *Receiver) sendToNextConsumer(longLivedRPCCtx context.Context, node *commonpb.Node, resource *resourcepb.Resource, metrics []*ocmetrics.Metric) error { ctx := obsreport.StartMetricsReceiveOp( longLivedRPCCtx, ocr.id, receiverTransport, obsreport.WithLongLivedCtx()) - numTimeSeries := 0 numPoints := 0 // Count number of time series and data points. - for _, metric := range md.Metrics { - numTimeSeries += len(metric.Timeseries) + for _, metric := range metrics { for _, ts := range metric.GetTimeseries() { numPoints += len(ts.GetPoints()) } } var consumerErr error - if len(md.Metrics) > 0 { - consumerErr = ocr.nextConsumer.ConsumeMetrics(ctx, internaldata.OCToMetrics(md)) + if len(metrics) > 0 { + consumerErr = ocr.nextConsumer.ConsumeMetrics(ctx, internaldata.OCToMetrics(node, resource, metrics)) } obsreport.EndMetricsReceiveOp( diff --git a/receiver/opencensusreceiver/ocmetrics/opencensus_test.go b/receiver/opencensusreceiver/ocmetrics/opencensus_test.go index 5878ed544b6..1e795152461 100644 --- a/receiver/opencensusreceiver/ocmetrics/opencensus_test.go +++ b/receiver/opencensusreceiver/ocmetrics/opencensus_test.go @@ -151,9 +151,10 @@ func TestExportMultiplexing(t *testing.T) { // Examination time! resultsMapping := make(map[string][]*metricspb.Metric) for _, md := range metricSink.AllMetrics() { - ocmds := internaldata.MetricsToOC(md) - for _, ocmd := range ocmds { - resultsMapping[nodeToKey(ocmd.Node)] = append(resultsMapping[nodeToKey(ocmd.Node)], ocmd.Metrics...) + rms := md.ResourceMetrics() + for i := 0; i < rms.Len(); i++ { + node, _, metrics := internaldata.ResourceMetricsToOC(rms.At(i)) + resultsMapping[nodeToKey(node)] = append(resultsMapping[nodeToKey(node)], metrics...) } } @@ -292,9 +293,10 @@ func TestExportProtocolConformation_metricsInFirstMessage(t *testing.T) { // Examination time! resultsMapping := make(map[string][]*metricspb.Metric) for _, md := range metricSink.AllMetrics() { - ocmds := internaldata.MetricsToOC(md) - for _, ocmd := range ocmds { - resultsMapping[nodeToKey(ocmd.Node)] = append(resultsMapping[nodeToKey(ocmd.Node)], ocmd.Metrics...) + rms := md.ResourceMetrics() + for i := 0; i < rms.Len(); i++ { + node, _, metrics := internaldata.ResourceMetricsToOC(rms.At(i)) + resultsMapping[nodeToKey(node)] = append(resultsMapping[nodeToKey(node)], metrics...) } } diff --git a/receiver/prometheusreceiver/internal/transaction.go b/receiver/prometheusreceiver/internal/transaction.go index a2a735db4ec..8baedf85ed2 100644 --- a/receiver/prometheusreceiver/internal/transaction.go +++ b/receiver/prometheusreceiver/internal/transaction.go @@ -183,11 +183,7 @@ func (tr *transaction) Commit() error { numPoints := 0 if len(metrics) > 0 { - md := internaldata.OCToMetrics(internaldata.MetricsData{ - Node: tr.node, - Resource: tr.resource, - Metrics: metrics, - }) + md := internaldata.OCToMetrics(tr.node, tr.resource, metrics) _, numPoints = md.MetricAndDataPointCount() err = tr.sink.ConsumeMetrics(ctx, md) } diff --git a/receiver/prometheusreceiver/internal/transaction_test.go b/receiver/prometheusreceiver/internal/transaction_test.go index ebef43cb840..62b42e5ed0c 100644 --- a/receiver/prometheusreceiver/internal/transaction_test.go +++ b/receiver/prometheusreceiver/internal/transaction_test.go @@ -20,9 +20,11 @@ import ( "testing" "time" + agentmetricspb "github.com/census-instrumentation/opencensus-proto/gen-go/agent/metrics/v1" "github.com/prometheus/common/model" "github.com/prometheus/prometheus/pkg/labels" "github.com/prometheus/prometheus/scrape" + "github.com/stretchr/testify/require" "google.golang.org/protobuf/proto" "go.opentelemetry.io/collector/config" @@ -117,10 +119,14 @@ func Test_transaction(t *testing.T) { if len(mds) != 1 { t.Fatalf("wanted one batch, got %v\n", sink.AllMetrics()) } - ocmds := internaldata.MetricsToOC(mds[0]) - if len(ocmds) != 1 { - t.Fatalf("wanted one batch per node, got %v\n", sink.AllMetrics()) + var ocmds []*agentmetricspb.ExportMetricsServiceRequest + rms := mds[0].ResourceMetrics() + for i := 0; i < rms.Len(); i++ { + ocmd := &agentmetricspb.ExportMetricsServiceRequest{} + ocmd.Node, ocmd.Resource, ocmd.Metrics = internaldata.ResourceMetricsToOC(rms.At(i)) + ocmds = append(ocmds, ocmd) } + require.Len(t, ocmds, 1) if !proto.Equal(ocmds[0].Node, expectedNode) { t.Errorf("generated node %v and expected node %v is different\n", ocmds[0].Node, expectedNode) } diff --git a/receiver/prometheusreceiver/metrics_receiver_test.go b/receiver/prometheusreceiver/metrics_receiver_test.go index 092d787d1b0..77e3b39aa73 100644 --- a/receiver/prometheusreceiver/metrics_receiver_test.go +++ b/receiver/prometheusreceiver/metrics_receiver_test.go @@ -28,6 +28,7 @@ import ( "testing" commonpb "github.com/census-instrumentation/opencensus-proto/gen-go/agent/common/v1" + agentmetricspb "github.com/census-instrumentation/opencensus-proto/gen-go/agent/metrics/v1" metricspb "github.com/census-instrumentation/opencensus-proto/gen-go/metrics/v1" resourcepb "github.com/census-instrumentation/opencensus-proto/gen-go/resource/v1" gokitlog "github.com/go-kit/kit/log" @@ -112,7 +113,7 @@ type testData struct { pages []mockPrometheusResponse node *commonpb.Node resource *resourcepb.Resource - validateFunc func(t *testing.T, td *testData, result []internaldata.MetricsData) + validateFunc func(t *testing.T, td *testData, result []*agentmetricspb.ExportMetricsServiceRequest) } // setupMockPrometheus to create a mocked prometheus based on targets, returning the server and a prometheus exporting @@ -170,7 +171,7 @@ func setupMockPrometheus(tds ...*testData) (*mockPrometheus, *promcfg.Config, er return mp, pCfg, err } -func verifyNumScrapeResults(t *testing.T, td *testData, mds []internaldata.MetricsData) { +func verifyNumScrapeResults(t *testing.T, td *testData, mds []*agentmetricspb.ExportMetricsServiceRequest) { want := 0 for _, p := range td.pages { if p.code == 200 { @@ -182,7 +183,7 @@ func verifyNumScrapeResults(t *testing.T, td *testData, mds []internaldata.Metri } } -func doCompare(name string, t *testing.T, want, got *internaldata.MetricsData) { +func doCompare(name string, t *testing.T, want, got *agentmetricspb.ExportMetricsServiceRequest) { t.Run(name, func(t *testing.T) { assert.EqualValues(t, want, got) }) @@ -251,7 +252,7 @@ rpc_duration_seconds_sum 5002 rpc_duration_seconds_count 1001 ` -func verifyTarget1(t *testing.T, td *testData, mds []internaldata.MetricsData) { +func verifyTarget1(t *testing.T, td *testData, mds []*agentmetricspb.ExportMetricsServiceRequest) { verifyNumScrapeResults(t, td, mds) m1 := mds[0] // m1 shall only have a gauge @@ -280,12 +281,12 @@ func verifyTarget1(t *testing.T, td *testData, mds []internaldata.MetricsData) { // set this timestamp to wantG1 wantG1.Timeseries[0].Points[0].Timestamp = ts1 doCompare("scrape1", t, - &internaldata.MetricsData{ + &agentmetricspb.ExportMetricsServiceRequest{ Node: td.node, Resource: td.resource, Metrics: []*metricspb.Metric{wantG1}, }, - &internaldata.MetricsData{ + &agentmetricspb.ExportMetricsServiceRequest{ Node: td.node, Resource: td.resource, Metrics: []*metricspb.Metric{gotG1}, @@ -295,7 +296,7 @@ func verifyTarget1(t *testing.T, td *testData, mds []internaldata.MetricsData) { m2 := mds[1] ts2 := m2.Metrics[0].Timeseries[0].Points[0].Timestamp - want2 := &internaldata.MetricsData{ + want2 := &agentmetricspb.ExportMetricsServiceRequest{ Node: td.node, Resource: td.resource, Metrics: []*metricspb.Metric{ @@ -421,7 +422,7 @@ func verifyTarget1(t *testing.T, td *testData, mds []internaldata.MetricsData) { }, } - doCompare("scrape2", t, want2, &m2) + doCompare("scrape2", t, want2, m2) } // target2 is going to have 5 pages, and there's a newly appeared item from the 2nd page. we are expecting the new @@ -488,7 +489,7 @@ http_requests_total{method="post",code="400"} 59 http_requests_total{method="post",code="500"} 5 ` -func verifyTarget2(t *testing.T, td *testData, mds []internaldata.MetricsData) { +func verifyTarget2(t *testing.T, td *testData, mds []*agentmetricspb.ExportMetricsServiceRequest) { verifyNumScrapeResults(t, td, mds) m1 := mds[0] // m1 shall only have a gauge @@ -516,12 +517,12 @@ func verifyTarget2(t *testing.T, td *testData, mds []internaldata.MetricsData) { // set this timestamp to wantG1 wantG1.Timeseries[0].Points[0].Timestamp = ts1 doCompare("scrape1", t, - &internaldata.MetricsData{ + &agentmetricspb.ExportMetricsServiceRequest{ Node: td.node, Resource: td.resource, Metrics: []*metricspb.Metric{wantG1}, }, - &internaldata.MetricsData{ + &agentmetricspb.ExportMetricsServiceRequest{ Node: td.node, Resource: td.resource, Metrics: []*metricspb.Metric{gotG1}, @@ -531,7 +532,7 @@ func verifyTarget2(t *testing.T, td *testData, mds []internaldata.MetricsData) { m2 := mds[1] ts2 := m2.Metrics[0].Timeseries[0].Points[0].Timestamp - want2 := &internaldata.MetricsData{ + want2 := &agentmetricspb.ExportMetricsServiceRequest{ Node: td.node, Resource: td.resource, Metrics: []*metricspb.Metric{ @@ -581,14 +582,14 @@ func verifyTarget2(t *testing.T, td *testData, mds []internaldata.MetricsData) { }, }, } - doCompare("scrape2", t, want2, &m2) + doCompare("scrape2", t, want2, m2) // verify the 3rd metricData, with the new code=500 counter which first appeared on 2nd run m3 := mds[2] // its start timestamp shall be from the 2nd run ts3 := m3.Metrics[0].Timeseries[0].Points[0].Timestamp - want3 := &internaldata.MetricsData{ + want3 := &agentmetricspb.ExportMetricsServiceRequest{ Node: td.node, Resource: td.resource, Metrics: []*metricspb.Metric{ @@ -648,13 +649,13 @@ func verifyTarget2(t *testing.T, td *testData, mds []internaldata.MetricsData) { }, }, } - doCompare("scrape3", t, want3, &m3) + doCompare("scrape3", t, want3, m3) // verify the 4th metricData which reset happens, all cumulative types shall be absent m4 := mds[3] ts4 := m4.Metrics[0].Timeseries[0].Points[0].Timestamp - want4 := &internaldata.MetricsData{ + want4 := &agentmetricspb.ExportMetricsServiceRequest{ Node: td.node, Resource: td.resource, Metrics: []*metricspb.Metric{ @@ -674,14 +675,14 @@ func verifyTarget2(t *testing.T, td *testData, mds []internaldata.MetricsData) { }, }, } - doCompare("scrape4", t, want4, &m4) + doCompare("scrape4", t, want4, m4) // verify the 4th metricData which reset happens, all cumulative types shall be absent m5 := mds[4] // its start timestamp shall be from the 3rd run ts5 := m5.Metrics[0].Timeseries[0].Points[0].Timestamp - want5 := &internaldata.MetricsData{ + want5 := &agentmetricspb.ExportMetricsServiceRequest{ Node: td.node, Resource: td.resource, Metrics: []*metricspb.Metric{ @@ -741,7 +742,7 @@ func verifyTarget2(t *testing.T, td *testData, mds []internaldata.MetricsData) { }, }, } - doCompare("scrape5", t, want5, &m5) + doCompare("scrape5", t, want5, m5) } // target3 for complicated data types, including summaries and histograms. one of the summary and histogram have only @@ -816,7 +817,7 @@ rpc_duration_seconds_sum{foo="no_quantile"} 101 rpc_duration_seconds_count{foo="no_quantile"} 55 ` -func verifyTarget3(t *testing.T, td *testData, mds []internaldata.MetricsData) { +func verifyTarget3(t *testing.T, td *testData, mds []*agentmetricspb.ExportMetricsServiceRequest) { verifyNumScrapeResults(t, td, mds) m1 := mds[0] // m1 shall only have a gauge @@ -843,12 +844,12 @@ func verifyTarget3(t *testing.T, td *testData, mds []internaldata.MetricsData) { // set this timestamp to wantG1 wantG1.Timeseries[0].Points[0].Timestamp = ts1 doCompare("scrape1", t, - &internaldata.MetricsData{ + &agentmetricspb.ExportMetricsServiceRequest{ Node: td.node, Resource: td.resource, Metrics: []*metricspb.Metric{wantG1}, }, - &internaldata.MetricsData{ + &agentmetricspb.ExportMetricsServiceRequest{ Node: td.node, Resource: td.resource, Metrics: []*metricspb.Metric{gotG1}, @@ -858,7 +859,7 @@ func verifyTarget3(t *testing.T, td *testData, mds []internaldata.MetricsData) { m2 := mds[1] ts2 := m2.Metrics[0].Timeseries[0].Points[0].Timestamp - want2 := &internaldata.MetricsData{ + want2 := &agentmetricspb.ExportMetricsServiceRequest{ Node: td.node, Resource: td.resource, Metrics: []*metricspb.Metric{ @@ -984,7 +985,7 @@ func verifyTarget3(t *testing.T, td *testData, mds []internaldata.MetricsData) { }, } - doCompare("scrape2", t, want2, &m2) + doCompare("scrape2", t, want2, m2) } // TestEndToEnd end to end test executor @@ -1058,7 +1059,7 @@ var startTimeMetricPageStartTimestamp = ×tamppb.Timestamp{Seconds: 400, Nan const numStartTimeMetricPageTimeseries = 6 -func verifyStartTimeMetricPage(t *testing.T, _ *testData, mds []internaldata.MetricsData) { +func verifyStartTimeMetricPage(t *testing.T, _ *testData, mds []*agentmetricspb.ExportMetricsServiceRequest) { numTimeseries := 0 for _, cmd := range mds { for _, metric := range cmd.Metrics { @@ -1110,13 +1111,15 @@ func testEndToEnd(t *testing.T, targets []*testData, useStartTimeMetric bool) { metrics := cms.AllMetrics() // split and store results by target name - results := make(map[string][]internaldata.MetricsData) - for _, m := range metrics { - ocmds := internaldata.MetricsToOC(m) - for _, ocmd := range ocmds { + results := make(map[string][]*agentmetricspb.ExportMetricsServiceRequest) + for _, md := range metrics { + rms := md.ResourceMetrics() + for i := 0; i < rms.Len(); i++ { + ocmd := &agentmetricspb.ExportMetricsServiceRequest{} + ocmd.Node, ocmd.Resource, ocmd.Metrics = internaldata.ResourceMetricsToOC(rms.At(i)) result, ok := results[ocmd.Node.ServiceInfo.Name] if !ok { - result = make([]internaldata.MetricsData, 0) + result = make([]*agentmetricspb.ExportMetricsServiceRequest, 0) } results[ocmd.Node.ServiceInfo.Name] = append(result, ocmd) } @@ -1201,13 +1204,15 @@ func testEndToEndRegex(t *testing.T, targets []*testData, useStartTimeMetric boo metrics := cms.AllMetrics() // split and store results by target name - results := make(map[string][]internaldata.MetricsData) - for _, m := range metrics { - ocmds := internaldata.MetricsToOC(m) - for _, ocmd := range ocmds { + results := make(map[string][]*agentmetricspb.ExportMetricsServiceRequest) + for _, md := range metrics { + rms := md.ResourceMetrics() + for i := 0; i < rms.Len(); i++ { + ocmd := &agentmetricspb.ExportMetricsServiceRequest{} + ocmd.Node, ocmd.Resource, ocmd.Metrics = internaldata.ResourceMetricsToOC(rms.At(i)) result, ok := results[ocmd.Node.ServiceInfo.Name] if !ok { - result = make([]internaldata.MetricsData, 0) + result = make([]*agentmetricspb.ExportMetricsServiceRequest, 0) } results[ocmd.Node.ServiceInfo.Name] = append(result, ocmd) } diff --git a/translator/internaldata/consumerdata.go b/translator/internaldata/consumerdata.go deleted file mode 100644 index 7ce0807a3d1..00000000000 --- a/translator/internaldata/consumerdata.go +++ /dev/null @@ -1,29 +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 internaldata - -import ( - commonpb "github.com/census-instrumentation/opencensus-proto/gen-go/agent/common/v1" - metricspb "github.com/census-instrumentation/opencensus-proto/gen-go/metrics/v1" - resourcepb "github.com/census-instrumentation/opencensus-proto/gen-go/resource/v1" -) - -// MetricsData is a struct that groups proto metrics with a unique node and a resource. -// Deprecated: use pdata.Metrics instead. -type MetricsData struct { - Node *commonpb.Node - Resource *resourcepb.Resource - Metrics []*metricspb.Metric -} diff --git a/translator/internaldata/metrics_to_oc.go b/translator/internaldata/metrics_to_oc.go index 6c5a772bdaa..ed77e5b2348 100644 --- a/translator/internaldata/metrics_to_oc.go +++ b/translator/internaldata/metrics_to_oc.go @@ -17,7 +17,9 @@ package internaldata import ( "sort" + occommon "github.com/census-instrumentation/opencensus-proto/gen-go/agent/common/v1" ocmetrics "github.com/census-instrumentation/opencensus-proto/gen-go/metrics/v1" + ocresource "github.com/census-instrumentation/opencensus-proto/gen-go/resource/v1" "github.com/golang/protobuf/ptypes/wrappers" "go.opentelemetry.io/collector/consumer/pdata" @@ -31,29 +33,14 @@ type labelKeys struct { keyIndices map[string]int } -// MetricsToOC may be used only by OpenCensus receiver and exporter implementations. +// ResourceMetricsToOC may be used only by OpenCensus receiver and exporter implementations. +// Deprecated: Use pdata.Metrics. // TODO: move this function to OpenCensus package. -func MetricsToOC(md pdata.Metrics) []MetricsData { - resourceMetrics := md.ResourceMetrics() - - if resourceMetrics.Len() == 0 { - return nil - } - - ocResourceMetricsList := make([]MetricsData, 0, resourceMetrics.Len()) - for i := 0; i < resourceMetrics.Len(); i++ { - ocResourceMetricsList = append(ocResourceMetricsList, resourceMetricsToOC(resourceMetrics.At(i))) - } - - return ocResourceMetricsList -} - -func resourceMetricsToOC(rm pdata.ResourceMetrics) MetricsData { - ocMetricsData := MetricsData{} - ocMetricsData.Node, ocMetricsData.Resource = internalResourceToOC(rm.Resource()) +func ResourceMetricsToOC(rm pdata.ResourceMetrics) (*occommon.Node, *ocresource.Resource, []*ocmetrics.Metric) { + node, resource := internalResourceToOC(rm.Resource()) ilms := rm.InstrumentationLibraryMetrics() if ilms.Len() == 0 { - return ocMetricsData + return node, resource, nil } // Approximate the number of the metrics as the number of the metrics in the first // instrumentation library info. @@ -66,10 +53,10 @@ func resourceMetricsToOC(rm pdata.ResourceMetrics) MetricsData { ocMetrics = append(ocMetrics, metricToOC(metrics.At(j))) } } - if len(ocMetrics) != 0 { - ocMetricsData.Metrics = ocMetrics + if len(ocMetrics) == 0 { + ocMetrics = nil } - return ocMetricsData + return node, resource, ocMetrics } func metricToOC(metric pdata.Metric) *ocmetrics.Metric { diff --git a/translator/internaldata/metrics_to_oc_test.go b/translator/internaldata/metrics_to_oc_test.go index 5e34e9fbfbe..9f981e2fc77 100644 --- a/translator/internaldata/metrics_to_oc_test.go +++ b/translator/internaldata/metrics_to_oc_test.go @@ -19,6 +19,7 @@ import ( "time" occommon "github.com/census-instrumentation/opencensus-proto/gen-go/agent/common/v1" + agentmetricspb "github.com/census-instrumentation/opencensus-proto/gen-go/agent/metrics/v1" ocmetrics "github.com/census-instrumentation/opencensus-proto/gen-go/metrics/v1" ocresource "github.com/census-instrumentation/opencensus-proto/gen-go/resource/v1" "github.com/stretchr/testify/assert" @@ -43,46 +44,32 @@ func TestMetricsToOC(t *testing.T) { tests := []struct { name string internal pdata.Metrics - oc []MetricsData + oc *agentmetricspb.ExportMetricsServiceRequest }{ - { - name: "empty", - internal: testdata.GenerateMetricsEmpty(), - oc: []MetricsData(nil), - }, - { name: "one-empty-resource-metrics", internal: testdata.GenerateMetricsOneEmptyResourceMetrics(), - oc: []MetricsData{ - {}, - }, + oc: &agentmetricspb.ExportMetricsServiceRequest{}, }, { name: "no-libraries", internal: testdata.GenerateMetricsNoLibraries(), - oc: []MetricsData{ - generateOCTestDataNoMetrics(), - }, + oc: generateOCTestDataNoMetrics(), }, { name: "one-empty-instrumentation-library", internal: testdata.GenerateMetricsOneEmptyInstrumentationLibrary(), - oc: []MetricsData{ - generateOCTestDataNoMetrics(), - }, + oc: generateOCTestDataNoMetrics(), }, { name: "one-metric-no-resource", internal: testdata.GenerateMetricsOneMetricNoResource(), - oc: []MetricsData{ - { - Metrics: []*ocmetrics.Metric{ - generateOCTestMetricInt(), - }, + oc: &agentmetricspb.ExportMetricsServiceRequest{ + Metrics: []*ocmetrics.Metric{ + generateOCTestMetricInt(), }, }, }, @@ -90,72 +77,66 @@ func TestMetricsToOC(t *testing.T) { { name: "one-metric", internal: testdata.GenerateMetricsOneMetric(), - oc: []MetricsData{ - generateOCTestDataMetricsOneMetric(), - }, + oc: generateOCTestDataMetricsOneMetric(), }, { name: "one-metric-no-labels", internal: testdata.GenerateMetricsOneMetricNoLabels(), - oc: []MetricsData{ - generateOCTestDataNoLabels(), - }, + oc: generateOCTestDataNoLabels(), }, { name: "all-types-no-data-points", internal: testdata.GenerateMetricsAllTypesNoDataPoints(), - oc: []MetricsData{ - generateOCTestDataNoPoints(), - }, + oc: generateOCTestDataNoPoints(), }, { name: "sample-metric", internal: sampleMetricData, - oc: []MetricsData{ - generateOCTestData(), - }, + oc: generateOCTestData(), }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - got := MetricsToOC(test.internal) - assert.EqualValues(t, test.oc, got) + gotNode, gotResource, gotMetrics := ResourceMetricsToOC(test.internal.ResourceMetrics().At(0)) + assert.EqualValues(t, test.oc.Node, gotNode) + assert.EqualValues(t, test.oc.Resource, gotResource) + assert.EqualValues(t, test.oc.Metrics, gotMetrics) }) } } func TestMetricsToOC_InvalidDataType(t *testing.T) { internal := testdata.GenerateMetricsMetricTypeInvalid() - want := []MetricsData{ - { - Node: &occommon.Node{}, - Resource: &ocresource.Resource{ - Labels: map[string]string{"resource-attr": "resource-attr-val-1"}, - }, - Metrics: []*ocmetrics.Metric{ - { - MetricDescriptor: &ocmetrics.MetricDescriptor{ - Name: testdata.TestCounterIntMetricName, - Unit: "1", - Type: ocmetrics.MetricDescriptor_UNSPECIFIED, - LabelKeys: nil, - }, + want := &agentmetricspb.ExportMetricsServiceRequest{ + Node: &occommon.Node{}, + Resource: &ocresource.Resource{ + Labels: map[string]string{"resource-attr": "resource-attr-val-1"}, + }, + Metrics: []*ocmetrics.Metric{ + { + MetricDescriptor: &ocmetrics.MetricDescriptor{ + Name: testdata.TestCounterIntMetricName, + Unit: "1", + Type: ocmetrics.MetricDescriptor_UNSPECIFIED, + LabelKeys: nil, }, }, }, } - got := MetricsToOC(internal) - assert.EqualValues(t, want, got) + gotNode, gotResource, gotMetrics := ResourceMetricsToOC(internal.ResourceMetrics().At(0)) + assert.EqualValues(t, want.Node, gotNode) + assert.EqualValues(t, want.Resource, gotResource) + assert.EqualValues(t, want.Metrics, gotMetrics) } -func generateOCTestData() MetricsData { +func generateOCTestData() *agentmetricspb.ExportMetricsServiceRequest { ts := timestamppb.New(time.Date(2020, 2, 11, 20, 26, 0, 0, time.UTC)) - return MetricsData{ + return &agentmetricspb.ExportMetricsServiceRequest{ Node: &occommon.Node{ Identifier: &occommon.ProcessIdentifier{ HostName: "host1", diff --git a/translator/internaldata/oc_testdata_test.go b/translator/internaldata/oc_testdata_test.go index 739d0f403fc..98c09a85da1 100644 --- a/translator/internaldata/oc_testdata_test.go +++ b/translator/internaldata/oc_testdata_test.go @@ -18,6 +18,7 @@ import ( "time" occommon "github.com/census-instrumentation/opencensus-proto/gen-go/agent/common/v1" + agentmetricspb "github.com/census-instrumentation/opencensus-proto/gen-go/agent/metrics/v1" ocmetrics "github.com/census-instrumentation/opencensus-proto/gen-go/metrics/v1" ocresource "github.com/census-instrumentation/opencensus-proto/gen-go/resource/v1" "google.golang.org/protobuf/types/known/timestamppb" @@ -29,8 +30,8 @@ import ( "go.opentelemetry.io/collector/translator/conventions" ) -func generateOCTestDataNoMetrics() MetricsData { - return MetricsData{ +func generateOCTestDataNoMetrics() *agentmetricspb.ExportMetricsServiceRequest { + return &agentmetricspb.ExportMetricsServiceRequest{ Node: &occommon.Node{}, Resource: &ocresource.Resource{ Labels: map[string]string{"resource-attr": "resource-attr-val-1"}, @@ -38,8 +39,8 @@ func generateOCTestDataNoMetrics() MetricsData { } } -func generateOCTestDataNoPoints() MetricsData { - return MetricsData{ +func generateOCTestDataNoPoints() *agentmetricspb.ExportMetricsServiceRequest { + return &agentmetricspb.ExportMetricsServiceRequest{ Node: &occommon.Node{}, Resource: &ocresource.Resource{ Labels: map[string]string{"resource-attr": "resource-attr-val-1"}, @@ -105,12 +106,12 @@ func generateOCTestDataNoPoints() MetricsData { } } -func generateOCTestDataNoLabels() MetricsData { +func generateOCTestDataNoLabels() *agentmetricspb.ExportMetricsServiceRequest { m := generateOCTestMetricInt() m.MetricDescriptor.LabelKeys = nil m.Timeseries[0].LabelValues = nil m.Timeseries[1].LabelValues = nil - return MetricsData{ + return &agentmetricspb.ExportMetricsServiceRequest{ Node: &occommon.Node{}, Resource: &ocresource.Resource{ Labels: map[string]string{"resource-attr": "resource-attr-val-1"}, @@ -119,8 +120,8 @@ func generateOCTestDataNoLabels() MetricsData { } } -func generateOCTestDataMetricsOneMetric() MetricsData { - return MetricsData{ +func generateOCTestDataMetricsOneMetric() *agentmetricspb.ExportMetricsServiceRequest { + return &agentmetricspb.ExportMetricsServiceRequest{ Node: &occommon.Node{}, Resource: &ocresource.Resource{ Labels: map[string]string{"resource-attr": "resource-attr-val-1"}, @@ -129,8 +130,8 @@ func generateOCTestDataMetricsOneMetric() MetricsData { } } -func generateOCTestDataMetricsOneMetricOneNil() MetricsData { - return MetricsData{ +func generateOCTestDataMetricsOneMetricOneNil() *agentmetricspb.ExportMetricsServiceRequest { + return &agentmetricspb.ExportMetricsServiceRequest{ Node: &occommon.Node{}, Resource: &ocresource.Resource{ Labels: map[string]string{"resource-attr": "resource-attr-val-1"}, @@ -139,10 +140,10 @@ func generateOCTestDataMetricsOneMetricOneNil() MetricsData { } } -func generateOCTestDataMetricsOneMetricOneNilTimeseries() MetricsData { +func generateOCTestDataMetricsOneMetricOneNilTimeseries() *agentmetricspb.ExportMetricsServiceRequest { m := generateOCTestMetricInt() m.Timeseries = append(m.Timeseries, nil) - return MetricsData{ + return &agentmetricspb.ExportMetricsServiceRequest{ Node: &occommon.Node{}, Resource: &ocresource.Resource{ Labels: map[string]string{"resource-attr": "resource-attr-val-1"}, @@ -151,10 +152,10 @@ func generateOCTestDataMetricsOneMetricOneNilTimeseries() MetricsData { } } -func generateOCTestDataMetricsOneMetricOneNilPoint() MetricsData { +func generateOCTestDataMetricsOneMetricOneNilPoint() *agentmetricspb.ExportMetricsServiceRequest { m := generateOCTestMetricInt() m.Timeseries[0].Points = append(m.Timeseries[0].Points, nil) - return MetricsData{ + return &agentmetricspb.ExportMetricsServiceRequest{ Node: &occommon.Node{}, Resource: &ocresource.Resource{ Labels: map[string]string{"resource-attr": "resource-attr-val-1"}, diff --git a/translator/internaldata/oc_to_metrics.go b/translator/internaldata/oc_to_metrics.go index 5279c8eb6a2..2f328672b84 100644 --- a/translator/internaldata/oc_to_metrics.go +++ b/translator/internaldata/oc_to_metrics.go @@ -17,45 +17,28 @@ package internaldata import ( occommon "github.com/census-instrumentation/opencensus-proto/gen-go/agent/common/v1" ocmetrics "github.com/census-instrumentation/opencensus-proto/gen-go/metrics/v1" + ocresource "github.com/census-instrumentation/opencensus-proto/gen-go/resource/v1" "go.opentelemetry.io/collector/consumer/pdata" ) -// OCSliceToMetrics converts a slice of OC data format to data.MetricData. -// Deprecated: use pdata.Metrics instead. -func OCSliceToMetrics(ocmds []MetricsData) pdata.Metrics { - metricData := pdata.NewMetrics() - if len(ocmds) == 0 { - return metricData - } - for _, ocmd := range ocmds { - appendOcToMetrics(ocmd, metricData) - } - return metricData -} - // OCToMetrics converts OC data format to data.MetricData. // Deprecated: use pdata.Metrics instead. OCToMetrics may be used only by OpenCensus // receiver and exporter implementations. -func OCToMetrics(md MetricsData) pdata.Metrics { - metricData := pdata.NewMetrics() - appendOcToMetrics(md, metricData) - return metricData -} - -func appendOcToMetrics(md MetricsData, dest pdata.Metrics) { - if md.Node == nil && md.Resource == nil && len(md.Metrics) == 0 { - return +func OCToMetrics(node *occommon.Node, resource *ocresource.Resource, metrics []*ocmetrics.Metric) pdata.Metrics { + dest := pdata.NewMetrics() + if node == nil && resource == nil && len(metrics) == 0 { + return dest } rms := dest.ResourceMetrics() initialRmsLen := rms.Len() - if len(md.Metrics) == 0 { + if len(metrics) == 0 { // At least one of the md.Node or md.Resource is not nil. Set the resource and return. rms.Resize(initialRmsLen + 1) - ocNodeResourceToInternal(md.Node, md.Resource, rms.At(initialRmsLen).Resource()) - return + ocNodeResourceToInternal(node, resource, rms.At(initialRmsLen).Resource()) + return dest } // We may need to split OC metrics into several ResourceMetrics. OC metrics can have a @@ -80,7 +63,7 @@ func appendOcToMetrics(md MetricsData, dest pdata.Metrics) { // in one slice. combinedMetricCount := 0 distinctResourceCount := 0 - for _, ocMetric := range md.Metrics { + for _, ocMetric := range metrics { if ocMetric == nil { // Skip nil metrics. continue @@ -104,7 +87,7 @@ func appendOcToMetrics(md MetricsData, dest pdata.Metrics) { if combinedMetricCount > 0 { rm0 := rms.At(initialRmsLen) - ocNodeResourceToInternal(md.Node, md.Resource, rm0.Resource()) + ocNodeResourceToInternal(node, resource, rm0.Resource()) // Allocate a slice for metrics that need to be combined into first ResourceMetrics. ilms := rm0.InstrumentationLibraryMetrics() @@ -114,7 +97,7 @@ func appendOcToMetrics(md MetricsData, dest pdata.Metrics) { // Index to next available slot in "combinedMetrics" slice. combinedMetricIdx := 0 - for _, ocMetric := range md.Metrics { + for _, ocMetric := range metrics { if ocMetric == nil { // Skip nil metrics. continue @@ -139,7 +122,7 @@ func appendOcToMetrics(md MetricsData, dest pdata.Metrics) { // First resourcemetric is used for the default resource, so start with 1. resourceMetricIdx = 1 } - for _, ocMetric := range md.Metrics { + for _, ocMetric := range metrics { if ocMetric == nil { // Skip nil metrics. continue @@ -152,9 +135,10 @@ func appendOcToMetrics(md MetricsData, dest pdata.Metrics) { // This metric has a different Resource and must be placed in a different // ResourceMetrics instance. Create a separate ResourceMetrics item just for this metric // and store at resourceMetricIdx. - ocMetricToResourceMetrics(ocMetric, md.Node, rms.At(initialRmsLen+resourceMetricIdx)) + ocMetricToResourceMetrics(ocMetric, node, rms.At(initialRmsLen+resourceMetricIdx)) resourceMetricIdx++ } + return dest } func ocMetricToResourceMetrics(ocMetric *ocmetrics.Metric, node *occommon.Node, out pdata.ResourceMetrics) { diff --git a/translator/internaldata/oc_to_metrics_test.go b/translator/internaldata/oc_to_metrics_test.go index 6997fa877df..45f97d02fdf 100644 --- a/translator/internaldata/oc_to_metrics_test.go +++ b/translator/internaldata/oc_to_metrics_test.go @@ -18,6 +18,7 @@ import ( "testing" occommon "github.com/census-instrumentation/opencensus-proto/gen-go/agent/common/v1" + agentmetricspb "github.com/census-instrumentation/opencensus-proto/gen-go/agent/metrics/v1" ocmetrics "github.com/census-instrumentation/opencensus-proto/gen-go/metrics/v1" ocresource "github.com/census-instrumentation/opencensus-proto/gen-go/resource/v1" "github.com/stretchr/testify/assert" @@ -42,18 +43,18 @@ func TestOCToMetrics(t *testing.T) { tests := []struct { name string - oc MetricsData + oc *agentmetricspb.ExportMetricsServiceRequest internal pdata.Metrics }{ { name: "empty", - oc: MetricsData{}, + oc: &agentmetricspb.ExportMetricsServiceRequest{}, internal: testdata.GenerateMetricsEmpty(), }, { name: "one-empty-resource-metrics", - oc: MetricsData{ + oc: &agentmetricspb.ExportMetricsServiceRequest{ Node: &occommon.Node{}, Resource: &ocresource.Resource{}, }, @@ -86,7 +87,7 @@ func TestOCToMetrics(t *testing.T) { { name: "one-metric-one-summary", - oc: MetricsData{ + oc: &agentmetricspb.ExportMetricsServiceRequest{ Resource: generateOCTestResource(), Metrics: []*ocmetrics.Metric{ generateOCTestMetricInt(), @@ -122,7 +123,7 @@ func TestOCToMetrics(t *testing.T) { { name: "sample-metric", - oc: MetricsData{ + oc: &agentmetricspb.ExportMetricsServiceRequest{ Resource: generateOCTestResource(), Metrics: []*ocmetrics.Metric{ generateOCTestMetricInt(), @@ -138,21 +139,8 @@ func TestOCToMetrics(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - got := OCToMetrics(test.oc) + got := OCToMetrics(test.oc.Node, test.oc.Resource, test.oc.Metrics) assert.EqualValues(t, test.internal, got) - - ocslice := []MetricsData{ - test.oc, - test.oc, - } - wantSlice := pdata.NewMetrics() - // Double the ResourceMetrics only if not empty. - if test.internal.ResourceMetrics().Len() != 0 { - test.internal.Clone().ResourceMetrics().MoveAndAppendTo(wantSlice.ResourceMetrics()) - test.internal.Clone().ResourceMetrics().MoveAndAppendTo(wantSlice.ResourceMetrics()) - } - gotSlice := OCSliceToMetrics(ocslice) - assert.EqualValues(t, wantSlice, gotSlice) }) } } @@ -168,7 +156,7 @@ func TestOCToMetrics_ResourceInMetric(t *testing.T) { oc.Metrics = append(oc.Metrics, oc2.Metrics...) oc.Metrics[1].Resource = oc2.Resource oc.Metrics[1].Resource.Labels["resource-attr"] = "another-value" - got := OCToMetrics(oc) + got := OCToMetrics(oc.Node, oc.Resource, oc.Metrics) assert.EqualValues(t, want, got) } @@ -181,55 +169,49 @@ func TestOCToMetrics_ResourceInMetricOnly(t *testing.T) { // We shouldn't have a "combined" resource after conversion oc.Metrics[0].Resource = oc.Resource oc.Resource = nil - got := OCToMetrics(oc) + got := OCToMetrics(oc.Node, oc.Resource, oc.Metrics) assert.EqualValues(t, want, got) } func BenchmarkMetricIntOCToMetrics(b *testing.B) { - ocMetric := MetricsData{ - Resource: generateOCTestResource(), - Metrics: []*ocmetrics.Metric{ - generateOCTestMetricInt(), - generateOCTestMetricInt(), - generateOCTestMetricInt(), - }, + ocResource := generateOCTestResource() + ocMetrics := []*ocmetrics.Metric{ + generateOCTestMetricInt(), + generateOCTestMetricInt(), + generateOCTestMetricInt(), } b.ResetTimer() for n := 0; n < b.N; n++ { - OCToMetrics(ocMetric) + OCToMetrics(nil, ocResource, ocMetrics) } } func BenchmarkMetricDoubleOCToMetrics(b *testing.B) { - ocMetric := MetricsData{ - Resource: generateOCTestResource(), - Metrics: []*ocmetrics.Metric{ - generateOCTestMetricDouble(), - generateOCTestMetricDouble(), - generateOCTestMetricDouble(), - }, + ocResource := generateOCTestResource() + ocMetrics := []*ocmetrics.Metric{ + generateOCTestMetricDouble(), + generateOCTestMetricDouble(), + generateOCTestMetricDouble(), } b.ResetTimer() for n := 0; n < b.N; n++ { - OCToMetrics(ocMetric) + OCToMetrics(nil, ocResource, ocMetrics) } } func BenchmarkMetricHistogramOCToMetrics(b *testing.B) { - ocMetric := MetricsData{ - Resource: generateOCTestResource(), - Metrics: []*ocmetrics.Metric{ - generateOCTestMetricDoubleHistogram(), - generateOCTestMetricDoubleHistogram(), - generateOCTestMetricDoubleHistogram(), - }, + ocResource := generateOCTestResource() + ocMetrics := []*ocmetrics.Metric{ + generateOCTestMetricDoubleHistogram(), + generateOCTestMetricDoubleHistogram(), + generateOCTestMetricDoubleHistogram(), } b.ResetTimer() for n := 0; n < b.N; n++ { - OCToMetrics(ocMetric) + OCToMetrics(nil, ocResource, ocMetrics) } }