From dbf57a28701856a0aa0058e364d3c15c59fcdd25 Mon Sep 17 00:00:00 2001 From: Jonatan Ivanov Date: Fri, 29 Jan 2021 20:52:00 -0800 Subject: [PATCH] Fixing infinite wrapping of LazyTracingFeignClient, see: gh-1824 (#1837) --- .../client/feign/TraceFeignObjectWrapper.java | 3 +- .../feign/TracingFeignObjectWrapperTests.java | 46 +++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignObjectWrapper.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignObjectWrapper.java index fcf0f9d64..f54af344c 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignObjectWrapper.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignObjectWrapper.java @@ -83,7 +83,8 @@ final class TraceFeignObjectWrapper { } Object wrap(Object bean) { - if (bean instanceof Client && !(bean instanceof TracingFeignClient)) { + if (bean instanceof Client && !(bean instanceof TracingFeignClient) + && !(bean instanceof LazyTracingFeignClient)) { if (ribbonPresent && bean instanceof LoadBalancerFeignClient && !(bean instanceof TraceLoadBalancerFeignClient)) { return instrumentedFeignRibbonClient(bean); diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TracingFeignObjectWrapperTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TracingFeignObjectWrapperTests.java index d355f36dc..1705b77c1 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TracingFeignObjectWrapperTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TracingFeignObjectWrapperTests.java @@ -30,6 +30,8 @@ import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryFactory; import org.springframework.cloud.loadbalancer.blocking.client.BlockingLoadBalancerClient; import org.springframework.cloud.openfeign.loadbalancer.FeignBlockingLoadBalancerClient; import org.springframework.cloud.openfeign.loadbalancer.RetryableFeignBlockingLoadBalancerClient; +import org.springframework.cloud.openfeign.ribbon.LoadBalancerFeignClient; +import org.springframework.test.util.ReflectionTestUtils; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.BDDAssertions.then; @@ -61,6 +63,19 @@ public class TracingFeignObjectWrapperTests { .isSameAs(notFeignRelatedObject); } + // gh-1824 + @Test + public void should_not_wrap_nor_modify_lazy_tracing_feign_client() { + Client delegate = mock(Client.class); + LazyTracingFeignClient lazyTracingFeignClient = new LazyTracingFeignClient( + beanFactory, delegate); + + assertThat(traceFeignObjectWrapper.wrap(lazyTracingFeignClient)) + .isSameAs(lazyTracingFeignClient); + assertThat(ReflectionTestUtils.getField(lazyTracingFeignClient, "delegate")) + .isSameAs(delegate); + } + // gh-1528 @Test public void should_wrap_feign_loadbalancer_client() { @@ -128,6 +143,29 @@ public class TracingFeignObjectWrapperTests { .isInstanceOf(TraceRetryableFeignBlockingLoadBalancerClient.class); } + // gh-1824, multiwrap should be ok too + @Test + public void should_wrap_subclass_of_load_balancer_feign_client() { + Client delegate = mock(Client.class); + TestLoadBalancerFeignClient testLoadBalancerFeignClient = new TestLoadBalancerFeignClient( + delegate); + + traceFeignObjectWrapper.wrap(testLoadBalancerFeignClient); + traceFeignObjectWrapper.wrap(testLoadBalancerFeignClient); + Object wrapped = traceFeignObjectWrapper.wrap(testLoadBalancerFeignClient); + assertThat(wrapped).isExactlyInstanceOf(TraceLoadBalancerFeignClient.class); + TraceLoadBalancerFeignClient wrappedClient = (TraceLoadBalancerFeignClient) wrapped; + assertThat(wrappedClient.getDelegate()).isSameAs(testLoadBalancerFeignClient); + TestLoadBalancerFeignClient firstDelegate = (TestLoadBalancerFeignClient) wrappedClient + .getDelegate(); + assertThat(firstDelegate.getDelegate()) + .isExactlyInstanceOf(LazyTracingFeignClient.class); + LazyTracingFeignClient secondDelegate = (LazyTracingFeignClient) firstDelegate + .getDelegate(); + assertThat(ReflectionTestUtils.getField(secondDelegate, "delegate")) + .isSameAs(delegate); + } + static class TestFeignBlockingLoadBalancerClient extends FeignBlockingLoadBalancerClient { @@ -149,4 +187,12 @@ public class TracingFeignObjectWrapperTests { } + static class TestLoadBalancerFeignClient extends LoadBalancerFeignClient { + + TestLoadBalancerFeignClient(Client delegate) { + super(delegate, null, null); + } + + } + }