Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

envoy prometheus endpoint fails promlint #2597

Closed
vjsamuel opened this issue Feb 13, 2018 · 4 comments · Fixed by #2757
Closed

envoy prometheus endpoint fails promlint #2597

vjsamuel opened this issue Feb 13, 2018 · 4 comments · Fixed by #2757
Labels

Comments

@vjsamuel
Copy link

Title: envoy prometheus endpoint fails promlint

Description:

The admin endpoint of envoy when queried as a prometheus endpoint fails promlint. Promlint is a sanity check tool offered by Prometheus to check the correctness of a prometheus endpoint.

[optional Relevant Links:]

About promlint: Promlint is a metrics sanity checker exposed as part of promtool. As per their documentation:

Pass Prometheus metrics over stdin to lint them for consistency and correctness.
examples:
$ cat metrics.prom | promtool check metrics
$ curl -s http://localhost:9090/metrics | promtool check metrics
`)

The error reported by promlint is as follows:

curl -s https://gist.githubusercontent.com/vjsamuel/5898d02f8039d8d1bae4d8642182d1c3/raw/deff0c16f746bb2ab7a959c4d5ed9856d17c425d/gistfile1.txt | ./promtool check metrics
error while linting: text format parsing error in line 9: second TYPE line for metric name "envoy_listener_http_downstream_rq_xx", or TYPE reported after samples

Can this be fixed please so that the prometheus endpoint abides to the standard?

@dio
Copy link
Member

dio commented Feb 19, 2018

This is interesting, while I'm not familiar with the code, should we fix this in tags level (how we tag things) or just how we present this as what Prometheus wants? For the latter I can see that we can have: envoy_listener_http_downstream_rq_5xx as metric_name for a tag with envoy_response_code_class="5", hence no duplication in TYPEs.

@vjsamuel
Copy link
Author

The TYPE should be represented only once in the endpoint per unique metric name. Hence while looping through the counters/gauges there should be a check in place to do it only once. There might be other lint based errors once this is fixed. The entire endpoint should be reviewed for compliance hopefully with this issue.

@dio
Copy link
Member

dio commented Feb 24, 2018

In this case, should we have an "integration" to promlint on designing the test scenario?

pitiwari added a commit to pitiwari/envoy that referenced this issue Mar 7, 2018
       Destructor failig while calling derived class function envoyproxy#2722
Description:

The admin endpoint of envoy when queried as a prometheus endpoint fails promlint.
Promlint is a sanity check tool offered by Prometheus to check the correctness
of a prometheus endpoint.

following issues were fixed
-fixed issue with metric type being prtinted twice
-added unit test as well
-fixed issue in destructors of CounterImpl and GuageImpl
-changed thread_local_store_test to remove check for count of free func calls

Signed-off-by: Piyush Tiwari <pitiwari@ebay.com>
pitiwari added a commit to pitiwari/envoy that referenced this issue Mar 12, 2018
Description:

The admin endpoint of envoy when queried as a prometheus endpoint fails promlint.
Promlint is a sanity check tool offered by Prometheus to check the correctness
of a prometheus endpoint.

Signed-off-by: pitiwari <pitiwari@ebay.com>
htuch pushed a commit that referenced this issue Mar 16, 2018
Description: Fixes #2597"

The admin endpoint of envoy when queried as a prometheus endpoint fails promlint.
Promlint is a sanity check tool offered by Prometheus to check the correctness
of a prometheus endpoint.

following issues were fixed
-fixed issue with metric type being printed twice
-added unit test as well

Risk Level: Low

Signed-off-by: pitiwari <pitiwari@ebay.com>
@vjsamuel
Copy link
Author

@pitiwari awesome work! thanks for this!

jpsim pushed a commit that referenced this issue Nov 28, 2022
Signed-off-by: Filip Busic <61715767+filip-doordash@users.noreply.github.com>
Signed-off-by: JP Simard <jp@jpsim.com>
jpsim pushed a commit that referenced this issue Nov 29, 2022
Signed-off-by: Filip Busic <61715767+filip-doordash@users.noreply.github.com>
Signed-off-by: JP Simard <jp@jpsim.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants