Skip to content

Commit

Permalink
Merge pull request #132 from Tetha/single_value_sum
Browse files Browse the repository at this point in the history
Handled single-value .sum's gracefully.
  • Loading branch information
steveschnepp committed Oct 16, 2013
2 parents 83b3ab3 + 03eadc3 commit f4cda9c
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 4 deletions.
16 changes: 12 additions & 4 deletions master/lib/Munin/Master/GraphOld.pm
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,10 @@ use strict;

use Exporter;

our (@ISA, @EXPORT);
our (@ISA, @EXPORT, @EXPORT_OK);
@ISA = qw(Exporter);
@EXPORT = qw(graph_startup graph_check_cron graph_main graph_config);
@EXPORT_OK = qw(build_sum_cdef); # exportable for tests

use IO::Socket;
use IO::Handle;
Expand Down Expand Up @@ -599,6 +600,14 @@ sub get_stack_command {
return munin_get($field, "stack");
}

sub build_sum_cdef {
if (@_ == 1) {
return "";
} else {
return "," . join(",$AddNAN,", @_[0 .. @_ - 2]) . ",$AddNAN";
}
}

sub expand_specials {
my $service = shift;
my $order = shift;
Expand Down Expand Up @@ -723,9 +732,8 @@ sub expand_specials {
push(@spc_stack, $name);
push(@$preproc, "$name=$pre");
}
$service->{$last_name}->{"cdef"} .=
"," . join(",$AddNAN,", @spc_stack[0 .. @spc_stack - 2]) .
",$AddNAN";
$service->{$last_name}->{"cdef"} .= build_sum_cdef(@spc_stack);


if (my $tc = munin_get($service->{$field}, "cdef", 0))
{ # Oh bugger...
Expand Down
32 changes: 32 additions & 0 deletions master/t/munin_master_graphold_sum_cdef.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# -*- cperl -*-
# vim: ts=4 : sw=4 : et
use warnings;
use strict;

use Test::More tests => 3;
use Test::MockModule;

my $mock = Test::MockModule->new('RRDs', no_auto => 1);
$mock->mock('errors' => sub { });

use_ok('Munin::Master::GraphOld', qw(build_sum_cdef));

# It should be considered to move the RRDTool compatibility mechanisms into a centralized
# module.
my $AddNAN = "+";
if ($RRDs::VERSION >= 1.3) {
$AddNAN = 'ADDNAN';
}
is(build_sum_cdef("a", "b", "c"), ",a,$AddNAN,b,$AddNAN", ".sum with >= 2 values");
is(build_sum_cdef("a"), "", ".sum with = 1 value");

# This test makes sure that both single value sums behave as expected (by
# just returning that one value) and multiple value sums behave as before.
#
# Rationale:
# Our hosts have multiple instances of the same service. Each instance has
# it's own status graph and we aggregate these status graphs into a single
# host status graph.
# However, it is quite a pain to work around hosts with one instance of
# a single instance, because "total.sum value_of_instance1" tended to output
# invalid CDEFs and thus, rrdtool balked.

0 comments on commit f4cda9c

Please sign in to comment.