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

add prometheus display format #58

Closed
andrewsmedina opened this issue Oct 16, 2016 · 18 comments
Closed

add prometheus display format #58

andrewsmedina opened this issue Oct 16, 2016 · 18 comments

Comments

@andrewsmedina
Copy link

andrewsmedina commented Oct 16, 2016

Prometheus (prometheus.io) is a very powerfull open-source monitoring solution. Prometheus get the data scraping metrics from the applications.

To use prometheus with nginx-module-vts is a good way to monitoring and aggregate metrics from more than one nginx servers.

A way to do it is the nginx-module-vts support prometheus as a display format.

@vozlt
Copy link
Owner

vozlt commented Oct 20, 2016

Hi, Thanks for your suggestion.
I'll get it checked soon.

@vozlt
Copy link
Owner

vozlt commented Dec 9, 2016

Hi, the prometheus exporter for nginx-module-vts has been released by hnlq715 in issue #62.
Try to nginx-vts-exporter.

The screenshot nginx_server_connections as follows:
nginx-vts-exporter

@gianrubio
Copy link

@vozlt I'm already using nginx-vts-exporter to collect prometheus metrics. It's really easy to use.
My question is why not merge the nginx-vts-exporter with this project and have prometheus metrics by default?

@vozlt
Copy link
Owner

vozlt commented Dec 22, 2016

@gianrubio Thanks for your good suggestion.
I have never used prometheus but I installed it because of this issue.
So I don't know detail about it yet.
But I think that it is better to make it a 3rd-party module like nginx-vts-exporter outside nginx-module-vts to do not affect nginx performance.

@gianrubio
Copy link

@vozlt prometheus could be an another format for the stats. It'll be a new endpoint like status/format/prometheus. The kubernetes project are using your module for the nginx ingress controller. I'm really interested to make the prometheus format easy to use for everyone.

It'll be similar to this screenshot.

screen shot 2016-12-21 at 15 21 42

kubernetes/ingress-nginx#72

@hnlq715 what do you think?

@sysulq
Copy link

sysulq commented Dec 23, 2016

The requested feature may break the internal design in vozlt/nginx-module-vts. In my oppinion, it's much wiser to start a new project for nginx-prometheus-metrics, or use hnlq715/nginx-vts-exporter instead.

@gianrubio
Copy link

@hnlq715, could you explain why this will break the design?

@pavelnemirovsky
Copy link

Guys did u merge this PR to master?

@trnl
Copy link

trnl commented Dec 29, 2017

Guys, I also agree that it will not break any designs in nginx-module-vts. Prometheus is just another output similar to existing json, jsonp and html and rendering of it will be a no brainer.

@sysulq
Copy link

sysulq commented Jan 1, 2018

@gianrubio sorry to reply your question so lately...

Prometheus in vts should use histogram instead of gauge type for time metrics, not like hnlq715/nginx-vts-exporter, which means a huge change for vts.

Another choice here would be hnlq715/nginx-prometheus-metrics, which use lua embedded in nginx.

@trnl
Copy link

trnl commented Jan 2, 2018

@hnlq715, I agree that this will be a change for vts, however, I believe histograms are not mandatory for now and simple nginx-vts-exporter style gauge output will be fine. I believe that was an original suggestion by @gianrubio.

@sysulq
Copy link

sysulq commented Jan 3, 2018

@trnl @vozlt @gianrubio
I agree, without supporting histogram type, this change would be acceptable.

But it would be great to support histogram for time metrics, and we could achieve this with lua or c module. After all, we could have p90, p95 or p99 request/response time metrics... or even apdex score

@sysulq
Copy link

sysulq commented Feb 21, 2018

Hope this could help you guys, simple lua code generates nginx's prometheus metrics.
https://github.com/hnlq715/nginx-prometheus-metrics

@trnl
Copy link

trnl commented Feb 21, 2018

@hnlq715, actually yes. We rely on that now. However it will be nice to have the same stuff in nginx-ingress-controller for Kubernetes.

@ghost
Copy link

ghost commented Mar 15, 2018

#114

@vozlt
Copy link
Owner

vozlt commented Jun 1, 2018

That feature has added.
All mertric names are started with nginx_vts_(*).
I can not guarantee that it will work well because I know little about prometheus.
Please test it.
Thanks.

Latest commit: 2f9434f

@vozlt
Copy link
Owner

vozlt commented Jun 10, 2018

The metric type of histogram in format/prometheus has added.
See the directive vhost_traffic_status_histogram_buckets for details.
I quickly added the feature and did not test it much.
So it is necessary to verify that I have implemented the function correctly according to prometheus histogram metric type.
Please test it.
Thanks.

Latest commit: 2fd3ae4

@hadret
Copy link

hadret commented Apr 2, 2019

Overall format/prometheus appears to work very well. I haven't test the histogram buckets yet, but I noticed some other things missing when it comes to the upstreams part:

  • State of the backend is not provided
  • Weight of the backend is not provided
  • MaxFails of the backend is not provided
  • FailTimeout of the backend is not provided

All of the above are very useful to have and are provided by other formats.

@vozlt vozlt closed this as completed Jan 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants