diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/FeignEventPublisher.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/FeignEventPublisher.java index 143bd7dd3..d66ccba03 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/FeignEventPublisher.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/FeignEventPublisher.java @@ -16,6 +16,7 @@ package org.springframework.cloud.sleuth.instrument.web.client.feign; +import org.springframework.beans.factory.BeanFactory; import org.springframework.cloud.sleuth.Span; import org.springframework.cloud.sleuth.Tracer; @@ -30,18 +31,26 @@ abstract class FeignEventPublisher { private final FeignRequestContext feignRequestContext = FeignRequestContext.getInstance(); - private final Tracer tracer; + protected final BeanFactory beanFactory; + private Tracer tracer; - protected FeignEventPublisher(Tracer tracer) { - this.tracer = tracer; + protected FeignEventPublisher(BeanFactory beanFactory) { + this.beanFactory = beanFactory; } protected void finish() { Span span = this.feignRequestContext.getCurrentSpan(); if (span != null) { span.logEvent(Span.CLIENT_RECV); - this.tracer.close(span); + getTracer().close(span); this.feignRequestContext.clearContext(); } } + + Tracer getTracer() { + if (this.tracer == null) { + this.tracer = this.beanFactory.getBean(Tracer.class); + } + return this.tracer; + } } 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 59fae1a2d..27c608602 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 @@ -16,8 +16,7 @@ package org.springframework.cloud.sleuth.instrument.web.client.feign; -import org.springframework.cloud.sleuth.Tracer; -import org.springframework.cloud.sleuth.instrument.web.HttpTraceKeysInjector; +import org.springframework.beans.factory.BeanFactory; import feign.Feign; import feign.hystrix.HystrixFeign; @@ -35,11 +34,11 @@ final class SleuthFeignBuilder { private SleuthFeignBuilder() {} - static Feign.Builder builder(Tracer tracer, HttpTraceKeysInjector keysInjector) { + static Feign.Builder builder(BeanFactory beanFactory) { return HystrixFeign.builder() - .client(new TraceFeignClient(tracer, keysInjector)) - .retryer(new TraceFeignRetryer(tracer)) - .decoder(new TraceFeignDecoder(tracer)) - .errorDecoder(new TraceFeignErrorDecoder(tracer)); + .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/TraceFeignClient.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignClient.java index 0c4af84be..d0807d197 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 @@ -20,7 +20,7 @@ import java.io.IOException; import java.net.URI; import java.util.Objects; -import org.springframework.cloud.sleuth.Tracer; +import org.springframework.beans.factory.BeanFactory; import org.springframework.cloud.sleuth.instrument.web.HttpTraceKeysInjector; import feign.Client; @@ -39,18 +39,16 @@ import feign.RetryableException; final class TraceFeignClient extends FeignEventPublisher implements Client { private final Client delegate; - private final HttpTraceKeysInjector keysInjector; + private HttpTraceKeysInjector keysInjector; - TraceFeignClient(Tracer tracer, HttpTraceKeysInjector keysInjector) { - super(tracer); + TraceFeignClient(BeanFactory beanFactory) { + super(beanFactory); this.delegate = new Client.Default(null, null); - this.keysInjector = keysInjector; } - TraceFeignClient(Tracer tracer, Client delegate, HttpTraceKeysInjector keysInjector) { - super(tracer); + TraceFeignClient(BeanFactory beanFactory, Client delegate) { + super(beanFactory); this.delegate = delegate; - this.keysInjector = keysInjector; } @Override @@ -81,7 +79,14 @@ final class TraceFeignClient extends FeignEventPublisher implements Client { */ private void addRequestTags(Request request) { URI uri = URI.create(request.url()); - this.keysInjector.addRequestTags(uri.toString(), uri.getHost(), uri.getPath(), + getKeysInjector().addRequestTags(uri.toString(), uri.getHost(), uri.getPath(), request.method(), request.headers()); } + + HttpTraceKeysInjector getKeysInjector() { + if (this.keysInjector == null) { + this.keysInjector = this.beanFactory.getBean(HttpTraceKeysInjector.class); + } + return this.keysInjector; + } } 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 93fa64c3e..74d0cb7d4 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 @@ -40,7 +40,6 @@ import org.springframework.cloud.netflix.feign.support.SpringDecoder; import org.springframework.cloud.sleuth.SpanInjector; import org.springframework.cloud.sleuth.Tracer; import org.springframework.cloud.sleuth.instrument.hystrix.SleuthHystrixAutoConfiguration; -import org.springframework.cloud.sleuth.instrument.web.HttpTraceKeysInjector; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Primary; @@ -76,8 +75,8 @@ public class TraceFeignClientAutoConfiguration { @Scope("prototype") @ConditionalOnClass(HystrixCommand.class) @ConditionalOnProperty(name = "feign.hystrix.enabled", matchIfMissing = true) - Feign.Builder feignHystrixBuilder(Tracer tracer, HttpTraceKeysInjector keysInjector) { - return SleuthFeignBuilder.builder(tracer, keysInjector); + Feign.Builder feignHystrixBuilder(BeanFactory beanFactory) { + return SleuthFeignBuilder.builder(beanFactory); } @Configuration @@ -97,8 +96,8 @@ public class TraceFeignClientAutoConfiguration { @Bean @Primary - Decoder feignDecoder(final Tracer tracer) { - return new TraceFeignDecoder(tracer, + Decoder feignDecoder(BeanFactory beanFactory) { + return new TraceFeignDecoder(beanFactory, new ResponseEntityDecoder(new SpringDecoder(this.messageConverters)) { @Override public Object decode(Response response, Type type) diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignDecoder.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignDecoder.java index d1d304066..e4d130154 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignDecoder.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignDecoder.java @@ -19,11 +19,10 @@ package org.springframework.cloud.sleuth.instrument.web.client.feign; import java.io.IOException; import java.lang.reflect.Type; -import org.springframework.cloud.sleuth.Tracer; +import org.springframework.beans.factory.BeanFactory; import feign.FeignException; import feign.Response; -import feign.codec.DecodeException; import feign.codec.Decoder; /** @@ -37,19 +36,19 @@ final class TraceFeignDecoder extends FeignEventPublisher implements Decoder { private final Decoder delegate; - TraceFeignDecoder(Tracer tracer) { - super(tracer); + TraceFeignDecoder(BeanFactory beanFactory) { + super(beanFactory); this.delegate = new Decoder.Default(); } - TraceFeignDecoder(Tracer tracer, Decoder delegate) { - super(tracer); + TraceFeignDecoder(BeanFactory beanFactory, Decoder delegate) { + super(beanFactory); this.delegate = delegate; } @Override public Object decode(Response response, Type type) - throws IOException, DecodeException, FeignException { + throws IOException, FeignException { try { return this.delegate.decode(response, type); } finally { diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignErrorDecoder.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignErrorDecoder.java index dc5104901..ec634fe8d 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignErrorDecoder.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignErrorDecoder.java @@ -16,7 +16,7 @@ package org.springframework.cloud.sleuth.instrument.web.client.feign; -import org.springframework.cloud.sleuth.Tracer; +import org.springframework.beans.factory.BeanFactory; import feign.Response; import feign.codec.ErrorDecoder; @@ -32,13 +32,13 @@ final class TraceFeignErrorDecoder extends FeignEventPublisher implements ErrorD private final ErrorDecoder delegate; - TraceFeignErrorDecoder(Tracer tracer) { - super(tracer); + TraceFeignErrorDecoder(BeanFactory beanFactory) { + super(beanFactory); this.delegate = new ErrorDecoder.Default(); } - TraceFeignErrorDecoder(Tracer tracer, ErrorDecoder delegate) { - super(tracer); + TraceFeignErrorDecoder(BeanFactory beanFactory, ErrorDecoder delegate) { + super(beanFactory); this.delegate = delegate; } 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 e4dc0022a..be7bc6a48 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,8 +1,6 @@ package org.springframework.cloud.sleuth.instrument.web.client.feign; import org.springframework.beans.factory.BeanFactory; -import org.springframework.cloud.sleuth.Tracer; -import org.springframework.cloud.sleuth.instrument.web.HttpTraceKeysInjector; import feign.Client; import feign.Retryer; @@ -18,8 +16,6 @@ import feign.codec.ErrorDecoder; final class TraceFeignObjectWrapper { private final BeanFactory beanFactory; - private Tracer tracer; - private HttpTraceKeysInjector keysInjector; TraceFeignObjectWrapper(BeanFactory beanFactory) { this.beanFactory = beanFactory; @@ -27,28 +23,14 @@ final class TraceFeignObjectWrapper { Object wrap(Object bean) { if (bean instanceof Decoder && !(bean instanceof TraceFeignDecoder)) { - return new TraceFeignDecoder(getTracer(), (Decoder) bean); + return new TraceFeignDecoder(this.beanFactory, (Decoder) bean); } else if (bean instanceof Retryer && !(bean instanceof TraceFeignRetryer)) { - return new TraceFeignRetryer(getTracer(), (Retryer) bean); + return new TraceFeignRetryer(this.beanFactory, (Retryer) bean); } else if (bean instanceof Client && !(bean instanceof TraceFeignClient)) { - return new TraceFeignClient(getTracer(), (Client) bean, getHttpTraceKeysInjector()); + return new TraceFeignClient(this.beanFactory, (Client) bean); } else if (bean instanceof ErrorDecoder && !(bean instanceof TraceFeignErrorDecoder)) { - return new TraceFeignErrorDecoder(getTracer(), (ErrorDecoder) bean); + return new TraceFeignErrorDecoder(this.beanFactory, (ErrorDecoder) bean); } return bean; } - - private Tracer getTracer() { - if (this.tracer == null) { - this.tracer = this.beanFactory.getBean(Tracer.class); - } - return this.tracer; - } - - private HttpTraceKeysInjector getHttpTraceKeysInjector() { - if (this.keysInjector == null) { - this.keysInjector = this.beanFactory.getBean(HttpTraceKeysInjector.class); - } - return this.keysInjector; - } } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignRetryer.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignRetryer.java index b0f5b07a2..4fa8cbe4a 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignRetryer.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignRetryer.java @@ -16,6 +16,7 @@ package org.springframework.cloud.sleuth.instrument.web.client.feign; +import org.springframework.beans.factory.BeanFactory; import org.springframework.cloud.sleuth.Tracer; import feign.RetryableException; @@ -33,35 +34,43 @@ import feign.Retryer; */ final class TraceFeignRetryer implements Retryer { - private final Tracer tracer; + private final BeanFactory beanFactory; + private Tracer tracer; private final FeignRequestContext feignRequestContext = FeignRequestContext .getInstance(); private final Retryer delegate; - TraceFeignRetryer(Tracer tracer) { - this(tracer, new Retryer.Default()); + TraceFeignRetryer(BeanFactory beanFactory) { + this(beanFactory, new Retryer.Default()); } - TraceFeignRetryer(Tracer tracer, Retryer delegate) { - this.tracer = tracer; + TraceFeignRetryer(BeanFactory beanFactory, Retryer delegate) { + this.beanFactory = beanFactory; this.delegate = delegate; } @Override public void continueOrPropagate(RetryableException e) { try { - this.feignRequestContext.putSpan(this.tracer.getCurrentSpan(), true); - this.tracer.getCurrentSpan().logEvent("feign.retry"); + this.feignRequestContext.putSpan(getTracer().getCurrentSpan(), true); + getTracer().getCurrentSpan().logEvent("feign.retry"); this.delegate.continueOrPropagate(e); } catch (RetryableException e2) { - this.tracer.close(this.tracer.getCurrentSpan()); + getTracer().close(getTracer().getCurrentSpan()); throw e2; } } @Override public Retryer clone() { - return new TraceFeignRetryer(this.tracer, this.delegate.clone()); + return new TraceFeignRetryer(this.beanFactory, this.delegate.clone()); + } + + Tracer getTracer() { + if (this.tracer == null) { + this.tracer = this.beanFactory.getBean(Tracer.class); + } + return this.tracer; } } \ No newline at end of file diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/WebClientTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/WebClientTests.java index 32931d4eb..8222c0431 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/WebClientTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/WebClientTests.java @@ -58,6 +58,7 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.http.HttpHeaders; import org.springframework.http.ResponseEntity; +import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.junit4.rules.SpringClassRule; import org.springframework.test.context.junit4.rules.SpringMethodRule; import org.springframework.web.bind.annotation.RequestHeader; @@ -78,6 +79,7 @@ import static org.springframework.cloud.sleuth.assertions.SleuthAssertions.then; @RunWith(JUnitParamsRunner.class) @SpringApplicationConfiguration(classes = { WebClientTests.TestConfiguration.class }) @WebIntegrationTest(value = { "spring.application.name=fooservice" }, randomPort = true) +@DirtiesContext public class WebClientTests { @ClassRule public static final SpringClassRule SCR = new SpringClassRule(); 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 new file mode 100644 index 000000000..dd891e000 --- /dev/null +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/issues/Issue307Tests.java @@ -0,0 +1,111 @@ +/* + * 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.ArrayList; +import java.util.List; + +import com.netflix.hystrix.contrib.javanica.annotation.HystrixCommand; + +import org.apache.log4j.Level; +import org.apache.log4j.Logger; +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.cloud.client.circuitbreaker.EnableCircuitBreaker; +import org.springframework.cloud.netflix.feign.EnableFeignClients; +import org.springframework.cloud.netflix.feign.FeignClient; +import org.springframework.cloud.sleuth.sampler.AlwaysSampler; +import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.stereotype.Component; +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.RestController; +import org.springframework.web.client.RestTemplate; + +public class Issue307Tests { + + @Test + public void should_start_context() { + try (ConfigurableApplicationContext applicationContext = SpringApplication + .run(SleuthSampleApplication.class, "")) { + } + } +} + +@SpringBootApplication +@RestController +@EnableFeignClients +@EnableCircuitBreaker +class SleuthSampleApplication { + + private static final Logger LOG = Logger.getLogger(SleuthSampleApplication.class.getName()); + + @Autowired + private RestTemplate restTemplate; + + @Autowired + private ParticipantsBean participantsBean; + + @Bean + public RestTemplate getRestTemplate() { + return new RestTemplate(); + } + + @Bean + public AlwaysSampler defaultSampler() { + return new AlwaysSampler(); + } + + @RequestMapping("/") + public String home() { + LOG.log(Level.INFO, "you called home"); + return "Hello World"; + } + + @RequestMapping("/callhome") + public String callHome() { + LOG.log(Level.INFO, "calling home"); + return restTemplate.getForObject("http://localhost:8080", String.class); + } +} + +@Component +class ParticipantsBean { + @Autowired + private ParticipantsClient participantsClient; + + @HystrixCommand(fallbackMethod = "defaultParticipants") + public List getParticipants(String raceId) { + return participantsClient.getParticipants(raceId); + } + + public List defaultParticipants(String raceId) { + return new ArrayList<>(); + } +} + +@FeignClient("participants") +interface ParticipantsClient { + + @RequestMapping(method = RequestMethod.GET, value="/races/{raceId}") + List getParticipants(@PathVariable("raceId") String raceId); + +} diff --git a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-messaging/src/test/java/integration/MessagingApplicationTests.java b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-messaging/src/test/java/integration/MessagingApplicationTests.java index 516299e70..a2456630e 100644 --- a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-messaging/src/test/java/integration/MessagingApplicationTests.java +++ b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-messaging/src/test/java/integration/MessagingApplicationTests.java @@ -31,6 +31,7 @@ import org.springframework.boot.test.WebIntegrationTest; import org.springframework.cloud.sleuth.zipkin.ZipkinSpanReporter; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.TestPropertySource; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; @@ -46,6 +47,7 @@ import static org.assertj.core.api.BDDAssertions.then; @SpringApplicationConfiguration(classes = { IntegrationSpanCollectorConfig.class, SampleMessagingApplication.class }) @WebIntegrationTest @TestPropertySource(properties="sample.zipkin.enabled=true") +@DirtiesContext public class MessagingApplicationTests extends AbstractIntegrationTest { private static int port = 3381;