Repository: spring-cloud/spring-cloud-sleuth
Issue: For LoadBalancerFeignClient extensions, the calls fails due to "double load-balancing"

The issue is in the following code: 

https://github.com/spring-cloud/spring-cloud-sleuth/blob/37645fbf9d3e50dc341f333cf8bfd35189594798/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignObjectWrapper.java#L85-L100

Instead of passing the delegate of the load-balanced client as the first argument of `TraceLoadBalancerFeignClient` (which itself implements `LoadBalancerFeignClient`), the actual load-balanced client is passed.

This results in the call being load-balanced first by the `TraceLoadBalancerFeignClient` `super.execute(request, options)` call, which already resolves the `clientId` to a correct host IP and then again by that load-balanced client passed as the delegate, which fails because it cannot resolve a host IP from what is already a host IP.

Changed files:
tests/spring-cloud-sleuth-instrumentation-feign-tests/src/test/java/org/springframework/cloud/sleuth/instrument/feign/issues/issue1125delegates/ManuallyCreatedDelegateLoadBalancerFeignClientTests.java
spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceLoadBalancerFeignClient.java
tests/spring-cloud-sleuth-instrumentation-feign-tests/src/test/java/org/springframework/cloud/sleuth/instrument/feign/issues/issue1125/ManuallyCreatedLoadBalancerFeignClientTests.java

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<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();
 
 }
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<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();
 
 }