You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2022/04/11 18:46:21 UTC
[GitHub] [cloudstack] JoaoJandre opened a new pull request, #6245: Fix VM stats inconsistencies
JoaoJandre opened a new pull request, #6245:
URL: https://github.com/apache/cloudstack/pull/6245
### Description
This PR aims to fix various inconsistencies in the front and back-end concerning the VM statistics. I mainly enhanced the descriptions, adding explanations and/or context, but also corrected some errors in the units of measurements (there were cases of metrics being represented in bytes when the correct one should be KiB, for example).
Also, many inconsistencies were tested and found in the way stats were being collected for the KVM hypervisor. Namely, when generating the `VmMetricsResponse`, the network usage was being overwritten with the disk usage, so neither disk nor network usage was correct. Another issue was that memory usage wasn't being properly collected.
### Types of changes
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [X] Bug fix (non-breaking change which fixes an issue)
- [ ] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
### Feature/Enhancement Scale or Bug Severity
#### Feature/Enhancement Scale
- [ ] Major
- [X] Minor
#### Bug Severity
- [ ] BLOCKER
- [ ] Critical
- [X] Major
- [ ] Minor
- [ ] Trivial
### Screenshots (if appropriate):
### How Has This Been Tested?
This was tested in a local lab, by creating VMs and comparing their internal stats with the ones gathered by CloudStack.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #6245: Fix VM stats inconsistencies
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on code in PR #6245:
URL: https://github.com/apache/cloudstack/pull/6245#discussion_r848224671
##########
ui/public/locales/pl.json:
##########
@@ -623,8 +623,8 @@
"label.diskiopswriterate": "Disk Write Rate (IOPS)",
"label.diskioread": "Disk Read (IO)",
"label.diskiowrite": "Disk Write (IO)",
-"label.diskkbsread": "Disk Read (Bytes)",
-"label.diskkbswrite": "Disk Write (Bytes)",
+"label.diskkbsread": "Disk Read",
+"label.diskkbswrite": "Disk Write",
Review Comment:
should these contain a `(KiB)` as well?
##########
plugins/metrics/src/main/java/org/apache/cloudstack/response/VmMetricsResponse.java:
##########
@@ -36,7 +36,7 @@ public class VmMetricsResponse extends UserVmResponse {
private String cpuTotal;
@SerializedName("memorytotal")
- @Param(description = "the total memory capacity in GiB")
+ @Param(description = "the total memory capacity in GB")
Review Comment:
is this `1000^3` and not `1024^3`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] JoaoJandre commented on a diff in pull request #6245: Fix VM stats inconsistencies
Posted by GitBox <gi...@apache.org>.
JoaoJandre commented on code in PR #6245:
URL: https://github.com/apache/cloudstack/pull/6245#discussion_r848694296
##########
plugins/metrics/src/main/java/org/apache/cloudstack/response/VmMetricsResponse.java:
##########
@@ -36,7 +36,7 @@ public class VmMetricsResponse extends UserVmResponse {
private String cpuTotal;
@SerializedName("memorytotal")
- @Param(description = "the total memory capacity in GiB")
+ @Param(description = "the total memory capacity in GB")
Review Comment:
I think you're correct, I believe this is indeed 1024^3.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6245: Fix VM stats inconsistencies
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6245:
URL: https://github.com/apache/cloudstack/pull/6245#issuecomment-1103363164
@nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6245: Fix VM stats inconsistencies
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6245:
URL: https://github.com/apache/cloudstack/pull/6245#issuecomment-1100306149
@rohityadavcloud a Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a diff in pull request #6245: Fix VM stats inconsistencies
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on code in PR #6245:
URL: https://github.com/apache/cloudstack/pull/6245#discussion_r849591620
##########
ui/public/locales/pl.json:
##########
@@ -623,8 +623,8 @@
"label.diskiopswriterate": "Disk Write Rate (IOPS)",
"label.diskioread": "Disk Read (IO)",
"label.diskiowrite": "Disk Write (IO)",
-"label.diskkbsread": "Disk Read (Bytes)",
-"label.diskkbswrite": "Disk Write (Bytes)",
+"label.diskkbsread": "Disk Read",
+"label.diskkbswrite": "Disk Write",
Review Comment:
@JoaoJandre but they are included in the other languages for `label.diskkbsread` and `label.diskkbswrite`, and not in the polish json. Why is that?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] JoaoJandre commented on a diff in pull request #6245: Fix VM stats inconsistencies
Posted by GitBox <gi...@apache.org>.
JoaoJandre commented on code in PR #6245:
URL: https://github.com/apache/cloudstack/pull/6245#discussion_r848676611
##########
ui/public/locales/pl.json:
##########
@@ -623,8 +623,8 @@
"label.diskiopswriterate": "Disk Write Rate (IOPS)",
"label.diskioread": "Disk Read (IO)",
"label.diskiowrite": "Disk Write (IO)",
-"label.diskkbsread": "Disk Read (Bytes)",
-"label.diskkbswrite": "Disk Write (Bytes)",
+"label.diskkbsread": "Disk Read",
+"label.diskkbswrite": "Disk Write",
Review Comment:
They shouldn't, because these labels are used as column titles in instances tab when the metrics toggle is on. And to maintain the same pattern with the other columns, the units of measurements should be alongside the values, not the title.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #6245: Fix VM stats inconsistencies
Posted by GitBox <gi...@apache.org>.
nvazquez commented on PR #6245:
URL: https://github.com/apache/cloudstack/pull/6245#issuecomment-1103361201
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6245: Fix VM stats inconsistencies
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6245:
URL: https://github.com/apache/cloudstack/pull/6245#issuecomment-1103797017
<b>Trillian test result (tid-3934)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 31127 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6245-t3934-kvm-centos7.zip
Smoke tests completed. 93 look OK, 0 have errors
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez merged pull request #6245: Fix VM stats inconsistencies
Posted by GitBox <gi...@apache.org>.
nvazquez merged PR #6245:
URL: https://github.com/apache/cloudstack/pull/6245
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #6245: Fix VM stats inconsistencies
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on PR #6245:
URL: https://github.com/apache/cloudstack/pull/6245#issuecomment-1100346796
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 3183
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] JoaoJandre commented on a diff in pull request #6245: Fix VM stats inconsistencies
Posted by GitBox <gi...@apache.org>.
JoaoJandre commented on code in PR #6245:
URL: https://github.com/apache/cloudstack/pull/6245#discussion_r849726831
##########
ui/public/locales/pl.json:
##########
@@ -623,8 +623,8 @@
"label.diskiopswriterate": "Disk Write Rate (IOPS)",
"label.diskioread": "Disk Read (IO)",
"label.diskiowrite": "Disk Write (IO)",
-"label.diskkbsread": "Disk Read (Bytes)",
-"label.diskkbswrite": "Disk Write (Bytes)",
+"label.diskkbsread": "Disk Read",
+"label.diskkbswrite": "Disk Write",
Review Comment:
I'm sorry @DaanHoogland , I misread the labels, I thought that you were talking about the label `label.diskread`. You're correct, these should contain a (KiB) as well. I'll fix this ASAP.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rohityadavcloud commented on pull request #6245: Fix VM stats inconsistencies
Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on PR #6245:
URL: https://github.com/apache/cloudstack/pull/6245#issuecomment-1100305874
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org