From cfb909f419fa728552cdf04711151260e85c76ed Mon Sep 17 00:00:00 2001 From: gotjosh Date: Tue, 14 Jun 2022 14:40:22 +0100 Subject: [PATCH 1/2] Marker: Rename `SetSilenced` to `SetActiveOrSilenced` This accurately reflects what the function _actually_ does. If no active silences IDs are provided and the list of inhibitions we have is already empty the alert is actually set to Active. Took me a while to realise this as I was understanding how do we populate the alert list. Signed-off-by: gotjosh --- notify/notify_test.go | 2 +- provider/mem/mem_test.go | 2 +- silence/silence.go | 4 ++-- types/types.go | 12 ++++++------ 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/notify/notify_test.go b/notify/notify_test.go index a78fbf74f6..5beaf18242 100644 --- a/notify/notify_test.go +++ b/notify/notify_test.go @@ -673,7 +673,7 @@ func TestMuteStageWithSilences(t *testing.T) { // Set the second alert as previously silenced with an old version // number. This is expected to get unsilenced by the stage. - marker.SetSilenced(inAlerts[1].Fingerprint(), 0, []string{"123"}, nil) + marker.SetActiveOrSilenced(inAlerts[1].Fingerprint(), 0, []string{"123"}, nil) _, alerts, err := stage.Exec(context.Background(), log.NewNopLogger(), inAlerts...) if err != nil { diff --git a/provider/mem/mem_test.go b/provider/mem/mem_test.go index 038a3fea46..1d396cd248 100644 --- a/provider/mem/mem_test.go +++ b/provider/mem/mem_test.go @@ -297,7 +297,7 @@ func TestAlertsGC(t *testing.T) { } for _, a := range insert { - marker.SetSilenced(a.Fingerprint(), 0, nil, nil) + marker.SetActiveOrSilenced(a.Fingerprint(), 0, nil, nil) marker.SetInhibited(a.Fingerprint()) if !marker.Active(a.Fingerprint()) { t.Errorf("error setting status: %v", a) diff --git a/silence/silence.go b/silence/silence.go index 408b628230..7372a36411 100644 --- a/silence/silence.go +++ b/silence/silence.go @@ -154,7 +154,7 @@ func (s *Silencer) Mutes(lset model.LabelSet) bool { } if len(allSils) == 0 { // Easy case, neither active nor pending silences anymore. - s.marker.SetSilenced(fp, newVersion, nil, nil) + s.marker.SetActiveOrSilenced(fp, newVersion, nil, nil) return false } // It is still possible that nothing has changed, but finding out is not @@ -183,7 +183,7 @@ func (s *Silencer) Mutes(lset model.LabelSet) bool { sort.Strings(activeIDs) sort.Strings(pendingIDs) - s.marker.SetSilenced(fp, newVersion, activeIDs, pendingIDs) + s.marker.SetActiveOrSilenced(fp, newVersion, activeIDs, pendingIDs) return len(activeIDs) > 0 } diff --git a/types/types.go b/types/types.go index ce45301d5b..f3101f8234 100644 --- a/types/types.go +++ b/types/types.go @@ -53,18 +53,18 @@ type AlertStatus struct { // Marker helps to mark alerts as silenced and/or inhibited. // All methods are goroutine-safe. type Marker interface { - // SetSilenced replaces the previous SilencedBy by the provided IDs of + // SetActiveOrSilenced replaces the previous SilencedBy by the provided IDs of // active and pending silences, including the version number of the // silences state. The set of provided IDs is supposed to represent the // complete set of relevant silences. If no active silence IDs are provided and // InhibitedBy is already empty, it sets the provided alert to AlertStateActive. // Otherwise, it sets the provided alert to AlertStateSuppressed. - SetSilenced(alert model.Fingerprint, version int, activeSilenceIDs, pendingSilenceIDs []string) + SetActiveOrSilenced(alert model.Fingerprint, version int, activeSilenceIDs, pendingSilenceIDs []string) // SetInhibited replaces the previous InhibitedBy by the provided IDs of - // alerts. In contrast to SetSilenced, the set of provided IDs is not + // alerts. In contrast to SetActiveOrSilenced, the set of provided IDs is not // expected to represent the complete set of inhibiting alerts. (In // practice, this method is only called with one or zero IDs. However, - // this expectation might change in the future.) If no IDs are provided + // this expectation might change in the future. If no IDs are provided // and InhibitedBy is already empty, it sets the provided alert to // AlertStateActive. Otherwise, it sets the provided alert to // AlertStateSuppressed. @@ -150,8 +150,8 @@ func (m *memMarker) Count(states ...AlertState) int { return count } -// SetSilenced implements Marker. -func (m *memMarker) SetSilenced(alert model.Fingerprint, version int, activeIDs, pendingIDs []string) { +// SetActiveOrSilenced implements Marker. +func (m *memMarker) SetActiveOrSilenced(alert model.Fingerprint, version int, activeIDs, pendingIDs []string) { m.mtx.Lock() defer m.mtx.Unlock() From f66bbab4217da4df2fce1a2bed67ff32c173ece0 Mon Sep 17 00:00:00 2001 From: gotjosh Date: Fri, 17 Jun 2022 13:13:05 +0100 Subject: [PATCH 2/2] Fix tests after rebase Signed-off-by: gotjosh --- provider/mem/mem_test.go | 2 +- types/types_test.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/provider/mem/mem_test.go b/provider/mem/mem_test.go index 1d396cd248..a45321844c 100644 --- a/provider/mem/mem_test.go +++ b/provider/mem/mem_test.go @@ -441,7 +441,7 @@ func TestAlerts_Count(t *testing.T) { // When insert an alert, and then silence it. It shows up with the correct filter. alerts.Put(a2) - marker.SetSilenced(a2.Fingerprint(), 1, []string{"1"}, nil) + marker.SetActiveOrSilenced(a2.Fingerprint(), 1, []string{"1"}, nil) require.Equal(t, 1, countByState(types.AlertStateSuppressed)) require.Equal(t, 1, countTotal()) diff --git a/types/types_test.go b/types/types_test.go index edb6969776..04ed3e19e1 100644 --- a/types/types_test.go +++ b/types/types_test.go @@ -62,17 +62,17 @@ func TestMemMarker_Count(t *testing.T) { } // Insert an active alert. - marker.SetSilenced(a1.Fingerprint(), 1, nil, nil) + marker.SetActiveOrSilenced(a1.Fingerprint(), 1, nil, nil) require.Equal(t, 1, countByState(AlertStateActive)) require.Equal(t, 1, countTotal()) // Insert a suppressed alert. - marker.SetSilenced(a2.Fingerprint(), 1, []string{"1"}, nil) + marker.SetActiveOrSilenced(a2.Fingerprint(), 1, []string{"1"}, nil) require.Equal(t, 1, countByState(AlertStateSuppressed)) require.Equal(t, 2, countTotal()) // Insert a resolved alert - it'll count as active. - marker.SetSilenced(a3.Fingerprint(), 1, []string{"1"}, nil) + marker.SetActiveOrSilenced(a3.Fingerprint(), 1, []string{"1"}, nil) require.Equal(t, 1, countByState(AlertStateActive)) require.Equal(t, 3, countTotal()) }