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

Support prometheus metrics with nginx-module-vts #62

Closed
sysulq opened this issue Dec 9, 2016 · 18 comments
Closed

Support prometheus metrics with nginx-module-vts #62

sysulq opened this issue Dec 9, 2016 · 18 comments

Comments

@sysulq
Copy link

sysulq commented Dec 9, 2016

Recently I came across prometheus, and I tried to implement prometheus metrics with nginx-module-vts.

Here is nginx-vts-exporter, which is heavily used in our system.

Hope this can be helpful, and any questions would be pleasure 😆

@vozlt
Copy link
Owner

vozlt commented Dec 9, 2016

Thank you for publishing your code. 👍
I would like to add your nginx-vts-exporter link in nginx-module-vts's README.md if you allow.

@sysulq
Copy link
Author

sysulq commented Dec 9, 2016

sure 😄

@discordianfish
Copy link

Lemme hijack this issue.. @vozlt With prometheus become the defactor standard of metrics exposition, would you be open to add prometheus metrics natively? It's a simple text based format and it wouldn't require users to somehow 'translate' it by using either @hnlq715 exporter or Kubernetes ingress controller which does the same: https://github.com/kubernetes/ingress-nginx/blob/master/docs/examples/customization/custom-vts-metrics-prometheus/README.md

@SuperQ
Copy link
Collaborator

SuperQ commented Feb 21, 2018

This appears to be a dupe of #58.

@sysulq
Copy link
Author

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

@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

@towolf
Copy link

towolf commented Jun 5, 2018

@vozlt I've tested this and it seems pretty good. It seems to make https://github.com/hnlq715/nginx-vts-exporter entirely unnecessary. It compared both and replaced just the metric names and it was basically a drop-in replacement. Only some connection states in nginx_vts_main_connections seem to differ:

Separate VTS exporter
image

Native VTS
image

Now, for the future, request times sorted into histogram buckets or percentiles would be great.

@sysulq
Copy link
Author

sysulq commented Jun 6, 2018

nice job,maybe i should encouter this feature in nginx-vts-exporter project

@SuperQ
Copy link
Collaborator

SuperQ commented Jun 6, 2018

@towolf Looks good. The connections graph doesn't seem far off, I'm guessing it's a difference of polling jitter. What scrape interval are you using there? Maybe try speeding it up to something like 5-10s to get higher resolution on the gauge. Gauges are inherently sensitive to polling jitter.

It would be useful to have some counters, like nginx_connections_total.

I sent a couple of PRs to do some best practices cleanup, but otherwise this looks pretty good.

@discordianfish
Copy link

Now, for the future, request times sorted into histogram buckets or percentiles would be great.
@vozlt Is that something you would be willing to add? I guess it's a bit more complicated and the bucket sizes would need to be configurable. Here are a few words on how it looks like: https://prometheus.io/docs/concepts/metric_types/#histogram

But basically the vts module would have to observe the duration of each request and add it to the right bucket. Pre-sampled averages and cut-off samples like exposed right now are unfortunately not enough to get any meaningful averages (see sysulq/nginx-vts-exporter#43 (comment))

@vozlt
Copy link
Owner

vozlt commented Jun 7, 2018

@discordianfish I will check out the histogram of prometheus soon.
Thanks.

@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

@towolf
Copy link

towolf commented Jun 10, 2018

Tested this again, and from what I can see at first glance, this works as it should.

Grafana can directly present histogram data as is as a heatmap. And Prometheus can transform histogram buckets into percentiles using this approximative function.

I only have sparse data at the moment:

image

Pretty great, thanks.

@discordianfish
Copy link

This is great, I'll test it and report back!

@draskolnikova
Copy link

@vozlt @hnlq715

I've seen upstreame state (down|up) in json format, would you mind to expose that in prometheus format?

@yangboyd
Copy link

yangboyd commented Dec 6, 2018

@towolf Would you please share your grafana dashboard?
Thanks a lot!

Tested this again, and from what I can see at first glance, this works as it should.

Grafana can directly present histogram data as is as a heatmap. And Prometheus can transform histogram buckets into percentiles using this approximative function.

I only have sparse data at the moment:

image

Pretty great, thanks.

@towolf
Copy link

towolf commented Dec 6, 2018

@yangboyd sure, here you go

But this dashboard is pretty specific for our old Kubernetes ingress setup, which at the time was a stand-alone thing.

But you can check it out, if you want.

@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