jonathanio / monitoring-munin-haproxy

Extensive Munin plugin for monitoring HAProxy services.
GNU General Public License v2.0
9 stars 4 forks source link

Illegal division by Zero at Line 589 #1

Closed ericzc0423 closed 8 years ago

ericzc0423 commented 8 years ago

2016/03/22-19:00:02 [1983] Error output from haproxy: 2016/03/22-19:00:02 [1983] do_fetch: Illegal division by zero at /etc/munin/plugins/haproxy line 589. 2016/03/22-19:00:02 [1983] at /etc/munin/plugins/haproxy line 144. 2016/03/22-19:00:02 [1983] Service 'haproxy' exited with status 255/0.

Seems the count should start 1, or the ++$count is not working.

583 my ($count, $total) = (0,0); 584 foreach my $server (sort keys %{$data{$service}{'servers'}}) { 585 ++$count; 586 $total+=$data{$service}{'servers'}{$server}{'timing'}{'total'}; 587 } 588 589 $values{$service} = ($total/$count);

flintforge commented 8 years ago

if count start at 1, count would be 2 for one service in one server. Is that correct ? what about 588 $count=1 if $count==0; ?

ericzc0423 commented 8 years ago

My scripting is not very good, but after changing to 1, it is fixed my issues.

On Thu, Apr 14, 2016 at 10:20 AM, flintforge notifications@github.com wrote:

if count start at 1, count will be 2 for one service in one server. Is that correct ? what about 588 $count=1 if $count==0; ?

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub https://github.com/jonathanio/monitoring-munin-haproxy/issues/1#issuecomment-209726955

Thanks Eric Zhang

Skype: eric.zhang1008 Mobile: 008615000723581 Email: eric.zc0423@gmail.com

You are warmly welcome to visit my website: http://www.itopslife.com, the best practice for OPERATION.

geertn commented 8 years ago

Do you happen to have a stats listener configured? That will generate stats for a backend without any servers resulting in the error. The following quick fix made it work for me:

otalb0 20161108-17:15:58 /opt # diff -u haproxyng munin-plugins/haproxyng 
--- haproxyng   2016-11-08 17:15:58.877352196 +0100
+++ munin-plugins/haproxyng 2016-11-08 17:15:11.716604569 +0100
@@ -585,8 +585,9 @@
           ++$count;
           $total+=$data{$service}{'servers'}{$server}{'timing'}{'total'};
         }
-
-        $values{$service} = ($total/$count);
+        if($count != 0){
+          $values{$service} = ($total/$count);
+        }
       }

       mg_fetch(TYPE_BACKEND, ['timing'], \%values);
jonathanio commented 8 years ago

Sorry. I've no idea how I was never e-mailed about this until now.

I think @geertn is right - there's certainly a bad assumption on my part that should you select a type to be checked you have at least one of it. However, I supect it should be more like:

--- haproxyng
+++ munin-plugins/haproxyng
@@ -585,8 +585,8 @@
           ++$count;
           $total+=$data{$service}{'servers'}{$server}{'timing'}{'total'};
         }

-        $values{$service} = ($total/$count);
+        $values{$service} = ($count > 0 ? $total/$count : 0);
       }

       mg_fetch(TYPE_BACKEND, ['timing'], \%values);

This at least ensures that $values{$service} is set for any following code and required checks are properly conducted against the service. I'll try and fix it on the weekend.

Todo

jonathanio commented 8 years ago

@ericzc0423, @geertn,

I've added a commit which should prevent this error, and I cannot see anywhere else where I use a divisor, so which should be the only place that needs a check. Please feel free to test the version in PR #3 and let me know if this fixes your problem.