Skip to content

SystemRule calculate process cpu usage to support application running in container #1204

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

Merged

Conversation

tianhaowhu
Copy link
Contributor

@tianhaowhu tianhaowhu commented Dec 16, 2019

Describe what this PR does / why we need it

SystemRule currently use the host's cpu usage, so it doesn't support the application running in the container such as docker, etc. This PR leverage both the process cpu usage and host's cpu usage to support application running in container.

Does this pull request fix one issue?

NONE

Describe how you did it

In the SystemStatusListener, I leverage JMX to get processCpuTime, processUpTime and cpuCores(the number of cpu cores, and it is the number of cpu cores which assigned to the container if application running in container).

Then the process cpu usage can be calculated as below:
processCpuUsage := delta(processCpuTime) / delta(processUpTime) / cpuCores

After that, we can use the max value between host's cpu usage and processCpuUsage to check the system status

Describe how to verify it

Special notes for reviews

@sczyh30
Copy link
Member

sczyh30 commented Dec 16, 2019

Hi, thanks for contributing. Could you please sign the CLA here? And please make sure the email of your commits match your GitHub email. You may refer to the instruction here, or rebase your commits with the correct email.


感谢贡献,请将 commit 对应的 email 调整成与 GitHub 的 email 相匹配并 确认一下 CLA

@sczyh30
Copy link
Member

sczyh30 commented Dec 16, 2019

And could you please illustrate your design (and how you did it) in the PR description?

@codecov-io
Copy link

Codecov Report

Merging #1204 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1204      +/-   ##
============================================
+ Coverage     43.03%   43.05%   +0.01%     
+ Complexity     1570     1568       -2     
============================================
  Files           337      337              
  Lines          9877     9890      +13     
  Branches       1332     1332              
============================================
+ Hits           4251     4258       +7     
- Misses         5099     5102       +3     
- Partials        527      530       +3
Impacted Files Coverage Δ Complexity Δ
...sp/sentinel/slots/system/SystemStatusListener.java 57.5% <100%> (+20.46%) 3 <0> (ø) ⬇️
...ava/com/alibaba/csp/sentinel/node/ClusterNode.java 96.55% <0%> (-3.45%) 10% <0%> (-1%)
...a/csp/sentinel/slots/statistic/base/LeapArray.java 67.32% <0%> (-2.98%) 33% <0%> (-1%)
...tinel/slots/block/flow/param/ParamFlowChecker.java 52.7% <0%> (-2.71%) 28% <0%> (-1%)
...m/alibaba/csp/sentinel/log/DateFileLogHandler.java 57.57% <0%> (+3.03%) 7% <0%> (+1%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 00dbb5c...855b3bb. Read the comment docs.

@tianhaowhu
Copy link
Contributor Author

And could you please illustrate your design (and how you did it) in the PR description?

sure

@sczyh30 sczyh30 added the to-review To review label Dec 17, 2019
@sczyh30 sczyh30 added the kind/enhancement Category issues or prs related to enhancement. label Feb 21, 2020
@CLAassistant
Copy link

CLA assistant check
All committers have signed the CLA.

@sczyh30 sczyh30 requested review from sczyh30 and jasonjoo2010 March 4, 2020 15:52
Copy link
Collaborator

@jasonjoo2010 jasonjoo2010 left a comment

Choose a reason for hiding this comment

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

Oh this PR was submitted months ago. Is the submitter still here?

@tianhaowhu tianhaowhu requested a review from jasonjoo2010 March 6, 2020 04:41
Copy link
Collaborator

@jasonjoo2010 jasonjoo2010 left a comment

Choose a reason for hiding this comment

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

LGTM

@jasonjoo2010 jasonjoo2010 merged commit 6e0d116 into alibaba:master Mar 6, 2020
@jasonjoo2010
Copy link
Collaborator

Thanks for contribution and look forward for more!

@brucelwl
Copy link

为什么不直接用double processCpuLoad = osBean.getProcessCpuLoad();获取呢?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Category issues or prs related to enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants