From d8f8aa6c975c3ae60fd474d549c7e4e971d5a63d Mon Sep 17 00:00:00 2001 From: Ahmet Ozturk Date: Fri, 26 Jun 2026 00:27:15 +0200 Subject: [PATCH 01/18] wip: selectively applicable conditions and metrics save a list of refrences to entries that a Condition object wont be evaulated against. using Condition.Match with these data will return inconclusive results save a reference to the listdata entry that a Metric object was generated from. if such a reference is present, and the Condition object has it in its skip list, it wont be used when building the perf threshold string add helper function that looks through a condition list, takes out Condition objects where specialized keywords are present, then filters to Condition objects that have a generallized keyword present. These generallized versions can have entries added to their evaluation skip list improve .String() on Condition objects, add more log points, add helper function for slice subtraction use check_drivesize as a testing ground for these changes. If ' used_pct' keyword is present, it will be taken as a specialized keyword, and generallized 'used_pct' containing Conditions will have the drive entry added to their blacklist --- pkg/snclient/check_drivesize.go | 51 +++++++++++++++++++++---------- pkg/snclient/checkdata.go | 28 ++++++++++++++--- pkg/snclient/checkmetric.go | 26 ++++++++++++++-- pkg/snclient/condition.go | 53 +++++++++++++++++++++++---------- pkg/utils/utils.go | 18 +++++++++++ 5 files changed, 139 insertions(+), 37 deletions(-) diff --git a/pkg/snclient/check_drivesize.go b/pkg/snclient/check_drivesize.go index 146fc3df..d0dc8d97 100644 --- a/pkg/snclient/check_drivesize.go +++ b/pkg/snclient/check_drivesize.go @@ -298,7 +298,26 @@ func (l *CheckDrivesize) isExcluded(drive map[string]string, excludes []string) return false } -func (l *CheckDrivesize) addMetrics(drive string, check *CheckData, usage *disk.UsageStat, magic float64) { +func (l *CheckDrivesize) handleDriveUsagePctThresholds(driveName string, check *CheckData, driveEntry *map[string]string) { + // convert ' used_pct' keywords in conditions to ' used %' as that matches the metric name + convertDriveUsagePctMetric1 := fmt.Sprintf("%s used_pct", driveName) + + // metrics are normally added if the operand is simply 'used' , 'used_pct' , 'used_bytes' etc. and do not have a drive prefix + // detect conditions where the operand is named ' used %', this is the default way snclient names percent usage metrics. + // if there is a condition using that as an operand, add usage metrics for that drive as well. during the metrics condition checking, they will take effect. + // this helps to check usage metrics specific to drives. + driveUsagePctMetric := fmt.Sprintf("%s used %%", driveName) + + check.warnThreshold = check.TransformMultipleKeywords([]string{convertDriveUsagePctMetric1}, driveUsagePctMetric, check.warnThreshold) + check.critThreshold = check.TransformMultipleKeywords([]string{convertDriveUsagePctMetric1}, driveUsagePctMetric, check.critThreshold) + check.okThreshold = check.TransformMultipleKeywords([]string{convertDriveUsagePctMetric1}, driveUsagePctMetric, check.okThreshold) + + check.warnThreshold.disableGenerallizedConditionsForEntry(check, driveEntry, []string{driveUsagePctMetric}, []string{"used", "used_pct"}) + check.critThreshold.disableGenerallizedConditionsForEntry(check, driveEntry, []string{driveUsagePctMetric}, []string{"used", "used_pct"}) + check.okThreshold.disableGenerallizedConditionsForEntry(check, driveEntry, []string{driveUsagePctMetric}, []string{"used", "used_pct"}) +} + +func (l *CheckDrivesize) addMetrics(drive *map[string]string, check *CheckData, usage *disk.UsageStat, magic float64) { total := usage.Total if !l.freespaceIgnoreReserved { total = usage.Used + usage.Free // use this total instead of usage.Total to account in the root reserved space @@ -307,34 +326,34 @@ func (l *CheckDrivesize) addMetrics(drive string, check *CheckData, usage *disk. if check.HasThreshold("free") || check.HasThreshold("free_pct") || check.HasThreshold("free_bytes") { check.warnThreshold = check.TransformMultipleKeywords([]string{"free_pct", "free_bytes"}, "free", check.warnThreshold) check.critThreshold = check.TransformMultipleKeywords([]string{"free_pct", "free_bytes"}, "free", check.critThreshold) - check.AddBytePercentMetrics("free", drive+" free", magic*float64(usage.Free), magic*float64(total)) + perfLabel := fmt.Sprintf("%s free", (*drive)["drive"]) + check.AddBytePercentMetrics("free", perfLabel, magic*float64(usage.Free), magic*float64(total)) } - // convert ' used_pct' keywords in conditions to ' used %' as that matches the metric name - convertDriveUsagePctMetric1 := fmt.Sprintf("%s used_pct", drive) - // metrics are normally added if the operand is simply 'used' , 'used_pct' , 'used_bytes' etc. and do not have a drive prefix - // detect conditions where the operand is named ' used %', this is the default way snclient names percent usage metrics. - // if there is a condition using that as an operand, add usage metrics for that drive as well. during the metrics condition checking, they will take effect. - // this helps to check usage metrics specific to drives. driveUsagePctMetric := fmt.Sprintf("%s used %%", drive) - check.warnThreshold = check.TransformMultipleKeywords([]string{convertDriveUsagePctMetric1}, driveUsagePctMetric, check.warnThreshold) - check.critThreshold = check.TransformMultipleKeywords([]string{convertDriveUsagePctMetric1}, driveUsagePctMetric, check.critThreshold) - if check.HasThreshold(driveUsagePctMetric) || check.HasThreshold("used") || check.HasThreshold("used_pct") || check.HasThreshold("used_bytes") { check.warnThreshold = check.TransformMultipleKeywords([]string{"used_pct", "used_bytes"}, "used", check.warnThreshold) check.critThreshold = check.TransformMultipleKeywords([]string{"used_pct", "used_bytes"}, "used", check.critThreshold) - check.AddBytePercentMetrics("used", drive+" used", magic*float64(usage.Used), magic*float64(total)) + perfLabel := fmt.Sprintf("%s used", (*drive)["drive"]) + check.AddBytePercentMetrics("used", perfLabel, magic*float64(usage.Used), magic*float64(total)) + for _, m := range check.result.Metrics { + if strings.HasPrefix(m.Name, perfLabel) { + m.Entry = drive + } + } } if check.HasThreshold("inodes") || check.HasThreshold("inodes_used") || check.HasThreshold("inodes_used_pct") { check.warnThreshold = check.TransformMultipleKeywords([]string{"inodes_used_pct", "inodes_used"}, "inodes", check.warnThreshold) check.critThreshold = check.TransformMultipleKeywords([]string{"inodes_used_pct", "inodes_used"}, "inodes", check.critThreshold) - check.AddPercentMetrics("inodes", drive+" inodes", float64(usage.InodesUsed), float64(usage.InodesTotal)) + perfLabel := fmt.Sprintf("%s inodes", (*drive)["drive"]) + check.AddPercentMetrics("inodes", perfLabel, float64(usage.InodesUsed), float64(usage.InodesTotal)) } if check.HasThreshold("inodes_free") || check.HasThreshold("inodes_free_pct") { check.warnThreshold = check.TransformMultipleKeywords([]string{"inodes_free_pct"}, "inodes_free", check.warnThreshold) check.critThreshold = check.TransformMultipleKeywords([]string{"inodes_free_pct"}, "inodes_free", check.critThreshold) - check.AddPercentMetrics("inodes_free", drive+" inodes free", float64(usage.InodesFree), float64(usage.InodesTotal)) + perfLabel := fmt.Sprintf("%s inodes free", (*drive)["drive"]) + check.AddPercentMetrics("inodes_free", perfLabel, float64(usage.InodesFree), float64(usage.InodesTotal)) } } @@ -452,7 +471,9 @@ func (l *CheckDrivesize) addDriveSizeDetails(check *CheckData, drive map[string] return } - l.addMetrics(drive["drive"], check, usage, magic) + l.handleDriveUsagePctThresholds(drive["drive"], check, &drive) + + l.addMetrics(&drive, check, usage, magic) } func (l *CheckDrivesize) getFlagNames(drive map[string]string) []string { diff --git a/pkg/snclient/checkdata.go b/pkg/snclient/checkdata.go index 2948ed35..22c6aba9 100644 --- a/pkg/snclient/checkdata.go +++ b/pkg/snclient/checkdata.go @@ -158,7 +158,9 @@ func (cd *CheckData) Finalize() (*CheckResult, error) { log.Debugf("condition critical: %s", cd.critThreshold.String()) log.Debugf("condition ok: %s", cd.okThreshold.String()) // Run thresholds once on cd.details. This is done separately than metrics or entries + // details are of type map[string]string, like entries in cd.listData, but there is only one per check // This can possibly set a value to cd.details[_state] + log.Tracef("checking warning, critical, and ok thresholds on check details") cd.Check(cd.details, cd.warnThreshold, cd.critThreshold, cd.okThreshold) log.Tracef("details:") logTraceASCIIMap(cd.details) @@ -210,6 +212,7 @@ func (cd *CheckData) finalizeOutput() (*CheckResult, error) { // each entry in the list data is individually checked // This may set "_state" of each entry + log.Tracef("checking warning, critical, and ok thresholds on a check entry") cd.Check(entry, cd.warnThreshold, cd.critThreshold, cd.okThreshold) } @@ -230,11 +233,12 @@ func (cd *CheckData) finalizeOutput() (*CheckResult, error) { } cd.result.ApplyPerfSyntax(cd.perfSyntax, cd.timezone) - // Run a separate check on the macros + log.Tracef("checking warning, critical, and ok thresholds on check macros") cd.Check(finalMacros, cd.warnThreshold, cd.critThreshold, cd.okThreshold) cd.setStateFromMaps(finalMacros) // Metrics are checked last, which also sets the final state + log.Tracef("checking warning, critical, and ok thresholds on check metrics") cd.CheckMetrics(cd.warnThreshold, cd.critThreshold, cd.okThreshold) switch { @@ -458,21 +462,21 @@ func (cd *CheckData) Check(data map[string]string, warnCond, critCond, okCond Co for i := range warnCond { if res, ok := warnCond[i].Match(data); res && ok { - log.Debugf("This data '%s' matched the WARNING Condition", warnCond[i].original) + log.Debugf("The given data matched the WARNING condition: '%s' ", warnCond[i].String()) data["_state"] = fmt.Sprintf("%d", CheckExitWarning) } } for i := range critCond { if res, ok := critCond[i].Match(data); res && ok { - log.Debugf("This data '%s' matched the CRITICAL Condition", critCond[i].original) + log.Debugf("This given data matched the CRITICAL condition: '%s' ", critCond[i].String()) data["_state"] = fmt.Sprintf("%d", CheckExitCritical) } } for i := range okCond { if res, ok := okCond[i].Match(data); res && ok { - log.Debugf("This data '%s' matched the OK Condition", okCond[i].original) + log.Debugf("This given data matched the OK condition: '%s' ", okCond[i].String()) data["_state"] = fmt.Sprintf("%d", CheckExitOK) } } @@ -1157,6 +1161,22 @@ func (cd *CheckData) hasThresholdCond(condList ConditionList, name string) bool return false } +func (cd *CheckData) filterThresholdConditionsUsingKeywords(condList ConditionList, keywords []string) []*Condition { + ret := []*Condition{} + for _, cond := range condList { + if len(cond.group) > 0 { + groupRet := cd.filterThresholdConditionsUsingKeywords(cond.group, keywords) + ret = append(ret, groupRet...) + } + + if slices.Contains(keywords, cond.keyword) { + ret = append(ret, cond) + } + } + + return ret +} + // hasThresholdCond returns true is the given list of conditions uses the given name at least once. func (cd *CheckData) getAllThresholdKeywords(condList ConditionList) []string { keywords := []string{} diff --git a/pkg/snclient/checkmetric.go b/pkg/snclient/checkmetric.go index 7046e2b7..e5abfef1 100644 --- a/pkg/snclient/checkmetric.go +++ b/pkg/snclient/checkmetric.go @@ -3,6 +3,7 @@ package snclient import ( "bytes" "fmt" + "slices" "strconv" "strings" @@ -23,7 +24,8 @@ type CheckMetric struct { CriticalStr *string // set critical from string Min *float64 Max *float64 - PerfConfig *PerfConfig // apply perf tweaks + PerfConfig *PerfConfig // apply perf tweaks + Entry *map[string]string // entry that this metric is generated from } func (m *CheckMetric) String() string { @@ -156,6 +158,7 @@ func (m *CheckMetric) tweakedNum(rawNum any) (num, unit string) { return convert.Num2String(rawNum), m.Unit } +// Generate a string to be used in perfdata about this threshold func (m *CheckMetric) ThresholdString(conditions ConditionList) string { conv := func(rawNum any) string { num, _ := m.tweakedNum(rawNum) @@ -163,9 +166,26 @@ func (m *CheckMetric) ThresholdString(conditions ConditionList) string { return num } + conditionsToUseWhenBuildingPerfString := ConditionList{} + + for _, cond := range conditions { + if m.Entry == nil { + conditionsToUseWhenBuildingPerfString = append(conditionsToUseWhenBuildingPerfString, cond) + } + + if slices.Contains(cond.skipEntries, m.Entry) { + log.Tracef("condition: %q , skipping to add to list before generating threshold perf string", cond) + continue + } + + conditionsToUseWhenBuildingPerfString = append(conditionsToUseWhenBuildingPerfString, cond) + } + + namesToUseWhenBuildingPerfString := []string{m.Name} + if m.ThresholdName != "" { - return ThresholdString([]string{m.Name, m.ThresholdName}, conditions, conv) + namesToUseWhenBuildingPerfString = append(namesToUseWhenBuildingPerfString, m.ThresholdName) } - return ThresholdString([]string{m.Name}, conditions, conv) + return ThresholdString(namesToUseWhenBuildingPerfString, conditionsToUseWhenBuildingPerfString, conv) } diff --git a/pkg/snclient/condition.go b/pkg/snclient/condition.go index ec23c8ad..16613df2 100644 --- a/pkg/snclient/condition.go +++ b/pkg/snclient/condition.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "math" + "reflect" "regexp" "slices" "strconv" @@ -42,6 +43,9 @@ type Condition struct { // back reference to check attributes (used to expand by unit) attr *[]CheckAttribute + + // back reference to check entries to skip + skipEntries [](*map[string]string) } // Operator defines a filter operator. @@ -228,15 +232,6 @@ func NewCondition(input string, attr *[]CheckAttribute) (*Condition, error) { } func (c *Condition) String() string { - if c.original != "" { - // keyword might have been changed by a transform function, print it out separately if that is the case - if strings.Contains(c.original, c.keyword) { - return c.original - } - - return fmt.Sprintf("(original: %s | keyword: %s)", c.original, c.keyword) - } - if len(c.group) > 0 { groups := []string{} for _, g := range c.group { @@ -246,7 +241,7 @@ func (c *Condition) String() string { return "(" + strings.Join(groups, " "+c.groupOperator.String()+" ") + ")" } - return fmt.Sprintf("%s %s %v%s", c.keyword, c.operator.String(), c.value, c.unit) + return fmt.Sprintf("Condition{kw: %q , op: %s , val: %v , un: %s , org: %s}", c.keyword, c.operator.String(), c.value, c.unit, c.original) } // Match checks if given map matches current condition @@ -255,6 +250,16 @@ func (c *Condition) Match(data map[string]string) (res, ok bool) { if c.isNone { return false, true } + + for _, skipEntry := range c.skipEntries { + // need to use reflect Pointer() to compare + // 'data' argument is passed by value + if reflect.ValueOf(data).Pointer() == reflect.ValueOf(*skipEntry).Pointer() { + log.Tracef("Condition: %q , skipping entry due to it being in skip list", c.String()) + return false, false + } + } + if len(c.group) > 0 { finalOK := true for i := range c.group { @@ -510,6 +515,7 @@ func (c *Condition) Clone() *Condition { group: make(ConditionList, 0), attr: c.attr, original: c.original, + skipEntries: slices.Clone(c.skipEntries), } for i := range c.group { @@ -1039,19 +1045,20 @@ func conditionFixTokenOperator(token []string) []string { } // ThresholdString returns string used in warn/crit threshold performance data. -func ThresholdString(name []string, conditions ConditionList, numberFormat func(any) string) string { +// The name should be contained within the condition +func ThresholdString(names []string, conditions ConditionList, numberFormat func(any) string) string { // fetch warning conditions for name of metric filtered := make(ConditionList, 0) var group GroupOperator for num := range conditions { cond := conditions[num] - if slices.Contains(name, cond.keyword) { + if slices.Contains(names, cond.keyword) { filtered = append(filtered, cond) } if cond.groupOperator == GroupOr { group = cond.groupOperator for i := range cond.group { - if slices.Contains(name, cond.group[i].keyword) { + if slices.Contains(names, cond.group[i].keyword) { filtered = append(filtered, cond.group[i]) } } @@ -1059,7 +1066,7 @@ func ThresholdString(name []string, conditions ConditionList, numberFormat func( if cond.groupOperator == GroupAnd { group = cond.groupOperator for i := range cond.group { - if slices.Contains(name, cond.group[i].keyword) { + if slices.Contains(names, cond.group[i].keyword) { filtered = append(filtered, cond.group[i]) } } @@ -1105,7 +1112,7 @@ func ThresholdString(name []string, conditions ConditionList, numberFormat func( return fmt.Sprintf("%s:%s", numberFormat(low), numberFormat(high)) } - // implicite And + // implicit And return fmt.Sprintf("@%s:%s", numberFormat(low), numberFormat(high)) } @@ -1170,3 +1177,19 @@ func replaceStrOp(input string) string { return strings.Join(output, "") } + +// A condition list can contain some conditions that use a specialized keyword, and generallized keywords. +// This function only does modifications if there are conditions using the specialized keyword +// For all others that do not use the specizalied keyword, check if they are using a generallized keyword. +// After these two rounds of filtering conditions, disable a entry from this condition. +func (cl *ConditionList) disableGenerallizedConditionsForEntry(cd *CheckData, entry *map[string]string, specializedKeywords, generallizedKeywords []string) { + conditionsWithSpecializedKeyword := cd.filterThresholdConditionsUsingKeywords(*cl, specializedKeywords) + if len(conditionsWithSpecializedKeyword) > 0 { + conditionsWithoutSpecializedKeyword := utils.SubtractSlice(*cl, conditionsWithSpecializedKeyword) + conditionsWithoutSpecializedKeywordAndGenerallizedKeyword := cd.filterThresholdConditionsUsingKeywords(conditionsWithoutSpecializedKeyword, generallizedKeywords) + for _, cond := range conditionsWithoutSpecializedKeywordAndGenerallizedKeyword { + cond.skipEntries = append(cond.skipEntries, entry) + log.Tracef("Condition: %q is marked to skip an entry", cond.String()) + } + } +} diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 78713a8e..c8f9f2e2 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -697,3 +697,21 @@ func ReplaceNumbersWithZeroPadded(s string, padding int) string { return fmt.Sprintf(format, num) }) } + +// generic function to sutract all elements of op1 from op2 +// this does not modify op1 or op2 +func SubtractSlice[T comparable](op1 []T, op2 []T) (ret []T) { + toRemove := make(map[T]struct{}, len(op2)) + + for _, elem := range op2 { + toRemove[elem] = struct{}{} + } + + op1Copy := make([]T, len(op1)) + copy(op1Copy, op1) + + return slices.DeleteFunc(op1Copy, func(elem T) bool { + _, exists := toRemove[elem] + return exists + }) +} From 45975e4b782be459ec1d1f2bccaea0310de95e76 Mon Sep 17 00:00:00 2001 From: Ahmet Oeztuerk Date: Fri, 26 Jun 2026 11:08:49 +0200 Subject: [PATCH 02/18] fix adding conditions twice if metric does not have an entry saved --- pkg/snclient/checkmetric.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/snclient/checkmetric.go b/pkg/snclient/checkmetric.go index e5abfef1..003991ef 100644 --- a/pkg/snclient/checkmetric.go +++ b/pkg/snclient/checkmetric.go @@ -158,7 +158,8 @@ func (m *CheckMetric) tweakedNum(rawNum any) (num, unit string) { return convert.Num2String(rawNum), m.Unit } -// Generate a string to be used in perfdata about this threshold +// Generate a string to be used in naemon like perfdata output about this threshold +// if a metric has a reference to its originating entry, the conditions will check if it is in their skip list func (m *CheckMetric) ThresholdString(conditions ConditionList) string { conv := func(rawNum any) string { num, _ := m.tweakedNum(rawNum) @@ -171,10 +172,13 @@ func (m *CheckMetric) ThresholdString(conditions ConditionList) string { for _, cond := range conditions { if m.Entry == nil { conditionsToUseWhenBuildingPerfString = append(conditionsToUseWhenBuildingPerfString, cond) + + continue } if slices.Contains(cond.skipEntries, m.Entry) { log.Tracef("condition: %q , skipping to add to list before generating threshold perf string", cond) + continue } From bda7c13dc3c37839e7a7f8525eabf1db2cf6cfc1 Mon Sep 17 00:00:00 2001 From: Ahmet Oeztuerk Date: Fri, 26 Jun 2026 11:10:23 +0200 Subject: [PATCH 03/18] fix specifying drive name when adding metrics --- pkg/snclient/check_drivesize.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/pkg/snclient/check_drivesize.go b/pkg/snclient/check_drivesize.go index d0dc8d97..314d7a51 100644 --- a/pkg/snclient/check_drivesize.go +++ b/pkg/snclient/check_drivesize.go @@ -299,13 +299,15 @@ func (l *CheckDrivesize) isExcluded(drive map[string]string, excludes []string) } func (l *CheckDrivesize) handleDriveUsagePctThresholds(driveName string, check *CheckData, driveEntry *map[string]string) { - // convert ' used_pct' keywords in conditions to ' used %' as that matches the metric name + // ' used_pct' is a drive specific usage keyword that can be used in Condition + // ' used %' is a metric name + // convert ' used_pct' keywords in conditions to ' used %' + // They can then be checked during metric checking convertDriveUsagePctMetric1 := fmt.Sprintf("%s used_pct", driveName) - // metrics are normally added if the operand is simply 'used' , 'used_pct' , 'used_bytes' etc. and do not have a drive prefix - // detect conditions where the operand is named ' used %', this is the default way snclient names percent usage metrics. - // if there is a condition using that as an operand, add usage metrics for that drive as well. during the metrics condition checking, they will take effect. - // this helps to check usage metrics specific to drives. + // metrics are normally added when Condition keywords are 'used' , 'used_pct' , 'used_bytes' etc. and do not have a drive prefix + // detect conditions where the operand is named ' used %' + // if there is a condition using ' used %' as an operand, add usage metrics for that drive as well. They do not need to use generic keywords like 'used' , 'used_pct' , 'used_bytes' driveUsagePctMetric := fmt.Sprintf("%s used %%", driveName) check.warnThreshold = check.TransformMultipleKeywords([]string{convertDriveUsagePctMetric1}, driveUsagePctMetric, check.warnThreshold) @@ -330,7 +332,7 @@ func (l *CheckDrivesize) addMetrics(drive *map[string]string, check *CheckData, check.AddBytePercentMetrics("free", perfLabel, magic*float64(usage.Free), magic*float64(total)) } - driveUsagePctMetric := fmt.Sprintf("%s used %%", drive) + driveUsagePctMetric := fmt.Sprintf("%s used %%", (*drive)["drive"]) if check.HasThreshold(driveUsagePctMetric) || check.HasThreshold("used") || check.HasThreshold("used_pct") || check.HasThreshold("used_bytes") { check.warnThreshold = check.TransformMultipleKeywords([]string{"used_pct", "used_bytes"}, "used", check.warnThreshold) From ed6f06d31a9fd6bf6a723d69fd26e166dd04f61f Mon Sep 17 00:00:00 2001 From: Ahmet Oeztuerk Date: Fri, 26 Jun 2026 11:50:56 +0200 Subject: [PATCH 04/18] ai pass: use map[string]string instead of *map[string]string when saving skip list for entry. this prevents golangci-lint from complaining this works because golang maps use an internal pointer to heap to save their key-value data. while maps are not directly comparable, using reflect.ValueOf(map).Pointer() to compare their pointers this way, we can save the skipList using map[string]string, and use the internal pointer comparsion to check if we need to skip. see utils.go for more details check_drivesize now passes the entry normally as map[string]string --- pkg/snclient/check_drivesize.go | 18 +++++++++--------- pkg/snclient/checkmetric.go | 7 +++---- pkg/snclient/condition.go | 10 ++++------ pkg/utils/utils.go | 28 ++++++++++++++++++++++++++-- 4 files changed, 42 insertions(+), 21 deletions(-) diff --git a/pkg/snclient/check_drivesize.go b/pkg/snclient/check_drivesize.go index 314d7a51..f5bc591b 100644 --- a/pkg/snclient/check_drivesize.go +++ b/pkg/snclient/check_drivesize.go @@ -298,7 +298,7 @@ func (l *CheckDrivesize) isExcluded(drive map[string]string, excludes []string) return false } -func (l *CheckDrivesize) handleDriveUsagePctThresholds(driveName string, check *CheckData, driveEntry *map[string]string) { +func (l *CheckDrivesize) handleDriveUsagePctThresholds(driveName string, check *CheckData, driveEntry map[string]string) { // ' used_pct' is a drive specific usage keyword that can be used in Condition // ' used %' is a metric name // convert ' used_pct' keywords in conditions to ' used %' @@ -319,7 +319,7 @@ func (l *CheckDrivesize) handleDriveUsagePctThresholds(driveName string, check * check.okThreshold.disableGenerallizedConditionsForEntry(check, driveEntry, []string{driveUsagePctMetric}, []string{"used", "used_pct"}) } -func (l *CheckDrivesize) addMetrics(drive *map[string]string, check *CheckData, usage *disk.UsageStat, magic float64) { +func (l *CheckDrivesize) addMetrics(drive map[string]string, check *CheckData, usage *disk.UsageStat, magic float64) { total := usage.Total if !l.freespaceIgnoreReserved { total = usage.Used + usage.Free // use this total instead of usage.Total to account in the root reserved space @@ -328,16 +328,16 @@ func (l *CheckDrivesize) addMetrics(drive *map[string]string, check *CheckData, if check.HasThreshold("free") || check.HasThreshold("free_pct") || check.HasThreshold("free_bytes") { check.warnThreshold = check.TransformMultipleKeywords([]string{"free_pct", "free_bytes"}, "free", check.warnThreshold) check.critThreshold = check.TransformMultipleKeywords([]string{"free_pct", "free_bytes"}, "free", check.critThreshold) - perfLabel := fmt.Sprintf("%s free", (*drive)["drive"]) + perfLabel := fmt.Sprintf("%s free", drive["drive"]) check.AddBytePercentMetrics("free", perfLabel, magic*float64(usage.Free), magic*float64(total)) } - driveUsagePctMetric := fmt.Sprintf("%s used %%", (*drive)["drive"]) + driveUsagePctMetric := fmt.Sprintf("%s used %%", drive["drive"]) if check.HasThreshold(driveUsagePctMetric) || check.HasThreshold("used") || check.HasThreshold("used_pct") || check.HasThreshold("used_bytes") { check.warnThreshold = check.TransformMultipleKeywords([]string{"used_pct", "used_bytes"}, "used", check.warnThreshold) check.critThreshold = check.TransformMultipleKeywords([]string{"used_pct", "used_bytes"}, "used", check.critThreshold) - perfLabel := fmt.Sprintf("%s used", (*drive)["drive"]) + perfLabel := fmt.Sprintf("%s used", drive["drive"]) check.AddBytePercentMetrics("used", perfLabel, magic*float64(usage.Used), magic*float64(total)) for _, m := range check.result.Metrics { if strings.HasPrefix(m.Name, perfLabel) { @@ -348,13 +348,13 @@ func (l *CheckDrivesize) addMetrics(drive *map[string]string, check *CheckData, if check.HasThreshold("inodes") || check.HasThreshold("inodes_used") || check.HasThreshold("inodes_used_pct") { check.warnThreshold = check.TransformMultipleKeywords([]string{"inodes_used_pct", "inodes_used"}, "inodes", check.warnThreshold) check.critThreshold = check.TransformMultipleKeywords([]string{"inodes_used_pct", "inodes_used"}, "inodes", check.critThreshold) - perfLabel := fmt.Sprintf("%s inodes", (*drive)["drive"]) + perfLabel := fmt.Sprintf("%s inodes", drive["drive"]) check.AddPercentMetrics("inodes", perfLabel, float64(usage.InodesUsed), float64(usage.InodesTotal)) } if check.HasThreshold("inodes_free") || check.HasThreshold("inodes_free_pct") { check.warnThreshold = check.TransformMultipleKeywords([]string{"inodes_free_pct"}, "inodes_free", check.warnThreshold) check.critThreshold = check.TransformMultipleKeywords([]string{"inodes_free_pct"}, "inodes_free", check.critThreshold) - perfLabel := fmt.Sprintf("%s inodes free", (*drive)["drive"]) + perfLabel := fmt.Sprintf("%s inodes free", drive["drive"]) check.AddPercentMetrics("inodes_free", perfLabel, float64(usage.InodesFree), float64(usage.InodesTotal)) } } @@ -473,9 +473,9 @@ func (l *CheckDrivesize) addDriveSizeDetails(check *CheckData, drive map[string] return } - l.handleDriveUsagePctThresholds(drive["drive"], check, &drive) + l.handleDriveUsagePctThresholds(drive["drive"], check, drive) - l.addMetrics(&drive, check, usage, magic) + l.addMetrics(drive, check, usage, magic) } func (l *CheckDrivesize) getFlagNames(drive map[string]string) []string { diff --git a/pkg/snclient/checkmetric.go b/pkg/snclient/checkmetric.go index 003991ef..dc9c08d5 100644 --- a/pkg/snclient/checkmetric.go +++ b/pkg/snclient/checkmetric.go @@ -3,7 +3,6 @@ package snclient import ( "bytes" "fmt" - "slices" "strconv" "strings" @@ -24,8 +23,8 @@ type CheckMetric struct { CriticalStr *string // set critical from string Min *float64 Max *float64 - PerfConfig *PerfConfig // apply perf tweaks - Entry *map[string]string // entry that this metric is generated from + PerfConfig *PerfConfig // apply perf tweaks + Entry map[string]string // entry that this metric is generated from } func (m *CheckMetric) String() string { @@ -176,7 +175,7 @@ func (m *CheckMetric) ThresholdString(conditions ConditionList) string { continue } - if slices.Contains(cond.skipEntries, m.Entry) { + if utils.ContainsMap(cond.skipEntries, m.Entry) { log.Tracef("condition: %q , skipping to add to list before generating threshold perf string", cond) continue diff --git a/pkg/snclient/condition.go b/pkg/snclient/condition.go index 16613df2..eb7fc016 100644 --- a/pkg/snclient/condition.go +++ b/pkg/snclient/condition.go @@ -4,7 +4,6 @@ import ( "errors" "fmt" "math" - "reflect" "regexp" "slices" "strconv" @@ -45,7 +44,7 @@ type Condition struct { attr *[]CheckAttribute // back reference to check entries to skip - skipEntries [](*map[string]string) + skipEntries []map[string]string } // Operator defines a filter operator. @@ -252,10 +251,9 @@ func (c *Condition) Match(data map[string]string) (res, ok bool) { } for _, skipEntry := range c.skipEntries { - // need to use reflect Pointer() to compare - // 'data' argument is passed by value - if reflect.ValueOf(data).Pointer() == reflect.ValueOf(*skipEntry).Pointer() { + if utils.MapsEqual(data, skipEntry) { log.Tracef("Condition: %q , skipping entry due to it being in skip list", c.String()) + return false, false } } @@ -1182,7 +1180,7 @@ func replaceStrOp(input string) string { // This function only does modifications if there are conditions using the specialized keyword // For all others that do not use the specizalied keyword, check if they are using a generallized keyword. // After these two rounds of filtering conditions, disable a entry from this condition. -func (cl *ConditionList) disableGenerallizedConditionsForEntry(cd *CheckData, entry *map[string]string, specializedKeywords, generallizedKeywords []string) { +func (cl *ConditionList) disableGenerallizedConditionsForEntry(cd *CheckData, entry map[string]string, specializedKeywords, generallizedKeywords []string) { conditionsWithSpecializedKeyword := cd.filterThresholdConditionsUsingKeywords(*cl, specializedKeywords) if len(conditionsWithSpecializedKeyword) > 0 { conditionsWithoutSpecializedKeyword := utils.SubtractSlice(*cl, conditionsWithSpecializedKeyword) diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index c8f9f2e2..48c95876 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -17,6 +17,7 @@ import ( "net/http" "os" "path/filepath" + "reflect" "regexp" "runtime" "slices" @@ -698,9 +699,9 @@ func ReplaceNumbersWithZeroPadded(s string, padding int) string { }) } -// generic function to sutract all elements of op1 from op2 +// generic function to subtract all elements of op2 from op1 // this does not modify op1 or op2 -func SubtractSlice[T comparable](op1 []T, op2 []T) (ret []T) { +func SubtractSlice[T comparable](op1, op2 []T) (ret []T) { toRemove := make(map[T]struct{}, len(op2)) for _, elem := range op2 { @@ -712,6 +713,29 @@ func SubtractSlice[T comparable](op1 []T, op2 []T) (ret []T) { return slices.DeleteFunc(op1Copy, func(elem T) bool { _, exists := toRemove[elem] + return exists }) } + +// MapsEqual returns true if two map[string]string values refer to the same underlying hash table. +// In Go, maps are reference types — copying a map copies the header (a pointer to the internal hmap struct), +// so all copies of the same map share the same underlying data and the same hmap address. +// Go does not move heap objects (no compacting GC), so the hmap pointer is stable for the map's lifetime. +// Maps are not directly == comparable, so reflect is used to extract and compare the internal pointer. +// This is useful for checking whether two maps represent the same logical entry (e.g. skip-list lookups). +func MapsEqual(a, b map[string]string) bool { + return reflect.ValueOf(a).Pointer() == reflect.ValueOf(b).Pointer() +} + +// ContainsMap returns true if a map in the slice shares the same underlying hash table as target. +// See MapsEqual for details on the comparison mechanism. +func ContainsMap(slice []map[string]string, target map[string]string) bool { + for _, m := range slice { + if MapsEqual(m, target) { + return true + } + } + + return false +} From ba020db5393cc3491552b5bbbfe7657e14926137 Mon Sep 17 00:00:00 2001 From: Ahmet Oeztuerk Date: Fri, 26 Jun 2026 11:53:19 +0200 Subject: [PATCH 05/18] golangi-lint fix: preallocate slice in Condition.String() --- pkg/snclient/condition.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/snclient/condition.go b/pkg/snclient/condition.go index eb7fc016..ce2dda9a 100644 --- a/pkg/snclient/condition.go +++ b/pkg/snclient/condition.go @@ -232,9 +232,9 @@ func NewCondition(input string, attr *[]CheckAttribute) (*Condition, error) { func (c *Condition) String() string { if len(c.group) > 0 { - groups := []string{} - for _, g := range c.group { - groups = append(groups, g.String()) + groups := make([]string, len(c.group)) + for i, g := range c.group { + groups[i] = g.String() } return "(" + strings.Join(groups, " "+c.groupOperator.String()+" ") + ")" From d5373713a001be482d9cd5e7a6629025f9bb401f Mon Sep 17 00:00:00 2001 From: Ahmet Oeztuerk Date: Fri, 26 Jun 2026 11:53:33 +0200 Subject: [PATCH 06/18] add comments about details, entry and metric checks --- pkg/snclient/checkdata.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pkg/snclient/checkdata.go b/pkg/snclient/checkdata.go index 22c6aba9..4eaa70b5 100644 --- a/pkg/snclient/checkdata.go +++ b/pkg/snclient/checkdata.go @@ -158,8 +158,9 @@ func (cd *CheckData) Finalize() (*CheckResult, error) { log.Debugf("condition critical: %s", cd.critThreshold.String()) log.Debugf("condition ok: %s", cd.okThreshold.String()) // Run thresholds once on cd.details. This is done separately than metrics or entries - // details are of type map[string]string, like entries in cd.listData, but there is only one per check - // This can possibly set a value to cd.details[_state] + // cd.details are of type map[string]string, + // same as elements of the slice cd.listData, but there is only one per check + // This can possibly set a value to cd.details[_state] , influencing check state log.Tracef("checking warning, critical, and ok thresholds on check details") cd.Check(cd.details, cd.warnThreshold, cd.critThreshold, cd.okThreshold) log.Tracef("details:") @@ -211,7 +212,7 @@ func (cd *CheckData) finalizeOutput() (*CheckResult, error) { } // each entry in the list data is individually checked - // This may set "_state" of each entry + // This can possibly set "_state" of each entry, influencing the final state log.Tracef("checking warning, critical, and ok thresholds on a check entry") cd.Check(entry, cd.warnThreshold, cd.critThreshold, cd.okThreshold) } @@ -1161,6 +1162,7 @@ func (cd *CheckData) hasThresholdCond(condList ConditionList, name string) bool return false } +// this function does not create new conditions, only filters to existing conditions of checkdata func (cd *CheckData) filterThresholdConditionsUsingKeywords(condList ConditionList, keywords []string) []*Condition { ret := []*Condition{} for _, cond := range condList { From 6741d09e71acf545e2effdf63c37727ebf9befc8 Mon Sep 17 00:00:00 2001 From: Ahmet Oeztuerk Date: Fri, 26 Jun 2026 14:18:38 +0200 Subject: [PATCH 07/18] write tests for the new utility and utility: subtractslice , mapsequal , containsmap condition: testing skiplist --- pkg/snclient/condition_test.go | 41 ++++++++++ pkg/utils/utils_test.go | 132 +++++++++++++++++++++++++++++++++ 2 files changed, 173 insertions(+) diff --git a/pkg/snclient/condition_test.go b/pkg/snclient/condition_test.go index 6c67c7a0..59df3221 100644 --- a/pkg/snclient/condition_test.go +++ b/pkg/snclient/condition_test.go @@ -329,3 +329,44 @@ func TestConditionStrOp(t *testing.T) { output = replaceStrOp(input) assert.Equal(t, input, output) } + +func TestConditionSkipList(t *testing.T) { + + cond, err := NewCondition("a > 5", nil) + require.NoErrorf(t, err, "ConditionParse should throw no error") + + data1 := map[string]string{ + "a": "10", + "b": "20", + "c": "30", + } + + data1copysamevalues := map[string]string{ + "a": "10", + "b": "20", + "c": "30", + } + + res, ok := cond.Match(data1) + + assert.True(t, res, "result of the check before adding to skip list should be true") + assert.True(t, ok, "result of the check before adding to skip list should be conclusive") + + // add it to entry skip list + cond.skipEntries = append(cond.skipEntries, data1) + + res, ok = cond.Match(data1) + + assert.False(t, res, "result of the check after adding to skip list should be false") + assert.False(t, ok, "result of the check after adding to skip list should be inconclusive") + + res, ok = cond.Match(data1copysamevalues) + + assert.True(t, res, "result of the check using copy of data1 should be true, skip list only has data1") + assert.True(t, ok, "result of the check using copy of data1 should be conclusive, skip list only has data1") + + res, ok = cond.Match(data1) + data1["d"] = "40" + assert.False(t, res, "result of the check after modifying data1 should still be false") + assert.False(t, ok, "result of the check after modifying data1 should still be inconclusive") +} diff --git a/pkg/utils/utils_test.go b/pkg/utils/utils_test.go index d26a0325..8783b500 100644 --- a/pkg/utils/utils_test.go +++ b/pkg/utils/utils_test.go @@ -193,3 +193,135 @@ func TestRankedSort(t *testing.T) { assert.Equalf(t, expected, sorted, "sorted by rank") } + +func TestSubtractSlice(t *testing.T) { + + numbersSlice1 := []int{1, 2, 3, 4, 5, 6, 7, 8, 9, 10} + numbersSlice2 := []int{1, 3, 5, 7, 9} + + numberSliceRes := SubtractSlice(numbersSlice1, numbersSlice2) + numberSlicesExpected := []int{2, 4, 6, 8, 10} + + assert.ElementsMatch(t, numberSliceRes, numberSlicesExpected, "removing odd numbers from [1..10] should give even numbers.") +} + +func TestMapsEqual(t *testing.T) { + + dataMaster := map[string]string{ + "x": "x", + "y": "y", + "z": "z", + } + + copyUsingAssignment := dataMaster + assert.True(t, MapsEqual(dataMaster, copyUsingAssignment), "map comparisons should be equal, assigned to new value") + + savedPtr := &dataMaster + assert.True(t, MapsEqual(dataMaster, *savedPtr), "map comparisons should be equal, reference assigned to new value") + + type st struct { + name string + data map[string]string + dataPtr *map[string]string + } + savedInStruct := st{ + name: "anan", + data: dataMaster, + dataPtr: &dataMaster, + } + assert.True(t, MapsEqual(dataMaster, savedInStruct.data), "map comparisons should be equal, assigned to new value in struct") + assert.True(t, MapsEqual(dataMaster, *savedInStruct.dataPtr), "map comparisons should be equal, referece assigned to new value in struct") + + savedInList := []map[string]string{dataMaster} + assert.True(t, MapsEqual(dataMaster, savedInList[0]), "map comparisons should be equal, saved to a list") + + savedInPtrList := [](*map[string]string){&dataMaster} + assert.True(t, MapsEqual(dataMaster, *(savedInPtrList[0])), "map comparisons should be equal, referece saved to a list") + + // modifying master, the references should stay the same + dataMaster["a"] = "a" + dataMaster["b"] = "b" + dataMaster["c"] = "c" + dataMaster["d"] = "d" + dataMaster["e"] = "e" + dataMaster["f"] = "f" + + // check again after master is modified + + assert.True(t, MapsEqual(dataMaster, copyUsingAssignment), "map comparisons should be equal, assigned to new value") + assert.Equal(t, "a", copyUsingAssignment["a"], "map variable should point to same table, assigned to new value") + + assert.True(t, MapsEqual(dataMaster, *savedPtr), "map comparisons should be equal, reference assigned to new value") + assert.Equal(t, "b", (*savedPtr)["b"], "map variable should point to same table, reference assigned to new value") + + assert.True(t, MapsEqual(dataMaster, savedInStruct.data), "map comparisons should be equal, assigned to new value in struct") + assert.Equal(t, "c", savedInStruct.data["c"], "map variable should point to same table, assigned to new value in struct") + + assert.True(t, MapsEqual(dataMaster, *savedInStruct.dataPtr), "map comparisons should be equal, referece assigned to new value in struct") + assert.Equal(t, "d", savedInStruct.data["d"], "map variable should point to same table, referece assigned to new value in struct") + + assert.True(t, MapsEqual(dataMaster, savedInList[0]), "map comparisons should be equal, saved to a list") + assert.Equal(t, "e", savedInStruct.data["e"], "map variable should point to same table, saved to a list") + + assert.True(t, MapsEqual(dataMaster, *(savedInPtrList[0])), "map comparisons should be equal, referece saved to a list") + assert.Equal(t, "f", savedInStruct.data["f"], "map variable should point to same table, referece saved to a list") + + dataMaster2 := map[string]string{} + assert.False(t, MapsEqual(dataMaster, dataMaster2), "map comparisons should be false, dataMaster2 is a new map") + + // fill up the newData to be same as dataMaster + dataMaster2["a"] = "a" + dataMaster2["b"] = "b" + dataMaster2["c"] = "c" + dataMaster2["d"] = "d" + dataMaster2["e"] = "e" + dataMaster2["f"] = "f" + dataMaster2["x"] = "x" + dataMaster2["y"] = "y" + dataMaster2["z"] = "z" + + assert.Equal(t, dataMaster, dataMaster2, "both dataMaster and dataMaster2 has the same key-values, in the same order") + assert.False(t, MapsEqual(dataMaster, dataMaster2), "map comparisons should be false, dataMaster2 has the same key-values but is a new table") + +} + +func TestContainsMap(t *testing.T) { + + map1 := map[string]string{ + "asd": "asd", + "xyz": "xyz", + } + + map1assigned := map1 + + map2 := map[string]string{ + "snclient": "snclient", + } + + map2referenceassined := &map2 + + map3 := map[string]string{ + "foo": "foo", + "bar": "bar", + } + + map3inlist := []map[string]string{map3} + + list := []map[string]string{map1, map2, map3} + + assert.True(t, ContainsMap(list, map1), "should contain first map") + assert.True(t, ContainsMap(list, map1assigned), "should contain first map assigned") + + assert.True(t, ContainsMap(list, map2), "should contain second map") + assert.True(t, ContainsMap(list, *map2referenceassined), "should contain second map refernece assigned") + + assert.True(t, ContainsMap(list, map3), "should contain third map") + assert.True(t, ContainsMap(list, map3inlist[0]), "should contain third map saved in list") + + map4 := map[string]string{ + "foo": " foo", + "bar": "bar", + } + + assert.False(t, ContainsMap(list, map4), "should not fourth map, it has same keys-values as map3 but is seperately initialized") +} From bd11ca4474766ef0dae91eed19c8afedf410976a Mon Sep 17 00:00:00 2001 From: Ahmet Oeztuerk Date: Fri, 26 Jun 2026 14:21:15 +0200 Subject: [PATCH 08/18] condition: revert Condition.String() it was being used in tests, and is intended to give a parseable string output make a new function called DetailString() that gives more debug-like output usable in trace logs --- pkg/snclient/checkdata.go | 6 +++--- pkg/snclient/condition.go | 29 ++++++++++++++++++++++++++--- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/pkg/snclient/checkdata.go b/pkg/snclient/checkdata.go index 4eaa70b5..a2dd1acf 100644 --- a/pkg/snclient/checkdata.go +++ b/pkg/snclient/checkdata.go @@ -463,21 +463,21 @@ func (cd *CheckData) Check(data map[string]string, warnCond, critCond, okCond Co for i := range warnCond { if res, ok := warnCond[i].Match(data); res && ok { - log.Debugf("The given data matched the WARNING condition: '%s' ", warnCond[i].String()) + log.Debugf("The given data matched the WARNING condition: '%s' ", warnCond[i].DetailedString()) data["_state"] = fmt.Sprintf("%d", CheckExitWarning) } } for i := range critCond { if res, ok := critCond[i].Match(data); res && ok { - log.Debugf("This given data matched the CRITICAL condition: '%s' ", critCond[i].String()) + log.Debugf("This given data matched the CRITICAL condition: '%s' ", critCond[i].DetailedString()) data["_state"] = fmt.Sprintf("%d", CheckExitCritical) } } for i := range okCond { if res, ok := okCond[i].Match(data); res && ok { - log.Debugf("This given data matched the OK condition: '%s' ", okCond[i].String()) + log.Debugf("This given data matched the OK condition: '%s' ", okCond[i].DetailedString()) data["_state"] = fmt.Sprintf("%d", CheckExitOK) } } diff --git a/pkg/snclient/condition.go b/pkg/snclient/condition.go index ce2dda9a..48935823 100644 --- a/pkg/snclient/condition.go +++ b/pkg/snclient/condition.go @@ -231,10 +231,33 @@ func NewCondition(input string, attr *[]CheckAttribute) (*Condition, error) { } func (c *Condition) String() string { + if c.original != "" { + // keyword might have been changed by a transform function, print it out separately if that is the case + if strings.Contains(c.original, c.keyword) { + return c.original + } + + return fmt.Sprintf("(original: %s | keyword: %s)", c.original, c.keyword) + } + + if len(c.group) > 0 { + groups := []string{} + for _, g := range c.group { + groups = append(groups, g.String()) + } + + return "(" + strings.Join(groups, " "+c.groupOperator.String()+" ") + ")" + } + + return fmt.Sprintf("%s %s %v%s", c.keyword, c.operator.String(), c.value, c.unit) +} + +// use this function to see more detail about a condition, including its original, unit and keyword +func (c *Condition) DetailedString() string { if len(c.group) > 0 { groups := make([]string, len(c.group)) for i, g := range c.group { - groups[i] = g.String() + groups[i] = g.DetailedString() } return "(" + strings.Join(groups, " "+c.groupOperator.String()+" ") + ")" @@ -252,7 +275,7 @@ func (c *Condition) Match(data map[string]string) (res, ok bool) { for _, skipEntry := range c.skipEntries { if utils.MapsEqual(data, skipEntry) { - log.Tracef("Condition: %q , skipping entry due to it being in skip list", c.String()) + log.Tracef("Condition: %q , skipping entry due to it being in skip list", c.DetailedString()) return false, false } @@ -1187,7 +1210,7 @@ func (cl *ConditionList) disableGenerallizedConditionsForEntry(cd *CheckData, en conditionsWithoutSpecializedKeywordAndGenerallizedKeyword := cd.filterThresholdConditionsUsingKeywords(conditionsWithoutSpecializedKeyword, generallizedKeywords) for _, cond := range conditionsWithoutSpecializedKeywordAndGenerallizedKeyword { cond.skipEntries = append(cond.skipEntries, entry) - log.Tracef("Condition: %q is marked to skip an entry", cond.String()) + log.Tracef("Condition: %q is marked to skip an entry", cond.DetailedString()) } } } From 551bc13397f85e4feaf115ef164cd8b7209f4e40 Mon Sep 17 00:00:00 2001 From: Ahmet Ozturk Date: Fri, 26 Jun 2026 14:50:26 +0200 Subject: [PATCH 09/18] use utils.ContainsMap in Condition.Match improve log messages around condition and metric entry skipping --- pkg/snclient/checkmetric.go | 2 +- pkg/snclient/condition.go | 10 ++++------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/pkg/snclient/checkmetric.go b/pkg/snclient/checkmetric.go index dc9c08d5..1cc36127 100644 --- a/pkg/snclient/checkmetric.go +++ b/pkg/snclient/checkmetric.go @@ -176,7 +176,7 @@ func (m *CheckMetric) ThresholdString(conditions ConditionList) string { } if utils.ContainsMap(cond.skipEntries, m.Entry) { - log.Tracef("condition: %q , skipping to add to list before generating threshold perf string", cond) + log.Tracef("metric knows which entry it was genereated from, its conditions skip list has that entry, skipping this condition in perf string, condition: %q , entry: %q", cond, m.Entry) continue } diff --git a/pkg/snclient/condition.go b/pkg/snclient/condition.go index 48935823..d1368f19 100644 --- a/pkg/snclient/condition.go +++ b/pkg/snclient/condition.go @@ -273,12 +273,10 @@ func (c *Condition) Match(data map[string]string) (res, ok bool) { return false, true } - for _, skipEntry := range c.skipEntries { - if utils.MapsEqual(data, skipEntry) { - log.Tracef("Condition: %q , skipping entry due to it being in skip list", c.DetailedString()) + if utils.ContainsMap(c.skipEntries, data) { + log.Tracef("Skipping to match condition against the data, as it was in conditions skip list, condition: %q , entry: %q", c.DetailedString(), data) - return false, false - } + return false, false } if len(c.group) > 0 { @@ -1210,7 +1208,7 @@ func (cl *ConditionList) disableGenerallizedConditionsForEntry(cd *CheckData, en conditionsWithoutSpecializedKeywordAndGenerallizedKeyword := cd.filterThresholdConditionsUsingKeywords(conditionsWithoutSpecializedKeyword, generallizedKeywords) for _, cond := range conditionsWithoutSpecializedKeywordAndGenerallizedKeyword { cond.skipEntries = append(cond.skipEntries, entry) - log.Tracef("Condition: %q is marked to skip an entry", cond.DetailedString()) + log.Tracef("Adding an entry to conditions skip list as it has a generellized keyword, condition: %q , entry: %q", cond.DetailedString(), entry) } } } From 5345ec55f3c5ba2fcccfa6dea2317da4eb5a0817 Mon Sep 17 00:00:00 2001 From: Ahmet Oeztuerk Date: Fri, 26 Jun 2026 14:51:37 +0200 Subject: [PATCH 10/18] spacing fixes for golangci-lint --- pkg/snclient/condition_test.go | 1 - pkg/utils/utils_test.go | 16 +++++----------- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/pkg/snclient/condition_test.go b/pkg/snclient/condition_test.go index 59df3221..75396c82 100644 --- a/pkg/snclient/condition_test.go +++ b/pkg/snclient/condition_test.go @@ -331,7 +331,6 @@ func TestConditionStrOp(t *testing.T) { } func TestConditionSkipList(t *testing.T) { - cond, err := NewCondition("a > 5", nil) require.NoErrorf(t, err, "ConditionParse should throw no error") diff --git a/pkg/utils/utils_test.go b/pkg/utils/utils_test.go index 8783b500..4bfaeb45 100644 --- a/pkg/utils/utils_test.go +++ b/pkg/utils/utils_test.go @@ -195,7 +195,6 @@ func TestRankedSort(t *testing.T) { } func TestSubtractSlice(t *testing.T) { - numbersSlice1 := []int{1, 2, 3, 4, 5, 6, 7, 8, 9, 10} numbersSlice2 := []int{1, 3, 5, 7, 9} @@ -206,7 +205,6 @@ func TestSubtractSlice(t *testing.T) { } func TestMapsEqual(t *testing.T) { - dataMaster := map[string]string{ "x": "x", "y": "y", @@ -220,12 +218,10 @@ func TestMapsEqual(t *testing.T) { assert.True(t, MapsEqual(dataMaster, *savedPtr), "map comparisons should be equal, reference assigned to new value") type st struct { - name string data map[string]string dataPtr *map[string]string } savedInStruct := st{ - name: "anan", data: dataMaster, dataPtr: &dataMaster, } @@ -235,8 +231,8 @@ func TestMapsEqual(t *testing.T) { savedInList := []map[string]string{dataMaster} assert.True(t, MapsEqual(dataMaster, savedInList[0]), "map comparisons should be equal, saved to a list") - savedInPtrList := [](*map[string]string){&dataMaster} - assert.True(t, MapsEqual(dataMaster, *(savedInPtrList[0])), "map comparisons should be equal, referece saved to a list") + savedInPtrList := []*map[string]string{&dataMaster} + assert.True(t, MapsEqual(dataMaster, *savedInPtrList[0]), "map comparisons should be equal, referece saved to a list") // modifying master, the references should stay the same dataMaster["a"] = "a" @@ -263,7 +259,7 @@ func TestMapsEqual(t *testing.T) { assert.True(t, MapsEqual(dataMaster, savedInList[0]), "map comparisons should be equal, saved to a list") assert.Equal(t, "e", savedInStruct.data["e"], "map variable should point to same table, saved to a list") - assert.True(t, MapsEqual(dataMaster, *(savedInPtrList[0])), "map comparisons should be equal, referece saved to a list") + assert.True(t, MapsEqual(dataMaster, *savedInPtrList[0]), "map comparisons should be equal, referece saved to a list") assert.Equal(t, "f", savedInStruct.data["f"], "map variable should point to same table, referece saved to a list") dataMaster2 := map[string]string{} @@ -282,11 +278,9 @@ func TestMapsEqual(t *testing.T) { assert.Equal(t, dataMaster, dataMaster2, "both dataMaster and dataMaster2 has the same key-values, in the same order") assert.False(t, MapsEqual(dataMaster, dataMaster2), "map comparisons should be false, dataMaster2 has the same key-values but is a new table") - } func TestContainsMap(t *testing.T) { - map1 := map[string]string{ "asd": "asd", "xyz": "xyz", @@ -313,7 +307,7 @@ func TestContainsMap(t *testing.T) { assert.True(t, ContainsMap(list, map1assigned), "should contain first map assigned") assert.True(t, ContainsMap(list, map2), "should contain second map") - assert.True(t, ContainsMap(list, *map2referenceassined), "should contain second map refernece assigned") + assert.True(t, ContainsMap(list, *map2referenceassined), "should contain second map reference assigned") assert.True(t, ContainsMap(list, map3), "should contain third map") assert.True(t, ContainsMap(list, map3inlist[0]), "should contain third map saved in list") @@ -323,5 +317,5 @@ func TestContainsMap(t *testing.T) { "bar": "bar", } - assert.False(t, ContainsMap(list, map4), "should not fourth map, it has same keys-values as map3 but is seperately initialized") + assert.False(t, ContainsMap(list, map4), "should not fourth map, it has same keys-values as map3 but is separately initialized") } From f0485d22d0b1c2d714627b3f012517baafc896fb Mon Sep 17 00:00:00 2001 From: Ahmet Oeztuerk Date: Fri, 26 Jun 2026 17:53:51 +0200 Subject: [PATCH 11/18] add a whitelist to the check - and change the skip list to blacklist use a combined whitelist/blacklist checker in condition.match() and to see if condition is applicable in perfdata string when adding metrics in check_drivesize, use a heuristic to check if a condition is a specialized condition, with drive keyword and matches an entry - if so add it if there are no specialized condition, present add every condition as it is generic --- pkg/snclient/check_drivesize.go | 73 ++++++++++++++++++++++++++++++--- pkg/snclient/checkmetric.go | 5 ++- pkg/snclient/condition.go | 61 +++++++++++++++++++++++---- pkg/snclient/condition_test.go | 18 ++++---- pkg/utils/utils.go | 14 +++++++ 5 files changed, 146 insertions(+), 25 deletions(-) diff --git a/pkg/snclient/check_drivesize.go b/pkg/snclient/check_drivesize.go index f5bc591b..072c03d3 100644 --- a/pkg/snclient/check_drivesize.go +++ b/pkg/snclient/check_drivesize.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "maps" + "slices" "sort" "strconv" "strings" @@ -330,6 +331,7 @@ func (l *CheckDrivesize) addMetrics(drive map[string]string, check *CheckData, u check.critThreshold = check.TransformMultipleKeywords([]string{"free_pct", "free_bytes"}, "free", check.critThreshold) perfLabel := fmt.Sprintf("%s free", drive["drive"]) check.AddBytePercentMetrics("free", perfLabel, magic*float64(usage.Free), magic*float64(total)) + l.filterDriveMetrics(check, drive, perfLabel) } driveUsagePctMetric := fmt.Sprintf("%s used %%", drive["drive"]) @@ -339,24 +341,85 @@ func (l *CheckDrivesize) addMetrics(drive map[string]string, check *CheckData, u check.critThreshold = check.TransformMultipleKeywords([]string{"used_pct", "used_bytes"}, "used", check.critThreshold) perfLabel := fmt.Sprintf("%s used", drive["drive"]) check.AddBytePercentMetrics("used", perfLabel, magic*float64(usage.Used), magic*float64(total)) - for _, m := range check.result.Metrics { - if strings.HasPrefix(m.Name, perfLabel) { - m.Entry = drive - } - } + l.filterDriveMetrics(check, drive, perfLabel) } if check.HasThreshold("inodes") || check.HasThreshold("inodes_used") || check.HasThreshold("inodes_used_pct") { check.warnThreshold = check.TransformMultipleKeywords([]string{"inodes_used_pct", "inodes_used"}, "inodes", check.warnThreshold) check.critThreshold = check.TransformMultipleKeywords([]string{"inodes_used_pct", "inodes_used"}, "inodes", check.critThreshold) perfLabel := fmt.Sprintf("%s inodes", drive["drive"]) check.AddPercentMetrics("inodes", perfLabel, float64(usage.InodesUsed), float64(usage.InodesTotal)) + l.filterDriveMetrics(check, drive, perfLabel) } if check.HasThreshold("inodes_free") || check.HasThreshold("inodes_free_pct") { check.warnThreshold = check.TransformMultipleKeywords([]string{"inodes_free_pct"}, "inodes_free", check.warnThreshold) check.critThreshold = check.TransformMultipleKeywords([]string{"inodes_free_pct"}, "inodes_free", check.critThreshold) perfLabel := fmt.Sprintf("%s inodes free", drive["drive"]) check.AddPercentMetrics("inodes_free", perfLabel, float64(usage.InodesFree), float64(usage.InodesTotal)) + l.filterDriveMetrics(check, drive, perfLabel) + } +} + +// filterDriveMetrics sets the Entry reference and filters Warning/Critical condition lists on metrics that belong to the given drive (matched by perfLabel prefix). +func (l *CheckDrivesize) filterDriveMetrics(check *CheckData, drive map[string]string, perfLabel string) { + for _, metric := range check.result.Metrics { + if !strings.HasPrefix(metric.Name, perfLabel) { + continue + } + metric.Entry = drive + metric.Warning = l.filterConditionsForDrive(check, metric.Warning, drive) + metric.Critical = l.filterConditionsForDrive(check, metric.Critical, drive) + } +} + +// filterConditionsForDrive returns a new ConditionList containing only the conditions that should apply to the given drive entry. +// If a condition has a "drive" keyword somewhere, and no other condition has a "drive" keyword matching the entry , it is added +// If a condition has a "drive" keyword somewhere, "drive" keyword and at least one drive sub-condition matches this entry , it is added +func (l *CheckDrivesize) filterConditionsForDrive(check *CheckData, conditions ConditionList, driveEntry map[string]string) ConditionList { + driveKeywordPresentAndMatchesEntry := false + for _, cond := range conditions { + keywords, _ := cond.GetListOfKeywords() + if !slices.Contains(keywords, "drive") { + continue + } + driveConds := check.filterThresholdConditionsUsingKeywords(ConditionList{cond}, []string{"drive"}) + for _, dc := range driveConds { + if match, ok := dc.Match(driveEntry); match && ok { + driveKeywordPresentAndMatchesEntry = true + + break + } + } + if driveKeywordPresentAndMatchesEntry { + break + } + } + + result := make(ConditionList, 0, len(conditions)) + for _, cond := range conditions { + keywords, _ := cond.GetListOfKeywords() + // drive keyword is not present + if !slices.Contains(keywords, "drive") { + if !driveKeywordPresentAndMatchesEntry { + // there is no specific condition matching this entry, which means + // add generic entry + result = append(result, cond.Clone()) + } + + continue + } + + // has "drive" keyword — include only if at least one drive sub-condition matches this entry + driveConds := check.filterThresholdConditionsUsingKeywords(ConditionList{cond}, []string{"drive"}) + for _, dc := range driveConds { + if match, ok := dc.Match(driveEntry); match && ok { + result = append(result, cond.Clone()) + + break + } + } } + + return result } func (l *CheckDrivesize) addTotal(check *CheckData) { diff --git a/pkg/snclient/checkmetric.go b/pkg/snclient/checkmetric.go index 1cc36127..53f16608 100644 --- a/pkg/snclient/checkmetric.go +++ b/pkg/snclient/checkmetric.go @@ -175,8 +175,9 @@ func (m *CheckMetric) ThresholdString(conditions ConditionList) string { continue } - if utils.ContainsMap(cond.skipEntries, m.Entry) { - log.Tracef("metric knows which entry it was genereated from, its conditions skip list has that entry, skipping this condition in perf string, condition: %q , entry: %q", cond, m.Entry) + if !cond.BlacklistWhitelistCheck(m.Entry) { + log.Tracef("metric knows which entry it was generated from, the condition does not allow this entry, skipping the condition for generating perf string, "+ + " condition: %q , entry: %q", cond.DetailedString(), m.Entry) continue } diff --git a/pkg/snclient/condition.go b/pkg/snclient/condition.go index d1368f19..8c905a02 100644 --- a/pkg/snclient/condition.go +++ b/pkg/snclient/condition.go @@ -43,8 +43,14 @@ type Condition struct { // back reference to check attributes (used to expand by unit) attr *[]CheckAttribute - // back reference to check entries to skip - skipEntries []map[string]string + // reference to data where this condition should NOT be evaluated + // can be any map[string]string, like check.details, or Entries in check.listData + blacklistData []map[string]string + + // reference to data where this condition should ONLY be evaluated + // can be any map[string]string, like check.details, or Entries in check.listData + // this works only if its populated + whitelistData []map[string]string } // Operator defines a filter operator. @@ -263,7 +269,27 @@ func (c *Condition) DetailedString() string { return "(" + strings.Join(groups, " "+c.groupOperator.String()+" ") + ")" } - return fmt.Sprintf("Condition{kw: %q , op: %s , val: %v , un: %s , org: %s}", c.keyword, c.operator.String(), c.value, c.unit, c.original) + return fmt.Sprintf("Condition{kw: %s , op: %s , val: %v , un: %s , org: %s}", c.keyword, c.operator.String(), c.value, c.unit, c.original) +} + +// checks if a data of type map[string]string is allowed through the whitelist and blacklist of the condition +// returns true if its allowed beyond blacklist/whitelist +func (c *Condition) BlacklistWhitelistCheck(data map[string]string) (allowed bool) { + if utils.ContainsMap(c.blacklistData, data) { + log.Tracef("Condition does not allow this data, it is in its black list, condition: %q , data: %q", c.DetailedString(), data) + + return false + } + + if len(c.whitelistData) > 0 { + if !utils.ContainsMap(c.whitelistData, data) { + log.Tracef("Condition does not allow this data, it is not in conditions populated white list, condition: %q , data: %q", c.DetailedString(), data) + + return false + } + } + + return true } // Match checks if given map matches current condition @@ -273,9 +299,7 @@ func (c *Condition) Match(data map[string]string) (res, ok bool) { return false, true } - if utils.ContainsMap(c.skipEntries, data) { - log.Tracef("Skipping to match condition against the data, as it was in conditions skip list, condition: %q , entry: %q", c.DetailedString(), data) - + if !c.BlacklistWhitelistCheck(data) { return false, false } @@ -534,7 +558,8 @@ func (c *Condition) Clone() *Condition { group: make(ConditionList, 0), attr: c.attr, original: c.original, - skipEntries: slices.Clone(c.skipEntries), + blacklistData: slices.Clone(c.blacklistData), + whitelistData: slices.Clone(c.whitelistData), } for i := range c.group { @@ -963,6 +988,24 @@ func (c *Condition) TransformMultipleKeywords(srcKeywords []string, targetKeywor return true } +// recursively gets list of all keywords used in the condition +func (c *Condition) GetListOfKeywords() (keywords []string, err error) { + addKeywordToList := func(c *Condition) (err error) { + keywords = append(keywords, c.keyword) + + return nil + } + + err = c.RunFuncRecursively(addKeywordToList) + if err != nil { + return nil, fmt.Errorf("error gathering keywords: %s", err.Error()) + } + + keywords = utils.Deduplicate(keywords) + + return keywords, nil +} + // pass an argument as a function. // the function should have a pointer receiver type, no arguments and return an error. // the function will be applied to the current instance. @@ -1207,8 +1250,8 @@ func (cl *ConditionList) disableGenerallizedConditionsForEntry(cd *CheckData, en conditionsWithoutSpecializedKeyword := utils.SubtractSlice(*cl, conditionsWithSpecializedKeyword) conditionsWithoutSpecializedKeywordAndGenerallizedKeyword := cd.filterThresholdConditionsUsingKeywords(conditionsWithoutSpecializedKeyword, generallizedKeywords) for _, cond := range conditionsWithoutSpecializedKeywordAndGenerallizedKeyword { - cond.skipEntries = append(cond.skipEntries, entry) - log.Tracef("Adding an entry to conditions skip list as it has a generellized keyword, condition: %q , entry: %q", cond.DetailedString(), entry) + cond.blacklistData = append(cond.blacklistData, entry) + log.Tracef("Adding an entry to conditions blacklist as it has a generellized keyword, condition: %q , entry: %q", cond.DetailedString(), entry) } } } diff --git a/pkg/snclient/condition_test.go b/pkg/snclient/condition_test.go index 75396c82..92662d16 100644 --- a/pkg/snclient/condition_test.go +++ b/pkg/snclient/condition_test.go @@ -330,7 +330,7 @@ func TestConditionStrOp(t *testing.T) { assert.Equal(t, input, output) } -func TestConditionSkipList(t *testing.T) { +func TestConditionBlacklist(t *testing.T) { cond, err := NewCondition("a > 5", nil) require.NoErrorf(t, err, "ConditionParse should throw no error") @@ -348,21 +348,21 @@ func TestConditionSkipList(t *testing.T) { res, ok := cond.Match(data1) - assert.True(t, res, "result of the check before adding to skip list should be true") - assert.True(t, ok, "result of the check before adding to skip list should be conclusive") + assert.True(t, res, "result of the check before adding to blacklist should be true") + assert.True(t, ok, "result of the check before adding to blacklist should be conclusive") - // add it to entry skip list - cond.skipEntries = append(cond.skipEntries, data1) + // add it to entry blacklist + cond.blacklistData = append(cond.blacklistData, data1) res, ok = cond.Match(data1) - assert.False(t, res, "result of the check after adding to skip list should be false") - assert.False(t, ok, "result of the check after adding to skip list should be inconclusive") + assert.False(t, res, "result of the check after adding to blacklist should be false") + assert.False(t, ok, "result of the check after adding to blacklist should be inconclusive") res, ok = cond.Match(data1copysamevalues) - assert.True(t, res, "result of the check using copy of data1 should be true, skip list only has data1") - assert.True(t, ok, "result of the check using copy of data1 should be conclusive, skip list only has data1") + assert.True(t, res, "result of the check using copy of data1 should be true, blacklist only has data1") + assert.True(t, ok, "result of the check using copy of data1 should be conclusive, blacklist only has data1") res, ok = cond.Match(data1) data1["d"] = "40" diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 48c95876..049b0574 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -739,3 +739,17 @@ func ContainsMap(slice []map[string]string, target map[string]string) bool { return false } + +// Deduplicate removes duplicate values from a slice, preserving order. +func Deduplicate[T comparable](slice []T) []T { + seen := make(map[T]struct{}, len(slice)) + result := make([]T, 0, len(slice)) + for _, v := range slice { + if _, exists := seen[v]; !exists { + seen[v] = struct{}{} + result = append(result, v) + } + } + + return result +} From 6b0af4bbe07374f33a13ecb3f69a1ba8620e6e38 Mon Sep 17 00:00:00 2001 From: Ahmet Oeztuerk Date: Fri, 26 Jun 2026 17:55:51 +0200 Subject: [PATCH 12/18] ai pass: write additional tests for blacklist and whitelist functionality --- pkg/snclient/condition_test.go | 195 ++++++++++++++++++++++++++++++++- 1 file changed, 194 insertions(+), 1 deletion(-) diff --git a/pkg/snclient/condition_test.go b/pkg/snclient/condition_test.go index 92662d16..2279ea85 100644 --- a/pkg/snclient/condition_test.go +++ b/pkg/snclient/condition_test.go @@ -365,7 +365,200 @@ func TestConditionBlacklist(t *testing.T) { assert.True(t, ok, "result of the check using copy of data1 should be conclusive, blacklist only has data1") res, ok = cond.Match(data1) + assert.False(t, res, "result after adding to blacklist should be false") + assert.False(t, ok, "result after adding to blacklist should be inconclusive") data1["d"] = "40" - assert.False(t, res, "result of the check after modifying data1 should still be false") + res, ok = cond.Match(data1) + assert.False(t, res, "result after modifying data1 should still be false") assert.False(t, ok, "result of the check after modifying data1 should still be inconclusive") } + +func TestConditionBlacklistMultipleEntries(t *testing.T) { + cond, err := NewCondition("a > 5", nil) + require.NoErrorf(t, err, "ConditionParse should throw no error") + + data1 := map[string]string{"a": "10"} + data2 := map[string]string{"a": "8"} + data3 := map[string]string{"a": "6"} + + cond.blacklistData = append(cond.blacklistData, data1, data2) + + res, ok := cond.Match(data1) + assert.False(t, res, "data1 is blacklisted") + assert.False(t, ok, "data1 inconclusive") + + res, ok = cond.Match(data2) + assert.False(t, res, "data2 is blacklisted") + assert.False(t, ok, "data2 inconclusive") + + res, ok = cond.Match(data3) + assert.True(t, res, "data3 is not blacklisted and matches the condition") + assert.True(t, ok, "data3 conclusive") +} + +func TestConditionBlacklistEmptyAllowsAll(t *testing.T) { + cond, err := NewCondition("a > 5", nil) + require.NoErrorf(t, err, "ConditionParse should throw no error") + + data1 := map[string]string{"a": "10"} + data2 := map[string]string{"a": "3"} + + // empty blacklist — all data passes through to normal matching + res, ok := cond.Match(data1) + assert.True(t, res, "data1 matches, blacklist empty") + assert.True(t, ok, "data1 conclusive") + + res, ok = cond.Match(data2) + assert.False(t, res, "data2 does not match condition, blacklist empty") + assert.True(t, ok, "data2 conclusive, only blocked by condition itself") +} + +func TestConditionBlacklistClone(t *testing.T) { + cond, err := NewCondition("a > 5", nil) + require.NoErrorf(t, err, "ConditionParse should throw no error") + + data1 := map[string]string{"a": "10"} + + cond.blacklistData = append(cond.blacklistData, data1) + + cloned := cond.Clone() + + res, ok := cloned.Match(data1) + assert.False(t, res, "cloned condition has blacklist data from original") + assert.False(t, ok, "inconclusive") + + // different underlying map with same values — not in blacklist + data2 := map[string]string{"a": "10"} + res, ok = cloned.Match(data2) + assert.True(t, res, "data2 has same values but different map, not in cloned blacklist") + assert.True(t, ok, "conclusive") +} + +func TestConditionWhitelist(t *testing.T) { + cond, err := NewCondition("a > 5", nil) + require.NoErrorf(t, err, "ConditionParse should throw no error") + + data1 := map[string]string{ + "a": "10", + "b": "20", + "c": "30", + } + + data2 := map[string]string{ + "a": "10", + "b": "20", + "c": "30", + } + + data3 := map[string]string{ + "a": "3", + } + + // before whitelist: matching data passes, non-matching fails + res, ok := cond.Match(data1) + assert.True(t, res, "data1 matches before whitelist is populated") + assert.True(t, ok, "data1 conclusive before whitelist is populated") + + res, ok = cond.Match(data3) + assert.False(t, res, "data3 does not match before whitelist is populated") + assert.True(t, ok, "data3 conclusive before whitelist is populated") + + // add data1 to whitelist — only data1 should be allowed through + cond.whitelistData = append(cond.whitelistData, data1) + + res, ok = cond.Match(data1) + assert.True(t, res, "data1 matches when it is in the whitelist") + assert.True(t, ok, "data1 conclusive when in the whitelist") + + // data2 has same values but different underlying map — whitelist uses map identity + res, ok = cond.Match(data2) + assert.False(t, res, "data2 does not match even with same values, not in whitelist") + assert.False(t, ok, "data2 inconclusive, not in whitelist") + + // data3: not in whitelist (even though it wouldn't match the condition anyway) + res, ok = cond.Match(data3) + assert.False(t, res, "data3 does not match, not in whitelist") + assert.False(t, ok, "data3 inconclusive, not in whitelist") + + // modifying whitelisted entry does not break identity + data1["d"] = "40" + res, ok = cond.Match(data1) + assert.True(t, res, "data1 still matches after modification, same underlying map") + assert.True(t, ok, "data1 still conclusive after modification") +} + +func TestConditionWhitelistOverridesBlacklist(t *testing.T) { + cond, err := NewCondition("a > 5", nil) + require.NoErrorf(t, err, "ConditionParse should throw no error") + + data1 := map[string]string{"a": "10"} + + // both blacklisted and whitelisted — blacklist takes precedence + cond.blacklistData = append(cond.blacklistData, data1) + cond.whitelistData = append(cond.whitelistData, data1) + + res, ok := cond.Match(data1) + assert.False(t, res, "blacklist takes precedence over whitelist") + assert.False(t, ok, "inconclusive due to blacklist") +} + +func TestConditionWhitelistMultipleEntries(t *testing.T) { + cond, err := NewCondition("a > 5", nil) + require.NoErrorf(t, err, "ConditionParse should throw no error") + + data1 := map[string]string{"a": "10"} + data2 := map[string]string{"a": "8"} + data3 := map[string]string{"a": "3"} + + cond.whitelistData = append(cond.whitelistData, data1, data2) + + res, ok := cond.Match(data1) + assert.True(t, res, "data1 is in whitelist and matches the condition") + assert.True(t, ok, "data1 conclusive") + + res, ok = cond.Match(data2) + assert.True(t, res, "data2 is in whitelist and matches the condition") + assert.True(t, ok, "data2 conclusive") + + res, ok = cond.Match(data3) + assert.False(t, res, "data3 is not in whitelist") + assert.False(t, ok, "data3 inconclusive, blocked by whitelist") +} + +func TestConditionWhitelistEmptyAllowsAll(t *testing.T) { + // empty whitelist means no whitelist restriction (all non-blacklisted data passes) + cond, err := NewCondition("a > 5", nil) + require.NoErrorf(t, err, "ConditionParse should throw no error") + + data1 := map[string]string{"a": "10"} + data2 := map[string]string{"a": "3"} + + res, ok := cond.Match(data1) + assert.True(t, res, "data1 matches, whitelist empty") + assert.True(t, ok, "data1 conclusive") + + res, ok = cond.Match(data2) + assert.False(t, res, "data2 does not match condition, whitelist empty") + assert.True(t, ok, "data2 conclusive, only blocked by condition itself") +} + +func TestConditionWhitelistClone(t *testing.T) { + cond, err := NewCondition("a > 5", nil) + require.NoErrorf(t, err, "ConditionParse should throw no error") + + data1 := map[string]string{"a": "10"} + + cond.whitelistData = append(cond.whitelistData, data1) + + cloned := cond.Clone() + + res, ok := cloned.Match(data1) + assert.True(t, res, "cloned condition has whitelist data from original") + assert.True(t, ok, "conclusive") + + // different underlying map with same values + data2 := map[string]string{"a": "10"} + res, ok = cloned.Match(data2) + assert.False(t, res, "data2 not in cloned whitelist") + assert.False(t, ok, "inconclusive") +} From 1d49d49c047efb66bab8c5640df2a6ffd394f560 Mon Sep 17 00:00:00 2001 From: Ahmet Oeztuerk Date: Fri, 26 Jun 2026 18:09:12 +0200 Subject: [PATCH 13/18] log filtered conditions for drives log metric name when using blacklist+whitelist to filter conditions for perfstring --- pkg/snclient/check_drivesize.go | 2 ++ pkg/snclient/checkmetric.go | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/snclient/check_drivesize.go b/pkg/snclient/check_drivesize.go index 072c03d3..8cc13421 100644 --- a/pkg/snclient/check_drivesize.go +++ b/pkg/snclient/check_drivesize.go @@ -419,6 +419,8 @@ func (l *CheckDrivesize) filterConditionsForDrive(check *CheckData, conditions C } } + log.Tracef("Filtered these conditions for drive entry: %q , result: %v ", driveEntry, result) + return result } diff --git a/pkg/snclient/checkmetric.go b/pkg/snclient/checkmetric.go index 53f16608..e6ad7de9 100644 --- a/pkg/snclient/checkmetric.go +++ b/pkg/snclient/checkmetric.go @@ -177,7 +177,7 @@ func (m *CheckMetric) ThresholdString(conditions ConditionList) string { if !cond.BlacklistWhitelistCheck(m.Entry) { log.Tracef("metric knows which entry it was generated from, the condition does not allow this entry, skipping the condition for generating perf string, "+ - " condition: %q , entry: %q", cond.DetailedString(), m.Entry) + " name: %q , condition: %q , entry: %q", m.Name, cond.DetailedString(), m.Entry) continue } From 17f9a964c4a6a384b503b7e5ba32ee5794843576 Mon Sep 17 00:00:00 2001 From: Ahmet Oeztuerk Date: Mon, 29 Jun 2026 14:53:31 +0200 Subject: [PATCH 14/18] move helper functions around to the proper receiver type, and make them more generallizable for further checks conditionlist now has four functions: disableGenerallizedConditionsForEntry filterConditionsUsingKeywords ifKeywordIsPresentAndPermitsEntry filterForSpecializedKeyword checkdata: addBytePercentMetrics and AddPercentMetrics now take an entry parameter, they add it while constructing a metric has three helper functions processMetricsWithSpecializedKeyword transformKeywordsUsingAttributes disableGenerallizedConditionsUsingAttributes Use these helper functions in check_drivesize.go: transformDrivePctMetrics disableGenerallizedConditionsForDrive go through all attributes of percent type, and check if they are specified specifically for a drive. transform them using this method. also specify the entries in other metrics, which enables specialized filtering using drive= and perflabel= for more attributes --- pkg/snclient/check_drivesize.go | 166 ++++++++++--------------- pkg/snclient/check_memory.go | 4 +- pkg/snclient/check_pagefile_windows.go | 12 +- pkg/snclient/checkdata.go | 77 +++++++++--- pkg/snclient/condition.go | 94 +++++++++++++- 5 files changed, 220 insertions(+), 133 deletions(-) diff --git a/pkg/snclient/check_drivesize.go b/pkg/snclient/check_drivesize.go index 8cc13421..31be73cb 100644 --- a/pkg/snclient/check_drivesize.go +++ b/pkg/snclient/check_drivesize.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "maps" - "slices" "sort" "strconv" "strings" @@ -299,25 +298,47 @@ func (l *CheckDrivesize) isExcluded(drive map[string]string, excludes []string) return false } -func (l *CheckDrivesize) handleDriveUsagePctThresholds(driveName string, check *CheckData, driveEntry map[string]string) { - // ' used_pct' is a drive specific usage keyword that can be used in Condition - // ' used %' is a metric name - // convert ' used_pct' keywords in conditions to ' used %' - // They can then be checked during metric checking - convertDriveUsagePctMetric1 := fmt.Sprintf("%s used_pct", driveName) - - // metrics are normally added when Condition keywords are 'used' , 'used_pct' , 'used_bytes' etc. and do not have a drive prefix - // detect conditions where the operand is named ' used %' - // if there is a condition using ' used %' as an operand, add usage metrics for that drive as well. They do not need to use generic keywords like 'used' , 'used_pct' , 'used_bytes' - driveUsagePctMetric := fmt.Sprintf("%s used %%", driveName) - - check.warnThreshold = check.TransformMultipleKeywords([]string{convertDriveUsagePctMetric1}, driveUsagePctMetric, check.warnThreshold) - check.critThreshold = check.TransformMultipleKeywords([]string{convertDriveUsagePctMetric1}, driveUsagePctMetric, check.critThreshold) - check.okThreshold = check.TransformMultipleKeywords([]string{convertDriveUsagePctMetric1}, driveUsagePctMetric, check.okThreshold) - - check.warnThreshold.disableGenerallizedConditionsForEntry(check, driveEntry, []string{driveUsagePctMetric}, []string{"used", "used_pct"}) - check.critThreshold.disableGenerallizedConditionsForEntry(check, driveEntry, []string{driveUsagePctMetric}, []string{"used", "used_pct"}) - check.okThreshold.disableGenerallizedConditionsForEntry(check, driveEntry, []string{driveUsagePctMetric}, []string{"used", "used_pct"}) +// The keyword after transformation matches the metric label added for that attribute +// So for single conditions without a group, these keywords will be translated +// They will then be processed during metric evaluation phase and possibly raise warning/critical/unknown. +func (l *CheckDrivesize) transformDrivePctMetrics(driveName string, check *CheckData) { + for _, attribute := range check.attributes { + if !strings.HasSuffix(attribute.name, "_pct") { + continue + } + + if attribute.unit != UPercent { + continue + } + + cut, _ := strings.CutSuffix(attribute.name, "_pct") + + // From: + // To: % + check.transformKeywordsUsingAttributes(`%[2]s %[1]s`, `%[2]s %[3]s %%`, []string{attribute.name}, driveName, cut) + } +} + +// If a condition akin to metric label is added, specialized for that drive +// Go through the conditions again, and disable generic versions of these +func (l *CheckDrivesize) disableGenerallizedConditionsForDrive(driveName string, entry map[string]string, check *CheckData) { + for _, attribute := range check.attributes { + if !strings.HasSuffix(attribute.name, "_pct") { + continue + } + + if attribute.unit != UPercent { + continue + } + + cut, _ := strings.CutSuffix(attribute.name, "_pct") + + // specialized: % generalized: + check.disableGenerallizedConditionsUsingAttributes(entry, `%[2]s %[3]s %%`, `%[1]s`, []string{attribute.name}, driveName, cut) + + // specialized: % generalized: % + check.disableGenerallizedConditionsUsingAttributes(entry, `%[2]s %[3]s %%`, `%[1]s %%`, []string{attribute.name}, driveName, cut) + } } func (l *CheckDrivesize) addMetrics(drive map[string]string, check *CheckData, usage *disk.UsageStat, magic float64) { @@ -326,104 +347,43 @@ func (l *CheckDrivesize) addMetrics(drive map[string]string, check *CheckData, u total = usage.Used + usage.Free // use this total instead of usage.Total to account in the root reserved space } - if check.HasThreshold("free") || check.HasThreshold("free_pct") || check.HasThreshold("free_bytes") { + driveFreeMetricLabel := fmt.Sprintf("%s free %%", drive["drive"]) + if check.HasThreshold(driveFreeMetricLabel) || check.HasThreshold("free") || check.HasThreshold("free_pct") || check.HasThreshold("free_bytes") { check.warnThreshold = check.TransformMultipleKeywords([]string{"free_pct", "free_bytes"}, "free", check.warnThreshold) check.critThreshold = check.TransformMultipleKeywords([]string{"free_pct", "free_bytes"}, "free", check.critThreshold) perfLabel := fmt.Sprintf("%s free", drive["drive"]) - check.AddBytePercentMetrics("free", perfLabel, magic*float64(usage.Free), magic*float64(total)) - l.filterDriveMetrics(check, drive, perfLabel) + check.AddBytePercentMetrics("free", perfLabel, magic*float64(usage.Free), magic*float64(total), drive) + check.processMetricsWithSpecializedKeyword("drive", perfLabel, drive) } - driveUsagePctMetric := fmt.Sprintf("%s used %%", drive["drive"]) - - if check.HasThreshold(driveUsagePctMetric) || check.HasThreshold("used") || check.HasThreshold("used_pct") || check.HasThreshold("used_bytes") { + driveUsagePctMetricLabel := fmt.Sprintf("%s used %%", drive["drive"]) + if check.HasThreshold(driveUsagePctMetricLabel) || check.HasThreshold("used") || check.HasThreshold("used_pct") || check.HasThreshold("used_bytes") { check.warnThreshold = check.TransformMultipleKeywords([]string{"used_pct", "used_bytes"}, "used", check.warnThreshold) check.critThreshold = check.TransformMultipleKeywords([]string{"used_pct", "used_bytes"}, "used", check.critThreshold) perfLabel := fmt.Sprintf("%s used", drive["drive"]) - check.AddBytePercentMetrics("used", perfLabel, magic*float64(usage.Used), magic*float64(total)) - l.filterDriveMetrics(check, drive, perfLabel) + check.AddBytePercentMetrics("used", perfLabel, magic*float64(usage.Used), magic*float64(total), drive) + check.processMetricsWithSpecializedKeyword("drive", perfLabel, drive) } - if check.HasThreshold("inodes") || check.HasThreshold("inodes_used") || check.HasThreshold("inodes_used_pct") { + + driveInodesPctMetricLabel := fmt.Sprintf("%s inodes_used %%", drive["drive"]) + if check.HasThreshold(driveInodesPctMetricLabel) || check.HasThreshold("inodes") || check.HasThreshold("inodes_used") || check.HasThreshold("inodes_used_pct") { check.warnThreshold = check.TransformMultipleKeywords([]string{"inodes_used_pct", "inodes_used"}, "inodes", check.warnThreshold) check.critThreshold = check.TransformMultipleKeywords([]string{"inodes_used_pct", "inodes_used"}, "inodes", check.critThreshold) perfLabel := fmt.Sprintf("%s inodes", drive["drive"]) - check.AddPercentMetrics("inodes", perfLabel, float64(usage.InodesUsed), float64(usage.InodesTotal)) - l.filterDriveMetrics(check, drive, perfLabel) + check.AddPercentMetrics("inodes", perfLabel, float64(usage.InodesUsed), float64(usage.InodesTotal), drive) + check.processMetricsWithSpecializedKeyword("drive", perfLabel, drive) } - if check.HasThreshold("inodes_free") || check.HasThreshold("inodes_free_pct") { + + driveInodesFreePctMetricLabel := fmt.Sprintf("%s inodes_free %%", drive["drive"]) + if check.HasThreshold(driveInodesFreePctMetricLabel) || check.HasThreshold("inodes_free") || check.HasThreshold("inodes_free_pct") { check.warnThreshold = check.TransformMultipleKeywords([]string{"inodes_free_pct"}, "inodes_free", check.warnThreshold) check.critThreshold = check.TransformMultipleKeywords([]string{"inodes_free_pct"}, "inodes_free", check.critThreshold) perfLabel := fmt.Sprintf("%s inodes free", drive["drive"]) - check.AddPercentMetrics("inodes_free", perfLabel, float64(usage.InodesFree), float64(usage.InodesTotal)) - l.filterDriveMetrics(check, drive, perfLabel) - } -} - -// filterDriveMetrics sets the Entry reference and filters Warning/Critical condition lists on metrics that belong to the given drive (matched by perfLabel prefix). -func (l *CheckDrivesize) filterDriveMetrics(check *CheckData, drive map[string]string, perfLabel string) { - for _, metric := range check.result.Metrics { - if !strings.HasPrefix(metric.Name, perfLabel) { - continue - } - metric.Entry = drive - metric.Warning = l.filterConditionsForDrive(check, metric.Warning, drive) - metric.Critical = l.filterConditionsForDrive(check, metric.Critical, drive) + check.AddPercentMetrics("inodes_free", perfLabel, float64(usage.InodesFree), float64(usage.InodesTotal), drive) + check.processMetricsWithSpecializedKeyword("drive", perfLabel, drive) } } -// filterConditionsForDrive returns a new ConditionList containing only the conditions that should apply to the given drive entry. -// If a condition has a "drive" keyword somewhere, and no other condition has a "drive" keyword matching the entry , it is added -// If a condition has a "drive" keyword somewhere, "drive" keyword and at least one drive sub-condition matches this entry , it is added -func (l *CheckDrivesize) filterConditionsForDrive(check *CheckData, conditions ConditionList, driveEntry map[string]string) ConditionList { - driveKeywordPresentAndMatchesEntry := false - for _, cond := range conditions { - keywords, _ := cond.GetListOfKeywords() - if !slices.Contains(keywords, "drive") { - continue - } - driveConds := check.filterThresholdConditionsUsingKeywords(ConditionList{cond}, []string{"drive"}) - for _, dc := range driveConds { - if match, ok := dc.Match(driveEntry); match && ok { - driveKeywordPresentAndMatchesEntry = true - - break - } - } - if driveKeywordPresentAndMatchesEntry { - break - } - } - - result := make(ConditionList, 0, len(conditions)) - for _, cond := range conditions { - keywords, _ := cond.GetListOfKeywords() - // drive keyword is not present - if !slices.Contains(keywords, "drive") { - if !driveKeywordPresentAndMatchesEntry { - // there is no specific condition matching this entry, which means - // add generic entry - result = append(result, cond.Clone()) - } - - continue - } - - // has "drive" keyword — include only if at least one drive sub-condition matches this entry - driveConds := check.filterThresholdConditionsUsingKeywords(ConditionList{cond}, []string{"drive"}) - for _, dc := range driveConds { - if match, ok := dc.Match(driveEntry); match && ok { - result = append(result, cond.Clone()) - - break - } - } - } - - log.Tracef("Filtered these conditions for drive entry: %q , result: %v ", driveEntry, result) - - return result -} - func (l *CheckDrivesize) addTotal(check *CheckData) { total := int64(0) free := int64(0) @@ -478,10 +438,10 @@ func (l *CheckDrivesize) addTotal(check *CheckData) { } if check.HasThreshold("free") || check.HasThreshold("free_bytes") { - check.AddBytePercentMetrics("free", drive["drive"]+" free", float64(free), float64(total)) + check.AddBytePercentMetrics("free", drive["drive"]+" free", float64(free), float64(total), map[string]string{}) } if check.HasThreshold("used") || check.HasThreshold("used_bytes") { - check.AddBytePercentMetrics("used", drive["drive"]+" used", float64(used), float64(total)) + check.AddBytePercentMetrics("used", drive["drive"]+" used", float64(used), float64(total), map[string]string{}) } } @@ -538,7 +498,9 @@ func (l *CheckDrivesize) addDriveSizeDetails(check *CheckData, drive map[string] return } - l.handleDriveUsagePctThresholds(drive["drive"], check, drive) + l.transformDrivePctMetrics(drive["drive"], check) + + l.disableGenerallizedConditionsForDrive(drive["drive"], drive, check) l.addMetrics(drive, check, usage, magic) } diff --git a/pkg/snclient/check_memory.go b/pkg/snclient/check_memory.go index 6a7a8353..e48ffc7e 100644 --- a/pkg/snclient/check_memory.go +++ b/pkg/snclient/check_memory.go @@ -168,11 +168,11 @@ func (l *CheckMemory) addMemType(check *CheckData, name string, used, free, tota if check.HasThreshold("free") || check.HasThreshold("free_pct") { check.warnThreshold = check.TransformMultipleKeywords([]string{"free_pct"}, "free", check.warnThreshold) check.critThreshold = check.TransformMultipleKeywords([]string{"free_pct"}, "free", check.critThreshold) - check.AddBytePercentMetrics("free", name+"_free", float64(free), float64(total)) + check.AddBytePercentMetrics("free", name+"_free", float64(free), float64(total), entry) } if check.HasThreshold("used") || check.HasThreshold("used_pct") { check.warnThreshold = check.TransformMultipleKeywords([]string{"used_pct"}, "used", check.warnThreshold) check.critThreshold = check.TransformMultipleKeywords([]string{"used_pct"}, "used", check.critThreshold) - check.AddBytePercentMetrics("used", name, float64(used), float64(total)) + check.AddBytePercentMetrics("used", name, float64(used), float64(total), entry) } } diff --git a/pkg/snclient/check_pagefile_windows.go b/pkg/snclient/check_pagefile_windows.go index 271d23b0..7b8cab2b 100644 --- a/pkg/snclient/check_pagefile_windows.go +++ b/pkg/snclient/check_pagefile_windows.go @@ -85,21 +85,21 @@ func (l *CheckPagefile) addPagefile(check *CheckData, name string, data map[stri check.listData = append(check.listData, entry) if check.HasThreshold("free") { - check.AddBytePercentMetrics("free", name, float64(data["AllocatedBaseSize"]-data["CurrentUsage"]), float64(data["AllocatedBaseSize"])) + check.AddBytePercentMetrics("free", name, float64(data["AllocatedBaseSize"]-data["CurrentUsage"]), float64(data["AllocatedBaseSize"]), entry) } if check.HasThreshold("used") { - check.AddBytePercentMetrics("used", name, float64(data["CurrentUsage"]), float64(data["AllocatedBaseSize"])) + check.AddBytePercentMetrics("used", name, float64(data["CurrentUsage"]), float64(data["AllocatedBaseSize"]), entry) } if check.HasThreshold("peak") { - check.AddBytePercentMetrics("peak", name, float64(data["PeakUsage"]), float64(data["AllocatedBaseSize"])) + check.AddBytePercentMetrics("peak", name, float64(data["PeakUsage"]), float64(data["AllocatedBaseSize"]), entry) } if check.HasThreshold("free_pct") { - check.AddPercentMetrics("free_pct", name, float64(data["AllocatedBaseSize"]-data["CurrentUsage"]), float64(data["AllocatedBaseSize"])) + check.AddPercentMetrics("free_pct", name, float64(data["AllocatedBaseSize"]-data["CurrentUsage"]), float64(data["AllocatedBaseSize"]), entry) } if check.HasThreshold("used_pct") { - check.AddPercentMetrics("used_pct", name, float64(data["AllocatedBaseSize"]-data["CurrentUsage"]), float64(data["AllocatedBaseSize"])) + check.AddPercentMetrics("used_pct", name, float64(data["AllocatedBaseSize"]-data["CurrentUsage"]), float64(data["AllocatedBaseSize"]), entry) } if check.HasThreshold("peak_pct") { - check.AddPercentMetrics("used_pct", name, float64(data["AllocatedBaseSize"]-data["PeakUsage"]), float64(data["AllocatedBaseSize"])) + check.AddPercentMetrics("used_pct", name, float64(data["AllocatedBaseSize"]-data["PeakUsage"]), float64(data["AllocatedBaseSize"]), entry) } } diff --git a/pkg/snclient/checkdata.go b/pkg/snclient/checkdata.go index a2dd1acf..a7a1ee05 100644 --- a/pkg/snclient/checkdata.go +++ b/pkg/snclient/checkdata.go @@ -1162,23 +1162,6 @@ func (cd *CheckData) hasThresholdCond(condList ConditionList, name string) bool return false } -// this function does not create new conditions, only filters to existing conditions of checkdata -func (cd *CheckData) filterThresholdConditionsUsingKeywords(condList ConditionList, keywords []string) []*Condition { - ret := []*Condition{} - for _, cond := range condList { - if len(cond.group) > 0 { - groupRet := cd.filterThresholdConditionsUsingKeywords(cond.group, keywords) - ret = append(ret, groupRet...) - } - - if slices.Contains(keywords, cond.keyword) { - ret = append(ret, cond) - } - } - - return ret -} - // hasThresholdCond returns true is the given list of conditions uses the given name at least once. func (cd *CheckData) getAllThresholdKeywords(condList ConditionList) []string { keywords := []string{} @@ -1315,7 +1298,7 @@ func (cd *CheckData) ExpandMetricMacros(srcThreshold ConditionList, data map[str return replaced } -func (cd *CheckData) AddBytePercentMetrics(threshold, perfLabel string, val, total float64) { +func (cd *CheckData) AddBytePercentMetrics(threshold, perfLabel string, val, total float64, entry map[string]string) { percent := float64(0) if threshold == "used" { percent = 100 @@ -1334,6 +1317,7 @@ func (cd *CheckData) AddBytePercentMetrics(threshold, perfLabel string, val, tot Critical: cd.TransformThreshold(cd.critThreshold, threshold, perfLabel, "%", "B", total), Min: &Zero, Max: &total, + Entry: entry, }, &CheckMetric{ Name: pctName, @@ -1343,11 +1327,12 @@ func (cd *CheckData) AddBytePercentMetrics(threshold, perfLabel string, val, tot Critical: cd.TransformThreshold(cd.critThreshold, threshold, pctName, "B", "%", total), Min: &Zero, Max: &Hundred, + Entry: entry, }, ) } -func (cd *CheckData) AddPercentMetrics(threshold, perfLabel string, val, total float64) { +func (cd *CheckData) AddPercentMetrics(threshold, perfLabel string, val, total float64, entry map[string]string) { percent := float64(0) if strings.Contains(threshold, "used") { percent = 100 @@ -1368,10 +1353,64 @@ func (cd *CheckData) AddPercentMetrics(threshold, perfLabel string, val, total f Critical: cd.critThreshold, Min: &Zero, Max: &Hundred, + Entry: entry, }, ) } +// processMetricsWithSpecializedKeyword sets the Entry reference and filters Warning/Critical thresholds of the Metric object +// the thresholds will be filtered using the special keyword. refer to that function for more details, as it uses a heurisic to determine how to filter +// +//nolint:unparam // this is only used in check_drivesize for now, so keyword is always "drive" +func (cd *CheckData) processMetricsWithSpecializedKeyword(keyword, metricName string, entry map[string]string) { + for _, metric := range cd.result.Metrics { + if !strings.HasPrefix(metric.Name, metricName) { + continue + } + metric.Entry = entry + metric.Warning = metric.Warning.filterForSpecializedKeyword(keyword, entry) + metric.Critical = metric.Critical.filterForSpecializedKeyword(keyword, entry) + } +} + +// transforms keywords of ok/warn/crit thresholds, using a source keyword and target keyword +// attribute.name of the CheckData.attributes is fed into the formats as the first argument +// formatArgs are fed as following arguments +func (cd *CheckData) transformKeywordsUsingAttributes(keywordSourceFormat, keywordTargetFormat string, attributeNames []string, formatArgs ...any) { + for _, attributeName := range attributeNames { + args := make([]any, len(formatArgs)+1) + args[0] = attributeName + copy(args[1:], formatArgs) + + keywordSource := fmt.Sprintf(keywordSourceFormat, args...) + keywordTarget := fmt.Sprintf(keywordTargetFormat, args...) + + log.Tracef("Transforming threshold keywords, soruceKeywords: %v , targetKeyword: %s", keywordSource, keywordTarget) + + cd.warnThreshold = cd.TransformMultipleKeywords([]string{keywordSource}, keywordTarget, cd.warnThreshold) + cd.critThreshold = cd.TransformMultipleKeywords([]string{keywordSource}, keywordTarget, cd.critThreshold) + cd.okThreshold = cd.TransformMultipleKeywords([]string{keywordSource}, keywordTarget, cd.okThreshold) + } +} + +// for a given entry, looks through the ok/warn/crit thresholds, disables conditions using generallized keywords if specialized keywords is present +// attribute.name of the CheckData.attributes is fed into the formats as the first argument +// formatArgs are fed as following arguments +func (cd *CheckData) disableGenerallizedConditionsUsingAttributes(entry map[string]string, specializedKeywordFormat, generallizedKeywordFormat string, attributeNames []string, formatArgs ...any) { + for _, attributeName := range attributeNames { + args := make([]any, len(formatArgs)+1) + args[0] = attributeName + copy(args[1:], formatArgs) + + specializedKeyword := fmt.Sprintf(specializedKeywordFormat, args) + generallizedKeyword := fmt.Sprintf(generallizedKeywordFormat, args) + + cd.warnThreshold.disableGenerallizedConditionsForEntry(entry, []string{specializedKeyword}, []string{generallizedKeyword}) + cd.critThreshold.disableGenerallizedConditionsForEntry(entry, []string{specializedKeyword}, []string{generallizedKeyword}) + cd.okThreshold.disableGenerallizedConditionsForEntry(entry, []string{specializedKeyword}, []string{generallizedKeyword}) + } +} + // expand arg definitions separated by pipe symbol // ex.: -w|--warning func (cd *CheckData) expandArgDefinitions() { diff --git a/pkg/snclient/condition.go b/pkg/snclient/condition.go index 8c905a02..26d14767 100644 --- a/pkg/snclient/condition.go +++ b/pkg/snclient/condition.go @@ -1244,14 +1244,100 @@ func replaceStrOp(input string) string { // This function only does modifications if there are conditions using the specialized keyword // For all others that do not use the specizalied keyword, check if they are using a generallized keyword. // After these two rounds of filtering conditions, disable a entry from this condition. -func (cl *ConditionList) disableGenerallizedConditionsForEntry(cd *CheckData, entry map[string]string, specializedKeywords, generallizedKeywords []string) { - conditionsWithSpecializedKeyword := cd.filterThresholdConditionsUsingKeywords(*cl, specializedKeywords) +func (cl *ConditionList) disableGenerallizedConditionsForEntry(entry map[string]string, specializedKeywords, generallizedKeywords []string) { + conditionsWithSpecializedKeyword := cl.filterConditionsUsingKeywords(specializedKeywords) if len(conditionsWithSpecializedKeyword) > 0 { conditionsWithoutSpecializedKeyword := utils.SubtractSlice(*cl, conditionsWithSpecializedKeyword) - conditionsWithoutSpecializedKeywordAndGenerallizedKeyword := cd.filterThresholdConditionsUsingKeywords(conditionsWithoutSpecializedKeyword, generallizedKeywords) + cl2 := ConditionList(conditionsWithoutSpecializedKeyword) + conditionsWithoutSpecializedKeywordAndGenerallizedKeyword := cl2.filterConditionsUsingKeywords(generallizedKeywords) for _, cond := range conditionsWithoutSpecializedKeywordAndGenerallizedKeyword { cond.blacklistData = append(cond.blacklistData, entry) - log.Tracef("Adding an entry to conditions blacklist as it has a generellized keyword, condition: %q , entry: %q", cond.DetailedString(), entry) + log.Tracef("Adding an entry to conditions blacklist, specialized keywords: %q , generallized keywords: %q , condition: %q , entry: %q", + specializedKeywords, generallizedKeywords, cond.DetailedString(), entry) } } } + +// this function does not create new conditions, only filters existing conditions of ConditionList +func (cl *ConditionList) filterConditionsUsingKeywords(keywords []string) (ret []*Condition) { + for _, condition := range *cl { + if len(condition.group) > 0 { + groupRet := condition.group.filterConditionsUsingKeywords(keywords) + ret = append(ret, groupRet...) + } + + if slices.Contains(keywords, condition.keyword) { + ret = append(ret, condition) + } + } + + return ret +} + +// The match does not have to return true, it can return false +// The important point is that it is conclusive. This means it is permitted to match against the entry +func (cl *ConditionList) ifKeywordIsPresentAndPermitsEntry(keyword string, entry map[string]string) (result bool) { + result = false + + for _, condition := range *cl { + keywords, _ := condition.GetListOfKeywords() + if !slices.Contains(keywords, keyword) { + continue + } + + cl := ConditionList{condition} + subconditionsWithKeyword := cl.filterConditionsUsingKeywords([]string{keyword}) + for _, subcondition := range subconditionsWithKeyword { + if match, ok := subcondition.Match(entry); match && ok { + result = true + + break + } + } + + if result { + break + } + } + + return result +} + +// If a specialized condition, i.e a condition using the keyword, exists in this condition list, filter to specialized condition +// If a specialized condition does not exist, every condition is generallized. Keep every generallized condition. +func (cl *ConditionList) filterForSpecializedKeyword(keyword string, entry map[string]string) (result ConditionList) { + // If the condition has this keyword, and permits an entry, the condition is specific to this entry + // It is specialized in terms of this keyword, it is not generallized + keywordIsPresentAndPermitsEntry := cl.ifKeywordIsPresentAndPermitsEntry(keyword, entry) + + for _, condition := range *cl { + keywords, _ := condition.GetListOfKeywords() + + // keyword is not present + if !slices.Contains(keywords, keyword) { + // add generic condition in terms of keyword + if !keywordIsPresentAndPermitsEntry { + result = append(result, condition.Clone()) + } + + continue + } + + // has keyword — include only if at least one drive sub-condition permits this entry + subconditionsUsingKeyword := cl.filterConditionsUsingKeywords([]string{keyword}) + for _, dc := range subconditionsUsingKeyword { + if match, ok := dc.Match(entry); match && ok { + result = append(result, condition.Clone()) + + break + } + } + } + + if len(result) > 0 { + log.Tracef("From this condition list: %v , filtering to this specialized keyword: %s and entry: %v , special keyword exists and permits entry: %t , result: %v ", + *cl, keyword, entry, keywordIsPresentAndPermitsEntry, result) + } + + return result +} From cfe9fa718817ca276786116678a6cb101f5e6587 Mon Sep 17 00:00:00 2001 From: Ahmet Oeztuerk Date: Mon, 29 Jun 2026 16:04:03 +0200 Subject: [PATCH 15/18] bug fix: formatArgs variadic argument was not expanded --- pkg/snclient/checkdata.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/snclient/checkdata.go b/pkg/snclient/checkdata.go index a7a1ee05..9ee8b0de 100644 --- a/pkg/snclient/checkdata.go +++ b/pkg/snclient/checkdata.go @@ -1402,8 +1402,8 @@ func (cd *CheckData) disableGenerallizedConditionsUsingAttributes(entry map[stri args[0] = attributeName copy(args[1:], formatArgs) - specializedKeyword := fmt.Sprintf(specializedKeywordFormat, args) - generallizedKeyword := fmt.Sprintf(generallizedKeywordFormat, args) + specializedKeyword := fmt.Sprintf(specializedKeywordFormat, args...) + generallizedKeyword := fmt.Sprintf(generallizedKeywordFormat, args...) cd.warnThreshold.disableGenerallizedConditionsForEntry(entry, []string{specializedKeyword}, []string{generallizedKeyword}) cd.critThreshold.disableGenerallizedConditionsForEntry(entry, []string{specializedKeyword}, []string{generallizedKeyword}) From 201f454c0334f5cd912c7c92414b37f8af68b1a0 Mon Sep 17 00:00:00 2001 From: Ahmet Oeztuerk Date: Mon, 29 Jun 2026 16:05:00 +0200 Subject: [PATCH 16/18] add another metric specialized to generallized case for percentage atributes --- pkg/snclient/check_drivesize.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/snclient/check_drivesize.go b/pkg/snclient/check_drivesize.go index 31be73cb..019e0473 100644 --- a/pkg/snclient/check_drivesize.go +++ b/pkg/snclient/check_drivesize.go @@ -338,6 +338,9 @@ func (l *CheckDrivesize) disableGenerallizedConditionsForDrive(driveName string, // specialized: % generalized: % check.disableGenerallizedConditionsUsingAttributes(entry, `%[2]s %[3]s %%`, `%[1]s %%`, []string{attribute.name}, driveName, cut) + + // specialized: % generalized: + check.disableGenerallizedConditionsUsingAttributes(entry, `%[2]s %[3]s %%`, `%[3]s`, []string{attribute.name}, driveName, cut) } } From a15e3128d07b5dd72108786d593e8ac234848a0e Mon Sep 17 00:00:00 2001 From: Ahmet Oeztuerk Date: Tue, 30 Jun 2026 14:58:24 +0200 Subject: [PATCH 17/18] fix addPercentMetrics function unlike AddBytePercentMetrics, it was not adding " %" to the name of the metric this caused problems when matching drive specific inodes_free and inodes_used conditions meant to match with perfdata labels --- pkg/snclient/check_drivesize.go | 14 +++++++------- pkg/snclient/checkdata.go | 3 ++- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/pkg/snclient/check_drivesize.go b/pkg/snclient/check_drivesize.go index 019e0473..ab0127cf 100644 --- a/pkg/snclient/check_drivesize.go +++ b/pkg/snclient/check_drivesize.go @@ -368,12 +368,12 @@ func (l *CheckDrivesize) addMetrics(drive map[string]string, check *CheckData, u check.processMetricsWithSpecializedKeyword("drive", perfLabel, drive) } - driveInodesPctMetricLabel := fmt.Sprintf("%s inodes_used %%", drive["drive"]) - if check.HasThreshold(driveInodesPctMetricLabel) || check.HasThreshold("inodes") || check.HasThreshold("inodes_used") || check.HasThreshold("inodes_used_pct") { - check.warnThreshold = check.TransformMultipleKeywords([]string{"inodes_used_pct", "inodes_used"}, "inodes", check.warnThreshold) - check.critThreshold = check.TransformMultipleKeywords([]string{"inodes_used_pct", "inodes_used"}, "inodes", check.critThreshold) - perfLabel := fmt.Sprintf("%s inodes", drive["drive"]) - check.AddPercentMetrics("inodes", perfLabel, float64(usage.InodesUsed), float64(usage.InodesTotal), drive) + driveInodesUsedPctMetricLabel := fmt.Sprintf("%s inodes_used %%", drive["drive"]) + if check.HasThreshold(driveInodesUsedPctMetricLabel) || check.HasThreshold("inodes") || check.HasThreshold("inodes_used") || check.HasThreshold("inodes_used_pct") { + check.warnThreshold = check.TransformMultipleKeywords([]string{"inodes_used_pct"}, "inodes_used", check.warnThreshold) + check.critThreshold = check.TransformMultipleKeywords([]string{"inodes_used_pct"}, "inodes_used", check.critThreshold) + perfLabel := fmt.Sprintf("%s inodes_used", drive["drive"]) + check.AddPercentMetrics("inodes_used", perfLabel, float64(usage.InodesUsed), float64(usage.InodesTotal), drive) check.processMetricsWithSpecializedKeyword("drive", perfLabel, drive) } @@ -381,7 +381,7 @@ func (l *CheckDrivesize) addMetrics(drive map[string]string, check *CheckData, u if check.HasThreshold(driveInodesFreePctMetricLabel) || check.HasThreshold("inodes_free") || check.HasThreshold("inodes_free_pct") { check.warnThreshold = check.TransformMultipleKeywords([]string{"inodes_free_pct"}, "inodes_free", check.warnThreshold) check.critThreshold = check.TransformMultipleKeywords([]string{"inodes_free_pct"}, "inodes_free", check.critThreshold) - perfLabel := fmt.Sprintf("%s inodes free", drive["drive"]) + perfLabel := fmt.Sprintf("%s inodes_free", drive["drive"]) check.AddPercentMetrics("inodes_free", perfLabel, float64(usage.InodesFree), float64(usage.InodesTotal), drive) check.processMetricsWithSpecializedKeyword("drive", perfLabel, drive) } diff --git a/pkg/snclient/checkdata.go b/pkg/snclient/checkdata.go index 9ee8b0de..ae50a287 100644 --- a/pkg/snclient/checkdata.go +++ b/pkg/snclient/checkdata.go @@ -1342,10 +1342,11 @@ func (cd *CheckData) AddPercentMetrics(threshold, perfLabel string, val, total f if total > 0 { percent = val * 100 / total } + pctName := perfLabel + " %" cd.result.Metrics = append( cd.result.Metrics, &CheckMetric{ - Name: perfLabel, + Name: pctName, ThresholdName: threshold, Unit: "%", Value: utils.ToPrecision(percent, 1), From 84850d756470a66ea371a2d08627cf5e1de2a814 Mon Sep 17 00:00:00 2001 From: Ahmet Oeztuerk Date: Tue, 30 Jun 2026 15:13:37 +0200 Subject: [PATCH 18/18] adjust failing test output to the new format of inodes_free --- pkg/snclient/check_drivesize_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/snclient/check_drivesize_test.go b/pkg/snclient/check_drivesize_test.go index 520291d9..9744fea8 100644 --- a/pkg/snclient/check_drivesize_test.go +++ b/pkg/snclient/check_drivesize_test.go @@ -49,7 +49,7 @@ func TestCheckDrivesize(t *testing.T) { res = snc.RunCheck("check_drivesize", []string{"warn=inodes>100%", "crit=inodes>100%", "folder=" + tmpFolder}) assert.Equalf(t, CheckExitOK, res.State, "state OK") assert.Contains(t, string(res.BuildPluginOutput()), `OK - All 1 drive`, "output matches") - assert.Contains(t, string(res.BuildPluginOutput()), `'`+tmpFolder+` inodes'=`, "output matches") + assert.Contains(t, string(res.BuildPluginOutput()), `'`+tmpFolder+` inodes_used %'=`, "output matches") StopTestAgent(t, snc) }