From 92e434e26129bef00215cd20253a58b32c8ee12d Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Fri, 10 Nov 2017 15:21:21 +0800 Subject: [PATCH 1/3] Updates to zipkin version fixing encoding bug --- spring-cloud-sleuth-dependencies/pom.xml | 10 ++++++++-- spring-cloud-sleuth-samples/pom.xml | 4 ++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/spring-cloud-sleuth-dependencies/pom.xml b/spring-cloud-sleuth-dependencies/pom.xml index f427e69e2..3a6876d53 100644 --- a/spring-cloud-sleuth-dependencies/pom.xml +++ b/spring-cloud-sleuth-dependencies/pom.xml @@ -14,8 +14,8 @@ spring-cloud-sleuth-dependencies Spring Cloud Sleuth Dependencies - 2.2.0 - 1.1.1 + 2.2.2 + 1.1.2 @@ -52,6 +52,12 @@ io.zipkin.java zipkin + + 2.2.1 + + + io.zipkin.zipkin2 + zipkin ${zipkin.version} diff --git a/spring-cloud-sleuth-samples/pom.xml b/spring-cloud-sleuth-samples/pom.xml index 9cd842a09..8f4929f80 100644 --- a/spring-cloud-sleuth-samples/pom.xml +++ b/spring-cloud-sleuth-samples/pom.xml @@ -59,12 +59,12 @@ io.zipkin.java zipkin - 2.2.0 + 2.2.2 io.zipkin.java zipkin-server - 2.2.0 + 2.2.2 From 8e9fa7125c0534f750febcba7dc8ff8ad55a74ae Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Tue, 21 Nov 2017 19:07:06 +0100 Subject: [PATCH 2/3] Ensure we're not tracing the traced feign clients fixes #791 --- .../web/client/feign/TraceFeignAspect.java | 16 ++-- .../client/feign/TraceFeignAspectTests.java | 77 +++++++++++++++++++ 2 files changed, 88 insertions(+), 5 deletions(-) create mode 100644 spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignAspectTests.java diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignAspect.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignAspect.java index 0486d4575..829decdbb 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignAspect.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignAspect.java @@ -16,6 +16,8 @@ package org.springframework.cloud.sleuth.instrument.web.client.feign; +import java.io.IOException; + import org.aspectj.lang.ProceedingJoinPoint; import org.aspectj.lang.annotation.Around; import org.aspectj.lang.annotation.Aspect; @@ -41,13 +43,17 @@ class TraceFeignAspect { @Around("execution (* feign.Client.*(..)) && !within(is(FinalType))") public Object feignClientWasCalled(final ProceedingJoinPoint pjp) throws Throwable { - Object[] args = pjp.getArgs(); - Request request = (Request) args[0]; - Request.Options options = (Request.Options) args[1]; Object bean = pjp.getTarget(); - if (!(bean instanceof TraceFeignClient)) { - return new TraceFeignClient(this.beanFactory, (Client) bean).execute(request, options); + if (!(bean instanceof TraceFeignClient) && !(bean instanceof TraceLoadBalancerFeignClient)) { + return executeTraceFeignClient(bean, pjp); } return pjp.proceed(); } + + Object executeTraceFeignClient(Object bean, ProceedingJoinPoint pjp) throws IOException { + Object[] args = pjp.getArgs(); + Request request = (Request) args[0]; + Request.Options options = (Request.Options) args[1]; + return new TraceFeignClient(this.beanFactory, (Client) bean).execute(request, options); + } } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignAspectTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignAspectTests.java new file mode 100644 index 000000000..98d231c74 --- /dev/null +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignAspectTests.java @@ -0,0 +1,77 @@ +package org.springframework.cloud.sleuth.instrument.web.client.feign; + +import java.io.IOException; +import java.nio.charset.Charset; +import java.util.HashMap; + +import feign.Client; +import feign.Request; +import org.aspectj.lang.ProceedingJoinPoint; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; +import org.springframework.beans.factory.BeanFactory; + +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; + +/** + * @author Marcin Grzejszczak + */ +@RunWith(MockitoJUnitRunner.class) +public class TraceFeignAspectTests { + + @Mock BeanFactory beanFactory; + @Mock Client client; + @Mock ProceedingJoinPoint pjp; + @Mock TraceLoadBalancerFeignClient traceLoadBalancerFeignClient; + TraceFeignAspect traceFeignAspect; + + @Before + public void setup() { + stubPjp(); + this.traceFeignAspect = new TraceFeignAspect(this.beanFactory) { + @Override Object executeTraceFeignClient(Object bean, ProceedingJoinPoint pjp) throws IOException { + return null; + } + }; + } + + private void stubPjp() { + Request request = Request.create("foo", "bar", new HashMap<>(), new byte[] {}, Charset + .defaultCharset()); + Request.Options options = new Request.Options(); + given(this.pjp.getArgs()).willReturn(new Object[] {request, options} ); + } + + @Test + public void should_wrap_feign_client_in_trace_representation() throws Throwable { + given(this.pjp.getTarget()).willReturn(this.client); + + this.traceFeignAspect.feignClientWasCalled(this.pjp); + + verify(this.pjp, never()).proceed(); + } + + @Test + public void should_not_wrap_traced_feign_client_in_trace_representation() throws Throwable { + given(this.pjp.getTarget()).willReturn(new TraceFeignClient(this.beanFactory, this.client)); + + this.traceFeignAspect.feignClientWasCalled(this.pjp); + + verify(this.pjp).proceed(); + } + + @Test + public void should_not_wrap_traced_load_balancer_feign_client_in_trace_representation() throws Throwable { + given(this.pjp.getTarget()).willReturn(this.traceLoadBalancerFeignClient); + + this.traceFeignAspect.feignClientWasCalled(this.pjp); + + verify(this.pjp).proceed(); + } + +} \ No newline at end of file From 2d53a38c142823b1e29b50d8e877ab04076b4f10 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Wed, 22 Nov 2017 10:07:40 +0100 Subject: [PATCH 3/3] Wrapping the feign object in a different way --- .../sleuth/instrument/web/client/feign/TraceFeignAspect.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignAspect.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignAspect.java index 829decdbb..76a476ad7 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignAspect.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignAspect.java @@ -44,7 +44,8 @@ class TraceFeignAspect { @Around("execution (* feign.Client.*(..)) && !within(is(FinalType))") public Object feignClientWasCalled(final ProceedingJoinPoint pjp) throws Throwable { Object bean = pjp.getTarget(); - if (!(bean instanceof TraceFeignClient) && !(bean instanceof TraceLoadBalancerFeignClient)) { + Object wrappedBean = new TraceFeignObjectWrapper(this.beanFactory).wrap(bean); + if (bean != wrappedBean) { return executeTraceFeignClient(bean, pjp); } return pjp.proceed(); @@ -54,6 +55,6 @@ class TraceFeignAspect { Object[] args = pjp.getArgs(); Request request = (Request) args[0]; Request.Options options = (Request.Options) args[1]; - return new TraceFeignClient(this.beanFactory, (Client) bean).execute(request, options); + return ((Client) bean).execute(request, options); } }