SpringCloud/spring-cloud-sleuth
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #1612 from spring-cloud/fix-gh-1610

Browse Source 
Fixes LoadBalancerFeignClient double loadbalancing
This commit is contained in:
Adrian Cole
2020-04-08 09:27:55 +08:00
committed by GitHub
parent fa6eb20914 20e29f2d80
commit 89ac3ed4aa
3 changed files with 44 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceLoadBalancerFeignClient.java
View File

@@ -30,6 +30,7 @@ import org.apache.commons.logging.LogFactory;
import org.springframework.beans.factory.BeanFactory;
import org.springframework.cloud.netflix.ribbon.SpringClientFactory;
import org.springframework.cloud.openfeign.loadbalancer.FeignBlockingLoadBalancerClient;
import org.springframework.cloud.openfeign.ribbon.CachingSpringLoadBalancerFactory;
import org.springframework.cloud.openfeign.ribbon.LoadBalancerFeignClient;
@@ -67,7 +68,12 @@ public class TraceLoadBalancerFeignClient extends LoadBalancerFeignClient {
Response response = null;
Span fallbackSpan = tracer().nextSpan().start();
try {
response = super.execute(request, options);
if (delegateIsALoadBalancer()) {
response = getDelegate().execute(request, options);
}
else {
response = super.execute(request, options);
}
if (log.isDebugEnabled()) {
log.debug("After receive");
}
@@ -95,6 +101,11 @@ public class TraceLoadBalancerFeignClient extends LoadBalancerFeignClient {
}
}
private boolean delegateIsALoadBalancer() {
return getDelegate() instanceof LoadBalancerFeignClient
|| getDelegate() instanceof FeignBlockingLoadBalancerClient;
}
private Tracer tracer() {
if (this.tracer == null) {
this.tracer = this.beanFactory.getBean(Tracer.class);

30
tests/spring-cloud-sleuth-instrumentation-feign-tests/src/test/java/org/springframework/cloud/sleuth/instrument/feign/issues/issue1125/ManuallyCreatedLoadBalancerFeignClientTests.java
View File

@@ -59,10 +59,10 @@ import static org.assertj.core.api.BDDAssertions.then;
public class ManuallyCreatedLoadBalancerFeignClientTests {
@Autowired
MyClient myClient;
MyLoadBalancerClient myLoadBalancerClient;
@Autowired
MyNameRemote myNameRemote;
AnnotatedFeignClient annotatedFeignClient;
@Autowired
ArrayListSpanReporter reporter;
@@ -74,29 +74,29 @@ public class ManuallyCreatedLoadBalancerFeignClientTests {
@Test
public void should_reuse_custom_feign_client() {
String response = this.myNameRemote.get();
String response = this.annotatedFeignClient.get();
then(this.myClient.wasCalled()).isTrue();
then(this.myLoadBalancerClient.wasCalled()).isTrue();
then(response).isEqualTo("foo");
List<Span> spans = this.reporter.getSpans();
// retries
then(spans).hasSize(1);
then(spans.get(0).tags().get("http.path")).isEqualTo("/");
then(spans.get(0).tags().get("http.path")).isEqualTo("/test");
}
@Test
public void my_client_called() {
this.myNameRemote.get();
then(this.myClient.wasCalled()).isTrue();
this.annotatedFeignClient.get();
then(this.myLoadBalancerClient.wasCalled()).isTrue();
}
@Test
public void span_captured() {
this.myNameRemote.get();
this.annotatedFeignClient.get();
List<Span> spans = this.reporter.getSpans();
// retries
then(spans).hasSize(1);
then(spans.get(0).tags().get("http.path")).isEqualTo("/");
then(spans.get(0).tags().get("http.path")).isEqualTo("/test");
}
}
@@ -109,7 +109,8 @@ class Application {
@Bean
public Client client(CachingSpringLoadBalancerFactory cachingFactory,
SpringClientFactory clientFactory) {
return new MyClient(new MyDelegateClient(), cachingFactory, clientFactory);
return new MyLoadBalancerClient(new MyDelegateClient(), cachingFactory,
clientFactory);
}
@Bean
@@ -124,9 +125,10 @@ class Application {
}
class MyClient extends LoadBalancerFeignClient {
class MyLoadBalancerClient extends LoadBalancerFeignClient {
MyClient(Client delegate, CachingSpringLoadBalancerFactory lbClientFactory,
MyLoadBalancerClient(Client delegate,
CachingSpringLoadBalancerFactory lbClientFactory,
SpringClientFactory clientFactory) {
super(delegate, lbClientFactory, clientFactory);
}
@@ -161,9 +163,9 @@ class MyDelegateClient implements Client {
}
@FeignClient(name = "foo", url = "http://foo")
interface MyNameRemote {
interface AnnotatedFeignClient {
@RequestMapping(value = "/", method = RequestMethod.GET)
@RequestMapping(value = "/test", method = RequestMethod.GET)
String get();
}

32
tests/spring-cloud-sleuth-instrumentation-feign-tests/src/test/java/org/springframework/cloud/sleuth/instrument/feign/issues/issue1125delegates/ManuallyCreatedDelegateLoadBalancerFeignClientTests.java
View File

@@ -65,13 +65,13 @@ import static org.assertj.core.api.BDDAssertions.then;
public class ManuallyCreatedDelegateLoadBalancerFeignClientTests {
@Autowired
MyClient myClient;
MyLoadBalancerClient myLoadBalancerClient;
@Autowired
MyDelegateClient myDelegateClient;
@Autowired
MyNameRemote myNameRemote;
AnnotatedFeignClient annotatedFeignClient;
@Autowired
ArrayListSpanReporter reporter;
@@ -83,31 +83,31 @@ public class ManuallyCreatedDelegateLoadBalancerFeignClientTests {
@Test
public void should_reuse_custom_feign_client() {
String response = this.myNameRemote.get();
String response = this.annotatedFeignClient.get();
then(this.myClient.wasCalled()).isTrue();
then(this.myLoadBalancerClient.wasCalled()).isTrue();
then(this.myDelegateClient.wasCalled()).isTrue();
then(response).isEqualTo("foo");
List<Span> spans = this.reporter.getSpans();
// retries
then(spans).hasSize(1);
then(spans.get(0).tags().get("http.path")).isEqualTo("/");
then(spans.get(0).tags().get("http.path")).isEqualTo("/test");
}
@Test
public void my_client_called() {
this.myNameRemote.get();
then(this.myClient.wasCalled()).isTrue();
this.annotatedFeignClient.get();
then(this.myLoadBalancerClient.wasCalled()).isTrue();
then(this.myDelegateClient.wasCalled()).isTrue();
}
@Test
public void span_captured() {
this.myNameRemote.get();
this.annotatedFeignClient.get();
List<Span> spans = this.reporter.getSpans();
// retries
then(spans).hasSize(1);
then(spans.get(0).tags().get("http.path")).isEqualTo("/");
then(spans.get(0).tags().get("http.path")).isEqualTo("/test");
}
}
@@ -126,15 +126,15 @@ class Application {
public Client client(MyDelegateClient myDelegateClient,
CachingSpringLoadBalancerFactory cachingFactory,
SpringClientFactory clientFactory) {
return new MyClient(myDelegateClient, cachingFactory, clientFactory);
return new MyLoadBalancerClient(myDelegateClient, cachingFactory, clientFactory);
}
@Bean
public MyNameRemote myNameRemote(Client client, Decoder decoder, Encoder encoder,
public AnnotatedFeignClient annotatedFeignClient(Client client, Decoder decoder, Encoder encoder,
Contract contract) {
return Feign.builder().client(client).encoder(encoder).decoder(decoder)
.contract(contract)
.target(new HardCodedTarget<>(MyNameRemote.class, "foo", "http://foo"));
.target(new HardCodedTarget<>(AnnotatedFeignClient.class, "foo", "http://foo"));
}
@Bean
@@ -149,9 +149,9 @@ class Application {
}
class MyClient extends LoadBalancerFeignClient {
class MyLoadBalancerClient extends LoadBalancerFeignClient {
MyClient(Client delegate, CachingSpringLoadBalancerFactory lbClientFactory,
MyLoadBalancerClient(Client delegate, CachingSpringLoadBalancerFactory lbClientFactory,
SpringClientFactory clientFactory) {
super(delegate, lbClientFactory, clientFactory);
}
@@ -190,9 +190,9 @@ class MyDelegateClient implements Client {
}
@FeignClient(name = "foo", url = "http://foo")
interface MyNameRemote {
interface AnnotatedFeignClient {
@RequestMapping(value = "/", method = RequestMethod.GET)
@RequestMapping(value = "/test", method = RequestMethod.GET)
String get();
}