From 3c3b9a5a14d7bdaf8d519cb6eb6d691eb5a60691 Mon Sep 17 00:00:00 2001 From: Olga Maciaszek-Sharma Date: Fri, 2 Oct 2020 10:15:20 -0500 Subject: [PATCH] Add support for passing hints to LoadBalancer. (#412) --- ...DefaultFeignLoadBalancerConfiguration.java | 11 ++++++---- .../FeignBlockingLoadBalancerClient.java | 20 +++++++++++++++++-- ...pClientFeignLoadBalancerConfiguration.java | 13 ++++++++---- .../OkHttpFeignLoadBalancerConfiguration.java | 13 ++++++++---- ...ryableFeignBlockingLoadBalancerClient.java | 15 ++++++++++++-- .../FeignBlockingLoadBalancerClientTests.java | 8 +++++--- ...eFeignBlockingLoadBalancerClientTests.java | 2 +- 7 files changed, 62 insertions(+), 20 deletions(-) diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/DefaultFeignLoadBalancerConfiguration.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/DefaultFeignLoadBalancerConfiguration.java index 8401c46c..98fa40c8 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/DefaultFeignLoadBalancerConfiguration.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/DefaultFeignLoadBalancerConfiguration.java @@ -22,8 +22,10 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancerClient; +import org.springframework.cloud.client.loadbalancer.reactive.LoadBalancerProperties; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Conditional; import org.springframework.context.annotation.Configuration; @@ -36,13 +38,14 @@ import org.springframework.context.annotation.Configuration; * @since 2.2.0 */ @Configuration(proxyBeanMethods = false) +@EnableConfigurationProperties(LoadBalancerProperties.class) class DefaultFeignLoadBalancerConfiguration { @Bean @ConditionalOnMissingBean @Conditional(OnRetryNotEnabledCondition.class) - public Client feignClient(LoadBalancerClient loadBalancerClient) { - return new FeignBlockingLoadBalancerClient(new Client.Default(null, null), loadBalancerClient); + public Client feignClient(LoadBalancerClient loadBalancerClient, LoadBalancerProperties properties) { + return new FeignBlockingLoadBalancerClient(new Client.Default(null, null), loadBalancerClient, properties); } @Bean @@ -52,9 +55,9 @@ class DefaultFeignLoadBalancerConfiguration { @ConditionalOnProperty(value = "spring.cloud.loadbalancer.retry.enabled", havingValue = "true", matchIfMissing = true) public Client feignRetryClient(LoadBalancerClient loadBalancerClient, - LoadBalancedRetryFactory loadBalancedRetryFactory) { + LoadBalancedRetryFactory loadBalancedRetryFactory, LoadBalancerProperties properties) { return new RetryableFeignBlockingLoadBalancerClient(new Client.Default(null, null), loadBalancerClient, - loadBalancedRetryFactory); + loadBalancedRetryFactory, properties); } } diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/FeignBlockingLoadBalancerClient.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/FeignBlockingLoadBalancerClient.java index 4b1bd714..0e9c5bbb 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/FeignBlockingLoadBalancerClient.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/FeignBlockingLoadBalancerClient.java @@ -27,7 +27,10 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.client.loadbalancer.DefaultRequest; +import org.springframework.cloud.client.loadbalancer.DefaultRequestContext; import org.springframework.cloud.client.loadbalancer.LoadBalancerClient; +import org.springframework.cloud.client.loadbalancer.reactive.LoadBalancerProperties; import org.springframework.http.HttpStatus; import org.springframework.util.Assert; @@ -46,9 +49,13 @@ public class FeignBlockingLoadBalancerClient implements Client { private final LoadBalancerClient loadBalancerClient; - public FeignBlockingLoadBalancerClient(Client delegate, LoadBalancerClient loadBalancerClient) { + private final LoadBalancerProperties properties; + + public FeignBlockingLoadBalancerClient(Client delegate, LoadBalancerClient loadBalancerClient, + LoadBalancerProperties properties) { this.delegate = delegate; this.loadBalancerClient = loadBalancerClient; + this.properties = properties; } @Override @@ -56,7 +63,10 @@ public class FeignBlockingLoadBalancerClient implements Client { final URI originalUri = URI.create(request.url()); String serviceId = originalUri.getHost(); Assert.state(serviceId != null, "Request URI does not contain a valid hostname: " + originalUri); - ServiceInstance instance = loadBalancerClient.choose(serviceId); + String hint = getHint(serviceId); + DefaultRequest lbRequest = new DefaultRequest<>( + new DefaultRequestContext(request, hint)); + ServiceInstance instance = loadBalancerClient.choose(serviceId, lbRequest); if (instance == null) { String message = "Load balancer does not contain an instance for the service " + serviceId; if (LOG.isWarnEnabled()) { @@ -76,4 +86,10 @@ public class FeignBlockingLoadBalancerClient implements Client { return delegate; } + private String getHint(String serviceId) { + String defaultHint = properties.getHint().getOrDefault("default", "default"); + String hintPropertyValue = properties.getHint().get(serviceId); + return hintPropertyValue != null ? hintPropertyValue : defaultHint; + } + } diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/HttpClientFeignLoadBalancerConfiguration.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/HttpClientFeignLoadBalancerConfiguration.java index 52084ab3..4ab677e2 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/HttpClientFeignLoadBalancerConfiguration.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/HttpClientFeignLoadBalancerConfiguration.java @@ -24,8 +24,10 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancerClient; +import org.springframework.cloud.client.loadbalancer.reactive.LoadBalancerProperties; import org.springframework.cloud.openfeign.clientconfig.HttpClientFeignConfiguration; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Conditional; @@ -44,14 +46,16 @@ import org.springframework.context.annotation.Import; @ConditionalOnBean(LoadBalancerClient.class) @ConditionalOnProperty(value = "feign.httpclient.enabled", matchIfMissing = true) @Import(HttpClientFeignConfiguration.class) +@EnableConfigurationProperties(LoadBalancerProperties.class) class HttpClientFeignLoadBalancerConfiguration { @Bean @ConditionalOnMissingBean @Conditional(OnRetryNotEnabledCondition.class) - public Client feignClient(LoadBalancerClient loadBalancerClient, HttpClient httpClient) { + public Client feignClient(LoadBalancerClient loadBalancerClient, HttpClient httpClient, + LoadBalancerProperties properties) { ApacheHttpClient delegate = new ApacheHttpClient(httpClient); - return new FeignBlockingLoadBalancerClient(delegate, loadBalancerClient); + return new FeignBlockingLoadBalancerClient(delegate, loadBalancerClient, properties); } @Bean @@ -61,9 +65,10 @@ class HttpClientFeignLoadBalancerConfiguration { @ConditionalOnProperty(value = "spring.cloud.loadbalancer.retry.enabled", havingValue = "true", matchIfMissing = true) public Client feignRetryClient(LoadBalancerClient loadBalancerClient, HttpClient httpClient, - LoadBalancedRetryFactory loadBalancedRetryFactory) { + LoadBalancedRetryFactory loadBalancedRetryFactory, LoadBalancerProperties properties) { ApacheHttpClient delegate = new ApacheHttpClient(httpClient); - return new RetryableFeignBlockingLoadBalancerClient(delegate, loadBalancerClient, loadBalancedRetryFactory); + return new RetryableFeignBlockingLoadBalancerClient(delegate, loadBalancerClient, loadBalancedRetryFactory, + properties); } } diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/OkHttpFeignLoadBalancerConfiguration.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/OkHttpFeignLoadBalancerConfiguration.java index 0ead2e9b..224901d9 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/OkHttpFeignLoadBalancerConfiguration.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/OkHttpFeignLoadBalancerConfiguration.java @@ -23,8 +23,10 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancerClient; +import org.springframework.cloud.client.loadbalancer.reactive.LoadBalancerProperties; import org.springframework.cloud.openfeign.clientconfig.OkHttpFeignConfiguration; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Conditional; @@ -43,14 +45,16 @@ import org.springframework.context.annotation.Import; @ConditionalOnProperty("feign.okhttp.enabled") @ConditionalOnBean(LoadBalancerClient.class) @Import(OkHttpFeignConfiguration.class) +@EnableConfigurationProperties(LoadBalancerProperties.class) class OkHttpFeignLoadBalancerConfiguration { @Bean @ConditionalOnMissingBean @Conditional(OnRetryNotEnabledCondition.class) - public Client feignClient(okhttp3.OkHttpClient okHttpClient, LoadBalancerClient loadBalancerClient) { + public Client feignClient(okhttp3.OkHttpClient okHttpClient, LoadBalancerClient loadBalancerClient, + LoadBalancerProperties properties) { OkHttpClient delegate = new OkHttpClient(okHttpClient); - return new FeignBlockingLoadBalancerClient(delegate, loadBalancerClient); + return new FeignBlockingLoadBalancerClient(delegate, loadBalancerClient, properties); } @Bean @@ -60,9 +64,10 @@ class OkHttpFeignLoadBalancerConfiguration { @ConditionalOnProperty(value = "spring.cloud.loadbalancer.retry.enabled", havingValue = "true", matchIfMissing = true) public Client feignRetryClient(LoadBalancerClient loadBalancerClient, okhttp3.OkHttpClient okHttpClient, - LoadBalancedRetryFactory loadBalancedRetryFactory) { + LoadBalancedRetryFactory loadBalancedRetryFactory, LoadBalancerProperties properties) { OkHttpClient delegate = new OkHttpClient(okHttpClient); - return new RetryableFeignBlockingLoadBalancerClient(delegate, loadBalancerClient, loadBalancedRetryFactory); + return new RetryableFeignBlockingLoadBalancerClient(delegate, loadBalancerClient, loadBalancedRetryFactory, + properties); } } diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/RetryableFeignBlockingLoadBalancerClient.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/RetryableFeignBlockingLoadBalancerClient.java index d844d361..530ca65f 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/RetryableFeignBlockingLoadBalancerClient.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/RetryableFeignBlockingLoadBalancerClient.java @@ -40,6 +40,7 @@ import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicy; import org.springframework.cloud.client.loadbalancer.LoadBalancerClient; import org.springframework.cloud.client.loadbalancer.RetryableRequestContext; import org.springframework.cloud.client.loadbalancer.RetryableStatusCodeException; +import org.springframework.cloud.client.loadbalancer.reactive.LoadBalancerProperties; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; import org.springframework.http.HttpRequest; @@ -67,11 +68,14 @@ public class RetryableFeignBlockingLoadBalancerClient implements Client { private final LoadBalancedRetryFactory loadBalancedRetryFactory; + private final LoadBalancerProperties properties; + public RetryableFeignBlockingLoadBalancerClient(Client delegate, LoadBalancerClient loadBalancerClient, - LoadBalancedRetryFactory loadBalancedRetryFactory) { + LoadBalancedRetryFactory loadBalancedRetryFactory, LoadBalancerProperties properties) { this.delegate = delegate; this.loadBalancerClient = loadBalancerClient; this.loadBalancedRetryFactory = loadBalancedRetryFactory; + this.properties = properties; } @Override @@ -95,8 +99,9 @@ public class RetryableFeignBlockingLoadBalancerClient implements Client { + "Reattempting service instance selection"); } ServiceInstance previousServiceInstance = lbContext.getPreviousServiceInstance(); + String hint = getHint(serviceId); DefaultRequest lbRequest = new DefaultRequest<>( - new RetryableRequestContext(previousServiceInstance, request)); + new RetryableRequestContext(previousServiceInstance, request, hint)); serviceInstance = loadBalancerClient.choose(serviceId, lbRequest); if (LOG.isDebugEnabled()) { LOG.debug(String.format("Selected service instance: %s", serviceInstance)); @@ -188,4 +193,10 @@ public class RetryableFeignBlockingLoadBalancerClient implements Client { }; } + private String getHint(String serviceId) { + String defaultHint = properties.getHint().getOrDefault("default", "default"); + String hintPropertyValue = properties.getHint().get(serviceId); + return hintPropertyValue != null ? hintPropertyValue : defaultHint; + } + } diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/loadbalancer/FeignBlockingLoadBalancerClientTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/loadbalancer/FeignBlockingLoadBalancerClientTests.java index 553e4a4b..b3cabd7b 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/loadbalancer/FeignBlockingLoadBalancerClientTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/loadbalancer/FeignBlockingLoadBalancerClientTests.java @@ -34,6 +34,7 @@ import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.cloud.client.DefaultServiceInstance; import org.springframework.cloud.client.ServiceInstance; +import org.springframework.cloud.client.loadbalancer.reactive.LoadBalancerProperties; import org.springframework.cloud.loadbalancer.blocking.client.BlockingLoadBalancerClient; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; @@ -41,6 +42,7 @@ import org.springframework.http.MediaType; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalStateException; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; @@ -65,7 +67,7 @@ class FeignBlockingLoadBalancerClientTests { private BlockingLoadBalancerClient loadBalancerClient = mock(BlockingLoadBalancerClient.class); private FeignBlockingLoadBalancerClient feignBlockingLoadBalancerClient = new FeignBlockingLoadBalancerClient( - delegate, loadBalancerClient); + delegate, loadBalancerClient, new LoadBalancerProperties()); @Test void shouldExtractServiceIdFromRequestUrl() throws IOException { @@ -73,7 +75,7 @@ class FeignBlockingLoadBalancerClientTests { feignBlockingLoadBalancerClient.execute(request, new Request.Options()); - verify(loadBalancerClient).choose("test"); + verify(loadBalancerClient).choose(eq("test"), any()); } @Test @@ -102,7 +104,7 @@ class FeignBlockingLoadBalancerClientTests { Request.Options options = new Request.Options(); String url = "http://127.0.0.1/path"; ServiceInstance serviceInstance = new DefaultServiceInstance("test-1", "test", "test-host", 8888, false); - when(loadBalancerClient.choose("test")).thenReturn(serviceInstance); + when(loadBalancerClient.choose(eq("test"), any())).thenReturn(serviceInstance); when(loadBalancerClient.reconstructURI(serviceInstance, URI.create("http://test/path"))) .thenReturn(URI.create(url)); diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/loadbalancer/RetryableFeignBlockingLoadBalancerClientTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/loadbalancer/RetryableFeignBlockingLoadBalancerClientTests.java index 12d539af..4c27fded 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/loadbalancer/RetryableFeignBlockingLoadBalancerClientTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/loadbalancer/RetryableFeignBlockingLoadBalancerClientTests.java @@ -73,7 +73,7 @@ class RetryableFeignBlockingLoadBalancerClientTests { private LoadBalancerProperties properties = new LoadBalancerProperties(); private RetryableFeignBlockingLoadBalancerClient feignBlockingLoadBalancerClient = new RetryableFeignBlockingLoadBalancerClient( - delegate, loadBalancerClient, retryFactory); + delegate, loadBalancerClient, retryFactory, properties); private ServiceInstance serviceInstance = new DefaultServiceInstance("test-a", "test", "testhost", 80, false);