MQTT transport enhancement: compliance with MQTT v5 standard message properties

[manifest]

The MQTT v5 has got support of request/response message pattern. For that reason, message properties such as: "response_topic" and "correlation_date" were introduced. Any client that supports MQTT v5 standard now is able to utilize request/response message pattern out the box. At the same time, the choice fully on client side and client applications can continue using their own ways to correlate requests and responses on top of async pub/sub messages.

The proposal is to add a support for the new clear way of communication over MQTT, that's now available by modern clients with the support of MQTT v5 standard.

What we can do:

1. We may store incoming message properties within an internal state of the MQTT transport plugin, where a state record consists of the key: "transaction" and the value: "message properties".
2. For any message sent with push_event a "transaction" is known, so we may find a record in the internal state.
3. Then, we may:
   • use "response_topic" from incoming message properties as a destination topic of an outgoing message
   • pass "correlation_data" from incoming message properties to outgoing message as is
     • a bit more flexible way may be to specify a list of properties to pass within the plugin configuration

That's may be important to mention that:

• These changes aren't affect in any way (source code, API, the way how clients interact with plugins) application plugins or the other transport plugins. The changes make sense only for MQTT transport plugin and are encapsulated within it.
• They are not breaking changes, so current users may proceed without any changes to their code. At the same time, those who want to utilize new features of MQTT v5 standard will be able to do that.

Specs:

• Correlation Data
  https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html#_Toc3901115
• Response Topic
  https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html#_Toc3901114

[lminiero]

I guess this is related to the discussion in #1844?
As I said there, we do something similar in the RabbitMQ transport, where we use the correlation ID to match responses and requests. What I don't think is needed is treating events the same way, even when they have a transaction ID set (application level correlation is enough in that case).

[manifest]

This issue sums up ideas described in the last post of the PR. The proposal doesn't prevent anyone of using correlation ID in payload, it just makes it possible to utilize full potential of MQTT transport. Since all the changes on the transport level, that doesn't affect work of Janus application plugins in any way. It just adds more convenience to MQTT users (that allows to use request/response feature of modern MQTT clients).

[lminiero]

I'm more concerned about people using the MQTT transport from their application right now: I suspect these changes would break the way they communicate with the plugin right now, as I guess they might be expecting a message on one topic and it might arrive on another. As long as these changes are optional and configurable (and, if they break backwards compatibility, disabled by default at least for the time being), it's fine by me. Just as for the other issue, though, this isn't something we plan to work on ourselves, sorry.

[manifest]

I'm more concerned about people using the MQTT transport from their application right now: I suspect these changes would break the way they communicate with the plugin right now, as I guess they might be expecting a message on one topic and it might arrive on another.

There couldn't be any issues with backward compatibility because sending the response topic property in a request only makes sense if you expected a response on a different topic. So, current users of the MQTT plugin won't be affected.

As long as these changes are optional and configurable (and, if they break backwards compatibility, disabled by default at least for the time being), it's fine by me.

We'll prepare a PR in the nearest future. Thanks.

[lminiero]

Thanks, this makes it much clearer, especially since I know so little about MQTT so this helps!

[feymartynov]

The PR is merged so I guess this issue can be closed.