diff --git a/cmd/karma/alerts.go b/cmd/karma/alerts.go index 64bac4add..0de49c666 100644 --- a/cmd/karma/alerts.go +++ b/cmd/karma/alerts.go @@ -52,7 +52,7 @@ func countersToLabelStats(counters map[string]map[string]int) models.LabelNameSt nameStats.Values[i].Percent = int(math.Floor((float64(value.Hits) / float64(nameStats.Hits)) * 100.0)) totalPercent += nameStats.Values[i].Percent } - sort.Sort(nameStats.Values) + slices.SortFunc(nameStats.Values, models.CompareLabelValueStats) for totalPercent < 100 { for i := range nameStats.Values { nameStats.Values[i].Percent++ @@ -72,7 +72,7 @@ func countersToLabelStats(counters map[string]map[string]int) models.LabelNameSt data = append(data, nameStats) } - sort.Sort(data) + slices.SortFunc(data, models.CompareLabelNameStats) return data } @@ -272,7 +272,7 @@ func autoGridLabel(dedupedAlerts []models.AlertGroup) string { var alertsCount int labelToAlertCount := map[models.UniqueString]map[string]int{} for _, ag := range dedupedAlerts { - alertsCount += ag.Alerts.Len() + alertsCount += len(ag.Alerts) for _, alert := range ag.Alerts { for _, l := range alert.Labels { if _, ok := labelToAlertCount[l.Name]; !ok { @@ -375,7 +375,7 @@ func filterAlerts(dedupedAlerts []models.AlertGroup, fl []filters.FilterT) (filt agCopy.Alerts = append(agCopy.Alerts, alert) agCopy.StateCount[alert.State.Value()]++ } - if agCopy.Alerts.Len() > 0 { + if len(agCopy.Alerts) > 0 { filteredAlerts = append(filteredAlerts, agCopy) } } diff --git a/cmd/karma/views.go b/cmd/karma/views.go index 405bc9630..c907677bb 100644 --- a/cmd/karma/views.go +++ b/cmd/karma/views.go @@ -426,7 +426,7 @@ func alerts(w http.ResponseWriter, r *http.Request) { } } } - sort.Sort(ag.Alerts) + slices.SortFunc(ag.Alerts, models.CompareAlerts) ag.LatestStartsAt = ag.FindLatestStartsAt() ag.Hash = ag.ContentFingerprint() apiAG := models.APIAlertGroup{AlertGroup: *ag, TotalAlerts: len(ag.Alerts)} @@ -584,12 +584,13 @@ func autocomplete(w http.ResponseWriter, r *http.Request) { dedupedAutocomplete := alertmanager.DedupAutocomplete() + lowerTerm := strings.ToLower(term) for _, hint := range dedupedAutocomplete { - if strings.HasPrefix(strings.ToLower(hint.Value.Value()), strings.ToLower(term)) { + if strings.HasPrefix(strings.ToLower(hint.Value.Value()), lowerTerm) { acData = append(acData, hint.Value.Value()) } else { for _, token := range hint.Tokens { - if strings.HasPrefix(strings.ToLower(token.Value()), strings.ToLower(term)) { + if strings.HasPrefix(strings.ToLower(token.Value()), lowerTerm) { acData = append(acData, hint.Value.Value()) } } @@ -789,7 +790,7 @@ func alertList(w http.ResponseWriter, r *http.Request) { for k, v := range labels { alert = alert.Set(k, v) } - sort.Sort(alert) + slices.SortFunc(alert, models.CompareLabels) al.Alerts = append(al.Alerts, alert) } sortSliceOfLabels(al.Alerts, sortKeys, "alertname") diff --git a/internal/alertmanager/dedup.go b/internal/alertmanager/dedup.go index 52cf43eac..9d474597d 100644 --- a/internal/alertmanager/dedup.go +++ b/internal/alertmanager/dedup.go @@ -88,14 +88,14 @@ func DedupAlerts() []models.AlertGroup { continue } ag := agList[0] - ag.Labels = transform.StripLables( + ag.Labels = transform.StripLabels( config.Config.Labels.Keep, config.Config.Labels.Strip, config.Config.Labels.CompiledKeepRegex, config.Config.Labels.CompiledStripRegex, ag.Labels) ag.Alerts = make(models.AlertList, 0, len(alerts)) for _, alert := range alerts { // strip labels and annotations user doesn't want to see in the UI - alert.Labels = transform.StripLables( + alert.Labels = transform.StripLabels( config.Config.Labels.Keep, config.Config.Labels.Strip, config.Config.Labels.CompiledKeepRegex, config.Config.Labels.CompiledStripRegex, alert.Labels) diff --git a/internal/alertmanager/models.go b/internal/alertmanager/models.go index 5db86672a..6d1c7f705 100644 --- a/internal/alertmanager/models.go +++ b/internal/alertmanager/models.go @@ -5,6 +5,7 @@ import ( "maps" "net/http" "net/url" + "slices" "sort" "sync" "time" @@ -310,7 +311,7 @@ func (am *Alertmanager) pullAlerts(version string) error { autocompleteMap[hint.Value] = &hint } - sort.Sort(&alerts) + slices.SortFunc(alerts, models.CompareAlerts) ag.Alerts = alerts // Hash is a checksum of all alerts, used to tell when any alert in the group changed diff --git a/internal/alertmanager/models_test.go b/internal/alertmanager/models_test.go index 8b8583dba..922c2ae23 100644 --- a/internal/alertmanager/models_test.go +++ b/internal/alertmanager/models_test.go @@ -1,6 +1,7 @@ package alertmanager import ( + "errors" "fmt" "testing" "time" @@ -9,6 +10,7 @@ import ( "github.com/prymitive/karma/internal/config" "github.com/prymitive/karma/internal/mapper/v017/models" + internalModels "github.com/prymitive/karma/internal/models" "github.com/rs/zerolog" ) @@ -243,6 +245,141 @@ func TestAlertmanagerPullSilencesWithInvalidVersion(t *testing.T) { } } +func TestProbeVersionHTTPError(t *testing.T) { + // verifies that probeVersion returns empty string when the metrics endpoint is unreachable + zerolog.SetGlobalLevel(zerolog.PanicLevel) + defer zerolog.SetGlobalLevel(zerolog.ErrorLevel) + + uri := "http://probe-http-error.localhost" + httpmock.RegisterResponder("GET", uri+"/metrics", + httpmock.NewErrorResponder(errors.New("connection refused"))) + + am, err := NewAlertmanager("cluster", "probe-http-err", uri) + if err != nil { + t.Fatalf("NewAlertmanager failed: %s", err) + } + version := am.probeVersion() + if version != "" { + t.Errorf("probeVersion() returned %q, expected empty string on HTTP error", version) + } +} + +func TestProbeVersionInvalidMetrics(t *testing.T) { + // verifies that probeVersion returns empty string when metrics response contains no version info + zerolog.SetGlobalLevel(zerolog.PanicLevel) + defer zerolog.SetGlobalLevel(zerolog.ErrorLevel) + + uri := "http://probe-invalid-metrics.localhost" + httpmock.RegisterResponder("GET", uri+"/metrics", + httpmock.NewStringResponder(200, "some_random_metric 1\n")) + + am, err := NewAlertmanager("cluster", "probe-invalid", uri) + if err != nil { + t.Fatalf("NewAlertmanager failed: %s", err) + } + version := am.probeVersion() + if version != "" { + t.Errorf("probeVersion() returned %q, expected empty string on invalid metrics", version) + } +} + +func TestIsHealthCheckAlertAllMatch(t *testing.T) { + // verifies that an alert matching all healthcheck filters is identified as a healthcheck + am, err := NewAlertmanager("cluster", "hc-test", "http://localhost", + WithHealthchecks(map[string][]string{ + "prom1": {"alertname=Watchdog"}, + }), + ) + if err != nil { + t.Fatalf("NewAlertmanager failed: %s", err) + } + + alert := &internalModels.Alert{ + Labels: internalModels.Labels{ + {Name: internalModels.NewUniqueString("alertname"), Value: internalModels.NewUniqueString("Watchdog")}, + }, + } + name, hc := am.IsHealthCheckAlert(alert) + if name != "prom1" { + t.Errorf("IsHealthCheckAlert() returned name=%q, expected %q", name, "prom1") + } + if hc == nil { + t.Error("IsHealthCheckAlert() returned nil HealthCheck for matching alert") + } +} + +func TestIsHealthCheckAlertPartialMatch(t *testing.T) { + // verifies that an alert matching only some healthcheck filters is NOT identified as a healthcheck + am, err := NewAlertmanager("cluster", "hc-test-partial", "http://localhost", + WithHealthchecks(map[string][]string{ + "prom1": {"alertname=Watchdog", "severity=critical"}, + }), + ) + if err != nil { + t.Fatalf("NewAlertmanager failed: %s", err) + } + + alert := &internalModels.Alert{ + Labels: internalModels.Labels{ + {Name: internalModels.NewUniqueString("alertname"), Value: internalModels.NewUniqueString("Watchdog")}, + {Name: internalModels.NewUniqueString("severity"), Value: internalModels.NewUniqueString("warning")}, + }, + } + name, hc := am.IsHealthCheckAlert(alert) + if name != "" { + t.Errorf("IsHealthCheckAlert() returned name=%q, expected empty string for partial match", name) + } + if hc != nil { + t.Error("IsHealthCheckAlert() returned non-nil HealthCheck for partial match") + } +} + +func TestIsHealthCheckAlertNoMatch(t *testing.T) { + // verifies that an alert not matching any healthcheck filters returns empty result + am, err := NewAlertmanager("cluster", "hc-test-none", "http://localhost", + WithHealthchecks(map[string][]string{ + "prom1": {"alertname=Watchdog"}, + }), + ) + if err != nil { + t.Fatalf("NewAlertmanager failed: %s", err) + } + + alert := &internalModels.Alert{ + Labels: internalModels.Labels{ + {Name: internalModels.NewUniqueString("alertname"), Value: internalModels.NewUniqueString("DiskFull")}, + }, + } + name, hc := am.IsHealthCheckAlert(alert) + if name != "" { + t.Errorf("IsHealthCheckAlert() returned name=%q, expected empty string for non-matching alert", name) + } + if hc != nil { + t.Error("IsHealthCheckAlert() returned non-nil HealthCheck for non-matching alert") + } +} + +func TestIsHealthCheckAlertNoHealthchecks(t *testing.T) { + // verifies that an alertmanager with no healthchecks returns empty result + am, err := NewAlertmanager("cluster", "hc-test-empty", "http://localhost") + if err != nil { + t.Fatalf("NewAlertmanager failed: %s", err) + } + + alert := &internalModels.Alert{ + Labels: internalModels.Labels{ + {Name: internalModels.NewUniqueString("alertname"), Value: internalModels.NewUniqueString("Watchdog")}, + }, + } + name, hc := am.IsHealthCheckAlert(alert) + if name != "" { + t.Errorf("IsHealthCheckAlert() returned name=%q, expected empty string", name) + } + if hc != nil { + t.Error("IsHealthCheckAlert() returned non-nil HealthCheck") + } +} + func TestExpiredSilences(t *testing.T) { config.Config.Silences.Expired = time.Minute * 10 diff --git a/internal/alertmanager/upstream_test.go b/internal/alertmanager/upstream_test.go index 718634737..ec6a07c75 100644 --- a/internal/alertmanager/upstream_test.go +++ b/internal/alertmanager/upstream_test.go @@ -114,6 +114,149 @@ var testCases = []testCase{ }, } +func saveUpstreams() map[string]*Alertmanager { + saved := make(map[string]*Alertmanager, len(upstreams)) + for k, v := range upstreams { + saved[k] = v + } + return saved +} + +func restoreUpstreams(saved map[string]*Alertmanager) { + upstreams = saved +} + +func TestUnregisterAll(t *testing.T) { + // verifies that UnregisterAll removes all registered instances + saved := saveUpstreams() + defer restoreUpstreams(saved) + + am, err := NewAlertmanager("cluster", "unreg-test", "http://localhost") + if err != nil { + t.Fatalf("NewAlertmanager failed: %s", err) + } + _ = RegisterAlertmanager(am) + UnregisterAll() + ams := GetAlertmanagers() + if len(ams) != 0 { + t.Errorf("Expected 0 alertmanagers after UnregisterAll, got %d", len(ams)) + } +} + +func TestRegisterAlertmanagerDuplicate(t *testing.T) { + // verifies that registering the same name twice returns an error + saved := saveUpstreams() + defer restoreUpstreams(saved) + UnregisterAll() + + am, err := NewAlertmanager("cluster", "dup-test", "http://localhost") + if err != nil { + t.Fatalf("NewAlertmanager failed: %s", err) + } + if regErr := RegisterAlertmanager(am); regErr != nil { + t.Fatalf("First RegisterAlertmanager failed: %s", regErr) + } + + am2, err := NewAlertmanager("cluster", "dup-test", "http://localhost:9094") + if err != nil { + t.Fatalf("NewAlertmanager failed: %s", err) + } + if err := RegisterAlertmanager(am2); err == nil { + t.Error("Second RegisterAlertmanager with same name should have returned an error") + } +} + +func TestGetAlertmanagerByName(t *testing.T) { + saved := saveUpstreams() + defer restoreUpstreams(saved) + UnregisterAll() + + am, err := NewAlertmanager("cluster", "lookup-test", "http://localhost") + if err != nil { + t.Fatalf("NewAlertmanager failed: %s", err) + } + _ = RegisterAlertmanager(am) + + // verifies that a registered instance is found by name + found := GetAlertmanagerByName("lookup-test") + if found == nil { + t.Error("GetAlertmanagerByName returned nil for registered instance") + } + if found != nil && found.Name != "lookup-test" { + t.Errorf("GetAlertmanagerByName returned wrong instance: %s", found.Name) + } + + // verifies that a non-existent name returns nil + notFound := GetAlertmanagerByName("does-not-exist") + if notFound != nil { + t.Errorf("GetAlertmanagerByName returned non-nil for unregistered name: %s", notFound.Name) + } +} + +func TestWithHealthchecksVisible(t *testing.T) { + // verifies that WithHealthchecksVisible sets the healthchecksVisible field + am, err := NewAlertmanager("cluster", "hcv-test", "http://localhost", + WithHealthchecksVisible(true), + ) + if err != nil { + t.Fatalf("NewAlertmanager failed: %s", err) + } + if !am.healthchecksVisible { + t.Error("Expected healthchecksVisible to be true") + } +} + +func TestIsHealthy(t *testing.T) { + // verifies that a fresh alertmanager with no errors is healthy + am, err := NewAlertmanager("cluster", "healthy-test", "http://localhost") + if err != nil { + t.Fatalf("NewAlertmanager failed: %s", err) + } + if !am.IsHealthy() { + t.Error("Expected fresh alertmanager to be healthy") + } + + // verifies that setting lastError makes it unhealthy + am.lock.Lock() + am.lastError = "something went wrong" + am.lock.Unlock() + if am.IsHealthy() { + t.Error("Expected alertmanager with lastError to be unhealthy") + } +} + +func TestClusterMemberNames(t *testing.T) { + saved := saveUpstreams() + defer restoreUpstreams(saved) + UnregisterAll() + + // register three alertmanagers: two in same cluster, one different + am1, _ := NewAlertmanager("prod", "am-1", "http://localhost:9091") + am2, _ := NewAlertmanager("prod", "am-2", "http://localhost:9092") + am3, _ := NewAlertmanager("staging", "am-3", "http://localhost:9093") + _ = RegisterAlertmanager(am1) + _ = RegisterAlertmanager(am2) + _ = RegisterAlertmanager(am3) + + // verifies that am1 sees itself and am2 as cluster members, but not am3 + members := am1.ClusterMemberNames() + if len(members) != 2 { + t.Errorf("Expected 2 cluster members, got %d: %v", len(members), members) + } + if members[0] != "am-1" || members[1] != "am-2" { + t.Errorf("Expected [am-1 am-2], got %v", members) + } + + // verifies that am3 only sees itself + members3 := am3.ClusterMemberNames() + if len(members3) != 1 { + t.Errorf("Expected 1 cluster member for staging, got %d: %v", len(members3), members3) + } + if members3[0] != "am-3" { + t.Errorf("Expected [am-3], got %v", members3) + } +} + func TestOptions(t *testing.T) { for _, tc := range testCases { var httpTransport http.RoundTripper diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 5a6756a51..15ec08f10 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -310,9 +310,20 @@ func TestUrlSecretTest(t *testing.T) { } } -// FIXME check logged values func TestLogValues(_ *testing.T) { _, _ = mockConfigRead() + // add servers and auth users to exercise sanitization branches in LogValues + Config.Alertmanager.Servers = append(Config.Alertmanager.Servers, AlertmanagerConfig{ + Cluster: "cluster1", + Name: "am1", + URI: "http://user:pass@localhost:9093", + ExternalURI: "http://user:pass@am.example.com", + Headers: map[string]string{"Authorization": "Bearer secret"}, + }) + Config.Authentication.BasicAuth.Users = append(Config.Authentication.BasicAuth.Users, AuthenticationUser{ + Username: "admin", + Password: "secret", + }) Config.LogValues() } diff --git a/internal/filters/matcher_test.go b/internal/filters/matcher_test.go index 4abdbfa27..d9ee5cd89 100644 --- a/internal/filters/matcher_test.go +++ b/internal/filters/matcher_test.go @@ -132,6 +132,58 @@ func TestNegativeRegexpMatcher(t *testing.T) { } } +func TestRegexpMatcherWithStringValB(t *testing.T) { + tests := []matchTest{ + // verifies that passing a raw string pattern (not pre-compiled) compiles and matches + {"abcdef", "^abc", true, true}, + // verifies cache hit on the same string pattern + {"abcdef", "^abc", true, true}, + // verifies that a non-matching raw string pattern returns false + {"xyz", "^abc", true, false}, + } + for _, mt := range tests { + m := regexpMatcher{} + if result := m.Compare(mt.ValA, mt.ValB); result != mt.Expacted { + t.Errorf("RegexpMatcher(%#v, %#v string) returned %v when %v was expected", mt.ValA, mt.ValB, result, mt.Expacted) + } + } +} + +func TestRegexpMatcherWithInvalidPattern(t *testing.T) { + // verifies that an invalid regex pattern string returns false instead of panicking + m := regexpMatcher{} + result := m.Compare("foo", "[invalid") + if result != false { + t.Errorf("RegexpMatcher with invalid pattern returned %v, expected false", result) + } +} + +func TestNegativeRegexpMatcherWithStringValB(t *testing.T) { + tests := []matchTest{ + // verifies that passing a raw string pattern (not pre-compiled) compiles and negation works + {"abcdef", "^abcdef$", true, false}, + // verifies cache hit on the same string pattern + {"abcdef", "^abcdef$", true, false}, + // verifies that a non-matching raw string pattern returns true (negated) + {"xyz", "^abcdef$", true, true}, + } + for _, mt := range tests { + m := negativeRegexMatcher{} + if result := m.Compare(mt.ValA, mt.ValB); result != mt.Expacted { + t.Errorf("NegativeRegexMatcher(%#v, %#v string) returned %v when %v was expected", mt.ValA, mt.ValB, result, mt.Expacted) + } + } +} + +func TestNegativeRegexpMatcherWithInvalidPattern(t *testing.T) { + // verifies that an invalid regex pattern string returns false instead of panicking + m := negativeRegexMatcher{} + result := m.Compare("foo", "[invalid") + if result != false { + t.Errorf("NegativeRegexMatcher with invalid pattern returned %v, expected false", result) + } +} + func TestNewMatcher(t *testing.T) { operators := []string{ equalOperator, diff --git a/internal/mapper/v017/api.go b/internal/mapper/v017/api.go index f3480eeae..677fbcdaa 100644 --- a/internal/mapper/v017/api.go +++ b/internal/mapper/v017/api.go @@ -4,6 +4,7 @@ import ( "net/http" "net/url" "path" + "slices" "sort" "time" @@ -52,7 +53,7 @@ func groups(c *client.AlertmanagerAPI, timeout time.Duration) ([]models.AlertGro for k, v := range group.Labels { ls = ls.Set(k, v) } - sort.Sort(ls) + slices.SortFunc(ls, models.CompareLabels) g := models.AlertGroup{ Receiver: models.NewUniqueString(*group.Receiver.Name), Labels: ls, @@ -63,7 +64,7 @@ func groups(c *client.AlertmanagerAPI, timeout time.Duration) ([]models.AlertGro for k, v := range alert.Labels { ls = ls.Set(k, v) } - sort.Sort(ls) + slices.SortFunc(ls, models.CompareLabels) a := models.Alert{ Fingerprint: *alert.Fingerprint, Receiver: models.NewUniqueString(*group.Receiver.Name), diff --git a/internal/models/alert.go b/internal/models/alert.go index 142cb2167..0f551c851 100644 --- a/internal/models/alert.go +++ b/internal/models/alert.go @@ -1,6 +1,7 @@ package models import ( + "cmp" "encoding/json" "strconv" "time" @@ -65,33 +66,37 @@ func (ls Labels) Map() map[string]string { return m } -func (ls Labels) Len() int { - return len(ls) -} - -func (ls Labels) Swap(i, j int) { - ls[i], ls[j] = ls[j], ls[i] -} - -func (ls Labels) Less(i, j int) bool { - ai, aj := -1, -1 +func CompareLabels(a, b Label) int { + ai, bi := -1, -1 for index, name := range config.Config.Labels.Order { - if ls[i].Name.Value() == name { + if a.Name.Value() == name { ai = index - } else if ls[j].Name.Value() == name { - aj = index + } else if b.Name.Value() == name { + bi = index } - if ai >= 0 && aj >= 0 { - return ai < aj + if ai >= 0 && bi >= 0 { + return cmp.Compare(ai, bi) } } - if ai != aj { - return aj < ai + if ai != bi { + return cmp.Compare(bi, ai) } - if ls[i].Name.Value() == ls[j].Name.Value() { - return sortorder.NaturalLess(ls[i].Value.Value(), ls[j].Value.Value()) + if a.Name.Value() == b.Name.Value() { + if sortorder.NaturalLess(a.Value.Value(), b.Value.Value()) { + return -1 + } + if sortorder.NaturalLess(b.Value.Value(), a.Value.Value()) { + return 1 + } + return 0 } - return sortorder.NaturalLess(ls[i].Name.Value(), ls[j].Name.Value()) + if sortorder.NaturalLess(a.Name.Value(), b.Name.Value()) { + return -1 + } + if sortorder.NaturalLess(b.Name.Value(), a.Name.Value()) { + return 1 + } + return 0 } func (ls Labels) Get(name string) *Label { diff --git a/internal/models/alert_test.go b/internal/models/alert_test.go index b480151ea..6f2db332a 100644 --- a/internal/models/alert_test.go +++ b/internal/models/alert_test.go @@ -1,10 +1,12 @@ package models_test import ( + "encoding/json" "fmt" - "sort" + "slices" "strings" "testing" + "time" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" @@ -144,6 +146,42 @@ func TestSortLabels(t *testing.T) { {Name: models.NewUniqueString("bar"), Value: models.NewUniqueString("1")}, }, }, + // verifies that identical labels stay in their original positions + { + order: []string{}, + in: models.Labels{ + {Name: models.NewUniqueString("foo"), Value: models.NewUniqueString("bar")}, + {Name: models.NewUniqueString("foo"), Value: models.NewUniqueString("bar")}, + }, + out: models.Labels{ + {Name: models.NewUniqueString("foo"), Value: models.NewUniqueString("bar")}, + {Name: models.NewUniqueString("foo"), Value: models.NewUniqueString("bar")}, + }, + }, + // verifies that same-name labels with different values sort by value descending naturally + { + order: []string{}, + in: models.Labels{ + {Name: models.NewUniqueString("foo"), Value: models.NewUniqueString("z")}, + {Name: models.NewUniqueString("foo"), Value: models.NewUniqueString("a")}, + }, + out: models.Labels{ + {Name: models.NewUniqueString("foo"), Value: models.NewUniqueString("a")}, + {Name: models.NewUniqueString("foo"), Value: models.NewUniqueString("z")}, + }, + }, + // verifies that completely identical name labels (no order config) with equal names sort by name naturally + { + order: []string{}, + in: models.Labels{ + {Name: models.NewUniqueString("zzz"), Value: models.NewUniqueString("1")}, + {Name: models.NewUniqueString("aaa"), Value: models.NewUniqueString("1")}, + }, + out: models.Labels{ + {Name: models.NewUniqueString("aaa"), Value: models.NewUniqueString("1")}, + {Name: models.NewUniqueString("zzz"), Value: models.NewUniqueString("1")}, + }, + }, } defer func() { @@ -153,7 +191,7 @@ func TestSortLabels(t *testing.T) { for i, testCase := range testCases { t.Run(fmt.Sprintf("[%d] order=%v", i, testCase.order), func(t *testing.T) { config.Config.Labels.Order = testCase.order - sort.Sort(testCase.in) + slices.SortFunc(testCase.in, models.CompareLabels) if diff := cmp.Diff(testCase.in, testCase.out, cmpopts.EquateComparable(models.Label{})); diff != "" { t.Errorf("Incorrectly sorted labels (-want +got):\n%s", diff) t.FailNow() @@ -161,3 +199,156 @@ func TestSortLabels(t *testing.T) { }) } } + +func TestLabelsMap(t *testing.T) { + type testCaseT struct { + labels models.Labels + expected map[string]string + } + + testCases := []testCaseT{ + // verifies that empty labels produce an empty map + { + labels: models.Labels{}, + expected: map[string]string{}, + }, + // verifies that labels are converted to a name->value map + { + labels: models.Labels{ + {Name: models.NewUniqueString("foo"), Value: models.NewUniqueString("bar")}, + {Name: models.NewUniqueString("baz"), Value: models.NewUniqueString("qux")}, + }, + expected: map[string]string{"foo": "bar", "baz": "qux"}, + }, + } + + for _, tc := range testCases { + result := tc.labels.Map() + if diff := cmp.Diff(tc.expected, result); diff != "" { + t.Errorf("Labels.Map() mismatch (-want +got):\n%s", diff) + } + } +} + +func TestLabelsGetValue(t *testing.T) { + labels := models.Labels{ + {Name: models.NewUniqueString("foo"), Value: models.NewUniqueString("bar")}, + {Name: models.NewUniqueString("baz"), Value: models.NewUniqueString("qux")}, + } + + type testCaseT struct { + name string + expected string + } + + testCases := []testCaseT{ + // verifies that an existing label returns its value + {name: "foo", expected: "bar"}, + // verifies that another existing label returns its value + {name: "baz", expected: "qux"}, + // verifies that a missing label returns an empty string + {name: "missing", expected: ""}, + } + + for _, tc := range testCases { + result := labels.GetValue(tc.name) + if result != tc.expected { + t.Errorf("Labels.GetValue(%q) returned %q, expected %q", tc.name, result, tc.expected) + } + } +} + +func TestUniqueStringJSONRoundTrip(t *testing.T) { + // verifies that UniqueString survives a JSON marshal/unmarshal round-trip + original := models.NewUniqueString("test_value") + data, err := json.Marshal(&original) + if err != nil { + t.Fatalf("json.Marshal failed: %s", err) + } + if string(data) != `"test_value"` { + t.Errorf("json.Marshal produced %s, expected %q", string(data), `"test_value"`) + } + + var decoded models.UniqueString + err = json.Unmarshal(data, &decoded) + if err != nil { + t.Fatalf("json.Unmarshal failed: %s", err) + } + if decoded.Value() != "test_value" { + t.Errorf("json.Unmarshal produced %q, expected %q", decoded.Value(), "test_value") + } +} + +func TestUniqueStringUnmarshalJSONError(t *testing.T) { + // verifies that UnmarshalJSON returns an error for invalid JSON input + var us models.UniqueString + err := json.Unmarshal([]byte(`{invalid`), &us) + if err == nil { + t.Error("json.Unmarshal should have returned an error for invalid JSON") + } +} + +func TestUpdateFingerprints(t *testing.T) { + // verifies that UpdateFingerprints produces stable, non-empty fingerprints + // including the alertmanager instance, silenced-by, and inhibited-by branches + alert := models.Alert{ + StartsAt: time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC), + State: models.NewUniqueString("active"), + Receiver: models.NewUniqueString("default"), + Fingerprint: "abc123", + Labels: models.Labels{ + {Name: models.NewUniqueString("alertname"), Value: models.NewUniqueString("TestAlert")}, + }, + Annotations: models.Annotations{ + { + Name: models.NewUniqueString("summary"), + Value: models.NewUniqueString("test summary"), + Visible: true, + IsLink: false, + IsAction: false, + }, + }, + Alertmanager: []models.AlertmanagerInstance{ + { + Fingerprint: "fp1", + Name: "am1", + Cluster: "cluster1", + State: models.NewUniqueString("active"), + StartsAt: time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC), + Source: "http://source", + SilencedBy: []string{"silence1", "silence2"}, + InhibitedBy: []string{"inhibit1"}, + }, + }, + } + + alert.UpdateFingerprints() + + if alert.LabelsFingerprint() == "" { + t.Error("LabelsFingerprint() returned empty string after UpdateFingerprints()") + } + if alert.ContentFingerprint() == "" { + t.Error("ContentFingerprint() returned empty string after UpdateFingerprints()") + } + + // verifies that calling UpdateFingerprints again produces the same result + fp1 := alert.LabelsFingerprint() + cfp1 := alert.ContentFingerprint() + alert.UpdateFingerprints() + if alert.LabelsFingerprint() != fp1 { + t.Errorf("LabelsFingerprint() not stable: %q != %q", alert.LabelsFingerprint(), fp1) + } + if alert.ContentFingerprint() != cfp1 { + t.Errorf("ContentFingerprint() not stable: %q != %q", alert.ContentFingerprint(), cfp1) + } + + // verifies that changing a label produces a different fingerprint + alert2 := alert + alert2.Labels = models.Labels{ + {Name: models.NewUniqueString("alertname"), Value: models.NewUniqueString("DifferentAlert")}, + } + alert2.UpdateFingerprints() + if alert2.LabelsFingerprint() == fp1 { + t.Error("LabelsFingerprint() should differ when labels change") + } +} diff --git a/internal/models/alertgroup.go b/internal/models/alertgroup.go index 227a57a97..77340a083 100644 --- a/internal/models/alertgroup.go +++ b/internal/models/alertgroup.go @@ -1,6 +1,7 @@ package models import ( + "cmp" "io" "strconv" "time" @@ -11,23 +12,15 @@ import ( // AlertList is flat list of karmaAlert objects type AlertList []Alert -func (a AlertList) Len() int { - return len(a) -} - -func (a AlertList) Swap(i, j int) { - a[i], a[j] = a[j], a[i] -} - -func (a AlertList) Less(i, j int) bool { +func CompareAlerts(a, b Alert) int { // compare timestamps, if equal compare fingerprints to stable sort order - if a[i].StartsAt.After(a[j].StartsAt) { - return true + if a.StartsAt.After(b.StartsAt) { + return -1 } - if a[i].StartsAt.Before(a[j].StartsAt) { - return false + if a.StartsAt.Before(b.StartsAt) { + return 1 } - return a[i].LabelsFingerprint() < a[j].LabelsFingerprint() + return cmp.Compare(a.LabelsFingerprint(), b.LabelsFingerprint()) } // AlertGroup is vanilla Alertmanager group, but alerts are flattened diff --git a/internal/models/alertgroup_test.go b/internal/models/alertgroup_test.go index 484580062..3810ca249 100644 --- a/internal/models/alertgroup_test.go +++ b/internal/models/alertgroup_test.go @@ -1,7 +1,7 @@ package models_test import ( - "sort" + "slices" "testing" "time" @@ -83,7 +83,7 @@ func TestAlertListSort(t *testing.T) { iterations := 100 failures := 0 for i := 1; i <= iterations; i++ { - sort.Sort(al) + slices.SortFunc(al, models.CompareAlerts) for _, testCase := range alertListSortTests { testCase.alert.UpdateFingerprints() if al[testCase.position].ContentFingerprint() != testCase.alert.ContentFingerprint() { @@ -202,7 +202,7 @@ func TestAlertGroupContentFingerprint(t *testing.T) { alert.UpdateFingerprints() alerts = append(alerts, alert) } - sort.Sort(alerts) + slices.SortFunc(alerts, models.CompareAlerts) testCase.ag.Alerts = alerts // get alert group fingerprint fp := testCase.ag.ContentFingerprint() @@ -232,6 +232,45 @@ func TestFingerprint(t *testing.T) { } } +func TestLabelsFingerprint(t *testing.T) { + // verifies that LabelsFingerprint produces non-empty output when labels are present + ag := models.AlertGroup{ + Receiver: models.NewUniqueString("default"), + Labels: models.Labels{ + {Name: models.NewUniqueString("foo"), Value: models.NewUniqueString("bar")}, + {Name: models.NewUniqueString("baz"), Value: models.NewUniqueString("qux")}, + }, + } + fp := ag.LabelsFingerprint() + if fp == "" { + t.Error("LabelsFingerprint() returned empty string for group with labels") + } + + // verifies that different labels produce a different fingerprint + ag2 := models.AlertGroup{ + Receiver: models.NewUniqueString("default"), + Labels: models.Labels{ + {Name: models.NewUniqueString("different"), Value: models.NewUniqueString("label")}, + }, + } + fp2 := ag2.LabelsFingerprint() + if fp == fp2 { + t.Errorf("LabelsFingerprint() should differ for groups with different labels, both returned %q", fp) + } + + // verifies that same receiver and labels produce the same fingerprint + ag3 := models.AlertGroup{ + Receiver: models.NewUniqueString("default"), + Labels: models.Labels{ + {Name: models.NewUniqueString("foo"), Value: models.NewUniqueString("bar")}, + {Name: models.NewUniqueString("baz"), Value: models.NewUniqueString("qux")}, + }, + } + if ag3.LabelsFingerprint() != fp { + t.Errorf("LabelsFingerprint() not stable: %q != %q", ag3.LabelsFingerprint(), fp) + } +} + type findLatestStartsAtTest struct { expectedStartsAt time.Time alerts []models.Alert diff --git a/internal/models/annotation.go b/internal/models/annotation.go index 59aa7cee2..9ad594a9f 100644 --- a/internal/models/annotation.go +++ b/internal/models/annotation.go @@ -3,7 +3,6 @@ package models import ( "net/url" "slices" - "sort" "strings" "github.com/fvbommel/sortorder" @@ -24,36 +23,40 @@ type Annotation struct { // Annotations is a slice of Annotation structs, needed to implement sorting type Annotations []Annotation -func (a Annotations) Len() int { - return len(a) -} - -func (a Annotations) Swap(i, j int) { - a[i], a[j] = a[j], a[i] -} - -func (a Annotations) Less(i, j int) bool { - // Sort the anotations listed in config.Config.Annotations.Order first, in +func compareAnnotations(a, b Annotation) int { + // Sort the annotations listed in config.Config.Annotations.Order first, in // the order they appear in that list; remaining annotations are sorted alphabetically. - ai, aj := -1, -1 + ai, bi := -1, -1 for index, name := range config.Config.Annotations.Order { - if a[i].Name.Value() == name { + if a.Name.Value() == name { ai = index - } else if a[j].Name.Value() == name { - aj = index + } else if b.Name.Value() == name { + bi = index } // If both annotations are in c.C.A.Order, sort them according to the // order in that list. - if ai >= 0 && aj >= 0 { - return ai < aj + if ai >= 0 && bi >= 0 { + if ai < bi { + return -1 + } + return 1 } } // If only one of the annotations was in c.C.A.Order, that one goes first. - if ai != aj { - return aj < ai + if ai != bi { + if bi < ai { + return -1 + } + return 1 } // If neither annotation was in c.C.A.Order, sort alphabetically. - return sortorder.NaturalLess(a[i].Name.Value(), a[j].Name.Value()) + if sortorder.NaturalLess(a.Name.Value(), b.Name.Value()) { + return -1 + } + if sortorder.NaturalLess(b.Name.Value(), a.Name.Value()) { + return 1 + } + return 0 } // AnnotationsFromMap will convert a map[string]string to a list of Annotation @@ -70,10 +73,14 @@ func AnnotationsFromMap(m map[string]string) Annotations { } annotations = append(annotations, a) } - sort.Sort(annotations) + SortAnnotations(annotations) return annotations } +func SortAnnotations(annotations Annotations) { + slices.SortFunc(annotations, compareAnnotations) +} + var linkSchemes = []string{ "ftp", "http", diff --git a/internal/models/annotation_test.go b/internal/models/annotation_test.go index 6d7aed464..f14162a44 100644 --- a/internal/models/annotation_test.go +++ b/internal/models/annotation_test.go @@ -1,7 +1,6 @@ package models_test import ( - "sort" "testing" "unique" @@ -211,7 +210,7 @@ func TestAnnotationsSort(t *testing.T) { IsLink: true, }, } - sort.Stable(annotations) + models.SortAnnotations(annotations) if annotations[0].Name.Value() != "abc" { t.Errorf("Expected 'abc' to be first, got '%s'", annotations[0].Name.Value()) } @@ -248,7 +247,7 @@ func TestAnnotationsCustomOrderSort(t *testing.T) { }, } config.Config.Annotations.Order = []string{"xyz", "yyz"} - sort.Stable(annotations) + models.SortAnnotations(annotations) if annotations[0].Name.Value() != "xyz" { t.Errorf("Expected 'xyz' to be first, got '%s'", annotations[0].Name.Value()) } @@ -262,3 +261,35 @@ func TestAnnotationsCustomOrderSort(t *testing.T) { t.Errorf("Expected 'bar' to be last, got '%s'", annotations[3].Name.Value()) } } + +func TestAnnotationsSortIdenticalNames(t *testing.T) { + // verifies that annotations with the same name are treated as equal by the comparator + config.Config.Annotations.Order = []string{} + annotations := models.Annotations{ + models.Annotation{ + Name: models.NewUniqueString("dup"), + Value: models.NewUniqueString("second"), + Visible: true, + }, + models.Annotation{ + Name: models.NewUniqueString("dup"), + Value: models.NewUniqueString("first"), + Visible: true, + }, + models.Annotation{ + Name: models.NewUniqueString("aaa"), + Value: models.NewUniqueString("value"), + Visible: true, + }, + } + models.SortAnnotations(annotations) + if annotations[0].Name.Value() != "aaa" { + t.Errorf("Expected 'aaa' to be first, got '%s'", annotations[0].Name.Value()) + } + if annotations[1].Name.Value() != "dup" { + t.Errorf("Expected 'dup' to be second, got '%s'", annotations[1].Name.Value()) + } + if annotations[2].Name.Value() != "dup" { + t.Errorf("Expected 'dup' to be third, got '%s'", annotations[2].Name.Value()) + } +} diff --git a/internal/models/api.go b/internal/models/api.go index 8ec7941da..bd059bfd6 100644 --- a/internal/models/api.go +++ b/internal/models/api.go @@ -1,6 +1,7 @@ package models import ( + "cmp" "fmt" "net/url" "slices" @@ -28,7 +29,7 @@ type Color struct { Alpha uint8 `json:"alpha"` } -func (c *Color) ToString() string { +func (c *Color) String() string { return fmt.Sprintf("rgba(%d,%d,%d,%d)", c.Red, c.Green, c.Blue, c.Alpha) } @@ -55,19 +56,17 @@ type LabelValueStats struct { type LabelValueStatsList []LabelValueStats -func (lvsl LabelValueStatsList) Len() int { - return len(lvsl) -} - -func (lvsl LabelValueStatsList) Swap(i, j int) { - lvsl[i], lvsl[j] = lvsl[j], lvsl[i] -} - -func (lvsl LabelValueStatsList) Less(i, j int) bool { - if lvsl[i].Hits == lvsl[j].Hits { - return sortorder.NaturalLess(lvsl[i].Value, lvsl[j].Value) +func CompareLabelValueStats(a, b LabelValueStats) int { + if a.Hits != b.Hits { + return cmp.Compare(b.Hits, a.Hits) } - return lvsl[i].Hits > lvsl[j].Hits + if sortorder.NaturalLess(a.Value, b.Value) { + return -1 + } + if sortorder.NaturalLess(b.Value, a.Value) { + return 1 + } + return 0 } // LabelNameStats is used in the overview modal, it shows top labels across alerts @@ -79,19 +78,11 @@ type LabelNameStats struct { type LabelNameStatsList []LabelNameStats -func (lnsl LabelNameStatsList) Len() int { - return len(lnsl) -} - -func (lnsl LabelNameStatsList) Swap(i, j int) { - lnsl[i], lnsl[j] = lnsl[j], lnsl[i] -} - -func (lnsl LabelNameStatsList) Less(i, j int) bool { - if lnsl[i].Hits == lnsl[j].Hits { - return lnsl[i].Name < lnsl[j].Name +func CompareLabelNameStats(a, b LabelNameStats) int { + if a.Hits != b.Hits { + return cmp.Compare(b.Hits, a.Hits) } - return lnsl[i].Hits > lnsl[j].Hits + return cmp.Compare(a.Name, b.Name) } // APIAlertGroupSharedMaps defines shared part of APIAlertGroup @@ -121,13 +112,8 @@ func (ag *APIAlertGroup) dedupLabels() { for _, alert := range ag.Alerts { for _, l := range alert.Labels { - key := fmt.Sprintf("%s\n%s", l.Name.Value(), l.Value.Value()) - _, found := labelCounts[key] - if found { - labelCounts[key]++ - } else { - labelCounts[key] = 1 - } + key := l.Name.Value() + "\n" + l.Value.Value() + labelCounts[key]++ } } @@ -136,7 +122,7 @@ func (ag *APIAlertGroup) dedupLabels() { for i, alert := range ag.Alerts { newAlertLabels := Labels{} for _, l := range alert.Labels { - key := fmt.Sprintf("%s\n%s", l.Name.Value(), l.Value.Value()) + key := l.Name.Value() + "\n" + l.Value.Value() if labelCounts[key] == totalAlerts { sharedLabels = sharedLabels.Add(l) } else { @@ -183,27 +169,22 @@ func (ag *APIAlertGroup) dedupAnnotations() { for _, alert := range ag.Alerts { for _, annotation := range alert.Annotations { - key := fmt.Sprintf("%s\n%s", annotation.Name.Value(), annotation.Value.Value()) - _, found := annotationCount[key] - if found { - annotationCount[key]++ - } else { - annotationCount[key] = 1 - } + key := annotation.Name.Value() + "\n" + annotation.Value.Value() + annotationCount[key]++ } } sharedAnnotations := Annotations{} - sharedKeys := []string{} + sharedKeys := map[string]struct{}{} for i, alert := range ag.Alerts { newAlertAnnotations := Annotations{} for _, annotation := range alert.Annotations { - key := fmt.Sprintf("%s\n%s", annotation.Name.Value(), annotation.Value.Value()) + key := annotation.Name.Value() + "\n" + annotation.Value.Value() if annotationCount[key] == totalAlerts { - if !slices.Contains(sharedKeys, key) { + if _, ok := sharedKeys[key]; !ok { sharedAnnotations = append(sharedAnnotations, annotation) - sharedKeys = append(sharedKeys, key) + sharedKeys[key] = struct{}{} } } else { newAlertAnnotations = append(newAlertAnnotations, annotation) @@ -222,21 +203,16 @@ func (ag *APIAlertGroup) dedupSilences() { for _, alert := range ag.Alerts { // process each cluster only once, rather than each alertmanager instance - clusters := []string{} + clusters := map[string]struct{}{} for _, am := range alert.Alertmanager { - if slices.Contains(clusters, am.Cluster) { + if _, ok := clusters[am.Cluster]; ok { continue } - clusters = append(clusters, am.Cluster) + clusters[am.Cluster] = struct{}{} for _, silenceID := range am.SilencedBy { - _, ok := silencesByCluster[am.Cluster] - if !ok { + if _, ok := silencesByCluster[am.Cluster]; !ok { silencesByCluster[am.Cluster] = map[string]int{} } - _, ok = silencesByCluster[am.Cluster][silenceID] - if !ok { - silencesByCluster[am.Cluster][silenceID] = 0 - } silencesByCluster[am.Cluster][silenceID]++ } } diff --git a/internal/models/api_test.go b/internal/models/api_test.go index d5f3c4025..24bbc3c0a 100644 --- a/internal/models/api_test.go +++ b/internal/models/api_test.go @@ -3,7 +3,7 @@ package models_test import ( "bytes" "encoding/json" - "sort" + "slices" "testing" "github.com/beme/abide" @@ -11,6 +11,38 @@ import ( "github.com/prymitive/karma/internal/models" ) +func TestColorString(t *testing.T) { + type testCaseT struct { + color models.Color + expected string + } + + testCases := []testCaseT{ + // verifies zero-value color produces all zeros + { + color: models.Color{Red: 0, Green: 0, Blue: 0, Alpha: 0}, + expected: "rgba(0,0,0,0)", + }, + // verifies max-value color produces all 255s + { + color: models.Color{Red: 255, Green: 255, Blue: 255, Alpha: 255}, + expected: "rgba(255,255,255,255)", + }, + // verifies mixed values are formatted correctly + { + color: models.Color{Red: 10, Green: 20, Blue: 30, Alpha: 128}, + expected: "rgba(10,20,30,128)", + }, + } + + for _, tc := range testCases { + result := tc.color.String() + if result != tc.expected { + t.Errorf("Color.String() returned %q, expected %q", result, tc.expected) + } + } +} + func TestDedupSharedMaps(t *testing.T) { ag := models.APIAlertGroup{ AlertGroup: models.AlertGroup{ @@ -192,6 +224,105 @@ func TestDedupWithBadSource(t *testing.T) { } } +func TestDedupSharedMapsWithDropNames(t *testing.T) { + // verifies that passing dropNames to DedupSharedMaps removes those labels + // from both group labels and alert labels + ag := models.APIAlertGroup{ + AlertGroup: models.AlertGroup{ + Receiver: models.NewUniqueString("default"), + Labels: models.Labels{ + {Name: models.NewUniqueString("alertname"), Value: models.NewUniqueString("TestAlert")}, + {Name: models.NewUniqueString("cluster"), Value: models.NewUniqueString("prod")}, + }, + Alerts: models.AlertList{ + models.Alert{ + State: models.AlertStateActive, + Labels: models.Labels{ + {Name: models.NewUniqueString("alertname"), Value: models.NewUniqueString("TestAlert")}, + {Name: models.NewUniqueString("cluster"), Value: models.NewUniqueString("prod")}, + {Name: models.NewUniqueString("instance"), Value: models.NewUniqueString("1")}, + }, + }, + models.Alert{ + State: models.AlertStateActive, + Labels: models.Labels{ + {Name: models.NewUniqueString("alertname"), Value: models.NewUniqueString("TestAlert")}, + {Name: models.NewUniqueString("cluster"), Value: models.NewUniqueString("prod")}, + {Name: models.NewUniqueString("instance"), Value: models.NewUniqueString("2")}, + }, + }, + }, + }, + } + ag.DedupSharedMaps([]string{"cluster"}) + + // "cluster" should be removed from group labels + if ag.Labels.Get("cluster") != nil { + t.Error("Expected 'cluster' label to be removed from group labels") + } + // "alertname" should remain in group labels + if ag.Labels.Get("alertname") == nil { + t.Error("Expected 'alertname' label to remain in group labels") + } + // alert labels should not contain "cluster" (dropped) or "alertname" (shared with group) + for i, alert := range ag.Alerts { + if alert.Labels.Get("cluster") != nil { + t.Errorf("Alert[%d]: expected 'cluster' label to be removed", i) + } + if alert.Labels.Get("alertname") != nil { + t.Errorf("Alert[%d]: expected 'alertname' label to be removed (shared with group)", i) + } + } +} + +func TestCompareLabelValueStats(t *testing.T) { + type testCaseT struct { + a models.LabelValueStats + b models.LabelValueStats + expected int + } + + testCases := []testCaseT{ + // verifies that higher hits sorts before lower hits + { + a: models.LabelValueStats{Value: "a", Hits: 10}, + b: models.LabelValueStats{Value: "b", Hits: 5}, + expected: -1, + }, + // verifies that lower hits sorts after higher hits + { + a: models.LabelValueStats{Value: "a", Hits: 5}, + b: models.LabelValueStats{Value: "b", Hits: 10}, + expected: 1, + }, + // verifies that equal hits falls back to natural value ordering (a < b) + { + a: models.LabelValueStats{Value: "a", Hits: 5}, + b: models.LabelValueStats{Value: "b", Hits: 5}, + expected: -1, + }, + // verifies that equal hits falls back to natural value ordering (b > a) + { + a: models.LabelValueStats{Value: "b", Hits: 5}, + b: models.LabelValueStats{Value: "a", Hits: 5}, + expected: 1, + }, + // verifies that identical hits and values returns 0 + { + a: models.LabelValueStats{Value: "a", Hits: 5}, + b: models.LabelValueStats{Value: "a", Hits: 5}, + expected: 0, + }, + } + + for _, tc := range testCases { + result := models.CompareLabelValueStats(tc.a, tc.b) + if result != tc.expected { + t.Errorf("CompareLabelValueStats(%v, %v) returned %d, expected %d", tc.a, tc.b, result, tc.expected) + } + } +} + func TestNameStatsSort(t *testing.T) { nameStats := models.LabelNameStatsList{ { @@ -400,9 +531,9 @@ func TestNameStatsSort(t *testing.T) { before := string(b) for _, n := range nameStats { - sort.Sort(n.Values) + slices.SortFunc(n.Values, models.CompareLabelValueStats) } - sort.Sort(nameStats) + slices.SortFunc(nameStats, models.CompareLabelNameStats) a, err := json.Marshal(nameStats) if err != nil { diff --git a/internal/slices/slices.go b/internal/slices/slices.go index 27670dde9..6914533b5 100644 --- a/internal/slices/slices.go +++ b/internal/slices/slices.go @@ -4,36 +4,41 @@ import ( "crypto/sha1" "encoding/hex" "regexp" - "slices" ) // StringSliceToSHA1 returns a SHA1 hash computed from a slice of strings -func StringSliceToSHA1(stringArray []string) (string, error) { +func StringSliceToSHA1(stringArray []string) string { h := sha1.New() for _, s := range stringArray { _, _ = h.Write([]byte(s)) _, _ = h.Write([]byte("\n")) } - return hex.EncodeToString(h.Sum(nil)), nil + return hex.EncodeToString(h.Sum(nil)) } func StringSliceDiff(slice1, slice2 []string) ([]string, []string) { missing := []string{} extra := []string{} - var found bool + set1 := make(map[string]struct{}, len(slice1)) + for _, s := range slice1 { + set1[s] = struct{}{} + } - for _, s1 := range slice1 { - found = slices.Contains(slice2, s1) - if !found { - missing = append(missing, s1) + set2 := make(map[string]struct{}, len(slice2)) + for _, s := range slice2 { + set2[s] = struct{}{} + } + + for _, s := range slice1 { + if _, ok := set2[s]; !ok { + missing = append(missing, s) } } - for _, s2 := range slice2 { - found = slices.Contains(slice1, s2) - if !found { - extra = append(extra, s2) + for _, s := range slice2 { + if _, ok := set1[s]; !ok { + extra = append(extra, s) } } diff --git a/internal/slices/slices_test.go b/internal/slices/slices_test.go index 642823e2d..a9baed6ad 100644 --- a/internal/slices/slices_test.go +++ b/internal/slices/slices_test.go @@ -1,6 +1,7 @@ package slices_test import ( + "regexp" "testing" "github.com/prymitive/karma/internal/slices" @@ -9,10 +10,7 @@ import ( ) func TestStringSliceToSHA1(t *testing.T) { - s, err := slices.StringSliceToSHA1([]string{"a", "b", "c"}) - if err != nil { - t.Errorf("StringSliceToSHA1() returned error: %s", err) - } + s := slices.StringSliceToSHA1([]string{"a", "b", "c"}) if s == "" { t.Errorf("StringSliceToSHA1() returned empty string") } @@ -63,3 +61,45 @@ func TestStringSliceDiff(t *testing.T) { } } } + +func TestMatchesAnyRegex(t *testing.T) { + type testCaseT struct { + value string + regexes []*regexp.Regexp + expected bool + } + + testCases := []testCaseT{ + // verifies that an empty regex list never matches + { + value: "foo", + regexes: []*regexp.Regexp{}, + expected: false, + }, + // verifies that a matching regex returns true + { + value: "foo", + regexes: []*regexp.Regexp{regexp.MustCompile("^foo$")}, + expected: true, + }, + // verifies that a non-matching regex returns false + { + value: "bar", + regexes: []*regexp.Regexp{regexp.MustCompile("^foo$")}, + expected: false, + }, + // verifies that the second regex in the list can match + { + value: "bar", + regexes: []*regexp.Regexp{regexp.MustCompile("^foo$"), regexp.MustCompile("^bar$")}, + expected: true, + }, + } + + for _, tc := range testCases { + result := slices.MatchesAnyRegex(tc.value, tc.regexes) + if result != tc.expected { + t.Errorf("MatchesAnyRegex(%q, ...) returned %v, expected %v", tc.value, result, tc.expected) + } + } +} diff --git a/internal/transform/colors.go b/internal/transform/colors.go index 5e82dc592..aa7ae38f5 100644 --- a/internal/transform/colors.go +++ b/internal/transform/colors.go @@ -59,7 +59,7 @@ func parseCustomColor(colorStore models.LabelsColorMap, key, val, customColor st } colorStore[key][val] = models.LabelColors{ Brightness: brightness, - Background: bc.ToString(), + Background: bc.String(), } } @@ -102,7 +102,7 @@ func ColorLabel(colorStore models.LabelsColorMap, key, val string) { brightness := rgbToBrightness(bc.Red, bc.Green, bc.Blue) colorStore[key][val] = models.LabelColors{ Brightness: brightness, - Background: bc.ToString(), + Background: bc.String(), } } } diff --git a/internal/transform/strip.go b/internal/transform/strip.go index 1304bcd5c..42fba1aa3 100644 --- a/internal/transform/strip.go +++ b/internal/transform/strip.go @@ -3,17 +3,16 @@ package transform import ( "regexp" "slices" - "sort" "strings" "github.com/prymitive/karma/internal/models" sliceutils "github.com/prymitive/karma/internal/slices" ) -// StripLables allows filtering out some labels from alerts +// StripLabels allows filtering out some labels from alerts // it takes the list of label keys to ignore and alert label map // it will return label map without labels found on the ignore list -func StripLables(keptLabels, ignoredLabels []string, keptLabelsRegex, ignoredLabelsRegex []*regexp.Regexp, +func StripLabels(keptLabels, ignoredLabels []string, keptLabelsRegex, ignoredLabelsRegex []*regexp.Regexp, sourceLabels models.Labels, ) models.Labels { // empty keep lists means keep everything by default @@ -39,7 +38,7 @@ func StripLables(keptLabels, ignoredLabels []string, keptLabelsRegex, ignoredLab labels = labels.Add(l) } } - sort.Sort(labels) + slices.SortFunc(labels, models.CompareLabels) return labels } diff --git a/internal/transform/strip_test.go b/internal/transform/strip_test.go index 08f8530f0..feb2ff5ec 100644 --- a/internal/transform/strip_test.go +++ b/internal/transform/strip_test.go @@ -180,7 +180,7 @@ func TestStripLables(t *testing.T) { for _, testCase := range stripLabelTests { keepRegex := getCompiledRegex(testCase.keepRegex, t) stripRegex := getCompiledRegex(testCase.stripRegex, t) - labels := transform.StripLables(testCase.keep, testCase.strip, keepRegex, stripRegex, testCase.before) + labels := transform.StripLabels(testCase.keep, testCase.strip, keepRegex, stripRegex, testCase.before) if !reflect.DeepEqual(labels, testCase.after) { t.Errorf("StripLables failed, expected %v, got %v", testCase.after, labels) }