Add global label length limits#84
Open
bboreham wants to merge 1 commit intoprometheus-community:mainfrom
Open
Conversation
1. config/config.go — Default limits of 1 MiB - Added LabelNameLengthLimit: 1 << 20 and LabelValueLengthLimit: 1 << 20 to DefaultGlobalConfig - Added fallback logic in GlobalConfig.UnmarshalYAML so a partial global config (one that doesn't set these fields) still gets the defaults, matching the pattern already used for ScrapeInterval, EvaluationInterval, etc. 2. scrape/scrape.go — Pre-construction panic prevention Added a check before p.Labels(&lset) that rejects metrics whose raw text exceeds 16 MiB. Since the encoding format panics for any individual string > 16 MiB, and no single label value can be longer than the total metric string, len(met) > 1<<24 is a safe guard. The configured 1 MiB limit then catches the normal cases in verifyLabelLimits. 3. storage/remote/write_handler.go — Configurable limits in RW receiver - Added labelNameLengthLimit and labelValueLengthLimit fields to writeHandler - Added two int parameters to NewWriteHandler - Added checkLabelLengths() helper that enforces the limits - Wired the check into both v1 write() and v2 appendV2() paths 4. web/api/v1/api.go — Wires global config limits to RW handler Passes cfg.GlobalConfig.LabelNameLengthLimit and LabelValueLengthLimit from the config to NewWriteHandler at construction. 5. promql/functions.go — Guards in label_replace and label_join Added explicit checks for the 16 MiB encoding limit before the lb.Labels() call. The evaluator's existing defer ev.recover() already converts panics to errors, but these checks give a clear, explicit error message instead. Tests - Updated config/config_test.go to expect the new 1 MiB defaults in TestGetScrapeConfigs - Updated all ~16 NewWriteHandler call sites in write_handler_test.go to pass 0, 0 (no limit) for the new parameters
Pasarus
reviewed
Mar 25, 2026
Pasarus
left a comment
There was a problem hiding this comment.
The commit is not signed and therefore CI is failing.
Pasarus
reviewed
Mar 25, 2026
| MetricNameEscapingScheme: model.AllowUTF8, | ||
| // Default to 1 MiB to avoid crashes from the 16 MiB encoding limit. | ||
| LabelNameLengthLimit: 1 << 20, | ||
| LabelValueLengthLimit: 1 << 21, |
There was a problem hiding this comment.
Seemingly this is set to 2MiB is this not supposed to be 1MiB?
|
Release notes seem to be missing, unsure if it's appropriate. |
|
Issue is prometheus#16525 |
Pasarus
reviewed
Mar 25, 2026
| return fmt.Errorf("%w for global config", err) | ||
| } | ||
| } | ||
| if gc.LabelNameLengthLimit == 0 { |
There was a problem hiding this comment.
LabelValueLengthLimit also isn't set in the default perhaps we should be adding this default there aswell.
Pasarus
reviewed
Mar 25, 2026
| } | ||
| strval := strings.Join(srcVals, sep) | ||
| if len(strval) > 1<<24 { | ||
| ev.errorf("label_join: joined value too long (%d bytes)", len(strval)) |
There was a problem hiding this comment.
Perhaps mentioning that the combined value now exceeds the sane limit and state that limit?
Pasarus
reviewed
Mar 25, 2026
| } else { | ||
| // The label encoding format cannot represent strings longer than | ||
| // 2^24-1 bytes. Reject before constructing Labels to avoid a panic. | ||
| if len(met) > 1<<24 { |
There was a problem hiding this comment.
This doesn't do 1^24-1 bytes. It does 1<<24+1.
Suggested change
| if len(met) > 1<<24 { | |
| if len(met) >= 1<<24 { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note: this PR is an example for use in the KubeCon EU ContribFest session. Please leave if you are not in that session.
Added a check before p.Labels(&lset) that rejects metrics whose raw text exceeds 16 MiB. Since the encoding format panics for any individual string >
16 MiB, and no single label value can be longer than the total metric string, len(met) > 1<<24 is a safe guard. The configured 1 MiB limit then
catches the normal cases in verifyLabelLimits.
Passes cfg.GlobalConfig.LabelNameLengthLimit and LabelValueLengthLimit from the config to NewWriteHandler at construction.
Added explicit checks for the 16 MiB encoding limit before the lb.Labels() call. The evaluator's existing defer ev.recover() already converts panics
to errors, but these checks give a clear, explicit error message instead.
Tests
Which issue(s) does the PR fix:
Does this PR introduce a user-facing change?