RabbitMQ - RetryTemplate customization hook

[garyrussell]

Provide a mechanism so that users can customize RetryTemplates
used in RabbitTemplate and listeners.

An example would be to add a retry listener so the user can monitor/log
retry attempts.

https://stackoverflow.com/questions/48331502/logging-exceptions-thrown-by-message-listeners-for-spring-amqp/48332001#48332001

Or, to set a custom RetryContextCache.

temp

[snicoll]

@garyrussell I am not super keen to expand the surface area of Spring Boot for such specific requirement. I am guessing there are other areas where we want to customize things and I'd very much prefer if the API would allow us to do this with a broader scope (i.e. a single callback to tune everything RabbitTemplate related for instance).

For the listener we can have something on AbstractRabbitListenerContainerFactoryConfigurer where we can "consume" a RetryTemplate. This would allow for customizations without creating a separate contract. This is a different approach so I've started a separate issue to discuss this.

As for the retry template for the RabbitTemplate, I wished there was a way to customize the template rather than this individual piece. Perhaps we should start looking at RetryTemplate with a very specifc name? I am not a huge fan of that approach either but having two customizer interfaces with a base class isn't great either IMO.

[snicoll]

It looks like this PR offers a way to customize the retry template and to set it at all. To me, these are two separate features that deserve a separate commit. I've just done that in #13529 (calling the auto-config that way isn't something we do, see the commit for more details).

[snicoll]

I've started a separate discussion (#13530) but I wonder if a Consumer<RetryTemplate> wouldn't be enough here.

[snicoll]

I am not sure I understand the purpose of that test. We're creating a messaging template to show that it's taken into account. Why is retry processing added there?

[garyrussell]

I am testing that the customizer injected a listener into the listener container's retry template; in order to test that, we need to enable retry.

[snicoll]

I got that. But why are you testing that as part of testRabbitMessagingTemplateBackOff ?

[garyrussell]

I concur with the general comment about using Consumer<?> - the problem here is there are two retry templates (for consumers and publishers).

Perhaps another alternative would be for the RabbitTemplate to add a getter for its retry operations; then would have Consumer<RabbitTemplate> and the user could then use template.getRetryOperations().registerListener(...).

And for the listener, we could have Consumer<RetryTemplate> (it's not so easy to add a getter on the listener containers since the retry template is injected into an interceptor and added to the container's generic advice chain.

[snicoll]

Perhaps another alternative would be for the RabbitTemplate to add a getter for its retry operations

I thought of that but I think we should do that only if that makes sense generally speaking and not to ease that particular case. I don't know if exposing the retryTemplate is a good idea (probably not).

My proposal of using Consumer was declined so we're back to the two functional interfaces. Another idea would be to find out the cases that require to customize the RetryTemplate. Perhaps we can have some sort of explicit support?

Or perhaps having two interfaces for this is fine? It just doesn't seem right to me but I can't put my fingers on why exactly. Flagging for team attention to get more feedback.

[snicoll]

We've discussed this at the call and we're now leaning towards a unique RabbitRetryTemplateCustomizer that takes the RetryTemplate and the "source" (or the target). We don't want to expose a generic customizer and we are not keen on having two interfaces for this either. I'll try to spare some cycle to hack on this idea.

@garyrussell if you want to give it a try, please let me know.

[garyrussell]

@snicoll Sorry; mis-read; it's probably best for you to hack at it, to avoid a bunch of round trips 😄

[snicoll]

@garyrussell alright I hacked something and I've opened #13793 to track it. Thanks for the PR and the help on this issue.