diff --git a/spring-cloud-sleuth-core/pom.xml b/spring-cloud-sleuth-core/pom.xml index c83c0d53f..3cdd7b410 100644 --- a/spring-cloud-sleuth-core/pom.xml +++ b/spring-cloud-sleuth-core/pom.xml @@ -141,6 +141,11 @@ h2 test + + org.springframework.cloud + spring-cloud-starter-eureka + test + diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceFilter.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceFilter.java index cdf25664c..cf32f75de 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceFilter.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceFilter.java @@ -15,18 +15,18 @@ */ package org.springframework.cloud.sleuth.instrument.web; -import java.io.IOException; -import java.lang.invoke.MethodHandles; -import java.util.ArrayList; -import java.util.Collections; -import java.util.Enumeration; -import java.util.regex.Pattern; import javax.servlet.FilterChain; import javax.servlet.ServletException; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import java.io.IOException; +import java.lang.invoke.MethodHandles; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Enumeration; +import java.util.regex.Pattern; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -126,7 +126,7 @@ public class TraceFilter extends GenericFilterBean { continueSpan(request, spanFromRequest); } if (log.isDebugEnabled()) { - log.debug("Received a request to uri [" + uri + "] that should be skipped [" + skip + "]"); + log.debug("Received a request to uri [" + uri + "] that should not be sampled [" + skip + "]"); } // in case of a response with exception status a exception controller will close the span if (!httpStatusSuccessful(response) && isSpanContinued(request)) { 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 dae2e6b06..4d5b4f54e 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 @@ -1,6 +1,9 @@ package org.springframework.cloud.sleuth.instrument.web.client.feign; import org.springframework.beans.factory.BeanFactory; +import org.springframework.cloud.netflix.feign.ribbon.CachingSpringLoadBalancerFactory; +import org.springframework.cloud.netflix.feign.ribbon.LoadBalancerFeignClient; +import org.springframework.cloud.netflix.ribbon.SpringClientFactory; import feign.Client; @@ -14,14 +17,39 @@ final class TraceFeignObjectWrapper { private final BeanFactory beanFactory; + private CachingSpringLoadBalancerFactory cachingSpringLoadBalancerFactory; + private SpringClientFactory springClientFactory; + TraceFeignObjectWrapper(BeanFactory beanFactory) { this.beanFactory = beanFactory; } Object wrap(Object bean) { if (bean instanceof Client && !(bean instanceof TraceFeignClient)) { + if (bean instanceof LoadBalancerFeignClient) { + LoadBalancerFeignClient client = ((LoadBalancerFeignClient) bean); + return new TraceLoadBalancerFeignClient( + client.getDelegate(), factory(), + clientFactory(), this.beanFactory); + } return new TraceFeignClient(this.beanFactory, (Client) bean); } return bean; } + + CachingSpringLoadBalancerFactory factory() { + if (this.cachingSpringLoadBalancerFactory == null) { + this.cachingSpringLoadBalancerFactory = this.beanFactory + .getBean(CachingSpringLoadBalancerFactory.class); + } + return this.cachingSpringLoadBalancerFactory; + } + + SpringClientFactory clientFactory() { + if (this.springClientFactory == null) { + this.springClientFactory = this.beanFactory + .getBean(SpringClientFactory.class); + } + return this.springClientFactory; + } } 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 new file mode 100644 index 000000000..ec4ab9129 --- /dev/null +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceLoadBalancerFeignClient.java @@ -0,0 +1,28 @@ +package org.springframework.cloud.sleuth.instrument.web.client.feign; + +import org.springframework.beans.factory.BeanFactory; +import org.springframework.cloud.netflix.feign.ribbon.CachingSpringLoadBalancerFactory; +import org.springframework.cloud.netflix.feign.ribbon.LoadBalancerFeignClient; +import org.springframework.cloud.netflix.ribbon.SpringClientFactory; + +import feign.Client; + +/** + * We need to wrap the {@link LoadBalancerFeignClient} into a trace representation + * due to casts in {@link org.springframework.cloud.netflix.feign.FeignClientFactoryBean}. + * + * @author Marcin Grzejszczak + * @since 1.0.7 + */ +class TraceLoadBalancerFeignClient extends LoadBalancerFeignClient { + + public TraceLoadBalancerFeignClient(Client delegate, + CachingSpringLoadBalancerFactory lbClientFactory, + SpringClientFactory clientFactory, BeanFactory beanFactory) { + super(wrap(delegate, beanFactory), lbClientFactory, clientFactory); + } + + private static Client wrap(Client delegate, BeanFactory beanFactory) { + return (Client) new TraceFeignObjectWrapper(beanFactory).wrap(delegate); + } +} diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/issues/issue350/Issue350Tests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/issues/issue350/Issue350Tests.java index da379bd46..8c0c277ea 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/issues/issue350/Issue350Tests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/issues/issue350/Issue350Tests.java @@ -28,6 +28,7 @@ import org.springframework.boot.test.TestRestTemplate; import org.springframework.boot.test.WebIntegrationTest; import org.springframework.cloud.netflix.feign.EnableFeignClients; import org.springframework.cloud.netflix.feign.FeignClient; +import org.springframework.cloud.sleuth.Tracer; import org.springframework.cloud.sleuth.sampler.AlwaysSampler; import org.springframework.cloud.sleuth.util.ExceptionUtils; import org.springframework.context.annotation.Bean; @@ -54,6 +55,7 @@ import static org.assertj.core.api.BDDAssertions.then; public class Issue350Tests { RestTemplate template = new TestRestTemplate(); + @Autowired Tracer tracer; @Before public void setup() { @@ -64,6 +66,7 @@ public class Issue350Tests { public void should_successfully_work_without_hystrix() { this.template.getForEntity("http://localhost:9988/sleuth/test-not-ok", String.class); then(ExceptionUtils.getLastException()).isNull(); + then(this.tracer.getCurrentSpan()).isNull(); } } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/issues/issue362/Issue362Tests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/issues/issue362/Issue362Tests.java index 806b6c65f..dcfd729fa 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/issues/issue362/Issue362Tests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/issues/issue362/Issue362Tests.java @@ -30,6 +30,7 @@ import org.springframework.boot.test.SpringApplicationConfiguration; import org.springframework.boot.test.WebIntegrationTest; import org.springframework.cloud.netflix.feign.EnableFeignClients; import org.springframework.cloud.netflix.feign.FeignClient; +import org.springframework.cloud.sleuth.Tracer; import org.springframework.cloud.sleuth.assertions.SleuthAssertions; import org.springframework.cloud.sleuth.sampler.AlwaysSampler; import org.springframework.cloud.sleuth.util.ExceptionUtils; @@ -64,6 +65,7 @@ public class Issue362Tests { RestTemplate template = new RestTemplate(); @Autowired FeignComponentAsserter feignComponentAsserter; + @Autowired Tracer tracer; @Before public void setup() { @@ -77,8 +79,9 @@ public class Issue362Tests { ResponseEntity response = this.template.getForEntity(securedURl, String.class); - SleuthAssertions.then(response.getBody()).isEqualTo("I'm OK"); + then(response.getBody()).isEqualTo("I'm OK"); then(ExceptionUtils.getLastException()).isNull(); + then(this.tracer.getCurrentSpan()).isNull(); } @Test @@ -93,6 +96,7 @@ public class Issue362Tests { then(ExceptionUtils.getLastException()).isNull(); then(this.feignComponentAsserter.executedComponents) .containsEntry(ErrorDecoder.class, true); + then(this.tracer.getCurrentSpan()).isNull(); } } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/issues/issue393/Issue393Tests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/issues/issue393/Issue393Tests.java new file mode 100644 index 000000000..ac03cb6aa --- /dev/null +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/issues/issue393/Issue393Tests.java @@ -0,0 +1,154 @@ +/* + * Copyright 2013-2016 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.sleuth.instrument.web.client.feign.issues.issue393; + +import java.util.Date; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ExecutionException; + +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import com.fasterxml.jackson.databind.annotation.JsonPOJOBuilder; + +import org.junit.After; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; +import org.springframework.boot.test.SpringApplicationConfiguration; +import org.springframework.boot.test.WebIntegrationTest; +import org.springframework.cloud.client.discovery.EnableDiscoveryClient; +import org.springframework.cloud.netflix.feign.EnableFeignClients; +import org.springframework.cloud.netflix.feign.FeignClient; +import org.springframework.cloud.sleuth.Tracer; +import org.springframework.cloud.sleuth.assertions.SleuthAssertions; +import org.springframework.cloud.sleuth.sampler.AlwaysSampler; +import org.springframework.cloud.sleuth.trace.TestSpanContextHolder; +import org.springframework.cloud.sleuth.util.ExceptionUtils; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; +import org.springframework.test.context.TestPropertySource; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RequestMethod; +import org.springframework.web.bind.annotation.ResponseStatus; +import org.springframework.web.bind.annotation.RestController; +import org.springframework.web.client.RestTemplate; + +import feign.Logger; +import feign.Response; +import feign.RetryableException; +import feign.Retryer; +import feign.codec.ErrorDecoder; + +import static org.assertj.core.api.Assertions.fail; +import static org.assertj.core.api.Assertions.registerCustomDateFormat; +import static org.assertj.core.api.Assertions.registerFormatterForType; +import static org.assertj.core.api.BDDAssertions.then; + +/** + * @author Marcin Grzejszczak + */ +@RunWith(SpringJUnit4ClassRunner.class) +@SpringApplicationConfiguration(Application.class) +@WebIntegrationTest +@TestPropertySource(properties = {"spring.application.name=demo-feign-uri", + "server.port=9978"}) +public class Issue393Tests { + + RestTemplate template = new RestTemplate(); + @Autowired Tracer tracer; + + @Before + public void open() { + TestSpanContextHolder.removeCurrentSpan(); + ExceptionUtils.setFail(true); + } + + @After + public void close() { + TestSpanContextHolder.removeCurrentSpan(); + } + + @Test + public void should_successfully_work_when_service_discovery_is_on_classpath_and_feign_uses_url() { + String url = "http://localhost:9978/hello/mikesarver"; + + ResponseEntity response = this.template.getForEntity(url, String.class); + + then(response.getBody()).isEqualTo("mikesarver foo"); + then(ExceptionUtils.getLastException()).isNull(); + then(this.tracer.getCurrentSpan()).isNull(); + } +} + +@Configuration +@EnableAutoConfiguration +@EnableFeignClients +@EnableDiscoveryClient +class Application { + + @Bean + public DemoController demoController(MyNameRemote myNameRemote) { + return new DemoController(myNameRemote); + } + + @Bean + public feign.Logger.Level feignLoggerLevel() { + return feign.Logger.Level.BASIC; + } + + @Bean + public AlwaysSampler defaultSampler() { + return new AlwaysSampler(); + } + +} + +@FeignClient(name="no-name", + url="http://localhost:9978") +interface MyNameRemote { + + @RequestMapping(value = "/name/{id}", method = RequestMethod.GET) + String getName(@PathVariable("id") String id); +} + +@RestController +class DemoController { + + private final MyNameRemote myNameRemote; + + public DemoController(MyNameRemote myNameRemote) { + this.myNameRemote = myNameRemote; + } + + @RequestMapping(value = "/hello/{name}") + public String getHello(@PathVariable("name") String name) { + return myNameRemote.getName(name) + " foo"; + } + + @RequestMapping(value = "/name/{name}") + public String getName(@PathVariable("name") String name) { + return name; + } +}