SpringBoot Auto-configure virtual threads on JDKClientHttpRequestFactory when enabled

I have seen the conversation in spring-projects/spring-framework#34393, but I wanted to re-open the conversation because I disagree with the reasoning for why the issue was closed as won't fix and would like to make a new case to help people avoid the issue I see my teams running into constantly.

To recap, when a developer sets spring.threads.virtual.enabled to true, they have what I think is a reasonable expectation that the Spring framework will do it's best to make choices to configure anything that it can configure to use virtual threading for them. It does this in a lot of cases and, one of those cases, is to configure the default SimpleAsyncTaskExecutor to use virtual threads.

In the case of JDKClientHttpRequestFactory it will detect that no executor has been set on the HttpClient and create a brand new, dedicated SimpleAsyncTaskExecutor, but it doesn't bother consulting the spring.threads.virtual.enabled setting to see if it should be configured to use virtual threads.

IMHO, this level of indirection that JDKClientHttpRequestFactory provides is Spring's chance to "help" meet the expectations of developers and, today, it doesn't.

NOTE: if the developer "brings their own" HttpClient and it happens to have an executor already set, the logic in JDKClientHttpRequestFactory already won't override it, so making changes here should be non-breaking. It's really just about updating the implementation to configure the dedicated SimpleAsyncTaskExecutor with virtual threads when spring.threads.virtual.enabled is true.

Comment From: bclozel

Thanks for reaching out. This cannot be solved on the Spring Framework side for several reasons. RestClient.create() is a static call that does not consider the environment. Also, we cannot make RestClient depend on org.springframework.boot.autoconfigure.thread.Threading as this is a Spring Boot class and this would create a dependency cycle.

I think I should have considered spring-projects/spring-framework#34393 as a Spring Boot issue in the first place. I'm moving this issue to be considered by the Spring Boot team. There, we can consider whether enabling the spring.threads.virtual.enabled should trigger a client request factory change to use virtual threads. This would only be available if you are injecting the auto-configured RestClient.Builder to build your client instance. Would this work for you @drub0y ?

Comment From: drub0y

Totally understand the RestClient.create() case, but I think it's fair to say if you drop down to that level of API you should know you're bypassing all manner of injection/autoconfiguration... obviously wouldn't hurt to note that in documentation of course.

While we do use RestClient.Builder today, we're still the ones having to directly instantiate and configure the HttpClient and wrap it with the JdkHttpClientHttpRequestFactory and assign it via RestClient.Builder's requestExecutor(...) and that's where all the new'ing comes in that obviously bypasses Spring configuration and leads to this unfortunate situation.

If I'm following your proposal, yes it would be helpful if RestClient.Builder had some kind of dedicated useJdkClientRequestExecutor(JdkHttpClientBuilder builder) that properly configured the JDKClientHttpRequestFactory behind the scenes (taking into account the virtual threads property at that point) while also allowing us to configure the specifics of the HttpClient via that JdkHttpClientBuilder (which already exists). In fact, if this was the case then it would mean that you could just inject/use the default SimpleAsyncTaskExecutor which would have already been correctly initialized according to the spring.threads.virtual.enabled property and there'd be no extra work to do other than to link it up correctly.

Finally, let's not forget "full" autoconfiguration where JDK client is auto-selected and the app itself decides it doesn't want to configure the HttpClient directly at all. Obviously that scenario should also be defaulting to configuring the HttpClient with the default SimpleAsyncTaskExecutor for the best possible out-of-the-box experience.

Comment From: bclozel

If I'm following your proposal, yes it would be helpful if RestClient.Builder had some kind of dedicated useJdkClientRequestExecutor(JdkHttpClientBuilder builder) that properly configured the JDKClientHttpRequestFactory behind the scenes (taking into account the virtual threads property at that point) while also allowing us to configure the specifics of the HttpClient via that JdkHttpClientBuilder (which already exists). In fact, if this was the case then it would mean that you could just inject/use the default SimpleAsyncTaskExecutor which would have already been correctly initialized according to the spring.threads.virtual.enabled property and there'd be no extra work to do other than to link it up correctly.

You've lost me. We can't add a method in RestClient.Builder (a Framework class) that takes JdkHttpClientBuilder (a Spring Boot class) as an argument, for the same reason: Framework cannot depend on Spring Boot.

My proposal was mainly that the RestClient.Builder instance contributed as a bean by Spring Boot would have a JdkClientHttpRequestFactory with a virtual threads executor when the configuration option is enabled. You are right, if we do so we must ensure that we wouldn't override an opinion provided about the executor directly in HttpClient.

The use case would mainly look like this:

@Component
class SomeComponent {

  public SomeComponent(RestClient.Builder clientBuilder) {
    // the client builder would use a virtual threads executor
    //  if "spring.threads.virtual.enabled" is true
  }
}

If you need to provide somehow a custom HttpClient instance, I'm not sure we can improve the situation there and I think that creating the request factory yourself is the best choice still.

Comment From: nosan

I believe in 3.5, this can probably be done with ClientHttpRequestFactoryBuilderCustomizer?

    @Bean
    @ConditionalOnBean(name = TaskExecutionAutoConfiguration.APPLICATION_TASK_EXECUTOR_BEAN_NAME)
    @ConditionalOnClass(HttpClient.class)
    @ConditionalOnThreading(Threading.VIRTUAL)
    @Order(0)
    ClientHttpRequestFactoryBuilderCustomizer<JdkClientHttpRequestFactoryBuilder> jdkClientExecutorHttpRequestFactoryBuilderCustomizer(
            @Qualifier(TaskExecutionAutoConfiguration.APPLICATION_TASK_EXECUTOR_BEAN_NAME) ObjectProvider<TaskExecutor> taskExecutor) {
        return (builder) -> builder
            .withHttpClientCustomizer((httpClientBuilder) -> taskExecutor.ifAvailable(httpClientBuilder::executor));
    }

Comment From: mhalbritter

Yes, this would work. However, this would also overwrite any Executor already set on the HttpClient.Builder. AFAIK there's no way to check if the builder already has an executor set.

Comment From: nosan

AFAIK there's no way to check if the builder already has an executor set.

Indeed, there is no way to check if the builder already has an Executor set except by using reflection, but I am not happy with that solution.

However, this would also overwrite any Executor already set on the HttpClient.Builder.

Yes, it would overwrite an Executor if it was set before the Spring Boot customizer. However, in that case, users can declare their own Customizer bean with the @Order annotation and customize the builder with a custom Executor after Boot's one.