Skip to content

Conversation

@shwstppr
Copy link
Contributor

@shwstppr shwstppr commented Jul 28, 2025

Description

CPU and RAM values for external resource metrics was showing NaN values. This fixes the behaviour.

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • 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)
  • build/CI
  • test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

Before:
Screenshot from 2025-07-28 11-46-23

After:
image

How Has This Been Tested?

How did you try to break this feature and the system with this change?

CPU and RAM values for external resource metrics was showing NaN values. This fixes the behaviour.

Signed-off-by: Abhishek Kumar <[email protected]>
@shwstppr shwstppr force-pushed the fix-nanmetrics-external branch from 2437763 to 913655f Compare July 28, 2025 05:27
@codecov
Copy link

codecov bot commented Jul 28, 2025

Codecov Report

❌ Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 16.99%. Comparing base (cca8b2f) to head (913655f).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
.../java/com/cloud/api/query/dao/HostJoinDaoImpl.java 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##               main   #11302   +/-   ##
=========================================
  Coverage     16.99%   16.99%           
- Complexity    14715    14718    +3     
=========================================
  Files          5832     5832           
  Lines        517561   517562    +1     
  Branches      62982    62982           
=========================================
+ Hits          87975    87984    +9     
+ Misses       419651   419643    -8     
  Partials       9935     9935           
Flag Coverage Δ
uitests 3.82% <ø> (ø)
unittests 17.95% <0.00%> (+<0.01%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sureshanaparti
Copy link
Contributor

@blueorangutan package

@blueorangutan
Copy link

@sureshanaparti a [SL] 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.

Copy link
Contributor

@harikrishna-patnala harikrishna-patnala left a comment

Choose a reason for hiding this comment

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

code LGTM

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes NaN (Not a Number) values appearing in CPU and RAM metrics for external resources by adding zero-value checks before performing division operations. The fix prevents division by zero scenarios that were causing the NaN values to appear in the UI.

Key changes:

  • Added a null/zero check in the calculateResourceAllocatedPercentage method to return "0.00%" when either the resource or resourceWithOverProvision values are zero
  • Refactored existing inline percentage calculation to use the centralized calculateResourceAllocatedPercentage method for consistency

return "0.00%";
}
DecimalFormat decimalFormat = new DecimalFormat("#.##");
return decimalFormat.format(((float)resource / resourceWithOverProvision * 100.0f)) + "%";
Copy link

Copilot AI Jul 28, 2025

Choose a reason for hiding this comment

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

The cast to (float) is unnecessary since the parameters are already declared as float. This redundant cast reduces code readability.

Suggested change
return decimalFormat.format(((float)resource / resourceWithOverProvision * 100.0f)) + "%";
return decimalFormat.format((resource / resourceWithOverProvision * 100.0f)) + "%";

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can be a separate PR/change as not touched here

@shwstppr shwstppr marked this pull request as ready for review July 28, 2025 08:18
@blueorangutan
Copy link

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 14408

Copy link
Contributor

@DaanHoogland DaanHoogland left a comment

Choose a reason for hiding this comment

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

I think both copilot’s remarks make sense. otherwise clgtm

Copy link
Contributor

@kiranchavala kiranchavala left a comment

Choose a reason for hiding this comment

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

LGTM,

Added a external host with proxmox extension

The CPU allocated shows 0.0 Now

Before fix

Screenshot 2025-07-24 at 6 57 49 PM

After fix

Screenshot 2025-07-28 at 5 04 02 PM

@sureshanaparti
Copy link
Contributor

I think both copilot’s remarks make sense. otherwise clgtm

@shwstppr can check if copilot's commits are relevant?

@shwstppr
Copy link
Contributor Author

@sureshanaparti I don't think Copilot's comment are completely relevant. I've added my comments in their reply

@sureshanaparti sureshanaparti merged commit 524f0d8 into apache:main Jul 28, 2025
41 of 43 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in Apache CloudStack 4.21.0 Jul 28, 2025
@DaanHoogland DaanHoogland deleted the fix-nanmetrics-external branch July 28, 2025 13:00
dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Aug 6, 2025
CPU and RAM values for external resource metrics was showing NaN values. This fixes the behaviour.

Signed-off-by: Abhishek Kumar <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

7 participants