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

Add cpu usage stats to heartbeat messages #14

Merged
merged 3 commits into from
Feb 25, 2020
Merged

Add cpu usage stats to heartbeat messages #14

merged 3 commits into from
Feb 25, 2020

Conversation

aparmet-toast
Copy link
Contributor

@aparmet-toast aparmet-toast commented Feb 18, 2020

Locust since 0.14.0 requires CPU usage to be reported by clients. Somehow this also prevented the master from properly recording heartbeats (although I'm not sure how, as that happens earlier in the control flow).

CPU usage extraction: https://github.com/locustio/locust/blob/93eec54bd53576304c925ceb8c1e8de5981d7b24/locust/runners.py#L462

Heartbeat reset: https://github.com/locustio/locust/blob/93eec54bd53576304c925ceb8c1e8de5981d7b24/locust/runners.py#L460

No matter, this should be included and also appears to fix the heartbeat issue.

@codecov-io
Copy link

codecov-io commented Feb 19, 2020

Codecov Report

Merging #14 into master will increase coverage by 0.44%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #14      +/-   ##
=========================================
+ Coverage   78.25%   78.7%   +0.44%     
=========================================
  Files          19      19              
  Lines         860     864       +4     
  Branches       78      78              
=========================================
+ Hits          673     680       +7     
+ Misses        169     165       -4     
- Partials       18      19       +1
Impacted Files Coverage Δ
...ava/com/github/myzhan/locust4j/runtime/Runner.java 73.51% <100%> (+2.79%) ⬆️
...n/java/com/github/myzhan/locust4j/stats/Stats.java 94.48% <0%> (-0.79%) ⬇️

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 03327e1...c7ca05c. Read the comment docs.

@@ -371,6 +377,14 @@ public void run() {
}
}
}

private int getCpuUsage() {
return (int) osBean.getSystemCpuLoad() * 100;
Copy link
Owner

Choose a reason for hiding this comment

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

Why don't we use getProcessCpuLoad and return a float number?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe Locust is interested in the CPU usage of the entire machine in case there are other processes pegging the CPU. As I understand the concern is that the task will provide inaccurate results if it's restricted by underlying hardware, so the JVM process load isn't enough information.

As far as int vs. float, I'm actually not sure.

@myzhan myzhan merged commit 546727e into myzhan:master Feb 25, 2020
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