voxpupuli / puppet-collectd

Collectd module for Puppet
https://forge.puppet.com/puppet/collectd
Apache License 2.0
69 stars 273 forks source link

collectd::plugin::python::module does not support nested configuration #418

Open aequitas opened 8 years ago

aequitas commented 8 years ago

For example: https://github.com/NYTimes/collectd-rabbitmq#example-configuration

jyaworski commented 8 years ago

Setting multiple modules introduces multiple concat fragments. What are you seeing when you encounter this?

jyaworski commented 8 years ago

Ping @aequitas

aequitas commented 8 years ago

Sorry missed your question. I will come back with an example on Monday.

aequitas commented 8 years ago

The problem I have is the rabbitmq plugin can have nested configuration blocks inside its configuration, for example with the <Ignore> block below:

LoadPlugin python
<Plugin python>
  <Module rabbitmq>
    Host "localhost"
    <Ignore "queue">
      Regex "amq-gen-.*"
    </Ignore>
  </Module>
</Plugin>

The Python plugin only support key/value pairs. I have a workaround which works atm:

        collectd::plugin::python::module { 'rabbitmq':
            module        => 'rabbitmq',
            script_source => 'puppet:///modules/collectd/rabbitmq.py',
            config        => {
                '<Ignore "queue">' => "\nRegex \"[0-9a-z]{32}\"\nRegex \"celeryev\\..*\"\n</Ignore>",
            },
        }

Maybe having just a freeform config append variable would suffice. I think implementing nested support would make it a lot to complicated for simple exceptional use cases.

jyaworski commented 8 years ago

@aequitas just curious, why use the python plugin for rabbitmq vs curl_json?

I like the idea of a freeform config.

aequitas commented 8 years ago

Tbh, it was the first hit in Google and it fit our usecase :)

jyaworski commented 8 years ago

I'll see if I can get to this sometime soon. I also have to tackle #456.

I would check out curl_json. There's actually an example in our docs. I've found that the perl and python plugins should be avoided when possible due to their overhead.

aequitas commented 8 years ago

thanks, will be sure to check it out!

microblag commented 8 years ago

I've been struggling with this same issue. Here's the hackyish bit of erb I've thrown together to try and tackle it. I couldn't come up with a good way to represent the config data structure so there's a couple of quirks. It's got no error checking or validation also I'm no Ruby guy so I may be horribly miss-using Proc. So I'm providing it here in case it's a useful for someone having a proper go at adding this functionality.

if a hash has a key called $VALUE$ this will be used as the parameter of a directive e.g.

$config = {
  message => {
    '$VALUE' => 'hello',
    'how' => 'are you?'
  }
}

would produce

  Import ""

  <Module "">
    <message "hello">
      how "are you?"
    </message>
  </Module>

if a hash has a key called $REPEAT$ all other keys are ignored and $REPEAT$ is assumed to be an array of hashes to use as repeating directives.

to produce the example config for the collectd rabbitmq plugin

$config = {
  'Username'   => 'guest',
  'Password'   => 'guest',
  'Realm'      => 'RabbitMq Management',
  'Host'       => 'localhost',
  'Port'       => '15672',
  'Ignore'     => {
    '$REPEAT$' => [
      {
        '$VALUE$' => 'queue',
        'Regex'   => '^amq',
      },
      {
        '$VALUE$' => 'exchange',
        'Regex'   => 'my-exchange'
      }
    ]
  }
}

which produces

  Import ""

  <Module "">
    Host "localhost"
    <Ignore "queue">
      Regex "^amq"
    </Ignore>
    <Ignore "exchange">
      Regex "my-exchange"
    </Ignore>
    Password "guest"
    Port "15672"
    Realm "RabbitMq Management"
    Username "guest"

  </Module>
<%
process_child = Proc.new do |config,depth|
  pad = " " * 2 * depth
  config.sort.each do |k,v|
    if v.is_a?(Hash)
      if v.key?('$REPEAT$')
        v['$REPEAT$'].each do |vk|
%><%= pad %><<%= k %><%
        if vk.key?('$VALUE$')
%> "<%= vk['$VALUE$'] %>"<%
          vk.delete('$VALUE$')
        end
%>>
<%
        process_child.call(vk,depth+1)
%><%= pad %></<%= k %>>
<%
        end
      else
%><%= pad %><<%= k %><%
        if v.key?('$VALUE$')
%> "<%= v['$VALUE$'] %>"<%
          v.delete('$VALUE$')
        end
%>>
<%
        process_child.call(v,depth+1)
%><%= pad %></<%= k %>>
<%
      end
    else
%><%= pad %><%= k %><%
      if v.is_a?(Array)
        v.each do |vv|
%> "<%= vv %>"<%
        end
      else
%> "<%= v %>"<%
      end
%>
<%
    end
  end
end
%>
  Import "<%= @module %>"

  <Module "<%= @module %>">
<% process_child.call(config,2) %>
  </Module>
aequitas commented 8 years ago

@microblag very nice solution with some nice examples. I will not be able to test this in the short term as the current workaround is stable in production. But if it needs change I will definitely give it a try.