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.

Sign up for GitHub

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

Jump to bottom

Add system.cpu.physical.count and system.cpu.logical.count metrics #99

Merged
merged 7 commits into from
Aug 1, 2023
Merged

Add system.cpu.physical.count and system.cpu.logical.count metrics #99

merged 7 commits into from
Aug 1, 2023

Conversation

frzifus
Copy link
Member

@frzifus frzifus commented Jun 8, 2023 •
edited
Loading

@frzifus frzifus force-pushed the add_system_cpu_count_metric branch from 697940c to 7f076c4 Compare June 8, 2023 09:54
@trask trask mentioned this pull request Jun 8, 2023
Merged
@frzifus frzifus marked this pull request as ready for review June 8, 2023 21:46
@frzifus frzifus requested review from a team June 8, 2023 21:46
@mx-psi mx-psi mentioned this pull request Jun 9, 2023
Merged
@frzifus frzifus force-pushed the add_system_cpu_count_metric branch from 7f076c4 to 0449e70 Compare June 21, 2023 08:51
@frzifus frzifus requested a review from mx-psi June 21, 2023 08:51
mx-psi
mx-psi reviewed Jun 21, 2023
View reviewed changes
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.

Just two suggestions, otherwise looks good to me

specification/metrics/semantic_conventions/system-metrics.md Outdated Show resolved Hide resolved
specification/metrics/semantic_conventions/system-metrics.md Outdated Show resolved Hide resolved
@frzifus frzifus requested a review from mx-psi June 21, 2023 16:50
@frzifus
Copy link
Member Author

frzifus commented Jun 21, 2023 •
edited
Loading

cc @open-telemetry/specs-semconv-approvers @open-telemetry/specs-semconv-maintainers

@frzifus frzifus force-pushed the add_system_cpu_count_metric branch from 6375b31 to d01a81a Compare June 21, 2023 22:54
@frzifus frzifus force-pushed the add_system_cpu_count_metric branch from 60f24fe to 3bcb47f Compare June 26, 2023 13:53
@frzifus frzifus force-pushed the add_system_cpu_count_metric branch from bdf0191 to e3c6def Compare June 28, 2023 18:40
@frzifus frzifus force-pushed the add_system_cpu_count_metric branch from e3c6def to 4625204 Compare June 28, 2023 18:42
@frzifus frzifus requested a review from jamesmoessis June 29, 2023 11:59
@mx-psi
Copy link
Member

mx-psi commented Jun 30, 2023

@tigrannajaryan @jamesmoessis the metrics have been renamed and the description of system.cpu.time and system.cpu.utilization now clarify that there is one dimension per logical CPU. PTAL!

Separate note, this will likely conflict with #154

@frzifus frzifus force-pushed the add_system_cpu_count_metric branch from 9a6a50b to 7422683 Compare July 6, 2023 12:37
@frzifus frzifus force-pushed the add_system_cpu_count_metric branch 2 times, most recently from 30f35a1 to dd4cf63 Compare July 18, 2023 22:10
@frzifus
Copy link
Member Author

frzifus commented Jul 18, 2023

added that part @dmitryax :)

@frzifus frzifus force-pushed the add_system_cpu_count_metric branch from 74cd430 to 48d6b37 Compare July 18, 2023 22:29
@frzifus
Copy link
Member Author

frzifus commented Jul 18, 2023

Works for me too. I rephrased it a bit. Can you check one more time @dmitryax ? ^^

@frzifus @mx-psi
@tigrannajaryan @dmitryax
add system.cpu.physical.count and system.cpu.logical.count metrics
e1be1b1 
Signed-off-by: Benedikt Bongartz <[email protected]>
Co-authored-by: Pablo Baeyens <[email protected]>
Co-authored-by: Tigran Najaryan <[email protected]>
Co-authored-by: Dmitrii Anoshin <[email protected]>
@frzifus frzifus force-pushed the add_system_cpu_count_metric branch from bdc779f to e1be1b1 Compare July 18, 2023 23:25
@mx-psi
Copy link
Member

mx-psi commented Jul 26, 2023

@open-telemetry/specs-semconv-maintainers anything left to do on this PR?

@mx-psi mx-psi mentioned this pull request Jul 31, 2023
Merged
@mx-psi
Copy link
Member

mx-psi commented Aug 1, 2023

@frzifus

/home/runner/work/semantic-conventions/semantic-conventions/docs/system/system-metrics.md:36: MD012/no-multiple-blanks Multiple consecutive blank lines [Expected: 1; Actual: 2]

@frzifus
fix
a99f540 
Signed-off-by: Benedikt Bongartz <[email protected]>
@frzifus
Copy link
Member Author

frzifus commented Aug 1, 2023

Fixed

@jsuereth
Copy link
Contributor

jsuereth commented Aug 1, 2023

Ideally, we'd like this change to either be generated off YAML or wait until the system metrics are defined in YAML, then merge this PR with that change.

@arminru arminru mentioned this pull request Aug 1, 2023
Merged
@arminru arminru merged commit f4ed03e into open-telemetry:main Aug 1, 2023
@frzifus frzifus deleted the add_system_cpu_count_metric branch August 1, 2023 15:46
bryce-b pushed a commit to bryce-b/semantic-conventions that referenced this pull request Aug 8, 2023
@frzifus @mx-psi
@tigrannajaryan @dmitryax @arminru @bryce-b
Add system.cpu.physical.count and system.cpu.logical.count metrics (o…
0d5cc68 
…pen-telemetry#99)

Co-authored-by: Pablo Baeyens <[email protected]>
Co-authored-by: Tigran Najaryan <[email protected]>
Co-authored-by: Dmitrii Anoshin <[email protected]>
Co-authored-by: Armin Ruech <[email protected]>
rapphil pushed a commit to rapphil/semantic-conventions that referenced this pull request Aug 14, 2023
@frzifus @mx-psi
@tigrannajaryan @dmitryax @arminru @rapphil
Add system.cpu.physical.count and system.cpu.logical.count metrics (o…
0cf0e8d 
…pen-telemetry#99)

Co-authored-by: Pablo Baeyens <[email protected]>
Co-authored-by: Tigran Najaryan <[email protected]>
Co-authored-by: Dmitrii Anoshin <[email protected]>
Co-authored-by: Armin Ruech <[email protected]>
pvoborni pushed a commit to RedHatInsights/host-metering that referenced this pull request Sep 27, 2023
@poncovka @pvoborni
fix: Count the number of vCPUs instead of the number of CPU cores
7af0f82 
Based on the available documents, the client should collect and export "a single
metric that counts the number of VCPUs on the system". Let's use a value that
can be calculated as: `cat /proc/cpuinfo | grep processor | wc -l`

There are several alternative options and methods to consider, but let's start
with the easist one that doesn't require any additional dependencies.

Use the metric name `system_cpu_logical_count` that follows semantic conventions
for OTEL: open-telemetry/semantic-conventions#99
@mx-psi mx-psi mentioned this pull request Jul 18, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chambridge chambridge chambridge left review comments

@mx-psi mx-psi mx-psi approved these changes

@rockdaboot rockdaboot rockdaboot left review comments

@trask trask trask left review comments

@jamesmoessis jamesmoessis jamesmoessis approved these changes

@arminru arminru arminru approved these changes

@dmitryax dmitryax dmitryax approved these changes

@tigrannajaryan tigrannajaryan tigrannajaryan approved these changes

@ChrsMark ChrsMark Awaiting requested review from ChrsMark

Assignees

@jsuereth jsuereth

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

add system.cpu.count metric