Use BurntSushi/toml for config file parsing

[phemmer]

Bug report

Relevant telegraf.conf:

[[outputs.influxdb]]
  urls = ["http://127.0.0.1:1"]

[[inputs.postgresql_extensible]]
    address = "host=localhost sslmode=disable user=pgsql dbname=postgres"
    [[inputs.postgresql_extensible.query]]
        measurement = "postgresql_activity"
        sqlquery = """
            select
                datname as database,
                usename as user,
                pid,
                client_addr,
                waiting,
                case when state like 'active' or state = 'fastpath function call' then true end as active,
                round((extract(epoch from clock_timestamp()) - extract(epoch from backend_start)) * 1000) as connection_time,
                case when xact_start is not null then round((extract(epoch from clock_timestamp()) - extract(epoch from xact_start)) * 1000) end as transaction_time,
                case when state = 'active' or state = 'fastpath function call' then round((extract(epoch from clock_timestamp()) - extract(epoch from query_start)) * 1000) end as query_time,
                case when state like 'idle%' then round((extract(epoch from clock_timestamp()) - extract(epoch from state_change)) * 1000) end as idle_time
            from pg_stat_activity
            where pid != pg_backend_pid()
        """

System info:

Version: current master (49988b1)

Steps to reproduce:

1. telegraf -config telegraf.conf -test

Expected behavior:

No error

Actual behavior:

Error parsing telegraf.conf, toml: line 8: parse error

Additional info:

Official TOML spec says that multiline strings are supported by using """ as the initiator and terminator.

Support for multi-line strings is rather important for some plugins, such as the postgresql_extensible plugin, which may require very long parameter values.

[phemmer]

Is there any particular reason telegraf is using it's own toml parser (influxdata/toml) instead of the more popular and seemingly more maintained BurntSushi/toml? The other influx projects are (influxdb & kapacitor)

[phemmer]

Another thing I just noticed, tabs in strings also throw a parse error.

[sparrc]

we're using influxdata/toml because it's based on naoina/toml.

The reason for naoina/toml is that we need to be able to modify the toml AST directly. The reason being that Telegraf allows for adding configuration parameters on a per-plugin basis that we don't want to have to incorporate into the struct of every single plugin. I'm not aware of any toml library that allows for that besides naoina/toml, but I haven't checked in some months.

[sparrc]

I would happily welcome an alternative, naoina/toml has bugs and the maintainer appears to have disappeared from the internet...

[sparrc]

@phemmer does it work if you use single-quotes? like:

[[inputs.postgresql_extensible]]
    address = "host=localhost sslmode=disable user=pgsql dbname=postgres"
    [[inputs.postgresql_extensible.query]]
        measurement = "postgresql_activity"
        sqlquery = '''
            select
                datname as database,
                usename as user,
                pid,
                client_addr,
                waiting,
                case when state like 'active' or state = 'fastpath function call' then true end as active,
                round((extract(epoch from clock_timestamp()) - extract(epoch from backend_start)) * 1000) as connection_time,
                case when xact_start is not null then round((extract(epoch from clock_timestamp()) - extract(epoch from xact_start)) * 1000) end as transaction_time,
                case when state = 'active' or state = 'fastpath function call' then round((extract(epoch from clock_timestamp()) - extract(epoch from query_start)) * 1000) end as query_time,
                case when state like 'idle%' then round((extract(epoch from clock_timestamp()) - extract(epoch from state_change)) * 1000) end as idle_time
            from pg_stat_activity
            where pid != pg_backend_pid()
        '''

[phemmer]

The reason for naoina/toml is that we need to be able to modify the toml AST directly. The reason being that Telegraf allows for adding configuration parameters on a per-plugin basis that we don't want to have to incorporate into the struct of every single plugin. I'm not aware of any toml library that allows for that besides naoina/toml, but I haven't checked in some months.

BurntSushi supports this. In fact they have an example for this exact use case in the docs: https://godoc.org/github.com/BurntSushi/toml#example-MetaData-PrimitiveDecode

I took a brief poke at it, and it does indeed work. However it'll require some work to integrate as the syntax of the config does not reflect the data structures. For example, there's lots of places where in the config a map[string]fooStruct{} is being represented, but in the code it's a slice of structs with a .Name field instead.

---

does it work if you use single-quotes? like:

Ahha, indeed it does. Thanks

I guess I'll close this, though I think it would be good to put in the effort to switch to BurntSushi/toml

[sparrc]

agreed, I'm going to reopen. Doesn't necessarily need to be BurntSushi/toml, although last time I checked that seemed to be the general standard (although I believe it also didn't support the latest revision of the toml spec, which might not matter either)

[jmmk]

I've also found a bug in the parser that it seems like BurntSushi/toml supports as well (I tested with BurntSushi/toml-test).

Broken version:

templates = [
  "my_stat_name.* .env.host.measurement* tag=value", # Comment explaining this template
  "measurement*", # default
]

Working version (simply had to remove the comments):

templates = [
  "my_stat_name.* .env.host.measurement* tag=value",
  "measurement*",
]

[sparrc]

There is also a bug with adding tags to multiple plugins of the same type:

[[inputs.prometheus]]
    urls = ["http://localhost:16023/monitoring/metrics/prometheus"]
    [inputs.prometheus.tags]
        appKey = "REBO2"

[[inputs.prometheus]]
    urls = ["http://localhost:16023/monitoring/metrics/prometheus"]
    [inputs.prometheus.tags]
        appKey = "REBO"

--> Error parsing telegraf2.conf, toml: line 30: table `inputs.prometheus.tags' is in conflict with normal table in line 25

[fjl]

I'm the new maintainer of github.com/naoina/toml. All known issues are fixed on the master branch.

The fixed bugs I can see in this issue are:

• strings with """
• comments in arrays
• bad table conflict detection involving an array table

[fjl]

It also supports encoding.TextUnmarshaler now, you might be able to remove handling of string quotes in toml.Unmarshaler implementations if you switch to using this interface ;).

[danielnelson]

To clarify the resolution, in #5513 we rebased our fork onto the current version of naoina/toml. We still may switch in the future, I did a loading prototype with burntsushi and it can definitely be done, but it's not an explicit goal.