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

feat(internal/metadataproviders/system): do not return empty host.id #24239

Merged
merged 5 commits into from
Jul 14, 2023

Conversation

sumo-drosiek
Copy link
Member

@sumo-drosiek sumo-drosiek commented Jul 12, 2023

Description:

Do not return empty host ID

Link to tracking Issue: #24230

Testing: Unit tests

Documentation: N/A

Copy link
Member

@mx-psi mx-psi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would be great to merge for v0.82.0 as a temporary measure, we would just need a unit test and a changelog entry

@sumo-drosiek sumo-drosiek marked this pull request as ready for review July 14, 2023 08:14
@sumo-drosiek sumo-drosiek requested a review from a team July 14, 2023 08:14
@mx-psi mx-psi merged commit 7e6145c into open-telemetry:main Jul 14, 2023
@github-actions github-actions bot added this to the next release milestone Jul 14, 2023
@sumo-drosiek sumo-drosiek deleted the drosiek-host-id branch July 14, 2023 09:30
dmitryax added a commit to dmitryax/opentelemetry-collector-contrib that referenced this pull request Jul 28, 2023
Do not drop all system attributes if `host.id` cannot be fetched. This is a fix that restores the behavior closer to how it used to be before open-telemetry#24239.

This probably should be applied to all other attributes going forward: if any of the system attributes cannot be fetched, we should still try to set others. But that will be a significant change in the processor behavior, so it should be handled not as a hotfix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants