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

[resource/host] Change type of host.cpu.stepping to string #665

Merged
merged 4 commits into from
Feb 8, 2024

Conversation

ChrsMark
Copy link
Member

@ChrsMark ChrsMark commented Jan 23, 2024

Fixes #664

Changes

This PR changes the type of host.cpu.stepping to string in order to cover values like r1p1 as described at #664.

Merge requirement checklist

Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark ChrsMark requested review from a team January 23, 2024 13:50
@ChrsMark ChrsMark requested a review from mx-psi January 23, 2024 13:50
model/registry/host.yaml Show resolved Hide resolved
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
Copy link
Contributor

@jsuereth jsuereth left a comment

Choose a reason for hiding this comment

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

Ok with this change mostly assuming this is a rare/new field that isn't used much.

If someone can confirm this field is little used in the wild, that'd be ideal.

@mx-psi
Copy link
Member

mx-psi commented Feb 6, 2024

If someone can confirm this field is little used in the wild, that'd be ideal.

From a Github search, only the OpenTelemetry Collector resource detection processor system detector supports this attribute. It is not enabled by default and we have done similar changes to other host.cpu fields in the past without much complaint, so I think it's safe to assume it's not widely used.

@ChrsMark
Copy link
Member Author

ChrsMark commented Feb 8, 2024

I guess we need to update this PR to use the new changelog structure: https://github.com/open-telemetry/semantic-conventions/blob/main/CONTRIBUTING.md#adding-a-changelog-entry. @joaopgrassi is that correct?

@joaopgrassi
Copy link
Member

I guess we need to update this PR to use the new changelog structure: main/CONTRIBUTING.md#adding-a-changelog-entry. @joaopgrassi is that correct?

@ChrsMark yes that would be great, thanks! Then we are ready to merge :)

Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark
Copy link
Member Author

ChrsMark commented Feb 8, 2024

@joaopgrassi done cdbe5c2

@arminru arminru merged commit a78115e into open-telemetry:main Feb 8, 2024
10 checks passed
ChrsMark added a commit to ChrsMark/semantic-conventions that referenced this pull request Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

host.cpu.stepping should be string
9 participants