From 6ecfbca6ed6bcd38e715f03ca5d5a513c9593346 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Wed, 23 Dec 2015 18:08:09 +0100 Subject: [PATCH 1/4] [#39] Initial approach to Hystrix concurrency strategy --- .../SleuthHystrixConcurrencyStrategy.java | 24 ++++ .../hystrix/SleuthHystrixConfiguration.java | 19 +++ .../main/resources/META-INF/spring.factories | 1 + .../instrument/hystrix/JavanicaITest.java | 125 ++++++++++++++++++ .../instrument/web/TraceAsyncITest.java | 13 +- 5 files changed, 174 insertions(+), 8 deletions(-) create mode 100644 spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/hystrix/SleuthHystrixConcurrencyStrategy.java create mode 100644 spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/hystrix/SleuthHystrixConfiguration.java create mode 100644 spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/hystrix/JavanicaITest.java 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 new file mode 100644 index 000000000..89f555a51 --- /dev/null +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/hystrix/SleuthHystrixConcurrencyStrategy.java @@ -0,0 +1,24 @@ +package org.springframework.cloud.sleuth.instrument.hystrix; + +import java.util.concurrent.Callable; + +import org.springframework.cloud.sleuth.TraceManager; +import org.springframework.cloud.sleuth.instrument.TraceCallable; + +import com.netflix.hystrix.strategy.HystrixPlugins; +import com.netflix.hystrix.strategy.concurrency.HystrixConcurrencyStrategy; + +public class SleuthHystrixConcurrencyStrategy extends HystrixConcurrencyStrategy { + + private final TraceManager traceManager; + + public SleuthHystrixConcurrencyStrategy(TraceManager traceManager) { + this.traceManager = traceManager; + HystrixPlugins.getInstance().registerConcurrencyStrategy(this); + } + + @Override + public Callable wrapCallable(Callable callable) { + return new TraceCallable<>(traceManager, callable); + } +} diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/hystrix/SleuthHystrixConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/hystrix/SleuthHystrixConfiguration.java new file mode 100644 index 000000000..019e12cff --- /dev/null +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/hystrix/SleuthHystrixConfiguration.java @@ -0,0 +1,19 @@ +package org.springframework.cloud.sleuth.instrument.hystrix; + +import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.cloud.sleuth.TraceManager; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + +import com.netflix.hystrix.HystrixCommand; + +@Configuration +@ConditionalOnClass(HystrixCommand.class) +@ConditionalOnProperty(value = "spring.sleuth.hystrix.strategy.enabled", matchIfMissing = true) +public class SleuthHystrixConfiguration { + + @Bean SleuthHystrixConcurrencyStrategy sleuthHystrixConcurrencyStrategy(TraceManager traceManager) { + return new SleuthHystrixConcurrencyStrategy(traceManager); + } +} 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 96af97d05..a676845f5 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 @@ -5,6 +5,7 @@ org.springframework.cloud.sleuth.log.SleuthLogAutoConfiguration,\ org.springframework.cloud.sleuth.instrument.integration.TraceSpringIntegrationAutoConfiguration,\ org.springframework.cloud.sleuth.instrument.async.AsyncCustomAutoConfiguration,\ org.springframework.cloud.sleuth.instrument.async.AsyncDefaultAutoConfiguration,\ +org.springframework.cloud.sleuth.instrument.hystrix.SleuthHystrixConfiguration,\ org.springframework.cloud.sleuth.instrument.scheduling.TraceSchedulingAutoConfiguration,\ org.springframework.cloud.sleuth.instrument.web.TraceWebAutoConfiguration,\ org.springframework.cloud.sleuth.instrument.web.client.TraceWebClientAutoConfiguration,\ diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/hystrix/JavanicaITest.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/hystrix/JavanicaITest.java new file mode 100644 index 000000000..32789e6d8 --- /dev/null +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/hystrix/JavanicaITest.java @@ -0,0 +1,125 @@ +package org.springframework.cloud.sleuth.instrument.hystrix; + +import static org.assertj.core.api.BDDAssertions.then; + +import java.util.concurrent.atomic.AtomicReference; + +import org.junit.After; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.SpringApplicationConfiguration; +import org.springframework.cloud.netflix.hystrix.EnableHystrix; +import org.springframework.cloud.sleuth.Span; +import org.springframework.cloud.sleuth.TraceManager; +import org.springframework.cloud.sleuth.instrument.DefaultTestAutoConfiguration; +import org.springframework.cloud.sleuth.trace.TraceContextHolder; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.EnableAspectJAutoProxy; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; + +import com.jayway.awaitility.Awaitility; +import com.netflix.hystrix.contrib.javanica.annotation.HystrixCommand; + +@RunWith(SpringJUnit4ClassRunner.class) +@SpringApplicationConfiguration(classes = { + JavanicaITest.JavanicaITestConfiguration.class }) +public class JavanicaITest { + + @Autowired JavanicaClass javanicaClass; + @Autowired JavanicaDelegation javanicaDelegation; + @Autowired TraceManager traceManager; + + @Test + public void should_set_span_on_an_hystrix_command_annotated_method() { + final Span span = givenASpanInCurrentThread(); + + whenHystrixCommandGetsExecutedViaJavanica(); + + thenSpanPutInTheAsyncThreadIsSameAs(span); + } + + private Span givenASpanInCurrentThread() { + Span span = this.traceManager.startSpan("existing").getSpan(); + this.traceManager.continueSpan(span); + return span; + } + + private void whenHystrixCommandGetsExecutedViaJavanica() { + this.javanicaDelegation.doSthThatDelegatesToJavanica(); + } + + private void thenSpanPutInTheAsyncThreadIsSameAs(final Span span) { + Awaitility.await().until(new Runnable() { + @Override + public void run() { + then(span.getTraceId()).isNotNull() + .isEqualTo(javanicaClass.getTraceId()); + then(span.getName()) + .isNotEqualTo(javanicaClass.getSpanName()); + } + }); + } + + @After + public void cleanTrace() { + TraceContextHolder.removeCurrentTrace(); + } + + @DefaultTestAutoConfiguration + @EnableHystrix + @EnableAspectJAutoProxy(proxyTargetClass = true) + @Configuration + public static class JavanicaITestConfiguration { + + @Bean + JavanicaClass javanicaClass() { + return new JavanicaClass(); + } + + @Bean + JavanicaDelegation javanicaDelegation() { + return new JavanicaDelegation(javanicaClass()); + } + } + + public static class JavanicaDelegation { + + private final JavanicaClass javanicaClass; + + public JavanicaDelegation(JavanicaClass javanicaClass) { + this.javanicaClass = javanicaClass; + } + + public void doSthThatDelegatesToJavanica() { + this.javanicaClass.doSth(); + } + } + + public static class JavanicaClass { + + AtomicReference span; + + @HystrixCommand + public void doSth() { + this.span = new AtomicReference<>(TraceContextHolder.getCurrentSpan()); + } + + public String getTraceId() { + if (this.span == null || this.span.get() == null || (this.span.get() != null + && this.span.get().getTraceId() == null)) { + return null; + } + return this.span.get().getTraceId(); + } + + public String getSpanName() { + if (this.span == null + || (this.span.get() != null && this.span.get().getName() == null)) { + return null; + } + return this.span.get().getName(); + } + } +} diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceAsyncITest.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceAsyncITest.java index dacbc5113..619b1697c 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceAsyncITest.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceAsyncITest.java @@ -25,15 +25,12 @@ import com.jayway.awaitility.Awaitility; @RunWith(SpringJUnit4ClassRunner.class) @SpringApplicationConfiguration(classes = { - TraceAsyncITest.CorrelationIdAsyncSpecConfiguration.class }) + TraceAsyncITest.TraceAsyncITestConfiguration.class }) public class TraceAsyncITest { - @Autowired - AsyncClass asyncClass; - @Autowired - AsyncDelegation asyncDelegation; - @Autowired - TraceManager traceManager; + @Autowired AsyncClass asyncClass; + @Autowired AsyncDelegation asyncDelegation; + @Autowired TraceManager traceManager; @Test public void should_set_span_on_an_async_annotated_method() { @@ -75,7 +72,7 @@ public class TraceAsyncITest { @EnableAsync @EnableAspectJAutoProxy(proxyTargetClass = true) @Configuration - public static class CorrelationIdAsyncSpecConfiguration { + public static class TraceAsyncITestConfiguration { @Bean AsyncClass asyncClass() { From 6f71402cf94fab349efbf6a8baf408939d57771b Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Wed, 23 Dec 2015 18:50:50 +0100 Subject: [PATCH 2/4] [#39] Fixed naming and added dependency between hystrix components --- ...onfiguration.java => SleuthHystrixAutoConfiguration.java} | 2 +- .../web/client/TraceFeignClientAutoConfiguration.java | 5 +++++ .../src/main/resources/META-INF/spring.factories | 2 +- 3 files changed, 7 insertions(+), 2 deletions(-) rename spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/hystrix/{SleuthHystrixConfiguration.java => SleuthHystrixAutoConfiguration.java} (94%) diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/hystrix/SleuthHystrixConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/hystrix/SleuthHystrixAutoConfiguration.java similarity index 94% rename from spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/hystrix/SleuthHystrixConfiguration.java rename to spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/hystrix/SleuthHystrixAutoConfiguration.java index 019e12cff..dd3d27978 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/hystrix/SleuthHystrixConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/hystrix/SleuthHystrixAutoConfiguration.java @@ -11,7 +11,7 @@ import com.netflix.hystrix.HystrixCommand; @Configuration @ConditionalOnClass(HystrixCommand.class) @ConditionalOnProperty(value = "spring.sleuth.hystrix.strategy.enabled", matchIfMissing = true) -public class SleuthHystrixConfiguration { +public class SleuthHystrixAutoConfiguration { @Bean SleuthHystrixConcurrencyStrategy sleuthHystrixConcurrencyStrategy(TraceManager traceManager) { return new SleuthHystrixConcurrencyStrategy(traceManager); 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/TraceFeignClientAutoConfiguration.java index 0772ac0e9..6b5d04504 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/TraceFeignClientAutoConfiguration.java @@ -26,6 +26,7 @@ import java.util.Map; import org.springframework.beans.factory.ObjectFactory; 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; @@ -40,6 +41,8 @@ import org.springframework.cloud.sleuth.TraceAccessor; import org.springframework.cloud.sleuth.TraceManager; import org.springframework.cloud.sleuth.event.ClientReceivedEvent; import org.springframework.cloud.sleuth.event.ClientSentEvent; +import org.springframework.cloud.sleuth.instrument.hystrix.SleuthHystrixConcurrencyStrategy; +import org.springframework.cloud.sleuth.instrument.hystrix.SleuthHystrixAutoConfiguration; import org.springframework.context.ApplicationEvent; import org.springframework.context.ApplicationEventPublisher; import org.springframework.context.annotation.Bean; @@ -67,6 +70,7 @@ import feign.hystrix.HystrixFeign; @ConditionalOnProperty(value = "spring.sleuth.feign.enabled", matchIfMissing = true) @ConditionalOnClass(Client.class) @AutoConfigureBefore(FeignAutoConfiguration.class) +@AutoConfigureAfter(SleuthHystrixAutoConfiguration.class) public class TraceFeignClientAutoConfiguration { @Autowired @@ -81,6 +85,7 @@ public class TraceFeignClientAutoConfiguration { @Bean @Scope("prototype") @ConditionalOnClass(HystrixCommand.class) + @ConditionalOnMissingBean(SleuthHystrixConcurrencyStrategy.class) @ConditionalOnProperty(name = "feign.hystrix.enabled", matchIfMissing = true) public Feign.Builder feignHystrixBuilder(TraceManager traceManager) { return HystrixFeign.builder() 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 a676845f5..d3e81a9f7 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 @@ -5,7 +5,7 @@ org.springframework.cloud.sleuth.log.SleuthLogAutoConfiguration,\ org.springframework.cloud.sleuth.instrument.integration.TraceSpringIntegrationAutoConfiguration,\ org.springframework.cloud.sleuth.instrument.async.AsyncCustomAutoConfiguration,\ org.springframework.cloud.sleuth.instrument.async.AsyncDefaultAutoConfiguration,\ -org.springframework.cloud.sleuth.instrument.hystrix.SleuthHystrixConfiguration,\ +org.springframework.cloud.sleuth.instrument.hystrix.SleuthHystrixAutoConfiguration,\ org.springframework.cloud.sleuth.instrument.scheduling.TraceSchedulingAutoConfiguration,\ org.springframework.cloud.sleuth.instrument.web.TraceWebAutoConfiguration,\ org.springframework.cloud.sleuth.instrument.web.client.TraceWebClientAutoConfiguration,\ From 980e3f2bd5c686a80ff638d30c39d4dfbb133c5c Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Thu, 24 Dec 2015 09:50:10 +0100 Subject: [PATCH 3/4] [#39] Introduced custom Sleuth assertions --- .../sleuth/assertions/SleuthAssertions.java | 12 +++++++ .../cloud/sleuth/assertions/SpanAssert.java | 33 +++++++++++++++++++ .../instrument/hystrix/JavanicaITest.java | 9 +++-- .../instrument/web/TraceAsyncITest.java | 9 +++-- 4 files changed, 53 insertions(+), 10 deletions(-) create mode 100644 spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/assertions/SleuthAssertions.java create mode 100644 spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/assertions/SpanAssert.java diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/assertions/SleuthAssertions.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/assertions/SleuthAssertions.java new file mode 100644 index 000000000..6a73c118b --- /dev/null +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/assertions/SleuthAssertions.java @@ -0,0 +1,12 @@ +package org.springframework.cloud.sleuth.assertions; + +import org.assertj.core.api.BDDAssertions; +import org.springframework.cloud.sleuth.Span; + +public class SleuthAssertions extends BDDAssertions { + + public static SpanAssert then(Span actual) { + return new SpanAssert(actual); + } + +} diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/assertions/SpanAssert.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/assertions/SpanAssert.java new file mode 100644 index 000000000..56420aa86 --- /dev/null +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/assertions/SpanAssert.java @@ -0,0 +1,33 @@ +package org.springframework.cloud.sleuth.assertions; + +import java.util.Objects; + +import org.assertj.core.api.AbstractAssert; +import org.springframework.cloud.sleuth.Span; + +public class SpanAssert extends AbstractAssert { + + public SpanAssert(Span actual) { + super(actual, SpanAssert.class); + } + + public static SpanAssert then(Span actual) { + return new SpanAssert(actual); + } + + public SpanAssert hasTraceId(String traceId) { + isNotNull(); + if (!Objects.equals(actual.getTraceId(), traceId)) { + failWithMessage("Expected span's traceId to be <%s> but was <%s>", traceId, actual.getTraceId()); + } + return this; + } + + public SpanAssert hasNameNotEqualTo(String name) { + isNotNull(); + if (Objects.equals(actual.getName(), name)) { + failWithMessage("Expected span's name not to be <%s> but was <%s>", name, actual.getName()); + } + return this; + } +} \ No newline at end of file diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/hystrix/JavanicaITest.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/hystrix/JavanicaITest.java index 32789e6d8..df065914c 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/hystrix/JavanicaITest.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/hystrix/JavanicaITest.java @@ -1,6 +1,6 @@ package org.springframework.cloud.sleuth.instrument.hystrix; -import static org.assertj.core.api.BDDAssertions.then; +import static org.springframework.cloud.sleuth.assertions.SleuthAssertions.then; import java.util.concurrent.atomic.AtomicReference; @@ -54,10 +54,9 @@ public class JavanicaITest { Awaitility.await().until(new Runnable() { @Override public void run() { - then(span.getTraceId()).isNotNull() - .isEqualTo(javanicaClass.getTraceId()); - then(span.getName()) - .isNotEqualTo(javanicaClass.getSpanName()); + then(span) + .hasTraceId(javanicaClass.getTraceId()) + .hasNameNotEqualTo(javanicaClass.getSpanName()); } }); } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceAsyncITest.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceAsyncITest.java index 619b1697c..745a05e88 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceAsyncITest.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceAsyncITest.java @@ -1,7 +1,7 @@ package org.springframework.cloud.sleuth.instrument.web; -import static org.assertj.core.api.BDDAssertions.then; +import static org.springframework.cloud.sleuth.assertions.SleuthAssertions.then; import java.util.concurrent.atomic.AtomicReference; @@ -55,10 +55,9 @@ public class TraceAsyncITest { Awaitility.await().until(new Runnable() { @Override public void run() { - then(span.getTraceId()).isNotNull() - .isEqualTo(TraceAsyncITest.this.asyncClass.getTraceId()); - then(span.getName()) - .isNotEqualTo(TraceAsyncITest.this.asyncClass.getSpanName()); + then(span) + .hasTraceId(asyncClass.getTraceId()) + .hasNameNotEqualTo(asyncClass.getSpanName()); } }); } From 7e8a48314973eeba26d95a2af4787ac80a2a477d Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Thu, 24 Dec 2015 10:17:14 +0100 Subject: [PATCH 4/4] [#39] Refactored code according to the code review --- .../sleuth/assertions/SleuthAssertions.java | 4 + .../cloud/sleuth/assertions/SpanAssert.java | 2 +- .../instrument/hystrix/JavanicaITest.java | 124 ------------------ ...nPassingForHystrixViaAnnotationsITest.java | 106 +++++++++++++++ .../instrument/web/TraceAsyncITest.java | 62 +++------ 5 files changed, 132 insertions(+), 166 deletions(-) delete mode 100644 spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/hystrix/JavanicaITest.java create mode 100644 spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/hystrix/SpanPassingForHystrixViaAnnotationsITest.java diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/assertions/SleuthAssertions.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/assertions/SleuthAssertions.java index 6a73c118b..bc1369f36 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/assertions/SleuthAssertions.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/assertions/SleuthAssertions.java @@ -6,6 +6,10 @@ import org.springframework.cloud.sleuth.Span; public class SleuthAssertions extends BDDAssertions { public static SpanAssert then(Span actual) { + return assertThat(actual); + } + + public static SpanAssert assertThat(Span actual) { return new SpanAssert(actual); } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/assertions/SpanAssert.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/assertions/SpanAssert.java index 56420aa86..296340dde 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/assertions/SpanAssert.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/assertions/SpanAssert.java @@ -15,7 +15,7 @@ public class SpanAssert extends AbstractAssert { return new SpanAssert(actual); } - public SpanAssert hasTraceId(String traceId) { + public SpanAssert hasTraceIdEqualTo(String traceId) { isNotNull(); if (!Objects.equals(actual.getTraceId(), traceId)) { failWithMessage("Expected span's traceId to be <%s> but was <%s>", traceId, actual.getTraceId()); diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/hystrix/JavanicaITest.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/hystrix/JavanicaITest.java deleted file mode 100644 index df065914c..000000000 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/hystrix/JavanicaITest.java +++ /dev/null @@ -1,124 +0,0 @@ -package org.springframework.cloud.sleuth.instrument.hystrix; - -import static org.springframework.cloud.sleuth.assertions.SleuthAssertions.then; - -import java.util.concurrent.atomic.AtomicReference; - -import org.junit.After; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.boot.test.SpringApplicationConfiguration; -import org.springframework.cloud.netflix.hystrix.EnableHystrix; -import org.springframework.cloud.sleuth.Span; -import org.springframework.cloud.sleuth.TraceManager; -import org.springframework.cloud.sleuth.instrument.DefaultTestAutoConfiguration; -import org.springframework.cloud.sleuth.trace.TraceContextHolder; -import org.springframework.context.annotation.Bean; -import org.springframework.context.annotation.Configuration; -import org.springframework.context.annotation.EnableAspectJAutoProxy; -import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; - -import com.jayway.awaitility.Awaitility; -import com.netflix.hystrix.contrib.javanica.annotation.HystrixCommand; - -@RunWith(SpringJUnit4ClassRunner.class) -@SpringApplicationConfiguration(classes = { - JavanicaITest.JavanicaITestConfiguration.class }) -public class JavanicaITest { - - @Autowired JavanicaClass javanicaClass; - @Autowired JavanicaDelegation javanicaDelegation; - @Autowired TraceManager traceManager; - - @Test - public void should_set_span_on_an_hystrix_command_annotated_method() { - final Span span = givenASpanInCurrentThread(); - - whenHystrixCommandGetsExecutedViaJavanica(); - - thenSpanPutInTheAsyncThreadIsSameAs(span); - } - - private Span givenASpanInCurrentThread() { - Span span = this.traceManager.startSpan("existing").getSpan(); - this.traceManager.continueSpan(span); - return span; - } - - private void whenHystrixCommandGetsExecutedViaJavanica() { - this.javanicaDelegation.doSthThatDelegatesToJavanica(); - } - - private void thenSpanPutInTheAsyncThreadIsSameAs(final Span span) { - Awaitility.await().until(new Runnable() { - @Override - public void run() { - then(span) - .hasTraceId(javanicaClass.getTraceId()) - .hasNameNotEqualTo(javanicaClass.getSpanName()); - } - }); - } - - @After - public void cleanTrace() { - TraceContextHolder.removeCurrentTrace(); - } - - @DefaultTestAutoConfiguration - @EnableHystrix - @EnableAspectJAutoProxy(proxyTargetClass = true) - @Configuration - public static class JavanicaITestConfiguration { - - @Bean - JavanicaClass javanicaClass() { - return new JavanicaClass(); - } - - @Bean - JavanicaDelegation javanicaDelegation() { - return new JavanicaDelegation(javanicaClass()); - } - } - - public static class JavanicaDelegation { - - private final JavanicaClass javanicaClass; - - public JavanicaDelegation(JavanicaClass javanicaClass) { - this.javanicaClass = javanicaClass; - } - - public void doSthThatDelegatesToJavanica() { - this.javanicaClass.doSth(); - } - } - - public static class JavanicaClass { - - AtomicReference span; - - @HystrixCommand - public void doSth() { - this.span = new AtomicReference<>(TraceContextHolder.getCurrentSpan()); - } - - public String getTraceId() { - if (this.span == null || this.span.get() == null || (this.span.get() != null - && this.span.get().getTraceId() == null)) { - return null; - } - return this.span.get().getTraceId(); - } - - public String getSpanName() { - if (this.span == null - || (this.span.get() != null && this.span.get().getName() == null)) { - return null; - } - return this.span.get().getName(); - } - } -} diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/hystrix/SpanPassingForHystrixViaAnnotationsITest.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/hystrix/SpanPassingForHystrixViaAnnotationsITest.java new file mode 100644 index 000000000..407c48f3a --- /dev/null +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/hystrix/SpanPassingForHystrixViaAnnotationsITest.java @@ -0,0 +1,106 @@ +package org.springframework.cloud.sleuth.instrument.hystrix; + +import static org.springframework.cloud.sleuth.assertions.SleuthAssertions.then; + +import java.util.concurrent.atomic.AtomicReference; + +import org.junit.After; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.SpringApplicationConfiguration; +import org.springframework.cloud.netflix.hystrix.EnableHystrix; +import org.springframework.cloud.sleuth.Span; +import org.springframework.cloud.sleuth.TraceManager; +import org.springframework.cloud.sleuth.instrument.DefaultTestAutoConfiguration; +import org.springframework.cloud.sleuth.trace.TraceContextHolder; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; + +import com.jayway.awaitility.Awaitility; +import com.netflix.hystrix.contrib.javanica.annotation.HystrixCommand; + +@RunWith(SpringJUnit4ClassRunner.class) +@SpringApplicationConfiguration(classes = { + SpanPassingForHystrixViaAnnotationsITest.TestConfig.class }) +public class SpanPassingForHystrixViaAnnotationsITest { + + @Autowired HystrixCommandInvocationSpanCatcher hystrixCommandInvocationSpanCatcher; + @Autowired TraceManager traceManager; + + @Test + public void should_set_span_on_an_hystrix_command_annotated_method() { + Span span = givenASpanInCurrentThread(); + + whenHystrixCommandAnnotatedMethodGetsExecuted(); + + thenTraceIdIsPassedFromTheCurrentThreadToTheHystrixOne(span); + } + + private Span givenASpanInCurrentThread() { + Span span = traceManager.startSpan("existing").getSpan(); + traceManager.continueSpan(span); + return span; + } + + private void whenHystrixCommandAnnotatedMethodGetsExecuted() { + hystrixCommandInvocationSpanCatcher.invokeLogicWrappedInHystrixCommand(); + } + + private void thenTraceIdIsPassedFromTheCurrentThreadToTheHystrixOne(final Span span) { + Awaitility.await().until(new Runnable() { + @Override + public void run() { + then(span) + .hasTraceIdEqualTo(hystrixCommandInvocationSpanCatcher.getTraceId()) + .hasNameNotEqualTo(hystrixCommandInvocationSpanCatcher.getSpanName()); + } + }); + } + + @After + public void cleanTrace() { + TraceContextHolder.removeCurrentTrace(); + } + + @DefaultTestAutoConfiguration + @EnableHystrix + @Configuration + static class TestConfig { + + @Bean HystrixCommandInvocationSpanCatcher spanCatcher() { + return new HystrixCommandInvocationSpanCatcher(); + } + + } + + static class HystrixCommandInvocationSpanCatcher { + + AtomicReference spanCaughtFromHystrixThread; + + @HystrixCommand + public void invokeLogicWrappedInHystrixCommand() { + spanCaughtFromHystrixThread = new AtomicReference<>(TraceContextHolder.getCurrentSpan()); + } + + public String getTraceId() { + if (spanCaughtFromHystrixThread == null || + spanCaughtFromHystrixThread.get() == null || + (spanCaughtFromHystrixThread.get() != null && + spanCaughtFromHystrixThread.get().getTraceId() == null)) { + return null; + } + return spanCaughtFromHystrixThread.get().getTraceId(); + } + + public String getSpanName() { + if (spanCaughtFromHystrixThread == null || + (spanCaughtFromHystrixThread.get() != null && + spanCaughtFromHystrixThread.get().getName() == null)) { + return null; + } + return spanCaughtFromHystrixThread.get().getName(); + } + } +} diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceAsyncITest.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceAsyncITest.java index 745a05e88..896c2854e 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceAsyncITest.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceAsyncITest.java @@ -16,7 +16,6 @@ import org.springframework.cloud.sleuth.instrument.DefaultTestAutoConfiguration; import org.springframework.cloud.sleuth.trace.TraceContextHolder; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; -import org.springframework.context.annotation.EnableAspectJAutoProxy; import org.springframework.scheduling.annotation.Async; import org.springframework.scheduling.annotation.EnableAsync; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; @@ -28,36 +27,35 @@ import com.jayway.awaitility.Awaitility; TraceAsyncITest.TraceAsyncITestConfiguration.class }) public class TraceAsyncITest { - @Autowired AsyncClass asyncClass; - @Autowired AsyncDelegation asyncDelegation; + @Autowired ClassPerformingAsyncLogic classPerformingAsyncLogic; @Autowired TraceManager traceManager; @Test public void should_set_span_on_an_async_annotated_method() { - final Span span = givenASpanInCurrentThread(); + Span span = givenASpanInCurrentThread(); whenAsyncProcessingTakesPlace(); - thenSpanPutInTheAsyncThreadIsSameAs(span); + thenTraceIdIsPassedFromTheCurrentThreadToTheAsyncOne(span); } private Span givenASpanInCurrentThread() { - Span span = this.traceManager.startSpan("existing").getSpan(); - this.traceManager.continueSpan(span); + Span span = traceManager.startSpan("existing").getSpan(); + traceManager.continueSpan(span); return span; } private void whenAsyncProcessingTakesPlace() { - this.asyncDelegation.doSthThatDelegatesToAsync(); + classPerformingAsyncLogic.invokeAsynchronousLogic(); } - private void thenSpanPutInTheAsyncThreadIsSameAs(final Span span) { + private void thenTraceIdIsPassedFromTheCurrentThreadToTheAsyncOne(final Span span) { Awaitility.await().until(new Runnable() { @Override public void run() { then(span) - .hasTraceId(asyncClass.getTraceId()) - .hasNameNotEqualTo(asyncClass.getSpanName()); + .hasTraceIdEqualTo(classPerformingAsyncLogic.getTraceId()) + .hasNameNotEqualTo(classPerformingAsyncLogic.getSpanName()); } }); } @@ -69,57 +67,39 @@ public class TraceAsyncITest { @DefaultTestAutoConfiguration @EnableAsync - @EnableAspectJAutoProxy(proxyTargetClass = true) @Configuration - public static class TraceAsyncITestConfiguration { + static class TraceAsyncITestConfiguration { @Bean - AsyncClass asyncClass() { - return new AsyncClass(); + ClassPerformingAsyncLogic asyncClass() { + return new ClassPerformingAsyncLogic(); } - @Bean - AsyncDelegation asyncDelegation() { - return new AsyncDelegation(asyncClass()); - } } - public static class AsyncDelegation { - - private final AsyncClass asyncClass; - - public AsyncDelegation(AsyncClass asyncClass) { - this.asyncClass = asyncClass; - } - - public void doSthThatDelegatesToAsync() { - this.asyncClass.doSth(); - } - } - - public static class AsyncClass { + static class ClassPerformingAsyncLogic { AtomicReference span; @Async - public void doSth() { - this.span = new AtomicReference<>(TraceContextHolder.getCurrentSpan()); + public void invokeAsynchronousLogic() { + span = new AtomicReference<>(TraceContextHolder.getCurrentSpan()); } public String getTraceId() { - if (this.span == null || (this.span.get() != null - && this.span.get().getTraceId() == null)) { + if (span == null || (span.get() != null + && span.get().getTraceId() == null)) { return null; } - return this.span.get().getTraceId(); + return span.get().getTraceId(); } public String getSpanName() { - if (this.span == null - || (this.span.get() != null && this.span.get().getName() == null)) { + if (span == null + || (span.get() != null && span.get().getName() == null)) { return null; } - return this.span.get().getName(); + return span.get().getName(); } } }