Skip to content

Commit

Permalink
Be able to handle 1k SNMP plugins (#981)
Browse files Browse the repository at this point in the history
This isn't only related to SNMP. It is only aimed at SNMP since they're
typically slow for config, as they remotely ask their config.

Therefore any plugin that has a slow config with impede the node startup
speed.

Here we'll add an optional config sub command to signal that we don't
need the whole config output. Only the hostname part to know if it is a
remote plugin or a local one
  • Loading branch information
steveschnepp authored Nov 5, 2023
2 parents 1d5d60d + a1c8689 commit 16eff1e
Show file tree
Hide file tree
Showing 12 changed files with 133 additions and 21 deletions.
3 changes: 2 additions & 1 deletion contrib/munin-node-debug
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use warnings;
use IO::Socket;
use IO::Select;
use Getopt::Long;
use POSIX;

# No buffering
$| = 1;
Expand Down Expand Up @@ -149,7 +150,7 @@ while (my @ready = $select->can_read()) {
service($client);
# Exit the child
debug("[$$] Finished");
exit;
POSIX::_exit(0);
}
}
}
Expand Down
1 change: 1 addition & 0 deletions doc/architecture/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ We have some more standard arguments, which play a role in the process of automa
../plugin/multigraphing.rst
../plugin/protocol-dirtyconfig.rst
../plugin/protocol-multigraph.rst
../plugin/protocol-cap.rst
../plugin/snmp.rst
../plugin/use.rst
../plugin/writing-tips.rst
Expand Down
71 changes: 71 additions & 0 deletions doc/plugin/protocol-cap.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
.. index::
triple: protocol; extension; cap
pair: capability; cap

.. _plugin-protocol-dirtyconfig:

=================================
Protocol extension: cap
=================================

The "config cap" capability is implemented in munin 3.0 and later.

Objective
---------

Reduce plugin execution time for node startup.

Description
-----------

When the node starts up, it launches every plugin once in order to
discover their capabilities:

#. if it is part of a virtual host (for example SNMP plugins)
#. if it requires a multigraph master (for example the diskstats plugin)

That information is almost always known immediatly to the plugin,
as item 1 is usually either set by the environment or the plugin symlink name
and item 2 is usually hardcoded in the plugin code.

Plugins therefore don't have to any time doing autoconfiguration yet to provide
that information. This is important as during its startup, the node will loop
sequentially over all the active plugins. Any speedup is welcome, specially if
the node has thousands of plugins, such as a SNMP-rich one.

Network protocol
----------------

The network protocol is fully unchanged. It is purely a node-scoped
optimisation.

Plugin protocol
----------------

During the node startup, the plugin will be called with a "config cap" argument instead of "config".
The plugin can then reply with a reduced set of information, as the node will not use the full output.

Old behavior is still fully supported as we want to preserve backwards compatibility. It will simply not
benefit from any speedup.

sample plugin output
--------------------

When the `cap` subcommand is asked, the output can be *very* small.

::

$ munin-run my_plugin config cap
host_name remote.host.lan
multigraph dummy

$ munin-run my_plugin config
host_name remote.host.lan
multigraph my_plugin_1
graph_title My Plugin 1
graph_category sample
field.label My Field
multigraph my_plugin_2
graph_title My Plugin 2
graph_category sample
field.label My Field
2 changes: 1 addition & 1 deletion doc/plugin/protocol-dirtyconfig.rst
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
Protocol extension: dirtyconfig
=================================

The dirtyconfig capability is implemented in munin 2.0 and on.
The dirtyconfig capability is implemented in munin 2.0 and later.

Objective
---------
Expand Down
2 changes: 1 addition & 1 deletion doc/plugin/protocol-multigraph.rst
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
Protocol extension: multiple graphs from one plugin
=====================================================

Multigraph plugins are implemented in 1.4.0 and on.
Multigraph plugins are implemented in 1.4.0 and later.

Objective
---------
Expand Down
6 changes: 3 additions & 3 deletions lib/Munin/Master/UpdateWorker.pm
Original file line number Diff line number Diff line change
Expand Up @@ -919,7 +919,7 @@ sub _create_rrd_file {
);

INFO "[INFO] RRDs::create @args";
RRDs::create @args;
RRDs::create @args unless $ENV{NO_UPDATE_RRD};
if (my $ERROR = RRDs::error) {
ERROR "[ERROR] Unable to create '$rrd_file': $ERROR";
}
Expand Down Expand Up @@ -1069,14 +1069,14 @@ sub _update_rrd_file {
# https://lists.oetiker.ch/pipermail/rrd-users/2011-October/018196.html
for my $update_rrd_data (@update_rrd_data) {
DEBUG "RRDs::update($rrd_file, $update_rrd_data)";
RRDs::update($rrd_file, $update_rrd_data);
RRDs::update($rrd_file, $update_rrd_data) unless $ENV{NO_UPDATE_RRD};
# Break on error.
last if RRDs::error;
}
} else {
# normal vector-update the RRD
DEBUG "RRDs::update($rrd_file, @update_rrd_data)";
RRDs::update($rrd_file, @update_rrd_data);
RRDs::update($rrd_file, @update_rrd_data) unless $ENV{NO_UPDATE_RRD};
}

if (my $ERROR = RRDs::error) {
Expand Down
5 changes: 4 additions & 1 deletion lib/Munin/Node/Server.pm
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,10 @@ sub _add_services_to_nodes
for my $service (@services) {
DEBUG("Configuring $service\n") if $config->{DEBUG};

my @response = _run_service($service, 'config');
# Adding a 'cap' sub-argument to 'config' to signal 3.0+ plugins that
# they don't need to do any expensive stuff, they are only called for
# the initial node config
my @response = _run_service($service, [ 'config', 'cap', ]);

if (!@response or grep(/# Timed out/, @response)) {
DEBUG("Error running $service. Dropping it.") if $config->{DEBUG};
Expand Down
13 changes: 9 additions & 4 deletions lib/Munin/Node/Service.pm
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,10 @@ sub exec_service

Munin::Node::OS::set_umask();

my @command = grep defined, _service_command($self->{servicedir}, $service, $arg);
# Dereference $argument if ARRAYREF
my @arguments = ref($arg) ? @$arg : ( $arg );

my @command = grep defined, _service_command($self->{servicedir}, $service, @arguments);
print STDERR "# About to run '", join (' ', @command), "'\n"
if $config->{DEBUG};

Expand All @@ -275,7 +278,9 @@ sub exec_service
# <http://munin-monitoring.org/wiki/plugin-conf.d>).
sub _service_command
{
my ($dir, $service, $argument) = @_;
my $dir = shift;
my $service = shift;
my @arguments = @_;

my @run;
my $sconf = $config->{sconf};
Expand All @@ -290,14 +295,14 @@ sub _service_command
# though, since who will ever need to pass "%c" in a place
# like that?
if ($t =~ s/%c/$dir\/$service/g) {
push @run, ($t, $argument);
push @run, ($t, @arguments);
} else {
push @run, ($t);
}
}
}
else {
@run = ("$dir/$service", $argument);
@run = ("$dir/$service", @arguments);
}

return @run;
Expand Down
23 changes: 23 additions & 0 deletions lib/Munin/Plugin/SNMP.pm
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,29 @@ sub get_by_regex
return \%result;
}

=head2 handle_caps() - Emit relevant "config" output for node
This emits the relevant config lines for the initial run of the node
=cut
sub handle_caps
{
# XXX - Should handle this way better !
return unless ($ARGV[1] && $ARGV[0] eq "config" && $ARGV[1] eq "cap" );

# Only asking for "cap"
my ($host,undef,$version) = config_session();

# We are a virtualhost enabled plugin
print "host_name $host\n" unless $host eq 'localhost';

# We are a multigraph enabled plugin
print "multigraph dummy\n";

# Stopping here
exit(0);
}

1;

=head1 TODO
Expand Down
2 changes: 2 additions & 0 deletions plugins/node.d/snmp__if_multi
Original file line number Diff line number Diff line change
Expand Up @@ -822,6 +822,8 @@ sub do_fetch {

# ############################## MAIN ################################

Munin::Plugin::SNMP::handle_caps();

do_collect;
do_preprocess;

Expand Down
2 changes: 2 additions & 0 deletions plugins/node.d/snmp__uptime
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ GPLv2 or (at your option) any later version.
use strict;
use Munin::Plugin::SNMP;

Munin::Plugin::SNMP::handle_caps();

if (defined $ARGV[0] and $ARGV[0] eq "snmpconf") {
print "require 1.3.6.1.2.1.1.3.0 [0-9]\n"; # Number
exit 0;
Expand Down
24 changes: 14 additions & 10 deletions script/munin-run
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ my $config = Munin::Node::Config->instance();
sub main
{
my @original_argv = @ARGV;
my ($plugin, $arg) = parse_args();
my ($plugin, $args) = parse_args();

# Loads the settings from munin-node.conf.
# Ensures that, where options can be set both in the config and in
Expand All @@ -123,12 +123,12 @@ sub main
return execute_plugin($plugin, $arg);
} elsif (!check_systemd_run_permissions()) {
print STDERR "# Skipping systemd properties simulation due to lack of permissions.\n" if $config->{DEBUG};
return execute_plugin($plugin, $arg);
return execute_plugin($plugin, $args);
} else {
my $systemd_version = get_systemd_version();
if ((not defined $systemd_version) or ($systemd_version < $REQUIRED_SYSTEMD_VERSION)) {
print STDERR "# Skipping systemd properties simulation due to required systemd version ($REQUIRED_SYSTEMD_VERSION)\n" if $config->{DEBUG};
return execute_plugin($plugin, $arg);
return execute_plugin($plugin, $args);
} else {
my @munin_node_hardening_flags;
my $parse_flags_success = 0;
Expand All @@ -143,7 +143,7 @@ sub main
# Failed to retrieve systemd properties of munin-node service.
# Probable causes: systemd is not installed/enabled or the
# service unit does not exist.
return execute_plugin($plugin, $arg);
return execute_plugin($plugin, $args);
}
}
}
Expand Down Expand Up @@ -329,7 +329,7 @@ sub parse_args
my $sconfdir = "$Munin::Common::Defaults::MUNIN_CONFDIR/plugin-conf.d";
my $sconffile;

my ($plugin, $arg);
my ($plugin, $args);

print_usage_and_exit() unless GetOptions(
"config=s" => \$conffile,
Expand All @@ -348,10 +348,14 @@ sub parse_args
print_usage_and_exit() unless $ARGV[0];

# Detaint the plugin name
($plugin) = ($ARGV[0] =~ m/^([-\w.:]+)$/) or die "# ERROR: Invalid plugin name '$ARGV[0].\n";
if ($ARGV[1]) {
($arg) = ($ARGV[1] =~ m/^(\w+)$/)
or die "# ERROR: Invalid characters in argument '$ARGV[1]'.\n";
my $dirty_plugin = shift @ARGV;
($plugin) = ($dirty_plugin =~ m/^([-\w.:]+)$/) or die "# ERROR: Invalid plugin name '$dirty_plugin.\n";

# Detaint the plugin args
for my $dirty_arg (@ARGV) {
my ($arg) = ($dirty_arg =~ m/^(\w+)$/)
or die "# ERROR: Invalid characters in argument '$dirty_arg'.\n";
push @$args, $arg;
}

# Detaint service directory. FIXME: do more strict detainting?
Expand All @@ -372,7 +376,7 @@ sub parse_args
paranoia => $paranoia,
});

return ($plugin, $arg);
return ($plugin, $args);
}


Expand Down

0 comments on commit 16eff1e

Please sign in to comment.