search

shirou / gopsutil

psutil for golang
Other
10.54k stars 1.59k forks source link

Return the same HostID on Linux when running as root or not #1420

Open andrzej-stencel opened 1 year ago

andrzej-stencel commented 1 year ago

Version: v3.23.1

Is your feature request related to a problem? Please describe.

Hi, many thanks for the great work on this package. It is being used in the OpenTelemetry Collector a lot.

I have recently hit a problem with the host.HostID() function on Linux, in that the function returns different values depending on whether the process containing the library (the OpenTelemetry Collector Contrib binary in my case) is being run with root privileges or without them. See discussion on this PR for details: https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/18740.

From my understanding, this is currently by design, as when running as root, the value is read from /sys/class/dmi/id/product_uuid path, but when not running as root, this path is not readable, in which case the value is read from /etc/machine-id (the third fallback of /proc/sys/kernel/random/boot_id is not relevant to this issue).

Why is this a problem? I want to use the value returned by host.HostID() function in the OpenTelemetry Collector as the value for the host.id attribute, that is attached to telemetry (logs, metrics, traces) sent by the collector, alongside host.name and other attributes. This value of this attribute should be consistent, i.e. it should not change depending on how the collector is run - i.e. with or without root privileges. In this case I believe it's better to always ignore the product_uuid value and always only use the /etc/machine-id value.

Describe the solution you'd like Make it possible to specify the source(s) of the host ID that the function should use. E.g. I would like to specify "I want host ID from /etc/machine-id or from the random boot ID, but not from the product_uuid". I can imagine someone else might want to only rely on product_uuid and not the other sources (and in case the function returns an empty value, do something else). This could be done via parameters to this function, but it's platform specific, so not sure if it makes sense. Perhaps environment variables would be the better option? Here's an example:

Describe alternatives you've considered

The alternative (for OpenTelemetry Collector) is to not use gopsutil package in this case and either use some other package or implement this functionality in the collector. We use the gopsutil package a lot in the collector, so using it for this seems like the natural option. Also I wonder if anybody else has a similar problem. If yes, it's probably worth to solve it in the library. If no, maybe I'm doing something wrong or making incorrect assumptions.

Additional context

I'd be very happy to discuss possible solutions, or whether this is actually a problem. If needed, you can reach me and other OpenTelemetry Collector community members in the #otel-collector channel of the CNCF Slack. Please let me know if there's anything I can help with.

shirou commented 1 year ago

Thank you for using gopsutil.

The implementation to get the HostID is a very old function, introduced 7 years ago in #237. The fallback to /etc/machine-id was introduced in #603.

/sys/class/dmi/id/product_uuid is preferred than /etc/machine-id because it is more likely to be fixed since it is a hardware ID. I also thought that this inconsistency would not be a problem, since it is rare case that an application running with root privileges would be changed to running without root privileges.

If this difference is going to be a problem this time, then I am willing to change the order. However, this code is already widely used, such as consul according to #603 (I do not know if it is still used today, though), changing this order would replace all existing HostIDs, and I believe it would cause havoc.

Therefore, I think that changing by environment variable as you suggested is the preferred method. The GOPSUTIL_HOST_ID_LINUX_SOURCES name is not consistent with other environment variables such as HOST_ETC, but I now believe that environment variables should have a prefix, so it is fine as is. Rather, I think I should add a prefix the existing environment variable name with GOPSUTIL_. (I didn't have much experience with this when I first set it up.)

Alternatively, we could add the functions HostIDWithOptions and HostIDWithContextWithOptions and pass them as an argument. The problem with this approach is that the method names become long and we have to define the functions even though the options are meaningless on other platforms. (I have always had an implementation method in my mind that uses this approach, but it has not been implemented due to these problems. This is somewhat off topic, but I would be grateful if we could exchange ideas on this matter)