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

koordlet: add metrics for batch resources #913

Merged
merged 1 commit into from
Jan 5, 2023

Conversation

saintube
Copy link
Member

@saintube saintube commented Dec 30, 2022

Signed-off-by: saintube saintube@foxmail.com

Ⅰ. Describe what this PR does

Add Prometheus metrics for node's and pod's batch resources, including metrics below:

# node-level batch resources
koordlet_node_resource_allocatable{resource="kubernetes.io/batch-cpu|kubernetes.io/batch-memory", node=}
# container-level batch resources (aggregatable to pod-level)
koordlet_container_resource_request{resource="kubernetes.io/batch-cpu|kubernetes.io/batch-memory", node=, pod=, namespace=, container=}
koordlet_container_resource_limit{resource="kubernetes.io/batch-cpu|kubernetes.io/batch-memory", node=, pod=, namespace=, container=}

Fix some spelling errors.

Fix metrics koordlet_node_used_cpu_cores and koordlet_be_suppress_ls_used_cpu_cores, and use cores instead of milli-cores as the unit.

Ⅱ. Does this pull request fix one issue?

Ⅲ. Describe how to verify it

Ⅳ. Special notes for reviews

V. Checklist

  • I have written necessary docs and comments
  • I have added necessary unit tests and integration tests
  • All checks passed in make test

@LeoLiuYan
Copy link
Contributor

Very cool ! Finally, we have the BE-pods metrics. @saintube

@codecov
Copy link

codecov bot commented Dec 30, 2022

Codecov Report

Base: 66.56% // Head: 66.64% // Increases project coverage by +0.08% 🎉

Coverage data is based on head (817962a) compared to base (3c6b1b8).
Patch coverage: 75.42% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #913      +/-   ##
==========================================
+ Coverage   66.56%   66.64%   +0.08%     
==========================================
  Files         240      241       +1     
  Lines       27754    27858     +104     
==========================================
+ Hits        18474    18567      +93     
- Misses       8009     8015       +6     
- Partials     1271     1276       +5     
Flag Coverage Δ
unittests 66.64% <75.42%> (+0.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/koordlet/metrics/common.go 67.85% <ø> (ø)
pkg/koordlet/metricsadvisor/collector.go 43.79% <0.00%> (ø)
pkg/koordlet/resmanager/resmanager.go 55.90% <0.00%> (ø)
pkg/koordlet/statesinformer/states_node.go 55.14% <73.91%> (+4.55%) ⬆️
pkg/koordlet/metrics/resource_summary.go 74.28% <74.28%> (ø)
pkg/koordlet/statesinformer/states_pods.go 62.05% <91.30%> (+10.39%) ⬆️
pkg/koordlet/metrics/metrics.go 88.88% <100.00%> (+0.42%) ⬆️
pkg/koordlet/resmanager/cpu_suppress.go 68.88% <100.00%> (ø)
pkg/util/resource.go 48.57% <100.00%> (+3.11%) ⬆️
pkg/util/httputil/reverseproxy.go 85.10% <0.00%> (+0.79%) ⬆️
... and 2 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@saintube saintube force-pushed the add-resource-metrics branch 5 times, most recently from b4f21a7 to a879b43 Compare January 3, 2023 06:40
@zwzhang0107
Copy link
Contributor

/lgtm

Copy link
Member

@jasonliu747 jasonliu747 left a comment

Choose a reason for hiding this comment

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

🤏

pkg/koordlet/metricsadvisor/collector.go Outdated Show resolved Hide resolved
@saintube
Copy link
Member Author

saintube commented Jan 3, 2023

/cc @jasonliu747 @zwzhang0107 @FillZpp

Copy link
Member

@jasonliu747 jasonliu747 left a comment

Choose a reason for hiding this comment

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

/lgtm

Signed-off-by: saintube <saintube@foxmail.com>
@zwzhang0107
Copy link
Contributor

/approve

Copy link
Member

@jasonliu747 jasonliu747 left a comment

Choose a reason for hiding this comment

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

/lgtm

@koordinator-bot koordinator-bot bot added the lgtm label Jan 4, 2023
Copy link
Member

@FillZpp FillZpp left a comment

Choose a reason for hiding this comment

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

/lgtm

@koordinator-bot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: FillZpp, zwzhang0107

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@koordinator-bot koordinator-bot bot merged commit a17a9cd into koordinator-sh:main Jan 5, 2023
FillZpp pushed a commit that referenced this pull request Jan 16, 2023
Signed-off-by: saintube <saintube@foxmail.com>
lucming pushed a commit to lucming/koordinator that referenced this pull request Feb 8, 2023
Signed-off-by: saintube <saintube@foxmail.com>
@zwzhang0107 zwzhang0107 added this to the v1.2 milestone Apr 6, 2023
@saintube
Copy link
Member Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants