[datadogexporter] Improve hostname resolution

[mx-psi]

Description:

Improve system hostname detection for the Datadog exporter.
This PR:

• Moves config and host code to their own packages to avoid dependency cycles
• Adds hostname validation
• Adds fully qualified domain name hostname resolution on some platforms
• Adds support for caching hostname

Link to tracking Issue: n/a

Testing:
Added unit tests, tested on an end to end test environment with the component activated.

Documentation:
Documentation was added to all public functions.

[pmcollins]

Hi Pablo :-)

[pmcollins]

Does CanonicalHostnameKey always just refer to the host name specified in the config? If so, what do you think about naming it something like ConfiguredHostNameKey?

[mx-psi]

Does CanonicalHostnameKey always just refer to the host name specified in the config?

Only if it is valid. If it is invalid (for example, it is unset), the canonical host name will be the one fetched from the system (either the FQDN or the OS hostname) and that will be the one saved in the cache (we always save to the cache before returning):

opentelemetry-collector-contrib/exporter/datadogexporter/metadata/host.go

Line 62 in 9f0d201

cache.SetNoExpire(cache.CanonicalHostnameKey, &hostname)

[pmcollins]

I'm wondering if, since the current function, GetHost, is the top level function that returns the hostname to the caller, it could be made smaller, with a lot of the fallback logic encapsulated at a lower level, such as methods on HostInfo. Maybe GetHost could then just 1) get the cached value 2) if there's no value, get the hostname 3) put the hostname into the cache if necessary and 4) return it? The configured hostname could also be passed into the HostInfo so it could do the full fallback of configuredValue -> systemValue -> osValue? Just a thought.

[mx-psi]

It makes sense to me to have a method on HostInfo to handle which one to return (I will change that), but I think passing the configured hostname will be a little cumbersome in the future. The end goal is to have other metadata sources (like EC2 or GCE metadata) that are considered in a certain order (a very simplified version of this code in our Datadog Agent) and I would prefer to avoid passing the configured hostname to each of them

[mx-psi]

I reorganized the code so that some of the fallback logic is handled in a HostInfo method

opentelemetry-collector-contrib/exporter/datadogexporter/metadata/system/host_unix.go

Lines 42 to 50 in 1b0973a

	func (hi *HostInfo) GetHostname(logger *zap.Logger) string {
	if err := valid.ValidHostname(hi.FQDN); err != nil {
	logger.Info("FQDN is not valid", zap.Error(err))
	return hi.OS
	} else {
	return hi.FQDN
	}
	}

[mx-psi]

Hi Pablo 😄 ! I tried to address all your comments, feel free to have a second look

[pmcollins]

LGTM Pablo :-)

I do think the global cache could be made a field of the exporter and then passed in to GetHost just to make the dependency more explicit, but not a requirement for this PR from my perspective. 👍

[mx-psi]

Awesome, thanks!

I do think the global cache could be made a field of the exporter and then passed in to GetHost just to make the dependency more explicit, but not a requirement for this PR from my perspective. 👍

Thanks for the feedback, I will reconsider changing that in a future PR: first I want to see the whole picture on where we use the cache after we add some extra metadata features that are still missing, so I think it's best if the cache stays as it is for now.