Add metric name in limiter per-metric exceeded errors #6422
Conversation
pkg/ingester/limiter.go
Outdated
| if actualLimit := l.maxSeriesPerMetric(userID); series < actualLimit { | ||
| return nil | ||
| } | ||
|
|
||
| return errMaxSeriesPerMetricLimitExceeded | ||
| return errors.Wrap(errMaxSeriesPerMetricLimitExceeded, "{metric name: " + metric + "}") |
There was a problem hiding this comment.
Can we change errMaxSeriesPerMetricLimitExceeded to take metric name as a parameter and we use errors.Wrapf(errMaxSeriesPerMetricLimitExceeded, metric)?
There was a problem hiding this comment.
We already have the labels on the handleAppendFailure, no? we can just send those labels down to format the error?
There was a problem hiding this comment.
Ex:
cortex/pkg/ingester/ingester.go
Line 1161 in 6be0378
Signed-off-by: Essam Eldaly <[email protected]>
pkg/ingester/limiter.go
Outdated
| } | ||
|
|
||
| func (l *Limiter) formatMaxSeriesPerLabelSetError(err errMaxSeriesPerLabelSetLimitExceeded) error { | ||
| func (l *Limiter) formatMaxSeriesPerLabelSetError(err errMaxSeriesPerLabelSetLimitExceeded, lbls labels.Labels) error { |
There was a problem hiding this comment.
This seems not used. Do we need it?
pkg/ingester/limiter.go
Outdated
| } | ||
|
|
||
| func (l *Limiter) formatMaxMetadataPerUserError(userID string) error { | ||
| func (l *Limiter) formatMaxMetadataPerUserError(userID string, lbls labels.Labels) error { |
There was a problem hiding this comment.
This seems not used. Do we need it?
pkg/ingester/limiter.go
Outdated
| default: | ||
| return err | ||
| } | ||
| } | ||
|
|
||
| func (l *Limiter) formatMaxSeriesPerUserError(userID string) error { | ||
| func (l *Limiter) formatMaxSeriesPerUserError(userID string, lbls labels.Labels) error { |
There was a problem hiding this comment.
This seems not used. Do we need it?
| @@ -158,16 +158,16 @@ func (l *Limiter) formatMaxSeriesPerUserError(userID string) error { | |||
| minNonZero(localLimit, globalLimit), l.AdminLimitMessage, localLimit, globalLimit, actualLimit) | |||
| } | |||
|
|
|||
| func (l *Limiter) formatMaxSeriesPerMetricError(userID string) error { | |||
| func (l *Limiter) formatMaxSeriesPerMetricError(userID string, lbls labels.Labels) error { | |||
| actualLimit := l.maxSeriesPerMetric(userID) | |||
There was a problem hiding this comment.
We can just pass the metric name string. No need to pass labels
Signed-off-by: Essam Eldaly <[email protected]>
Signed-off-by: Essam Eldaly <[email protected]>
Signed-off-by: Ben Ye <[email protected]>
What this PR does:
Adds metric name in limiter per-metric exceeded errors
Which issue(s) this PR fixes:
Fixes #6416
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]