Skip to content

[Instrumentation.Process] Exposed required metric process.cpu.count for computing CPU utilization in the backend.#981

Merged
cijothomas merged 22 commits into
open-telemetry:mainfrom
Yun-Ting:yunl/processorCount
Feb 15, 2023
Merged

[Instrumentation.Process] Exposed required metric process.cpu.count for computing CPU utilization in the backend.#981
cijothomas merged 22 commits into
open-telemetry:mainfrom
Yun-Ting:yunl/processorCount

Conversation

@Yun-Ting

@Yun-Ting Yun-Ting commented Feb 9, 2023

Copy link
Copy Markdown
Contributor

Based on spec discussion: open-telemetry/opentelemetry-specification#2392 (comment)
Related to: #972.

Changes

Exposed required metric process.processors for computing CPU utilization in the backend.

For significant contributions please make sure you have completed the following items:

  • Appropriate CHANGELOG.md updated for non-trivial changes
  • Design discussion issue #
  • Changes in public API reviewed

@Yun-Ting
Yun-Ting marked this pull request as ready for review February 9, 2023 01:10
@Yun-Ting
Yun-Ting requested a review from a team February 9, 2023 01:10
@Kielek

Kielek commented Feb 9, 2023

Copy link
Copy Markdown
Member

Can we open discussion on spec side once more time? The minimal effort - decide the name in the spec side.
I would like to avoid some custom metric in the repository created only for one language and then potential subject to change.

@CodeBlanch CodeBlanch left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

process.processors is kind of tongue twister 🤣 process.cpu.count is much nicer IMO. Probably process.threads -> process.thread.count would also be good.

Comment thread src/OpenTelemetry.Instrumentation.Process/README.md Outdated
@Yun-Ting

Yun-Ting commented Feb 9, 2023

Copy link
Copy Markdown
Contributor Author

Can we open discussion on spec side once more time? The minimal effort - decide the name in the spec side. I would like to avoid some custom metric in the repository created only for one language and then potential subject to change.

I've created an issue in the spec side!
open-telemetry/semantic-conventions#651

@Yun-Ting Yun-Ting changed the title [Instrumentation.Process] Exposed required metric process.processors for computing CPU utilization in the backend. [Instrumentation.Process] Exposed required metric process.processors for computing CPU utilization in the backend. (pending spec discussion) Feb 9, 2023

@reyang reyang left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, the spec dependency is not a blocker IMHO given the component and the spec are both preview/experimental.

Comment thread src/OpenTelemetry.Instrumentation.Process/README.md Outdated
@Yun-Ting Yun-Ting changed the title [Instrumentation.Process] Exposed required metric process.processors for computing CPU utilization in the backend. (pending spec discussion) [Instrumentation.Process] Exposed required metric process.processors for computing CPU utilization in the backend. Feb 13, 2023
Co-authored-by: Cijo Thomas <cithomas@microsoft.com>
@Yun-Ting Yun-Ting changed the title [Instrumentation.Process] Exposed required metric process.processors for computing CPU utilization in the backend. [Instrumentation.Process] Exposed required metric process.cpu.count for computing CPU utilization in the backend. Feb 13, 2023
@codecov

codecov Bot commented Feb 14, 2023

Copy link
Copy Markdown

Codecov Report

Merging #981 (5649f33) into main (4d7f93e) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     open-telemetry/opentelemetry-specification#981      +/-   ##
==========================================
+ Coverage   68.11%   68.13%   +0.02%     
==========================================
  Files         184      184              
  Lines        7013     7021       +8     
==========================================
+ Hits         4777     4784       +7     
- Misses       2236     2237       +1     
Impacted Files Coverage Δ
...elemetry.Instrumentation.Process/ProcessMetrics.cs 100.00% <100.00%> (ø)
...try.Extensions/Trace/AutoFlushActivityProcessor.cs 77.77% <0.00%> (-3.71%) ⬇️

Comment thread src/OpenTelemetry.Instrumentation.Process/README.md Outdated
Co-authored-by: Reiley Yang <reyang@microsoft.com>
@Kielek Kielek added the comp:instrumentation.process Things related to OpenTelemetry.Instrumentation.Process label Feb 14, 2023
Comment thread src/OpenTelemetry.Instrumentation.Process/README.md Outdated
@cijothomas
cijothomas merged commit c686047 into open-telemetry:main Feb 15, 2023
@Yun-Ting
Yun-Ting deleted the yunl/processorCount branch February 15, 2023 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp:instrumentation.process Things related to OpenTelemetry.Instrumentation.Process

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants