From 74e96e5bf11e9925dccf8b17f851b31ea2598300 Mon Sep 17 00:00:00 2001 From: Emmanuel T Odeke Date: Mon, 10 May 2021 18:35:37 -0700 Subject: [PATCH 1/4] receiver/prometheus/internal: add createNodeAndResourcePdata for Prometheus->OTLP Pdata Starts the progressive effort to convert directly from Prometheus->OTLPPdata directly instead of Prometheus->OpenCensusProto->OTLPPdata by adding a converter for node+resource -> pdata.Resource. Updates #3137 --- .../internal/prom_to_otlp.go | 39 ++++++++++ .../internal/prom_to_otlp_test.go | 74 +++++++++++++++++++ 2 files changed, 113 insertions(+) create mode 100644 receiver/prometheusreceiver/internal/prom_to_otlp.go create mode 100644 receiver/prometheusreceiver/internal/prom_to_otlp_test.go diff --git a/receiver/prometheusreceiver/internal/prom_to_otlp.go b/receiver/prometheusreceiver/internal/prom_to_otlp.go new file mode 100644 index 00000000000..25d0bec79e3 --- /dev/null +++ b/receiver/prometheusreceiver/internal/prom_to_otlp.go @@ -0,0 +1,39 @@ +// 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 internal + +import ( + "net" + + "go.opentelemetry.io/collector/consumer/pdata" + "go.opentelemetry.io/collector/translator/conventions" +) + +func createNodeAndResourcePdata(job, instance, scheme string) pdata.Resource { + host, port, err := net.SplitHostPort(instance) + if err != nil { + host = instance + } + resource := pdata.NewResource() + attrs := resource.Attributes() + attrs.UpsertString(conventions.AttributeServiceName, job) + attrs.UpsertString(conventions.AttributeHostName, host) + attrs.UpsertString(jobAttr, job) + attrs.UpsertString(instanceAttr, instance) + attrs.UpsertString(portAttr, port) + attrs.UpsertString(schemeAttr, scheme) + + return resource +} diff --git a/receiver/prometheusreceiver/internal/prom_to_otlp_test.go b/receiver/prometheusreceiver/internal/prom_to_otlp_test.go new file mode 100644 index 00000000000..d6c02f221fb --- /dev/null +++ b/receiver/prometheusreceiver/internal/prom_to_otlp_test.go @@ -0,0 +1,74 @@ +// 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 internal + +import ( + "sort" + "testing" + "unsafe" + + metricspb "github.com/census-instrumentation/opencensus-proto/gen-go/metrics/v1" + "github.com/google/go-cmp/cmp" + "google.golang.org/protobuf/testing/protocmp" + + "go.opentelemetry.io/collector/consumer/pdata" + otlpresource "go.opentelemetry.io/collector/internal/data/protogen/resource/v1" + "go.opentelemetry.io/collector/translator/internaldata" +) + +func TestCreateNodeAndResourceConversion(t *testing.T) { + job, instance, scheme := "converter", "ocmetrics", "http" + ocNode, ocResource := createNodeAndResource(job, instance, scheme) + mdFromOC := internaldata.OCToMetrics(internaldata.MetricsData{ + Node: ocNode, + Resource: ocResource, + // We need to pass in a dummy set of metrics + // just to populate and allow for full conversion. + Metrics: []*metricspb.Metric{ + { + MetricDescriptor: &metricspb.MetricDescriptor{ + Name: "m1", + Description: "d1", + Unit: "By", + }, + }, + }, + }) + + fromOCResource := protoResource(mdFromOC.ResourceMetrics().At(0).Resource()) + byDirectOTLPResource := protoResource(createNodeAndResourcePdata(job, instance, scheme)) + + if diff := cmp.Diff(byDirectOTLPResource, fromOCResource, protocmp.Transform()); diff != "" { + t.Fatalf("Resource mismatch: got: - want: +\n%s", diff) + } +} + +// Unfortunately pdata doesn't expose a way for us to retrieve the underlying resource, +// yet we need to compare the resources which are hidden by an unexported value. +func protoResource(presource pdata.Resource) *otlpresource.Resource { + type extract struct { + orig *otlpresource.Resource + } + extracted := (*extract)(unsafe.Pointer(&presource)) + if extracted == nil { + return nil + } + // Ensure that the attributes are sorted so that we can properly compare the raw values. + resource := extracted.orig + sort.Slice(resource.Attributes, func(i, j int) bool { + return resource.Attributes[i].Key < resource.Attributes[j].Key + }) + return resource +} From dd55c2441dc146c568cf0c1d030afd981b46d63d Mon Sep 17 00:00:00 2001 From: Emmanuel T Odeke Date: Wed, 12 May 2021 00:06:47 -0700 Subject: [PATCH 2/4] Use .Attributes to skip unsafe extraction plus use require.Equal Addresses feedback pointed out by Bogdan, Tigran, Jaana by using Resource.Attributes().Sort() instead and also avoid using go-cmp/cmp and instead opt to using require.Equal. --- .../internal/prom_to_otlp_test.go | 33 +++---------------- 1 file changed, 4 insertions(+), 29 deletions(-) diff --git a/receiver/prometheusreceiver/internal/prom_to_otlp_test.go b/receiver/prometheusreceiver/internal/prom_to_otlp_test.go index d6c02f221fb..72b1d88251f 100644 --- a/receiver/prometheusreceiver/internal/prom_to_otlp_test.go +++ b/receiver/prometheusreceiver/internal/prom_to_otlp_test.go @@ -15,16 +15,11 @@ package internal import ( - "sort" "testing" - "unsafe" metricspb "github.com/census-instrumentation/opencensus-proto/gen-go/metrics/v1" - "github.com/google/go-cmp/cmp" - "google.golang.org/protobuf/testing/protocmp" + "github.com/stretchr/testify/require" - "go.opentelemetry.io/collector/consumer/pdata" - otlpresource "go.opentelemetry.io/collector/internal/data/protogen/resource/v1" "go.opentelemetry.io/collector/translator/internaldata" ) @@ -47,28 +42,8 @@ func TestCreateNodeAndResourceConversion(t *testing.T) { }, }) - fromOCResource := protoResource(mdFromOC.ResourceMetrics().At(0).Resource()) - byDirectOTLPResource := protoResource(createNodeAndResourcePdata(job, instance, scheme)) + fromOCResource := mdFromOC.ResourceMetrics().At(0).Resource().Attributes().Sort() + byDirectOTLPResource := createNodeAndResourcePdata(job, instance, scheme).Attributes().Sort() - if diff := cmp.Diff(byDirectOTLPResource, fromOCResource, protocmp.Transform()); diff != "" { - t.Fatalf("Resource mismatch: got: - want: +\n%s", diff) - } -} - -// Unfortunately pdata doesn't expose a way for us to retrieve the underlying resource, -// yet we need to compare the resources which are hidden by an unexported value. -func protoResource(presource pdata.Resource) *otlpresource.Resource { - type extract struct { - orig *otlpresource.Resource - } - extracted := (*extract)(unsafe.Pointer(&presource)) - if extracted == nil { - return nil - } - // Ensure that the attributes are sorted so that we can properly compare the raw values. - resource := extracted.orig - sort.Slice(resource.Attributes, func(i, j int) bool { - return resource.Attributes[i].Key < resource.Attributes[j].Key - }) - return resource + require.Equal(t, byDirectOTLPResource, fromOCResource) } From 3977f60632afe0776d5dedc4128897e3bc2775de Mon Sep 17 00:00:00 2001 From: Emmanuel T Odeke Date: Thu, 13 May 2021 20:30:52 -0700 Subject: [PATCH 3/4] Add comment and rename parity test check --- receiver/prometheusreceiver/internal/prom_to_otlp_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/receiver/prometheusreceiver/internal/prom_to_otlp_test.go b/receiver/prometheusreceiver/internal/prom_to_otlp_test.go index 72b1d88251f..8d7f0008ca5 100644 --- a/receiver/prometheusreceiver/internal/prom_to_otlp_test.go +++ b/receiver/prometheusreceiver/internal/prom_to_otlp_test.go @@ -23,7 +23,8 @@ import ( "go.opentelemetry.io/collector/translator/internaldata" ) -func TestCreateNodeAndResourceConversion(t *testing.T) { +// Parity test to ensure that createNodeAndResource produces identical results to createNodeAndResourcePdata. +func TestCreateNodeAndResourceEquivalence(t *testing.T) { job, instance, scheme := "converter", "ocmetrics", "http" ocNode, ocResource := createNodeAndResource(job, instance, scheme) mdFromOC := internaldata.OCToMetrics(internaldata.MetricsData{ From 6ea4b9e11c8beff4f8c7fa528afa87fcdde957bd Mon Sep 17 00:00:00 2001 From: Emmanuel T Odeke Date: Thu, 27 May 2021 11:17:10 +0300 Subject: [PATCH 4/4] Add createNodeAndResource unit test --- .../internal/prom_to_otlp_test.go | 80 +++++++++++++++++-- 1 file changed, 75 insertions(+), 5 deletions(-) diff --git a/receiver/prometheusreceiver/internal/prom_to_otlp_test.go b/receiver/prometheusreceiver/internal/prom_to_otlp_test.go index 8d7f0008ca5..a2de99c8243 100644 --- a/receiver/prometheusreceiver/internal/prom_to_otlp_test.go +++ b/receiver/prometheusreceiver/internal/prom_to_otlp_test.go @@ -20,6 +20,7 @@ import ( metricspb "github.com/census-instrumentation/opencensus-proto/gen-go/metrics/v1" "github.com/stretchr/testify/require" + "go.opentelemetry.io/collector/consumer/pdata" "go.opentelemetry.io/collector/translator/internaldata" ) @@ -27,12 +28,10 @@ import ( func TestCreateNodeAndResourceEquivalence(t *testing.T) { job, instance, scheme := "converter", "ocmetrics", "http" ocNode, ocResource := createNodeAndResource(job, instance, scheme) - mdFromOC := internaldata.OCToMetrics(internaldata.MetricsData{ - Node: ocNode, - Resource: ocResource, + mdFromOC := internaldata.OCToMetrics(ocNode, ocResource, // We need to pass in a dummy set of metrics // just to populate and allow for full conversion. - Metrics: []*metricspb.Metric{ + []*metricspb.Metric{ { MetricDescriptor: &metricspb.MetricDescriptor{ Name: "m1", @@ -41,10 +40,81 @@ func TestCreateNodeAndResourceEquivalence(t *testing.T) { }, }, }, - }) + ) fromOCResource := mdFromOC.ResourceMetrics().At(0).Resource().Attributes().Sort() byDirectOTLPResource := createNodeAndResourcePdata(job, instance, scheme).Attributes().Sort() require.Equal(t, byDirectOTLPResource, fromOCResource) } + +type jobInstanceDefinition struct { + job, instance, host, scheme, port string +} + +func makeResourceWithJobInstanceScheme(def *jobInstanceDefinition) pdata.Resource { + resource := pdata.NewResource() + attrs := resource.Attributes() + // Using hardcoded values to assert on outward expectations so that + // when variables change, these tests will fail and we'll have reports. + attrs.UpsertString("service.name", def.job) + attrs.UpsertString("host.name", def.host) + attrs.UpsertString("job", def.job) + attrs.UpsertString("instance", def.instance) + attrs.UpsertString("port", def.port) + attrs.UpsertString("scheme", def.scheme) + return resource +} + +func TestCreateNodeAndResourcePromToOTLP(t *testing.T) { + tests := []struct { + name, job string + instance string + scheme string + want pdata.Resource + }{ + { + name: "all attributes proper", + job: "job", instance: "localhost:8888", scheme: "http", + want: makeResourceWithJobInstanceScheme(&jobInstanceDefinition{ + "job", "localhost:8888", "localhost", "http", "8888", + }), + }, + { + name: "missing port", + job: "job", instance: "myinstance", scheme: "https", + want: makeResourceWithJobInstanceScheme(&jobInstanceDefinition{ + "job", "myinstance", "myinstance", "https", "", + }), + }, + { + name: "blank scheme", + job: "job", instance: "myinstance:443", scheme: "", + want: makeResourceWithJobInstanceScheme(&jobInstanceDefinition{ + "job", "myinstance:443", "myinstance", "", "443", + }), + }, + { + name: "blank instance, blank scheme", + job: "job", instance: "", scheme: "", + want: makeResourceWithJobInstanceScheme(&jobInstanceDefinition{ + "job", "", "", "", "", + }), + }, + { + name: "blank instance, non-blank scheme", + job: "job", instance: "", scheme: "http", + want: makeResourceWithJobInstanceScheme(&jobInstanceDefinition{ + "job", "", "", "http", "", + }), + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + got := createNodeAndResourcePdata(tt.job, tt.instance, tt.scheme) + require.Equal(t, got, tt.want) + }) + } +}