Skip to content

Commit ba0efe0

Browse files
vikin91vikin91
authored
Add generic histogram rule
* make sure that the output in TUI mode can be marked and copied * add a generic rule about histograms that alert if >50% of observations are in the +Inf bucket * add actionable steps for the users and developers (separate fields) * fix too many extra lines in the markdown output
1 parent ef17252 commit ba0efe0

17 files changed

Lines changed: 561 additions & 89 deletions

File tree

‎internal/evaluator/cache.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ func EvaluateCacheHit(rule rules.Rule, metrics parser.MetricsData, loadLevel rul
1313
result := rules.EvaluationResult{
1414
RuleName: rule.DisplayName,
1515
Status: rules.StatusGreen,
16-
Details: make(map[string]interface{}),
16+
Details: []string{},
1717
Timestamp: time.Now(),
1818
}
1919

@@ -80,12 +80,13 @@ func EvaluateCacheHit(rule rules.Rule, metrics parser.MetricsData, loadLevel rul
8080
"hits": hits,
8181
"misses": misses,
8282
})
83-
if result.Status == rules.StatusYellow {
83+
switch result.Status {
84+
case rules.StatusYellow:
8485
result.Message = interpolate(rule.Messages.Yellow, hitRate, map[string]interface{}{
8586
"hits": hits,
8687
"misses": misses,
8788
})
88-
} else if result.Status == rules.StatusRed {
89+
case rules.StatusRed:
8990
result.Message = interpolate(rule.Messages.Red, hitRate, map[string]interface{}{
9091
"hits": hits,
9192
"misses": misses,

‎internal/evaluator/composite.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ func EvaluateComposite(rule rules.Rule, metrics parser.MetricsData, loadLevel ru
1414
result := rules.EvaluationResult{
1515
RuleName: rule.DisplayName,
1616
Status: rules.StatusGreen,
17-
Details: make(map[string]interface{}),
17+
Details: []string{},
1818
Timestamp: time.Now(),
1919
}
2020

@@ -33,7 +33,7 @@ func EvaluateComposite(rule rules.Rule, metrics parser.MetricsData, loadLevel ru
3333
}
3434
value, _ := metric.GetSingleValue()
3535
metricValues[metricDef.Name] = value
36-
result.Details[metricDef.Name] = value
36+
result.Details = append(result.Details, fmt.Sprintf("%s: %.3f", metricDef.Name, value))
3737
}
3838

3939
// Evaluate checks in order

‎internal/evaluator/evaluator_common.go‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,9 @@ func EvaluateAllRules(rulesList []rules.Rule, metrics parser.MetricsData, loadLe
108108
result = EvaluateCorrelation(rule, metrics, result)
109109
}
110110

111-
// Add remediation
111+
// Add potential actions (user-facing)
112112
result.Remediation = getRemediation(rule, result.Status)
113+
result.PotentialActionUser = result.Remediation
113114
result.Timestamp = time.Now()
114115

115116
report.Results = append(report.Results, result)
@@ -125,6 +126,22 @@ func EvaluateAllRules(rulesList []rules.Rule, metrics parser.MetricsData, loadLe
125126
}
126127
}
127128

129+
// Apply general histogram +Inf overflow rule to all histogram metrics
130+
infOverflowResults := EvaluateHistogramInfOverflow(metrics, loadLevel)
131+
for _, result := range infOverflowResults {
132+
report.Results = append(report.Results, result)
133+
134+
// Update summary
135+
switch result.Status {
136+
case rules.StatusRed:
137+
report.Summary.RedCount++
138+
case rules.StatusYellow:
139+
report.Summary.YellowCount++
140+
case rules.StatusGreen:
141+
report.Summary.GreenCount++
142+
}
143+
}
144+
128145
report.Summary.TotalAnalyzed = len(report.Results)
129146

130147
return report

‎internal/evaluator/evaluator_test.go‎

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package evaluator
22

33
import (
4+
"strings"
45
"testing"
56

67
"github.com/stackrox/sensor-metrics-analyzer/internal/parser"
@@ -403,3 +404,140 @@ func TestIsRuleApplicable(t *testing.T) {
403404
})
404405
}
405406
}
407+
408+
func TestEvaluateHistogramInfOverflow(t *testing.T) {
409+
tests := map[string]struct {
410+
metrics parser.MetricsData
411+
wantStatus map[string]rules.Status // metric name -> expected status
412+
wantCount int // expected number of results
413+
}{
414+
"should return red status when >50% in +Inf": {
415+
metrics: parser.MetricsData{
416+
"test_histogram_bucket": &parser.Metric{
417+
Name: "test_histogram_bucket",
418+
Type: "histogram",
419+
Values: []parser.MetricValue{
420+
{Value: 10, Labels: map[string]string{"le": "0.1"}},
421+
{Value: 20, Labels: map[string]string{"le": "0.5"}},
422+
{Value: 30, Labels: map[string]string{"le": "1.0"}},
423+
{Value: 100, Labels: map[string]string{"le": "+Inf"}}, // 70 in +Inf (70%)
424+
},
425+
},
426+
},
427+
wantStatus: map[string]rules.Status{
428+
"test_histogram (+Inf overflow check)": rules.StatusRed,
429+
},
430+
wantCount: 1,
431+
},
432+
"should return yellow status when >25% but <=50% in +Inf": {
433+
metrics: parser.MetricsData{
434+
"test_histogram_bucket": &parser.Metric{
435+
Name: "test_histogram_bucket",
436+
Type: "histogram",
437+
Values: []parser.MetricValue{
438+
{Value: 10, Labels: map[string]string{"le": "0.1"}},
439+
{Value: 20, Labels: map[string]string{"le": "0.5"}},
440+
{Value: 30, Labels: map[string]string{"le": "1.0"}},
441+
{Value: 50, Labels: map[string]string{"le": "+Inf"}}, // 20 in +Inf (40%)
442+
},
443+
},
444+
},
445+
wantStatus: map[string]rules.Status{
446+
"test_histogram (+Inf overflow check)": rules.StatusYellow,
447+
},
448+
wantCount: 1,
449+
},
450+
"should return green status when <=25% in +Inf": {
451+
metrics: parser.MetricsData{
452+
"test_histogram_bucket": &parser.Metric{
453+
Name: "test_histogram_bucket",
454+
Type: "histogram",
455+
Values: []parser.MetricValue{
456+
{Value: 10, Labels: map[string]string{"le": "0.1"}},
457+
{Value: 20, Labels: map[string]string{"le": "0.5"}},
458+
{Value: 30, Labels: map[string]string{"le": "1.0"}},
459+
{Value: 35, Labels: map[string]string{"le": "+Inf"}}, // 5 in +Inf (14.3%)
460+
},
461+
},
462+
},
463+
wantStatus: map[string]rules.Status{
464+
"test_histogram (+Inf overflow check)": rules.StatusGreen,
465+
},
466+
wantCount: 1,
467+
},
468+
"should skip histogram without +Inf bucket": {
469+
metrics: parser.MetricsData{
470+
"test_histogram_bucket": &parser.Metric{
471+
Name: "test_histogram_bucket",
472+
Type: "histogram",
473+
Values: []parser.MetricValue{
474+
{Value: 10, Labels: map[string]string{"le": "0.1"}},
475+
{Value: 20, Labels: map[string]string{"le": "0.5"}},
476+
},
477+
},
478+
},
479+
wantStatus: map[string]rules.Status{},
480+
wantCount: 0,
481+
},
482+
"should handle multiple histograms": {
483+
metrics: parser.MetricsData{
484+
"hist1_bucket": &parser.Metric{
485+
Name: "hist1_bucket",
486+
Type: "histogram",
487+
Values: []parser.MetricValue{
488+
{Value: 10, Labels: map[string]string{"le": "1.0"}},
489+
{Value: 100, Labels: map[string]string{"le": "+Inf"}}, // 90 in +Inf (90%)
490+
},
491+
},
492+
"hist2_bucket": &parser.Metric{
493+
Name: "hist2_bucket",
494+
Type: "histogram",
495+
Values: []parser.MetricValue{
496+
{Value: 10, Labels: map[string]string{"le": "1.0"}},
497+
{Value: 15, Labels: map[string]string{"le": "+Inf"}}, // 5 in +Inf (33%)
498+
},
499+
},
500+
},
501+
wantStatus: map[string]rules.Status{
502+
"hist1 (+Inf overflow check)": rules.StatusRed,
503+
"hist2 (+Inf overflow check)": rules.StatusYellow,
504+
},
505+
wantCount: 2,
506+
},
507+
}
508+
509+
for name, tt := range tests {
510+
t.Run(name, func(t *testing.T) {
511+
results := EvaluateHistogramInfOverflow(tt.metrics, rules.LoadLevelMedium)
512+
513+
if len(results) != tt.wantCount {
514+
t.Errorf("EvaluateHistogramInfOverflow() returned %d results, want %d", len(results), tt.wantCount)
515+
}
516+
517+
for _, result := range results {
518+
wantStatus, exists := tt.wantStatus[result.RuleName]
519+
if !exists {
520+
t.Errorf("Unexpected result for metric %s", result.RuleName)
521+
continue
522+
}
523+
524+
if result.Status != wantStatus {
525+
t.Errorf("EvaluateHistogramInfOverflow() for %s = %v, want %v", result.RuleName, result.Status, wantStatus)
526+
}
527+
528+
// Verify message contains expected information
529+
if result.Status != rules.StatusGreen {
530+
if result.Message == "" {
531+
t.Error("Message should not be empty for non-green status")
532+
}
533+
if !strings.Contains(result.Message, "Highest non-infinity bucket") {
534+
t.Error("Message should contain 'Highest non-infinity bucket'")
535+
}
536+
if !strings.Contains(result.Message, "didn't expect") {
537+
t.Error("Message should contain explanation about designer expectations")
538+
}
539+
}
540+
}
541+
})
542+
}
543+
}

‎internal/evaluator/gauge.go‎

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ func EvaluateGauge(rule rules.Rule, metrics parser.MetricsData, loadLevel rules.
1515
result := rules.EvaluationResult{
1616
RuleName: rule.MetricName,
1717
Status: rules.StatusGreen,
18-
Details: make(map[string]interface{}),
18+
Details: []string{},
1919
Timestamp: time.Now(),
2020
}
2121

@@ -98,20 +98,18 @@ func interpolate(template string, value float64, extras map[string]interface{})
9898
})
9999

100100
// Replace other placeholders from extras map
101-
if extras != nil {
102-
for key, val := range extras {
103-
// Handle format specifiers
104-
keyRe := regexp.MustCompile(`\{` + regexp.QuoteMeta(key) + `(?::[^}]+)?\}`)
105-
result = keyRe.ReplaceAllStringFunc(result, func(match string) string {
106-
if strings.Contains(match, ":") {
107-
formatMatch := regexp.MustCompile(`:([^}]+)`)
108-
if fm := formatMatch.FindStringSubmatch(match); len(fm) == 2 {
109-
return fmt.Sprintf("%"+fm[1], val)
110-
}
101+
for key, val := range extras {
102+
// Handle format specifiers
103+
keyRe := regexp.MustCompile(`\{` + regexp.QuoteMeta(key) + `(?::[^}]+)?\}`)
104+
result = keyRe.ReplaceAllStringFunc(result, func(match string) string {
105+
if strings.Contains(match, ":") {
106+
formatMatch := regexp.MustCompile(`:([^}]+)`)
107+
if fm := formatMatch.FindStringSubmatch(match); len(fm) == 2 {
108+
return fmt.Sprintf("%"+fm[1], val)
111109
}
112-
return fmt.Sprintf("%v", val)
113-
})
114-
}
110+
}
111+
return fmt.Sprintf("%v", val)
112+
})
115113
}
116114

117115
return result

0 commit comments

Comments
 (0)