modbus: improved config format

[h-schwanbeck]

Dear influxdata team,

I prepared a few patches for the MODBUS input plugin.
Besides some smaller ones to suppress unwanted tags, there is also a breaking change to the config format.

The idea behind this update is that MODBUS devices, like PLCs, often control multiple pieces of equipment one wants to monitor and the semantics of the configuration file should reflect this.
I also think that this is more consistent with the config of other telegraf plugins than the current format.
The introduction of custom tags per measurement enables the user to use more of the potential of InfluxDB.

As an example, let's monitor two identical generators connected to the same PLC.

Current format:

[[inputs.modbus]]
...
holding_registers = [
    { measurement = "generator_roof", name = "voltage",      byte_order = "AB",   data_type = "FIXED", scale=0.1,   address = [0]},
    { measurement = "generator_roof", name = "current",      byte_order = "ABCD", data_type = "FIXED", scale=0.001, address = [1,2]},
    { measurement = "generator_roof", name = "frequency",    byte_order = "AB",   data_type = "UFIXED", scale=0.1,  address = [7]},
    { measurement = "generator_basement", name = "voltage",      byte_order = "AB",   data_type = "FIXED", scale=0.1,   address = [10]},
    { measurement = "generator_basement", name = "current",      byte_order = "ABCD", data_type = "FIXED", scale=0.001, address = [11,12]},
    { measurement = "generator_basement", name = "frequency",    byte_order = "AB",   data_type = "UFIXED", scale=0.1,  address = [17]},
  ]

Output:

generator_roof,type=holding_registers,[..] voltage=0i,current=0i,frequency=0i
generator_basement,type=holding_registers,[..] voltage=0i,current=0i,frequency=0i

Proposed format:

[[inputs.modbus]]
...
  [[inputs.modbus.measurement]]
    name = "generator"
    tags = [{key = "location", value = "roof"},]
    holding_registers = [
      { name = "voltage",      byte_order = "AB",   data_type = "FIXED", scale=0.1,   address = [0]},
      { name = "current",      byte_order = "ABCD", data_type = "FIXED", scale=0.001, address = [1,2]},
      { name = "frequency",    byte_order = "AB",   data_type = "UFIXED", scale=0.1,  address = [7]}
    ]

  [[inputs.modbus.measurement]]
    name = "generator"
    tags = [{key = "location", value = "basement"},]
    holding_registers = [
      { name = "voltage",      byte_order = "AB",   data_type = "FIXED", scale=0.1,   address = [10]},
      { name = "current",      byte_order = "ABCD", data_type = "FIXED", scale=0.001, address = [11,12]},
      { name = "frequency",    byte_order = "AB",   data_type = "UFIXED", scale=0.1,  address = [17]}
    ]

Output:

generator,type=holding_registers,location=roof,[..] voltage=0i,current=0i,frequency=0i
generator,type=holding_registers,location=basement,[..] voltage=0i,current=0i,frequency=0i

I am looking forward to your feedback and hope you consider merging.

Best regards,

Henry Schwanbeck

Required for all PRs:

• Signed CLA.
• Associated README.md updated.
• Has appropriate unit tests.

[srebhan]

Hey @h-schwanbeck,

thanks very much for caring about this overdue topic! While I like the idea of a more clean config I have some comments:

1. We cannot just change the config format as otherwise we break existing setups with the next release. So the only chance is to support both formats (at least for a number of releases).
2. There is more and more demand to support multiple modbus-slaves with one plugin (due to constraint number of concurrent connections). Could you please think about how this can be done with your format change!? We really should take this into account.
3. Is there any advantage in adding the omit_... options instead of using the existing global tagdrop option?

Regarding 2: I think one option is to have the slave as subsections and allow to specify the measurements as a subsection of this subsection. Do you think this would be possible? Or can you come up with a better structure?

[reimda]

Hi Henry, I like being able to specify tags here, but saying key = foo, value = bar feels wrong. The existing way to add tags to a plugin lets you say foo = bar. see https://github.com/influxdata/telegraf/blob/master/docs/CONFIGURATION.md#input-plugins I don't think we should add a second tag settings format.

There's not currently a way to have a plugin get arbitrary settings from toml, like we would need in order to get tags in the foo = bar format. The existing plugin level tags table is handled as a special case that accesses the toml ast.

telegraf/config/config.go

Line 1231 in d369003

cp.Tags = make(map[string]string)

I'm working on a PR on the OPC UA input (#8389) that groups variables in a similar way to this PR. I'd like to add tags to a group just like you're doing here. I suspect other similar inputs will need this in the future too. I think the tag config syntax should be the same across plugins and I'd like it to be in foo = bar format.

Lets work together to add a way to get tags in foo = bar format for both PRs.

[h-schwanbeck]

Hello @srebhan,

1. We cannot just change the config format as otherwise we break existing setups with the next release. So the only chance is to support both formats (at least for a number of releases).

Right, stability matters. I'll have a look at the code to see if I can come up with an elegant solution.

2. There is more and more demand to support multiple modbus-slaves with one plugin (due to constraint number of concurrent connections). Could you please think about how this can be done with your format change!? We really should take this into account.
   I think one option is to have the slave as subsections and allow to specify the measurements as a subsection of this subsection. Do you think this would be possible? Or can you come up with a better structure?

Not sure if I understand you correctly. I query two PLCs by including multiple [[inputs.modbus]] sections in my telegraf config. My assumption was that the telegraf core would take care of managing concurrency.
If we need to serialize inside the plugin, a device section would probably be the way to go.

3. Is there any advantage in adding the omit_... options instead of using the existing global tagdrop option?

I guess you mean 'tagexclude' and no, there's no advantage, quite the opposite. I have not thought about that when I added the patch. Thanks for the hint, saves a few lines of code.

[srebhan]

Not sure if I understand you correctly. I query two PLCs by including multiple [[inputs.modbus]] sections in my telegraf config. My assumption was that the telegraf core would take care of managing concurrency.
If we need to serialize inside the plugin, a device section would probably be the way to go.

Sure telegraf can handle this. However, there are gateway devices which themselves handles slaves e.g. on a serial bus and provide access to those slaves with different slave IDs. I have a SOCOMEC D70 for example having one IP but you can query a dozen of slaves through this one IP. Working with multiple [[inputs.modbus]] sections is fine as long as you do not exceed the client limit of the gateway. However this can happen quickly as you setup an individual TCP/IP connection per [[inputs.modbus]] sections to this device and most devices only support a low number of simultaneous connection (e.g. 5).

Maybe you can take a look at PR #8013 to see if we can come up with an overarching config scheme!?

[WeiTangLau]

Is there any further update regarding this!
Think this is an awesome upgrade!

[srebhan]

I agree. It's on my list once #9279 is merged.

[srebhan]

@h-schwanbeck the modbus plugin now has the possibility to specify different configuration formats (see #9279 for an example). Do you want to rebase your PR on that new possibility?

[telegraf-tiger]

Hello! I am closing this issue due to inactivity. I hope you were able to resolve your problem, if not please try posting this question in our Community Slack or Community Page. Thank you!