Skip to content

Adds dynamic response diversion between connections#2190

Merged
alstanchev merged 8 commits intoeclipse-ditto:masterfrom
bosch-io:feature/response-rerouting
Oct 9, 2025
Merged

Adds dynamic response diversion between connections#2190
alstanchev merged 8 commits intoeclipse-ditto:masterfrom
bosch-io:feature/response-rerouting

Conversation

@alstanchev
Copy link
Contributor

@alstanchev alstanchev commented Jun 19, 2025 •
edited
Loading

Adds response diversion functionality to route command responses to different connections at runtime. Addresses use cases where responses need to go to systems other than the original sender.

Implements #2106

@alstanchev alstanchev requested a review from thjaeckle June 19, 2025 06:43
@alstanchev alstanchev changed the title dynamic response diversion between connections Adds dynamic response diversion between connections Jun 19, 2025
@alstanchev
Copy link
Contributor Author

alstanchev commented Jun 19, 2025 •
edited
Loading

TODO:

  • System tests (in progress)
  • Few TODOs to be discused

@alstanchev alstanchev added this to the 3.8.0 milestone Jun 19, 2025
@alstanchev alstanchev linked an issue Jun 19, 2025 that may be closed by this pull request
Add support for diverting twin responses to another connection #2106
Closed
@alstanchev alstanchev self-assigned this Jun 19, 2025
@alstanchev alstanchev moved this to In Progress in Ditto Planning Jun 19, 2025
thjaeckle
thjaeckle requested changes Jun 25, 2025
View reviewed changes
Copy link
Member

@thjaeckle thjaeckle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alstanchev I had a first brief look and left some feedback .. but did not yet "dive into" the pub/sub stuff too much :D

@alstanchev alstanchev force-pushed the feature/response-rerouting branch from 72885b9 to 4e95a6b Compare August 8, 2025 11:38
thjaeckle
thjaeckle requested changes Sep 1, 2025
View reviewed changes
Copy link
Member

@thjaeckle thjaeckle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @alstanchev

Already looking quite good 👍

I left some little remarks inline - I also prepared a review commit, but have no permission to push it on your fork :)

I will upload it as diff to the PR.

@thjaeckle
Copy link
Member

thjaeckle commented Sep 1, 2025

2106__review__changed_Signal_to_CommandResponse_everywhere_applicable_for__response_diver.patch

@thjaeckle
Copy link
Member

thjaeckle commented Sep 9, 2025

@alstanchev is this PR still in "Draft"?
Did you see my remarks (the open threads)?

@alstanchev alstanchev marked this pull request as ready for review September 11, 2025 12:30
@alstanchev
Copy link
Contributor Author

alstanchev commented Sep 11, 2025 •
edited
Loading

@alstanchev is this PR still in "Draft"? Did you see my remarks (the open threads)?

Hi @thjaeckle it is no longer a "Draft". I have addressed the remarks, updated the doc and made some minor improvements.

@thjaeckle
Copy link
Member

thjaeckle commented Sep 11, 2025

@alstanchev did/do you also add system-tests for this feature?

@thjaeckle
Copy link
Member

thjaeckle commented Sep 11, 2025

I started a SystemTests run: https://github.com/eclipse-ditto/ditto/actions/runs/17647744642

@alstanchev alstanchev mentioned this pull request Sep 12, 2025
Merged
@alstanchev
Copy link
Contributor Author

alstanchev commented Sep 12, 2025

@alstanchev did/do you also add system-tests for this feature?

I wondered why you ask me this until i saw i had wrong remote for my testing repo. 😁

The tests are pushed and i have started a run.
https://github.com/eclipse-ditto/ditto/actions/runs/17672621161

@thjaeckle
Copy link
Member

thjaeckle commented Sep 16, 2025

@alstanchev the Kafka IT test divertAndPreserveOriginalResponseInSourceConnection failed in the test run you linked - same in the Amqp10 IT

@alstanchev
Copy link
Contributor Author

alstanchev commented Sep 16, 2025 •
edited
Loading

Hi @alstanchev, yes i saw them but was dealing with other issues. I will see why, fix them and write back as soon as possible

@alstanchev
Copy link
Contributor Author

alstanchev commented Sep 25, 2025 •
edited
Loading

@thjaeckle, fixed the tests and they pass every time locally. But as we are lucky people 😤, they fail in the GH action. Looking in to it now.

@alstanchev
Connection response diversion functionality and unit tests
663a69c 
Signed-off-by: Aleksandar Stanchev <aleksandar.stanchev@bosch.com>
@alstanchev
Connection response diversion doc
6112c2a 
Signed-off-by: Aleksandar Stanchev <aleksandar.stanchev@bosch.com>
@alstanchev
Connection response diversion blogpost
3ae8bf3 
Signed-off-by: Aleksandar Stanchev <aleksandar.stanchev@bosch.com>
@alstanchev
rename headers
6c25b7b 
Signed-off-by: Aleksandar Stanchev <aleksandar.stanchev@bosch.com>
@alstanchev
Some review fixes and little refactoring
3364198 
Signed-off-by: Aleksandar Stanchev <aleksandar.stanchev@bosch.com>
@alstanchev
eclipse-ditto#2106: review: changed Signal to CommandResponse everywh…
711b4b8 
…ere applicable for "response diversion", providing a little more "safety"

Signed-off-by: Aleksandar Stanchev <aleksandar.stanchev@bosch.com>
@alstanchev
Review fixes and little refactoring 2
4adb76b 
Signed-off-by: Aleksandar Stanchev <aleksandar.stanchev@bosch.com>
@alstanchev alstanchev force-pushed the feature/response-rerouting branch from a2be9c0 to 4adb76b Compare October 1, 2025 13:14
@alstanchev
Copy link
Contributor Author

alstanchev commented Oct 2, 2025

@thjaeckle fixed the tests. Currently there are failing RestConnectionsIT but they also fail on master. There seems to be some issue with thr JWT configuration.

@thjaeckle
Copy link
Member

thjaeckle commented Oct 2, 2025

@alstanchev aright. I will have a look at why they fail on master

And will do another final review of the PR

@alstanchev
Copy link
Contributor Author

alstanchev commented Oct 2, 2025 •
edited
Loading

@thjaeckle The RestConnectionsIT are green now. The OAUTH_PROTOCOL env was not working for some reason but setting CONFIG_FORCE_ditto_gateway_authentication_oauth_protocol in the gateway confgi in the compose did the job.

thjaeckle
thjaeckle reviewed Oct 7, 2025
View reviewed changes
documentation/src/main/resources/pages/ditto/connectivity-response-diversion.md Outdated

#### Header mapping keys

#### divert-response-to-connection
Copy link
Member

@thjaeckle thjaeckle Oct 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If "Header mapping keys" is level 4 heading, this (and the other header keys as well) must be a level 5

thjaeckle
thjaeckle approved these changes Oct 7, 2025
View reviewed changes
Copy link
Member

@thjaeckle thjaeckle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from one little remark about the documentation, this looks very good 👍

Please also update the blogpost date before merging :)

@alstanchev
blogpost date update
7f511fa 
Signed-off-by: Aleksandar Stanchev <aleksandar.stanchev@bosch.com>
@alstanchev alstanchev merged commit 14c5e30 into eclipse-ditto:master Oct 9, 2025
3 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in Ditto Planning Oct 9, 2025
@alstanchev alstanchev deleted the feature/response-rerouting branch October 9, 2025 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thjaeckle thjaeckle thjaeckle approved these changes

Assignees

@alstanchev alstanchev

Labels

enhancement

Projects

Ditto Planning
Status: Done

Milestone

3.8.0

Development

Successfully merging this pull request may close these issues.

Add support for diverting twin responses to another connection

2 participants

@alstanchev @thjaeckle