From 7150e41e28892181cec94578c7443f66f2066c7d Mon Sep 17 00:00:00 2001 From: Pablo Collins Date: Tue, 10 May 2022 15:55:28 -0400 Subject: [PATCH 1/4] [processor/resourcedetection]: add "cname" and "lookup" hostname sources The "dns" hostname source uses the Showmax/go-fqdn library, whose FqdnHostname() function tries to get the fully qualified domain name of the current host by 1) Reading the hosts file and looking for a match of the os.Hostname and a corresponding fqdn on that line. If that fails, then it falls back to: 2) Getting the canonical name via net.LookupCNAME. If that fails, then it falls back to: 3) Getting the current IP address, and doing a reverse DNS lookup of that address A problem that least one Collector user encountered running on Windows in Azure, is that the call to net.LookupCNAME just returns the simple hostname, which then gets returned by FqdnHostname(), even though on a Linux host net.LookupCNAME returns a fully qualified domain name. Instead of trying to work around what might be an issue with net.LookupCNAME by modifying the Showmax/go-fqdn code, which has hard-coded fallback logic, and changing behavior for existing users, this change proposes creating two new hostname sources, "cname", and "lookup". The "cname" source corresponds to a call to net.LookupCNAME, and the "lookup" source corresponds to a call to net.LookupHost and then a call to net.LookupAddr for each ip address it returns. The "lookup" source produces the same result on Windows as net.LookupCNAME does on Linux and would solve the problem of the Azure user in question. In addition, users can then build the fallback logic that they prefer. Eventually, we could also add a "hostfile" hostname source that uses the same logic as the corresponding part of the Showmax/"dns" implementation, so that users could replace the "dns" hostname source with the equivalent three sources, explicitly indicating the fallback logic rather than using Showmax's hard-coded logic: processors: resourcedetection: detectors: ["system"] system: hostname_sources: ["hostsfile", "cname", "lookup"] At that point, we could deprecate the "dns" source or make it effectively an alias for ["hostsfile", "cname", "lookup"] and remove the dependency. For now, the user that would benefit from this change, however, will just use the "lookup" source on their Windows hosts, as "cname" on Windows doesn't produce a useful result. --- CHANGELOG.md | 1 + .../resourcedetectionprocessor/README.md | 26 ++++++++- .../internal/system/metadata.go | 56 +++++++++++++++++++ .../internal/system/system.go | 27 +++++++-- .../internal/system/system_test.go | 10 ++++ 5 files changed, 115 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7689f059a1bd8..84ffd04b70260 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,7 @@ - `transformprocessor`: Add new `limit` function to allow limiting the number of items in a map, such as the number of attributes in `attributes` or `resource.attributes` (#9552) - `processor/attributes`: Support attributes set by server authenticator (#9420) - `datadogexporter`: Experimental support for Exponential Histograms with delta aggregation temporality (#8350) +- `resourcedetectionprocessor`: Add "cname" and "lookup" hostname sources ### 🧰 Bug fixes 🧰 diff --git a/processor/resourcedetectionprocessor/README.md b/processor/resourcedetectionprocessor/README.md index 785b9b283bffc..6523285684cb2 100644 --- a/processor/resourcedetectionprocessor/README.md +++ b/processor/resourcedetectionprocessor/README.md @@ -26,6 +26,8 @@ processors: ### System metadata +Note: use the Docker detector (see below) if running the Collector as a Docker container. + Queries the host machine to retrieve the following resource attributes: * host.name @@ -47,8 +49,30 @@ processors: * all valid options for `hostname_sources`: * "dns" * "os" + * "cname" + * "lookup" -Note: use the Docker detector (see below) if running the Collector as a Docker container. +#### Hostname Sources + +##### dns + +The "dns" hostname source uses multiple sources to get the fully qualified domain name. First, it looks up the +host name in the local machine's `hosts` file. If that fails, it looks up the CNAME. Lastly, if that fails, +it does a reverse DNS query. Note: this hostname source may produce unreliable results on Windows. To produce +a FQDN, Windows hosts might have better results using the "lookup" hostname source, which is mentioned below. + +##### os + +The "os" hostname source provides the hostname provided by the local machine's kernel. + +##### cname + +The "cname" hostname source provides the canonical name, as provided by net.LookupCNAME in the Go standard library. +Note: this hostname source may produce unreliable results on Windows. + +##### lookup + +The "lookup" hostname source does a reverse DNS lookup of the current host's IP address. ### Docker metadata diff --git a/processor/resourcedetectionprocessor/internal/system/metadata.go b/processor/resourcedetectionprocessor/internal/system/metadata.go index da1bd55e74bda..7b8799712f4e9 100644 --- a/processor/resourcedetectionprocessor/internal/system/metadata.go +++ b/processor/resourcedetectionprocessor/internal/system/metadata.go @@ -15,6 +15,8 @@ package system // import "github.com/open-telemetry/opentelemetry-collector-contrib/processor/resourcedetectionprocessor/internal/system" import ( + "fmt" + "net" "os" "runtime" @@ -32,6 +34,10 @@ type systemMetadata interface { // OSType returns the host operating system OSType() (string, error) + + LookupCNAME() (string, error) + + ReverseLookupHost() (string, error) } type systemMetadataImpl struct{} @@ -47,3 +53,53 @@ func (*systemMetadataImpl) FQDN() (string, error) { func (*systemMetadataImpl) Hostname() (string, error) { return os.Hostname() } + +func (m *systemMetadataImpl) LookupCNAME() (string, error) { + hostname, err := m.Hostname() + if err != nil { + return "", fmt.Errorf("LookupCNAME failed to get hostname: %w", err) + } + cname, err := net.LookupCNAME(hostname) + if err != nil { + return "", fmt.Errorf("LookupCNAME failed to get CNAME: %w", err) + } + return stripTrailingDot(cname), nil +} + +func (m *systemMetadataImpl) ReverseLookupHost() (string, error) { + hostname, err := m.Hostname() + if err != nil { + return "", fmt.Errorf("ReverseLookupHost failed to get hostname: %w", err) + } + return hostnameToDomainName(hostname) +} + +func hostnameToDomainName(hostname string) (string, error) { + ipAddresses, err := net.LookupHost(hostname) + if err != nil { + return "", fmt.Errorf("hostnameToDomainName failed to convert hostname to IP addresses: %w", err) + } + return reverseLookup(ipAddresses) +} + +func reverseLookup(ipAddresses []string) (string, error) { + var err error + for _, ip := range ipAddresses { + var names []string + names, err = net.LookupAddr(ip) + if err != nil { + continue + } + return stripTrailingDot(names[0]), nil + } + return "", fmt.Errorf("reverseLookup failed to convert IP address to name: %w", err) +} + +func stripTrailingDot(name string) string { + nameLen := len(name) + lastIdx := nameLen - 1 + if nameLen > 0 && name[lastIdx] == '.' { + name = name[:lastIdx] + } + return name +} diff --git a/processor/resourcedetectionprocessor/internal/system/system.go b/processor/resourcedetectionprocessor/internal/system/system.go index 1d5eb9c446ace..c7b2f97757582 100644 --- a/processor/resourcedetectionprocessor/internal/system/system.go +++ b/processor/resourcedetectionprocessor/internal/system/system.go @@ -33,8 +33,10 @@ const ( ) var hostnameSourcesMap = map[string]func(*Detector) (string, error){ - "dns": getFQDN, - "os": getHostname, + "os": getHostname, + "dns": getFQDN, + "cname": lookupCNAME, + "lookup": reverseLookupHost, } var _ internal.Detector = (*Detector)(nil) @@ -52,6 +54,7 @@ func NewDetector(p component.ProcessorCreateSettings, dcfg internal.DetectorConf if len(cfg.HostnameSources) == 0 { cfg.HostnameSources = []string{"dns", "os"} } + return &Detector{provider: &systemMetadataImpl{}, logger: p.Logger, hostnameSources: cfg.HostnameSources}, nil } @@ -78,7 +81,7 @@ func (d *Detector) Detect(_ context.Context) (resource pcommon.Resource, schemaU d.logger.Debug(err.Error()) } - return res, "", errors.New("all hostname sources are failed to get hostname") + return res, "", errors.New("all hostname sources failed to get hostname") } // getHostname returns OS hostname @@ -94,7 +97,23 @@ func getHostname(d *Detector) (string, error) { func getFQDN(d *Detector) (string, error) { hostname, err := d.provider.FQDN() if err != nil { - return "", fmt.Errorf("failed getting FQDN: %w", err) + return "", fmt.Errorf("getFQDN failed getting FQDN: %w", err) + } + return hostname, nil +} + +func lookupCNAME(d *Detector) (string, error) { + cname, err := d.provider.LookupCNAME() + if err != nil { + return "", fmt.Errorf("lookupCNAME failed to get CNAME: %w", err) + } + return cname, nil +} + +func reverseLookupHost(d *Detector) (string, error) { + hostname, err := d.provider.ReverseLookupHost() + if err != nil { + return "", fmt.Errorf("reverseLookupHost failed to lookup host: %w", err) } return hostname, nil } diff --git a/processor/resourcedetectionprocessor/internal/system/system_test.go b/processor/resourcedetectionprocessor/internal/system/system_test.go index 93642b026b443..075266f95ad2c 100644 --- a/processor/resourcedetectionprocessor/internal/system/system_test.go +++ b/processor/resourcedetectionprocessor/internal/system/system_test.go @@ -48,6 +48,16 @@ func (m *mockMetadata) OSType() (string, error) { return args.String(0), args.Error(1) } +func (m *mockMetadata) LookupCNAME() (string, error) { + args := m.MethodCalled("LookupCNAME") + return args.String(0), args.Error(1) +} + +func (m *mockMetadata) ReverseLookupHost() (string, error) { + args := m.MethodCalled("ReverseLookupHost") + return args.String(0), args.Error(1) +} + func TestNewDetector(t *testing.T) { tests := []struct { name string From 734d233eabfc75d78b559abcf37f85a36db0d408 Mon Sep 17 00:00:00 2001 From: Pablo Collins Date: Wed, 18 May 2022 12:50:46 -0400 Subject: [PATCH 2/4] test --- .../internal/system/metadata.go | 78 +++++++++----- .../internal/system/metadata_test.go | 102 ++++++++++++++++++ .../internal/system/system.go | 4 +- 3 files changed, 153 insertions(+), 31 deletions(-) create mode 100644 processor/resourcedetectionprocessor/internal/system/metadata_test.go diff --git a/processor/resourcedetectionprocessor/internal/system/metadata.go b/processor/resourcedetectionprocessor/internal/system/metadata.go index 7b8799712f4e9..073594ea432ac 100644 --- a/processor/resourcedetectionprocessor/internal/system/metadata.go +++ b/processor/resourcedetectionprocessor/internal/system/metadata.go @@ -19,13 +19,34 @@ import ( "net" "os" "runtime" + "strings" "github.com/Showmax/go-fqdn" "github.com/open-telemetry/opentelemetry-collector-contrib/processor/resourcedetectionprocessor/internal" ) -type systemMetadata interface { +// nameInfoProvider abstracts domain name resolution so it can be swapped for +// testing +type nameInfoProvider struct { + osHostname func() (string, error) + lookupCNAME func(string) (string, error) + lookupHost func(string) ([]string, error) + lookupAddr func(string) ([]string, error) +} + +// newNameInfoProvider creates a name info provider for production use, using +// DNS to resolve domain names +func newNameInfoProvider() nameInfoProvider { + return nameInfoProvider{ + osHostname: os.Hostname, + lookupCNAME: net.LookupCNAME, + lookupHost: net.LookupHost, + lookupAddr: net.LookupAddr, + } +} + +type metadataProvider interface { // Hostname returns the OS hostname Hostname() (string, error) @@ -35,71 +56,70 @@ type systemMetadata interface { // OSType returns the host operating system OSType() (string, error) + // LookupCNAME returns the canonical name for the current host LookupCNAME() (string, error) + // ReverseLookupHost does a reverse DNS query on the current host's IP address ReverseLookupHost() (string, error) } -type systemMetadataImpl struct{} +type systemMetadataProvider struct { + nameInfoProvider nameInfoProvider +} -func (*systemMetadataImpl) OSType() (string, error) { +func newSystemMetadataProvider() metadataProvider { + return systemMetadataProvider{nameInfoProvider: newNameInfoProvider()} +} + +func (systemMetadataProvider) OSType() (string, error) { return internal.GOOSToOSType(runtime.GOOS), nil } -func (*systemMetadataImpl) FQDN() (string, error) { +func (systemMetadataProvider) FQDN() (string, error) { return fqdn.FqdnHostname() } -func (*systemMetadataImpl) Hostname() (string, error) { - return os.Hostname() +func (p systemMetadataProvider) Hostname() (string, error) { + return p.nameInfoProvider.osHostname() } -func (m *systemMetadataImpl) LookupCNAME() (string, error) { - hostname, err := m.Hostname() +func (p systemMetadataProvider) LookupCNAME() (string, error) { + hostname, err := p.Hostname() if err != nil { return "", fmt.Errorf("LookupCNAME failed to get hostname: %w", err) } - cname, err := net.LookupCNAME(hostname) + cname, err := p.nameInfoProvider.lookupCNAME(hostname) if err != nil { return "", fmt.Errorf("LookupCNAME failed to get CNAME: %w", err) } - return stripTrailingDot(cname), nil + return strings.TrimRight(cname, "."), nil } -func (m *systemMetadataImpl) ReverseLookupHost() (string, error) { - hostname, err := m.Hostname() +func (p systemMetadataProvider) ReverseLookupHost() (string, error) { + hostname, err := p.Hostname() if err != nil { return "", fmt.Errorf("ReverseLookupHost failed to get hostname: %w", err) } - return hostnameToDomainName(hostname) + return p.hostnameToDomainName(hostname) } -func hostnameToDomainName(hostname string) (string, error) { - ipAddresses, err := net.LookupHost(hostname) +func (p systemMetadataProvider) hostnameToDomainName(hostname string) (string, error) { + ipAddresses, err := p.nameInfoProvider.lookupHost(hostname) if err != nil { return "", fmt.Errorf("hostnameToDomainName failed to convert hostname to IP addresses: %w", err) } - return reverseLookup(ipAddresses) + return p.reverseLookup(ipAddresses) } -func reverseLookup(ipAddresses []string) (string, error) { +func (p systemMetadataProvider) reverseLookup(ipAddresses []string) (string, error) { var err error for _, ip := range ipAddresses { var names []string - names, err = net.LookupAddr(ip) + names, err = p.nameInfoProvider.lookupAddr(ip) if err != nil { continue } - return stripTrailingDot(names[0]), nil - } - return "", fmt.Errorf("reverseLookup failed to convert IP address to name: %w", err) -} - -func stripTrailingDot(name string) string { - nameLen := len(name) - lastIdx := nameLen - 1 - if nameLen > 0 && name[lastIdx] == '.' { - name = name[:lastIdx] + return strings.TrimRight(names[0], "."), nil } - return name + return "", fmt.Errorf("reverseLookup failed to convert IP addresses to name: %w", err) } diff --git a/processor/resourcedetectionprocessor/internal/system/metadata_test.go b/processor/resourcedetectionprocessor/internal/system/metadata_test.go new file mode 100644 index 0000000000000..1111b1124d870 --- /dev/null +++ b/processor/resourcedetectionprocessor/internal/system/metadata_test.go @@ -0,0 +1,102 @@ +// 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 system + +import ( + "fmt" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestLookupCNAME_Linux(t *testing.T) { + p := fakeLinuxSystemMetadataProvider() + cname, err := p.LookupCNAME() + require.NoError(t, err) + assert.Equal(t, "my-linux-vm.abcdefghijklmnopqrstuvwxyz.xx.internal.foo.net", cname) +} + +func TestLookupCNAME_Windows(t *testing.T) { + p := fakeWindowsSystemMetadataProvider() + cname, err := p.LookupCNAME() + require.NoError(t, err) + assert.Equal(t, "my-windows-vm.abcdefghijklmnopqrstuvwxyz.xx.internal.foo.net", cname) +} + +func TestReverseLookupHost_Linux(t *testing.T) { + p := fakeLinuxSystemMetadataProvider() + fqdn, err := p.ReverseLookupHost() + require.NoError(t, err) + assert.Equal(t, "my-linux-vm.internal.cloudapp.net", fqdn) +} + +func TestReverseLookupHost_Windows(t *testing.T) { + p := fakeWindowsSystemMetadataProvider() + fqdn, err := p.ReverseLookupHost() + require.NoError(t, err) + assert.Equal(t, "my-windows-vm.abcdefghijklmnopqrstuvwxyz.xx.internal.foo.net", fqdn) +} + +func fakeLinuxSystemMetadataProvider() *systemMetadataProvider { + return &systemMetadataProvider{ + nameInfoProvider: fakeLinuxNameInfoProvider(), + } +} + +func fakeWindowsSystemMetadataProvider() *systemMetadataProvider { + return &systemMetadataProvider{ + nameInfoProvider: fakeWindowsNameInfoProvider(), + } +} + +func fakeLinuxNameInfoProvider() nameInfoProvider { + return nameInfoProvider{ + osHostname: func() (string, error) { + return "my-linux-vm", nil + }, + lookupCNAME: func(s string) (string, error) { + return "my-linux-vm.abcdefghijklmnopqrstuvwxyz.xx.internal.foo.net.", nil + }, + lookupHost: func(s string) ([]string, error) { + return []string{"172.24.0.4"}, nil + }, + lookupAddr: func(s string) ([]string, error) { + return []string{"my-linux-vm.internal.cloudapp.net."}, nil + }, + } +} + +func fakeWindowsNameInfoProvider() nameInfoProvider { + fqdn := "my-windows-vm.abcdefghijklmnopqrstuvwxyz.xx.internal.foo.net." + return nameInfoProvider{ + osHostname: func() (string, error) { + return "my-windows-vm", nil + }, + lookupCNAME: func(s string) (string, error) { + return fqdn, nil + }, + lookupHost: func(s string) ([]string, error) { + return []string{"ffff::0000:1111:2222:3333%Ethernet", "1.2.3.4"}, nil + }, + lookupAddr: func(s string) ([]string, error) { + if strings.HasSuffix(s, "%Ethernet") { + return nil, fmt.Errorf("lookup %s: unrecognized address", s) + } + return []string{fqdn}, nil + }, + } +} diff --git a/processor/resourcedetectionprocessor/internal/system/system.go b/processor/resourcedetectionprocessor/internal/system/system.go index c7b2f97757582..892d6e81fb1eb 100644 --- a/processor/resourcedetectionprocessor/internal/system/system.go +++ b/processor/resourcedetectionprocessor/internal/system/system.go @@ -43,7 +43,7 @@ var _ internal.Detector = (*Detector)(nil) // Detector is a system metadata detector type Detector struct { - provider systemMetadata + provider metadataProvider logger *zap.Logger hostnameSources []string } @@ -55,7 +55,7 @@ func NewDetector(p component.ProcessorCreateSettings, dcfg internal.DetectorConf cfg.HostnameSources = []string{"dns", "os"} } - return &Detector{provider: &systemMetadataImpl{}, logger: p.Logger, hostnameSources: cfg.HostnameSources}, nil + return &Detector{provider: newSystemMetadataProvider(), logger: p.Logger, hostnameSources: cfg.HostnameSources}, nil } // Detect detects system metadata and returns a resource with the available ones From b575366e04ca8f81b2fe3867f37b28900809527f Mon Sep 17 00:00:00 2001 From: Pablo Collins Date: Wed, 18 May 2022 12:53:33 -0400 Subject: [PATCH 3/4] test --- .../resourcedetectionprocessor/internal/system/metadata_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/processor/resourcedetectionprocessor/internal/system/metadata_test.go b/processor/resourcedetectionprocessor/internal/system/metadata_test.go index 1111b1124d870..4b328137aab80 100644 --- a/processor/resourcedetectionprocessor/internal/system/metadata_test.go +++ b/processor/resourcedetectionprocessor/internal/system/metadata_test.go @@ -75,7 +75,7 @@ func fakeLinuxNameInfoProvider() nameInfoProvider { return []string{"172.24.0.4"}, nil }, lookupAddr: func(s string) ([]string, error) { - return []string{"my-linux-vm.internal.cloudapp.net."}, nil + return []string{"my-linux-vm.internal.foo.net."}, nil }, } } From 962a428dcb89b23fd1d2fef8450b210155c1df4b Mon Sep 17 00:00:00 2001 From: Pablo Collins Date: Wed, 18 May 2022 12:54:23 -0400 Subject: [PATCH 4/4] test --- .../resourcedetectionprocessor/internal/system/metadata_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/processor/resourcedetectionprocessor/internal/system/metadata_test.go b/processor/resourcedetectionprocessor/internal/system/metadata_test.go index 4b328137aab80..3dba1fb2854a6 100644 --- a/processor/resourcedetectionprocessor/internal/system/metadata_test.go +++ b/processor/resourcedetectionprocessor/internal/system/metadata_test.go @@ -41,7 +41,7 @@ func TestReverseLookupHost_Linux(t *testing.T) { p := fakeLinuxSystemMetadataProvider() fqdn, err := p.ReverseLookupHost() require.NoError(t, err) - assert.Equal(t, "my-linux-vm.internal.cloudapp.net", fqdn) + assert.Equal(t, "my-linux-vm.internal.foo.net", fqdn) } func TestReverseLookupHost_Windows(t *testing.T) {