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 <[email protected]>

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,
 		)

notify/notify.go

@@ -374,16 +374,16 @@ 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 {
 	rs := make(RoutingStage, len(receivers))
 
 	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
-}

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

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)

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)
+		})
+	}
+}

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