diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/FeignBeanPostProcessor.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/FeignBeanPostProcessor.java deleted file mode 100644 index 36b09f289..000000000 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/FeignBeanPostProcessor.java +++ /dev/null @@ -1,48 +0,0 @@ -/* - * 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.BeansException; -import org.springframework.beans.factory.config.BeanPostProcessor; - -/** - * Post processor that wraps Feign related classes in their tracing representations. - * - * @author Marcin Grzejszczak - * - * @since 1.0.0 - */ -final class FeignBeanPostProcessor implements BeanPostProcessor { - - private final TraceFeignObjectWrapper traceFeignObjectWrapper; - - FeignBeanPostProcessor(TraceFeignObjectWrapper traceFeignObjectWrapper) { - this.traceFeignObjectWrapper = traceFeignObjectWrapper; - } - - @Override - public Object postProcessBeforeInitialization(Object bean, String beanName) - throws BeansException { - return this.traceFeignObjectWrapper.wrap(bean); - } - - @Override - public Object postProcessAfterInitialization(Object bean, String beanName) - throws BeansException { - return bean; - } -} 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 new file mode 100644 index 000000000..2a8dd6395 --- /dev/null +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignAspect.java @@ -0,0 +1,53 @@ +/* + * 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.aspectj.lang.ProceedingJoinPoint; +import org.aspectj.lang.annotation.Around; +import org.aspectj.lang.annotation.Aspect; +import org.springframework.beans.factory.BeanFactory; + +import feign.Client; +import feign.Request; + +/** + * Aspect for Feign clients so that you can autowire your custom components + * + * @author Marcin Grzejszczak + * @since 1.1.2 + */ +@Aspect +class TraceFeignAspect { + + private final BeanFactory beanFactory; + + TraceFeignAspect(BeanFactory beanFactory) { + this.beanFactory = beanFactory; + } + + @Around("execution (* feign.Client.*(..))") + 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); + } + return pjp.proceed(); + } +} diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignClient.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignClient.java index f0d596892..3d83732f2 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignClient.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignClient.java @@ -25,6 +25,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.beans.factory.BeanFactory; import org.springframework.cloud.sleuth.instrument.web.HttpSpanInjector; +import org.springframework.beans.factory.NoSuchBeanDefinitionException; import org.springframework.cloud.sleuth.Span; import org.springframework.cloud.sleuth.Tracer; import org.springframework.cloud.sleuth.instrument.web.HttpTraceKeysInjector; @@ -54,7 +55,15 @@ class TraceFeignClient implements Client { TraceFeignClient(BeanFactory beanFactory) { this.beanFactory = beanFactory; - this.delegate = new Client.Default(null, null); + this.delegate = client(beanFactory); + } + + private Client client(BeanFactory beanFactory) { + try { + return beanFactory.getBean(Client.class); + } catch (NoSuchBeanDefinitionException e) { + return new Client.Default(null, null); + } } TraceFeignClient(BeanFactory beanFactory, Client delegate) { 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 664f57bc8..b42fbf584 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 @@ -60,7 +60,7 @@ public class TraceFeignClientAutoConfiguration { @Bean @ConditionalOnMissingBean @Scope("prototype") - @ConditionalOnProperty(name = "feign.hystrix.enabled", havingValue = "false", matchIfMissing = false) + @ConditionalOnProperty(name = "feign.hystrix.enabled", havingValue = "false") Feign.Builder feignBuilder(BeanFactory beanFactory) { return SleuthFeignBuilder.builder(beanFactory); } @@ -69,11 +69,6 @@ public class TraceFeignClientAutoConfiguration { @ConditionalOnProperty(name = "spring.sleuth.feign.processor.enabled", matchIfMissing = true) protected static class FeignBeanPostProcessorConfiguration { - @Bean - FeignBeanPostProcessor feignBeanPostProcessor(TraceFeignObjectWrapper traceFeignObjectWrapper) { - return new FeignBeanPostProcessor(traceFeignObjectWrapper); - } - @Bean FeignContextBeanPostProcessor feignContextBeanPostProcessor(BeanFactory beanFactory) { return new FeignContextBeanPostProcessor(beanFactory); @@ -84,4 +79,9 @@ public class TraceFeignClientAutoConfiguration { TraceFeignObjectWrapper traceFeignObjectWrapper(BeanFactory beanFactory) { return new TraceFeignObjectWrapper(beanFactory); } + + @Bean + TraceFeignAspect traceFeignAspect(BeanFactory beanFactory) { + return new TraceFeignAspect(beanFactory); + } } 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 index fd4655e72..4efc57638 100644 --- 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 @@ -1,19 +1,11 @@ package org.springframework.cloud.sleuth.instrument.web.client.feign; -import java.io.IOException; -import java.lang.invoke.MethodHandles; - -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; 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 org.springframework.cloud.sleuth.Tracer; import feign.Client; -import feign.Request; -import feign.Response; /** * We need to wrap the {@link LoadBalancerFeignClient} into a trace representation @@ -24,31 +16,13 @@ import feign.Response; */ class TraceLoadBalancerFeignClient extends LoadBalancerFeignClient { - private static final Log log = LogFactory.getLog(MethodHandles.lookup().lookupClass()); - - private final BeanFactory beanFactory; - private Tracer tracer; - TraceLoadBalancerFeignClient(Client delegate, CachingSpringLoadBalancerFactory lbClientFactory, SpringClientFactory clientFactory, BeanFactory beanFactory) { super(wrap(delegate, beanFactory), lbClientFactory, clientFactory); - this.beanFactory = beanFactory; - } - - @Override public Response execute(Request request, Request.Options options) - throws IOException { - return super.execute(request, options); } private static Client wrap(Client delegate, BeanFactory beanFactory) { return (Client) new TraceFeignObjectWrapper(beanFactory).wrap(delegate); } - - private Tracer tracer() { - if (this.tracer == null) { - this.tracer = this.beanFactory.getBean(Tracer.class); - } - return this.tracer; - } } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/issues/issue502/Issue502Tests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/issues/issue502/Issue502Tests.java new file mode 100644 index 000000000..d8d6bb3e6 --- /dev/null +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/issues/issue502/Issue502Tests.java @@ -0,0 +1,121 @@ +/* + * 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.issue502; + +import java.io.IOException; +import java.nio.charset.Charset; +import java.util.HashMap; + +import org.junit.After; +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.context.SpringBootTest; +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.trace.TestSpanContextHolder; +import org.springframework.cloud.sleuth.util.ExceptionUtils; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.test.context.junit4.SpringRunner; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RequestMethod; + +import feign.Client; +import feign.Request; +import feign.Response; + +import static org.assertj.core.api.BDDAssertions.then; + +/** + * @author Marcin Grzejszczak + */ +@RunWith(SpringRunner.class) +@SpringBootTest(classes = Application.class, + webEnvironment = SpringBootTest.WebEnvironment.NONE, + properties = {"feign.hystrix.enabled=false"}) +public class Issue502Tests { + + @Autowired MyClient myClient; + @Autowired MyNameRemote myNameRemote; + + @Before + public void open() { + TestSpanContextHolder.removeCurrentSpan(); + ExceptionUtils.setFail(true); + } + + @After + public void close() { + TestSpanContextHolder.removeCurrentSpan(); + } + + @Test + public void should_reuse_custom_feign_client() { + String response = this.myNameRemote.get(); + + then(this.myClient.wasCalled()).isTrue(); + then(response).isEqualTo("foo"); + then(ExceptionUtils.getLastException()).isNull(); + } +} + +@Configuration +@EnableAutoConfiguration +@EnableFeignClients +class Application { + + @Bean + public Client client() { + return new MyClient(); + } + + @Bean + public AlwaysSampler defaultSampler() { + return new AlwaysSampler(); + } + +} + +@FeignClient(name="foo", + url="http://non.existing.url") +interface MyNameRemote { + + @RequestMapping(value = "/", method = RequestMethod.GET) + String get(); +} + +class MyClient implements Client { + + boolean wasCalled; + + @Override public Response execute(Request request, Request.Options options) + throws IOException { + this.wasCalled = true; + return Response.builder() + .body("foo", Charset.forName("UTF-8")) + .headers(new HashMap<>()) + .status(200).build(); + } + + boolean wasCalled() { + return this.wasCalled; + } +}