sensu-plugins / sensu-plugins-graphite

This plugin provides native Graphite instrumentation for monitoring, including: replication status, various Graphite data queries, mutators, and handlers
http://sensu-plugins.io
MIT License
22 stars 44 forks source link

Fix comparison of number against array #15

Closed jennytoo closed 8 years ago

jennytoo commented 8 years ago

_last_graphitemetric returns the last datapoint as a list of up to count datapoint [value, timeseries] pairs; where count is 1. Thus we get following back: {target => [[value, timestamp]], ...}

the code was treating this instead as a single datapoint pair, not a list of datapoint pairs, resulting in checking number < list

instead use _last_graphitevalue which returns the mean of all values returned by _last_graphitemetric as a single value, giving us instead {target => mean, ...} where again count=1

jennytoo commented 8 years ago

Upstream:

bin/check-graphite.rb --host graphite.example.tld --target 'backend_threads.threads.current' --last 600
Check failed to run: comparison of Float with Array failed, ["bin/check-graphite.rb:446:in `>'", "bin/check-graphite.rb:446:in `block (2 levels) in check_last'", "bin/check-graphite.rb:441:in `each'", "bin/check-graphite.rb:441:in `block in check_last'", "bin/check-graphite.rb:437:in `each'", "bin/check-graphite.rb:437:in `check_last'", "bin/check-graphite.rb:482:in `block in run'", "bin/check-graphite.rb:473:in `each'", "bin/check-graphite.rb:473:in `run'", "/opt/local/lib/ruby2.0/gems/2.0.0/gems/sensu-plugin-1.2.0/lib/sensu-plugin/cli.rb:56:in `block in <class:CLI>'"]

Patched:

bin/check-graphite.rb --host graphite.example.tld --target 'backend_threads.threads.current' --last 600
Graphite WARNING: The metric backend_threads.threads.current is 536.0 that is less than max allowed 600

I do notice a minor phrasing issue - while the logic is consistent with --average_value, the wording is misleading. It would be better to say 'less than min allowed'.

bin/check-graphite.rb --host graphite.example.tld --target 'backend_threads.threads.current' --average_value 600,600
Graphite CRITICAL: The average value of metric backend_threads.threads.current is 129.84745762711864 that is less than allowed average of 600

bin/check-graphite.rb --host graphite.example.tld --target 'backend_threads.threads.current' --last 600,600
Graphite CRITICAL: The metric backend_threads.threads.current is 516.0 that is less than max allowed 600

bin/check-graphite.rb --host graphite.example.tld --target 'backend_threads.threads.current' --average_value 100,100 --greater_than
Graphite CRITICAL: The average value of metric backend_threads.threads.current is 129.84745762711864 that is greater than allowed average of 100

bin/check-graphite.rb --host graphite.example.tld --target 'backend_threads.threads.current' --last 100,100 --greater_than
Graphite CRITICAL: The metric backend_threads.threads.current is 516.0 that is greater than max allowed 100

The above snippets also give an example of how to reproduce this - and validate the changes. Note these were all done with ruby 2.0

sstarcher commented 8 years ago

@jennytoo I tested it out everything looks great can you update the changelog for your fix.

analytically commented 8 years ago

Bump.