add host.id to resource auto-detection #3812

mwear · 2023-02-27T22:05:45Z

This PR adds host id detection support for non-containerized hosts. The implementation is based on this spec pr: open-telemetry/opentelemetry-specification#3173.

Fixes #3811

Summary

This PR introduces a hostIDReader interface with platform specific implementations for Darwin, Linux, BSD, Windows and a fallback for unsupported platfoms.
The platform specific implementations use unprivileged means to lookup the host.id, aka machine id for non-containerized environments. Depending on platform this involves reading files in well known locations, excuting shell commands, or a combination of the two.
I followed the code organization used for the OS Description Detector, which also has platform specific implementations, however, there is one notable difference. Currently the Go SDK divides platform specific code into darwin, unix, and windows and the associated build tags for those platforms. Host id detection is more granular and requires dividing unix into linux and BSD.

See also

Spec PR: clarify how to collect host.id for non-containerized systems opentelemetry-specification#3173
JS Implementation: feat: detect host id for non-cloud environments opentelemetry-js#3575
Collector PR (using gopsutil): resourcedetectionprocessor: use opentelemetry-go for host id detection opentelemetry-collector-contrib#18740

I realize we will want to see the spec PR merge before getting too serious about this work, but I wanted to get some eyes on the approach and encourage anyone with opinions to provide feedback here and on the spec PR.

CHANGELOG.md

sdk/resource/config.go

sdk/resource/host_id_linux.go

sdk/resource/host_id_unsupported.go

sdk/resource/resource_test.go

codecov · 2023-03-01T01:07:21Z

Codecov Report

❗ No coverage uploaded for pull request base (main@1eab60f). Click here to learn what that means.
The diff coverage is 74.0%.

❗ Current head 6df6880 differs from pull request most recent head 3230472. Consider uploading reports for the commit 3230472 to get more accurate results

Additional details and impacted files

@@          Coverage Diff           @@
##             main   #3812   +/-   ##
======================================
  Coverage        ?   81.5%           
======================================
  Files           ?     170           
  Lines           ?   12765           
  Branches        ?       0           
======================================
  Hits            ?   10415           
  Misses          ?    2131           
  Partials        ?     219

Impacted Files	Coverage Δ
sdk/resource/host_id.go	`73.0% <73.0%> (ø)`
sdk/resource/config.go	`100.0% <100.0%> (ø)`

sdk/resource/export_test.go

sdk/resource/host_id.go

sdk/resource/host_id_bsd.go

sdk/resource/host_id_bsd_test.go

sdk/resource/resource_test.go

pellared · 2023-03-08T18:33:48Z

sdk/resource/config.go

@@ -71,6 +71,11 @@ func WithHost() Option {
 	return WithDetectors(host{})
 }

+// WithHostID adds host ID information to the configured resource.
+func WithHostID() Option {
+	return WithDetectors(hostIDDetector{})


Shouldn't we add hostIDDetector to WithHost()? We might consider adding WithHostName to offer a resource detector that only sets host.name resource attribute.

Because the rules for detecting HostID are different for containerized vs non-containerized hosts I didn't want to couple the hostIDDetector to WithHost, although I had considered it. My thoughts were that someone who wanted to use WithHost on a cloud provider wouldn't have to worry about detector order when detecting HostID. However, I could see having WithHost that detects HostID and HostName, and separate options WithHostID and WithHostName to collect the attributes individually. Let me know what your preference is.

EDIT: After second thought, I have no strong preference. Maybe we should improve the docs and list all the attributes that given detector (option) should set? This could be addressed in a separate issue as I feel this is a problem not only for this detector/option.

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>

This allows us to run tests for the BSD, Darwin, and Linux readers on all platforms.

…s updated

sdk/resource/resource_test.go

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>

mwear · 2023-03-20T23:15:55Z

I wanted to check in to see if there is anything that anyone would like to see addressed in this PR?

mwear requested review from jmacd, MrAlias, Aneurysm9, evantorrie, XSAM, dashpole, MadVikingGod, pellared, hanyuancheung and dmathieu as code owners February 27, 2023 22:05

Aneurysm9 linked an issue Feb 27, 2023 that may be closed by this pull request

host.id resource auto-detection #3811

Closed

mwear mentioned this pull request Feb 27, 2023

resourcedetectionprocessor: use opentelemetry-go for host id detection open-telemetry/opentelemetry-collector-contrib#18740

Merged

MrAlias reviewed Feb 28, 2023

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

MrAlias reviewed Feb 28, 2023

View reviewed changes

mwear force-pushed the detect_host_id branch from 79f141b to 0358f54 Compare March 1, 2023 00:54

mwear force-pushed the detect_host_id branch from aca8b68 to 14e6efa Compare March 1, 2023 01:29

MadVikingGod reviewed Mar 2, 2023

View reviewed changes

sdk/resource/export_test.go Outdated Show resolved Hide resolved

sdk/resource/host_id.go Outdated Show resolved Hide resolved

sdk/resource/host_id_bsd.go Outdated Show resolved Hide resolved

sdk/resource/host_id_bsd_test.go Outdated Show resolved Hide resolved

mwear force-pushed the detect_host_id branch 3 times, most recently from 4eb70b5 to a154c62 Compare March 7, 2023 23:28

pellared reviewed Mar 8, 2023

View reviewed changes

mwear and others added 8 commits March 8, 2023 15:28

add platform specific hostIDReaders

cad0ef1

add WithHostID option to Resource

5e50909

add changelog entry

b9c7878

Apply suggestions from code review

ab30fde

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>

linting

8c65da8

combine platform specific readers and tests

0f16d39

This allows us to run tests for the BSD, Darwin, and Linux readers on all platforms.

add todo to use assert.AnError after resource.Detect error handling i…

a9bdee8

…s updated

move HostID test utilities to host_id_test

8f6b557

mwear force-pushed the detect_host_id branch from 90385de to f850251 Compare March 8, 2023 23:43

return assert.AnError from mockHostIDProviderWithError

0260501

mwear force-pushed the detect_host_id branch from f850251 to 0260501 Compare March 8, 2023 23:47

MrAlias reviewed Mar 9, 2023

View reviewed changes

sdk/resource/resource_test.go Outdated Show resolved Hide resolved

pellared approved these changes Mar 9, 2023

View reviewed changes

mwear and others added 2 commits March 9, 2023 09:37

use assert.ErrorIs

7c093e1

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>

Merge branch 'main' into detect_host_id

6df6880

pellared requested review from MadVikingGod, pellared and MrAlias March 21, 2023 06:35

pellared approved these changes Mar 21, 2023

View reviewed changes

MadVikingGod approved these changes Mar 21, 2023

View reviewed changes

Merge branch 'main' into detect_host_id

3230472

MadVikingGod merged commit 282a47e into open-telemetry:main Mar 21, 2023

MrAlias mentioned this pull request Mar 22, 2023

Release v1.15.0-rc.2/v0.38.0-rc.2 #3923

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add host.id to resource auto-detection #3812

add host.id to resource auto-detection #3812

mwear commented Feb 27, 2023 •

edited

Loading

codecov bot commented Mar 1, 2023 •

edited

Loading

pellared Mar 8, 2023

mwear Mar 8, 2023 •

edited

Loading

pellared Mar 9, 2023 •

edited

Loading

mwear commented Mar 20, 2023

add host.id to resource auto-detection #3812

add host.id to resource auto-detection #3812

Conversation

mwear commented Feb 27, 2023 • edited Loading

codecov bot commented Mar 1, 2023 • edited Loading

Codecov Report

pellared Mar 8, 2023

Choose a reason for hiding this comment

mwear Mar 8, 2023 • edited Loading

Choose a reason for hiding this comment

pellared Mar 9, 2023 • edited Loading

Choose a reason for hiding this comment

mwear commented Mar 20, 2023

mwear commented Feb 27, 2023 •

edited

Loading

codecov bot commented Mar 1, 2023 •

edited

Loading

mwear Mar 8, 2023 •

edited

Loading

pellared Mar 9, 2023 •

edited

Loading