Simplify rate limit config by using burst-size=rate limit.

Signed-off-by: Peter Štibraný <[email protected]>

Author: pstibrany

CHANGELOG.md

@@ -7,7 +7,7 @@
   - `-alertmanager.receivers-firewall.block.cidr-networks` renamed to `-alertmanager.receivers-firewall-block-cidr-networks`
   - `-alertmanager.receivers-firewall.block.private-addresses` renamed to `-alertmanager.receivers-firewall-block-private-addresses`
 * [CHANGE] Change default value of `-server.grpc.keepalive.min-time-between-pings` to `10s` and `-server.grpc.keepalive.ping-without-stream-allowed` to `true`. #4168
-* [FEATURE] Alertmanager: Added rate-limits to notifiers. Rate limits used by all integrations can be configured using `-alertmanager.notification-limits.rate-limit` and `-alertmanager.notification-limits.burst-size`, while per-integration limits can be specified via `-alertmanager.notification-limits.per-integration` parameter. Both shared and per-integration limits can be overwritten using overrides mechanism. These limits are applied on individual (per-tenant) alertmanagers. Rate-limited notifications are failed notifications. It is possible to monitor rate-limited notifications via new `cortex_alertmanager_notification_rate_limited_total` metric. #4135 #4163
+* [FEATURE] Alertmanager: Added rate-limits to notifiers. Rate limits used by all integrations can be configured using `-alertmanager.notification-rate-limit`, while per-integration rate limits can be specified via `-alertmanager.notification-rate-limit-per-integration` parameter. Both shared and per-integration limits can be overwritten using overrides mechanism. These limits are applied on individual (per-tenant) alertmanagers. Rate-limited notifications are failed notifications. It is possible to monitor rate-limited notifications via new `cortex_alertmanager_notification_rate_limited_total` metric. #4135 #4163
 * [ENHANCEMENT] Alertmanager: introduced new metrics to monitor operation when using `-alertmanager.sharding-enabled`: #4149
   * `cortex_alertmanager_state_fetch_replica_state_total`
   * `cortex_alertmanager_state_fetch_replica_state_failed_total`

docs/configuration/config-file-reference.md

@@ -26,7 +26,6 @@ To specify which configuration file to load, pass the `-config.file` flag at the
 * `<prefix>`: a CLI flag prefix based on the context (look at the parent configuration block to see which CLI flags prefix should be used)
 * `<relabel_config>`: a [Prometheus relabeling configuration](https://prometheus.io/docs/prometheus/latest/configuration/configuration/#relabel_config).
 * `<time>`: a timestamp, with available formats: `2006-01-20` (midnight, local timezone), `2006-01-20T15:04` (local timezone), and RFC 3339 formats: `2006-01-20T15:04:05Z` (UTC) or `2006-01-20T15:04:05+07:00` (explicit timezone)
-* `<limits_map>`: Map with integration names as keys, mapped to object with two fields: `rate_limit` and `burst_size`. If either of them is specified, this per-integration limit will take precedence over shared notification limit, even if other field is not specified (in which case, it defaults to 0!).
 
 ### Use environment variables in the configuration
 
@@ -4108,28 +4107,20 @@ The `limits_config` configures default and per-tenant limits imposed by Cortex s
 # CLI flag: -alertmanager.receivers-firewall-block-private-addresses
 [alertmanager_receivers_firewall_block_private_addresses: <boolean> | default = false]
 
-alertmanager_notification_limits:
-  # Per-user rate limit for sending notifications from Alertmanager in
-  # notifications/sec. 0 = rate limit disabled. Negative value = no
-  # notifications are allowed.
-  # CLI flag: -alertmanager.notification-limits.rate-limit
-  [rate_limit: <float> | default = 0]
+# Per-user rate limit for sending notifications from Alertmanager in
+# notifications/sec. 0 = rate limit disabled. Negative value = no notifications
+# are allowed.
+# CLI flag: -alertmanager.notification-rate-limit
+[alertmanager_notification_rate_limit: <float> | default = 0]
 
-  # Per-user burst size for notifications. If set to 0, no notifications will be
-  # sent, unless rate-limit is disabled, in which case all notifications are
-  # allowed.
-  # CLI flag: -alertmanager.notification-limits.burst-size
-  [burst_size: <int> | default = 1]
-
-# Per-integration notification limits. Value is a map, where each key is
-# integration name and value is an object with rate_limit and burst_size fields.
-# On command line, this map is given in JSON format. Rate limit and burst size
-# have the same meaning as -alertmanager.notification-limits.rate-limit and
-# -alertmanager.notification-limits.burst-size, but only apply for specific
+# Per-integration notification rate limits. Value is a map, where each key is
+# integration name and value is a rate-limit (float). On command line, this map
+# is given in JSON format. Rate limit has the same meaning as
+# -alertmanager.notification-rate-limit, but only applies for specific
 # integration. Allowed integration names: webhook, email, pagerduty, opsgenie,
 # wechat, slack, victorops, pushover.
-# CLI flag: -alertmanager.notification-limits.per-integration
-[alertmanager_notification_limits_per_integration: <limits_map> | default = {}]
+# CLI flag: -alertmanager.notification-rate-limit-per-integration
+[alertmanager_notification_rate_limit_per_integration: <map of string to float64> | default = {}]
 ```
 
 ### `redis_config`

docs/configuration/config-file-reference.template

@@ -26,7 +26,6 @@ To specify which configuration file to load, pass the `-config.file` flag at the
 * `<prefix>`: a CLI flag prefix based on the context (look at the parent configuration block to see which CLI flags prefix should be used)
 * `<relabel_config>`: a [Prometheus relabeling configuration](https://prometheus.io/docs/prometheus/latest/configuration/configuration/#relabel_config).
 * `<time>`: a timestamp, with available formats: `2006-01-20` (midnight, local timezone), `2006-01-20T15:04` (local timezone), and RFC 3339 formats: `2006-01-20T15:04:05Z` (UTC) or `2006-01-20T15:04:05+07:00` (explicit timezone)
-* `<limits_map>`: Map with integration names as keys, mapped to object with two fields: `rate_limit` and `burst_size`. If either of them is specified, this per-integration limit will take precedence over shared notification limit, even if other field is not specified (in which case, it defaults to 0!).
 
 ### Use environment variables in the configuration
 

pkg/util/validation/limits.go

@@ -104,13 +104,8 @@ type Limits struct {
 	AlertmanagerReceiversBlockPrivateAddresses bool                 `yaml:"alertmanager_receivers_firewall_block_private_addresses" json:"alertmanager_receivers_firewall_block_private_addresses"`
 
 	// Alertmanager limits
-	NotificationLimits            NotificationLimits    `yaml:"alertmanager_notification_limits" json:"alertmanager_notification_limits"`
-	NotificationIntegrationLimits NotificationLimitsMap `yaml:"alertmanager_notification_limits_per_integration" json:"alertmanager_notification_limits_per_integration"`
-}
-
-type NotificationLimits struct {
-	RateLimit float64 `yaml:"rate_limit" json:"rate_limit"`
-	BurstSize int     `yaml:"burst_size" json:"burst_size"`
+	NotificationRateLimit               float64                  `yaml:"alertmanager_notification_rate_limit" json:"alertmanager_notification_rate_limit"`
+	NotificationRateLimitPerIntegration NotificationRateLimitMap `yaml:"alertmanager_notification_rate_limit_per_integration" json:"alertmanager_notification_rate_limit_per_integration"`
 }
 
 // RegisterFlags adds the flags required to config this to the given FlagSet
@@ -172,13 +167,12 @@ func (l *Limits) RegisterFlags(f *flag.FlagSet) {
 	f.Var(&l.AlertmanagerReceiversBlockCIDRNetworks, "alertmanager.receivers-firewall-block-cidr-networks", "Comma-separated list of network CIDRs to block in Alertmanager receiver integrations.")
 	f.BoolVar(&l.AlertmanagerReceiversBlockPrivateAddresses, "alertmanager.receivers-firewall-block-private-addresses", false, "True to block private and local addresses in Alertmanager receiver integrations. It blocks private addresses defined by  RFC 1918 (IPv4 addresses) and RFC 4193 (IPv6 addresses), as well as loopback, local unicast and local multicast addresses.")
 
-	f.Float64Var(&l.NotificationLimits.RateLimit, "alertmanager.notification-limits.rate-limit", 0, "Per-user rate limit for sending notifications from Alertmanager in notifications/sec. 0 = rate limit disabled. Negative value = no notifications are allowed.")
-	f.IntVar(&l.NotificationLimits.BurstSize, "alertmanager.notification-limits.burst-size", 1, "Per-user burst size for notifications. If set to 0, no notifications will be sent, unless rate-limit is disabled, in which case all notifications are allowed.")
+	f.Float64Var(&l.NotificationRateLimit, "alertmanager.notification-rate-limit", 0, "Per-user rate limit for sending notifications from Alertmanager in notifications/sec. 0 = rate limit disabled. Negative value = no notifications are allowed.")
 
-	if l.NotificationIntegrationLimits == nil {
-		l.NotificationIntegrationLimits = NotificationLimitsMap{}
+	if l.NotificationRateLimitPerIntegration == nil {
+		l.NotificationRateLimitPerIntegration = NotificationRateLimitMap{}
 	}
-	f.Var(&l.NotificationIntegrationLimits, "alertmanager.notification-limits.per-integration", "Per-integration notification limits. Value is a map, where each key is integration name and value is an object with rate_limit and burst_size fields. On command line, this map is given in JSON format. Rate limit and burst size have the same meaning as -alertmanager.notification-limits.rate-limit and -alertmanager.notification-limits.burst-size, but only apply for specific integration. Allowed integration names: "+strings.Join(allowedIntegrationNames, ", ")+".")
+	f.Var(&l.NotificationRateLimitPerIntegration, "alertmanager.notification-rate-limit-per-integration", "Per-integration notification rate limits. Value is a map, where each key is integration name and value is a rate-limit (float). On command line, this map is given in JSON format. Rate limit has the same meaning as -alertmanager.notification-rate-limit, but only applies for specific integration. Allowed integration names: "+strings.Join(allowedIntegrationNames, ", ")+".")
 }
 
 // Validate the limits config and returns an error if the validation
@@ -203,7 +197,7 @@ func (l *Limits) UnmarshalYAML(unmarshal func(interface{}) error) error {
 	if defaultLimits != nil {
 		*l = *defaultLimits
 		// Make copy of default limits. Otherwise unmarshalling would modify map in default limits.
-		l.copyNotificationIntegrationLimits(defaultLimits.NotificationIntegrationLimits)
+		l.copyNotificationIntegrationLimits(defaultLimits.NotificationRateLimitPerIntegration)
 	}
 	type plain Limits
 	return unmarshal((*plain)(l))
@@ -217,17 +211,17 @@ func (l *Limits) UnmarshalJSON(data []byte) error {
 	if defaultLimits != nil {
 		*l = *defaultLimits
 		// Make copy of default limits. Otherwise unmarshalling would modify map in default limits.
-		l.copyNotificationIntegrationLimits(defaultLimits.NotificationIntegrationLimits)
+		l.copyNotificationIntegrationLimits(defaultLimits.NotificationRateLimitPerIntegration)
 	}
 
 	type plain Limits
 	return json.Unmarshal(data, (*plain)(l))
 }
 
-func (l *Limits) copyNotificationIntegrationLimits(defaults NotificationLimitsMap) {
-	l.NotificationIntegrationLimits = make(map[string]NotificationLimits, len(defaults))
+func (l *Limits) copyNotificationIntegrationLimits(defaults NotificationRateLimitMap) {
+	l.NotificationRateLimitPerIntegration = make(map[string]float64, len(defaults))
 	for k, v := range defaults {
-		l.NotificationIntegrationLimits[k] = v
+		l.NotificationRateLimitPerIntegration[k] = v
 	}
 }
 
@@ -536,17 +530,17 @@ func (o *Overrides) AlertmanagerReceiversBlockPrivateAddresses(user string) bool
 // 2. default limits for given integration
 // 3. per-tenant limits
 // 4. default limits
-func (o *Overrides) getNotificationLimitForUser(user, integration string) NotificationLimits {
+func (o *Overrides) getNotificationLimitForUser(user, integration string) float64 {
 	u := o.getOverridesForUser(user)
-	if n, ok := u.NotificationIntegrationLimits[integration]; ok {
+	if n, ok := u.NotificationRateLimitPerIntegration[integration]; ok {
 		return n
 	}
 
-	return u.NotificationLimits
+	return u.NotificationRateLimit
 }
 
 func (o *Overrides) NotificationRateLimit(user string, integration string) rate.Limit {
-	l := o.getNotificationLimitForUser(user, integration).RateLimit
+	l := o.getNotificationLimitForUser(user, integration)
 	if l == 0 || math.IsInf(l, 1) {
 		return rate.Inf // No rate limit.
 	}
@@ -557,12 +551,26 @@ func (o *Overrides) NotificationRateLimit(user string, integration string) rate.
 	return rate.Limit(l)
 }
 
+const maxInt = int(^uint(0) >> 1)
+
 func (o *Overrides) NotificationBurstSize(user string, integration string) int {
-	b := o.getNotificationLimitForUser(user, integration).BurstSize
-	if b < 0 {
-		b = 0
+	// Burst size is computed from rate limit. Rate limit is already normalized to [0, +inf), where 0 means disabled.
+	l := o.NotificationRateLimit(user, integration)
+	if l == 0 {
+		return 0
 	}
-	return b
+
+	// floats can be larger than max int. This also handles case where l == rate.Inf.
+	if float64(l) >= float64(maxInt) {
+		return maxInt
+	}
+
+	// For values between (0, 1), allow single notification per second (every 1/limit seconds).
+	if l < 1 {
+		return 1
+	}
+
+	return int(l)
 }
 
 func (o *Overrides) getOverridesForUser(userID string) *Limits {

pkg/util/validation/limits_test.go

@@ -349,59 +349,47 @@ func TestAlertmanagerNotificationLimits(t *testing.T) {
 	}{
 		"no email specific limit": {
 			inputYAML: `
-alertmanager_notification_limits:
-  rate_limit: 100
-  burst_size: 100
+alertmanager_notification_rate_limit: 100
 `,
 			expectedRateLimit: 100,
 			expectedBurstSize: 100,
 		},
 		"zero limit": {
 			inputYAML: `
-alertmanager_notification_limits:
-  rate_limit: 100
-  burst_size: 100
-
-alertmanager_notification_limits_per_integration:
-  email:
-    rate_limit: 0
-    burst_size: 0
+alertmanager_notification_rate_limit: 100
+
+alertmanager_notification_rate_limit_per_integration:
+  email: 0
 `,
 			expectedRateLimit: rate.Inf,
-			expectedBurstSize: 0,
+			expectedBurstSize: maxInt,
 		},
 
 		"negative limit": {
 			inputYAML: `
-alertmanager_notification_limits_per_integration:
-  email:
-    rate_limit: -10
-    burst_size: 5
+alertmanager_notification_rate_limit_per_integration:
+  email: -10
 `,
 			expectedRateLimit: 0,
-			expectedBurstSize: 5,
+			expectedBurstSize: 0,
 		},
 
 		"positive limit, negative burst": {
 			inputYAML: `
-alertmanager_notification_limits_per_integration:
-  email:
-    rate_limit: 222
-    burst_size: -1
+alertmanager_notification_rate_limit_per_integration:
+  email: 222
 `,
 			expectedRateLimit: 222,
-			expectedBurstSize: 0,
+			expectedBurstSize: 222,
 		},
 
 		"infinte limit": {
 			inputYAML: `
-alertmanager_notification_limits_per_integration:
-  email:
-    rate_limit: .inf
-    burst_size: 50
+alertmanager_notification_rate_limit_per_integration:
+  email: .inf
 `,
 			expectedRateLimit: rate.Inf,
-			expectedBurstSize: 50,
+			expectedBurstSize: maxInt,
 		},
 	} {
 		t.Run(name, func(t *testing.T) {
@@ -420,49 +408,35 @@ alertmanager_notification_limits_per_integration:
 
 func TestAlertmanagerNotificationLimitsOverrides(t *testing.T) {
 	baseYaml := `
-alertmanager_notification_limits:
-  rate_limit: 5
-  burst_size: 5
-
-alertmanager_notification_limits_per_integration:
- email:
-   rate_limit: 100
-   burst_size: 100
+alertmanager_notification_rate_limit: 5
+
+alertmanager_notification_rate_limit_per_integration:
+ email: 100
 `
 
 	overrideGenericLimitsOnly := `
 testuser:
-  alertmanager_notification_limits:
-    rate_limit: 333
-    burst_size: 333
+  alertmanager_notification_rate_limit: 333
 `
 
 	overrideEmailLimits := `
 testuser:
-  alertmanager_notification_limits_per_integration:
-    email:
-      rate_limit: 7777
-      burst_size: 7777
+  alertmanager_notification_rate_limit_per_integration:
+    email: 7777
 `
 
 	overrideGenericLimitsAndEmailLimits := `
 testuser:
-  alertmanager_notification_limits:
-    rate_limit: 333
-    burst_size: 333
+  alertmanager_notification_rate_limit: 333
 
-  alertmanager_notification_limits_per_integration:
-    email:
-      rate_limit: 7777
-      burst_size: 7777
+  alertmanager_notification_rate_limit_per_integration:
+    email: 7777
 `
 
 	differentUserOverride := `
 differentuser:
   alertmanager_notification_limits_per_integration:
-    email:
-      rate_limit: 500
-      burst_size: 20
+    email: 500
 `
 
 	for name, tc := range map[string]struct {
@@ -529,12 +503,11 @@ differentuser:
 			testedIntegration: "email",
 			overrides: `
 testuser:
-  alertmanager_notification_limits_per_integration:
-    email:
-      rate_limit: 500
+  alertmanager_notification_rate_limit_per_integration:
+    email: 500
 `,
 			expectedRateLimit: 500, // overridden
-			expectedBurstSize: 0,   // also overridden, but not visible
+			expectedBurstSize: 500, // same as rate limit
 		},
 
 		"different user override, pushover": {

pkg/util/validation/notifications_limit_flag.go

@@ -13,32 +13,32 @@ var allowedIntegrationNames = []string{
 	"webhook", "email", "pagerduty", "opsgenie", "wechat", "slack", "victorops", "pushover",
 }
 
-type NotificationLimitsMap map[string]NotificationLimits
+type NotificationRateLimitMap map[string]float64
 
 // String implements flag.Value
-func (m NotificationLimitsMap) String() string {
-	out, err := json.Marshal(map[string]NotificationLimits(m))
+func (m NotificationRateLimitMap) String() string {
+	out, err := json.Marshal(map[string]float64(m))
 	if err != nil {
 		return fmt.Sprintf("failed to marshal: %v", err)
 	}
 	return string(out)
 }
 
 // Set implements flag.Value
-func (m NotificationLimitsMap) Set(s string) error {
-	newMap := map[string]NotificationLimits{}
+func (m NotificationRateLimitMap) Set(s string) error {
+	newMap := map[string]float64{}
 	return m.updateMap(json.Unmarshal([]byte(s), &newMap), newMap)
 }
 
 // UnmarshalYAML implements yaml.Unmarshaler.
-func (m NotificationLimitsMap) UnmarshalYAML(unmarshal func(interface{}) error) error {
-	newMap := map[string]NotificationLimits{}
+func (m NotificationRateLimitMap) UnmarshalYAML(unmarshal func(interface{}) error) error {
+	newMap := map[string]float64{}
 	return m.updateMap(unmarshal(newMap), newMap)
 }
 
-func (m NotificationLimitsMap) updateMap(err error, newMap map[string]NotificationLimits) error {
-	if err != nil {
-		return err
+func (m NotificationRateLimitMap) updateMap(unmarshalErr error, newMap map[string]float64) error {
+	if unmarshalErr != nil {
+		return unmarshalErr
 	}
 
 	for k, v := range newMap {
@@ -51,6 +51,6 @@ func (m NotificationLimitsMap) updateMap(err error, newMap map[string]Notificati
 }
 
 // MarshalYAML implements yaml.Marshaler.
-func (m NotificationLimitsMap) MarshalYAML() (interface{}, error) {
-	return map[string]NotificationLimits(m), nil
+func (m NotificationRateLimitMap) MarshalYAML() (interface{}, error) {
+	return map[string]float64(m), nil
 }