Add idle time in cpu utilization attributes, revise cpu utilization check, add some helpers to Counter #294
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
November 28, 2025 17:13
- log error explanations before returning default values in cpu utilization - WIP: wait during the cpu utilization check if the gathered points are not enough apart for the range. now the checks will return a valid value instead of zeros, causing problems rn
- add counter.GetFirst() to get the oldest value of the counter - add counter.checkRetenton() to see if the designed counter retention time is adequate - fix counter.AvgForDuration function possibly including the latest value twice in its calculation - add tests for the new counter functions - check_cpu_utilization -> check ahread if the counter can fit the requested period - check_cpu_utlization -> print logs if the counter is not filled up enough for the requested period - check_cpu_utilization -> print logs about after looking for a value in the past according to the requested period
sni
requested changes
Dec 3, 2025
Contributor
sni
left a comment
sni
left a comment
There was a problem hiding this comment.
looks good, i have only some minor notes
| // round retention time to a multiple of interval | ||
| retention := int64(math.Ceil(float64(retentionMilli)/float64(intervalMilli))) * intervalMilli | ||
| size := retention / intervalMilli | ||
| // round retentionMili time to a multiple of interval |
| for seen := int64(0); seen <= c.size; seen++ { | ||
|
|
||
| //nolint:intrange // tracking the seen elements is easier | ||
| for seen := int64(0); seen < c.size; seen++ { |
Contributor
There was a problem hiding this comment.
what the linter suggested here is:
for range c.size {
which is simpler and cleaner.
| if val.UnixMilli < useAfterUnix { | ||
| return last | ||
| var previouslyComparedValue *Value | ||
| for valuesSeen := int64(0); valuesSeen <= c.size; valuesSeen++ { |
Contributor
There was a problem hiding this comment.
a simple for range c.size should work here as well.
| } | ||
|
|
||
| if err = counter.CheckRetention(time.Second*time.Duration(scanLookBack64), 0); err != nil { | ||
| log.Tracef("cpuinfo counter cant hold the query range: %s", err.Error()) |
| } | ||
|
|
||
| if err = counter.CheckRetention(time.Second*time.Duration(scanLookBack64), 100); err != nil { | ||
| log.Warnf("cpuinfo counter cant hold the query range even when extended: %s", err.Error()) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The cpu utilization check will always return the default value, if it cannot get proper values from the cpuinfo counter.
It will complain to the logs better.
By default I set the acceptablePercentage to %50. If there is a check of 100s, and the values I get from the counter is currently < 50s apart, it will complain differently to the log. It will still return the calculated values for that <50s period though. If its above >50s it is deemed representative.