From 19249cdc23a655125a58d022525276e1f74aae09 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Mon, 7 Mar 2016 11:18:45 +0100 Subject: [PATCH] Fixed Feign integration --- .../main/asciidoc/spring-cloud-sleuth.adoc | 12 +- .../SleuthHystrixAutoConfiguration.java | 1 + .../SleuthHystrixConcurrencyStrategy.java | 2 +- .../SleuthHystrixInvocationHandler.java | 100 -------------- .../client/feign/FeignBeanPostProcessor.java | 64 +++++++++ .../web/client/feign/FeignEventPublisher.java | 51 ++++++++ .../web/client/feign/FeignRequestContext.java | 75 +++++++++++ .../web/client/feign/SleuthFeignBuilder.java | 42 ++++++ .../web/client/feign/TraceFeignClient.java | 59 +++++++++ .../TraceFeignClientAutoConfiguration.java | 101 ++++----------- .../web/client/feign/TraceFeignDecoder.java | 59 +++++++++ .../feign/TraceFeignRequestInterceptor.java | 122 ++++++++++++++++++ .../web/client/feign/TraceFeignRetryer.java | 66 ++++++++++ .../main/resources/META-INF/spring.factories | 2 +- .../feign/FeignTraceExceptionTests.java | 98 ++++++++++++++ .../client/{ => feign}/FeignTraceTests.java | 53 +++++++- 16 files changed, 726 insertions(+), 181 deletions(-) delete mode 100644 spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/SleuthHystrixInvocationHandler.java create mode 100644 spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/FeignBeanPostProcessor.java create mode 100644 spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/FeignEventPublisher.java create mode 100644 spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/FeignRequestContext.java create mode 100644 spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/SleuthFeignBuilder.java create mode 100644 spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignClient.java rename spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/{ => feign}/TraceFeignClientAutoConfiguration.java (62%) create mode 100644 spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignDecoder.java create mode 100644 spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignRequestInterceptor.java create mode 100644 spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignRetryer.java create mode 100644 spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/FeignTraceExceptionTests.java rename spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/{ => feign}/FeignTraceTests.java (77%) diff --git a/docs/src/main/asciidoc/spring-cloud-sleuth.adoc b/docs/src/main/asciidoc/spring-cloud-sleuth.adoc index 895a2f3b4..351831ec0 100644 --- a/docs/src/main/asciidoc/spring-cloud-sleuth.adoc +++ b/docs/src/main/asciidoc/spring-cloud-sleuth.adoc @@ -269,4 +269,14 @@ Currently Spring Cloud Sleuth registers very simple metrics related to spans. It's using the http://docs.spring.io/spring-boot/docs/current/reference/html/production-ready-metrics.html#production-ready-recording-metrics[Spring Boot's metrics support] to calculate the number of accepted and dropped spans. Each time a span gets sent to Zipkin the number of accepted spans will increase. If there's an error then -the number of dropped spans will get increased. \ No newline at end of file +the number of dropped spans will get increased. + +== Integrations + +=== Feign + +By default Spring Cloud Sleuth provides integration with feign via the `TraceFeignClientAutoConfiguration`. You can disable it +by setting `spring.sleuth.feign.enabled` to false. + +We're taking care of Feign instrumentation by means of a `FeignBeanPostProcessor`. +You can disable the post processor by providing the `spring.sleuth.feign.processor.enabled` equal to `false`. \ No newline at end of file diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/hystrix/SleuthHystrixAutoConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/hystrix/SleuthHystrixAutoConfiguration.java index 2c7d85773..a699382cd 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/hystrix/SleuthHystrixAutoConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/hystrix/SleuthHystrixAutoConfiguration.java @@ -27,4 +27,5 @@ public class SleuthHystrixAutoConfiguration { SleuthHystrixConcurrencyStrategy sleuthHystrixConcurrencyStrategy(Tracer tracer, TraceKeys traceKeys) { return new SleuthHystrixConcurrencyStrategy(tracer, traceKeys); } + } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/hystrix/SleuthHystrixConcurrencyStrategy.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/hystrix/SleuthHystrixConcurrencyStrategy.java index b880a5428..3ff063c45 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/hystrix/SleuthHystrixConcurrencyStrategy.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/hystrix/SleuthHystrixConcurrencyStrategy.java @@ -7,8 +7,8 @@ import javax.annotation.PreDestroy; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.cloud.sleuth.Span; -import org.springframework.cloud.sleuth.Tracer; import org.springframework.cloud.sleuth.TraceKeys; +import org.springframework.cloud.sleuth.Tracer; import com.netflix.hystrix.strategy.HystrixPlugins; import com.netflix.hystrix.strategy.concurrency.HystrixConcurrencyStrategy; diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/SleuthHystrixInvocationHandler.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/SleuthHystrixInvocationHandler.java deleted file mode 100644 index ea98384ff..000000000 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/SleuthHystrixInvocationHandler.java +++ /dev/null @@ -1,100 +0,0 @@ -/* - * Copyright 2015 Netflix, Inc. - * - * 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; - -import java.lang.reflect.InvocationHandler; -import java.lang.reflect.Method; -import java.util.Map; - -import org.springframework.cloud.sleuth.TraceKeys; -import org.springframework.cloud.sleuth.Tracer; -import org.springframework.cloud.sleuth.instrument.hystrix.TraceCommand; -import org.springframework.util.ReflectionUtils; - -import com.netflix.hystrix.HystrixCommand; -import com.netflix.hystrix.HystrixCommandGroupKey; -import com.netflix.hystrix.HystrixCommandKey; - -import feign.InvocationHandlerFactory; -import feign.InvocationHandlerFactory.MethodHandler; -import feign.Target; - -import static feign.Util.checkNotNull; - -/** - * Wraps {@link HystrixCommand} execution in Sleuth's {@link TraceCommand} - * - * @since 1.0.0 - */ -final class SleuthHystrixInvocationHandler implements InvocationHandler { - - private final Target target; - private final Map dispatch; - private final Tracer tracer; - private final TraceKeys traceKeys; - - SleuthHystrixInvocationHandler(Target target, Map dispatch, - Tracer tracer, TraceKeys traceKeys) { - this.tracer = checkNotNull(tracer, "traceManager"); - this.target = checkNotNull(target, "target"); - this.dispatch = checkNotNull(dispatch, "dispatch"); - this.traceKeys = checkNotNull(traceKeys, "traceKeys"); - } - - @Override public Object invoke(final Object proxy, final Method method, - final Object[] args) throws Throwable { - String groupKey = this.target.name(); - String commandKey = method.getName(); - HystrixCommand.Setter setter = HystrixCommand.Setter - .withGroupKey(HystrixCommandGroupKey.Factory.asKey(groupKey)) - .andCommandKey(HystrixCommandKey.Factory.asKey(commandKey)); - HystrixCommand hystrixCommand = new TraceCommand(this.tracer, this.traceKeys, - setter) { - @Override public Object doRun() throws Exception { - try { - return SleuthHystrixInvocationHandler.this.dispatch.get(method) - .invoke(args); - } - catch (Throwable throwable) { - ReflectionUtils.rethrowException(throwable); - } - return null; - } - }; - if (HystrixCommand.class.isAssignableFrom(method.getReturnType())) { - return hystrixCommand; - } - return hystrixCommand.execute(); - } - - static final class Factory implements InvocationHandlerFactory { - - private final Tracer tracer; - private final TraceKeys traceKeys; - - public Factory(Tracer tracer, TraceKeys traceKeys) { - this.tracer = tracer; - this.traceKeys = traceKeys; - } - - @Override public InvocationHandler create( - @SuppressWarnings("rawtypes") Target target, - Map dispatch) { - return new SleuthHystrixInvocationHandler(target, dispatch, this.tracer, - this.traceKeys); - } - } -} \ No newline at end of file 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 new file mode 100644 index 000000000..5f51d65f8 --- /dev/null +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/FeignBeanPostProcessor.java @@ -0,0 +1,64 @@ +/* + * 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; +import org.springframework.cloud.sleuth.Tracer; +import org.springframework.context.ApplicationEventPublisher; + +import feign.Client; +import feign.Retryer; +import feign.codec.Decoder; + +/** + * Post processor that wraps Feign related classes {@link Decoder}, + * {@link Retryer} + * + * @author Marcin Grzejszczak + * + * @since 1.0.0 + */ +class FeignBeanPostProcessor implements BeanPostProcessor { + + private final ApplicationEventPublisher publisher; + private final Tracer tracer; + + FeignBeanPostProcessor(ApplicationEventPublisher publisher, Tracer tracer) { + this.publisher = publisher; + this.tracer = tracer; + } + + @Override + public Object postProcessBeforeInitialization(Object bean, String beanName) + throws BeansException { + if (bean instanceof Decoder && !(bean instanceof TraceFeignDecoder)) { + return new TraceFeignDecoder(this.publisher, this.tracer, (Decoder) bean); + } else if (bean instanceof Retryer && !(bean instanceof TraceFeignRetryer)) { + return new TraceFeignRetryer(this.tracer, (Retryer) bean); + } else if (bean instanceof Client && !(bean instanceof TraceFeignClient)) { + return new TraceFeignClient(this.publisher, this.tracer, (Client) bean); + } + return 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/FeignEventPublisher.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/FeignEventPublisher.java new file mode 100644 index 000000000..3a4570db0 --- /dev/null +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/FeignEventPublisher.java @@ -0,0 +1,51 @@ +/* + * 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.cloud.sleuth.Span; +import org.springframework.cloud.sleuth.Tracer; +import org.springframework.cloud.sleuth.event.ClientReceivedEvent; +import org.springframework.context.ApplicationEventPublisher; + +/** + * Abstract class for publishing {@link org.springframework.cloud.sleuth.event.ClientReceivedEvent} + * + * @author Marcin Grzejszczak + * + * @since 1.0.0 + */ +abstract class FeignEventPublisher { + + private final FeignRequestContext feignRequestContext = FeignRequestContext.getInstance(); + + private final ApplicationEventPublisher publisher; + private final Tracer tracer; + + protected FeignEventPublisher(ApplicationEventPublisher publisher, Tracer tracer) { + this.publisher = publisher; + this.tracer = tracer; + } + + protected void finish() { + Span span = this.feignRequestContext.getCurrentSpan(); + if (span != null) { + this.publisher.publishEvent(new ClientReceivedEvent(this, span)); + this.tracer.close(span); + this.feignRequestContext.clearContext(); + } + } +} diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/FeignRequestContext.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/FeignRequestContext.java new file mode 100644 index 000000000..b30d4c2a4 --- /dev/null +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/FeignRequestContext.java @@ -0,0 +1,75 @@ +/* + * 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.cloud.sleuth.Span; + +/** + * Class that holds the information for the span processed by the current + * request. It also knows whether the request has already been retried. + * + * The implementation works on a {@link ThreadLocal} thus is thread-safe. + * + * @author Marcin Grzejszczak + * + * @since 1.0.0 + */ +class FeignRequestContext { + + private static final FeignRequestContext INSTANCE = new FeignRequestContext(); + + private FeignRequestContext() {} + + private static final ThreadLocal THREAD_LOCAL = new ThreadLocal<>(); + + private static class SpanHolder { + final Span span; + final boolean retried; + + private SpanHolder(Span span, boolean retried) { + this.span = span; + this.retried = retried; + } + } + + boolean hasSpanInProcess() { + return THREAD_LOCAL.get() != null; + } + + Span getCurrentSpan() { + if (hasSpanInProcess()) { + return THREAD_LOCAL.get().span; + } + return null; + } + + boolean wasSpanRetried() { + return hasSpanInProcess() && THREAD_LOCAL.get().retried; + } + + void putSpan(Span span, boolean retried) { + THREAD_LOCAL.set(new SpanHolder(span, retried)); + } + + void clearContext() { + THREAD_LOCAL.remove(); + } + + static FeignRequestContext getInstance() { + return INSTANCE; + } +} 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 new file mode 100644 index 000000000..d80ffe53c --- /dev/null +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/SleuthFeignBuilder.java @@ -0,0 +1,42 @@ +/* + * 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.cloud.sleuth.Tracer; +import org.springframework.context.ApplicationEventPublisher; + +import feign.Feign; +import feign.hystrix.HystrixFeign; + +/** + * Contains {@link feign.Feign.Builder} implementation that delegates execution + * {@link feign.hystrix.HystrixFeign} with custom retryer and decoder + * that close spans on exceptions / success and continues them on retries. + * + * @author Marcin Grzejszczak + * + * @since 1.0.0 + */ +class SleuthFeignBuilder { + + static Feign.Builder builder(ApplicationEventPublisher publisher, Tracer tracer) { + return HystrixFeign.builder() + .client(new TraceFeignClient(publisher, tracer)) + .retryer(new TraceFeignRetryer(tracer)) + .decoder(new TraceFeignDecoder(publisher, tracer)); + } +} 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 new file mode 100644 index 000000000..00bc76ce2 --- /dev/null +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignClient.java @@ -0,0 +1,59 @@ +/* + * 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 java.io.IOException; +import java.util.Objects; + +import org.springframework.cloud.sleuth.Tracer; +import org.springframework.context.ApplicationEventPublisher; + +import feign.Client; +import feign.Request; +import feign.Response; + +/** + * A Feign Client that closes a Span if there is no response body. + * In other cases Span will not get closed cause the Decoder will not get called + * + * @author Marcin Grzejszczak + * + * @since 1.0.0 + */ +class TraceFeignClient extends FeignEventPublisher implements Client { + + private final Client delegate; + + TraceFeignClient(ApplicationEventPublisher publisher, Tracer tracer) { + super(publisher, tracer); + this.delegate = new Client.Default(null, null); + } + + TraceFeignClient(ApplicationEventPublisher publisher, Tracer tracer, Client delegate) { + super(publisher, tracer); + this.delegate = delegate; + } + + @Override + public Response execute(Request request, Request.Options options) throws IOException { + Response response = this.delegate.execute(request, options); + if (response.body() == null || (response.body() != null && Objects.equals(response.body().length(), 0))) { + finish(); + } + return response; + } +} 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/feign/TraceFeignClientAutoConfiguration.java similarity index 62% rename from spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceFeignClientAutoConfiguration.java rename to spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignClientAutoConfiguration.java index 424320998..760e5bd26 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/feign/TraceFeignClientAutoConfiguration.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2015 the original author or authors. + * 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. @@ -14,13 +14,12 @@ * limitations under the License. */ -package org.springframework.cloud.sleuth.instrument.web.client; +package org.springframework.cloud.sleuth.instrument.web.client.feign; import static java.util.Collections.singletonList; import java.io.IOException; import java.lang.reflect.Type; -import java.net.URI; import java.util.Collection; import java.util.HashMap; import java.util.Map; @@ -30,7 +29,6 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.AutoConfigureAfter; import org.springframework.boot.autoconfigure.AutoConfigureBefore; 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; @@ -38,12 +36,8 @@ 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.Tracer; -import org.springframework.cloud.sleuth.event.ClientReceivedEvent; -import org.springframework.cloud.sleuth.event.ClientSentEvent; import org.springframework.cloud.sleuth.TraceKeys; import org.springframework.cloud.sleuth.instrument.hystrix.SleuthHystrixAutoConfiguration; -import org.springframework.cloud.sleuth.instrument.hystrix.SleuthHystrixConcurrencyStrategy; -import org.springframework.context.ApplicationEvent; import org.springframework.context.ApplicationEventPublisher; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -57,10 +51,8 @@ import feign.Client; import feign.Feign; import feign.FeignException; import feign.RequestInterceptor; -import feign.RequestTemplate; import feign.Response; import feign.codec.Decoder; -import feign.hystrix.HystrixFeign; /** * {@link org.springframework.boot.autoconfigure.EnableAutoConfiguration Auto-configuration} @@ -86,93 +78,50 @@ public class TraceFeignClientAutoConfiguration { @Autowired private Tracer tracer; + private final FeignRequestContext feignRequestContext = FeignRequestContext.getInstance(); + @Bean @Scope("prototype") @ConditionalOnClass(HystrixCommand.class) - @ConditionalOnMissingBean(SleuthHystrixConcurrencyStrategy.class) @ConditionalOnProperty(name = "feign.hystrix.enabled", matchIfMissing = true) public Feign.Builder feignHystrixBuilder(Tracer tracer, TraceKeys traceKeys) { - return HystrixFeign.builder().invocationHandlerFactory( - new SleuthHystrixInvocationHandler.Factory(tracer, traceKeys)); + return SleuthFeignBuilder.builder(this.publisher, tracer); + } + + @Bean + @ConditionalOnProperty(name = "spring.sleuth.feign.processor.enabled", matchIfMissing = true) + public FeignBeanPostProcessor feignBeanPostProcessor(Tracer tracer) { + return new FeignBeanPostProcessor(this.publisher, tracer); } @Bean @Primary - public Decoder feignDecoder() { - return new ResponseEntityDecoder(new SpringDecoder(this.messageConverters)) { + public Decoder feignDecoder(final Tracer tracer) { + return new TraceFeignDecoder(this.publisher, tracer, new ResponseEntityDecoder(new SpringDecoder(this.messageConverters)) { @Override public Object decode(Response response, Type type) throws IOException, FeignException { - 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)); - TraceFeignClientAutoConfiguration.this.tracer.close(span); - } - } } - }; + }); } + /** + * Sleuth {@link feign.RequestInterceptor} that either starts a new Span + * or continues an existing one if a retry takes place. + */ @Bean - public RequestInterceptor traceIdRequestInterceptor() { - return new RequestInterceptor() { - @Override - public void apply(RequestTemplate template) { - URI uri = URI.create(template.url()); - String spanName = uriScheme(uri) + ":" + uri.getPath(); - Span span = TraceFeignClientAutoConfiguration.this.tracer.createSpan(spanName); - if (span == null) { - setHeader(template, Span.NOT_SAMPLED_NAME, "true"); - return; - } - template.header(Span.TRACE_ID_NAME, Span.idToHex(span.getTraceId())); - setHeader(template, Span.SPAN_NAME_NAME, span.getName()); - setHeader(template, Span.SPAN_ID_NAME, Span.idToHex(span.getSpanId())); - if (!span.isExportable()) { - setHeader(template, Span.NOT_SAMPLED_NAME, "true"); - } - Long parentId = getParentId(span); - if (parentId != null) { - setHeader(template, Span.PARENT_ID_NAME, Span.idToHex(parentId)); - } - setHeader(template, Span.PROCESS_ID_NAME, span.getProcessId()); - publish(new ClientSentEvent(this, span)); - } - }; - } - - private String uriScheme(URI uri) { - return uri.getScheme() == null ? "http" : uri.getScheme(); - } - - private void publish(ApplicationEvent event) { - if (this.publisher != null) { - this.publisher.publishEvent(event); - } - } - - private Long getParentId(Span span) { - return !span.getParents().isEmpty() ? span.getParents().get(0) : null; - } - - public void setHeader(RequestTemplate request, String name, String value) { - if (StringUtils.hasText(value) && !request.headers().containsKey(name) - && this.tracer.isTracing()) { - request.header(name, value); - } + public RequestInterceptor traceIdRequestInterceptor(Tracer tracer) { + return new TraceFeignRequestInterceptor(tracer); } private Map> headersWithTraceId( Map> headers) { Map> newHeaders = new HashMap<>(); newHeaders.putAll(headers); - Span span = getCurrentSpan(); + Span span = this.feignRequestContext.getCurrentSpan(); if (span == null) { setHeader(newHeaders, Span.NOT_SAMPLED_NAME, "true"); return newHeaders; @@ -183,6 +132,10 @@ public class TraceFeignClientAutoConfiguration { return newHeaders; } + private Long getParentId(Span span) { + return !span.getParents().isEmpty() ? span.getParents().get(0) : null; + } + public void setHeader(Map> headers, String name, String value) { if (StringUtils.hasText(value) && !headers.containsKey(name) @@ -198,8 +151,4 @@ public class TraceFeignClientAutoConfiguration { } } - private Span getCurrentSpan() { - return this.tracer.getCurrentSpan(); - } - } 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 new file mode 100644 index 000000000..d13d7f7c8 --- /dev/null +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignDecoder.java @@ -0,0 +1,59 @@ +/* + * 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 java.io.IOException; +import java.lang.reflect.Type; + +import org.springframework.cloud.sleuth.Tracer; +import org.springframework.context.ApplicationEventPublisher; + +import feign.FeignException; +import feign.Response; +import feign.codec.DecodeException; +import feign.codec.Decoder; + +/** + * A decoder that closes a span upon decoding the response. + * + * @author Marcin Grzejszczak + * + * @since 1.0.0 + */ +class TraceFeignDecoder extends FeignEventPublisher implements Decoder { + + private final Decoder delegate; + + public TraceFeignDecoder(ApplicationEventPublisher publisher, Tracer tracer) { + super(publisher, tracer); + this.delegate = new Decoder.Default(); + } + public TraceFeignDecoder(ApplicationEventPublisher publisher, Tracer tracer, Decoder delegate) { + super(publisher, tracer); + this.delegate = delegate; + } + + @Override + public Object decode(Response response, Type type) + throws IOException, DecodeException, FeignException { + try { + return this.delegate.decode(response, type); + } finally { + finish(); + } + } +} diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignRequestInterceptor.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignRequestInterceptor.java new file mode 100644 index 000000000..6aff3af32 --- /dev/null +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignRequestInterceptor.java @@ -0,0 +1,122 @@ +/* + * 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 java.net.URI; + +import org.springframework.cloud.sleuth.Span; +import org.springframework.cloud.sleuth.Tracer; +import org.springframework.cloud.sleuth.event.ClientSentEvent; +import org.springframework.context.ApplicationEvent; +import org.springframework.context.ApplicationEventPublisher; +import org.springframework.context.ApplicationEventPublisherAware; +import org.springframework.util.StringUtils; + +import feign.RequestInterceptor; +import feign.RequestTemplate; + +/** + * A request interceptor that sets tracing information in the headers + * and retrieves the span from the current {@link FeignRequestContext}. + * + * @author Marcin Grzejszczak + * + * @since 1.0.0 + */ +class TraceFeignRequestInterceptor implements RequestInterceptor, + ApplicationEventPublisherAware { + + private final Tracer tracer; + private final FeignRequestContext feignRequestContext = FeignRequestContext.getInstance(); + + private ApplicationEventPublisher publisher; + + public TraceFeignRequestInterceptor(Tracer tracer) { + this.tracer = tracer; + } + + @Override + public void apply(RequestTemplate template) { + String spanName = getSpanName(template); + Span span = getSpan(spanName); + if (span == null) { + setHeader(template, Span.NOT_SAMPLED_NAME, "true"); + return; + } + template.header(Span.TRACE_ID_NAME, Span.idToHex(span.getTraceId())); + setHeader(template, Span.SPAN_NAME_NAME, span.getName()); + setHeader(template, Span.SPAN_ID_NAME, Span.idToHex(span.getSpanId())); + if (!span.isExportable()) { + setHeader(template, Span.NOT_SAMPLED_NAME, "true"); + } + Long parentId = getParentId(span); + if (parentId != null) { + setHeader(template, Span.PARENT_ID_NAME, Span.idToHex(parentId)); + } + setHeader(template, Span.PROCESS_ID_NAME, span.getProcessId()); + publish(new ClientSentEvent(this, span)); + } + + protected String getSpanName(RequestTemplate template) { + URI uri = URI.create(template.url()); + return uriScheme(uri) + ":" + uri.getPath(); + } + + /** + * Depending on the presence of a Span in context, either starts a new Span + * or continues an existing one. + */ + protected Span getSpan(String spanName) { + if (!this.feignRequestContext.hasSpanInProcess()) { + Span span = this.tracer.createSpan(spanName); + this.feignRequestContext.putSpan(span, false); + return span; + } else { + if (this.feignRequestContext.wasSpanRetried()) { + return this.tracer.continueSpan(this.feignRequestContext.getCurrentSpan()); + } + } + return this.tracer.createSpan(spanName); + } + + private String uriScheme(URI uri) { + return uri.getScheme() == null ? "http" : uri.getScheme(); + } + + private Long getParentId(Span span) { + return !span.getParents().isEmpty() ? span.getParents().get(0) : null; + } + + protected void setHeader(RequestTemplate request, String name, String value) { + if (StringUtils.hasText(value) && !request.headers().containsKey(name) + && this.tracer.isTracing()) { + request.header(name, value); + } + } + + private void publish(ApplicationEvent event) { + if (this.publisher != null) { + this.publisher.publishEvent(event); + } + } + + @Override + public void setApplicationEventPublisher( + ApplicationEventPublisher applicationEventPublisher) { + this.publisher = applicationEventPublisher; + } +} 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 new file mode 100644 index 000000000..d8300c618 --- /dev/null +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/feign/TraceFeignRetryer.java @@ -0,0 +1,66 @@ +/* + * 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.cloud.sleuth.Tracer; + +import feign.RetryableException; +import feign.Retryer; + +/** + * Execution of this retryer means that an exception occurred while trying to + * send the request. In that case we need to put information about this span + * into the {@link FeignRequestContext} in order for the {@link feign.RequestInterceptor} + * to know that it should be continued or a new one should be created. + * + * @author Marcin Grzejszczak + * + * @since 1.0.0 + */ +class TraceFeignRetryer implements Retryer { + + private final Tracer tracer; + private final FeignRequestContext feignRequestContext = FeignRequestContext.getInstance(); + private final Retryer delegate; + + public TraceFeignRetryer(Tracer tracer) { + this(tracer, new Retryer.Default()); + } + + public TraceFeignRetryer(Tracer tracer, Retryer delegate) { + this.tracer = tracer; + this.delegate = delegate; + } + + @Override + public void continueOrPropagate(RetryableException e) { + try { + this.feignRequestContext.putSpan(this.tracer.getCurrentSpan(), true); + this.tracer.getCurrentSpan().logEvent("feign.retry"); + this.delegate.continueOrPropagate(e); + } + catch (RetryableException e2) { + this.tracer.close(this.tracer.getCurrentSpan()); + throw e2; + } + } + + @Override + public Retryer clone() { + return new TraceFeignRetryer(this.tracer); + } +} \ No newline at end of file diff --git a/spring-cloud-sleuth-core/src/main/resources/META-INF/spring.factories b/spring-cloud-sleuth-core/src/main/resources/META-INF/spring.factories index 8292270a7..fc1f7cb64 100644 --- a/spring-cloud-sleuth-core/src/main/resources/META-INF/spring.factories +++ b/spring-cloud-sleuth-core/src/main/resources/META-INF/spring.factories @@ -12,7 +12,7 @@ org.springframework.cloud.sleuth.instrument.scheduling.TraceSchedulingAutoConfig org.springframework.cloud.sleuth.instrument.web.TraceWebAutoConfiguration,\ org.springframework.cloud.sleuth.instrument.web.client.TraceWebClientAutoConfiguration,\ org.springframework.cloud.sleuth.instrument.web.client.TraceWebAsyncClientAutoConfiguration,\ -org.springframework.cloud.sleuth.instrument.web.client.TraceFeignClientAutoConfiguration,\ +org.springframework.cloud.sleuth.instrument.web.client.feign.TraceFeignClientAutoConfiguration,\ org.springframework.cloud.sleuth.instrument.zuul.TraceZuulAutoConfiguration # Environment Post Processor diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/FeignTraceExceptionTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/FeignTraceExceptionTests.java new file mode 100644 index 000000000..e1f4b85fe --- /dev/null +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/FeignTraceExceptionTests.java @@ -0,0 +1,98 @@ +/* + * 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 java.io.IOException; +import java.net.UnknownHostException; + +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.SpringApplicationConfiguration; +import org.springframework.cloud.netflix.feign.EnableFeignClients; +import org.springframework.cloud.netflix.feign.FeignClient; +import org.springframework.cloud.sleuth.Span; +import org.springframework.cloud.sleuth.Tracer; +import org.springframework.cloud.sleuth.trace.TestSpanContextHolder; +import org.springframework.cloud.sleuth.util.ExceptionUtils; +import org.springframework.context.annotation.Configuration; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RequestMethod; + +import com.netflix.config.ConfigurationManager; +import com.netflix.hystrix.HystrixCommandProperties; + +import static org.assertj.core.api.BDDAssertions.then; +import static org.junit.Assert.fail; + +@RunWith(SpringJUnit4ClassRunner.class) +@SpringApplicationConfiguration(classes = { FeignTraceExceptionTests.TestConfiguration.class }) +@DirtiesContext +public class FeignTraceExceptionTests { + + @Autowired + TestFeignInterfaceWithException testFeignInterfaceWithException; + + @Autowired + Tracer tracer; + + @Before + public void before() { + ExceptionUtils.setFail(true); + } + + @After + public void close() { + TestSpanContextHolder.removeCurrentSpan(); + } + + @Test + public void shouldRemoveSpanFromThreadUponConnectionException() throws IOException { + Span span = this.tracer.createSpan("new trace"); + ConfigurationManager + .getConfigInstance().setProperty("hystrix.command.shouldFailToConnect.execution.isolation.strategy", + HystrixCommandProperties.ExecutionIsolationStrategy.SEMAPHORE); + + try { + this.testFeignInterfaceWithException.shouldFailToConnect(); + fail("should throw an exception"); + } catch (Exception e) { + then(e).hasRootCauseInstanceOf(UnknownHostException.class); + } + + then(this.tracer.getCurrentSpan()).isEqualTo(span); + this.tracer.close(span); + } + + @FeignClient(name = "exceptionService", url = "http://invalid.host.to.break.tests") + public interface TestFeignInterfaceWithException { + @RequestMapping(method = RequestMethod.GET, value = "/") + String shouldFailToConnect(); + } + + @Configuration + @EnableAutoConfiguration + @EnableFeignClients + public static class TestConfiguration { + + } +} diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/FeignTraceTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/FeignTraceTests.java similarity index 77% rename from spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/FeignTraceTests.java rename to spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/FeignTraceTests.java index c1cf51d09..a46d72b70 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/FeignTraceTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/feign/FeignTraceTests.java @@ -1,4 +1,20 @@ -package org.springframework.cloud.sleuth.instrument.web.client; +/* + * 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 java.util.ArrayList; import java.util.Arrays; @@ -90,7 +106,7 @@ public class FeignTraceTests { public void shouldAttachTraceIdWhenUsingFeignClient() { Long currentTraceId = 1L; Long currentParentId = 2L; - Long currentSpanId = generatedId(); + Long currentSpanId = 100L; this.tracer.continueSpan(Span.builder().traceId(currentTraceId) .spanId(currentSpanId).parent(currentParentId).build()); @@ -100,7 +116,28 @@ public class FeignTraceTests { .isEqualTo(currentTraceId); then(Span.hexToId(getHeader(response, Span.PARENT_ID_NAME))) .isEqualTo(currentSpanId); + thenRegisteredClientSentAndReceivedEvents(); + } + + @Test + public void shouldAttachTraceIdWhenUsingFeignClientWithoutResponseBody() { + Long currentTraceId = 1L; + Long currentParentId = 2L; + Long currentSpanId = generatedId(); + Span span = Span.builder().traceId(currentTraceId) + .spanId(currentSpanId).parent(currentParentId).build(); + this.tracer.continueSpan(span); + + this.testFeignInterface.noResponseBody(); + + thenRegisteredClientSentAndReceivedEvents(); + then(this.tracer.getCurrentSpan()).isEqualTo(span); + } + + private void thenRegisteredClientSentAndReceivedEvents() { then(this.listener.getEvents().size()).isEqualTo(2); + then(this.listener.getEvents().get(0)).isExactlyInstanceOf(ClientSentEvent.class); + then(this.listener.getEvents().get(1)).isExactlyInstanceOf(ClientReceivedEvent.class); } private Long generatedId() { @@ -122,6 +159,9 @@ public class FeignTraceTests { @RequestMapping(method = RequestMethod.GET, value = "/") ResponseEntity> headers(); + + @RequestMapping(method = RequestMethod.GET, value = "/noresponse") + void noResponseBody(); } @Configuration @@ -189,6 +229,15 @@ public class FeignTraceTests { return map; } + @RequestMapping("/noresponse") + public void noResponse(@RequestHeader(Span.TRACE_ID_NAME) String traceId, + @RequestHeader(Span.SPAN_ID_NAME) String spanId, + @RequestHeader(Span.PARENT_ID_NAME) String parentId) { + then(traceId).isNotEmpty(); + then(parentId).isNotEmpty(); + then(spanId).isNotEmpty(); + } + } @Configuration