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.

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

Open
wants to merge 1 commit into
base: 7.4
Choose a base branch
from

Conversation

Samik081
Copy link

@Samik081 Samik081 commented May 4, 2022

Q/A

Q A
Branch? 6.3
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
Copy link

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:

  • Always add tests
  • Keep backward compatibility (see https://symfony.com/bc).
  • Bug fixes must be submitted against the lowest maintained branch where they apply (see https://symfony.com/releases)
  • Features and deprecations must be submitted against the 6.1 branch.

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

@carsonbot
Copy link

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
Add possibility to configure exchange to exchange bindings in AMQP transport
@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 modified the milestones: 7.3, 7.4 May 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants