Skip to content
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

Add host.id detection to HostDetector #3437

Closed
svrnm opened this issue Nov 23, 2022 · 6 comments · Fixed by #3575
Closed

Add host.id detection to HostDetector #3437

svrnm opened this issue Nov 23, 2022 · 6 comments · Fixed by #3575

Comments

@svrnm
Copy link
Member

svrnm commented Nov 23, 2022

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

Similar to container.id the attribute host.id can be super helpful to provide correlation between infra & app data in the backend. Many (non-containerised) linux systems have a file called "machine-id" that holds a unique identifier, that is created on first boot and stays stable from there. I checked CentOS, Debian & Alpine and all of them provide that file in /etc/machine-id. Historically this file could also live at /var/lib/dbus/machine-id. I would like to have the HostDetector extended to check for these files and add host.id if available.

Describe the solution you'd like

I have the code ready at https://github.com/svrnm/opentelemetry-js/tree/add-host-id-detection, so if this is something worth being added I can raise the PR:

Additional context

Spec Issue: open-telemetry/opentelemetry-specification#2978

@github-actions
Copy link

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the stale label Feb 13, 2023
@svrnm
Copy link
Member Author

svrnm commented Feb 13, 2023

@open-telemetry/javascript-approvers any thoughts on this? happy to raise the PR

@legendecas
Copy link
Member

PR: #3575

@legendecas legendecas linked a pull request Feb 13, 2023 that will close this issue
4 tasks
@legendecas legendecas removed the stale label Feb 13, 2023
@svrnm
Copy link
Member Author

svrnm commented Feb 13, 2023

PR: #3575

Makes me code obsolete 😢 ... na, I'm fine with that, @mwear's code is much better than mine, will give it a look :-)

@mwear
Copy link
Member

mwear commented Feb 15, 2023

Sorry @svrnm, I didn't realize you were also working this. My bad. I appreciate the reviews! Let me know if I can make it up somehow.

@svrnm
Copy link
Member Author

svrnm commented Feb 15, 2023

Sorry @svrnm, I didn't realize you were also working this. My bad. I appreciate the reviews! Let me know if I can make it up somehow.

No worries, your code is much better:-) If your eally insist on making it up, take a few minutes to update the docs after your PR gets merged:

https://opentelemetry.io/docs/instrumentation/js/resources/

:-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants