Ensures that a custom status code works fine with Sleuth & WebClient
fixes gh-1462
This commit is contained in:
Marcin Grzejszczak
@@ -346,7 +346,7 @@ final class TraceExchangeFilterFunction implements ExchangeFilterFunction {
|
||||
|
||||
void terminateSpan(@Nullable ClientResponse clientResponse,
|
||||
@Nullable Throwable throwable) {
|
||||
if (clientResponse == null || clientResponse.statusCode() == null) {
|
||||
if (clientResponse == null) {
|
||||
if (log.isDebugEnabled()) {
|
||||
log.debug("No response was returned. Will close the span ["
|
||||
+ this.span + "]");
|
||||
@@ -354,8 +354,8 @@ final class TraceExchangeFilterFunction implements ExchangeFilterFunction {
|
||||
handleReceive(this.span, this.ws, clientResponse, throwable);
|
||||
return;
|
||||
}
|
||||
boolean error = clientResponse.statusCode().is4xxClientError()
|
||||
|| clientResponse.statusCode().is5xxServerError();
|
||||
int statusCode = clientResponse.rawStatusCode();
|
||||
boolean error = statusCode >= 400;
|
||||
if (error) {
|
||||
if (log.isDebugEnabled()) {
|
||||
log.debug(
|
||||
@@ -363,9 +363,8 @@ final class TraceExchangeFilterFunction implements ExchangeFilterFunction {
|
||||
+ this.span + "]");
|
||||
}
|
||||
throwable = new RestClientException("Status code of the response is ["
|
||||
+ clientResponse.statusCode().value()
|
||||
+ "] and the reason is ["
|
||||
+ clientResponse.statusCode().getReasonPhrase() + "]");
|
||||
+ statusCode
|
||||
+ "]");
|
||||
}
|
||||
handleReceive(this.span, this.ws, clientResponse, throwable);
|
||||
}
|
||||
|
||||
@@ -16,20 +16,6 @@
|
||||
|
||||
package org.springframework.cloud.sleuth.instrument.web.client.integration;
|
||||
|
||||
import java.time.Duration;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collections;
|
||||
import java.util.HashMap;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.Optional;
|
||||
import java.util.concurrent.Future;
|
||||
import java.util.concurrent.TimeUnit;
|
||||
import java.util.concurrent.atomic.AtomicReference;
|
||||
import java.util.stream.Collectors;
|
||||
|
||||
import javax.servlet.http.HttpServletRequest;
|
||||
|
||||
import brave.Span;
|
||||
import brave.Tracer;
|
||||
import brave.Tracing;
|
||||
@@ -50,18 +36,8 @@ import org.apache.http.impl.client.HttpClientBuilder;
|
||||
import org.apache.http.impl.nio.client.CloseableHttpAsyncClient;
|
||||
import org.apache.http.impl.nio.client.HttpAsyncClientBuilder;
|
||||
import org.awaitility.Awaitility;
|
||||
import org.junit.After;
|
||||
import org.junit.Before;
|
||||
import org.junit.ClassRule;
|
||||
import org.junit.Ignore;
|
||||
import org.junit.Rule;
|
||||
import org.junit.Test;
|
||||
import org.junit.*;
|
||||
import org.junit.runner.RunWith;
|
||||
import reactor.netty.http.client.HttpClient;
|
||||
import reactor.netty.http.client.HttpClientResponse;
|
||||
import zipkin2.Annotation;
|
||||
import zipkin2.reporter.Reporter;
|
||||
|
||||
import org.springframework.beans.factory.annotation.Autowired;
|
||||
import org.springframework.beans.factory.annotation.Value;
|
||||
import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
|
||||
@@ -94,7 +70,19 @@ import org.springframework.web.bind.annotation.RequestMethod;
|
||||
import org.springframework.web.bind.annotation.RestController;
|
||||
import org.springframework.web.client.HttpClientErrorException;
|
||||
import org.springframework.web.client.RestTemplate;
|
||||
import org.springframework.web.reactive.function.client.UnknownHttpStatusCodeException;
|
||||
import org.springframework.web.reactive.function.client.WebClient;
|
||||
import reactor.netty.http.client.HttpClient;
|
||||
import reactor.netty.http.client.HttpClientResponse;
|
||||
import zipkin2.Annotation;
|
||||
import zipkin2.reporter.Reporter;
|
||||
|
||||
import javax.servlet.http.HttpServletRequest;
|
||||
import java.util.*;
|
||||
import java.util.concurrent.Future;
|
||||
import java.util.concurrent.TimeUnit;
|
||||
import java.util.concurrent.atomic.AtomicReference;
|
||||
import java.util.stream.Collectors;
|
||||
|
||||
import static org.assertj.core.api.Assertions.fail;
|
||||
import static org.assertj.core.api.BDDAssertions.then;
|
||||
@@ -377,46 +365,25 @@ public class WebClientTests {
|
||||
.contains("CLIENT");
|
||||
}
|
||||
|
||||
@Test
|
||||
@Ignore("Flakey on CI")
|
||||
public void shouldReportTraceForCancelledRequestViaWebClient() {
|
||||
Span span = this.tracer.nextSpan().name("foo").start();
|
||||
|
||||
try (Tracer.SpanInScope ws = this.tracer.withSpanInScope(span)) {
|
||||
this.webClient.get().uri("http://localhost:" + this.port + "/noresponse")
|
||||
.retrieve().bodyToMono(String.class).timeout(Duration.ofMillis(0))
|
||||
.block();
|
||||
}
|
||||
catch (Exception e) {
|
||||
|
||||
}
|
||||
finally {
|
||||
span.finish();
|
||||
}
|
||||
|
||||
Awaitility.await().untilAsserted(() -> {
|
||||
System.out.println("Found spans " + this.reporter.getSpans());
|
||||
final Optional<zipkin2.Span> clientSpan = this.reporter.getSpans().stream()
|
||||
.filter(s -> s.kind() == zipkin2.Span.Kind.CLIENT).findFirst();
|
||||
then(clientSpan).isPresent();
|
||||
then(clientSpan.get().tags()).containsEntry("error", "CANCELLED");
|
||||
});
|
||||
}
|
||||
|
||||
@Test
|
||||
@SuppressWarnings("unchecked")
|
||||
public void shouldNotBreakWhenCustomStatusCodeIsSetViaWebClient() {
|
||||
public void shouldWorkWhenCustomStatusCodeIsReturned() throws InterruptedException {
|
||||
Span span = this.tracer.nextSpan().name("foo").start();
|
||||
|
||||
try (Tracer.SpanInScope ws = this.tracer.withSpanInScope(span)) {
|
||||
this.webClient.get()
|
||||
.uri("http://localhost:" + this.port + "/customstatuscode").exchange()
|
||||
.block();
|
||||
this.webClient.get().uri("http://localhost:" + this.port + "/issue1462")
|
||||
.retrieve().bodyToMono(String.class).block();
|
||||
}
|
||||
catch (UnknownHttpStatusCodeException ex) {
|
||||
|
||||
}
|
||||
finally {
|
||||
span.finish();
|
||||
}
|
||||
|
||||
then(this.tracer.currentSpan()).isNull();
|
||||
then(this.reporter.getSpans()).isNotEmpty().extracting("kind.name")
|
||||
.contains("CLIENT");
|
||||
}
|
||||
|
||||
Object[] parametersForShouldAttachTraceIdWhenCallingAnotherService() {
|
||||
@@ -567,6 +534,11 @@ public class WebClientTests {
|
||||
return new FooController();
|
||||
}
|
||||
|
||||
@Bean
|
||||
WebClientController webClientController() {
|
||||
return new WebClientController();
|
||||
}
|
||||
|
||||
@LoadBalanced
|
||||
@Bean
|
||||
public RestTemplate restTemplate() {
|
||||
@@ -675,7 +647,6 @@ public class WebClientTests {
|
||||
then(traceId).isNotEmpty();
|
||||
then(parentId).isNotEmpty();
|
||||
then(spanId).isNotEmpty();
|
||||
this.span = this.tracer.currentSpan();
|
||||
return traceId;
|
||||
}
|
||||
|
||||
@@ -707,6 +678,17 @@ public class WebClientTests {
|
||||
|
||||
}
|
||||
|
||||
@RestController
|
||||
public static class WebClientController {
|
||||
|
||||
@RequestMapping(value = "/issue1462", method = RequestMethod.GET)
|
||||
public ResponseEntity<String> issue1462() {
|
||||
System.out.println("GOT IT");
|
||||
return ResponseEntity.status(499).body("issue1462");
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@Configuration
|
||||
public static class SimpleRibbonClientConfiguration {
|
||||
|
||||
|
||||
Reference in New Issue
Block a user