Skip to content

Allow Prometheus label values to contain - and .#3830

Merged
htuch merged 1 commit intoenvoyproxy:masterfrom
JonathanO:revert-incorrect-prometheus-label-value-sanitize
Jul 10, 2018
Merged

Allow Prometheus label values to contain - and .#3830
htuch merged 1 commit intoenvoyproxy:masterfrom
JonathanO:revert-incorrect-prometheus-label-value-sanitize

Conversation

@JonathanO
Copy link
Copy Markdown
Contributor

This reverts commit c2751df (PR #3049)

Description:
The originally reported bug (#3019) was due to metric names not being sanitized, which I think was resolved by #2303.
Change #3049, which was intended to fix #3019, erroneously applied sanitization to label values instead. Prometheus label values are permitted to contain any Unicode char, so this change was unnecessary and leads to an unexpected change in e.g. cluster names.

This PR reverts the change, so that - and . will be preserved in label values.

Risk Level: low

… issue envoyproxy#3019 (envoyproxy#3049)"

This reverts commit c2751df.

Prometheus label values are permitted to contain any Unicode char. The
original bug (envoyproxy#3019) was due to metric names not being sanitized, which
was resolved by envoyproxy#2303. Change envoyproxy#3049 was therefore unnecessary and
leads to an unexpected change in label values.

Signed-off-by: Jonathan Oddy <jonathan.oddy@transferwise.com>
@JonathanO JonathanO force-pushed the revert-incorrect-prometheus-label-value-sanitize branch from 36a74c1 to e1e9db1 Compare July 10, 2018 16:40
@htuch htuch self-assigned this Jul 10, 2018
@htuch htuch merged commit 9333f23 into envoyproxy:master Jul 10, 2018
@JonathanO JonathanO deleted the revert-incorrect-prometheus-label-value-sanitize branch July 11, 2018 08:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

stats: envoy prometheus endpoint fails promlint due to format issue

2 participants