Added Custom Message Processor for Hub & Things services. #35

yasen-aleksandrov · 2019-11-27T12:33:03Z

Please, refer to https://wiki.bosch-si.com/display/GTM/Custom+Message+Processor+in+Detail !

dguggemos

thank you very much for your contribution! I reviewed the pull request and have some general remarks/questions, apart from the comments made to the code directly:

the Hub changed its endpoint format, the control endpoints are now deprecated (I know, at the time of your pull request, they were not, sorry for the late review 😇) and will be removed soon. Could you adopt the processor to the new endpoint format? (see https://www.eclipse.org/hono/docs/api/command-and-control/)
I do not yet understand why we need two amqp clients (hub and internal). FMPOV this makes the architecture more complex than necessary. Both directions (upstream and downstream) could be achieved with one ProtonServer and one ProtonClient. Maybe I missed something?
the MessageProcessingService does the processing only in one direction? But we have upstream and downstream messages? Maybe we need two methods?
is it necessary that the processor always opens the connection to the Hub on its own and not only on command, when there is a connection from Things
we discussed if it is possible to also forward authentication to avoid specifying the credentials for the Hub connection. Did you try this or was this out of scope?

Regards, Dominik

dguggemos · 2020-01-14T11:38:19Z

message-processor/pom.xml

+        <dependency>
+            <groupId>io.vertx</groupId>
+            <artifactId>vertx-proton</artifactId>
+            <version>3.8.2</version>


use ${stack.version}

use properties for the other versions as well?

dguggemos · 2020-01-14T11:40:15Z

message-processor/src/main/java/com/bosch/iot/things/example/message/processor/Constants.java

+public class Constants {
+    public static final String TELEMETRY_ENDPOINT = "telemetry/";
+    public static final String EVENT_ENDPOINT = "event/";
+    public static final String CONTROL_ENDPOINT = "control/";


control endpoint is deprecated, replace with command/command_response

Yes, I did notice some changes must have been made recently, as some things didn't work accordingly!
Will have a look at the new endpoints and make the necessary adjustments!
thanks

dguggemos · 2020-01-14T11:42:15Z

...java/com/bosch/iot/things/example/message/processor/processing/MessageProcessingService.java

+import org.springframework.stereotype.Service;
+
+@Service
+public class MessageProcessingService {


It makes sense to define an interface for this service, as this will be a custom implementation, depending on the use case. Furthermore we need two methods for both directions (upstream/downstream) to e.g. decrypt/encrypt, right?

Sure, that could be done!
At the time I was just aiming to introduce the message processing in as simple as possible manner and later figure out the details.

dguggemos · 2020-01-14T11:43:13Z

...essor/src/main/java/com/bosch/iot/things/example/message/processor/transport/AmqpClient.java

+        });
+    }
+
+    public boolean isConnected(int timeout) {


I'm not sure if we should always start a connection, or only when a connection is requested.

Well, this one is essentially going be to replacing the "out of the box" connection being defined on initial provisioning of the Asset Communication Package. Thus, I thought this connection would need to be set up and ready to handle traffic at all times?

dguggemos · 2020-01-14T11:44:12Z

...essor/src/main/java/com/bosch/iot/things/example/message/processor/transport/AmqpClient.java

+    @Value(value = "${hub.client.host}")
+    private String hubHost;
+
+    @Value(value = "${hub.client.port}")
+    private Integer hubPort;
+
+    @Value(value = "${hub.client.username}")
+    private String hubUsername;
+
+    @Value(value = "${hub.client.password}")
+    private String hubPassword;


Is it possible to "forward" the credentials? Then we would not have to configure it for every instance.

I will have a look and see what I could do about it!

dguggemos · 2020-01-14T11:46:52Z

...essor/src/main/java/com/bosch/iot/things/example/message/processor/transport/AmqpServer.java

+    }
+
+    private void sendMessage(Message msg, String address) {
+        if (address.startsWith(CONTROL_ENDPOINT) && !address.endsWith(CONTROL_REPLY_ENDPOINT)) {


This would not be necessary if we wire the associated receivers/senders on creation (e.g. a receiver on Things side will never receive a telemetry or event message and a receiver on hub side will never receive a command message).

yasen-aleksandrov · 2020-01-16T10:41:36Z

@dguggemos
As for the approach taken by me with the two underlying proton clients - I couldn't figure out other way of putting messages through one end(Hub) to the other(Things). I thought we'd need these two clients since we've practically got two servers. So by each client establishing connection to the respective server(Hub or internal AMQP) we are able to have a separate channel for sending/receiving messages from Hub on one side and then forwarding them through the other internal connection up to the Things service. I don't claim to be right here( just that's how much I knew at the time), correct me if I'm wrong, but can one Proton Client be connected to both Hub and internal AMQP servers simultaneously?

dguggemos · 2020-01-23T08:38:32Z

No, a client can only be connected to one server at a time. But the basic idea is

we have one amqp server, that accepts connections from things (it mimics the behavior of the hub)
when connection from things is opened, a client is started that connects to the hub
usually for acp several receivers are opened from things side (e.g. telemetry)
- this creates a sender in the server
- now we must also open a receiver on the same address at the hub (we simply forward the "receive request")
now we have
- one server with a sender
- one client with a receiver
- if we forward the messages from the receiver to the sender (after custom processing), we are done
- the message flow is then hub -> receiver (client) -> processor -> sender (server) -> things

So there is no need to have multiple clients. The other direction works the same, but with a receiver at the server and a sender at the client.

Address some more issues mentioned in PR comments.

yasen-aleksandrov · 2020-01-24T12:58:02Z

I get your point!
That was also my process of thinking, but I just couldn't quite adopt it at the time , so I went with my approach. Let me see If I can get rid of the extra client and stick to your explanation.
Thanks

Initialize Hub connection upon Things connecting to internal amqp server.

yasen-aleksandrov · 2020-01-30T14:03:19Z

@dguggemos
I addressed both your remarks within the last commit.
That being:

request/establish a hub connection once Things service has successfully opened a connection to internal AMQP server.
get rid of additional internal AMQP client I'd previously use with my initial approach.

Added message-processor module.

9da0ea0

dguggemos reviewed Jan 14, 2020

View reviewed changes

Changed deprecated control endpoint with command.

eddfa48

Address some more issues mentioned in PR comments.

Simplified architecture by removing internal proton client.

eef08da

Initialize Hub connection upon Things connecting to internal amqp server.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added Custom Message Processor for Hub & Things services. #35

Added Custom Message Processor for Hub & Things services. #35

yasen-aleksandrov commented Nov 27, 2019 •

edited

Loading

dguggemos left a comment

dguggemos Jan 14, 2020

dguggemos Jan 14, 2020

dguggemos Jan 14, 2020

yasen-aleksandrov Jan 16, 2020

dguggemos Jan 14, 2020

yasen-aleksandrov Jan 16, 2020

dguggemos Jan 14, 2020

yasen-aleksandrov Jan 16, 2020

dguggemos Jan 14, 2020

yasen-aleksandrov Jan 16, 2020

dguggemos Jan 14, 2020

yasen-aleksandrov commented Jan 16, 2020

dguggemos commented Jan 23, 2020

yasen-aleksandrov commented Jan 24, 2020 •

edited

Loading

yasen-aleksandrov commented Jan 30, 2020

Added Custom Message Processor for Hub & Things services. #35

Are you sure you want to change the base?

Added Custom Message Processor for Hub & Things services. #35

Conversation

yasen-aleksandrov commented Nov 27, 2019 • edited Loading

dguggemos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yasen-aleksandrov commented Jan 16, 2020

dguggemos commented Jan 23, 2020

yasen-aleksandrov commented Jan 24, 2020 • edited Loading

yasen-aleksandrov commented Jan 30, 2020

yasen-aleksandrov commented Nov 27, 2019 •

edited

Loading

yasen-aleksandrov commented Jan 24, 2020 •

edited

Loading