-
Notifications
You must be signed in to change notification settings - Fork 7
Add global label length limits #83
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -189,6 +189,9 @@ var ( | |
| ExtraScrapeMetrics: boolPtr(false), | ||
| MetricNameValidationScheme: model.UTF8Validation, | ||
| MetricNameEscapingScheme: model.AllowUTF8, | ||
| // Default to 1 MiB to avoid crashes from the 16 MiB encoding limit. | ||
| LabelNameLengthLimit: 1 << 20, | ||
| LabelValueLengthLimit: 1 << 21, | ||
| } | ||
|
|
||
| DefaultRuntimeConfig = RuntimeConfig{ | ||
|
|
@@ -698,6 +701,9 @@ func (c *GlobalConfig) UnmarshalYAML(unmarshal func(any) error) error { | |
| return fmt.Errorf("%w for global config", err) | ||
| } | ||
| } | ||
| if gc.LabelNameLengthLimit == 0 { | ||
| gc.LabelNameLengthLimit = DefaultGlobalConfig.LabelNameLengthLimit | ||
| } | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| *c = *gc | ||
| return nil | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2000,6 +2000,9 @@ func (ev *evaluator) evalLabelReplace(ctx context.Context, args parser.Expressio | |
| indexes := regex.FindStringSubmatchIndex(srcVal) | ||
| if indexes != nil { // Only replace when regexp matches. | ||
| res := regex.ExpandString([]byte{}, repl, srcVal, indexes) | ||
| if len(res) > 1<<24 { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we set |
||
| ev.errorf("label_replace: replacement value too long (%d bytes)", len(res)) | ||
| } | ||
| lb.Reset(el.Metric) | ||
| lb.Set(dst, string(res)) | ||
| matrix[i].Metric = lb.Labels() | ||
|
|
@@ -2051,6 +2054,9 @@ func (ev *evaluator) evalLabelJoin(ctx context.Context, args parser.Expressions) | |
| srcVals[i] = el.Metric.Get(src) | ||
| } | ||
| strval := strings.Join(srcVals, sep) | ||
| if len(strval) > 1<<24 { | ||
| ev.errorf("label_join: joined value too long (%d bytes)", len(strval)) | ||
| } | ||
| lb.Reset(el.Metric) | ||
| lb.Set(dst, strval) | ||
| matrix[i].Metric = lb.Labels() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -48,6 +48,9 @@ type writeHandler struct { | |
| ingestSTZeroSample bool | ||
| enableTypeAndUnitLabels bool | ||
| appendMetadata bool | ||
|
|
||
| labelNameLengthLimit int | ||
| labelValueLengthLimit int | ||
| } | ||
|
|
||
| const maxAheadTime = 10 * time.Minute | ||
|
|
@@ -57,7 +60,7 @@ const maxAheadTime = 10 * time.Minute | |
| // | ||
| // NOTE(bwplotka): When accepting v2 proto and spec, partial writes are possible | ||
| // as per https://prometheus.io/docs/specs/remote_write_spec_2_0/#partial-write. | ||
| func NewWriteHandler(logger *slog.Logger, reg prometheus.Registerer, appendable storage.Appendable, acceptedMsgs remoteapi.MessageTypes, ingestSTZeroSample, enableTypeAndUnitLabels, appendMetadata bool) http.Handler { | ||
| func NewWriteHandler(logger *slog.Logger, reg prometheus.Registerer, appendable storage.Appendable, acceptedMsgs remoteapi.MessageTypes, ingestSTZeroSample, enableTypeAndUnitLabels, appendMetadata bool, labelNameLengthLimit, labelValueLengthLimit int) http.Handler { | ||
| h := &writeHandler{ | ||
| logger: logger, | ||
| appendable: appendable, | ||
|
|
@@ -77,6 +80,9 @@ func NewWriteHandler(logger *slog.Logger, reg prometheus.Registerer, appendable | |
| ingestSTZeroSample: ingestSTZeroSample, | ||
| enableTypeAndUnitLabels: enableTypeAndUnitLabels, | ||
| appendMetadata: appendMetadata, | ||
|
|
||
| labelNameLengthLimit: labelNameLengthLimit, | ||
| labelValueLengthLimit: labelValueLengthLimit, | ||
| } | ||
| return remoteapi.NewWriteHandler(h, acceptedMsgs, remoteapi.WithWriteHandlerLogger(logger)) | ||
| } | ||
|
|
@@ -146,6 +152,23 @@ func (h *writeHandler) Store(r *http.Request, msgType remoteapi.WriteMessageType | |
| return wr, nil | ||
| } | ||
|
|
||
| // checkLabelLengths returns an error if any label name or value in lset exceeds | ||
| // the configured limits. A limit of 0 means no limit. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the user wishes to set no limit, and |
||
| func (h *writeHandler) checkLabelLengths(lset labels.Labels) error { | ||
| if h.labelNameLengthLimit == 0 && h.labelValueLengthLimit == 0 { | ||
| return nil | ||
| } | ||
| return lset.Validate(func(l labels.Label) error { | ||
| if h.labelNameLengthLimit > 0 && len(l.Name) > h.labelNameLengthLimit { | ||
| return fmt.Errorf("label name too long (label: %.50s, length: %d, limit: %d)", l.Name, len(l.Name), h.labelNameLengthLimit) | ||
| } | ||
| if h.labelValueLengthLimit > 0 && len(l.Value) > h.labelValueLengthLimit { | ||
| return fmt.Errorf("label value too long (label: %.50s, length: %d, limit: %d)", l.Name, len(l.Value), h.labelValueLengthLimit) | ||
| } | ||
| return nil | ||
| }) | ||
| } | ||
|
|
||
| func (h *writeHandler) write(ctx context.Context, req *prompb.WriteRequest) (err error) { | ||
| outOfOrderExemplarErrs := 0 | ||
| samplesWithInvalidLabels := 0 | ||
|
|
@@ -181,6 +204,10 @@ func (h *writeHandler) write(ctx context.Context, req *prompb.WriteRequest) (err | |
| h.logger.Warn("Invalid labels for series.", "labels", ls.String(), "duplicated_label", duplicateLabel) | ||
| samplesWithInvalidLabels++ | ||
| continue | ||
| } else if err := h.checkLabelLengths(ls); err != nil { | ||
| h.logger.Warn("Label length limit exceeded", "err", err) | ||
| samplesWithInvalidLabels++ | ||
| continue | ||
| } | ||
|
|
||
| if err := h.appendV1Samples(app, ts.Samples, ls); err != nil { | ||
|
|
@@ -345,6 +372,10 @@ func (h *writeHandler) appendV2(app storage.Appender, req *writev2.Request, rs * | |
| badRequestErrs = append(badRequestErrs, fmt.Errorf("invalid labels for series, labels %v, duplicated label %s", ls.String(), duplicateLabel)) | ||
| samplesWithInvalidLabels += len(ts.Samples) + len(ts.Histograms) | ||
| continue | ||
| } else if err := h.checkLabelLengths(ls); err != nil { | ||
| badRequestErrs = append(badRequestErrs, fmt.Errorf("label length limit exceeded for series %v: %w", ls.String(), err)) | ||
| samplesWithInvalidLabels += len(ts.Samples) + len(ts.Histograms) | ||
| continue | ||
| } | ||
|
|
||
| // Validate that the TimeSeries has at least one sample or histogram. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For consistency, could you omit the spaces here to match how the syntax is done later on in the code?