spring-cloud-aws icon indicating copy to clipboard operation
spring-cloud-aws copied to clipboard

Improve Documentation on Mixed use of AcknowledgementMode and MessageListenerContainerFactory

Open iparadiso opened this issue 1 year ago • 6 comments

Type: Bug

Component:

Describe the bug The new addition of acknowledgementMode in @SqsListener is great, but it fails on startup if there are any usage of MessageListenerContainerFactory beans in the app. For example, I may be listening to two different queues. One using MessageListenerContainerFactory via the factory annotation param, and another queue using acknowledgementMode annotation param.

The failure exception is

java.lang.IllegalArgumentException: No MessageListenerContainerFactory bean with name defaultSqsListenerContainerFactory found for endpoint names [test-queue] at org.springframework.util.Assert.isTrue(Assert.java:129) at io.awspring.cloud.sqs.config.EndpointRegistrar.createContainerFor(EndpointRegistrar.java:223) at io.awspring.cloud.sqs.config.EndpointRegistrar.process(EndpointRegistrar.java:218) at java.base/java.util.ArrayList.forEach(ArrayList.java:1511) at io.awspring.cloud.sqs.config.EndpointRegistrar.afterSingletonsInstantiated(EndpointRegistrar.java:213) at io.awspring.cloud.sqs.annotation.AbstractListenerAnnotationBeanPostProcessor.afterSingletonsInstantiated(AbstractListenerAnnotationBeanPostProcessor.java:256) at org.springframework.beans.factory.support.DefaultListableBeanFactory.preInstantiateSingletons(DefaultListableBeanFactory.java:984) at org.springframework.context.support.AbstractApplicationContext.finishBeanFactoryInitialization(AbstractApplicationContext.java:946) at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:616) at org.springframework.boot.web.servlet.context.ServletWebServerApplicationContext.refresh(ServletWebServerApplicationContext.java:146) at org.springframework.boot.SpringApplication.refresh(SpringApplication.java:753) at org.springframework.boot.SpringApplication.refreshContext(SpringApplication.java:455) at org.springframework.boot.SpringApplication.run(SpringApplication.java:323) at org.springframework.boot.SpringApplication.run(SpringApplication.java:1342) at org.springframework.boot.SpringApplication.run(SpringApplication.java:1331)

This is because SqsAutoConfiguration has a @CondituionalOnMissingBean, so it is never created if the same bean type exists in the app.

	@ConditionalOnMissingBean
	@Bean
	public SqsMessageListenerContainerFactory<Object> defaultSqsListenerContainerFactory(...)

Perhaps this can be conditional on a named bean instead to allow both to exist in an app?

Sample The following example can reproduce the issue.

@Configuration
public class MyAppConfig {
    @ConditionalOnMissingBean(name = "ACK_ON_SUCCESS_FACTORY_BEAN_NAME")
    @Bean(name = "ACK_ON_SUCCESS_FACTORY_BEAN_NAME")
    SqsMessageListenerContainerFactory<Object> acknowledgeOnSuccessFactory(SqsAsyncClient sqsAsyncClient) {
        return SqsMessageListenerContainerFactory.builder()
                .configure(options -> options.acknowledgementMode(AcknowledgementMode.ALWAYS))
                .sqsAsyncClient(sqsAsyncClient)
                .build();
    }
}

@Component 
public class SqsConsumers {
    @SqsListener(value = "test-queue-2", factory = "ACK_ON_SUCCESS_FACTORY_BEAN_NAME")
    public void consumeSqsQueue2(String message) {

    }
    
     @SqsListener(value = "test-queue", acknowledgementMode = SqsListenerAcknowledgementMode.ON_SUCCESS)
    public void consumeSqsQueue(String message) {
    }
}

iparadiso avatar Dec 01 '23 18:12 iparadiso

If I understand the issue correctly, this is just how Spring AutoConfiguration works, and has nothing to do with acknowledgement mode itself.

If you declare your own SqsMessageListenerContainerFactory bean, auto configuration will back off and no defaultSqsListenerContainerFactory will be created, which means you have to either create a bean with this name yourself, or explicitly give a factory name to the annotation.

This is pretty much standard Spring Boot behavior and doesn't look like something we should change.

Am I missing something?

Let me know if it makes sense for you.

tomazfernandes avatar Dec 03 '23 19:12 tomazfernandes

Yes, this is basic spring autoconfiguration behavior. The problem is you're expecting a specific bean name to exist in the bean factory. This bean name is expected to be defaultSqsListenerContainerFactory as derived by the bean method name.

If a user wants to define their own message listener container factory for custom behavior for one queue, and use the default ack mode for another queue in the same app, then cannot be done without supplying a bean name for the one being suppressed by autoconfiguration.

Since you're expecting a specific bean name, the most extensible way to solve this is to change the SqsAutoConfiguration to be conditional on a named bean.

	@ConditionalOnMissingBean(name = "defaultSqsListenerContainerFactory")
	@Bean(name = "defaultSqsListenerContainerFactory")
	public SqsMessageListenerContainerFactory<Object> defaultSqsListenerContainerFactory(...)

Doing this allows users to configure both their own factories and use your OSS annotation acknowledgement mode knobs.

Either way, it's a non-intuitive exception for users to trace down, and not well documented whether both approaches can be used in an app.

iparadiso avatar Dec 04 '23 19:12 iparadiso

Yeah, I think this is more of a documentation issue.

This is the expected behavior of auto configuration - if you declare your own SqsMessageListenerContainerFactory bean it'll backoff and you can use your own, and just add the name you choose to the appropriate annotations. No need to know any bean name.

You can set the bean name that will be looked up by declaring a SqsListenerConfigurer bean, and using the setDefaultListenerContainerFactoryBeanName method in the registrar (which is missing from the documentation BTW).

Now, you can use any combination of factory bean and annotation property for acknowledgement. Basically what the annotation property does is override whatever value was set in the factory used by that annotation.

So there you go, you have full flexibility to set it up any way you want. But I remember having lost some time figuring this out the first few times I used Spring Kafka which uses the same strategy.

Does this answer all your questions?

If so, how do you propose we document it so that we can make it clear for users? Would you like to open a PR for that?

Thanks.

tomazfernandes avatar Dec 06 '23 02:12 tomazfernandes

Thanks @tomazfernandes I think this helps clear things up! It wasn't clear to me that we need to register that factory bean name with the SqsListenerConfigurer when using both techniques.

I'll open a PR to help document this.

iparadiso avatar Dec 06 '23 23:12 iparadiso

Submitted https://github.com/awspring/spring-cloud-aws/pull/979 for consideration.

iparadiso avatar Dec 06 '23 23:12 iparadiso

Is it tied with the acknowledgementMode param? Seems like it would still happen regardless of having the ack mode (or any other param) on the annotation.

maybe it is worth updating the Specifying a MessageListenerContainerFactory section of the docs as well

jvcalassio avatar Dec 09 '23 23:12 jvcalassio