sensu-plugins / sensu-plugins-mysql

This plugin provides native MySQL instrumentation for monitoring and metrics collection, including: service health, database connectivity, `InnoDB` locks, replication status, metrics collection, and sending metrics to a MySQL database.
http://sensu-plugins.io
MIT License
18 stars 62 forks source link

check-mysql-disk.rb error : CheckMysqlDisk CRITICAL: comparison of Float with String failed #66

Open taejoonmoon opened 6 years ago

taejoonmoon commented 6 years ago

I'm not familiar with ruby.

When I run check-mysql-disk.rb, I found this error. "CheckMysqlDisk CRITICAL: comparison of Float with String failed"

$ check-mysql-disk.rb -h 127.0.0.1 -i /etc/sensu/conf.d/mysql.cnf --size 1000
CheckMysqlDisk CRITICAL: comparison of Float with String failed

If I add "to_f" to disk_size, critical_usage, warning_usage, it works well. Is this the right way? If then, I will make a pull request.

$ diff check-mysql-disk.rb test-check-mysql-disk.rb
88,90c88,90
<     disk_size = config[:size]
<     critical_usage = config[:crit]
<     warning_usage = config[:warn]
---
>     disk_size = config[:size].to_f
>     critical_usage = config[:crit].to_f
>     warning_usage = config[:warn].to_f
majormoses commented 6 years ago

Hey sorry for the late response, let me review the code real quick to verify but that looks fairly reasonable to me.

majormoses commented 6 years ago

So that works but I think that it would be better to fix the defaults to not be a string in the first place. We see in multiple locations we know we want to convert it to a float but then we set the default to a string which makes no sense. My conclusion is that the proc block does not run on the default value.

Your fix is totally valid but I think we should remove the quotes from the default and confirm that it works with default and non default values. I look forward to reviewing and getting this fixed.

jtomaszon commented 5 years ago

That's right, putting real number instead of string on default value did the trick