Skip to content

Commit

Permalink
Add ingester flag to customize message in limiter error messages (#5460)
Browse files Browse the repository at this point in the history
* Add ingester flag to add admin contact details in limiter error messages

Signed-off-by: Nikhil Kumar <nikhil.kumar.92219@gmail.com>

* Rename flag to AdminLimitMessage

Signed-off-by: Nikhil Kumar <nikhil.kumar.92219@gmail.com>

* Fix CLI Flag in documentation

Signed-off-by: Nikhil Kumar <nikhil.kumar.92219@gmail.com>

* Fix Documentation

Signed-off-by: Nikhil Kumar <nikhil.kumar.92219@gmail.com>

* Changelog updated

Signed-off-by: Nikhil Kumar <nikhil.kumar.92219@gmail.com>

* Update CHANGELOG.md

Signed-off-by: Friedrich Gonzalez <friedrichg@gmail.com>

---------

Signed-off-by: Nikhil Kumar <nikhil.kumar.92219@gmail.com>
Signed-off-by: Friedrich Gonzalez <friedrichg@gmail.com>
Co-authored-by: Friedrich Gonzalez <friedrichg@gmail.com>
  • Loading branch information
kumanik and friedrichg authored Jul 18, 2023
1 parent e52014e commit 025e723
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 17 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Changelog

## master / unreleased
* [FEATURE] Ingester: added `-admin-limit-message` to customize the message contained in limit errors.#5460
* [CHANGE] AlertManager: include reason label in cortex_alertmanager_notifications_failed_total.#5409
* [CHANGE] Query: Set CORS Origin headers for Query API #5388
* [CHANGE] Updating prometheus/alertmanager from v0.25.0 to v0.25.1-0.20230505130626-263ca5c9438e. This includes the below changes. #5276
Expand Down
4 changes: 4 additions & 0 deletions docs/configuration/config-file-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -2620,6 +2620,10 @@ instance_limits:
# max-global-series-per-metric limits.
# CLI flag: -ingester.ignore-series-limit-for-metric-names
[ignore_series_limit_for_metric_names: <string> | default = ""]
# Customize the message contained in limit errors
# CLI flag: -ingester.admin-limit-message
[admin_limit_message: <string> | default = "please contact administrator to raise it"]
```

### `ingester_client_config`
Expand Down
10 changes: 9 additions & 1 deletion pkg/ingester/ingester.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,9 @@ type Config struct {

// For testing, you can override the address and ID of this ingester.
ingesterClientFactory func(addr string, cfg client.Config) (client.HealthAndIngesterClient, error)

// For admin contact details
AdminLimitMessage string `yaml:"admin_limit_message"`
}

// RegisterFlags adds the flags required to config this to the given FlagSet
Expand All @@ -144,6 +147,9 @@ func (cfg *Config) RegisterFlags(f *flag.FlagSet) {
f.Int64Var(&cfg.DefaultLimits.MaxInflightPushRequests, "ingester.instance-limits.max-inflight-push-requests", 0, "Max inflight push requests that this ingester can handle (across all tenants). Additional requests will be rejected. 0 = unlimited.")

f.StringVar(&cfg.IgnoreSeriesLimitForMetricNames, "ingester.ignore-series-limit-for-metric-names", "", "Comma-separated list of metric names, for which -ingester.max-series-per-metric and -ingester.max-global-series-per-metric limits will be ignored. Does not affect max-series-per-user or max-global-series-per-metric limits.")

f.StringVar(&cfg.AdminLimitMessage, "ingester.admin-limit-message", "please contact administrator to raise it", "Customize the message contained in limit errors")

}

func (cfg *Config) getIgnoreSeriesLimitForMetricNamesMap() map[string]struct{} {
Expand Down Expand Up @@ -653,7 +659,9 @@ func New(cfg Config, limits *validation.Overrides, registerer prometheus.Registe
cfg.DistributorShardingStrategy,
cfg.DistributorShardByAllLabels,
cfg.LifecyclerConfig.RingConfig.ReplicationFactor,
cfg.LifecyclerConfig.RingConfig.ZoneAwarenessEnabled)
cfg.LifecyclerConfig.RingConfig.ZoneAwarenessEnabled,
cfg.AdminLimitMessage,
)

i.TSDBState.shipperIngesterID = i.lifecycler.ID

Expand Down
19 changes: 11 additions & 8 deletions pkg/ingester/limiter.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ type Limiter struct {
shuffleShardingEnabled bool
shardByAllLabels bool
zoneAwarenessEnabled bool
AdminLimitMessage string
}

// NewLimiter makes a new in-memory series limiter
Expand All @@ -44,6 +45,7 @@ func NewLimiter(
shardByAllLabels bool,
replicationFactor int,
zoneAwarenessEnabled bool,
AdminLimitMessage string,
) *Limiter {
return &Limiter{
limits: limits,
Expand All @@ -52,6 +54,7 @@ func NewLimiter(
shuffleShardingEnabled: shardingStrategy == util.ShardingStrategyShuffle,
shardByAllLabels: shardByAllLabels,
zoneAwarenessEnabled: zoneAwarenessEnabled,
AdminLimitMessage: AdminLimitMessage,
}
}

Expand Down Expand Up @@ -122,35 +125,35 @@ func (l *Limiter) formatMaxSeriesPerUserError(userID string) error {
localLimit := l.limits.MaxLocalSeriesPerUser(userID)
globalLimit := l.limits.MaxGlobalSeriesPerUser(userID)

return fmt.Errorf("per-user series limit of %d exceeded, please contact administrator to raise it (local limit: %d global limit: %d actual local limit: %d)",
minNonZero(localLimit, globalLimit), localLimit, globalLimit, actualLimit)
return fmt.Errorf("per-user series limit of %d exceeded, %s (local limit: %d global limit: %d actual local limit: %d)",
minNonZero(localLimit, globalLimit), l.AdminLimitMessage, localLimit, globalLimit, actualLimit)
}

func (l *Limiter) formatMaxSeriesPerMetricError(userID string) error {
actualLimit := l.maxSeriesPerMetric(userID)
localLimit := l.limits.MaxLocalSeriesPerMetric(userID)
globalLimit := l.limits.MaxGlobalSeriesPerMetric(userID)

return fmt.Errorf("per-metric series limit of %d exceeded, please contact administrator to raise it (local limit: %d global limit: %d actual local limit: %d)",
minNonZero(localLimit, globalLimit), localLimit, globalLimit, actualLimit)
return fmt.Errorf("per-metric series limit of %d exceeded, %s (local limit: %d global limit: %d actual local limit: %d)",
minNonZero(localLimit, globalLimit), l.AdminLimitMessage, localLimit, globalLimit, actualLimit)
}

func (l *Limiter) formatMaxMetadataPerUserError(userID string) error {
actualLimit := l.maxMetadataPerUser(userID)
localLimit := l.limits.MaxLocalMetricsWithMetadataPerUser(userID)
globalLimit := l.limits.MaxGlobalMetricsWithMetadataPerUser(userID)

return fmt.Errorf("per-user metric metadata limit of %d exceeded, please contact administrator to raise it (local limit: %d global limit: %d actual local limit: %d)",
minNonZero(localLimit, globalLimit), localLimit, globalLimit, actualLimit)
return fmt.Errorf("per-user metric metadata limit of %d exceeded, %s (local limit: %d global limit: %d actual local limit: %d)",
minNonZero(localLimit, globalLimit), l.AdminLimitMessage, localLimit, globalLimit, actualLimit)
}

func (l *Limiter) formatMaxMetadataPerMetricError(userID string) error {
actualLimit := l.maxMetadataPerMetric(userID)
localLimit := l.limits.MaxLocalMetadataPerMetric(userID)
globalLimit := l.limits.MaxGlobalMetadataPerMetric(userID)

return fmt.Errorf("per-metric metadata limit of %d exceeded, please contact administrator to raise it (local limit: %d global limit: %d actual local limit: %d)",
minNonZero(localLimit, globalLimit), localLimit, globalLimit, actualLimit)
return fmt.Errorf("per-metric metadata limit of %d exceeded, %s (local limit: %d global limit: %d actual local limit: %d)",
minNonZero(localLimit, globalLimit), l.AdminLimitMessage, localLimit, globalLimit, actualLimit)
}

func (l *Limiter) maxSeriesPerMetric(userID string) int {
Expand Down
14 changes: 7 additions & 7 deletions pkg/ingester/limiter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,12 +223,12 @@ func runLimiterMaxFunctionTest(
require.NoError(t, err)

// Assert on default sharding strategy.
limiter := NewLimiter(overrides, ring, util.ShardingStrategyDefault, testData.shardByAllLabels, testData.ringReplicationFactor, testData.ringZoneAwarenessEnabled)
limiter := NewLimiter(overrides, ring, util.ShardingStrategyDefault, testData.shardByAllLabels, testData.ringReplicationFactor, testData.ringZoneAwarenessEnabled, "")
actual := runMaxFn(limiter)
assert.Equal(t, testData.expectedDefaultSharding, actual)

// Assert on shuffle sharding strategy.
limiter = NewLimiter(overrides, ring, util.ShardingStrategyShuffle, testData.shardByAllLabels, testData.ringReplicationFactor, testData.ringZoneAwarenessEnabled)
limiter = NewLimiter(overrides, ring, util.ShardingStrategyShuffle, testData.shardByAllLabels, testData.ringReplicationFactor, testData.ringZoneAwarenessEnabled, "")
actual = runMaxFn(limiter)
assert.Equal(t, testData.expectedShuffleSharding, actual)
})
Expand Down Expand Up @@ -290,7 +290,7 @@ func TestLimiter_AssertMaxSeriesPerMetric(t *testing.T) {
}, nil)
require.NoError(t, err)

limiter := NewLimiter(limits, ring, util.ShardingStrategyDefault, testData.shardByAllLabels, testData.ringReplicationFactor, false)
limiter := NewLimiter(limits, ring, util.ShardingStrategyDefault, testData.shardByAllLabels, testData.ringReplicationFactor, false, "")
actual := limiter.AssertMaxSeriesPerMetric("test", testData.series)

assert.Equal(t, testData.expected, actual)
Expand Down Expand Up @@ -352,7 +352,7 @@ func TestLimiter_AssertMaxMetadataPerMetric(t *testing.T) {
}, nil)
require.NoError(t, err)

limiter := NewLimiter(limits, ring, util.ShardingStrategyDefault, testData.shardByAllLabels, testData.ringReplicationFactor, false)
limiter := NewLimiter(limits, ring, util.ShardingStrategyDefault, testData.shardByAllLabels, testData.ringReplicationFactor, false, "")
actual := limiter.AssertMaxMetadataPerMetric("test", testData.metadata)

assert.Equal(t, testData.expected, actual)
Expand Down Expand Up @@ -415,7 +415,7 @@ func TestLimiter_AssertMaxSeriesPerUser(t *testing.T) {
}, nil)
require.NoError(t, err)

limiter := NewLimiter(limits, ring, util.ShardingStrategyDefault, testData.shardByAllLabels, testData.ringReplicationFactor, false)
limiter := NewLimiter(limits, ring, util.ShardingStrategyDefault, testData.shardByAllLabels, testData.ringReplicationFactor, false, "")
actual := limiter.AssertMaxSeriesPerUser("test", testData.series)

assert.Equal(t, testData.expected, actual)
Expand Down Expand Up @@ -478,7 +478,7 @@ func TestLimiter_AssertMaxMetricsWithMetadataPerUser(t *testing.T) {
}, nil)
require.NoError(t, err)

limiter := NewLimiter(limits, ring, util.ShardingStrategyDefault, testData.shardByAllLabels, testData.ringReplicationFactor, false)
limiter := NewLimiter(limits, ring, util.ShardingStrategyDefault, testData.shardByAllLabels, testData.ringReplicationFactor, false, "")
actual := limiter.AssertMaxMetricsWithMetadataPerUser("test", testData.metadata)

assert.Equal(t, testData.expected, actual)
Expand All @@ -501,7 +501,7 @@ func TestLimiter_FormatError(t *testing.T) {
}, nil)
require.NoError(t, err)

limiter := NewLimiter(limits, ring, util.ShardingStrategyDefault, true, 3, false)
limiter := NewLimiter(limits, ring, util.ShardingStrategyDefault, true, 3, false, "please contact administrator to raise it")

actual := limiter.FormatError("user-1", errMaxSeriesPerUserLimitExceeded)
assert.EqualError(t, actual, "per-user series limit of 100 exceeded, please contact administrator to raise it (local limit: 0 global limit: 100 actual local limit: 100)")
Expand Down
2 changes: 1 addition & 1 deletion pkg/ingester/user_state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func TestMetricCounter(t *testing.T) {

// We're testing code that's not dependant on sharding strategy, replication factor, etc. To simplify the test,
// we use local limit only.
limiter := NewLimiter(overrides, nil, util.ShardingStrategyDefault, true, 3, false)
limiter := NewLimiter(overrides, nil, util.ShardingStrategyDefault, true, 3, false, "")
mc := newMetricCounter(limiter, ignored)

for i := 0; i < tc.series; i++ {
Expand Down

0 comments on commit 025e723

Please sign in to comment.