stats: envoy prometheus endpoint fails promlint due to format issue #3019#3049
stats: envoy prometheus endpoint fails promlint due to format issue #3019#3049mattklein123 merged 3 commits intoenvoyproxy:masterfrom
Conversation
mattklein123
left a comment
mattklein123
left a comment
There was a problem hiding this comment.
LGTM. Tiny nit since you need to fix DCO anyway. Also, please mark "Fixes " in the PR description so the issue this is fixing will auto close. Thanks!
There was a problem hiding this comment.
nit: This comment can be on a single line, start with capital ("If") and end with a period.
…nvoyproxy#3019 Signed-off-by: pitiwari <pitiwari@ebay.com>
pitiwari
force-pushed
the
prom_issue_3019
branch
from
0af0b10 to
5de0753
Compare
…ixes envoyproxy#3019 Signed-off-by: pitiwari <pitiwari@ebay.com>
pitiwari
changed the title
pitiwari
changed the title
pitiwari
changed the title
| } | ||
|
|
||
| TEST(PrometheusStatsFormatter, FormattedTags) { | ||
| // If value has - then it should be replaced by - . |
There was a problem hiding this comment.
I think you mean s/-/_ for the last one?
There was a problem hiding this comment.
yes. please let me know if you want any changes in this comment
There was a problem hiding this comment.
I would change the comment to be accurate. :)
Signed-off-by: pitiwari <pitiwari@ebay.com>
|
Hi, Is there any reason not to revert this? |
… 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>
… issue #3019 (#3049)" (#3830) This reverts commit c2751df. 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 Signed-off-by: Jonathan Oddy <jonathan.oddy@transferwise.com>
3 participants
Fix Envoy route config name for Prometheus metrics
Description: Fixes #3019
Need to move route config name as part of Prometheus metrics label
Prometheus fails to scrap data because of format incompatibility. Having "-" in the tag value causes the failure.
Risk Level: Low