Fix SanitizeLabelName for certain edge case invalid labels #11936
Conversation
SanitizeLabelName was not correctly sanitizing label names that: 1. Started with a digit (0-9) 2. Were empty This commit changes the santization code to catch both of these edge cases and adds new tests to validate it works correctly in them both. In the first case, a leading digit will be replaced with an underscore, and in the latter, the function will return a single underscore. Signed-off-by: Nick Moore <[email protected]>
|
Thanks for your pull request. What issue is this fixing? In Prometheus, this function is used to append label names after __meta labels, therefore it is fine to be empty or to start with a number. It would be a breaking change as is. |
|
That makes sense - thanks for the context @roidelapluie. I think the issue is that the name of the function implies (at least it does to me) that it would sanitize any label name, and other projects (e.g., promtail) that depend on Prometheus appear to be using it in that way. Would you prefer it if I instead:
|
|
That sounds good to me |
Rather than removing the previous implementation of SanitizeLabelName, offer another version named SanitizeFullLabelName that achieved the desired requirements, without breaking existing Prometheus code. Update testing to validate correctness of new variant. Signed-off-by: Nick Moore <[email protected]>
kelnage
force-pushed
the
sanitize-labels-edge-cases
branch
from
03b79a7 to
c05ebef
Compare
|
@roidelapluie I went with option 2 - hope that looks more reasonable. Not entirely sure why the tests are failing, given my changes should have minimal effect (if any) on the existing code base. |
|
Thanks! |
SanitizeLabelName does not correctly sanitize label names that:
This commit changes the santization code to catch both of these edge cases and adds new tests to validate it works correctly in them both. In the first case, a leading digit will be replaced with an underscore, and in the latter, the function will return a single underscore.
@roidelapluie
Signed-off-by: Nick Moore [email protected]