Skip to content

Add host cpu info#209

Merged
joaopgrassi merged 10 commits intoopen-telemetry:mainfrom
ChrsMark:add_host_cpu_info
Sep 7, 2023
Merged

Add host cpu info#209
joaopgrassi merged 10 commits intoopen-telemetry:mainfrom
ChrsMark:add_host_cpu_info

Conversation

@ChrsMark
Copy link
Copy Markdown
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 <chrismarkou92@gmail.com>
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark ChrsMark requested review from a team July 25, 2023 14:00
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
@ChrsMark
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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

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

mx-psi commented Aug 30, 2023

@AlexanderWert mind taking another look?

Copy link
Copy Markdown
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

Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
Co-authored-by: Joao Grassi <joao@joaograssi.com>
@joaopgrassi
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member Author

ChrsMark commented Sep 5, 2023

@joaopgrassi @AlexanderWert anything else missing here?

@AlexanderWert
Copy link
Copy Markdown
Member

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

@joaopgrassi
Copy link
Copy Markdown
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.

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