Add host.id resource detector#4843
Conversation
| return SimpleNamespace(stdout=stdout, stderr="", returncode=return_code) | ||
|
|
||
|
|
||
| class HostIdResourceDetectorTest(TestBase): |
There was a problem hiding this comment.
Can we update these tests so that they test as much of the functionality end-to-end as possible? Almost everything is being patched here, which doesn't make these tests that useful. Our CI runs on both Linux and Windows so you should be able to at least test these systems end-to-end.
…, switch statement
| self.assertIn(HOST_ID, resource.attributes) | ||
| host_id = resource.attributes[HOST_ID] | ||
| self.assertIsInstance(host_id, str) | ||
| self.assertTrue(host_id) |
There was a problem hiding this comment.
Can we make a stronger assertion here: that this is a 32 character lowercase hexadecimal string, or better yet read the contents of /etc/machine-id or /var/lib/dbus/machine-id
| pip install -e ./instrumentation-genai/opentelemetry-instrumentation-openai-v2 | ||
| pip install -e ./sdk-extension/opentelemetry-sdk-extension-aws | ||
| pip install -e ./resource/opentelemetry-resource-detector-containerid | ||
| pip install -e ./resource/opentelemetry-resource-detector-host |
There was a problem hiding this comment.
I think the package name like this can create confusion with the default HostResourceDetector in the SDK. Maybe rename it to hostid, host-id?
Maybe I'm being conservative here from the user perspective, but just sharing some thoughts.
There was a problem hiding this comment.
i agree, i'm going to update it, thanks
Description
This PR adds a
host.idresource detector following the semconv spec, and using the java agent implementation as a reference.Fixes #4830
Type of change
How Has This Been Tested?
I wrote a small test app, and added
Ran the app (on macOS) and observed the attribute
Does This PR Require a Core Repo Change?
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.