validation: allow more labels for _info metrics by default #10028
Conversation
The info metrics are generally low cardinality even though there are many informational labels. This is to support conversion from OTLP attributes easier. Signed-off-by: György Krajcsovits <[email protected]>
Signed-off-by: György Krajcsovits <[email protected]>
Signed-off-by: György Krajcsovits <[email protected]>
|
|
||
| This non-critical error occurs when Mimir receives a write request that contains an info series with a number of labels that exceed the configured limit. | ||
| An info series is a series where the metric name ends in `_info`. | ||
| The limit protects the system’s stability from potential abuse or mistakes. To configure the limit on a per-tenant basis, use the `-validation.max-label-names-per-info-series` option. |
There was a problem hiding this comment.
To configure per tenant, you need YAML don't you rather than the flag:
| The limit protects the system’s stability from potential abuse or mistakes. To configure the limit on a per-tenant basis, use the `-validation.max-label-names-per-info-series` option. | |
| The limit protects the system’s stability from potential abuse or mistakes. To configure the limit on a per-tenant basis, use the `validation.max_label_names_per_info_series` option. |
There was a problem hiding this comment.
ack, I'll update this and where I copy pasted from above as well
There was a problem hiding this comment.
also it becomes limits.max_label_names_per_info_series I think
There was a problem hiding this comment.
Actually there's 10 places where we use -validation... flag and zero where we write yaml option. Do you want to change all?
There was a problem hiding this comment.
I certainly wouldn't change existing code as part of this PR. I just don't think it makes sense to refer to a flag, when you have to use YAML per-tenant.
There was a problem hiding this comment.
Nice work, @krajorama . I left a few minor comments and suggestions.
| ### err-mimir-max-label-names-per-info-series | ||
|
|
||
| This non-critical error occurs when Mimir receives a write request that contains an info series with a number of labels that exceed the configured limit. | ||
| An info series is a series where the metric name ends in `_info`. |
There was a problem hiding this comment.
Is info series an abbreviation for information series or is it really just called an info series? If it's an abbreviation, we should spell it out except when using a code-formatted label.
There was a problem hiding this comment.
not an abbreviation, these are called info series/metrics everywhere
|
|
||
| This non-critical error occurs when Mimir receives a write request that contains an info series with a number of labels that exceed the configured limit. | ||
| An info series is a series where the metric name ends in `_info`. | ||
| The limit protects the system’s stability from potential abuse or mistakes. To configure the limit on a per-tenant basis, use the `-validation.max-label-names-per-info-series` option. |
There was a problem hiding this comment.
Nit: I think misuse has a more positive connotation than abuse.
There was a problem hiding this comment.
The doc uses abuse 21 times, and misuse zero times. Let's change in a different PR if we want to.
Co-authored-by: Arve Knudsen <[email protected]> Co-authored-by: Taylor C <[email protected]>
Signed-off-by: György Krajcsovits <[email protected]>
Signed-off-by: György Krajcsovits <[email protected]>
|
Could you rebase on main? |
Signed-off-by: György Krajcsovits <[email protected]>
What this PR does
The info metrics are generally low cardinality even though there are many informational labels. This is to support conversion from OTLP attributes easier.
Which issue(s) this PR fixes or relates to
Related to prometheus/prometheus#15448
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX].about-versioning.mdupdated with experimental features.