spring-projects / spring-boot

Spring Boot helps you to create Spring-powered, production-grade applications and services with absolute minimum fuss.
https://spring.io/projects/spring-boot
Apache License 2.0
75.19k stars 40.69k forks source link

Thread name prefix is not always set when using virtual threads #39748

Closed OathMeadow closed 8 months ago

OathMeadow commented 8 months ago

We noticed that the thread name was missing as a field in our logs logged by the rabbit listener after activating VirtualThreads. We use Loki in Grafana where we have multiple fields, where the thread name is one of them. To be able to see which thread logs what is useful to identify issues.

We noticed that in the class org.springframework.boot.autoconfigure.amqp.RabbitAnnotationDrivenConfiguration, Spring Boot does not set a thread prefix to the VirtualThreadTaskExecutor class when VirtualThreads is configured. In fact, when not set, it defaults to null. Perhaps that is by design, but It seems Spring Boot sets a thread name prefix in some AutoConfigurations and some not.

May I suggest names something like:

    @Bean(name = "directRabbitListenerContainerFactoryConfigurer")
    @ConditionalOnMissingBean
    @ConditionalOnThreading(Threading.VIRTUAL)
    DirectRabbitListenerContainerFactoryConfigurer directRabbitListenerContainerFactoryConfigurerVirtualThreads() {
        DirectRabbitListenerContainerFactoryConfigurer configurer = directListenerConfigurer();
        configurer.setTaskExecutor(new VirtualThreadTaskExecutor("direct-rabbit-listener-"));
        return configurer;
    }

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

Thank you

mhalbritter commented 8 months ago

We set the names for Redis, Kafka and Undertow. We don't set them for Rabbit and for Pulsar. We should make that consistent. IMHO we should name threads in all cases.

MazizEsa commented 8 months ago

Can I help out with this @mhalbritter ?

mhalbritter commented 8 months ago

Sure. I'll assign the issue to you. This is targeted for the 3.2.x branch, so please create your PR based on that. If you need help, please chime back in.

MazizEsa commented 8 months ago

@mhalbritter . When is the 3.2.x going to be released. I will do this outside of my working time.

mhalbritter commented 8 months ago

Don't worry about timelines. We release 3.2.x every month (see https://calendar.spring.io/). Please take your time, it's done when it's done :)

MazizEsa commented 8 months ago

Thanks. :)

somayaj commented 8 months ago

Hello, is this open to work on?

wilkinsona commented 8 months ago

Thanks for the offer, @somayaj, but @MazizEsa is already assigned and working on this issue.

MazizEsa commented 8 months ago

Sorry, can anyone reassign me back. Accidentally click unassign. Sorry again @mhalbritter .

mhalbritter commented 8 months ago

Superseded by #39958.