-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
[processor/resourcedetection]: add "cname" and "lookup" hostname sources #10015
Conversation
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.
I like this idea. We should do that and eventually remove Showmax/go-fqdn dependency. Could you please create a follow up issue? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add some test coverage?
processor/resourcedetectionprocessor/internal/system/metadata.go
Outdated
Show resolved
Hide resolved
Sure -- I've added some. |
Created follow-up issue #10138 |
…ces (open-telemetry#10015) * [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.
…ces (open-telemetry#10015) * [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.
[processor/resourcedetection]: add "cname" and "lookup" hostname sources
The existing "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
os.Hostname
and a corresponding fqdnon that line. If that fails, then it falls back to:
net.LookupCNAME
. If that fails, then it falls back to:A problem that least one Collector user encountered while running on Windows in Azure,
is that the call to
net.LookupCNAME
just returns the simple hostname, whichthen gets returned by
FqdnHostname()
, even though on a Linux host, deployed in the sameenvironment,
net.LookupCNAME
returns a fully qualified domain name.Instead of trying to work around what might be an issue with
net.LookupCNAME
by modifyingthe 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 correspondsto a call to
net.LookupHost
and then a call tonet.LookupAddr
for each ip address it returns.The "lookup" source produces the same result on Windows as
net.LookupCNAME
does on Linux and wouldsolve 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:
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 customer 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.