From f7930a10aafbc539f0e65a681ce5351fb58cc0e2 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Mon, 7 Sep 2015 12:34:14 +0100 Subject: [PATCH] Ensure ClientSentEvent is emitted by Feign clients Fixes gh-38 --- .../TraceFeignClientAutoConfiguration.java | 10 +++- .../instrument/web/client/FeignTraceTest.java | 53 +++++++++++++++---- 2 files changed, 53 insertions(+), 10 deletions(-) diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceFeignClientAutoConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceFeignClientAutoConfiguration.java index d5a84bb35..3c6a0aac7 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceFeignClientAutoConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceFeignClientAutoConfiguration.java @@ -38,6 +38,7 @@ import org.springframework.boot.autoconfigure.web.HttpMessageConverters; import org.springframework.cloud.netflix.feign.support.ResponseEntityDecoder; import org.springframework.cloud.netflix.feign.support.SpringDecoder; import org.springframework.cloud.sleuth.Span; +import org.springframework.cloud.sleuth.event.ClientReceivedEvent; import org.springframework.cloud.sleuth.event.ClientSentEvent; import org.springframework.context.ApplicationEvent; import org.springframework.context.ApplicationEventPublisher; @@ -75,8 +76,15 @@ public class TraceFeignClientAutoConfiguration { return new ResponseEntityDecoder(new SpringDecoder(messageConverters)) { @Override public Object decode(Response response, Type type) throws IOException, FeignException { - return super.decode(Response.create(response.status(), response.reason(), + try { + return super.decode(Response.create(response.status(), response.reason(), headersWithTraceId(response.headers()), response.body()), type); + } finally { + Span span = getCurrentSpan(); + if (span != null) { + publish(new ClientReceivedEvent(this, span)); + } + } } }; } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/FeignTraceTest.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/FeignTraceTest.java index ffd06a578..c499128d8 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/FeignTraceTest.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/FeignTraceTest.java @@ -5,6 +5,7 @@ import static org.springframework.cloud.sleuth.Trace.PARENT_ID_NAME; import static org.springframework.cloud.sleuth.Trace.SPAN_ID_NAME; import static org.springframework.cloud.sleuth.Trace.TRACE_ID_NAME; +import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -21,10 +22,15 @@ import org.springframework.cloud.netflix.feign.FeignClient; import org.springframework.cloud.netflix.ribbon.RibbonClient; import org.springframework.cloud.sleuth.MilliSpan; import org.springframework.cloud.sleuth.TraceContextHolder; +import org.springframework.cloud.sleuth.event.ClientReceivedEvent; +import org.springframework.cloud.sleuth.event.ClientSentEvent; import org.springframework.cloud.sleuth.instrument.web.TraceWebAutoConfiguration; +import org.springframework.context.ApplicationEvent; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.context.event.EventListener; import org.springframework.http.ResponseEntity; +import org.springframework.stereotype.Component; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; import org.springframework.web.bind.annotation.RequestHeader; import org.springframework.web.bind.annotation.RequestMapping; @@ -36,17 +42,20 @@ import com.netflix.loadbalancer.ILoadBalancer; import com.netflix.loadbalancer.Server; @RunWith(SpringJUnit4ClassRunner.class) -@SpringApplicationConfiguration(classes = { TraceWebAutoConfiguration.class, - FeignTraceTest.TestConfiguration.class }) +@SpringApplicationConfiguration(classes = { TraceWebAutoConfiguration.class, FeignTraceTest.TestConfiguration.class }) @WebIntegrationTest(value = "spring.application.name=fooservice", randomPort = true) public class FeignTraceTest { @Autowired TestFeignInterface testFeignInterface; - + + @Autowired + Listener listener; + @After public void close() { TraceContextHolder.removeCurrentSpan(); + listener.getEvents().clear(); } @Test @@ -56,6 +65,7 @@ public class FeignTraceTest { // then assertThat(getHeader(response, TRACE_ID_NAME)).isNull(); + assertThat(listener.getEvents()).isEmpty(); } @Test @@ -64,8 +74,8 @@ public class FeignTraceTest { String currentTraceId = "currentTraceId"; String currentSpanId = "currentSpanId"; String currentParentId = "currentParentId"; - TraceContextHolder.setCurrentSpan(MilliSpan.builder().traceId(currentTraceId) - .spanId(currentSpanId).parent(currentParentId).build()); + TraceContextHolder.setCurrentSpan( + MilliSpan.builder().traceId(currentTraceId).spanId(currentSpanId).parent(currentParentId).build()); // when ResponseEntity response = testFeignInterface.getTraceId(); @@ -74,17 +84,19 @@ public class FeignTraceTest { assertThat(getHeader(response, TRACE_ID_NAME)).isEqualTo(currentTraceId); assertThat(getHeader(response, SPAN_ID_NAME)).isEqualTo(currentSpanId); assertThat(getHeader(response, PARENT_ID_NAME)).isEqualTo(currentParentId); + assertThat(listener.getEvents().size()).isEqualTo(2); } private String getHeader(ResponseEntity response, String name) { List headers = response.getHeaders().get(name); - return headers==null || headers.isEmpty() ? null : headers.get(0); + return headers == null || headers.isEmpty() ? null : headers.get(0); } @FeignClient("fooservice") public interface TestFeignInterface { @RequestMapping(method = RequestMethod.GET, value = "/traceid") ResponseEntity getTraceId(); + @RequestMapping(method = RequestMethod.GET, value = "/notrace") ResponseEntity getNoTrace(); } @@ -99,20 +111,43 @@ public class FeignTraceTest { FooController fooController() { return new FooController(); } + + @Bean + Listener listener() { + return new Listener(); + } + } + + @Component + public static class Listener { + private List events = new ArrayList<>(); + + @EventListener(ClientSentEvent.class) + public void sent(ClientSentEvent event) { + events.add(event); + } + + @EventListener(ClientReceivedEvent.class) + public void received(ClientReceivedEvent event) { + events.add(event); + } + + public List getEvents() { + return events; + } } @RestController public static class FooController { @RequestMapping(value = "/notrace", method = RequestMethod.GET) - public String notrace(@RequestHeader(name=TRACE_ID_NAME, required=false) String traceId) { + public String notrace(@RequestHeader(name = TRACE_ID_NAME, required = false) String traceId) { assertThat(traceId).isNull(); return "OK"; } @RequestMapping(value = "/traceid", method = RequestMethod.GET) - public String traceId(@RequestHeader(TRACE_ID_NAME) String traceId, - @RequestHeader(SPAN_ID_NAME) String spanId, + public String traceId(@RequestHeader(TRACE_ID_NAME) String traceId, @RequestHeader(SPAN_ID_NAME) String spanId, @RequestHeader(PARENT_ID_NAME) String parentId) { assertThat(traceId).isNotEmpty(); assertThat(parentId).isNotEmpty();