Debugging DBD::Chart

[fuzzyping]

Hi monks- I'm trying to debug a script I wrote which uses the DBD::Chart module for chart creation. Everything works great, but I get the following warning when it runs via cron:

Illegal division by zero at 
/usr/local/libdata/perl5/site_perl/DBD/Chart/Plot.pm line 3626.

Obviously, this doesn't tell me much about what part of my code might be inserting the zero value. The funny thing is, if I run the script manually, I don't get that warning anymore. But I do get this when I enable warnings:

v-string in use/require non-portable at /usr/local/libdata/perl5/site_
+perl/DBD/Chart/Plot.pm line 97.
v-string in use/require non-portable at /usr/local/libdata/perl5/site_
+perl/DBD/Chart.pm line 98.

Any ideas why I'm only seeing the divide-by-zero warning from cron? Both are run as root. Thanks in advance. Here's the code...

#!/usr/bin/perl
#
# Hatchet chart grapher (hatchart)
# v 0.6.2, 2004.02.18
# Jason Dixon <[e-mail]>
# http://www.dixongroup.net/hatchet/
#

use strict;
use DBI;
use DBD::SQLite;
use DBD::Chart;

##################################################
# Configuration Section
##################################################
my $db_file = "/var/db/pflog.db";
my $max_wedges = 5;
my $graphs_dir = "/var/www/htdocs/hatchet/graphs";
my $width = 500;
my $height = 400;
##################################################

my $dbh = DBI->connect("DBI:SQLite:dbname=$db_file", "", "") || die $D
+BI::errstr;
my $date = get_date();

print_graph(get_top_sources(), $graphs_dir, 'hosts_all.png');
print_graph(get_top_services(), $graphs_dir, 'ports_all.png');
print_graph(get_top_sources($$date), $graphs_dir, 'hosts_today.png');
print_graph(get_top_services($$date), $graphs_dir, 'ports_today.png');

sub get_date {
        my $day;
        if ([split(/ /,localtime)]->[2] =~ /^[\d]{1}$/) {
                $day = '0' . [split(/ /,localtime)]->[2];
        } else {
                $day = [split(/ /,localtime)]->[2];
        }
        my $date = [split(/ /,localtime)]->[1] . ' ' . $day;
        return \$date;
}

sub get_top_services {
        my $date = shift || undef;
        my $select_query = "select dst_port from logs";
        $select_query .= " where date like '%$date%'" if ($date);
        my $sth = $dbh->prepare($select_query);
        $sth->execute;
        my %events;
        my $total = 0;
        while (my $result = $sth->fetchrow_hashref) {
                $events{$result->{'dst_port'}}{'count'}++;
                $total++;
        }
        my @sorted = sort {
                $events{$b}{'count'} <=> $events{$a}{'count'}
        } keys %events;
        my @top_entries;
        for (my $i=0; $i<$max_wedges; $i++) {
                my %hash = ($sorted[$i] => $events{$sorted[$i]}{'count
+'});
                push(@top_entries, \%hash);
        }
        push(@top_entries, { 'other' => $total });
        return \@top_entries;
}

sub get_top_sources {
        my $date = shift || undef;
        my $select_query = "select src_host from logs";
        $select_query .= " where date like '%$date%'" if ($date);
        my $sth = $dbh->prepare($select_query);
        $sth->execute;
        my %events;
        my $total = 0;
        while (my $result = $sth->fetchrow_hashref) {
                $events{$result->{'src_host'}}{'count'}++;
                $total++;
        }
        my @sorted = sort {
                $events{$b}{'count'} <=> $events{$a}{'count'}
        } keys %events;
        my @top_entries;
        for (my $i=0; $i<$max_wedges; $i++) {
                my %hash = ($sorted[$i] => $events{$sorted[$i]}{'count
+'});
                push(@top_entries, \%hash);
        }
        push(@top_entries, { 'other' => $total });
        return \@top_entries;
}

sub print_graph {
        my ($data, $dir, $filename) = @_;
        my $dbh = DBI->connect('dbi:Chart:') || die $DBI::errstr;
        my $create_query = "CREATE TABLE pie (source CHAR(15), count F
+LOAT)";
        $dbh->do($create_query);
        my $sth = $dbh->prepare("INSERT INTO pie VALUES (?,?)");
        foreach (@$data) {
                my ($key, $value) = each(%$_);
                $sth->execute($key, $value);
        }
        my $select_query = "SELECT PIECHART FROM pie WHERE WIDTH=? AND
+ HEIGHT=? AND 
                                COLOR IN ('lred', 'lgreen', 'blue', 'y
+ellow', 'marine', 'purple', 
                                                'orange', 'lblue', 'pi
+nk', 'cyan', 'lyellow') AND 
                                BACKGROUND='white' AND 
                                SIGNATURE='Hatchet v.0.6.2, Jason Dixo
+n'";
        my $rsth = $dbh->prepare($select_query);
        $rsth->execute($width, $height);
        my $result = $rsth->fetchrow_arrayref;
        open(BAR, ">$graphs_dir/$filename") || die "Can't open file fo
+r writing: $!";
        binmode BAR;
        print BAR $$result[0];
        close(BAR);
        $dbh->do('DROP TABLE pie');
}

BazB added readmore tags

[tilly]

Look at your installation of DBD::Chart::Plot, and skip to line 3626. That may give you some idea what is happening.

A random guess might be that it is trying to figure out some parameter from an environment variable, and a different environment running from cron causes the division by 0 error. That would explain why you get a different result when running from cron. You can control for that by using RE (tilly) 3: Get default login environment to be sure that you get the same environment running from cron and by hand.

Another idea might be to create an edited version of DBD::Chart that inserts a Carp::cluck right before the warning. Examine the messages emitted. The stack backtrace on the last one before you die tells you which call to print_graph gave a division by 0, which is at least somewhere to start.

FWIW it looks like the warning caused by DBD::Graph is a red herring for your problem (though it should be reported to the author).

UPDATE: Random style tips.

I should point out that there is research indicating that a consistent indent in the 2-4 character range improves comprehension over larger indent.

Also you used a C-style for-loop (for (my $i=0; $i<$max_wedges; $i++) { ... }). The most common coding error in C is an off-by-one error, and if you use them liberally in Perl, you will also have off-by-one errors. Being in the habit of using ranges will avoid those. Though admittedly in this case. for my $i (0..($max_wedges - 1)) { ... } involves more thought about the boundary condition than the C-style loop did.

UPDATE 2: Cunningly hide the fact that I misread the error message by fixing which module I said to examine. Stupidly ruin the effect by confessing to everything.

[fuzzyping]

First, there is no line 3626. That's the first thing I looked for, and found that Chart.pm contains only 3253 lines. Figure that one out.

As far as the red herring, I don't have any other problem. I'm simply trying to determine why I'm seeing these errors, and only via cron.

Update: Damn, I read that module path wrong. I was checking Chart.pm instead of Plot.pm.

[tilly]

It looks like you figured out how Chart.pm can only have 3253 lines. :-) The misreading was mine originally. :-(

And on the red herring, I guessed that. I'm just reassuring you that the warning just says that someone else in a different situation could have a problem. Which isn't the problem that you are having. So you can safely put it out of your mind while trying to debug this.

[fizbin]

tilly's addressed your main point, but I thought I should point out two things about get_top_sources:
1. You're never subtracting from $total when you add stuff to your return value, meaning that the "other" slice is forever too big
2. Including the "other" slice, you actually produce $max_wedges+1 wedges
Here's what I might do, but I'm a bit odd. After you calculate @sorted...:

  my @sorted = sort {
        $events{$b}{'count'} <=> $events{$a}{'count'}
               } keys %events;
  if (@sorted > $max_wedges)
  {
    map { $events{'other'}{'count'} += $_ } 
        @sorted[$max_wedges..$#sorted];
    @sorted[$max_wedges..$#sorted] = ('other');
  }
  return [map { {$_ => $events{$_}{'count'}}; } @sorted];

Note that $total is now never used, so there's no longer any reason to be calculating it above.

This uses map heavily, which is sometimes a bit odd to get used to, but I find it leads me to write stuff that I can look at later and easily read. YMMV.

[jeffa]

Seems to me like the datastructure is more complex than it needs to be, but if it gets the job done, cool. :) However, there is no need to use map in void context:

if (@sorted > $max_wedges) {
   $events{other}{count} += $_ for @sorted[$max_wedges..$#sorted];
   @sorted[$max_wedges..$#sorted] = ('other'); # loaded gun? 
}