RedHatInsights / yggdrasil

GNU General Public License v3.0
21 stars 37 forks source link

Echo worker should set fields in reply messages correctly #136

Closed jirihnidek closed 1 year ago

jirihnidek commented 1 year ago

Describe the bug The example echo worker just send messages as it was received, but it is not correct. Structure of data message is described here: https://github.com/RedHatInsights/yggdrasil/wiki/Protocol#data-messages

To Reproduce

  1. Start yggd on Terminal 1: go run ./cmd/yggd --server tcp://localhost:1883 --log-level trace --client-id $(hostname)
  2. Start echo worker on Terminal 2: go run ./worker/echo -log-level trace
  3. Subscribe to MQTT topics on Terminal 3: sub -broker tcp://localhost:1883 -topic yggdrasil/$(hostname)/#
  4. Send some testing MQTT message on Terminal 4: go run ./cmd/yggctl generate data-message --directive echo "hello" | pub -broker tcp://localhost:1883 -topic yggdrasil/$(hostname)/data/in

Current behavior You can see following messages on Terminal 3:

Message send from Terminal 4: 2023/06/23 15:55:16 [yggdrasil/thinkpad-p1/data/in]

{
  "type":"data",
  "message_id":"ebe2e691-1420-4cce-adcc-d1228c6c34d0",
  "response_to":"",
  "version":1,
  "sent":"2023-06-23T15:55:16.136331448+02:00",
  "directive":"echo",
  "metadata":{},
  "content":"aGVsbG8="
}

And response from echo worker: 2023/06/23 15:55:16 [yggdrasil/thinkpad-p1/data/out]

{
  "type":"data",
  "message_id":"ebe2e691-1420-4cce-adcc-d1228c6c34d0",
  "response_to":"",
  "version":1,
  "sent":"2023-06-23T15:55:16.141249454+02:00",
  "directive":"echo",
  "metadata":{},
  "content":"aGVsbG8="
}

As you can see the message_id is still the same in the response (ebe2e691-1420-4cce-adcc-d1228c6c34d0), but response_to is not set.

Expected behavior Response from echo worker should have unique message_id and it should set response_to to received message.

Additional Information

Open Questions How should echo worker work in remote content mode? Should we use metadata somehow? It looks like that return_url should have some special meaning: https://github.com/RedHatInsights/yggdrasil/wiki/Protocol#data-messages, but it is not described there. Documentation contains only example.

subpop commented 1 year ago

How should echo worker work in remote content mode? Should we use metadata somehow? It looks like that return_url should have some special meaning: https://github.com/RedHatInsights/yggdrasil/wiki/Protocol#data-messages, but it is not described there. Documentation contains only example.

The echo worker does not run in remote content mode, so it does not need to be addressed.

jirihnidek commented 1 year ago

How should echo worker work in remote content mode? Should we use metadata somehow? It looks like that return_url should have some special meaning: https://github.com/RedHatInsights/yggdrasil/wiki/Protocol#data-messages, but it is not described there. Documentation contains only example.

The echo worker does not run in remote content mode, so it does not need to be addressed.

The echo worker does work in remote content mode: https://github.com/RedHatInsights/yggdrasil/blob/main/worker/echo/main.go#L64

subpop commented 1 year ago

How should echo worker work in remote content mode? Should we use metadata somehow? It looks like that return_url should have some special meaning: https://github.com/RedHatInsights/yggdrasil/wiki/Protocol#data-messages, but it is not described there. Documentation contains only example.

The echo worker does not run in remote content mode, so it does not need to be addressed.

The echo worker does work in remote content mode: https://github.com/RedHatInsights/yggdrasil/blob/main/worker/echo/main.go#L64

Who would have enabled that?! I didn't approve it! Looks at git blame Oh. It was me. :blush:

subpop commented 1 year ago

How should echo worker work in remote content mode? Should we use metadata somehow? It looks like that return_url should have some special meaning: https://github.com/RedHatInsights/yggdrasil/wiki/Protocol#data-messages, but it is not described there. Documentation contains only example.

return_url does not have any special meaning. It exists in the documentation only as an example. The contents of the message metadata are converted into the HTTP headers for the resulting HTTP request. The only special field is the value of the addr field. In remote-content mode, the value of that field is parsed as a URL. If the echo worker should do anything, it should set the value of the addr field in its response to a valid URL.