From c8992a65b329aa5b6f23d0f670c59ceb2c3f05ec Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Fri, 29 Jul 2016 15:07:17 +0200 Subject: [PATCH] Adding tags for Hystrix spans with this change if a span hasnt set any Hystrix related spans then we're setting the tags even if the span gets continued (which is the case when you're using Javanica fixes #352 --- .../SleuthHystrixConcurrencyStrategy.java | 13 ++-- .../SleuthHystrixConcurrencyStrategyTest.java | 74 +++++++++++++++++-- 2 files changed, 75 insertions(+), 12 deletions(-) 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 1f2f41f15..1d6fe2838 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 @@ -129,13 +129,16 @@ public class SleuthHystrixConcurrencyStrategy extends HystrixConcurrencyStrategy } else { span = this.tracer.createSpan(HYSTRIX_COMPONENT); - this.tracer.addTag(Span.SPAN_LOCAL_COMPONENT_TAG_NAME, HYSTRIX_COMPONENT); - this.tracer.addTag( - this.traceKeys.getAsync().getPrefix() - + this.traceKeys.getAsync().getThreadNameKey(), - Thread.currentThread().getName()); created = true; } + if (!span.tags().containsKey(Span.SPAN_LOCAL_COMPONENT_TAG_NAME)) { + this.tracer.addTag(Span.SPAN_LOCAL_COMPONENT_TAG_NAME, HYSTRIX_COMPONENT); + } + String asyncKey = this.traceKeys.getAsync().getPrefix() + + this.traceKeys.getAsync().getThreadNameKey(); + if (!span.tags().containsKey(asyncKey)) { + this.tracer.addTag(asyncKey, Thread.currentThread().getName()); + } try { return this.callable.call(); } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/hystrix/SleuthHystrixConcurrencyStrategyTest.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/hystrix/SleuthHystrixConcurrencyStrategyTest.java index 087187403..6f9d831be 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/hystrix/SleuthHystrixConcurrencyStrategyTest.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/hystrix/SleuthHystrixConcurrencyStrategyTest.java @@ -16,16 +16,22 @@ package org.springframework.cloud.sleuth.instrument.hystrix; +import java.util.Random; import java.util.concurrent.Callable; import org.junit.After; import org.junit.Before; import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.Mock; -import org.mockito.runners.MockitoJUnitRunner; +import org.springframework.cloud.sleuth.DefaultSpanNamer; +import org.springframework.cloud.sleuth.Span; import org.springframework.cloud.sleuth.TraceKeys; import org.springframework.cloud.sleuth.Tracer; +import org.springframework.cloud.sleuth.assertions.ListOfSpans; +import org.springframework.cloud.sleuth.log.NoOpSpanLogger; +import org.springframework.cloud.sleuth.sampler.AlwaysSampler; +import org.springframework.cloud.sleuth.trace.DefaultTracer; +import org.springframework.cloud.sleuth.util.ArrayListSpanAccumulator; +import org.springframework.cloud.sleuth.util.ExceptionUtils; import com.netflix.hystrix.strategy.HystrixPlugins; import com.netflix.hystrix.strategy.concurrency.HystrixConcurrencyStrategy; @@ -34,21 +40,23 @@ import com.netflix.hystrix.strategy.executionhook.HystrixCommandExecutionHook; import com.netflix.hystrix.strategy.metrics.HystrixMetricsPublisher; import com.netflix.hystrix.strategy.properties.HystrixPropertiesStrategy; -import static org.assertj.core.api.BDDAssertions.then; - +import static org.springframework.cloud.sleuth.assertions.SleuthAssertions.then; /** * @author Marcin Grzejszczak */ -@RunWith(MockitoJUnitRunner.class) public class SleuthHystrixConcurrencyStrategyTest { - @Mock Tracer tracer; + ArrayListSpanAccumulator spanReporter = new ArrayListSpanAccumulator(); + Tracer tracer = new DefaultTracer(new AlwaysSampler(), new Random(), + new DefaultSpanNamer(), new NoOpSpanLogger(), this.spanReporter); TraceKeys traceKeys = new TraceKeys(); @Before @After public void setup() { + ExceptionUtils.setFail(true); HystrixPlugins.reset(); + this.spanReporter.getSpans().clear(); } @Test @@ -94,6 +102,58 @@ public class SleuthHystrixConcurrencyStrategyTest { then(callable).isInstanceOf(SleuthHystrixConcurrencyStrategy.HystrixTraceCallable.class); } + @Test + public void should_add_trace_keys_when_span_is_created() + throws Exception { + SleuthHystrixConcurrencyStrategy strategy = new SleuthHystrixConcurrencyStrategy( + this.tracer, this.traceKeys); + Callable callable = strategy.wrapCallable(() -> "hello"); + + callable.call(); + + String asyncKey = this.traceKeys.getAsync().getPrefix() + + this.traceKeys.getAsync().getThreadNameKey(); + then(new ListOfSpans(this.spanReporter.getSpans())) + .hasASpanWithTagEqualTo(Span.SPAN_LOCAL_COMPONENT_TAG_NAME, "hystrix") + .hasASpanWithTagKeyEqualTo(asyncKey); + } + + @Test + public void should_add_trace_keys_when_span_is_continued() + throws Exception { + Span span = this.tracer.createSpan("new_span"); + SleuthHystrixConcurrencyStrategy strategy = new SleuthHystrixConcurrencyStrategy( + this.tracer, this.traceKeys); + Callable callable = strategy.wrapCallable(() -> "hello"); + + callable.call(); + + String asyncKey = this.traceKeys.getAsync().getPrefix() + + this.traceKeys.getAsync().getThreadNameKey(); + then(span) + .hasATag(Span.SPAN_LOCAL_COMPONENT_TAG_NAME, "hystrix") + .hasATagWithKey(asyncKey); + } + + @Test + public void should_not_override_trace_keys_when_span_is_continued() + throws Exception { + Span span = this.tracer.createSpan("new_span"); + String asyncKey = this.traceKeys.getAsync().getPrefix() + + this.traceKeys.getAsync().getThreadNameKey(); + this.tracer.addTag(Span.SPAN_LOCAL_COMPONENT_TAG_NAME, "foo"); + this.tracer.addTag(asyncKey, "bar"); + SleuthHystrixConcurrencyStrategy strategy = new SleuthHystrixConcurrencyStrategy( + this.tracer, this.traceKeys); + Callable callable = strategy.wrapCallable(() -> "hello"); + + callable.call(); + + then(span) + .hasATag(Span.SPAN_LOCAL_COMPONENT_TAG_NAME, "foo") + .hasATag(asyncKey, "bar"); + } + static class MyHystrixCommandExecutionHook extends HystrixCommandExecutionHook {} @SuppressWarnings("unchecked") static class MyHystrixConcurrencyStrategy extends HystrixConcurrencyStrategy {