From ecfba4b6e9d92072b84750835b1dd1902b7ea958 Mon Sep 17 00:00:00 2001 From: Olga Maciaszek-Sharma Date: Fri, 18 Dec 2020 11:08:01 -0600 Subject: [PATCH] Support LB stats. (#447) --- .../FeignBlockingLoadBalancerClient.java | 8 ++-- .../loadbalancer/LoadBalancerUtils.java | 15 +++--- ...ryableFeignBlockingLoadBalancerClient.java | 23 +++++----- .../FeignBlockingLoadBalancerClientTests.java | 42 +++++++++++------ ...eFeignBlockingLoadBalancerClientTests.java | 46 +++++++++++++------ 5 files changed, 86 insertions(+), 48 deletions(-) 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 cef0ffe5..9c3751d2 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 @@ -93,15 +93,15 @@ public class FeignBlockingLoadBalancerClient implements Client { if (LOG.isWarnEnabled()) { LOG.warn(message); } - supportedLifecycleProcessors - .forEach(lifecycle -> lifecycle.onComplete(new CompletionContext( - CompletionContext.Status.DISCARD, lbResponse))); + supportedLifecycleProcessors.forEach(lifecycle -> lifecycle + .onComplete(new CompletionContext( + CompletionContext.Status.DISCARD, lbRequest, lbResponse))); return Response.builder().request(request).status(HttpStatus.SERVICE_UNAVAILABLE.value()) .body(message, StandardCharsets.UTF_8).build(); } String reconstructedUrl = loadBalancerClient.reconstructURI(instance, originalUri).toString(); Request newRequest = buildRequest(request, reconstructedUrl); - return executeWithLoadBalancerLifecycleProcessing(delegate, options, newRequest, lbResponse, + return executeWithLoadBalancerLifecycleProcessing(delegate, options, newRequest, lbRequest, lbResponse, supportedLifecycleProcessors); } diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/LoadBalancerUtils.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/LoadBalancerUtils.java index 61dd138a..7ecf773c 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/LoadBalancerUtils.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/loadbalancer/LoadBalancerUtils.java @@ -48,21 +48,23 @@ final class LoadBalancerUtils { } static Response executeWithLoadBalancerLifecycleProcessing(Client feignClient, Request.Options options, - Request feignRequest, org.springframework.cloud.client.loadbalancer.Response lbResponse, + Request feignRequest, org.springframework.cloud.client.loadbalancer.Request lbRequest, + org.springframework.cloud.client.loadbalancer.Response lbResponse, Set supportedLifecycleProcessors, boolean loadBalanced) throws IOException { + supportedLifecycleProcessors.forEach(lifecycle -> lifecycle.onStartRequest(lbRequest, lbResponse)); try { Response response = feignClient.execute(feignRequest, options); if (loadBalanced) { supportedLifecycleProcessors.forEach( lifecycle -> lifecycle.onComplete(new CompletionContext<>(CompletionContext.Status.SUCCESS, - lbResponse, buildResponseData(response)))); + lbRequest, lbResponse, buildResponseData(response)))); } return response; } catch (Exception exception) { if (loadBalanced) { - supportedLifecycleProcessors.forEach(lifecycle -> lifecycle - .onComplete(new CompletionContext<>(CompletionContext.Status.FAILED, exception, lbResponse))); + supportedLifecycleProcessors.forEach(lifecycle -> lifecycle.onComplete( + new CompletionContext<>(CompletionContext.Status.FAILED, exception, lbRequest, lbResponse))); } throw exception; } @@ -83,9 +85,10 @@ final class LoadBalancerUtils { } static Response executeWithLoadBalancerLifecycleProcessing(Client feignClient, Request.Options options, - Request feignRequest, org.springframework.cloud.client.loadbalancer.Response lbResponse, + Request feignRequest, org.springframework.cloud.client.loadbalancer.Request lbRequest, + org.springframework.cloud.client.loadbalancer.Response lbResponse, Set supportedLifecycleProcessors) throws IOException { - return executeWithLoadBalancerLifecycleProcessing(feignClient, options, feignRequest, lbResponse, + return executeWithLoadBalancerLifecycleProcessing(feignClient, options, feignRequest, lbRequest, lbResponse, supportedLifecycleProcessors, true); } 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 5951c2d6..0be4ea3d 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 @@ -44,7 +44,6 @@ import org.springframework.cloud.client.loadbalancer.LoadBalancerClient; import org.springframework.cloud.client.loadbalancer.LoadBalancerLifecycle; import org.springframework.cloud.client.loadbalancer.LoadBalancerLifecycleValidator; import org.springframework.cloud.client.loadbalancer.LoadBalancerProperties; -import org.springframework.cloud.client.loadbalancer.RequestData; import org.springframework.cloud.client.loadbalancer.ResponseData; import org.springframework.cloud.client.loadbalancer.RetryableRequestContext; import org.springframework.cloud.client.loadbalancer.RetryableStatusCodeException; @@ -59,7 +58,7 @@ import org.springframework.retry.policy.NeverRetryPolicy; import org.springframework.retry.support.RetryTemplate; import org.springframework.util.Assert; -import static org.springframework.cloud.openfeign.loadbalancer.LoadBalancerUtils.executeWithLoadBalancerLifecycleProcessing; +import static org.springframework.cloud.openfeign.loadbalancer.LoadBalancerUtils.buildRequestData; /** * A {@link Client} implementation that provides Spring Retry support for requests @@ -107,7 +106,10 @@ public class RetryableFeignBlockingLoadBalancerClient implements Client { Set supportedLifecycleProcessors = LoadBalancerLifecycleValidator .getSupportedLifecycleProcessors( loadBalancerClientFactory.getInstances(serviceId, LoadBalancerLifecycle.class), - RetryableRequestContext.class, RequestData.class, ServiceInstance.class); + RetryableRequestContext.class, ResponseData.class, ServiceInstance.class); + String hint = getHint(serviceId); + DefaultRequest lbRequest = new DefaultRequest<>( + new RetryableRequestContext(null, buildRequestData(request), hint)); // On retries the policy will choose the server and set it in the context // and extract the server and update the request being made if (context instanceof LoadBalancedRetryContext) { @@ -119,9 +121,7 @@ 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, hint)); + lbRequest.getContext().setPreviousServiceInstance(previousServiceInstance); supportedLifecycleProcessors.forEach(lifecycle -> lifecycle.onStart(lbRequest)); retrievedServiceInstance = loadBalancerClient.choose(serviceId, lbRequest); if (LOG.isDebugEnabled()) { @@ -136,9 +136,9 @@ public class RetryableFeignBlockingLoadBalancerClient implements Client { } org.springframework.cloud.client.loadbalancer.Response lbResponse = new DefaultResponse( retrievedServiceInstance); - supportedLifecycleProcessors.forEach( - lifecycle -> lifecycle.onComplete(new CompletionContext( - CompletionContext.Status.DISCARD, lbResponse))); + supportedLifecycleProcessors.forEach(lifecycle -> lifecycle + .onComplete(new CompletionContext( + CompletionContext.Status.DISCARD, lbRequest, lbResponse))); feignRequest = request; } else { @@ -153,8 +153,9 @@ public class RetryableFeignBlockingLoadBalancerClient implements Client { } org.springframework.cloud.client.loadbalancer.Response lbResponse = new DefaultResponse( retrievedServiceInstance); - Response response = executeWithLoadBalancerLifecycleProcessing(delegate, options, feignRequest, lbResponse, - supportedLifecycleProcessors, retrievedServiceInstance != null); + Response response = LoadBalancerUtils.executeWithLoadBalancerLifecycleProcessing(delegate, options, + feignRequest, lbRequest, lbResponse, supportedLifecycleProcessors, + retrievedServiceInstance != null); int responseStatus = response.status(); if (retryPolicy != null && retryPolicy.retryableStatusCode(responseStatus)) { if (LOG.isDebugEnabled()) { 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 25d1e216..87659240 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 @@ -37,9 +37,9 @@ import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.cloud.client.DefaultServiceInstance; import org.springframework.cloud.client.ServiceInstance; import org.springframework.cloud.client.loadbalancer.CompletionContext; -import org.springframework.cloud.client.loadbalancer.DefaultRequestContext; import org.springframework.cloud.client.loadbalancer.LoadBalancerLifecycle; import org.springframework.cloud.client.loadbalancer.LoadBalancerProperties; +import org.springframework.cloud.client.loadbalancer.RequestDataContext; import org.springframework.cloud.client.loadbalancer.ResponseData; import org.springframework.cloud.loadbalancer.blocking.client.BlockingLoadBalancerClient; import org.springframework.cloud.loadbalancer.support.LoadBalancerClientFactory; @@ -152,15 +152,18 @@ class FeignBlockingLoadBalancerClientTests { feignBlockingLoadBalancerClient.execute(request, options); - Collection> lifecycleLogRequests = ((TestLoadBalancerLifecycle) loadBalancerLifecycleBeans + Collection> lifecycleLogRequests = ((TestLoadBalancerLifecycle) loadBalancerLifecycleBeans .get("loadBalancerLifecycle")).getStartLog().values(); - Collection> anotherLifecycleLogRequests = ((AnotherLoadBalancerLifecycle) loadBalancerLifecycleBeans + Collection> lifecycleLogStartedRequests = ((TestLoadBalancerLifecycle) loadBalancerLifecycleBeans + .get("loadBalancerLifecycle")).getStartRequestLog().values(); + Collection> anotherLifecycleLogRequests = ((AnotherLoadBalancerLifecycle) loadBalancerLifecycleBeans .get("anotherLoadBalancerLifecycle")).getCompleteLog().values(); - assertThat(lifecycleLogRequests) - .extracting(lbRequest -> ((DefaultRequestContext) lbRequest.getContext()).getHint()) + assertThat(lifecycleLogRequests).extracting(lbRequest -> lbRequest.getContext().getHint()) + .contains(callbackTestHint); + assertThat(lifecycleLogStartedRequests).extracting(lbRequest -> lbRequest.getContext().getHint()) .contains(callbackTestHint); assertThat(anotherLifecycleLogRequests) - .extracting(completionContext -> ((ResponseData) completionContext.getClientResponse()).getHttpStatus()) + .extracting(completionContext -> completionContext.getClientResponse().getHttpStatus()) .contains(HttpStatus.OK); } @@ -180,30 +183,43 @@ class FeignBlockingLoadBalancerClientTests { } - protected static class TestLoadBalancerLifecycle implements LoadBalancerLifecycle { + protected static class TestLoadBalancerLifecycle + implements LoadBalancerLifecycle { - final ConcurrentHashMap> startLog = new ConcurrentHashMap<>(); + final Map> startLog = new ConcurrentHashMap<>(); - final ConcurrentHashMap> completeLog = new ConcurrentHashMap<>(); + final Map> startRequestLog = new ConcurrentHashMap<>(); + + final Map> completeLog = new ConcurrentHashMap<>(); @Override - public void onStart(org.springframework.cloud.client.loadbalancer.Request request) { + public void onStart(org.springframework.cloud.client.loadbalancer.Request request) { startLog.put(getName() + UUID.randomUUID(), request); } @Override - public void onComplete(CompletionContext completionContext) { + public void onStartRequest(org.springframework.cloud.client.loadbalancer.Request request, + org.springframework.cloud.client.loadbalancer.Response lbResponse) { + startRequestLog.put(getName() + UUID.randomUUID(), request); + } + + @Override + public void onComplete(CompletionContext completionContext) { completeLog.put(getName() + UUID.randomUUID(), completionContext); } - ConcurrentHashMap> getStartLog() { + Map> getStartLog() { return startLog; } - ConcurrentHashMap> getCompleteLog() { + Map> getCompleteLog() { return completeLog; } + Map> getStartRequestLog() { + return startRequestLog; + } + protected String getName() { return this.getClass().getSimpleName(); } 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 f30cf63d..7d9deaf8 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 @@ -38,11 +38,11 @@ import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.cloud.client.DefaultServiceInstance; import org.springframework.cloud.client.ServiceInstance; import org.springframework.cloud.client.loadbalancer.CompletionContext; -import org.springframework.cloud.client.loadbalancer.DefaultRequestContext; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryFactory; import org.springframework.cloud.client.loadbalancer.LoadBalancerLifecycle; import org.springframework.cloud.client.loadbalancer.LoadBalancerProperties; import org.springframework.cloud.client.loadbalancer.ResponseData; +import org.springframework.cloud.client.loadbalancer.RetryableRequestContext; import org.springframework.cloud.loadbalancer.blocking.client.BlockingLoadBalancerClient; import org.springframework.cloud.loadbalancer.blocking.retry.BlockingLoadBalancedRetryPolicy; import org.springframework.cloud.loadbalancer.support.LoadBalancerClientFactory; @@ -196,15 +196,18 @@ class RetryableFeignBlockingLoadBalancerClientTests { feignBlockingLoadBalancerClient.execute(request, options); - Collection> lifecycleLogRequests = ((TestLoadBalancerLifecycle) loadBalancerLifecycleBeans + Collection> lifecycleLogRequests = ((TestLoadBalancerLifecycle) loadBalancerLifecycleBeans .get("loadBalancerLifecycle")).getStartLog().values(); - Collection> anotherLifecycleLogRequests = ((AnotherLoadBalancerLifecycle) loadBalancerLifecycleBeans + Collection> lifecycleLogStartedRequests = ((TestLoadBalancerLifecycle) loadBalancerLifecycleBeans + .get("loadBalancerLifecycle")).getStartRequestLog().values(); + Collection> anotherLifecycleLogRequests = ((AnotherLoadBalancerLifecycle) loadBalancerLifecycleBeans .get("anotherLoadBalancerLifecycle")).getCompleteLog().values(); - assertThat(lifecycleLogRequests) - .extracting(lbRequest -> ((DefaultRequestContext) lbRequest.getContext()).getHint()) + assertThat(lifecycleLogRequests).extracting(lbRequest -> lbRequest.getContext().getHint()) + .contains(callbackTestHint); + assertThat(lifecycleLogStartedRequests).extracting(lbRequest -> lbRequest.getContext().getHint()) .contains(callbackTestHint); assertThat(anotherLifecycleLogRequests) - .extracting(completionContext -> ((ResponseData) completionContext.getClientResponse()).getHttpStatus()) + .extracting(completionContext -> completionContext.getClientResponse().getHttpStatus()) .contains(HttpStatus.OK); } @@ -224,30 +227,45 @@ class RetryableFeignBlockingLoadBalancerClientTests { } - protected static class TestLoadBalancerLifecycle implements LoadBalancerLifecycle { + protected static class TestLoadBalancerLifecycle + implements LoadBalancerLifecycle { - final ConcurrentHashMap> startLog = new ConcurrentHashMap<>(); + final Map> startLog = new ConcurrentHashMap<>(); - final ConcurrentHashMap> completeLog = new ConcurrentHashMap<>(); + final Map> startRequestLog = new ConcurrentHashMap<>(); + + final Map> completeLog = new ConcurrentHashMap<>(); @Override - public void onStart(org.springframework.cloud.client.loadbalancer.Request request) { + public void onStart(org.springframework.cloud.client.loadbalancer.Request request) { startLog.put(getName() + UUID.randomUUID(), request); } @Override - public void onComplete(CompletionContext completionContext) { + public void onStartRequest( + org.springframework.cloud.client.loadbalancer.Request request, + org.springframework.cloud.client.loadbalancer.Response lbResponse) { + startRequestLog.put(getName() + UUID.randomUUID(), request); + } + + @Override + public void onComplete( + CompletionContext completionContext) { completeLog.put(getName() + UUID.randomUUID(), completionContext); } - ConcurrentHashMap> getStartLog() { + Map> getStartLog() { return startLog; } - ConcurrentHashMap> getCompleteLog() { + Map> getCompleteLog() { return completeLog; } + Map> getStartRequestLog() { + return startRequestLog; + } + protected String getName() { return this.getClass().getSimpleName(); } @@ -255,7 +273,7 @@ class RetryableFeignBlockingLoadBalancerClientTests { } protected static class AnotherLoadBalancerLifecycle - extends FeignBlockingLoadBalancerClientTests.TestLoadBalancerLifecycle { + extends RetryableFeignBlockingLoadBalancerClientTests.TestLoadBalancerLifecycle { @Override protected String getName() {