Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add length and limit to error messages when label names and values are too long #4595

Merged
merged 2 commits into from
Jan 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
* [ENHANCEMENT] Query Frontend: Add setting `-frontend.forward-headers-list` in frontend to configure the set of headers from the requests to be forwarded to downstream requests. #4486
* [ENHANCEMENT] Blocks storage: Add `-blocks-storage.azure.http.*`, `-alertmanager-storage.azure.http.*`, and `-ruler-storage.azure.http.*` to configure the Azure storage client. #4581
* [ENHANCEMENT] Optimise memberlist receive path when used as a backing store for rings with a large number of members. #4601
* [ENHANCEMENT] Add length and limit to labelNameTooLongError and labelValueTooLongError #4595
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: CHANGELOG is for admins, not for developers, so it is better to use English than type names.

Copy link
Contributor Author

@jiachengxu jiachengxu Dec 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see. How about:
Add length and limit to error messages when label names and values are too long

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

* [BUGFIX] AlertManager: remove stale template files. #4495
* [BUGFIX] Distributor: fix bug in query-exemplar where some results would get dropped. #4582
* [BUGFIX] Update Thanos dependency: compactor tracing support, azure blocks storage memory fix. #4585
Expand Down
30 changes: 22 additions & 8 deletions pkg/util/validation/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,29 +27,43 @@ func (e *genericValidationError) Error() string {
return fmt.Sprintf(e.message, e.cause, formatLabelSet(e.series))
}

func newLabelNameTooLongError(series []cortexpb.LabelAdapter, labelName string) ValidationError {
return &genericValidationError{
message: "label name too long: %.200q metric %.200q",
cause: labelName,
series: series,
// labelNameTooLongError is a customized ValidationError, in that the cause and the series are
// formatted in different order in Error.
type labelNameTooLongError struct {
labelName string
series []cortexpb.LabelAdapter
limit int
}

func (e *labelNameTooLongError) Error() string {
return fmt.Sprintf("label name too long for metric (actual: %d, limit: %d) metric: %.200q label name: %.200q", len(e.labelName), e.limit, formatLabelSet(e.series), e.labelName)
}

func newLabelNameTooLongError(series []cortexpb.LabelAdapter, labelName string, limit int) ValidationError {
return &labelNameTooLongError{
labelName: labelName,
series: series,
limit: limit,
}
}

// labelValueTooLongError is a customized ValidationError, in that the cause and the series are
// are formatted in different order in Error.
// formatted in different order in Error.
type labelValueTooLongError struct {
labelValue string
series []cortexpb.LabelAdapter
limit int
}

func (e *labelValueTooLongError) Error() string {
return fmt.Sprintf("label value too long for metric: %.200q label value: %.200q", formatLabelSet(e.series), e.labelValue)
return fmt.Sprintf("label value too long for metric (actual: %d, limit: %d) metric: %.200q label value: %.200q", len(e.labelValue), e.limit, formatLabelSet(e.series), e.labelValue)
}

func newLabelValueTooLongError(series []cortexpb.LabelAdapter, labelValue string) ValidationError {
func newLabelValueTooLongError(series []cortexpb.LabelAdapter, labelValue string, limit int) ValidationError {
return &labelValueTooLongError{
labelValue: labelValue,
series: series,
limit: limit,
}
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/util/validation/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,10 +201,10 @@ func ValidateLabels(cfg LabelValidationConfig, userID string, ls []cortexpb.Labe
return newInvalidLabelError(ls, l.Name)
} else if len(l.Name) > maxLabelNameLength {
DiscardedSamples.WithLabelValues(labelNameTooLong, userID).Inc()
return newLabelNameTooLongError(ls, l.Name)
return newLabelNameTooLongError(ls, l.Name, maxLabelNameLength)
} else if len(l.Value) > maxLabelValueLength {
DiscardedSamples.WithLabelValues(labelValueTooLong, userID).Inc()
return newLabelValueTooLongError(ls, l.Value)
return newLabelValueTooLongError(ls, l.Value, maxLabelValueLength)
} else if cmp := strings.Compare(lastLabelName, l.Name); cmp >= 0 {
if cmp == 0 {
DiscardedSamples.WithLabelValues(duplicateLabelNames, userID).Inc()
Expand Down
4 changes: 2 additions & 2 deletions pkg/util/validation/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,15 +95,15 @@ func TestValidateLabels(t *testing.T) {
newLabelNameTooLongError([]cortexpb.LabelAdapter{
{Name: model.MetricNameLabel, Value: "badLabelName"},
{Name: "this_is_a_really_really_long_name_that_should_cause_an_error", Value: "test_value_please_ignore"},
}, "this_is_a_really_really_long_name_that_should_cause_an_error"),
}, "this_is_a_really_really_long_name_that_should_cause_an_error", cfg.maxLabelNameLength),
},
{
map[model.LabelName]model.LabelValue{model.MetricNameLabel: "badLabelValue", "much_shorter_name": "test_value_please_ignore_no_really_nothing_to_see_here"},
false,
newLabelValueTooLongError([]cortexpb.LabelAdapter{
{Name: model.MetricNameLabel, Value: "badLabelValue"},
{Name: "much_shorter_name", Value: "test_value_please_ignore_no_really_nothing_to_see_here"},
}, "test_value_please_ignore_no_really_nothing_to_see_here"),
}, "test_value_please_ignore_no_really_nothing_to_see_here", cfg.maxLabelValueLength),
},
{
map[model.LabelName]model.LabelValue{model.MetricNameLabel: "foo", "bar": "baz", "blip": "blop"},
Expand Down