Skip to content

Commit 6cdda61

Browse files
simonswinesimonswine
authored
Revert "Fix: Handle duplicate validation correctly when sanitizing (#4238)" (#4264)
This reverts commit d410cdf.
1 parent b95d12f commit 6cdda61

File tree

2 files changed

+23
-103
lines changed

2 files changed

+23
-103
lines changed

‎pkg/validation/validate.go

Lines changed: 22 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -63,23 +63,22 @@ const (
6363
// Those profiles were dropped because of relabeling rules
6464
DroppedByRelabelRules Reason = "dropped_by_relabel_rules"
6565

66-
SeriesLimitErrorMsg = "Maximum active series limit exceeded (%d/%d), reduce the number of active streams (reduce labels or reduce label values), or contact your administrator to see if the limit can be increased"
67-
MissingLabelsErrorMsg = "error at least one label pair is required per profile"
68-
InvalidLabelsErrorMsg = "invalid labels '%s' with error: %s"
69-
MaxLabelNamesPerSeriesErrorMsg = "profile series '%s' has %d label names; limit %d"
70-
LabelNameTooLongErrorMsg = "profile with labels '%s' has label name too long: '%s'"
71-
LabelValueTooLongErrorMsg = "profile with labels '%s' has label value too long: '%s'"
72-
DuplicateLabelNamesErrorMsg = "profile with labels '%s' has duplicate label name: '%s'"
73-
DuplicateLabelNamesAfterSanitizationErrorMsg = "profile with labels '%s' has duplicate label name '%s' after label name sanitization from '%s'"
74-
QueryTooLongErrorMsg = "the query time range exceeds the limit (max_query_length, actual: %s, limit: %s)"
75-
ProfileTooBigErrorMsg = "the profile with labels '%s' exceeds the size limit (max_profile_size_byte, actual: %d, limit: %d)"
76-
ProfileTooManySamplesErrorMsg = "the profile with labels '%s' exceeds the samples count limit (max_profile_stacktrace_samples, actual: %d, limit: %d)"
77-
ProfileTooManySampleLabelsErrorMsg = "the profile with labels '%s' exceeds the sample labels limit (max_profile_stacktrace_sample_labels, actual: %d, limit: %d)"
78-
NotInIngestionWindowErrorMsg = "profile with labels '%s' is outside of ingestion window (profile timestamp: %s, %s)"
79-
MaxFlameGraphNodesErrorMsg = "max flamegraph nodes limit %d is greater than allowed %d"
80-
MaxFlameGraphNodesUnlimitedErrorMsg = "max flamegraph nodes limit must be set (max allowed %d)"
81-
QueryMissingTimeRangeErrorMsg = "missing time range in the query"
82-
QueryStartAfterEndErrorMsg = "query start time is after end time"
66+
SeriesLimitErrorMsg = "Maximum active series limit exceeded (%d/%d), reduce the number of active streams (reduce labels or reduce label values), or contact your administrator to see if the limit can be increased"
67+
MissingLabelsErrorMsg = "error at least one label pair is required per profile"
68+
InvalidLabelsErrorMsg = "invalid labels '%s' with error: %s"
69+
MaxLabelNamesPerSeriesErrorMsg = "profile series '%s' has %d label names; limit %d"
70+
LabelNameTooLongErrorMsg = "profile with labels '%s' has label name too long: '%s'"
71+
LabelValueTooLongErrorMsg = "profile with labels '%s' has label value too long: '%s'"
72+
DuplicateLabelNamesErrorMsg = "profile with labels '%s' has duplicate label name: '%s'"
73+
QueryTooLongErrorMsg = "the query time range exceeds the limit (max_query_length, actual: %s, limit: %s)"
74+
ProfileTooBigErrorMsg = "the profile with labels '%s' exceeds the size limit (max_profile_size_byte, actual: %d, limit: %d)"
75+
ProfileTooManySamplesErrorMsg = "the profile with labels '%s' exceeds the samples count limit (max_profile_stacktrace_samples, actual: %d, limit: %d)"
76+
ProfileTooManySampleLabelsErrorMsg = "the profile with labels '%s' exceeds the sample labels limit (max_profile_stacktrace_sample_labels, actual: %d, limit: %d)"
77+
NotInIngestionWindowErrorMsg = "profile with labels '%s' is outside of ingestion window (profile timestamp: %s, %s)"
78+
MaxFlameGraphNodesErrorMsg = "max flamegraph nodes limit %d is greater than allowed %d"
79+
MaxFlameGraphNodesUnlimitedErrorMsg = "max flamegraph nodes limit must be set (max allowed %d)"
80+
QueryMissingTimeRangeErrorMsg = "missing time range in the query"
81+
QueryStartAfterEndErrorMsg = "query start time is after end time"
8382
)
8483

8584
var (
@@ -129,91 +128,32 @@ func ValidateLabels(limits LabelValidationLimits, tenantID string, ls []*typesv1
129128
if !isValidServiceName(serviceNameValue) {
130129
return NewErrorf(MissingLabels, InvalidLabelsErrorMsg, phlaremodel.LabelPairsString(ls), "service name is not provided")
131130
}
131+
lastLabelName := ""
132132

133-
var (
134-
lastLabelName = ""
135-
idx = 0
136-
)
137-
for idx < len(ls) {
138-
l := ls[idx]
133+
for _, l := range ls {
139134
if len(l.Name) > limits.MaxLabelNameLength(tenantID) {
140135
return NewErrorf(LabelNameTooLong, LabelNameTooLongErrorMsg, phlaremodel.LabelPairsString(ls), l.Name)
141136
}
142137
if len(l.Value) > limits.MaxLabelValueLength(tenantID) {
143138
return NewErrorf(LabelValueTooLong, LabelValueTooLongErrorMsg, phlaremodel.LabelPairsString(ls), l.Value)
144139
}
145-
if origName, newName, ok := SanitizeLabelName(l.Name); ok && origName != newName {
146-
var err error
147-
ls, idx, err = handleSanitizedLabel(ls, idx, origName, newName)
148-
if err != nil {
149-
return err
150-
}
151-
lastLabelName = ""
152-
if idx > 0 && idx <= len(ls) {
153-
lastLabelName = ls[idx-1].Name
154-
}
155-
continue
156-
} else if !ok {
140+
var origName string
141+
var ok bool
142+
if origName, l.Name, ok = SanitizeLabelName(l.Name); !ok {
157143
return NewErrorf(InvalidLabels, InvalidLabelsErrorMsg, phlaremodel.LabelPairsString(ls), "invalid label name '"+origName+"'")
158144
}
159145
if !model.LabelValue(l.Value).IsValid() {
160146
return NewErrorf(InvalidLabels, InvalidLabelsErrorMsg, phlaremodel.LabelPairsString(ls), "invalid label value '"+l.Value+"'")
161147
}
162148
if cmp := strings.Compare(lastLabelName, l.Name); cmp == 0 {
163-
return NewErrorf(DuplicateLabelNames, DuplicateLabelNamesErrorMsg, phlaremodel.LabelPairsString(ls), l.Name)
149+
return NewErrorf(DuplicateLabelNames, DuplicateLabelNamesErrorMsg, phlaremodel.LabelPairsString(ls), origName)
164150
}
165151
lastLabelName = l.Name
166-
idx += 1
167152
}
168153

169154
return nil
170155
}
171156

172-
// handleSanitizedLabel handles the case where a label name is sanitized. It ensures that the label name is unique and fails if the value is distinct.
173-
func handleSanitizedLabel(ls []*typesv1.LabelPair, origIdx int, origName, newName string) ([]*typesv1.LabelPair, int, error) {
174-
var (
175-
found = false
176-
origValue = ls[origIdx].Value
177-
newIdx int
178-
)
179-
180-
for idx := range ls {
181-
// label name matches
182-
if ls[idx].Name == newName {
183-
if ls[idx].Value == origValue {
184-
found = true
185-
} else {
186-
return ls, origIdx, NewErrorf(DuplicateLabelNames, DuplicateLabelNamesAfterSanitizationErrorMsg, phlaremodel.LabelPairsString(ls), newName, origName)
187-
}
188-
}
189-
// move up labels that are after the original index
190-
if idx > origIdx {
191-
idx -= 1
192-
ls[idx] = ls[idx+1]
193-
}
194-
// check if the new label should be inserted before the current label
195-
if newName < ls[idx].Name {
196-
newIdx = idx
197-
}
198-
}
199-
200-
// when the new label already exists with matchin value, we don't need to insert it and we continue at the original index
201-
if found {
202-
return ls[:len(ls)-1], origIdx, nil
203-
}
204-
205-
// insert the new label at the determined index
206-
copy(ls[newIdx+1:], ls[newIdx:])
207-
ls[newIdx] = &typesv1.LabelPair{Name: newName, Value: origValue}
208-
209-
// if the new label is inserted after the original index, we need to return the original index, so all labels after the original index are validated
210-
if newIdx > origIdx {
211-
return ls, origIdx, nil
212-
}
213-
214-
return ls, newIdx, nil
215-
}
216-
217157
// SanitizeLabelName reports whether the label name is valid,
218158
// and returns the sanitized value.
219159
//

‎pkg/validation/validate_test.go

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -123,27 +123,7 @@ func TestValidateLabels(t *testing.T) {
123123
{Name: phlaremodel.LabelNameServiceName, Value: "svc"},
124124
},
125125
expectedReason: DuplicateLabelNames,
126-
expectedErr: "profile with labels '{__name__=\"qux\", label.name=\"bar\", label_name=\"foo\", service_name=\"svc\"}' has duplicate label name 'label_name' after label name sanitization from 'label.name'",
127-
},
128-
{
129-
name: "duplicates once sanitized with matching values",
130-
lbs: []*typesv1.LabelPair{
131-
{Name: model.MetricNameLabel, Value: "qux"},
132-
{Name: "service.name", Value: "svc0"},
133-
{Name: "service_abc", Value: "def"},
134-
{Name: "service_name", Value: "svc0"},
135-
},
136-
},
137-
{
138-
name: "duplicates once sanitized with conflicting values",
139-
lbs: []*typesv1.LabelPair{
140-
{Name: model.MetricNameLabel, Value: "qux"},
141-
{Name: "service.name", Value: "svc1"},
142-
{Name: "service_abc", Value: "def"},
143-
{Name: "service_name", Value: "svc0"},
144-
},
145-
expectedReason: DuplicateLabelNames,
146-
expectedErr: "profile with labels '{__name__=\"qux\", service_abc=\"def\", service_abc=\"def\", service_name=\"svc0\"}' has duplicate label name 'service_name' after label name sanitization from 'service.name'",
126+
expectedErr: "profile with labels '{__name__=\"qux\", label_name=\"foo\", label_name=\"bar\", service_name=\"svc\"}' has duplicate label name: 'label.name'",
147127
},
148128
} {
149129
t.Run(tt.name, func(t *testing.T) {

0 commit comments

Comments
 (0)