Allow precision to be set in nginx collector

[scottcunningham]

This patch allows us to set precision in the Nginx collector's config file. Eg:

enabled = True
precision = 2
req_port = 9080

Changing the precision from 0 to 2 resulted in this change on a testing system when viewing the data in Grafana:

Note that the number of requests/sec hasn't changed, just the precision config value.

When you hover over a datapoint in Grafana:

• Some point in time before the patch:

  

• After the patch and changing the config value (with requests coming in at the same rate):

  

[scottcunningham]

Oh dear, failing tests! They look like some pretty obvious PEP-8 violations. I've got max-line-length=120 set in my flake8 config file so it didn't pick them up. I'll get them fixed up right now.

As an aside, are there any docs I can refer to about running the tests locally?

[coveralls]

Coverage remained the same at 59.386% when pulling 11cdeff on Ensighten:nginx_collector_precision_fix into fa8da84 on python-diamond:master.

[shortdudey123]

add to the config_help section to

[shortdudey123]

Doc needs to be updated too

[scottcunningham]

Thanks for the feedback! I'm away from a computer for the weekend but will address the issues on Monday.

On 29 Jul 2016 18:55, Grant Ridder notifications@github.com wrote:

Doc needs to be updated too

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

[scottcunningham]

Docs and config_help updated. I used the same wording for precision as in the NTP collector.

[coveralls]

Coverage remained the same at 59.386% when pulling 2298b04 on Ensighten:nginx_collector_precision_fix into fa8da84 on python-diamond:master.

[shortdudey123]

LGTM
@jaingaurav can you review?

[scottcunningham]

Hey @shortdudey123, @jaingaurav, do you know when you'll have a chance to take another look at this? Thanks!

[shortdudey123]

Already looks good to me
Waiting on @jaingaurav or another python-diamond member to merge

[rowleyaj]

We've had these changes running in our environment for a while now. Any chance someone can get this merged into master, or provide feedback as to what changes are needed?

[scottcunningham]

@jaingaurav Any chance you could take a quick look at this please? It's been ready and waiting for a few months now.

[shortdudey123]

@bmhatfield can you review when you have a change? :)

[rowleyaj]

@bmhatfield @shortdudey123 @josegonzalez Any updates on what's needed to get this PR merged. This is now the only custom part of the diamond install that we have.

[josegonzalez]

Seems legit, will go out in the next release when I have a moment.