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 cpu info #209

Merged
merged 10 commits into from
Sep 7, 2023
Merged

Conversation

ChrsMark
Copy link
Member

@ChrsMark ChrsMark commented Jul 25, 2023

This PR adds CPU information as proposed with #130.

The CPU related information go under the host.cpu namespace.

ChrsMark added 2 commits July 25, 2023 16:53
Signed-off-by: ChrsMark <[email protected]>
Signed-off-by: ChrsMark <[email protected]>
@ChrsMark ChrsMark requested review from a team July 25, 2023 14:00
model/resource/host.yaml Outdated Show resolved Hide resolved
docs/resource/host.md Outdated Show resolved Hide resolved
@ChrsMark
Copy link
Member Author

ChrsMark commented Aug 4, 2023

Hey @mx-psi @frzifus !

I have moved the Frequency information out of this patch and only kept the resource attributes. I think we can unblock this one and add the Frequency as a metric later after we have #89 merged and #226 can give more clarity about metrics' namespace.
Let me know what you think :).

Copy link
Member

@frzifus frzifus left a comment

Choose a reason for hiding this comment

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

LGTM, should we then add the frequency as a metric afterwards?

Missed (comment). 😄

@ChrsMark
Copy link
Member Author

ChrsMark commented Aug 4, 2023

LGTM, should we then add the frequency as a metric afterwards?

Missed (comment). smile

:) Yeap I think we should treat it as a Metric. example:

semantic-conventions git:(add_host_cpu_info) cat /proc/cpuinfo | grep MHz                                    
cpu MHz		: 3000.000
cpu MHz		: 1054.524
cpu MHz		: 1093.029
cpu MHz		: 3000.000
cpu MHz		: 3000.000
cpu MHz		: 3000.000
cpu MHz		: 3000.000
cpu MHz		: 3000.000semantic-conventions git:(add_host_cpu_info) cat /proc/cpuinfo | grep MHz
cpu MHz		: 1467.399
cpu MHz		: 3000.000
cpu MHz		: 3000.000
cpu MHz		: 3000.000
cpu MHz		: 3000.000
cpu MHz		: 3000.000
cpu MHz		: 2462.027
cpu MHz		: 3000.000

docs/resource/host.md Outdated Show resolved Hide resolved
@mx-psi
Copy link
Member

mx-psi commented Aug 30, 2023

@AlexanderWert mind taking another look?

Copy link
Member

@joaopgrassi joaopgrassi left a comment

Choose a reason for hiding this comment

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

Please add a changelog entry

docs/resource/host.md Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
Co-authored-by: Joao Grassi <[email protected]>
@joaopgrassi
Copy link
Member

joaopgrassi commented Sep 4, 2023

@ChrsMark make sure to resolve the discussions if you think they are resolved (if not then ask the persons if you can resolve). That would help others to see if this is in a final state or not.

@ChrsMark
Copy link
Member Author

ChrsMark commented Sep 4, 2023

Thanks @joaopgrassi, based on the latest discussions/approvals all those are resolved. Marked them properly.

@ChrsMark ChrsMark requested a review from frzifus September 4, 2023 11:34
@ChrsMark
Copy link
Member Author

ChrsMark commented Sep 5, 2023

@joaopgrassi @AlexanderWert anything else missing here?

@AlexanderWert
Copy link
Member

@open-telemetry/specs-semconv-maintainers I think we are good to merge this

@joaopgrassi
Copy link
Member

@open-telemetry/specs-semconv-approvers @open-telemetry/specs-semconv-maintainers we discussed this in the System semconv SIG and should be good to go. Will give it another day, but if nothing then I will merge.

@joaopgrassi joaopgrassi merged commit fcad0aa into open-telemetry:main Sep 7, 2023
8 checks passed
@ChrsMark ChrsMark mentioned this pull request Sep 7, 2023
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants