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

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

Closed
OathMeadow opened this issue Feb 24, 2024 · 10 comments
Closed

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

OathMeadow opened this issue Feb 24, 2024 · 10 comments
Assignees
Labels
status: superseded An issue that has been superseded by another

Comments

@OathMeadow
Copy link

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

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Feb 24, 2024
@mhalbritter
Copy link
Contributor

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.

@mhalbritter mhalbritter changed the title Add thread name prefix to Rabbit Listener when using VirtualThreads Add thread name prefix in all cases when using virtual threads Feb 27, 2024
@mhalbritter mhalbritter removed the status: waiting-for-triage An issue we've not yet triaged label Feb 27, 2024
@mhalbritter mhalbritter changed the title Add thread name prefix in all cases when using virtual threads Thread name prefix is not always set when using virtual threads Feb 27, 2024
@mhalbritter mhalbritter added the type: bug A general bug label Feb 27, 2024
@mhalbritter mhalbritter added this to the 3.2.x milestone Feb 27, 2024
@MazizEsa
Copy link

MazizEsa commented Mar 1, 2024

Can I help out with this @mhalbritter ?

@mhalbritter
Copy link
Contributor

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
Copy link

MazizEsa commented Mar 4, 2024

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

@mhalbritter
Copy link
Contributor

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
Copy link

MazizEsa commented Mar 4, 2024

Thanks. :)

@somayaj
Copy link
Contributor

somayaj commented Mar 5, 2024

Hello, is this open to work on?

@wilkinsona
Copy link
Member

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

@MazizEsa MazizEsa removed their assignment Mar 11, 2024
@MazizEsa
Copy link

MazizEsa commented Mar 11, 2024

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

@mhalbritter
Copy link
Contributor

Superseded by #39958.

@mhalbritter mhalbritter closed this as not planned Won't fix, can't repro, duplicate, stale Mar 18, 2024
@mhalbritter mhalbritter removed this from the 3.2.x milestone Mar 18, 2024
@mhalbritter mhalbritter added status: superseded An issue that has been superseded by another and removed type: bug A general bug labels Mar 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: superseded An issue that has been superseded by another
Projects
None yet
Development

No branches or pull requests

6 participants