SpringBoot Add support for naming Spring AMQP container threads

Requires https://github.com/spring-projects/spring-amqp/issues/2479.

Comment From: artembilan

Hi Andy!

Would you mind helping me to determine what we are pursuing with this? Looks like the current code in Spring Boot is like this:

    @Bean(name = "simpleRabbitListenerContainerFactoryConfigurer")
    @ConditionalOnMissingBean
    @ConditionalOnThreading(Threading.VIRTUAL)
    SimpleRabbitListenerContainerFactoryConfigurer simpleRabbitListenerContainerFactoryConfigurerVirtualThreads() {
        SimpleRabbitListenerContainerFactoryConfigurer configurer = simpleListenerConfigurer();
        configurer.setTaskExecutor(new VirtualThreadTaskExecutor());
        return configurer;
    }

So, the VirtualThreadTaskExecutor with its defaults is set into the target container. Why do we need to do anything in Spring AMQP, when those virtual threads simply can be named via this VirtualThreadTaskExecutor? Right, we rely at the moment on defaults, but we do exactly the same using SimpleAsyncTaskExecutor at the moment in the AbstractMessageListenerContainer.

So, why do we need to add some high-level custom API into every project, when that VirtualThreadTaskExecutor can be customized respectively?

Thanks

Comment From: wilkinsona

I think the naming convention that's used for threads belongs in the project that's using those threads for a specific purpose. Such a convention for Kafka is described in https://github.com/spring-projects/spring-kafka/issues/2501. Imagining that AMQP would have a similar convention, it should be defined by Spring AMQP and not by Spring Boot.

Comment From: artembilan

Sorry, Andy, I probably was not very clear. With the VirtualThreadTaskExecutor we can provide a desired customization to thread naming:

    @Bean
    SimpleRabbitListenerContainerFactoryConfigurer simpleRabbitListenerContainerFactoryConfigurer(
            RabbitProperties rabbitProperties) {

        SimpleRabbitListenerContainerFactoryConfigurer simpleRabbitListenerContainerFactoryConfigurer =
                new SimpleRabbitListenerContainerFactoryConfigurer(rabbitProperties);
        simpleRabbitListenerContainerFactoryConfigurer.setTaskExecutor(new VirtualThreadTaskExecutor("my-virtual-threads-"));
        return simpleRabbitListenerContainerFactoryConfigurer;
    }

and the result is like this:

2024-01-17T10:50:15.172-05:00  INFO 26372 --- [rtual-threads-0] .s.v.SpringAmqpVirtualThreadsApplication : Handle data: test data #2
2024-01-17T10:50:15.172-05:00  INFO 26372 --- [rtual-threads-1] .s.v.SpringAmqpVirtualThreadsApplication : Handle data: test data #1

This is exactly how we would customize thread naming for regular SimpleAsyncTaskExecutor. And if I rely on the Spring Boot defaults spring.threads.virtual.enabled=true, I really would not get any names in those threads:

2024-01-17T10:52:25.406-05:00  INFO 30992 --- [               ] .s.v.SpringAmqpVirtualThreadsApplication : Handle data: test data #1
2024-01-17T10:52:25.406-05:00  INFO 30992 --- [               ] .s.v.SpringAmqpVirtualThreadsApplication : Handle data: test data #2

Therefore my question is more about making that configurer.setTaskExecutor(new VirtualThreadTaskExecutor()); in the RabbitAnnotationDrivenConfiguration with more meaningful default prefix. And further expose respective property in RabbitProperties.

The point is that there is nothing to do on Spring AMQP side since with its API you simply can inject a respective VirtualThreadTaskExecutor. With Spring Boot we also can do that via mentioned configurer, but it might be more natural to expose a configuration property and wash our hands.

I believe the work in Spring Kafka was done before the VirtualThreadTaskExecutor was introduced in Spring Framework.

Thanks

Comment From: wilkinsona

I think I understood what you were suggesting, I just don't think Boot should really be involved. IMO, the thread names should be set by Spring AMQP as it understands exactly how they are going to be used. This means that it's best-placed to name them similar to what's being done in Spring Kafka. With hindsight, I'm not sure we need a Spring Boot issue for this at all as it could be handled completely within Spring AMQP. We could reconsider in the future if there's need for the thread naming scheme to be configurable but until we have seen some demand for that, I'm tempted to close this one.

Comment From: artembilan

OK. Thanks for confirmation. Since we have already a hook to set a custom TaskExecutor with desired thread name prefix, I don't see a reason in extra configuration complexity. That way end-user would be ever confused having the choice of both those properties.

Therefore I'm closing that issue on Spring AMQP side. Feel free to close this one and then https://github.com/spring-projects/spring-boot/issues/36345, respectively.

Comment From: wilkinsona

I've closed the Boot issues. FWIW, I think it would be a mistake to close the AMQP issue as well as I think it should provide a sensible default for its thread names without Boot having to be involved. That way the functionality is available to everyone using Spring AMQP, not just those using it through Boot.