Skip to content

[Messenger] Add AMQP exchange to exchange bindings #46257

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
fabpot merged 1 commit into from
Sep 17, 2025
Merged

[Messenger] Add AMQP exchange to exchange bindings #46257

fabpot merged 1 commit into from
Sep 17, 2025

Conversation

@Samik081
Copy link
Contributor

@Samik081 Samik081 commented May 4, 2022 •
edited
Loading

Q/A

Q A
Branch? 7.4
Bug fix? no
New feature? yes
Deprecations? no
Tickets
License MIT
Doc PR symfony/symfony-docs#16783

Changes description

This PR introduces very similar changes to this one: #34737, which was closed due to the lack of the feedback.
I'd like to continue this topic, share the missing feedback and discuss it further if needed.

I have introduced the possibility to configure exchange-to-exchange bindings in amqp transport. This feature uses \AMQPExchange::bind() method already provided by the stub in php-amqp/php-amqp: https://github.com/php-amqp/php-amqp/blob/bb7611220e341039a7f5d72e606ca1e16eda4642/stubs/AMQPExchange.php#L22

Example messenger.yaml:

framework:
    messenger:
        transports:
            some_transport:
                dsn: 'amqp://'
                options:
                    exchange:
                        type: topic
                        name: some_exchange
                        bindings: # added configuration
                            another_exchange:
                                binding_keys:
                                    - key1
                                    - key2
                                binding_arguments:
                                    x-match: all

With the above configuration, the Connection class creates some_exchange and binds it to another_exchange using ['key1', 'key2'] keys and [x-match =>'all'] arguments.

Reasoning

Binding an exchange to an exchange feature can be used to create more complex RabbitMQ topologies. It is also briefly described here: https://www.cloudamqp.com/blog/exchange-to-exchange-binding-in-rabbitmq.html

A real-world example could be kind of RabbitMQ publisher/subscriber pattern implementation between microservices, that could be visualized as follows:

rabbitmq

In the above example (all the exchanges in this example are of the topic type):

App `Foo` publishes events to its own exchange AND subscribes to the events that app `Bar` publishes on its exchange.
App `Bar` publishes events to its own exchange AND subscribes to the events that app `Foo` publishes on its exchange.
App `...` subscribes to the events that apps `Foo` and `Bar` publish on their exchanges.

This approach might have few advantages, such as easier maintainability, dependency management and monitoring, or better separation between microservices.

My thoughts

I feel that the fact, that https://github.com/php-amqp/php-amqp has \AMQPExchange::bind() implemented is already sufficient reason to have this supported in symfony/amqp-messenger. I am aware this feature might be rarely used, but it's already there in the extension, and having the ability to use it within amqp-messenger seems reasonable to me.

@carsonbot carsonbot added this to the 6.1 milestone May 4, 2022
@carsonbot
Copy link

carsonbot commented May 4, 2022

Hey!

I see that this is your first PR. That is great! Welcome!

Symfony has a contribution guide which I suggest you to read.

In short:

Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change.

When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor!
If this PR is merged in a lower version branch, it will be merged up to all maintained branches within a few days.

I am going to sit back now and wait for the reviews.

Cheers!

Carsonbot

@Samik081 Samik081 mentioned this pull request May 4, 2022
Open
@carsonbot
Copy link

carsonbot commented May 5, 2022

Hey!

I think @ruudk has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

@fabpot fabpot modified the milestones: 6.1, 6.2 May 8, 2022
@Samik081 Samik081 force-pushed the amqp-exch2exch-binding branch from 1b341a5 to 124b5f8 Compare May 10, 2022 11:10
@Samik081 Samik081 force-pushed the amqp-exch2exch-binding branch 2 times, most recently from 2f89e25 to 16f7122 Compare May 23, 2022 09:51
@Samik081 Samik081 force-pushed the amqp-exch2exch-binding branch from 16f7122 to af915cd Compare May 31, 2022 08:48
@Samik081 Samik081 force-pushed the amqp-exch2exch-binding branch 2 times, most recently from 09c351c to e5dfa54 Compare June 21, 2022 09:53
@Samik081 Samik081 force-pushed the amqp-exch2exch-binding branch 2 times, most recently from d39e7db to a304cd5 Compare August 10, 2022 12:18
@Samik081 Samik081 force-pushed the amqp-exch2exch-binding branch from a304cd5 to 1331e74 Compare September 12, 2022 12:42
@nicolas-grekas nicolas-grekas modified the milestones: 6.2, 6.3 Nov 5, 2022
@Samik081 Samik081 force-pushed the amqp-exch2exch-binding branch from 1331e74 to 6021296 Compare January 4, 2023 12:10
@nicolas-grekas nicolas-grekas modified the milestones: 6.3, 6.4 May 23, 2023
@nicolas-grekas nicolas-grekas modified the milestones: 6.4, 7.1 Nov 15, 2023
@xabbuh xabbuh modified the milestones: 7.1, 7.2 May 15, 2024
@fabpot fabpot modified the milestones: 7.2, 7.3 Nov 20, 2024
@fabpot fabpot removed this from the 7.3 milestone May 26, 2025
@fabpot fabpot added this to the 7.4 milestone May 26, 2025
@Samik081 Samik081 force-pushed the amqp-exch2exch-binding branch from 6021296 to 928a2a2 Compare September 4, 2025 09:44
@Samik081
Copy link
Contributor Author

Samik081 commented Sep 4, 2025

Hi @nicolas-grekas, @xabbuh,

Apologies for the direct ping, but I'm trying to get some attention on this PR. I saw your recent reviews on the Connection class, which this PR modifies, and hoped you might be able to take a look.

This feature is still very much needed for my projects and company; we've been relying on workarounds for years. Integrating this into the Messenger component would be a significant improvement for us.

Is there any possibility of a review, or could you advise on who might be best to ask?

Thanks a lot!

@jperdior
Copy link

jperdior commented Sep 9, 2025

Aside of the fact that this is already supported by the underlying php-amqplib and therefore supporting it also by symfony messenger should be natural and useful.
About use case for this, that was deemed too advanced or unnecessary in the comments in the previous PR; IMO, specially when it comes to working with micro-services, being able to have decentralized queue routing where each micro-service owns its own exchange it's a must, not a nice to have.

nicolas-grekas
nicolas-grekas reviewed Sep 9, 2025
View reviewed changes
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

No objection to merge this on my side. I just have a technical comment.

nicolas-grekas
nicolas-grekas approved these changes Sep 9, 2025
View reviewed changes
@Samik081
Copy link
Contributor Author

Samik081 commented Sep 9, 2025

Big thanks for the review and a catch, @nicolas-grekas, I really appreciate that!

@fabpot fabpot force-pushed the amqp-exch2exch-binding branch from 333e5cd to 9fd9049 Compare September 17, 2025 06:06
@fabpot
Copy link
Member

fabpot commented Sep 17, 2025

Thank you @Samik081.

@fabpot fabpot merged commit 46098d2 into symfony:7.4 Sep 17, 2025
11 of 12 checks passed
This was referenced Oct 27, 2025
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fabpot fabpot fabpot approved these changes

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

Assignees

No one assigned

Labels

Feature Messenger Status: Reviewed

Projects

None yet

Milestone

7.4

Development

Successfully merging this pull request may close these issues.

6 participants

@Samik081 @carsonbot @jperdior @fabpot @nicolas-grekas @xabbuh