Merge pull request #87 from atc0005/i85-notification-delay-explanatio…

…n-tweaks

Retry delay: Update flag, vars and docs to clarify

README.md

@@ -114,8 +114,8 @@ See also:
     - ignored user
     - ignored IP Address
     - error occurred
-  - configurable retries
-  - configurable notifications delay in order to respect remote API limits
+  - configurable retries and delay between retries
+  - hard-coded notifications rate limit in order to respect remote API limits
 
 - Logging
   - Payload receipt from monitoring system

cmd/brick/notify.go

@@ -341,7 +341,7 @@ func teamsNotifier(
 	ctx context.Context,
 	webhookURL string,
 	sendTimeout time.Duration,
-	sendDelay time.Duration,
+	sendRateLimit time.Duration,
 	retries int,
 	retriesDelay int,
 	incoming <-chan events.Record,
@@ -355,9 +355,10 @@ func teamsNotifier(
 	ourResultQueue := make(chan NotifyResult)
 
 	// Setup new scheduler that we can use to add an intentional delay between
-	// Microsoft Teams notification attempts
+	// Microsoft Teams notification attempts. This delay is added in order to
+	// rate limit our outgoing messages to comply with remote API limits.
 	// https://docs.microsoft.com/en-us/microsoftteams/platform/webhooks-and-connectors/how-to/connectors-using
-	notifyScheduler := newNotifyScheduler(sendDelay)
+	notifyScheduler := newNotifyScheduler(sendRateLimit)
 
 	for {
 
@@ -388,7 +389,6 @@ func teamsNotifier(
 				time.Now(), record)
 
 			log.Debug("Calculating next scheduled notification")
-
 			nextScheduledNotification := notifyScheduler()
 
 			log.Debugf("Now: %v, Next scheduled notification: %v",
@@ -468,7 +468,7 @@ func teamsNotifier(
 func emailNotifier(
 	ctx context.Context,
 	sendTimeout time.Duration,
-	sendDelay time.Duration,
+	sendRateLimit time.Duration,
 	retries int,
 	retriesDelay int,
 	incoming <-chan events.Record,
@@ -482,8 +482,10 @@ func emailNotifier(
 	ourResultQueue := make(chan NotifyResult)
 
 	// Setup new scheduler that we can use to add an intentional delay between
-	// email notification attempts
-	notifyScheduler := newNotifyScheduler(sendDelay)
+	// email notification attempts. This delay is added in order to rate limit
+	// our outgoing messages to comply with any destination email server
+	// limits.
+	notifyScheduler := newNotifyScheduler(sendRateLimit)
 
 	for {
 
@@ -627,10 +629,10 @@ func NotifyMgr(ctx context.Context, cfg *config.Config, notifyWorkQueue <-chan e
 		go teamsNotifier(
 			ctx,
 			cfg.TeamsWebhookURL(),
-			config.NotifyMgrTeamsTimeout,
-			config.NotifyMgrTeamsNotificationDelay,
+			config.NotifyMgrTeamsNotificationTimeout,
+			config.NotifyMgrTeamsNotificationRateLimit,
 			cfg.TeamsNotificationRetries(),
-			cfg.TeamsNotificationDelay(),
+			cfg.TeamsNotificationRetryDelay(),
 			teamsNotifyWorkQueue,
 			teamsNotifyResultQueue,
 			teamsNotifyDone,
@@ -644,10 +646,10 @@ func NotifyMgr(ctx context.Context, cfg *config.Config, notifyWorkQueue <-chan e
 		log.Debug("NotifyMgr: Starting up emailNotifier")
 		go emailNotifier(
 			ctx,
-			config.NotifyMgrEmailTimeout,
-			config.NotifyMgrEmailNotificationDelay,
+			config.NotifyMgrEmailNotificationTimeout,
+			config.NotifyMgrEmailNotificationRateLimit,
 			cfg.EmailNotificationRetries(),
-			cfg.EmailNotificationDelay(),
+			cfg.EmailNotificationRetryDelay(),
 			emailNotifyWorkQueue,
 			emailNotifyResultQueue,
 			emailNotifyDone,

config/config.go

@@ -50,7 +50,7 @@ func (c *Config) String() string {
 			"IgnoredIPAddressesFile: [%q %t],"+
 			"TeamsWebhookURL: %q,"+
 			"TeamsNotificationRetries: %q,"+
-			"TeamsNotificationDelay: %q,"+
+			"TeamsNotificationRetryDelay: %q,"+
 			"ConfigFile: %q}",
 		c.LocalTCPPort(),
 		c.LocalIPAddress(),
@@ -67,7 +67,7 @@ func (c *Config) String() string {
 		c.IsSetIgnoredIPAddressesFile(),
 		c.TeamsWebhookURL(),
 		c.TeamsNotificationRetries(),
-		c.TeamsNotificationDelay(),
+		c.TeamsNotificationRetryDelay(),
 		c.ConfigFile(),
 	)
 }
@@ -87,17 +87,24 @@ func (c configTemplate) Description() string {
 	return MyAppDescription
 }
 
-// GetNotificationTimeout accepts the next scheduled notification, the number
-// of message submission retries and the delay between each attempt and
-// returns the timeout value for the entire message submission process,
-// including the initial attempt and all retry attempts.
+// GetNotificationTimeout calculates the timeout value for the entire message
+// submission process, including the initial attempt and all retry attempts.
 //
 // This overall timeout value is computed using multiple values; (1) the base
 // timeout value for a single message submission attempt, (2) the next
 // scheduled notification (which was created using the configured delay we
 // wish to force between message submission attempts), (3) the total number of
-// retries allowed, (4) the delay between retry attempts
-func GetNotificationTimeout(baseTimeout time.Duration, schedule time.Time, retries int, retriesDelay int) time.Duration {
+// retries allowed, (4) the delay between each retry attempt.
+//
+// This computed timeout value is intended to be used to cancel a notification
+// attempt once it reaches this timeout threshold.
+//
+func GetNotificationTimeout(
+	baseTimeout time.Duration,
+	schedule time.Time,
+	retries int,
+	retriesDelay int,
+) time.Duration {
 
 	timeoutValue := (baseTimeout + time.Until(schedule)) +
 		(time.Duration(retriesDelay) * time.Duration(retries))

config/constants.go

@@ -69,7 +69,7 @@ const (
 
 	// the number of seconds to wait between retry attempts; applies to
 	// Microsoft Teams notifications only
-	defaultMSTeamsDelay int = 5
+	defaultMSTeamsRetryDelay int = 5
 
 	// this is based on the official installation instructions
 	defaultEZproxyExecutablePath string = "/usr/local/ezproxy/ezproxy"
@@ -119,21 +119,21 @@ const NotifyMgrServicesShutdownTimeout time.Duration = 2 * time.Second
 // and child goroutines to concurrently process notification requests.
 const (
 
-	// NotifyMgrTeamsTimeout is the timeout setting applied to each Microsoft
-	// Teams notification attempt. This value does NOT take into account the
-	// number of configured retries and retry delays. The final value timeout
-	// applied to each notification attempt should be based on those
-	// calculations. The GetTimeout method does just that.
-	NotifyMgrTeamsTimeout time.Duration = 10 * time.Second
-
-	// NotifyMgrTeamsSendAttemptTimeout
-
-	// NotifyMgrEmailTimeout is the timeout setting applied to each email
-	// notification attempt. This value does NOT take into account the number
-	// of configured retries and retry delays. The final value timeout applied
-	// to each notification attempt should be based on those calculations. The
-	// GetTimeout method does just that.
-	NotifyMgrEmailTimeout time.Duration = 30 * time.Second
+	// NotifyMgrTeamsNotificationTimeout is the timeout setting applied to
+	// each Microsoft Teams notification attempt. This value does NOT take
+	// into account the number of configured retries, retry delays or overall
+	// rate limit for Teams notifications. The final timeout value applied to
+	// each notification attempt should be based on those calculations and
+	// this value. The GetNotificationTimeout method does just that.
+	NotifyMgrTeamsNotificationTimeout time.Duration = 10 * time.Second
+
+	// NotifyMgrEmailNotificationTimeout is the timeout setting applied to
+	// each email notification attempt. This value does NOT take into account
+	// the number of configured retries, retry delays or overall rate limit
+	// for email notifications. The final timeout value applied to each
+	// notification attempt should be based on those calculations and this
+	// value. The GetNotificationTimeout method does just that.
+	NotifyMgrEmailNotificationTimeout time.Duration = 30 * time.Second
 
 	// NotifyStatsMonitorDelay limits notification stats logging to no more
 	// often than this duration. This limiter is to keep from logging the
@@ -145,15 +145,15 @@ const (
 	// details so often that the information simply becomes noise.
 	NotifyQueueMonitorDelay time.Duration = 15 * time.Second
 
-	// NotifyMgrTeamsNotificationDelay is the delay between Microsoft Teams
-	// notification attempts. This delay is intended to help prevent
+	// NotifyMgrTeamsNotificationRateLimit is the rate limit between Microsoft
+	// Teams notification attempts. This limit is intended to help prevent
 	// unintentional abuse of remote services.
-	NotifyMgrTeamsNotificationDelay time.Duration = 5 * time.Second
+	NotifyMgrTeamsNotificationRateLimit time.Duration = 5 * time.Second
 
-	// NotifyMgrEmailNotificationDelay is the delay between email notification
-	// attempts. This delay is intended to help prevent unintentional abuse of
-	// remote services.
-	NotifyMgrEmailNotificationDelay time.Duration = 5 * time.Second
+	// NotifyMgrEmailNotificationRateLimit is the rate limit between email
+	// notification attempts. This limit is intended to help prevent
+	// unintentional abuse of remote services.
+	NotifyMgrEmailNotificationRateLimit time.Duration = 5 * time.Second
 )
 
 // NotifyMgrQueueDepth is the number of items allowed into the queue/channel

config/getters.go

@@ -301,19 +301,18 @@ func (c Config) TeamsNotificationRetries() int {
 	}
 }
 
-// TeamsNotificationDelay returns the user-provided delay for Microsoft Teams
-// notifications or the default value if not provided. CLI flag values take
-// precedence if provided. This delay is added regardless of whether a
-// previous notification delivery attempt has been made.
-func (c Config) TeamsNotificationDelay() int {
+// TeamsNotificationRetryDelay returns the user-provided delay between retry
+// attempts for Microsoft Teams notifications or the default value if not
+// provided. CLI flag values take precedence if provided.
+func (c Config) TeamsNotificationRetryDelay() int {
 
 	switch {
-	case c.cliConfig.MSTeams.Delay != nil:
-		return *c.cliConfig.MSTeams.Delay
-	case c.fileConfig.MSTeams.Delay != nil:
-		return *c.fileConfig.MSTeams.Delay
+	case c.cliConfig.MSTeams.RetryDelay != nil:
+		return *c.cliConfig.MSTeams.RetryDelay
+	case c.fileConfig.MSTeams.RetryDelay != nil:
+		return *c.fileConfig.MSTeams.RetryDelay
 	default:
-		return defaultMSTeamsDelay
+		return defaultMSTeamsRetryDelay
 	}
 }
 
@@ -356,9 +355,8 @@ func (c Config) EmailNotificationRetries() int {
 
 // EmailNotificationRetries returns the user-provided delay for email
 // notifications or the default value if not provided. CLI flag values take
-// precedence if provided. This delay is added regardless of whether a
-// previous notification delivery attempt has been made.
-func (c Config) EmailNotificationDelay() int {
+// precedence if provided.
+func (c Config) EmailNotificationRetryDelay() int {
 
 	log.Warn("Implement this as part of GH-3")
 

config/types.go

@@ -138,9 +138,9 @@ type MSTeams struct {
 	// application.
 	WebhookURL *string `toml:"webhook_url" arg:"--teams-webhook-url,env:BRICK_MSTEAMS_WEBHOOK_URL" help:"The Webhook URL provided by a preconfigured Connector. If specified, this application will attempt to send client request details to the Microsoft Teams channel associated with the webhook URL."`
 
-	// Delay is the number of seconds to wait between Microsoft Teams message
-	// delivery attempts.
-	Delay *int `toml:"delay" arg:"--teams-notify-delay,env:BRICK_MSTEAMS_WEBHOOK_DELAY" help:"The number of seconds to wait between Microsoft Teams message delivery attempts."`
+	// RetryDelay is the number of seconds to wait between Microsoft Teams
+	// message delivery retry attempts.
+	RetryDelay *int `toml:"retry_delay" arg:"--teams-notify-retry-delay,env:BRICK_MSTEAMS_WEBHOOK_RETRY_DELAY" help:"The number of seconds to wait between Microsoft Teams message delivery retry attempts."`
 
 	// Retries is the number of attempts that this application will make to
 	// deliver Microsoft Teams messages before giving up.

config/validate.go

@@ -153,11 +153,11 @@ func validate(c Config) error {
 
 	}
 
-	if c.TeamsNotificationDelay() < 0 {
-		log.Debugf("unsupported delay specified for MS Teams notifications: %d ", c.TeamsNotificationDelay())
+	if c.TeamsNotificationRetryDelay() < 0 {
+		log.Debugf("unsupported retry delay specified for MS Teams notifications: %d ", c.TeamsNotificationRetryDelay())
 		return fmt.Errorf(
-			"invalid delay specified for MS Teams notifications: %d",
-			c.TeamsNotificationDelay(),
+			"invalid retry delay specified for MS Teams notifications: %d",
+			c.TeamsNotificationRetryDelay(),
 		)
 	}
 

contrib/brick/config.example.toml

@@ -104,9 +104,24 @@ file_path = "/usr/local/etc/brick/ips.brick-ignored.txt"
 
 [msteams]
 
+# The full URL used to submit messages to the Teams channel. This URL is in
+# the form of
+#
+# https://outlook.office.com/webhook/xxx or
+# https://outlook.office365.com/webhook/xxx.
+#
+# This URL needs to be created in advance by adding/configuring a Webhook
+# Connector in a Microsoft Teams channel that you wish to submit messages to
+# using this application.
 webhook_url = ""
+
+# The number of attempts that this application will make to deliver Microsoft
+# Teams messages before giving up.
 retries = 2
-delay = 5
+
+# The number of seconds to wait between Microsoft Teams message delivery retry
+# attempts.
+retry_delay = 5
 
 
 # TODO: Add this in later

doc.go

@@ -54,7 +54,7 @@ FEATURES
 
 • User configurable support for ignoring specific IP Addresses (i.e., prevent disabling associated account)
 
-• Microsoft Teams notifications generated for multiple events with configurable retries and notification delay
+• Microsoft Teams notifications generated for multiple events with configurable retries and notification retry delay
 
 • Logging of all events (e.g., payload receipt, action taken due to payload)