[Nagiosplug-devel] Nagios::Plugin api

[Ton Voon]

On 30 Aug 2006, at 01:40, Gavin Carr wrote:

> Ton asked me to kick off a discussion of the current Nagios::Plugin
> api, so here it is.
>
> Here's the current api:
>
>   use Nagios::Plugin qw(%ERRORS);
>   $p = Nagios::Plugin->new( shortname => "PAGESIZE" );
>
>   $threshold = $p->set_thresholds( warning => "10:25", critical =>  
> "25:" );
>
>   # ... collect current metric into $value
>   if ($trouble_getting_metric) {
>     $p->die( return_code => $ERRORS{UNKNOWN}, message => "Could not  
> retrieve page");
>     # Output: PAGESIZE UNKNOWN Could not retrieve page
>     # Return code: 3
>   }
>
>   $p->die( return_code => $threshold->get_status($value), message  
> => "page size at http://... was ${value}kB" );
>   # Output: PAGESIZE OK: page size at http://... was 36kB |  
> size=36kB;10:25;25: time=...
>   # Return code: 0
>
> (I've omitted the perfdata bits, as I haven't get my head around that
>  yet, and it seems largely orthogonal to the rest.)
>
> So base functionality is:
>
>   $p = Nagios::Plugin->new();
>   $p->die( return_code => $ERRORS{UNKNOWN}, message => $msg );
>
> Comments and questions:
>
> - using %ERRORS like that seems a bit weird. Why don't we just export
>   a set of constants ( OK, WARNING, CRITICAL, UNKNOWN ) and use them
>   directly? That gives better compile time checking and seems cleaner:
>
>   $p->die( return_code => UNKNOWN, message => $msg );
>
>   We can keep %ERRORS for backwards compatibility, of course.

Constants sounds like a good idea and should be the preferred  
approach. Keep %ERRORS for now, but don't expose in the docs. I think  
Nathan suggested this too - I don't know if it is in the current code.

Current perl module trends seem to be to only export values  
specifically requested. Could there be an option at use time to  
import these constants? My design was that all the Nagios::Plugins::*  
modules import everything, but a plugin writer would only import  
necessary things at 'use Nagios::Plugin' time.


> - what do people think about support a positional variant of die too,
>   given that we only really have two arguments? e.g.
>
>   $p->die( OK, $msg )
>   $p->die( CRITICAL, $msg )
>   $p->die( UNKNOWN, $msg )

I've been working with Class::DBI where they have sometimes  
positional and sometimes hash and it gets quite messy. However, I  
think Nagios::Plugin is meant to keep things simple and your second  
suggestion of

$p->die( $msg, [level, default CRITICAL])

does keep it simple from a writer point of view. Maybe we could  
extend so $p->die( { } ) would take a hash value for other options in  
future.


> - Also, $p->die( OK, $msg ) looks a bit weird too - should you die
>   if everything is okay? I think we keep it for completeness, but
>   what about supporting something like:
>
>   $p->exit( $msg )
>   $p->exit( message => $msg )
>
>   specifically for the OK case? (which exits rather than dies)

True. I'm happy to drop $p->die, as $p->exit does make more sense.


> - can we make shortname default to something like:
>
>   unless ($shortname) {
>     $shortname = uc( $ENV{NAGIOS_PLUGIN} || basename($0) );
>     $shortname =~ s/^CHECK_//;
>   }
>
>   so that in lots of cases it becomes effectively optional?

Agreed. Philosophically, defaults should always just "make sense", so  
they are only optional. Some of your other new() options in  
N::P::Getopt could go here.


> - With thresholds, returning an object from set_thresholds seems
>   strange to me:
>
>     $threshold = $p->set_thresholds( warning => "10:25", critical  
> => "25:" );
>
>   I think that if you're going to return an object, you should just
>   use Nagios::Plugin::Threshold directly:
>
>     $threshold = Nagios::Plugin::Threshold->new( warning =>  
> "10:25", critical => "25:" );
>
>   Or if you want to use composition, you shouldn't return the object
>   at all - just
>
>     $p->set_thresholds( warning => "10:25", critical => "25:" );
>
>   or perhaps:
>
>      $p = Nagios::Plugin->new( warning => "10:25", critical =>  
> "25:" );
>
>   (In practice, I guess thresholds are most often going to come in as
>    arguments anyway, so we would presumably most often want to get  
> them
>    via Nagios::Plugin::Getopt arguments somehow ...)
>
>   If you're going with composition, then you'd call the threshold
>   get_status implicitly rather than explicitly e.g. instead of:
>
>     $p->die( return_code => $threshold->get_status($value), message  
> => $msg )
>
>   perhaps something like:
>
>     $p->exit_check_threshold(check => $value);
>     $p->exit_check_threshold($value);

Hmmm. I can't remember why I did it like this. There could be  
multiple thresholds per plugin (check_disk can check absolute disk  
free, absolute disk used, percent disk used, percent disk free,  
absolute inodes free, etc), so there needs to be a list of  
thresholds. Yes, they are likely to come via N::P::Getopt arguments,  
but I guess they need to be pulled out at certain times too (you've  
put 'check => $value' above - how do we reference 'check'?).

In summary, I'm not sure what my position on this is yet. :)

$p->exit_check_threshold( check => $value ) is a good idea for a  
shortcut method to $p->exit( ... ).

BTW, I have a medium term plan (that I haven't articulated to the  
mailing list very well yet) that plugin and perf output will be  
determined by the options given. For instance, if check_disk is run  
with --freespace_units=100/120 --units=MB, then the plugin output  
will be in the form of free space values, and similar perf data;  
whereas if it is run with --usedspace_percent=98:100/90:100, then the  
output will be in this format instead. If you check both thresholds,  
then plugin output will show both.


> - Implementation detail: Nagios::Plugin does a 'use 5.008004' - is  
> that
>   deliberate? If not, how backwards compatible do we want to be? The
>   code uses lots of 'our's and 'use warnings', both of which are  
> 5.6-isms,
>   I think. Do we want to be able to work with older perls? There  
> are still
>   lots of 5.005003 installs out there, for instance.

I have to admit to ignorance between all the different perl versions.  
Is there somewhere that highlights the differences between perl  
releases? This is a tough call.

Ton

http://www.altinity.com
T: +44 (0)870 787 9243
F: +44 (0)845 280 1725
Skype: tonvoon


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://nagios-plugins.org/archive/devel/attachments/20060830/7c76eee1/attachment.html>