[Nagiosplug-devel] Nagios::Plugin api

[Gavin Carr]

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.


- 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 )


- 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)


- 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?
  

- 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);
 

- 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.


Comments?


Cheers,
Gavin


-- 
Gavin Carr
Open Fusion - Open Source Business Solutions [ Linux - Perl - Apache ]
http://www.openfusion.com.au
- Fashion is a variable, but style is a constant - Programming Perl