Metric conditions evaluation hotfix and check_drivesize tests#428
Closed
inqrphl wants to merge 9 commits into
Closed
Metric conditions evaluation hotfix and check_drivesize tests#428inqrphl wants to merge 9 commits into
inqrphl wants to merge 9 commits into
Conversation
it performed short-circuiting tests over the ConditionList, which leads to maximum of one correct resulting condition. the result thould then be len(corrects) > 0 , not > 1 which is always false
…h is fresher and canonical The metrics condition list may be modified during the check. When using the default check.warnCond and check.critCond , they might be stale.
…sed %" "inodes_free %" literally in thresholds check if these exist, then add metrics accordingly add tests that check for different keyword usage: '[drive] used %' '[drive] used_pct' 'used %' also add test for the specialzation, where having a "drive" keyword containing condition in the threshold, applies a filter for the metrics originating from that entry
…erly before adding to the metric
…ition, check if perfdata is specialized according to the conditions
… and darwin it uses both / and /tmp drives and those are not mounted as drives, or unavailable in those platforms
…data tests it is failing even on linux systems in github ci since /tmp is not a drive there
sni
reviewed
Jul 23, 2026
|
|
||
| // CheckMetrics tries warn/crit/ok conditions against given metrics and sets final state accordingly | ||
| func (cd *CheckData) CheckMetrics(warnCond, critCond, okCond ConditionList) { | ||
| func (cd *CheckData) CheckMetrics(_, _, okCond ConditionList) { |
Contributor
There was a problem hiding this comment.
this function is only used from one place, if 2 of 3 arguments are not used at all, they should be removed and the description should be adjusted. Otherwise it is misleading.
sni
reviewed
Jul 23, 2026
| minVal := utils.ToPrecision(sensor.Min, 3) | ||
| maxVal := utils.ToPrecision(sensor.High, 3) | ||
| var warning, critical ConditionList | ||
| if sensor.Min != 0 || sensor.Critical != 0 { |
Contributor
There was a problem hiding this comment.
how is the expansion of temp and temperature related to Min/Critical? I don't understand why this is necessary.
Contributor
Author
There was a problem hiding this comment.
The expansion has nothing to do with it, I am only adding the modified warning/critical thresholds if sensor.Min != 0 or sensor.Critical != 0
Some sensors report min==0 and then get added, only to fail directly in the check_temperature_test
Of course that problem only started happening after the CheckMetric.CheckForThresholds was fixed in the pr.
… CheckMetrics call metrics save their own warning and critical thresholds inside a ConditionList in the CheckMetric object. while saving these, the thresholds may be modified, including filtering and keyword transformations only the okCondition is supplied directly from the check itself.
Contributor
|
merged manually |
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
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.
metric: fix CheckMetric.CheckForThresholds
it performed short-circuiting tests over the ConditionList, which leads to maximum of one correct resulting condition.
the result thould then be len(corrects) > 0 , not > 1 which is always false
checks: CheckData.CheckMetrics uses the metric's condition list, which is fresher and canonical
The metrics condition list may be modified during the check.
When using the default check.warnCond and check.critCond , they might be stale.
check_drivesize: support keywords using "used %" , "free %" "inodes_used %" "inodes_free %" literally in thresholds
check if these exist, then add metrics accordingly
add tests that check for different keyword usage:
'[drive] used %'
'[drive] used_pct'
'used %'
also add test for the specialzation, where having a "drive" keyword containing condition in the threshold, applies a filter for the metrics originating from that entry