diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/SleuthFeignBuilder.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/SleuthFeignBuilder.java index 27c608602..cf4513062 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/SleuthFeignBuilder.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/SleuthFeignBuilder.java @@ -19,11 +19,9 @@ package org.springframework.cloud.sleuth.instrument.web.client.feign; import org.springframework.beans.factory.BeanFactory; import feign.Feign; -import feign.hystrix.HystrixFeign; /** - * Contains {@link feign.Feign.Builder} implementation that delegates execution - * {@link feign.hystrix.HystrixFeign} with tracing components + * Contains {@link feign.Feign.Builder} implementation with tracing components * that close spans on exceptions / success and continues them on retries. * * @author Marcin Grzejszczak @@ -35,7 +33,7 @@ final class SleuthFeignBuilder { private SleuthFeignBuilder() {} static Feign.Builder builder(BeanFactory beanFactory) { - return HystrixFeign.builder() + return Feign.builder() .client(new TraceFeignClient(beanFactory)) .retryer(new TraceFeignRetryer(beanFactory)) .decoder(new TraceFeignDecoder(beanFactory)) diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/SleuthHystrixFeignBuilder.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/SleuthHystrixFeignBuilder.java new file mode 100644 index 000000000..e7f9c59f1 --- /dev/null +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/SleuthHystrixFeignBuilder.java @@ -0,0 +1,44 @@ +/* + * 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; + +import org.springframework.beans.factory.BeanFactory; + +import feign.Feign; +import feign.hystrix.HystrixFeign; + +/** + * Contains {@link Feign.Builder} implementation that delegates execution + * {@link HystrixFeign} with tracing components + * that close spans on exceptions / success and continues them on retries. + * + * @author Marcin Grzejszczak + * + * @since 1.0.4 + */ +final class SleuthHystrixFeignBuilder { + + private SleuthHystrixFeignBuilder() {} + + static Feign.Builder builder(BeanFactory beanFactory) { + return HystrixFeign.builder() + .client(new TraceFeignClient(beanFactory)) + .retryer(new TraceFeignRetryer(beanFactory)) + .decoder(new TraceFeignDecoder(beanFactory)) + .errorDecoder(new TraceFeignErrorDecoder(beanFactory)); + } +} diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignClientAutoConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignClientAutoConfiguration.java index 504af1167..8b7afb920 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignClientAutoConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignClientAutoConfiguration.java @@ -28,6 +28,7 @@ import org.springframework.boot.autoconfigure.AutoConfigureAfter; import org.springframework.boot.autoconfigure.AutoConfigureBefore; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; +import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.autoconfigure.web.HttpMessageConverters; import org.springframework.cloud.netflix.feign.FeignAutoConfiguration; @@ -72,6 +73,14 @@ public class TraceFeignClientAutoConfiguration { @ConditionalOnClass(HystrixCommand.class) @ConditionalOnProperty(name = "feign.hystrix.enabled", matchIfMissing = true) Feign.Builder feignHystrixBuilder(BeanFactory beanFactory) { + return SleuthHystrixFeignBuilder.builder(beanFactory); + } + + @Bean + @ConditionalOnMissingBean + @Scope("prototype") + @ConditionalOnProperty(name = "feign.hystrix.enabled", havingValue = "false", matchIfMissing = false) + Feign.Builder feignBuilder(BeanFactory beanFactory) { return SleuthFeignBuilder.builder(beanFactory); } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/issues/Issue307Tests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/issues/Issue307Tests.java index 42d58f80e..c0d14ae8c 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/issues/Issue307Tests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/issues/Issue307Tests.java @@ -25,7 +25,7 @@ import org.junit.Before; import org.junit.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.SpringApplication; -import org.springframework.boot.autoconfigure.SpringBootApplication; +import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.cloud.client.circuitbreaker.EnableCircuitBreaker; import org.springframework.cloud.netflix.feign.EnableFeignClients; import org.springframework.cloud.netflix.feign.FeignClient; @@ -34,6 +34,7 @@ import org.springframework.cloud.sleuth.trace.TestSpanContextHolder; import org.springframework.cloud.sleuth.util.ExceptionUtils; import org.springframework.context.ConfigurableApplicationContext; import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Import; import org.springframework.core.env.Environment; import org.springframework.stereotype.Component; import org.springframework.web.bind.annotation.PathVariable; @@ -62,7 +63,8 @@ public class Issue307Tests { } } -@SpringBootApplication +@EnableAutoConfiguration +@Import({ParticipantsBean.class, ParticipantsClient.class}) @RestController @EnableFeignClients @EnableCircuitBreaker diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/issues/Issue350Tests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/issues/Issue350Tests.java new file mode 100644 index 000000000..fd26a1b00 --- /dev/null +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/issues/Issue350Tests.java @@ -0,0 +1,141 @@ +/* + * 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; + +import java.util.concurrent.ExecutionException; + +import org.junit.Before; +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.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.sampler.AlwaysSampler; +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.test.context.TestPropertySource; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.ResponseStatus; +import org.springframework.web.bind.annotation.RestController; +import org.springframework.web.client.RestTemplate; + +import feign.Logger; + +import static org.assertj.core.api.BDDAssertions.then; + +/** + * @author Marcin Grzejszczak + */ +@RunWith(SpringJUnit4ClassRunner.class) +@SpringApplicationConfiguration(classes = Application.class) +@WebIntegrationTest +@TestPropertySource(properties = {"ribbon.eureka.enabled=false", "feign.hystrix.enabled=false", "server.port=9988"}) +public class Issue350Tests { + + RestTemplate template = new TestRestTemplate(); + + @Before + public void setup() { + ExceptionUtils.setFail(true); + } + + @Test + public void should_successfully_work_without_hystrix() { + this.template.getForEntity("http://localhost:9988/sleuth/test-not-ok", String.class); + then(ExceptionUtils.getLastException()).isNull(); + } +} + +@Configuration +@EnableAutoConfiguration +@EnableFeignClients(basePackageClasses = {SleuthTestController.class}) +class Application { + + @Bean + public ServiceTestController serviceTestController() { + return new ServiceTestController(); + } + + @Bean + public SleuthTestController sleuthTestController() { + return new SleuthTestController(); + } + + @Bean + public Logger.Level feignLoggerLevel() { + return feign.Logger.Level.FULL; + } + + @Bean + public AlwaysSampler defaultSampler() { + return new AlwaysSampler(); + } +} + +@RestController +@RequestMapping(path = "/service") +class ServiceTestController { + + @RequestMapping("/ok") + public String ok() throws InterruptedException, ExecutionException { + String result = "I'm OK"; + return result; + } + + @RequestMapping("/not-ok") + @ResponseStatus(HttpStatus.NOT_ACCEPTABLE) + public String notOk() throws InterruptedException, ExecutionException { + return "Not OK"; + } +} + +@FeignClient(name="myFeignClient", url="localhost:9988") +interface MyFeignClient { + + @RequestMapping("/service/ok") + String ok(); + + @RequestMapping("/service/not-ok") + String exp(); +} + + +@RestController +@RequestMapping(path = "/sleuth") +class SleuthTestController { + + @Autowired + private MyFeignClient myFeignClient; + + @RequestMapping("/test-ok") + public String ok() throws InterruptedException, ExecutionException { + return myFeignClient.ok(); + } + + @RequestMapping("/test-not-ok") + public String notOk() throws InterruptedException, ExecutionException { + return myFeignClient.exp(); + } +} +