Skip to content

Commit ae1ac8b

Browse files
OlgaMaciaszekOlgaMaciaszek
authored
Handle null in LoadBalancer Request and Response (#1454)
* Handle null lbRequest and lbResponse values. Signed-off-by: Olga Maciaszek-Sharma <olga.maciaszek-sharma@broadcom.com> * Verify for null request on start. Signed-off-by: Olga Maciaszek-Sharma <olga.maciaszek-sharma@broadcom.com> --------- Signed-off-by: Olga Maciaszek-Sharma <olga.maciaszek-sharma@broadcom.com>
1 parent c50e976 commit ae1ac8b

File tree

3 files changed

+51
-12
lines changed

3 files changed

+51
-12
lines changed

‎spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/stats/LoadBalancerTags.java‎

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,10 @@
2727
import org.springframework.cloud.client.ServiceInstance;
2828
import org.springframework.cloud.client.loadbalancer.CompletionContext;
2929
import org.springframework.cloud.client.loadbalancer.LoadBalancerProperties;
30+
import org.springframework.cloud.client.loadbalancer.Request;
3031
import org.springframework.cloud.client.loadbalancer.RequestData;
3132
import org.springframework.cloud.client.loadbalancer.RequestDataContext;
33+
import org.springframework.cloud.client.loadbalancer.Response;
3234
import org.springframework.cloud.client.loadbalancer.ResponseData;
3335
import org.springframework.util.StringUtils;
3436

@@ -55,7 +57,11 @@ class LoadBalancerTags {
5557
}
5658

5759
Iterable<Tag> buildSuccessRequestTags(CompletionContext<Object, ServiceInstance, Object> completionContext) {
58-
ServiceInstance serviceInstance = completionContext.getLoadBalancerResponse().getServer();
60+
Response<ServiceInstance> lbResponse = completionContext.getLoadBalancerResponse();
61+
if (lbResponse == null) {
62+
return Tags.empty();
63+
}
64+
ServiceInstance serviceInstance = lbResponse.getServer();
5965
Tags tags = Tags.of(buildServiceInstanceTags(serviceInstance));
6066
Object clientResponse = completionContext.getClientResponse();
6167
if (clientResponse instanceof ResponseData responseData) {
@@ -100,9 +106,9 @@ private String getPath(RequestData requestData) {
100106
}
101107

102108
Iterable<Tag> buildDiscardedRequestTags(CompletionContext<Object, ServiceInstance, Object> completionContext) {
103-
if (completionContext.getLoadBalancerRequest().getContext() instanceof RequestDataContext) {
104-
RequestData requestData = ((RequestDataContext) completionContext.getLoadBalancerRequest().getContext())
105-
.getClientRequest();
109+
Request<Object> lbRequest = completionContext.getLoadBalancerRequest();
110+
if (lbRequest != null && lbRequest.getContext() instanceof RequestDataContext requestDataContext) {
111+
RequestData requestData = requestDataContext.getClientRequest();
106112
if (requestData != null) {
107113
return Tags.of(valueOrUnknown("method", requestData.getHttpMethod()),
108114
valueOrUnknown("uri", getPath(requestData)), valueOrUnknown("serviceId", getHost(requestData)));
@@ -118,11 +124,15 @@ private static String getHost(RequestData requestData) {
118124
}
119125

120126
Iterable<Tag> buildFailedRequestTags(CompletionContext<Object, ServiceInstance, Object> completionContext) {
121-
ServiceInstance serviceInstance = completionContext.getLoadBalancerResponse().getServer();
127+
Response<ServiceInstance> lbResponse = completionContext.getLoadBalancerResponse();
128+
if (lbResponse == null) {
129+
return Tags.empty();
130+
}
131+
ServiceInstance serviceInstance = lbResponse.getServer();
122132
Tags tags = Tags.of(buildServiceInstanceTags(serviceInstance)).and(exception(completionContext.getThrowable()));
123-
if (completionContext.getLoadBalancerRequest().getContext() instanceof RequestDataContext) {
124-
RequestData requestData = ((RequestDataContext) completionContext.getLoadBalancerRequest().getContext())
125-
.getClientRequest();
133+
Request<Object> lbRequest = completionContext.getLoadBalancerRequest();
134+
if (lbRequest != null && lbRequest.getContext() instanceof RequestDataContext requestDataContext) {
135+
RequestData requestData = requestDataContext.getClientRequest();
126136
if (requestData != null) {
127137
return tags.and(Tags.of(valueOrUnknown("method", requestData.getHttpMethod()),
128138
valueOrUnknown("uri", getPath(requestData))));

‎spring-cloud-loadbalancer/src/main/java/org/springframework/cloud/loadbalancer/stats/MicrometerStatsLoadBalancerLifecycle.java‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,10 +85,10 @@ public void onStart(Request<Object> request) {
8585

8686
@Override
8787
public void onStartRequest(Request<Object> request, Response<ServiceInstance> lbResponse) {
88-
if (request.getContext() instanceof TimedRequestContext) {
88+
if (request != null && request.getContext() instanceof TimedRequestContext) {
8989
((TimedRequestContext) request.getContext()).setRequestStartTime(System.nanoTime());
9090
}
91-
if (!lbResponse.hasServer()) {
91+
if (lbResponse == null || !lbResponse.hasServer()) {
9292
return;
9393
}
9494
ServiceInstance serviceInstance = lbResponse.getServer();
@@ -104,7 +104,11 @@ public void onStartRequest(Request<Object> request, Response<ServiceInstance> lb
104104

105105
@Override
106106
public void onComplete(CompletionContext<Object, ServiceInstance, Object> completionContext) {
107-
ServiceInstance serviceInstance = completionContext.getLoadBalancerResponse().getServer();
107+
ServiceInstance serviceInstance = null;
108+
Response<ServiceInstance> loadBalancerResponse = completionContext.getLoadBalancerResponse();
109+
if (loadBalancerResponse != null) {
110+
serviceInstance = loadBalancerResponse.getServer();
111+
}
108112
LoadBalancerProperties properties = serviceInstance != null
109113
? loadBalancerFactory.getProperties(serviceInstance.getServiceId())
110114
: loadBalancerFactory.getProperties(null);
@@ -121,7 +125,11 @@ public void onComplete(CompletionContext<Object, ServiceInstance, Object> comple
121125
if (activeRequestsCounter != null) {
122126
activeRequestsCounter.decrementAndGet();
123127
}
124-
Object loadBalancerRequestContext = completionContext.getLoadBalancerRequest().getContext();
128+
Request<Object> lbRequest = completionContext.getLoadBalancerRequest();
129+
if (lbRequest == null) {
130+
return;
131+
}
132+
Object loadBalancerRequestContext = lbRequest.getContext();
125133
if (requestHasBeenTimed(loadBalancerRequestContext)) {
126134
if (CompletionContext.Status.FAILED.equals(completionContext.status())) {
127135
Timer.builder("loadbalancer.requests.failed")

‎spring-cloud-loadbalancer/src/test/java/org/springframework/cloud/loadbalancer/stats/MicrometerStatsLoadBalancerLifecycleTests.java‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import org.springframework.util.MultiValueMapAdapter;
4848

4949
import static org.assertj.core.api.Assertions.assertThat;
50+
import static org.assertj.core.api.Assertions.assertThatCode;
5051
import static org.mockito.Mockito.mock;
5152
import static org.mockito.Mockito.when;
5253
import static org.springframework.cloud.loadbalancer.stats.LoadBalancerTags.UNKNOWN;
@@ -243,6 +244,26 @@ void shouldNotCreateNullTagsWhenEmptyDataObjects() {
243244
Tag.of("serviceInstance.port", "0"), Tag.of("status", "200"), Tag.of("uri", UNKNOWN));
244245
}
245246

247+
@Test
248+
void shouldHandleNullLoadBalancerResponse() {
249+
RequestData requestData = new RequestData(HttpMethod.GET, URI.create("http://test.org/test"), new HttpHeaders(),
250+
new HttpHeaders(), new HashMap<>());
251+
Request<Object> lbRequest = new DefaultRequest<>(new RequestDataContext(requestData));
252+
assertThatCode(() -> {
253+
statsLifecycle.onStartRequest(lbRequest, null);
254+
statsLifecycle.onComplete(new CompletionContext<>(CompletionContext.Status.DISCARD, lbRequest, null));
255+
}).doesNotThrowAnyException();
256+
}
257+
258+
@Test
259+
void shouldHandleNullLoadBalancerRequest() {
260+
Response<ServiceInstance> lbResponse = new EmptyResponse();
261+
assertThatCode(() -> {
262+
statsLifecycle.onStartRequest(null, lbResponse);
263+
statsLifecycle.onComplete(new CompletionContext<>(CompletionContext.Status.DISCARD, null, lbResponse));
264+
}).doesNotThrowAnyException();
265+
}
266+
246267
private static class StatsTestContext {
247268

248269
}

0 commit comments

Comments
 (0)