diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceLoadBalancerFeignClient.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceLoadBalancerFeignClient.java index d27656fb3..8644610e7 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceLoadBalancerFeignClient.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceLoadBalancerFeignClient.java @@ -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); diff --git a/tests/spring-cloud-sleuth-instrumentation-feign-tests/src/test/java/org/springframework/cloud/sleuth/instrument/feign/issues/issue1125/ManuallyCreatedLoadBalancerFeignClientTests.java b/tests/spring-cloud-sleuth-instrumentation-feign-tests/src/test/java/org/springframework/cloud/sleuth/instrument/feign/issues/issue1125/ManuallyCreatedLoadBalancerFeignClientTests.java index 2d60c1d3c..b26ecce6b 100644 --- a/tests/spring-cloud-sleuth-instrumentation-feign-tests/src/test/java/org/springframework/cloud/sleuth/instrument/feign/issues/issue1125/ManuallyCreatedLoadBalancerFeignClientTests.java +++ b/tests/spring-cloud-sleuth-instrumentation-feign-tests/src/test/java/org/springframework/cloud/sleuth/instrument/feign/issues/issue1125/ManuallyCreatedLoadBalancerFeignClientTests.java @@ -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 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 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(); } diff --git a/tests/spring-cloud-sleuth-instrumentation-feign-tests/src/test/java/org/springframework/cloud/sleuth/instrument/feign/issues/issue1125delegates/ManuallyCreatedDelegateLoadBalancerFeignClientTests.java b/tests/spring-cloud-sleuth-instrumentation-feign-tests/src/test/java/org/springframework/cloud/sleuth/instrument/feign/issues/issue1125delegates/ManuallyCreatedDelegateLoadBalancerFeignClientTests.java index b3a0f8a61..d28b39527 100644 --- a/tests/spring-cloud-sleuth-instrumentation-feign-tests/src/test/java/org/springframework/cloud/sleuth/instrument/feign/issues/issue1125delegates/ManuallyCreatedDelegateLoadBalancerFeignClientTests.java +++ b/tests/spring-cloud-sleuth-instrumentation-feign-tests/src/test/java/org/springframework/cloud/sleuth/instrument/feign/issues/issue1125delegates/ManuallyCreatedDelegateLoadBalancerFeignClientTests.java @@ -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 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 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(); }