- 
                Notifications
    You must be signed in to change notification settings 
- Fork 1.1k
Add container_spec_memory_limit_bytes metric #9140
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 container_spec_memory_limit_bytes metric #9140
Conversation
| Hi @bingikarthik. Thanks for your PR. I'm waiting for a cri-o member to verify that this patch is reasonable to test. If it is, they should reply with  Once the patch is verified, the new status will be reflected by the  I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. | 
d96a5e9    to
    77b3086      
    Compare
  
    | /ok-to-test | 
| /retest | 
| Codecov ReportAll modified and coverable lines are covered by tests ✅ 
 Additional details and impacted files@@            Coverage Diff             @@
##             main    #9140      +/-   ##
==========================================
- Coverage   47.00%   47.00%   -0.01%     
==========================================
  Files         161      161              
  Lines       23634    23634              
==========================================
- Hits        11109    11108       -1     
- Misses      11425    11426       +1     
  Partials     1100     1100              🚀 New features to boost your workflow:
 | 
| /retest | 
| @bingikarthik Can you rebase and align it with the new way of writing? | 
77b3086    to
    a77066b      
    Compare
  
    a41e951    to
    7f22c7d      
    Compare
  
    | 
 The PR has been updated now. Could you please review the changes? Thanks.. | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you run make lint to fix the lint errors? I think the errors can be fixed just by running it.
5ab1aef    to
    20c51cd      
    Compare
  
    | Thanks! lgtm if the tests pass | 
Signed-off-by: Bingi Narasimha Karthik <[email protected]>
20c51cd    to
    1a3c6f4      
    Compare
  
    Signed-off-by: Bingi Narasimha Karthik <[email protected]>
1a3c6f4    to
    68fe193      
    Compare
  
    | 
 Could you please review, seems tests passed now except  | 
| /lgtm @cri-o/cri-o-maintainers PTAL | 
| /override ci/prow/ci-e2e-evented-pleg | 
| @haircommander: Overrode contexts on behalf of haircommander: ci/prow/ci-e2e-evented-pleg In response to this: 
 Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. | 
| [APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bingikarthik, haircommander The full list of commands accepted by this bot can be found here. The pull request process is described here 
Needs approval from an approver in each of these files:
 
 Approvers can indicate their approval by writing  | 
| /retest | 
What type of PR is this?
/kind feature
What this PR does / why we need it:
This PR adds the container_spec_memory_limit_bytes metric to the CRI-O metrics suite, which reports the memory limit for containers in bytes. This metric is valuable for monitoring and alerting on memory configuration versus usage. It allows users to easily calculate memory usage percentages and visualize resource allocation versus consumption in monitoring systems like Prometheus and Grafana.
Which issue(s) this PR fixes:
Special notes for your reviewer:
The implementation follows the existing pattern for metrics in CRI-O. The new metric has been added to the "memory" metrics section and implemented in the generateSandboxMemoryMetrics function to collect memory limit data from cgroup stats.
Does this PR introduce a user-facing change?
None