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

Expose process CPU Entitlement in stats #3641

Merged
merged 4 commits into from
Feb 21, 2024

Conversation

acrmp
Copy link
Member

@acrmp acrmp commented Feb 15, 2024

Thanks for contributing to cloud_controller_ng. To speed up the process of reviewing your pull request please provide us with:

  • A short explanation of the proposed change:

Extend the existing v3 processes stats endpoint to include the CPU Entitlement for the process.

  • An explanation of the use cases your change solves

It can be difficult to reason about process CPU usage using the existing cpu stat. For example the value is impacted by changes to the underlying vCPUs available on the Diego Cell. CPU Entitlement is a more generally useful metric.

https://www.cloudfoundry.org/blog/better-way-split-cake-cpu-entitlements/

  • Links to any other associated PRs

cloudfoundry/diego-release#897

  • I have reviewed the contributing guide

  • I have viewed, signed, and submitted the Contributor License Agreement

  • I have made this pull request to the main branch

  • I have run all the unit tests using bundle exec rake

  • I have run CF Acceptance Tests

@mkocher

acrmp and others added 4 commits February 14, 2024 19:34
- Assert that we don't create a container metric batch when the
  envelopes contain no container metrics
- Change example units for cpu and log rate

Co-authored-by: Matthew Kocher <matthew.kocher@broadcom.com>
- There is existing logic to set all container metrics to zero when any
  given reported container metric is missing.
- Chose not to implement the same behaviour for cpu_entitlement as this
  would prevent other stats from being available before Diego Cells are
  upgraded.

Co-authored-by: Matthew Kocher <matthew.kocher@broadcom.com>
Co-authored-by: Matthew Kocher <matthew.kocher@broadcom.com>
Signed-off-by: Matthew Kocher <matthew.kocher@broadcom.com>
@sethboyles sethboyles merged commit b9f2933 into cloudfoundry:main Feb 21, 2024
9 checks passed
@acrmp acrmp deleted the cpu-entitlement branch February 21, 2024 00:18
acrmp added a commit to loggregator/cloud_controller_ng that referenced this pull request Jun 19, 2024
- In cloudfoundry#3641 we exposed CPU Entitlement in process stats and returned null
  when stats are not yet available in Log Cache.
- Change missing process stats to instead have CPU Entitlement of zero
  for consistency with other existing metrics. Recent versions of the cf
  CLI will show zero rather than an empty column for CPU Entitlement.
- CPU Entitlement may still be null when the metric is not available on
  the deployment.

Signed-off-by: Andrew Costa <andrew.costa@broadcom.com>
philippthun pushed a commit that referenced this pull request Jun 28, 2024
- In #3641 we exposed CPU Entitlement in process stats and returned null
  when stats are not yet available in Log Cache.
- Change missing process stats to instead have CPU Entitlement of zero
  for consistency with other existing metrics. Recent versions of the cf
  CLI will show zero rather than an empty column for CPU Entitlement.
- CPU Entitlement may still be null when the metric is not available on
  the deployment.

Signed-off-by: Andrew Costa <andrew.costa@broadcom.com>
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.

3 participants