network plugin: forwading broken since 5.6

[bzed]

• Version of collectd: 5.6.2.2.g11011dc-1~jessie
• Operating system / distribution:
  debian jessie

Expected behavior

The network plugin receives + forewards packages.

Actual behavior

Packets are received, written to the local output plugin (graphite in this case). Values from the local host are sent to the Server, but not the received values.

Steps to reproduce

Install packages from pkg.ci, use the following config (same config works perfectly fine with 5.5):

# Generated by Puppet

#Hostname localhost
FQDNLookup true

#BaseDir "/var/lib/collectd"
#PluginDir "/usr/lib/collectd"
TypesDB "/usr/share/collectd/types.db" "/etc/collectd/conova_types.db"
WriteQueueLimitHigh 5000000
WriteQueueLimitLow 2500000
Interval 10
Timeout 2
ReadThreads 5
Include "/etc/collectd/conf.d/*.conf"
Include "/etc/collectd/conf_local.d/*.conf"
# Generated by Puppet
<LoadPlugin syslog>
  Globals false
</LoadPlugin>

<Plugin syslog>
  LogLevel info
</Plugin>

# Generated by Puppet
<LoadPlugin apache>
  Globals false
</LoadPlugin>

<Plugin "apache">
  <Instance "apache">
    URL "http://127.0.0.1/?auto"
    User "collectd-apache"
    Password "password"
  </Instance>
</Plugin>

# Generated by Puppet
<LoadPlugin contextswitch>
  Globals false
</LoadPlugin>


# Generated by Puppet
<LoadPlugin cpu>
  Globals false
</LoadPlugin>

<Plugin cpu>
  ReportByState = true
  ReportByCpu = true
  ValuesPercentage = false
</Plugin>

# Generated by Puppet
<LoadPlugin df>
  Globals false
</LoadPlugin>

<Plugin df>
  IgnoreSelected false
  ReportByDevice false
  ReportInodes true
  ReportReserved true
  ValuesAbsolute true
  ValuesPercentage true
</Plugin>

# Generated by Puppet
<LoadPlugin disk>
  Globals false
</LoadPlugin>

<Plugin disk>
  IgnoreSelected false
</Plugin>

# Generated by Puppet
<LoadPlugin entropy>
  Globals false
</LoadPlugin>


# Generated by Puppet
<LoadPlugin interface>
  Globals false
</LoadPlugin>

<Plugin interface>
  IgnoreSelected false
</Plugin>

# Generated by Puppet
<LoadPlugin irq>
  Globals false
</LoadPlugin>

<Plugin irq>
  IgnoreSelected false
</Plugin>

# Generated by Puppet
<LoadPlugin load>
  Globals false
</LoadPlugin>


# Generated by Puppet
<LoadPlugin lvm>
  Globals false
</LoadPlugin>


# Generated by Puppet
<LoadPlugin memcached>
  Globals false
</LoadPlugin>

<Plugin memcached>
  Host "127.0.0.1"
  Port 11211
</Plugin>

# Generated by Puppet
<LoadPlugin memory>
  Globals false
</LoadPlugin>

<Plugin memory>
  ValuesAbsolute = true
  ValuesPercentage = false
</Plugin>

# Generated by Puppet
<LoadPlugin network>
  Globals false
</LoadPlugin>

<Plugin network>
    Forward "true"
    ReportStats "false"
  
</Plugin>

# Generated by Puppet
<LoadPlugin nfs>
  Globals false
</LoadPlugin>


# Generated by Puppet
<LoadPlugin nginx>
  Globals false
</LoadPlugin>

<Plugin nginx>
  URL "http://127.0.0.1:1234/"
  User "collectd-nginx"
  Password "password"
</Plugin>

# Generated by Puppet
<LoadPlugin ntpd>
  Globals false
</LoadPlugin>

<Plugin ntpd>
  Host "127.0.0.1"
  Port "123"
  ReverseLookups false
  IncludeUnitID false
</Plugin>

# Generated by Puppet
<LoadPlugin processes>
  Globals false
</LoadPlugin>


# Generated by Puppet
<LoadPlugin python>
  Globals true
</LoadPlugin>


# Generated by Puppet
<LoadPlugin swap>
  Globals false
</LoadPlugin>

<Plugin swap>
  ReportByDevice false
  ReportBytes true
  ValuesAbsolute = true
  ValuesPercentage = false
</Plugin>

# Generated by Puppet
<LoadPlugin tail>
  Globals false
</LoadPlugin>


# Generated by Puppet
<LoadPlugin unixsock>
  Globals false
</LoadPlugin>

<Plugin unixsock>
  SocketFile  "/var/run/collectd-unixsock"
  SocketGroup "root"
  SocketPerms "0770"
  DeleteSocket "false"
</Plugin>

# Generated by Puppet
<LoadPlugin uptime>
  Globals false
</LoadPlugin>


# Generated by Puppet
<LoadPlugin users>
  Globals false
</LoadPlugin>


# Generated by Puppet
<LoadPlugin vmem>
  Globals false
</LoadPlugin>

<Plugin vmem>
  Verbose false
</Plugin>

<Plugin network>
  <Listen "0.0.0.0" "25826">
    SecurityLevel "Sign"
    AuthFile "/etc/collectd/passwd"

  </Listen>
</Plugin>
<Plugin network>
  <Server "1.2.3.4" "25826">
    SecurityLevel "Encrypt"
    Username "user"
    Password "thisisapasswordanditissecret1111"

  </Server>
</Plugin>
<Plugin processes>
ProcessMatch "apache" "/usr/sbin/apache2"
ProcessMatch "collectd" "(collectdmon|collectd)"
ProcessMatch "exim" ".*bin/exim.*"
ProcessMatch "memcached" "/usr/bin/memcached"
ProcessMatch "nginx" "nginx:.*process"
</Plugin>
# Generated by Puppet
<Plugin "python">
  ModulePath "/usr/share/collectd/python"
  LogTraces false
  Interactive false
  Import "mailqueue"

  <Module "mailqueue">
    MTA "exim"
  </Module>
</Plugin>
# Generated by Puppet

<Plugin "tail">
 <File "/var/log/exim4/mainlog">
  Instance "exim"
  <Match>
   Regex "<="
   DSType "CounterInc"
   Type "counter"
   Instance "incoming"
  </Match>
  <Match>
   Regex "=>"
   DSType "CounterInc"
   Type "counter"
   Instance "outgoing"
  </Match>
  <Match>
   Regex "=="
   DSType "CounterInc"
   Type "counter"
   Instance "defer"
  </Match>
  <Match>
   Regex "\\*\\*"
   DSType "CounterInc"
   Type "counter"
   Instance "bounce"
  </Match>
 </File>
</Plugin>
# Generated by Puppet

<Plugin "tail">
 <File "/var/log/exim4/rejectlog">
  Instance "exim"
  <Match>
   Regex ".*([>:]|sender|\]) rejected.*"
   DSType "CounterInc"
   Type "counter"
   Instance "rejected"
  </Match>
  <Match>
   Regex ".*temporarily rejected.*greylisted.*"
   DSType "CounterInc"
   Type "counter"
   Instance "greylisted"
  </Match>
  <Match>
   Regex ".*login authenticator failed.*"
   DSType "CounterInc"
   Type "counter"
   Instance "login-failed"
  </Match>
 </File>
</Plugin>

[octo]

Moving this in the 5.7 milestone so we remember to fix this before the 5.7 release next week.

[octo]

While trying to reproduce this problem, I saw the following errors in the log:

[2016-12-08 08:26:28] cf_util_get_boolean: The Forward option requires exactly one boolean argument.
[2016-12-08 08:26:28] cf_util_get_boolean: The ReportStats option requires exactly one boolean argument.

I think the culprit are the quotes in:

    Forward "true"
    ReportStats "false"

because the updated config handling in 5.6 requires booleans. The config handling in 5.5 had a fall-back to handle strings, too, which was replaced in ac73c75 to unify boolean handling among other things.

I'm not entirely certain which way to take this. I'm not a fan of keeping the previous behavior, but I'm not a fan of breaking old configs either.

Best regards,
—octo

[tokkee]

I'd argue this is a backward-incompatible, breaking change, so we'll have to hold it back until 6.0. cf_util_get_boolean() could support the old behavior consistently across all plugins and potentially issue a warning if a string value is used (we'd only see that during startup).

[bzed]

A deprecation warning would be really nice! Then it would have been obvious why it failed....

[octo]

Merged to master.

[bzed]

unfortunately this is still broken in 5.7