Skip to content

Commit b59277f

Browse files
Not using httpStatus() method for custom status codes
curently whenever we try to retrieve the http status we get an exception for custom codes we've migrated to using the raw status code. Also, due to spring-projects/spring-framework#23366 we had to manually wrap the ClientResponse so as we don't throw an exception on httpStatus() method fixes gh-1393
1 parent d642ffd commit b59277f

File tree

2 files changed

+148
-11
lines changed

2 files changed

+148
-11
lines changed

spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceWebClientBeanPostProcessor.java

Lines changed: 126 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import org.reactivestreams.Publisher;
3434
import org.reactivestreams.Subscription;
3535
import reactor.core.CoreSubscriber;
36+
import reactor.core.publisher.Flux;
3637
import reactor.core.publisher.Mono;
3738
import reactor.util.annotation.Nullable;
3839
import reactor.util.context.Context;
@@ -41,12 +42,20 @@
4142
import org.springframework.beans.factory.BeanFactory;
4243
import org.springframework.beans.factory.config.BeanPostProcessor;
4344
import org.springframework.cloud.sleuth.instrument.reactor.ReactorSleuth;
45+
import org.springframework.core.ParameterizedTypeReference;
4446
import org.springframework.core.io.buffer.DataBuffer;
47+
import org.springframework.http.HttpStatus;
48+
import org.springframework.http.ResponseCookie;
49+
import org.springframework.http.ResponseEntity;
50+
import org.springframework.http.client.reactive.ClientHttpResponse;
51+
import org.springframework.util.MultiValueMap;
4552
import org.springframework.web.client.RestClientException;
53+
import org.springframework.web.reactive.function.BodyExtractor;
4654
import org.springframework.web.reactive.function.client.ClientRequest;
4755
import org.springframework.web.reactive.function.client.ClientResponse;
4856
import org.springframework.web.reactive.function.client.ExchangeFilterFunction;
4957
import org.springframework.web.reactive.function.client.ExchangeFunction;
58+
import org.springframework.web.reactive.function.client.ExchangeStrategies;
5059
import org.springframework.web.reactive.function.client.WebClient;
5160

5261
/**
@@ -290,17 +299,102 @@ public void onNext(ClientResponse response) {
290299
this.done = true;
291300
try {
292301
// decorate response body
293-
this.actual
294-
.onNext(ClientResponse.from(response)
295-
.body(response.bodyToFlux(DataBuffer.class)
296-
.transform(this.scopePassingTransformer))
297-
.build());
302+
this.actual.onNext(wrapped(response));
298303
}
299304
finally {
300305
terminateSpan(response, null);
301306
}
302307
}
303308

309+
// TODO: Remove once fixed
310+
// https://github.com/spring-projects/spring-framework/issues/23366
311+
private ClientResponse wrapped(ClientResponse response) {
312+
return new ClientResponse() {
313+
@Override
314+
public HttpStatus statusCode() {
315+
try {
316+
return response.statusCode();
317+
}
318+
catch (IllegalArgumentException ex) {
319+
return null;
320+
}
321+
}
322+
323+
@Override
324+
public int rawStatusCode() {
325+
return response.rawStatusCode();
326+
}
327+
328+
@Override
329+
public Headers headers() {
330+
return response.headers();
331+
}
332+
333+
@Override
334+
public MultiValueMap<String, ResponseCookie> cookies() {
335+
return response.cookies();
336+
}
337+
338+
@Override
339+
public ExchangeStrategies strategies() {
340+
return response.strategies();
341+
}
342+
343+
@Override
344+
public <T> T body(
345+
BodyExtractor<T, ? super ClientHttpResponse> extractor) {
346+
return response.body(extractor);
347+
}
348+
349+
@Override
350+
public <T> Mono<T> bodyToMono(Class<? extends T> elementClass) {
351+
return response.bodyToMono(elementClass);
352+
}
353+
354+
@Override
355+
public <T> Mono<T> bodyToMono(
356+
ParameterizedTypeReference<T> typeReference) {
357+
return response.bodyToMono(typeReference);
358+
}
359+
360+
@Override
361+
public <T> Flux<T> bodyToFlux(Class<? extends T> elementClass) {
362+
return (Flux<T>) response.bodyToFlux(DataBuffer.class)
363+
.transform(scopePassingTransformer);
364+
}
365+
366+
@Override
367+
public <T> Flux<T> bodyToFlux(
368+
ParameterizedTypeReference<T> typeReference) {
369+
return (Flux<T>) response.bodyToFlux(DataBuffer.class)
370+
.transform(scopePassingTransformer);
371+
}
372+
373+
@Override
374+
public <T> Mono<ResponseEntity<T>> toEntity(Class<T> bodyType) {
375+
return response.toEntity(bodyType);
376+
}
377+
378+
@Override
379+
public <T> Mono<ResponseEntity<T>> toEntity(
380+
ParameterizedTypeReference<T> typeReference) {
381+
return response.toEntity(typeReference);
382+
}
383+
384+
@Override
385+
public <T> Mono<ResponseEntity<List<T>>> toEntityList(
386+
Class<T> elementType) {
387+
return response.toEntityList(elementType);
388+
}
389+
390+
@Override
391+
public <T> Mono<ResponseEntity<List<T>>> toEntityList(
392+
ParameterizedTypeReference<T> typeReference) {
393+
return response.toEntityList(typeReference);
394+
}
395+
};
396+
}
397+
304398
@Override
305399
public void onError(Throwable t) {
306400
try {
@@ -346,30 +440,51 @@ void terminateSpanOnCancel() {
346440

347441
void terminateSpan(@Nullable ClientResponse clientResponse,
348442
@Nullable Throwable throwable) {
349-
if (clientResponse == null || clientResponse.statusCode() == null) {
443+
if (clientResponse == null) {
350444
if (log.isDebugEnabled()) {
351445
log.debug("No response was returned. Will close the span ["
352446
+ this.span + "]");
353447
}
354448
handleReceive(this.span, this.ws, clientResponse, throwable);
355449
return;
356450
}
357-
boolean error = clientResponse.statusCode().is4xxClientError()
358-
|| clientResponse.statusCode().is5xxServerError();
451+
int statusCode = statusCodeAsInt(clientResponse);
452+
boolean error = isError(statusCode);
359453
if (error) {
360454
if (log.isDebugEnabled()) {
361455
log.debug(
362456
"Non positive status code was returned from the call. Will close the span ["
363457
+ this.span + "]");
364458
}
365459
throwable = new RestClientException("Status code of the response is ["
366-
+ clientResponse.statusCode().value()
367-
+ "] and the reason is ["
368-
+ clientResponse.statusCode().getReasonPhrase() + "]");
460+
+ statusCode + "] and the reason is ["
461+
+ reasonPhrase(clientResponse) + "]");
369462
}
370463
handleReceive(this.span, this.ws, clientResponse, throwable);
371464
}
372465

466+
private String reasonPhrase(ClientResponse clientResponse) {
467+
try {
468+
return clientResponse.statusCode().getReasonPhrase();
469+
}
470+
catch (IllegalArgumentException ex) {
471+
return "";
472+
}
473+
}
474+
475+
private boolean isError(int code) {
476+
return code >= 400;
477+
}
478+
479+
private int statusCodeAsInt(ClientResponse response) {
480+
try {
481+
return response.rawStatusCode();
482+
}
483+
catch (Exception dontCare) {
484+
return 0;
485+
}
486+
}
487+
373488
}
374489

375490
}

spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/integration/WebClientTests.java

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,22 @@ public void shouldAttachTraceIdWhenCallingAnotherServiceViaWebClient() {
377377
.contains("CLIENT");
378378
}
379379

380+
@Test
381+
@SuppressWarnings("unchecked")
382+
public void shouldNotBreakWhenCustomStatusCodeIsSetViaWebClient() {
383+
Span span = this.tracer.nextSpan().name("foo").start();
384+
385+
try (Tracer.SpanInScope ws = this.tracer.withSpanInScope(span)) {
386+
this.webClient.get()
387+
.uri("http://localhost:" + this.port + "/customstatuscode").exchange()
388+
.block();
389+
}
390+
finally {
391+
span.finish();
392+
}
393+
then(this.tracer.currentSpan()).isNull();
394+
}
395+
380396
@Test
381397
@Ignore("Flakey on CI")
382398
public void shouldReportTraceForCancelledRequestViaWebClient() {
@@ -663,6 +679,12 @@ public String traceId(@RequestHeader(TRACE_ID_NAME) String traceId,
663679
return traceId;
664680
}
665681

682+
@RequestMapping(value = "/customstatuscode", method = RequestMethod.GET)
683+
public ResponseEntity customStatusCode() {
684+
this.span = this.tracer.currentSpan();
685+
return ResponseEntity.status(499).build();
686+
}
687+
666688
@RequestMapping("/")
667689
public Map<String, String> home(@RequestHeader HttpHeaders headers) {
668690
Map<String, String> map = new HashMap<>();

0 commit comments

Comments
 (0)