[Nagiosplug-devel] check_mssql

[Jonathan Rozes]

I don't know if you're talking about my version written in perl with sqsh or
the shell script somebody else posted recently, but if you want to talk
dirty, allow me to point out some issues with your script:

1. Don't require a password on the command line - it's bad security.

2. Use the utils.pm package included with nagios to get the default timeout,
exit codes, print_revision() and support() subroutines, etc.

3. You initialize $exitcode to 0, but there isn't anywhere it could ever be
modified.

4. Check the return value when you prepare your statement handle.

5. $nagline and $data_returned are scoped, but never used.

6. Date::Parse is included, but never used.

7. $lasterror is initialized from execute(), but never used.

8. Disconnect from the database when you finish.

9. Set an alarm timer in case your process hangs.

10. If the query is a SELECT statement, the call to execute() will always
succeed, even if the database has problems returning data. You need to make
sure you can actually fetch the data in such cases.

11. Maybe give the user a way to control the DBI connect parameters. Folks
might want to use password encryption, for example.

12. Maybe time the execute() and print the query time along with the query
return code when everything goes correctly.

Hopefully you'll take this as constructive criticism. I'm not trying to pick
a fight; I'm just concerned about the quality of code that goes into nagios.

jonathan

> -----Original Message-----
> From: Roy Sigurd Karlsbakk [mailto:roy at karlsbakk.net]
> Sent: Wednesday, May 14, 2003 12:10 AM
> To: Jonathan Rozes; NagiosPlug Devel
> Subject: Re: [Nagiosplug-devel] check_mssql
> 
> 
> the only check_mssql script I've seen was a dirty hack with 
> sqsh. I find it 
> better to do this in perl
> 
> On Tuesday 13 May 2003 18:26, Jonathan Rozes wrote:
> > This is the third version of a check_mssql plugin I've seen 
> now. Why does
> > everybody insist on reinventing the wheel (and poorly, at that)?
> >
> > jonathan
> >
> > > -----Original Message-----
> > > From: Roy Sigurd Karlsbakk [mailto:roy at karlsbakk.net]
> > > Sent: Tuesday, May 13, 2003 2:09 AM
> > > To: NagiosPlug Devel
> > > Subject: [Nagiosplug-devel] check_mssql
> > >
> > >
> > > hi
> > >
> > > just wrote this. works fine here. any chance to get this
> > > checked into cvs?
> > >
> > > roy
> > >
> > > #!/usr/bin/perl -w
> > >
> > > use DBI;
> > > use DBD::Sybase;
> > > use Getopt::Long;
> > > use Date::Parse;
> > > use strict;
> > >
> > > my (
> > >         
> $server,$database,$username,$password,$query,$help,$verbose,
> > >         $dbh,$sth,$lasterror,
> > >         $nagline,
> > >         $s
> > > );
> > > my $exitcode = 0;
> > > my $lastread = 0;
> > > my $data_returned = 0;
> > >
> > > sub syntax {
> > >         $s = shift or $s = 'Unknown';
> > >         printf("Error: ($s)\n") unless ($help);
> > >         printf("Syntax: %s -s <server> -d <database> -u 
> <username> -p
> > > <password> -q <query> [-v]\n", $0);
> > >         printf("* --database -d         Database name\n");
> > >         printf("* --server -s           Server name\n");
> > >         printf("* --username -u         Username\n");
> > >         printf("* --password -p         Password\n");
> > >         printf("* --query -q            SQL query to run\n");
> > >         printf("  --verbose -v          verbose\n");
> > >         exit(3);
> > > }
> > >
> > > Getopt::Long::Configure('bundling');
> > > GetOptions
> > >         ("p=s" => \$password, "password=s" => \$password,
> > >          "u=s" => \$username, "username=s" => \$username,
> > >          "s=s" => \$server, "server=s"    => \$server,
> > >          "d=s" => \$database, "database=s" => \$database,
> > >          "q=s" => \$query, "query=s" => \$query,
> > >          "h" => \$help, "help" => \$help,
> > >          "v" => \$verbose, "verbose" => \$verbose);
> > >
> > > syntax("Help:") if ($help);
> > > syntax("Missing username") unless (defined($username));
> > > syntax("Missing password") unless (defined($password));
> > > syntax("Missing server") unless (defined($server));
> > > syntax("Missing database") unless (defined($database));
> > > syntax("Missing query string") unless (defined($query));
> > >
> > > unless ($dbh = DBI->connect("dbi:Sybase:server=".uc($server),
> > > "$username",
> > > "$password")) {
> > >         printf ("Can't connect to mssql server $dbh->errstr");
> > >         exit(2);
> > > }
> > > unless ($dbh->do("use $database")) {
> > >         printf ("Can't 'use $database': $dbh->errstr");
> > >         exit(2);
> > > }
> > > $sth = $dbh->prepare($query);
> > > unless ($lasterror = $sth->execute()) {
> > >         printf("Error in query: $dbh->errstr\n");
> > >         exit(2);
> > > }
> > >
> > > exit($exitcode);
> > >
> > > --
> > > Roy Sigurd Karlsbakk, Datavaktmester
> > > ProntoTV AS - http://www.pronto.tv/
> > > Tel: +47 9801 3356
> > >
> > > Computers are like air conditioners.
> > > They stop working when you open Windows.
> > >
> > >
> > >
> > > -------------------------------------------------------
> > > Enterprise Linux Forum Conference & Expo, June 4-6, 2003, 
> Santa Clara
> > > The only event dedicated to issues related to Linux
> > > enterprise solutions
> > > www.enterpriselinuxforum.com
> > >
> > > _______________________________________________
> > > Nagiosplug-devel mailing list
> > > Nagiosplug-devel at lists.sourceforge.net
> > > https://lists.sourceforge.net/lists/listinfo/nagiosplug-devel
> > >
> > > ::: Please include plugins version (-v) and OS when reporting
> > >
> > > any issue.
> > >
> > > ::: Messages without supporting info will risk being sent 
> to /dev/null
> >
> > -------------------------------------------------------
> > Enterprise Linux Forum Conference & Expo, June 4-6, 2003, 
> Santa Clara
> > The only event dedicated to issues related to Linux 
> enterprise solutions
> > www.enterpriselinuxforum.com
> >
> > _______________________________________________
> > Nagiosplug-devel mailing list
> > Nagiosplug-devel at lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/nagiosplug-devel
> >
> > ::: Please include plugins version (-v) and OS when 
> reporting any issue.
> > ::: Messages without supporting info will risk being sent 
> to /dev/null
> 
> -- 
> Roy Sigurd Karlsbakk, Datavaktmester
> ProntoTV AS - http://www.pronto.tv/
> Tel: +47 9801 3356
> 
> Computers are like air conditioners.
> They stop working when you open Windows.
>