Truncate HELP metadata on ingest #2993
Conversation
Currently we drop the HELP metadata and return a 400 error even though we ingest the related series. This is confusing and not very productive. Truncate the HELP instead. Ref: support escalation 3976 Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
pkg/util/validation/validate.go
Outdated
| if cfg.EnforceMetadataMetricName(userID) && metadata.GetMetricFamilyName() == "" { | ||
| DiscardedMetadata.WithLabelValues(reasonMissingMetricName, userID).Inc() | ||
| return newMetadataMetricNameMissingError() | ||
| } | ||
|
|
||
| maxMetadataValueLength := cfg.MaxMetadataLength(userID) | ||
|
|
||
| if len(metadata.Help) > maxMetadataValueLength { | ||
| metadata.Help = metadata.Help[:maxMetadataValueLength] |
There was a problem hiding this comment.
HELP can be any UTF-8 bytes. I think we run the risk of corrupting it by using slice operations like this.
There was a problem hiding this comment.
I've added a fix that does a for loop but still uses slicing to avoid allocating memory
There was a problem hiding this comment.
I'm not sure it's a good idea to have this loop, plus we don't check if the input it utf-8 (I don't think), so maybe it's ok to just slice the string after all?
There was a problem hiding this comment.
I'm not sure what the loops solves. A bit of searching makes me think that "runes" are the right way to deal with this. That also matches what Prometheus does when dealing with exemplar labels. I don't feel particularly strongly about this but it seems prone to be an issue since HELP text is where users are most likely to put multi-byte characters.
There was a problem hiding this comment.
range on strings works on runes, not bytes, so this loop finds index of last rune that can still be added to be within the size limit. It seems fine to me. I would keep it as-is, so that users don't get corrupted texts.
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
krajorama
force-pushed
the
krajo/20220920-turncate-HELP
branch
from
2ff344a
to
a14c0b7
Compare
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
docs/sources/operators-guide/architecture/components/distributor.md
Outdated
Show resolved
Hide resolved
What this PR does
Currently we drop the HELP metadata and return a 400 error even though we ingest the related series. This is confusing and not very productive.
Truncate the HELP instead.
Which issue(s) this PR fixes or relates to
Ref: support escalation 3976
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]