From acb58400fd2b324dd4cc56110c8e891e8b1c0420 Mon Sep 17 00:00:00 2001 From: gotjosh Date: Fri, 13 Oct 2023 14:15:05 +0100 Subject: [PATCH] Refactor: Move `inTimeIntervals` from `notify` to `timeinterval` (#3556) * Refactor: Move `inTimeIntervals` from `notify` to `timeinterval` There's absolutely no change of functionality here and I've expanded coverage for similar logic in both places. --------- Signed-off-by: gotjosh --- cmd/alertmanager/main.go | 4 +- notify/notify.go | 43 ++++++-------- notify/notify_test.go | 6 +- timeinterval/timeinterval.go | 32 ++++++++++- timeinterval/timeinterval_test.go | 95 +++++++++++++++++++++++++++++++ types/types.go | 5 ++ 6 files changed, 152 insertions(+), 33 deletions(-) diff --git a/cmd/alertmanager/main.go b/cmd/alertmanager/main.go index 3dc48f9f4d..2d4a58e254 100644 --- a/cmd/alertmanager/main.go +++ b/cmd/alertmanager/main.go @@ -478,6 +478,8 @@ func run() int { timeIntervals[ti.Name] = ti.TimeIntervals } + intervener := timeinterval.NewIntervener(timeIntervals) + inhibitor.Stop() disp.Stop() @@ -497,7 +499,7 @@ func run() int { waitFunc, inhibitor, silencer, - timeIntervals, + intervener, notificationLog, pipelinePeer, ) diff --git a/notify/notify.go b/notify/notify.go index 11ddfba536..2c9f768a36 100644 --- a/notify/notify.go +++ b/notify/notify.go @@ -374,7 +374,7 @@ func (pb *PipelineBuilder) New( wait func() time.Duration, inhibitor *inhibit.Inhibitor, silencer *silence.Silencer, - times map[string][]timeinterval.TimeInterval, + intervener *timeinterval.Intervener, notificationLog NotificationLog, peer Peer, ) RoutingStage { @@ -382,8 +382,8 @@ func (pb *PipelineBuilder) New( ms := NewGossipSettleStage(peer) is := NewMuteStage(inhibitor) - tas := NewTimeActiveStage(times) - tms := NewTimeMuteStage(times) + tas := NewTimeActiveStage(intervener) + tms := NewTimeMuteStage(intervener) ss := NewMuteStage(silencer) for name := range receivers { @@ -868,13 +868,13 @@ func (n SetNotifiesStage) Exec(ctx context.Context, l log.Logger, alerts ...*typ } type timeStage struct { - Times map[string][]timeinterval.TimeInterval + muter types.TimeMuter } type TimeMuteStage timeStage -func NewTimeMuteStage(ti map[string][]timeinterval.TimeInterval) *TimeMuteStage { - return &TimeMuteStage{ti} +func NewTimeMuteStage(m types.TimeMuter) *TimeMuteStage { + return &TimeMuteStage{m} } // Exec implements the stage interface for TimeMuteStage. @@ -889,7 +889,12 @@ func (tms TimeMuteStage) Exec(ctx context.Context, l log.Logger, alerts ...*type return ctx, alerts, errors.New("missing now timestamp") } - muted, err := inTimeIntervals(now, tms.Times, muteTimeIntervalNames) + // Skip this stage if there are no mute timings. + if len(muteTimeIntervalNames) == 0 { + return ctx, alerts, nil + } + + muted, err := tms.muter.Mutes(muteTimeIntervalNames, now) if err != nil { return ctx, alerts, err } @@ -904,8 +909,8 @@ func (tms TimeMuteStage) Exec(ctx context.Context, l log.Logger, alerts ...*type type TimeActiveStage timeStage -func NewTimeActiveStage(ti map[string][]timeinterval.TimeInterval) *TimeActiveStage { - return &TimeActiveStage{ti} +func NewTimeActiveStage(m types.TimeMuter) *TimeActiveStage { + return &TimeActiveStage{m} } // Exec implements the stage interface for TimeActiveStage. @@ -926,32 +931,16 @@ func (tas TimeActiveStage) Exec(ctx context.Context, l log.Logger, alerts ...*ty return ctx, alerts, errors.New("missing now timestamp") } - active, err := inTimeIntervals(now, tas.Times, activeTimeIntervalNames) + muted, err := tas.muter.Mutes(activeTimeIntervalNames, now) if err != nil { return ctx, alerts, err } // If the current time is not inside an active time, all alerts are removed from the pipeline - if !active { + if !muted { level.Debug(l).Log("msg", "Notifications not sent, route is not within active time") return ctx, nil, nil } return ctx, alerts, nil } - -// inTimeIntervals returns true if the current time is contained in one of the given time intervals. -func inTimeIntervals(now time.Time, intervals map[string][]timeinterval.TimeInterval, intervalNames []string) (bool, error) { - for _, name := range intervalNames { - interval, ok := intervals[name] - if !ok { - return false, errors.Errorf("time interval %s doesn't exist in config", name) - } - for _, ti := range interval { - if ti.ContainsTime(now.UTC()) { - return true, nil - } - } - } - return false, nil -} diff --git a/notify/notify_test.go b/notify/notify_test.go index 810131775d..d3eeb4670a 100644 --- a/notify/notify_test.go +++ b/notify/notify_test.go @@ -840,7 +840,8 @@ func TestTimeMuteStage(t *testing.T) { t.Fatalf("Couldn't unmarshal time interval %s", err) } m := map[string][]timeinterval.TimeInterval{"test": intervals} - stage := NewTimeMuteStage(m) + intervener := timeinterval.NewIntervener(m) + stage := NewTimeMuteStage(intervener) outAlerts := []*types.Alert{} nonMuteCount := 0 @@ -924,7 +925,8 @@ func TestTimeActiveStage(t *testing.T) { t.Fatalf("Couldn't unmarshal time interval %s", err) } m := map[string][]timeinterval.TimeInterval{"test": intervals} - stage := NewTimeActiveStage(m) + intervener := timeinterval.NewIntervener(m) + stage := NewTimeActiveStage(intervener) outAlerts := []*types.Alert{} nonMuteCount := 0 diff --git a/timeinterval/timeinterval.go b/timeinterval/timeinterval.go index a5018aaef9..fe8c97d729 100644 --- a/timeinterval/timeinterval.go +++ b/timeinterval/timeinterval.go @@ -27,6 +27,35 @@ import ( "gopkg.in/yaml.v2" ) +// Intervener determines whether a given time and active route time interval should mute outgoing notifications. +// It implements the TimeMuter interface. +type Intervener struct { + intervals map[string][]TimeInterval +} + +func (i *Intervener) Mutes(names []string, now time.Time) (bool, error) { + for _, name := range names { + interval, ok := i.intervals[name] + if !ok { + return false, fmt.Errorf("time interval %s doesn't exist in config", name) + } + + for _, ti := range interval { + if ti.ContainsTime(now.UTC()) { + return true, nil + } + } + } + + return false, nil +} + +func NewIntervener(ti map[string][]TimeInterval) *Intervener { + return &Intervener{ + intervals: ti, + } +} + // TimeInterval describes intervals of time. ContainsTime will tell you if a golang time is contained // within the interval. type TimeInterval struct { @@ -436,9 +465,6 @@ func (ir InclusiveRange) MarshalYAML() (interface{}, error) { return string(bytes), err } -// TimeLayout specifies the layout to be used in time.Parse() calls for time intervals. -const TimeLayout = "15:04" - var ( validTime = "^((([01][0-9])|(2[0-3])):[0-5][0-9])$|(^24:00$)" validTimeRE = regexp.MustCompile(validTime) diff --git a/timeinterval/timeinterval_test.go b/timeinterval/timeinterval_test.go index 9c3a2c1d87..8377ac119f 100644 --- a/timeinterval/timeinterval_test.go +++ b/timeinterval/timeinterval_test.go @@ -19,6 +19,7 @@ import ( "testing" "time" + "github.com/stretchr/testify/require" "gopkg.in/yaml.v2" ) @@ -659,3 +660,97 @@ func mustLoadLocation(name string) *time.Location { } return loc } + +func TestIntervener_Mutes(t *testing.T) { + // muteIn mutes alerts outside business hours in November, using the +1100 timezone. + muteIn := ` +--- +- weekdays: + - monday:friday + location: Australia/Sydney + months: + - November + times: + - start_time: 00:00 + end_time: 09:00 + - start_time: 17:00 + end_time: 24:00 +- weekdays: + - saturday + - sunday + months: + - November + location: 'Australia/Sydney' +` + intervalName := "test" + var intervals []TimeInterval + err := yaml.Unmarshal([]byte(muteIn), &intervals) + require.NoError(t, err) + m := map[string][]TimeInterval{intervalName: intervals} + + tc := []struct { + name string + firedAt string + expected bool + err error + }{ + { + name: "Should not mute on Friday during business hours", + firedAt: "19 Nov 21 13:00 +1100", + expected: false, + }, + { + name: "Should not mute on a Tuesday before 5pm", + firedAt: "16 Nov 21 16:59 +1100", + expected: false, + }, + { + name: "Should mute on a Saturday", + firedAt: "20 Nov 21 10:00 +1100", + expected: true, + }, + { + name: "Should mute before 9am on a Wednesday", + firedAt: "17 Nov 21 05:00 +1100", + expected: true, + }, + { + name: "Should mute even if we are in a different timezone (KST)", + firedAt: "14 Nov 21 20:00 +0900", + expected: true, + }, + { + name: "Should mute even if the timezone is UTC", + firedAt: "14 Nov 21 21:30 +0000", + expected: true, + }, + { + name: "Should not mute different timezone (KST)", + firedAt: "15 Nov 22 14:30 +0900", + expected: false, + }, + { + name: "Should mute in a different timezone (PET)", + firedAt: "15 Nov 21 02:00 -0500", + expected: true, + }, + } + + for _, tt := range tc { + t.Run(tt.name, func(t *testing.T) { + now, err := time.Parse(time.RFC822Z, tt.firedAt) + require.NoError(t, err) + + intervener := NewIntervener(m) + + expected, err := intervener.Mutes([]string{intervalName}, now) + if err != nil { + require.Error(t, tt.err) + require.False(t, tt.expected) + } + + require.NoError(t, err) + require.Equal(t, expected, tt.expected) + }) + } +} diff --git a/types/types.go b/types/types.go index f3101f8234..b427a3d1d3 100644 --- a/types/types.go +++ b/types/types.go @@ -381,6 +381,11 @@ type Muter interface { Mutes(model.LabelSet) bool } +// TimeMuter determines if alerts should be muted based on the specified current time and active time interval on the route. +type TimeMuter interface { + Mutes(timeIntervalName []string, now time.Time) (bool, error) +} + // A MuteFunc is a function that implements the Muter interface. type MuteFunc func(model.LabelSet) bool