Fixing infinite wrapping of LazyTracingFeignClient, see: gh-1824 (#1837)

This commit is contained in:
Jonatan Ivanov
2021-01-29 20:52:00 -08:00
committed by GitHub
parent d0d462a8bf
commit dbf57a2870
2 changed files with 48 additions and 1 deletions

View File

@@ -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);

View File

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