Make the threshold plugin configurable

[smortex]

Pull Request (PR) description

Allow to configure the threshold plugin

class { 'collectd::plugin::processes':
  process_matches => [
    'carbon-cache',
    'python.+carbon-cache',
  ],
}

class { 'collectd::plugin::threshold':
  plugins => {
    processes => {
      instance => 'carbon-cache',
      types    => {
        data_source => 'processes',
        warning_min => 2,
        failure_min => 1,
      },
    },
  },
}

This Pull Request (PR) fixes the following issues

Fixes #292

[bastelfreak]

can you replace String with String[1]?

[smortex]

Not just here I guess 🤔

[bastelfreak]

Thanks for the excellent PR @smortex ! Can you please take a look at the inline comment I made?

[smortex]

I do not think it makes sense to pass an empty string to any of these strings, so I changed them all to String[1]. Thanks!

[smortex]

Hold a second! This data structure does not allow to build some valid hierarchies of configuration.

For example we might want to configure thresholds on the rx and tx data sources of a single network interface:

class { 'collectd::plugin::threshold':
  plugins => {
    interface => {
      instance => 'eth0',
      types    => {
        if_octets => {
          data_source => 'rx',
          failure_max => 10000,
        },
        if_octets => {             # <-------- Duplicate key "if_octets"
          data_source => 'tx',
          failure_max => 20000,
        },
      ],
    },
  }.
}

While it is possible to assign an array of hashes ({ if_octets => [ { ...tx }, { ...rx }]}), this impose a level of hierarchy that does not exist in collectd, and I would rather change this to something like that:

class { 'collectd::plugin::threshold':
  plugins => [
    {
      name     => 'interface',
      instance => 'eth0',
      types    => [
        {
          name        => 'if_octets',
          data_source => 'rx',
          failure_max => 10000,
        },
        {
          name        => 'if_octets',
          data_source => 'tx',
          failure_max => 20000,
        },
      ],
    },
  ],
}

I should be able to hack this and to test it tomorrow 😉

[smortex]

@bastelfreak, I fixed the problem described above and updated the README to describe a typical use-case in new commits (I guess it's easier for reviewing).

Feel free to squash all these commits into a single one when merging if you want to :-)

[bastelfreak]

interesting syntax. I never thought about appending dig() directly to the $facts[] hash.

[bastelfreak]

can you please align the = chars please?

[smortex]

Done in dd7fb6d

[bastelfreak]

This is a breaking change, right? Because of the custom datatype, an empty array isn't allowed anymore and users now have to pass data to the new parameters, correct? Is that desired? If so, I need to mark it as backwards-incompatible.

[smortex]

I don't think so… I am quite new to the threshold plugin of collectd, but as far as I experienced, the puppet-collectd module allowed to enable the threshold module but not to set a configuration, making it somewhat useless (the reason why I worked on this).

The default values of [] for $types, $plugins and $hosts imply that if a user previously had enabled the threshold module, if he does not change his configuration, the module is just loaded as it was before, without setting any thresholds (if the user used something external to the puppet-collectd module to setup a configuration, I guess this change should have no impact).

As a matter of facts, the first unit tests do not set $types, $plugins and $hosts:

https://github.com/smortex/puppet-collectd/blob/29a87d1a44acb80b238e22ca2168032025cb5c9f/spec/classes/collectd_plugin_threshold_spec.rb#L71-L99

[bastelfreak]

Thanks for the updates @smortex !