telegraf icon indicating copy to clipboard operation
telegraf copied to clipboard

MQTT Consumer: Support Reading User-Properties

Open Jayclifford345 opened this issue 2 years ago • 9 comments

Feature Request

Opening a feature request kicks off a discussion.

Proposal:

If the internet connection dies, our device will remember the unsent messages, and when the internet connection comes back send all of them.

Since Telegraf is adding the timestamp, all of those messages would get wrong timestamps once they reach the server.

Instead, we want to use a user-property “time”, which is the time when we measured our actual sensor. Once they reach Telegraf, we want Telegraf to use that timestamp for the InfluxDB insert.

Current behaviour:

Telegraf does not support reading data from user-properties within a MQTT payload.

Desired behavior:

Provide the ability to parse user-property data. Similar to how we parse topics.

Use case:

we want to use a user-property “time”, which is the time when we measured our actual sensor. Once they reach Telegraf, we want Telegraf to use that timestamp for the InfluxDB insert.

The alternative is to use json_v2/timestamp_key. It seems Telegraf is using paho.mqtt.golang, there is this fork which supports mqtt 5, but it seems to be a bit different GitHub - eclipse/paho.golang: Go libraries, for example there is no ClientOptions. Therefore it is not a drop-in replacement.

Community link: https://community.influxdata.com/t/handling-mqtt-5-properties-in-telegraf/25255/2

Jayclifford345 avatar Jun 06 '22 10:06 Jayclifford345

Adding the ability to read the timestamp from a v5 user-property sounds like a useful feature. The mqtt_consumer input plugin was first written in 2016 and I think user-property arrived in mqtt v5 in 2019. Telegraf needs to update to support the new mqtt feature.

We welcome any help adding this feature so I'll add the help wanted label.

reimda avatar Jun 06 '22 18:06 reimda

There is PR #11284 which switches the libraries you mentioned.. (Edit: but seems to go about output, not input)

Hipska avatar Jun 13 '22 12:06 Hipska

Dear all and also @cmackenzie1 ,

I am looking into this issue and I think is relatively easy to solve this. In short, what I have in mind, is to add support to some/many of the new MQTTv5 options. I am mostly keen to add support to the expiration time to the messages https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html#_Toc3901064 but given I will be working here, I could also add some related setting, specially thanks to the current library we already have.

However, I have some questions about how is best doing the implementation. The question to me is mostly about the decision of having a polymorphic integration against MQTT v3 and v5. Some other integrations have had pretty strong separation for different versions, having entirely different modules. The quintessential example here is of course InfluxDB itself.

MQTTv5 has a lot of backward compatible settings though, and so far, having a polymorphic integration has worked well. But when it comes to adding version specific new settings, I am unclear how to manage that.

From a config perspective, how should this be manage? Should we create a version specific sub-table in the config file?

Should we have something like

# Configuration for MQTT server to send metrics to
[[outputs.mqtt]]
  ## MQTT Brokers
  ## The list of brokers should only include the hostname or IP address and the
  ## port to the broker. This should follow the format `[{scheme}://]{host}:{port}`. For
  ## example, `localhost:1883` or `mqtt://localhost:1883`.
  ## Scheme can be any of the following: tcp://, mqtt://, tls://, mqtts://
  ## non-TLS and TLS servers can not be mix-and-matched.
  servers = ["localhost:1883", ] # or ["mqtts://tls.example.com:1883"]

  ## Protocol can be `3.1.1` or `5`. Default is `3.1.1`
  procotol = "5"
  [outputs.mqtt.v5]
  v5_specific_setting = $BAR

If I do that, the struct will have a new property, which only is populated for v5, and not for the older versions. I am fine doing this, I am mostly wondering what the maintainers think about this and if we have a different preference in mind.

@svagner @powersj what do you guys think? I am flexible here following recommendation.

serroba avatar Jan 18 '23 07:01 serroba

I don't think v5 specific setting should go into a sub-table, unless there are lots of them? I am pro the protocol setting with a default value that is backward compatible to current users of telegraf (there can't be any changes to them if they don't change their config)

Hipska avatar Jan 18 '23 08:01 Hipska

@Hipska There are many of them indeed. And those are only V5 specific. I don't have to address them all though. I'm caring only about one actually, but I'm thinking about eventual full support if that makes sense

https://github.com/eclipse/paho.golang/blob/master/paho/cp_publish.go#L12

Have a look at those values. We could add V5 settings for the publish struct , but of particular interest, the PublishProperties is what we are after.

I was thinking to create at minimum a table for the publish properties, I think it's cohesive enough

serroba avatar Jan 18 '23 11:01 serroba

This issue is about Telegraf reading user-properties when mqtt is the input. Your proposal is specific to adding settings to the mqtt output. As such I think this conversation deserves its own issue as to not muddle this issue further.

powersj avatar Jan 18 '23 14:01 powersj

Oh! That's obvious now. Will create a new issue then

serroba avatar Jan 18 '23 19:01 serroba

Issue finally created https://github.com/influxdata/telegraf/issues/12526

serroba avatar Jan 20 '23 07:01 serroba

Any news on this? I think the support of MQTT 5 userProperties would nicely add value to the MQTT plugin. For example, providing the sensor timestamp when the measurements was taken. This comes in useful when older messages are delivered and the use of the current date and time would be wrong.

NorbNorb avatar Apr 08 '24 13:04 NorbNorb