Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Usage of sections in telegraf::input #41

Closed
quadriq opened this issue Dec 6, 2016 · 7 comments
Closed

Usage of sections in telegraf::input #41

quadriq opened this issue Dec 6, 2016 · 7 comments

Comments

@quadriq
Copy link

quadriq commented Dec 6, 2016

Hi,
could it be, that the usage of "sections" is not correct? According to the docu here is the example of logparser input plugin: https://github.com/influxdata/telegraf/tree/master/plugins/inputs/logparser

So the section is starting like [inputs.logparser.grok], so only with one quote. If using your implementation the config is [[inputs.logparser.grok]] and I am getting in logs: logparser.LogParserPlugin.GrokParser' must be slice type, but ptr given

to corrected this I chnaged the template input.conf.erb like this one:

<%      unless @options == nil -%>
<%          @options.sort.each do | option, value | -%>
  <%= option -%> = <% if value.is_a?(String) %>"<%= value %>"<% elsif value.is_a?(Array) %><%= value.inspect %><% else %><%= value %><% end %>
<%          end -%>
<%      end -%>
<% if @sections -%>
<% @sections.sort.each do |section, option| -%>
 [inputs.<%= section %>]
<%      unless option == nil -%>
<%          option.sort.each do | suboption, value | -%>
  <%= suboption -%> = <% if value.is_a?(String) %>"<%= value %>"<% elsif value.is_a?(Array) %><%= value.inspect %><% else %><%= value %><% end %>
<%          end -%>
<%      end -%>
<%   end -%>
<% end -%>
~

What do you think?

@yankcrime
Copy link
Member

You could be right, hard to spot with just 👀 though.

I'd suggest making those changes, see if it passes the basic tests, and then submit a PR.

@cosmopetrich
Copy link
Contributor

cosmopetrich commented Feb 16, 2017

I've run into this as well while using the httpjson input.

From a quick look around the Telegraf codebase it looks like the problem might be that there are two cases we need to solve for: inputs that have single-use sections ([section]) and inputs that have repeatable sections ([[section]]). SNMP may be the only example of the later.

The current puppet template will work for inputs with repeatable sections, but doesn't actually allow multiple sections with the same name to be defined since it expects a hash.

@quadriq
Copy link
Author

quadriq commented Feb 22, 2017

I think we need a kind of decision here. Let's list possible cases. here is the final configuration with a single quotes section:

[[inputs.logparser]]
  files = ["/var/log/jobs.log"]
  from_beginning = false
 [inputs.logparser.grok]
  custom_pattern_files = ["/etc/telegraf/grok/jobs"]
  measurement = "logs"
  patterns = ["%{CUSTOM_LOG}"]

can someone list an example where we need double quotes?

@cosmopetrich
Copy link
Contributor

cosmopetrich commented Feb 22, 2017

By double quotes, do you mean double square braces?

In TOML those indicate an array of tables, rather than a single inline table. Double braces are needed whenever a section will be repeated. The only current use of that in telegraf that I'm aware of is the SNMP plugin. There's some examples in its readme.

Edit: It looks like win_perf_counters is another example.

Here's a couple of ways in which we might be able to change the puppet module.

Drop support for repeated tables

This is the simplest change. We can just have the template create [section] rather than [[section]]. I believe this will remove the ability to configure the SNMP input, but it will fix everything else. In its current state the module doesn't really work for any non-trivial SNMP inputs anyway, since it isn't possible to pass in multiple sections with the same name.

If someone needed to configure an SNMP input then they could manage it as a file resource (which is probably what they need to do right now anyway).

Support hashes of hashes and hashes of arrays of hashes

That is, something like this.

::telegraf::input { 'snmp':
  sections => {
    'snmp.tags' => {
      'some_tag' => 'some_value',
    },
    'snmp.field' => [
      {
        'name' => 'uptime',
        'oid'  => '.1.0.0.1.2',
      },
      {
        'name' => 'loadavg',
        'oid'  => '.1.0.0.1.3',
      },
    ]
  }
}

This is a bit more complex and still wouldn't fully support the SNMP module. To do that we'd also need to support a hash of arrays of arrays of hashes for [[inputs.snmp.table.field]]. At that point we'd probably be better off trying to have ruby serialise an arbitrary data structure directly to TOML.

Split off ::telegraf::input::section

Use puppetlabs-concat to allow specifying sections with a separate resource, e.g.

::telegraf::input { 'logparser':
  [...]
}

::telegraf::input::section { 'logparser.grok':
  input   => 'logparser',
  options => {
    'custom_pattern_files' => [
       '/etc/telegraf/grok/jobs',
    ],
    [...]
  }
}

This would make specifying inputs entirely in hiera a bit more complex and it doesn't really solve the SNMP vs non-SNMP problem on its own.

@kkzinger
Copy link
Contributor

I had the same problem and managed it in the following way.

[[inputs.<%= @plugin_type %>]]
<%      unless @options == nil -%> 
<%          @options.sort.each do | option, value | -%> 
  <%= option -%> = <% if value.is_a?(String) %>"<%= value %>"<% elsif value.is_a?(Array) %><%= value.inspect %><% else %><%= value %><% end %>
<%          end -%> 
<%      end -%> 
<% if @sections -%> 
<% @sections.each do |section| -%> 
<%   section.each do |name, option| -%> 
[[inputs.<%= name %>]]
<%      unless option == nil -%> 
<%          option.sort.each do | suboption, value | -%> 
  <%= suboption -%> = <% if value.is_a?(String) %>"<%= value %>"<% elsif value.is_a?(Array) %><%= value.inspect %><% else %><%= value %><% end %>
<%          end -%> 
<%      end -%> 
<%   end-%>
<% end -%> 
<% end -%> 

<% if @sections_no_repeat -%> 
<% @sections_no_repeat.each do |section, option| -%> 
<%   section.each do |name, option| -%> 
[inputs.<%= name %>] 
<%      unless option == nil -%> 
<%          option.sort.each do | suboption, value | -%> 
  <%= suboption -%> = <% if value.is_a?(String) %>"<%= value %>"<% elsif value.is_a?(Array) %><%= value.inspect %><% else %><%= value %><% end %>
<%          end -%> 
<%      end -%> 
<%   end -%> 
<% end -%> 
<% end -%> 

This allows me that I handle the repeating sections different from the non repeating.
Here is the hiera yaml structure I use.

telegraf_baseline:
  win_perfcounters:
    plugin_type: 'win_perf_counters'
    sections:
      - win_perf_counters.object:
          ObjectName: 'Processor'
          Instances:
            - '*'
          Counters:
            - '% Idle Time'
            - '% Interrupt Time'
            - '% Privileged Time'
            - '% User Time'
            - '% Processor Time'
          Measurement: 'win_cpu'
      - win_perf_counters.object:
          ObjectName: 'LogicalDisk'
          Instances:
            - '*'
          Counters:
            - '% Idle Time'
            - '% Disk Time'
            - '% Disk Read Time'
            - '% Disk Write Time'
            - '% User Time'
            - 'Current Disk Queue Length'
            - 'Free Megabytes'
          Measurement: 'win_disk'
    sections_no_repeat:
      - win_perf_counters.tagdrop:
          instance:
            - 'isatap*'
            - 'Harddisk*'

In my profile where I use this module it is called like this

 include ::telegraf

    $baseline = hiera("telegraf_baseline")
    create_resources(::telegraf::input, $baseline)

I could provide a PR to implement this. At the moment I had to change the module in more depth because the "conf.d" directory support on windows is not there at the moment. But I could refactor to make it suitable for the upstream version.

mxjessie added a commit to mxjessie/puppet-telegraf that referenced this issue May 26, 2017
Single-bracket directives are required for certain sorts of input plugin
options, e.g. setting tags on that input plugin. This adds a
single_section template and directive (via @kkzinger 's solution, ty!),
as well as tests and updated docs with an example.
derkgort added a commit to derkgort/puppet-telegraf that referenced this issue May 30, 2017
yankcrime pushed a commit that referenced this issue Jun 10, 2017
* fix fatal linter error

* Add single_section to input plugins (#41)

Single-bracket directives are required for certain sorts of input plugin
options, e.g. setting tags on that input plugin. This adds a
single_section template and directive (via @kkzinger 's solution, ty!),
as well as tests and updated docs with an example.
@JSN-1
Copy link

JSN-1 commented Jun 12, 2017

thanks for the update, how ever i need to know if it's possible to do single_section inside hiera / yaml file.

i want to create sysstat which uses single section [inputs.sysstat.options]

and i want to create it like this

[[inputs.sysstat]]
  [inputs.sysstat.options]
    -C = "cpu"
    -d = "disk"

tried like this but it's not working.

sysstat:
  sections_no_repeat:
    options:
      - "-C"
      - "-d"

please advice, and thanks for your help.

@yankcrime
Copy link
Member

Fixed via #80.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants