Retry delay: Update flag, vars and docs to clarify

BACKSTORY

As I went to start work towards implementing email notification
support per GH-3, I was struck by how unclear the arguments to the
`emailNotifier` function (and similarly, the `teamsNotifier` function)
were. The difference between `config.NotifyMgrEmailNotificationDelay`
and `cfg.EmailNotificationDelay()` were not immediately clear and it
took longer than I'd like to admit to fully backtrack and confirm
not only the original intent, but the current behavior surrounding
those settings.

This commit attempts to clarify not only the original intent, but the
purpose behind each related portion of code surrounding what I now
understand to be handling the rate limit (formerly named
`config.NotifyMgrEmailNotificationDelay`) and notification retry
delay.

CHANGES

- documentation
  - README.md
    - update "feature" entry to emphasize configurable retries, but
      hard-coded notifications rate limit (hopefully soon to change)
  - configure.md
    - updated description
    - updated command-line flag name
    - updated environment variable name
    - updated help text/description for flag
  - doc.go
    - explicitly note that it is the retry delay that is configurable

- configuration file
  - rename `delay` to `retry_delay`
  - add missing doc comments for each msteams section setting

- code
  - search/replace across codebase to replace `Delay` with `RetryDelay`
    - this includes renaming the `Delay` field for the `MSTeams` type,
      and its associated flag, environment variable and TOML config
      file setting
  - replace `sendDelay` parameter with `sendRateLimit` to better
    communicate what the incoming value to `teamsNotifier` and
    `emailNotifier` is intended for

  - doc comments
    - fixes to correct wrong information
    - fixes to better communicate the intent and real behavior

REFERENCES

- refs GH-85

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)