nginx_exporter

[Anton Tolchanov]

Hi!

I spent some time coding a prometheus exporter for Nginx in Lua.
It exposes request count (grouped by http code) and latency histogram, but can easily be used to track other counters and histograms.
Code is available at https://github.com/knyar/nginx_exporter

I would appreciate if someone from Prometheus team could take a look and let me know if I made any obvious mistakes.

Thanks,
Anton

[Brian Brazil]

This sounds more like a client library mixed with a custom collector than an exporter, I'd suggest reading both https://docs.google.com/document/d/1zHwWVigeAITbaAp6BR4uCByRJH7rtTv4ve6SsoEXJ_Q/edit#heading=h.9mfav7ntoa5r and https://docs.google.com/document/d/1JapuiRbp-XoyECgl2lPdxITrhm5IyCUq9iA_h6jp3OY/edit

  -- "le" label should be the last one to ensure that all buckets for a given

  -- metric are exposed together when all metrics get sorted.

This doesn't sound right, that constraint is too strict.

You appear to be allowing nginx_http_requests_total to have multiple distinct sets of labels - that's not a good idea. You need to create a new metric with it's own name for every usage of prometheus:measure.

Brian

 

Thanks,
Anton

--
You received this message because you are subscribed to the Google Groups "Prometheus Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to prometheus-devel...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

Brian Brazil

www.robustperception.io

[Anton Tolchanov]

Thanks for taking a look, Brian.

Code is available at https://github.com/knyar/nginx_exporter

This sounds more like a client library mixed with a custom collector than an exporter

I agree. I will probably rename the repo to nginx-lua-prometheus, to avoid confusion with an existing nginx_exporter.

I'd suggest reading both https://docs.google.com/document/d/1zHwWVigeAITbaAp6BR4uCByRJH7rtTv4ve6SsoEXJ_Q/edit#heading=h.9mfav7ntoa5r and https://docs.google.com/document/d/1JapuiRbp-XoyECgl2lPdxITrhm5IyCUq9iA_h6jp3OY/edit

Thanks. It seems that I need to add some additional safety checks that would prevent:

- using the same metric name with different sets of labels;

- using the same histogram metric with different bucketers.

  -- "le" label should be the last one to ensure that all buckets for a given

  -- metric are exposed together when all metrics get sorted.

This doesn't sound right, that constraint is too strict.

This is just an implementation detail. I don't think Prometheus server imposes a requirement for all buckets for a given set of labels to be reported as one group, but this makes the /metrics page a bit nicer to look at.

-- Anton

[Anton Tolchanov]

I'd suggest reading both https://docs.google.com/document/d/1zHwWVigeAITbaAp6BR4uCByRJH7rtTv4ve6SsoEXJ_Q/edit#heading=h.9mfav7ntoa5r and https://docs.google.com/document/d/1JapuiRbp-XoyECgl2lPdxITrhm5IyCUq9iA_h6jp3OY/edit

Thanks. It seems that I need to add some additional safety checks that would prevent:

- using the same metric name with different sets of labels;

- using the same histogram metric with different bucketers.

It seems that the best way to implement this would be to require all metrics to be explicitly declared in advance.

I am planning to implement an API like this:

init_by_lua:

metrics = require("prometheus").init("prometheus_metrics")

metrics:register_counter("nginx_http_request_size_bytes")  -- counter with no labels

metrics:register_counter("nginx_http_requests_total", {"host, "status"})  -- counter with labels

metrics:register_histogram("nginx_http_request_duration_seconds", {"host"})  -- histogram with labels

metrics:register_histogram("nginx_http_response_size_bytes", {"host"}, {10,100,1000,10000,100000,1000000})  -- histogram with custom bucketer

log_by_lua:

metrics.c["nginx_http_request_size_bytes"]:inc(tonumber(ngx.var.request_length))

metrics.c["nginx_http_requests_total"]:inc(1, {"host1", ngx.var.status})

metrics.h["nginx_http_request_duration_seconds"]:observe(ngx.now() - ngx.req.start_time(), {"host1"})

metrics.h["nginx_http_response_size_bytes"]:observe(tonumber(ngx.var.bytes_sent), {"host2"})

Please let me know if you have any comments.

- Anton

[Brian Brazil]

On 22 March 2016 at 10:40, Anton Tolchanov <m...@knyar.net> wrote:

I'd suggest reading both https://docs.google.com/document/d/1zHwWVigeAITbaAp6BR4uCByRJH7rtTv4ve6SsoEXJ_Q/edit#heading=h.9mfav7ntoa5r and https://docs.google.com/document/d/1JapuiRbp-XoyECgl2lPdxITrhm5IyCUq9iA_h6jp3OY/edit

Thanks. It seems that I need to add some additional safety checks that would prevent:

- using the same metric name with different sets of labels;

- using the same histogram metric with different bucketers.

It seems that the best way to implement this would be to require all metrics to be explicitly declared in advance.

That's generally how client libraries should be written.

 

I am planning to implement an API like this:

init_by_lua:

metrics = require("prometheus").init("prometheus_metrics")

metrics:register_counter("nginx_http_request_size_bytes")  -- counter with no labels

metrics:register_counter("nginx_http_requests_total", {"host, "status"})  -- counter with labels

metrics:register_histogram("nginx_http_request_duration_seconds", {"host"})  -- histogram with labels

metrics:register_histogram("nginx_http_response_size_bytes", {"host"}, {10,100,1000,10000,100000,1000000})  -- histogram with custom bucketer

log_by_lua:

metrics.c["nginx_http_request_size_bytes"]:inc(tonumber(ngx.var.request_length))

Usually you'd keep the metric object created in a variable and use that, rather than having to do a map lookup.

 

metrics.c["nginx_http_requests_total"]:inc(1, {"host1", ngx.var.status})

metrics.h["nginx_http_request_duration_seconds"]:observe(ngx.now() - ngx.req.start_time(), {"host1"})

metrics.h["nginx_http_response_size_bytes"]:observe(tonumber(ngx.var.bytes_sent), {"host2"})

Api wise it'd usually be something like myCounter.Labels("host2").inc(). This allows for caching of the children, once again avoiding map lookups (may not help in this situation as you presumably don't know the label values in advance).

Brian

 

Please let me know if you have any comments.

- Anton

--

Brian Brazil

www.robustperception.io

[Anton Tolchanov]

Usually you'd keep the metric object created in a variable and use that, rather than having to do a map lookup.

Thanks for the feedback, Brian.

I've changed the module to do that: https://github.com/knyar/nginx-lua-prometheus

- Anton