From 49257ec296c4529d4a69b20695d7fdc9fa451514 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Fri, 15 Mar 2019 10:49:33 +0000 Subject: [PATCH] fix(api): correctly deduplicate silences Right now deduplication only works when there's a single silence for the entire group, but not if the group is silences with multiple silences. This change fixes the logic so that we look at silences covering the entire group and move those into shared namespace --- internal/models/api.go | 47 +++++++++++++++------------- internal/models/api_test.go | 61 ++++++++++++++++++++++++++++++++----- 2 files changed, 79 insertions(+), 29 deletions(-) diff --git a/internal/models/api.go b/internal/models/api.go index ff2d40ba3..1805da55d 100644 --- a/internal/models/api.go +++ b/internal/models/api.go @@ -36,9 +36,9 @@ type LabelsColorMap map[string]map[string]LabelColors // APIAlertGroupSharedMaps defines shared part of APIAlertGroup type APIAlertGroupSharedMaps struct { - Annotations Annotations `json:"annotations"` - Labels map[string]string `json:"labels"` - Silences map[string]string `json:"silences"` + Annotations Annotations `json:"annotations"` + Labels map[string]string `json:"labels"` + Silences map[string][]string `json:"silences"` } // APIAlertGroup is how AlertGroup is returned in the API response @@ -140,42 +140,47 @@ func (ag *APIAlertGroup) dedupAnnotations() { } func (ag *APIAlertGroup) dedupSilences() { - ag.Shared.Silences = map[string]string{} + ag.Shared.Silences = map[string][]string{} - silencesByCluster := map[string][]string{} + silencesByCluster := map[string]map[string]int{} for _, alert := range ag.Alerts { if alert.State != AlertStateSuppressed { // if we find any alert that's not silenced then we can break early return } + // process each cluster only once, rather than each alertmanager instance + clusters := []string{} for _, am := range alert.Alertmanager { + if slices.StringInSlice(clusters, am.Cluster) { + continue + } + clusters = append(clusters, am.Cluster) for _, silenceID := range am.SilencedBy { _, ok := silencesByCluster[am.Cluster] if !ok { - silencesByCluster[am.Cluster] = []string{} + silencesByCluster[am.Cluster] = map[string]int{} } - if !slices.StringInSlice(silencesByCluster[am.Cluster], silenceID) { - silencesByCluster[am.Cluster] = append(silencesByCluster[am.Cluster], silenceID) + _, ok = silencesByCluster[am.Cluster][silenceID] + if !ok { + silencesByCluster[am.Cluster][silenceID] = 0 } + silencesByCluster[am.Cluster][silenceID]++ } } } - // only deduplicate if all alerts are silenced with the same silence from a - // single cluster - if len(silencesByCluster) != 1 { - return - } - - // now check that all alerts are silenced with the same silenceID - for cluster, silences := range silencesByCluster { - for _, silenceID := range silences { - if silenceID != silences[0] { - return + totalAlerts := len(ag.Alerts) + for cluster, silenceCountMap := range silencesByCluster { + for silenceID, affectedAlertsCount := range silenceCountMap { + if affectedAlertsCount == totalAlerts { + _, ok := ag.Shared.Silences[cluster] + if !ok { + ag.Shared.Silences[cluster] = []string{} + } + ag.Shared.Silences[cluster] = append(ag.Shared.Silences[cluster], silenceID) } } - ag.Shared.Silences[cluster] = silences[0] } } @@ -193,7 +198,7 @@ func (ag *APIAlertGroup) DedupSharedMaps() { ag.Shared = APIAlertGroupSharedMaps{ Labels: map[string]string{}, Annotations: Annotations{}, - Silences: map[string]string{}, + Silences: map[string][]string{}, } } } diff --git a/internal/models/api_test.go b/internal/models/api_test.go index ef42a2d71..8e1797798 100644 --- a/internal/models/api_test.go +++ b/internal/models/api_test.go @@ -12,7 +12,7 @@ import ( func TestDedupSharedMaps(t *testing.T) { am := models.AlertmanagerInstance{ Cluster: "fakeCluster", - SilencedBy: []string{"fakeSilenceID"}, + SilencedBy: []string{"fakeSilence1", "fakeSilence2"}, } ag := models.APIAlertGroup{ AlertGroup: models.AlertGroup{ @@ -37,7 +37,7 @@ func TestDedupSharedMaps(t *testing.T) { "job": "node_exporter", "instance": "1", }, - Alertmanager: []models.AlertmanagerInstance{am}, + Alertmanager: []models.AlertmanagerInstance{am, am}, }, models.Alert{ State: models.AlertStateSuppressed, @@ -52,7 +52,7 @@ func TestDedupSharedMaps(t *testing.T) { "job": "node_exporter", "instance": "2", }, - Alertmanager: []models.AlertmanagerInstance{am}, + Alertmanager: []models.AlertmanagerInstance{am, am}, }, models.Alert{ State: models.AlertStateSuppressed, @@ -67,7 +67,7 @@ func TestDedupSharedMaps(t *testing.T) { "job": "blackbox", "instance": "3", }, - Alertmanager: []models.AlertmanagerInstance{am}, + Alertmanager: []models.AlertmanagerInstance{am, am}, }, }, }, @@ -105,7 +105,21 @@ func TestDedupSharedMaps(t *testing.T) { "endsAt": "0001-01-01T00:00:00Z", "source": "", "silencedBy": [ - "fakeSilenceID" + "fakeSilence1", + "fakeSilence2" + ], + "inhibitedBy": null + }, + { + "name": "", + "cluster": "fakeCluster", + "state": "", + "startsAt": "0001-01-01T00:00:00Z", + "endsAt": "0001-01-01T00:00:00Z", + "source": "", + "silencedBy": [ + "fakeSilence1", + "fakeSilence2" ], "inhibitedBy": null } @@ -130,7 +144,21 @@ func TestDedupSharedMaps(t *testing.T) { "endsAt": "0001-01-01T00:00:00Z", "source": "", "silencedBy": [ - "fakeSilenceID" + "fakeSilence1", + "fakeSilence2" + ], + "inhibitedBy": null + }, + { + "name": "", + "cluster": "fakeCluster", + "state": "", + "startsAt": "0001-01-01T00:00:00Z", + "endsAt": "0001-01-01T00:00:00Z", + "source": "", + "silencedBy": [ + "fakeSilence1", + "fakeSilence2" ], "inhibitedBy": null } @@ -155,7 +183,21 @@ func TestDedupSharedMaps(t *testing.T) { "endsAt": "0001-01-01T00:00:00Z", "source": "", "silencedBy": [ - "fakeSilenceID" + "fakeSilence1", + "fakeSilence2" + ], + "inhibitedBy": null + }, + { + "name": "", + "cluster": "fakeCluster", + "state": "", + "startsAt": "0001-01-01T00:00:00Z", + "endsAt": "0001-01-01T00:00:00Z", + "source": "", + "silencedBy": [ + "fakeSilence1", + "fakeSilence2" ], "inhibitedBy": null } @@ -178,7 +220,10 @@ func TestDedupSharedMaps(t *testing.T) { ], "labels": {}, "silences": { - "fakeCluster": "fakeSilenceID" + "fakeCluster": [ + "fakeSilence1", + "fakeSilence2" + ] } } }`