feat(metrics): refactor idle power exposure #1810
Conversation
Signed-off-by: Huamin Chen <[email protected]>
|
🤖 SeineSailor Here is a concise summary of the pull request changes: Summary: The
Impact: These changes do not affect the external interface or behavior of the code. However, it's essential to ensure that the optional idle power classification does not introduce any performance or accuracy issues. Additionally, it would be beneficial to include tests to verify the correctness of the refactored code. |
There was a problem hiding this comment.
@rootfs Code lgtm. So this ensures idle Energy is always 0 now? Dynpower will be reflective of the total energy? This means if I validate kepler's energy metrics with node exporter, they should be essentially identical.
| @@ -174,7 +174,7 @@ func (s *Stats) CalcDynEnergy(absM, idleM, dynM, id string) { | |||
|
|
|||
| // calcDynEnergy calculates the dynamic energy. | |||
| func calcDynEnergy(totalE, idleE uint64) uint64 { | |||
| if (totalE == 0) || (idleE == 0) || (totalE < idleE) { | |||
| if (totalE == 0) || (totalE < idleE) { | |||
There was a problem hiding this comment.
I think we should fix this bit as well ..
If the totalE < the idle, then we have a new idle, IOW kepler over time will continue to improve its accuracy of idle by measuring the least amount of power consumed by the system.
There was a problem hiding this comment.
Agree but I think should note and fix this in the different issue and PR.
The logic seems to be around this line: https://github.com/rootfs/kepler/blob/20385219b8230fad9c6d1c792b57cde4c1419292/pkg/collector/stats/node_stats.go#L72
I think we may start from do not set idle power to any value if resource usage is more than some threshold.
As discussed in the Oct 15 meeting, we make the idle power classification optional, until we have an idle power regression model that can predict idle power by system utilization.