search

reachlin / sensu_exporter

a Prometheus exporter for Sensu
Apache License 2.0
8 stars 14 forks source link

Fix the timeout default (was 20ns, is now 20s) #7

Open andsens opened 4 years ago

andsens commented 4 years ago

The current default is easily verifiable via --help:

Usage of /bin/sensu_exporter:
  -api string
        Address to Sensu API. (default "http://localhost:4567")
  -listen string
        Address to listen on for serving Prometheus Metrics. (default ":9251")
  -timeout duration
        Timeout in seconds for the API request (default 20ns)

This PR multiplies the 20 default with time.Seconds, that's it.

EDIT: And the helptext said "in seconds". Since we are using time.Duration now this is not true. It would actually be nanoseconds. So I just removed that part, the default should be an OK hint that you need to use a unit suffix.

cstockton commented 4 years ago

@andsens You may want to revert this, then commit my line here: https://github.com/cstockton/sensu_exporter/blob/master/sensu_exporter.go#L119

This keeps the behavior identical to the way it was before, since some people may pull from this repo and expect seconds. Really the only difference between my pull req with the above change and your original code is I moved the multiplication operation to after the flag parsing, so both the default value provided to the flag and any command line args will both consistently be in seconds.

Thanks for getting back to this though :D We've been using it for a couple years without any issues, good work and thanks!