Skip to content
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

AMQP - Support multiple instances of RabbitTemplate #67

Closed
DmitriButorchin opened this issue May 24, 2022 · 6 comments · Fixed by #69
Closed

AMQP - Support multiple instances of RabbitTemplate #67

DmitriButorchin opened this issue May 24, 2022 · 6 comments · Fixed by #69
Assignees
Labels
enhancement New feature or request

Comments

@DmitriButorchin
Copy link
Contributor

There's a handful of problems with supporting a case when there are multiple RabbitTemplate's.
Here's a use case, instead of using one rabbit template with a default exchange, and specifying a concrete exchange and routing key when sending messages, there could be multiple rabbit template with set exchanges, so that when sending a message you specify only the routing key.
The issues:

  1. The app fails to start when there are multiple instances of RabbitTemplate's as the SpringwolfAmqpProducer requires a single instance (I can mark one of the RabbitTemplate's with Primary annotation, but that brings up the following issue)
  2. The send method in the SpringwolfAmqpProducer uses the channelName to send the message, which is ok when we use a default exchange - we could just use the queue name here, but when we're not - we have to supply a routing key instead.

I think it needs to use a List of RabbitTemplate's in the SpringwolfAmqpProducer bean, and choose an appropriate one to send the message.
In order to send the message correctly, can the channel binding and operation binding info be used? https://github.com/asyncapi/bindings/tree/master/amqp#channel-binding-object (Specifically exchange.name/queue.name from channel binding and cc from operation binding)

Here's a starter demo if you want to test it out: https://github.com/DmitriButorchin/amqp-demo/tree/multiple-rabbit-templates

@stavshamir stavshamir added the enhancement New feature or request label May 25, 2022
@DmitriButorchin
Copy link
Contributor Author

I thought about it a bit, and it seems this can be fixed by simply injecting a list of templates in the SpringwolfAmqpProducer and just using the first one.
But this still leaves the question open of how to specify the producer operation that send a message to a specific exchange with a specific routing key. Routing key can be put in the operation binding, which is already present in the model, but the channel binding has to be added for this to work as well.
Of course it's not really convenient to add this info manually building up the object, but this can be simplified with an annotation that you can instead put on a method and specify exchange name, routing key, description, payload type and channel name - but that is probably a separate issue.

Does this make sense?

@stavshamir
Copy link
Member

Thanks for the detailed issue!
I have only minimal knowledge regarding rabbit, so I just want to make sure that I understood - what you would like as a user is to be able to declare multiple producer channels, and to be able to publish a message to them from the UI?

We can definitely use the bindings and operation bindings to route the message correctly.

Does this make sense?
Sure, it does. I had the idea of providing an annotation based approach to declaring the producer channels, but I never got it.

To move this forward, I think we should need to clearly define:

  • What would you like to do as a user
  • Define the new interface/API changes and additions

Once we these, we can design a solution.

@DmitriButorchin
Copy link
Contributor Author

DmitriButorchin commented May 26, 2022

Just so we're on the same page, RabbitMQ (which is AMQP 0-9-1) publishes a message to specific exchange with some routing key. That message is routed to a queue based on that information.
So to describe the producer/consumer fully we need to supply the exchange name (if it's not a default exchange) and a routing key.
And based on the spec https://github.com/asyncapi/bindings/tree/master/amqp#channel-binding-object the exchange name is described at the channel binding level, and routing key at the operation binding level.

I think these are the main things user should be able to do (probably also in the order of importance):

  1. Be able to describe the exchange and the routing key for the producer/consumer.
  2. See that information in the UI nicely.
  3. Be able to send the test message from the UI.

As far as the new interface/API changes, would be nice to have annotations like AmqpProducer/AmqpConsumer to put on the methods for example and be able to add channelName, exchangeName, routingKey, description, payloadType there.

** at the moment publishing a message via UI to a queue works, because Rabbit's queues are automatically binded to the default exchange with their name as a routing key. So even if a queue 'book-added' is binded to an exchange 'books' with a routing key 'added', when you publish a message with RabbitTemplate.send('book-added', message) it's routed to the queue anyway. Which makes it seem as the message is sent directly to the queue, but it's not the case, as the message are published to an exchange only.

@stavshamir
Copy link
Member

stavshamir commented May 27, 2022

Thanks for the explanation, I think I understand it better now,

  1. Be able to describe the exchange and the routing key for the producer/consumer.
    It seems that RabbitListener already supports providing the exchange and routing key:
    @RabbitListener(bindings = {
            @QueueBinding(
                    exchange = @Exchange(name = "name", type = ExchangeTypes.TOPIC),
                    value = @Queue(name = "example-bindings-queue"),
                    key = "routing-key"
            )
    })
    public void bindingsExample(AnotherPayloadDto payload) {
        logger.info("Received new message in example-bindings-queue: {}", payload.toString());
    }

The RabbitListener scanner only uses the name of the queue at the moment, but it can easily map the exchange and key from the annotation to the channel object. Would that provide a solution regarding the consumers?

For the producers, we can for the moment ask the user to manually build the bindings object, or provide methods to construct it more conveniently.

  1. See that information in the UI nicely.

The bindings data will be displayed in the binding tab as json. I didn't have plans to make it prettier, but I'm always open to ideas.

  1. Be able to send the test message from the UI.

That's the trickier part. If I understand correctly, we would need to be able to use the binding information to route the message to the correct template, which also needs to find its way to the controller somehow. I will try and figure it out when I have some spare time, but again, ideas are welcome.

Anyway, we can start with (1) and move forward step by step. Would you like to work on it?

@DmitriButorchin
Copy link
Contributor Author

  1. Most of the time there won't be any other information in the RabbitListener besides the queue name, as the queue, exchange and binding to the exchange via specific routing key would already exist. It can be used if it's present, but I think there should be a solution for when there's only the queue name there.

  2. As mentioned, there are two types of binding, operation binding and channel binding. At the moment only operation binding is present and displayed. But the exchange information is tied to the channel binding. After fixing [1] at least both should be displayed. I think when the binding type is 'AMQP' it makes sense to display the exchange name and routing key alongside the channel name, but at least having a tab that has that info would be good.

  3. This seems straight-forward, we just take the ChannelItem from the ChannelService and send to message to exchange/routing key, no?

I think I'll have some time over the weekend, so I'd be happy to work on the first point.

@stavshamir
Copy link
Member

  1. Then how would you suggest to get the exchange an routing key? Another annotation on the consumer method? There must be a way to get this info automatically from the context and configurations - if spring does it then we can as well. It might require some digging into their code, but it is must likely doable.
  2. I agree that it would make sense to display the exchange and routing key for AMQP channels, we should do it.
  3. I guess so, we need to try.

Let me know if you need anything when working on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants