Change plugin config to be specified as a list

[sparrc]

This makes plugin configuration more similar to output configuration,
where we can specify multiple plugins as a list. The idea behind this is
that the Telegraf agent can handle the multi-processing and error
handling better than each plugin handling that internally. This will
also allow for having different plugin configurations for different
instances of the same type of plugin.

[sparrc]

This will NOT be a breaking change. In essence, it will make the config look like this (see bottom for plugins change):

$ ./telegraf -sample-config -filter cpu -outputfilter influxdb
# Tags can also be specified via a normal map, but only one form at a time:
[tags]
  # dc = "us-east-1"

# Configuration for telegraf agent
[agent]
  # Default data collection interval for all plugins
  interval = "10s"
  # Rounds collection interval to 'interval'
  # ie, if interval="10s" then always collect on :00, :10, :20, etc.
  round_interval = true

  # Default data flushing interval for all outputs. You should not set this below
  # interval. Maximum flush_interval will be flush_interval + flush_jitter
  flush_interval = "10s"
  # Jitter the flush interval by a random amount. This is primarily to avoid
  # large write spikes for users running a large number of telegraf instances.
  # ie, a jitter of 5s and interval 10s means flushes will happen every 10-15s
  flush_jitter = "0s"

  # Run telegraf in debug mode
  debug = false
  # Override default hostname, if empty use os.Hostname()
  hostname = ""


###############################################################################
#                                  OUTPUTS                                    #
###############################################################################

[outputs]

# Configuration for influxdb server to send metrics to
[[outputs.influxdb]]
  # The full HTTP or UDP endpoint URL for your InfluxDB instance.
  # Multiple urls can be specified but it is assumed that they are part of the same
  # cluster, this means that only ONE of the urls will be written to each interval.
  # urls = ["udp://localhost:8089"] # UDP endpoint example
  urls = ["http://localhost:8086"] # required
  # The target database for metrics (telegraf will create it if not exists)
  database = "telegraf" # required
  # Precision of writes, valid values are n, u, ms, s, m, and h
  # note: using second precision greatly helps InfluxDB compression
  precision = "s"

  # Connection timeout (for the connection with InfluxDB), formatted as a string.
  # If not provided, will default to 0 (no timeout)
  # timeout = "5s"
  # username = "telegraf"
  # password = "metricsmetricsmetricsmetrics"
  # Set the user agent for HTTP POSTs (can be useful for log differentiation)
  # user_agent = "telegraf"
  # Set UDP payload size, defaults to InfluxDB UDP Client default (512 bytes)
  # udp_payload = 512


###############################################################################
#                                  PLUGINS                                    #
###############################################################################

[plugins]

# Read metrics about cpu usage
[[plugins.cpu]]
  # Whether to report per-cpu stats or not
  percpu = true
  # Whether to report total system cpu stats or not
  totalcpu = true
  # Comment this line if you want the raw CPU time metrics
  drop = ["cpu_time"]

[sparrc]

@gotyaoi This is a change to the way that plugins are configured, I thought you would want to be in the loop on it.

Makes plugins configured in a way similar to outputs, which means that users can specify multiple plugin types with completely different configurations.

[sparrc]

cc @pauldix @beckettsean @gunnaraasen this PR would change the way the config file looks but is NOT a breaking change

[gotyaoi]

The only things I can think of is that for some plugins, like the CPU plugin, it may not make sense to have multiple copies, and that other plugins, like procstat, have already worked around this somewhat by having the plugin be a container, which controls a list of the actual things to check.

Granted if there are multiple plugins doing this, and I think there are, it may make sense to have a generic solution, but procstat and the others might need to be changed to take advantage of it.

[sparrc]

@gotyaoi Agreed, I was hoping to get away from plugins like procstat and httpjson, which have to do a lot of internal threading and error handling. I think it would be better for the telegraf agent to handle the threading.

Even the CPU plugin, while simple, could have a use-case for multiple instances. For example, what if you wanted cpu_time* for cpu-total, but not for cpu0, cpu1, etc. Currently that's not possible, but with this change you could do that like this:

[plugins]

# Get everything from totalcpu
[[plugins.cpu]]
percpu = false
totalcpu = true

# Drop cpu_time* from percpu measurements
[[plugins.cpu]]
percpu = true
totalcpu = false
drop = ["cpu_time"]

This particular scenario is not very likely, but I think it would be better to have this flexibility.

[pauldix]

Sure, if it's not a breaking change +1

[beckettsean]

Works for me

[sparrc]

@gotyaoi This is finally completed 😅

I made some significant changes, and namely I realized that almost all of the configuration merging had to be negated in order to accomodate specifying multiple plugins as a list. Since they are a list, there is not really a way to get it to understand when it should merge and overwrite a plugin or not.

I'm sorry that I have removed some of the power of the configuration changes that you made, but I would like to move Telegraf towards a configuration that allows for plugins to be lists, and for plugins to not need to deal with large amounts of parallelism internally.

[gotyaoi]

No worries. My code made sense in a context where there is only one instance of a given struct type, and that is no longer the case. There are potentially a few issues I can see though. On mobile right now, but I'll add some comments when I get home.

[gotyaoi]

The Tags have been shuffled around a bit over time. There currently seems to be Tags attributes on both the Agent and the Config. I think the one on the Config is the correct one, so removing the one from the Agent may reveal a few places where it's still being used.

[sparrc]

yes, makes sense, I'll make that change

[gotyaoi]

Last thing wouldn't relate to a specific line: My changes sort of tried to move all the details from the Config to the Agent, once all the config files were parsed. This seems to go back to keeping around the Config with the details in it, which is fine, but then there's the matter of the Agent's config, which is copied into the Agent from the Config. So now the Config has an *Ast.Table hanging around in it's agent attribute, and the agent refers to itself for some values, and to the config for other values, which feels sort of strange. Perhaps the various attributes on the agent struct should actually live on the Config, and the agent just holds a Config, which it refers to for details on what to do. This even enables a potentially nice path to config reloading in future, simply by building the new config in the background, then subbing out the Agent's Config attribute in one go.

[sparrc]

@gotyaoi I'll make changes to have the agent config be held within config, I think that makes more sense

[gotyaoi]

Because #395 made me stare at it for a few minutes, I noticed a possible panic in my directory loading code, if the filename is too small. Since you're rewriting here anyway, you might want to include a change here:

        if len(name) < 6 || name[len(name)-5:] != ".conf" {

The length check should protect the string slice from going out of bounds via short circuit evaluation of the condition.

[sparrc]

I'll fix that, good catch