Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[receiver/prometheusreceiver] Do not add host.name to metrics from localhost/unspecified targets #6476

Merged
merged 6 commits into from
Dec 6, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 24 additions & 2 deletions receiver/prometheusreceiver/internal/prom_to_otlp.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,37 @@ import (
conventions "go.opentelemetry.io/collector/model/semconv/v1.5.0"
)

func createNodeAndResourcePdata(job, instance, scheme string) pdata.Resource {
// isDiscernibleHost checks if a host can be used as a value for the 'host.name' key.
// localhost-like hosts and unspecified (0.0.0.0) hosts are not discernible.
func isDiscernibleHost(host string) bool {
ip := net.ParseIP(host)
if ip != nil {
// An IP is discernible if
// - it's not local (e.g. belongs to 127.0.0.0/8 or ::1/128) and
// - it's not unspecified (e.g. the 0.0.0.0 address).
return !ip.IsLoopback() && !ip.IsUnspecified()
}

if host == "localhost" {
return false
}

// not an IP, not 'localhost', assume it is discernible.
return true
}

// CreateNodeAndResourcePdata creates the resource data added to OTLP payloads.
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)
if isDiscernibleHost(host) {
attrs.UpsertString(conventions.AttributeHostName, host)
}
attrs.UpsertString(jobAttr, job)
attrs.UpsertString(instanceAttr, instance)
attrs.UpsertString(portAttr, port)
Expand Down
38 changes: 27 additions & 11 deletions receiver/prometheusreceiver/internal/prom_to_otlp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func TestCreateNodeAndResourceEquivalence(t *testing.T) {
)

fromOCResource := mdFromOC.ResourceMetrics().At(0).Resource().Attributes().Sort()
byDirectOTLPResource := createNodeAndResourcePdata(job, instance, scheme).Attributes().Sort()
byDirectOTLPResource := CreateNodeAndResourcePdata(job, instance, scheme).Attributes().Sort()

require.Equal(t, byDirectOTLPResource, fromOCResource)
}
Expand All @@ -52,13 +52,15 @@ type jobInstanceDefinition struct {
job, instance, host, scheme, port string
}

func makeResourceWithJobInstanceScheme(def *jobInstanceDefinition) pdata.Resource {
func makeResourceWithJobInstanceScheme(def *jobInstanceDefinition, hasHost bool) 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)
if hasHost {
attrs.UpsertString("host.name", def.host)
}
attrs.UpsertString("job", def.job)
attrs.UpsertString("instance", def.instance)
attrs.UpsertString("port", def.port)
Expand All @@ -75,45 +77,59 @@ func TestCreateNodeAndResourcePromToOTLP(t *testing.T) {
}{
{
name: "all attributes proper",
job: "job", instance: "localhost:8888", scheme: "http",
job: "job", instance: "hostname:8888", scheme: "http",
want: makeResourceWithJobInstanceScheme(&jobInstanceDefinition{
"job", "localhost:8888", "localhost", "http", "8888",
}),
"job", "hostname:8888", "hostname", "http", "8888",
}, true),
},
{
name: "missing port",
job: "job", instance: "myinstance", scheme: "https",
want: makeResourceWithJobInstanceScheme(&jobInstanceDefinition{
"job", "myinstance", "myinstance", "https", "",
}),
}, true),
},
{
name: "blank scheme",
job: "job", instance: "myinstance:443", scheme: "",
want: makeResourceWithJobInstanceScheme(&jobInstanceDefinition{
"job", "myinstance:443", "myinstance", "", "443",
}),
}, true),
},
{
name: "blank instance, blank scheme",
job: "job", instance: "", scheme: "",
want: makeResourceWithJobInstanceScheme(&jobInstanceDefinition{
"job", "", "", "", "",
}),
}, true),
},
{
name: "blank instance, non-blank scheme",
job: "job", instance: "", scheme: "http",
want: makeResourceWithJobInstanceScheme(&jobInstanceDefinition{
"job", "", "", "http", "",
}),
}, true),
},
{
name: "0.0.0.0 address",
job: "job", instance: "0.0.0.0:8888", scheme: "http",
want: makeResourceWithJobInstanceScheme(&jobInstanceDefinition{
"job", "0.0.0.0:8888", "", "http", "8888",
}, false),
},
{
name: "localhost",
job: "job", instance: "localhost:8888", scheme: "http",
want: makeResourceWithJobInstanceScheme(&jobInstanceDefinition{
"job", "localhost:8888", "", "http", "8888",
}, false),
},
}

for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
got := createNodeAndResourcePdata(tt.job, tt.instance, tt.scheme)
got := CreateNodeAndResourcePdata(tt.job, tt.instance, tt.scheme)
require.Equal(t, got, tt.want)
})
}
Expand Down
9 changes: 7 additions & 2 deletions receiver/prometheusreceiver/internal/transaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,12 +242,17 @@ func createNodeAndResource(job, instance, scheme string) (*commonpb.Node, *resou
if err != nil {
host = instance
}

node := &commonpb.Node{
ServiceInfo: &commonpb.ServiceInfo{Name: job},
Identifier: &commonpb.ProcessIdentifier{
}

if isDiscernibleHost(host) {
node.Identifier = &commonpb.ProcessIdentifier{
HostName: host,
},
}
}

resource := &resourcepb.Resource{
Labels: map[string]string{
jobAttr: job,
Expand Down
19 changes: 8 additions & 11 deletions receiver/prometheusreceiver/metrics_receiver_helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"context"
"fmt"
"log"
"net"
"net/http"
"net/http/httptest"
"net/url"
Expand All @@ -36,6 +35,8 @@ import (
"go.opentelemetry.io/collector/consumer/consumertest"
"go.opentelemetry.io/collector/model/pdata"
"gopkg.in/yaml.v2"

"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/prometheusreceiver/internal"
)

type mockPrometheusResponse struct {
Expand Down Expand Up @@ -129,7 +130,6 @@ func setupMockPrometheus(openMetricsContentType bool, tds ...*testData) (*mockPr
}
mp := newMockPrometheus(endpoints, openMetricsContentType)
u, _ := url.Parse(mp.srv.URL)
host, port, _ := net.SplitHostPort(u.Host)
for i := 0; i < len(tds); i++ {
job := make(map[string]interface{})
job["job_name"] = tds[i].name
Expand All @@ -149,13 +149,7 @@ func setupMockPrometheus(openMetricsContentType bool, tds ...*testData) (*mockPr
}
// update attributes value (will use for validation)
for _, t := range tds {
t.attributes = pdata.NewAttributeMap()
t.attributes.Insert("service.name", pdata.NewAttributeValueString(t.name))
t.attributes.Insert("host.name", pdata.NewAttributeValueString(host))
t.attributes.Insert("job", pdata.NewAttributeValueString(t.name))
t.attributes.Insert("instance", pdata.NewAttributeValueString(u.Host))
t.attributes.Insert("port", pdata.NewAttributeValueString(port))
t.attributes.Insert("scheme", pdata.NewAttributeValueString("http"))
t.attributes = internal.CreateNodeAndResourcePdata(t.name, u.Host, "http").Attributes()
}
pCfg, err := promcfg.Load(string(cfg), false, gokitlog.NewNopLogger())
return mp, pCfg, err
Expand Down Expand Up @@ -285,8 +279,11 @@ func doCompare(t *testing.T, name string, want pdata.AttributeMap, got *pdata.Re
assert.Equal(t, expectedScrapeMetricCount, countScrapeMetricsRM(got))
assert.Equal(t, want.Len(), got.Resource().Attributes().Len())
for k, v := range want.AsRaw() {
value, _ := got.Resource().Attributes().Get(k)
assert.EqualValues(t, v, value.AsString())
value, ok := got.Resource().Attributes().Get(k)
assert.True(t, ok, "%q attribute is missing", k)
if ok {
assert.EqualValues(t, v, value.AsString())
}
}
for _, e := range expectations {
e(t, got)
Expand Down