monitoring-plugins / monitoring-plugin-perl

Perl module Monitoring::Plugin - Nagios::Plugin
http://search.cpan.org/dist/Monitoring-Plugin/
42 stars 20 forks source link

Implicitly creating a threshold object in getopts rather than check_threshold #22

Closed infraweavers closed 4 years ago

infraweavers commented 4 years ago

Previously, the call to check_threshold would set the threshold on the Monitoring::Plugin object, if thresholds are defined, this means that these 2 snippets have different behaviour:

my $returnCode = $mp->check_threshold($connectionCount);

$mp->add_perfdata(
        label => "OpenVPN clients",
        value => $connectionCount,
        threshold => $mp->threshold
);

$mp->plugin_exit(
        return_code => $returnCode,
        message => $connectionCount . " OpenVPN clients"
);
$mp->add_perfdata(
        label => "OpenVPN clients",
        value => $connectionCount,
        threshold => $mp->threshold
);

$mp->plugin_exit(
        return_code => $mp->check_threshold($connectionCount),
        message => $connectionCount . " OpenVPN clients"
);

To us, this is unexpected. The first snippet, will have thresholds on the perfdata, whereas the second will not. This PR should change the behaviour so that $mp->threshold will exist from as early as possible, so need to run check_threshold before add_perfdata is no longer there.

Open to alternatives if this introduces some behaviour we're not aware of

sni commented 4 years ago

Let me have a look at the travis tests, they seem unrelated to this PR.

sni commented 4 years ago

could you please rebase this PR, should be fixed with f1e3e739775d5781553c207f856b618df9467a85

infraweavers commented 4 years ago

@sni done 👍

sni commented 4 years ago

thanks