From a8a8d4819f77b139e8153ae69676f5640591ded8 Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Wed, 12 Dec 2018 13:33:50 +0800 Subject: [PATCH 1/3] Bumps to brave that has nicer things --- pom.xml | 2 +- spring-cloud-sleuth-dependencies/pom.xml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index 627707b3c..7cedd01cb 100644 --- a/pom.xml +++ b/pom.xml @@ -273,7 +273,7 @@ Elmhurst.BUILD-SNAPSHOT 2.0.0.BUILD-SNAPSHOT 2.0.0.BUILD-SNAPSHOT - 5.5.2 + 5.6.0 2.0.0.RELEASE diff --git a/spring-cloud-sleuth-dependencies/pom.xml b/spring-cloud-sleuth-dependencies/pom.xml index 80ceb47fe..d53ba4731 100644 --- a/spring-cloud-sleuth-dependencies/pom.xml +++ b/spring-cloud-sleuth-dependencies/pom.xml @@ -30,7 +30,7 @@ spring-cloud-sleuth-dependencies Spring Cloud Sleuth Dependencies - 0.33.8 + 0.33.9 From 8979c343a5c04f562ae4edf1f6f782fa479e121f Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Mon, 21 Jan 2019 20:43:19 +0800 Subject: [PATCH 2/3] latest brave --- pom.xml | 2 +- spring-cloud-sleuth-dependencies/pom.xml | 2 +- spring-cloud-sleuth-samples/pom.xml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pom.xml b/pom.xml index 7cedd01cb..fc5f7232b 100644 --- a/pom.xml +++ b/pom.xml @@ -273,7 +273,7 @@ Elmhurst.BUILD-SNAPSHOT 2.0.0.BUILD-SNAPSHOT 2.0.0.BUILD-SNAPSHOT - 5.6.0 + 5.6.1 2.0.0.RELEASE diff --git a/spring-cloud-sleuth-dependencies/pom.xml b/spring-cloud-sleuth-dependencies/pom.xml index d53ba4731..67cf72b9b 100644 --- a/spring-cloud-sleuth-dependencies/pom.xml +++ b/spring-cloud-sleuth-dependencies/pom.xml @@ -30,7 +30,7 @@ spring-cloud-sleuth-dependencies Spring Cloud Sleuth Dependencies - 0.33.9 + 0.33.10 diff --git a/spring-cloud-sleuth-samples/pom.xml b/spring-cloud-sleuth-samples/pom.xml index b81f37097..6e44bee29 100644 --- a/spring-cloud-sleuth-samples/pom.xml +++ b/spring-cloud-sleuth-samples/pom.xml @@ -73,7 +73,7 @@ io.zipkin.zipkin2 zipkin - 2.11.10 + 2.12.0 From f6bc3e0fc3240c494bf0a1794d8f0105c95a16bf Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Thu, 21 Feb 2019 16:11:48 +0100 Subject: [PATCH 3/3] Added error tag for @Scheduled; fixes gh-1215 --- .../scheduling/TraceSchedulingAspect.java | 6 ++ .../scheduling/TracingOnScheduledTests.java | 102 +++++++++++++++--- 2 files changed, 96 insertions(+), 12 deletions(-) diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/scheduling/TraceSchedulingAspect.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/scheduling/TraceSchedulingAspect.java index de9250940..173e3109e 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/scheduling/TraceSchedulingAspect.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/scheduling/TraceSchedulingAspect.java @@ -24,6 +24,7 @@ import brave.Tracing; import org.aspectj.lang.ProceedingJoinPoint; import org.aspectj.lang.annotation.Around; import org.aspectj.lang.annotation.Aspect; + import org.springframework.cloud.sleuth.util.SpanNameUtil; /** @@ -66,6 +67,11 @@ public class TraceSchedulingAspect { span.tag(CLASS_KEY, pjp.getTarget().getClass().getSimpleName()); span.tag(METHOD_KEY, pjp.getSignature().getName()); return pjp.proceed(); + } catch(Throwable ex) { + String message = ex.getMessage() == null ? + ex.getClass().getSimpleName() : ex.getMessage(); + span.tag("error", message); + throw ex; } finally { span.finish(); } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/scheduling/TracingOnScheduledTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/scheduling/TracingOnScheduledTests.java index 856645c08..b942a4a1e 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/scheduling/TracingOnScheduledTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/scheduling/TracingOnScheduledTests.java @@ -27,6 +27,8 @@ import org.apache.commons.logging.LogFactory; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import zipkin2.reporter.Reporter; + import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.cloud.sleuth.instrument.DefaultTestAutoConfiguration; @@ -37,20 +39,24 @@ import org.springframework.scheduling.annotation.EnableScheduling; import org.springframework.scheduling.annotation.Scheduled; import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.junit4.SpringRunner; -import zipkin2.reporter.Reporter; import static java.util.concurrent.TimeUnit.SECONDS; import static org.assertj.core.api.BDDAssertions.then; import static org.awaitility.Awaitility.await; @RunWith(SpringRunner.class) -@SpringBootTest(classes = { ScheduledTestConfiguration.class }) +@SpringBootTest(classes = {ScheduledTestConfiguration.class}) @DirtiesContext public class TracingOnScheduledTests { - @Autowired TestBeanWithScheduledMethod beanWithScheduledMethod; - @Autowired TestBeanWithScheduledMethodToBeIgnored beanWithScheduledMethodToBeIgnored; - @Autowired ArrayListSpanReporter reporter; + @Autowired + TestBeanWithScheduledMethod beanWithScheduledMethod; + @Autowired + TestBeanWithScheduledMethodToBeIgnored beanWithScheduledMethodToBeIgnored; + @Autowired + TestBeanWithScheduledMethodThatThrowsAnException throwsAnException; + @Autowired + ArrayListSpanReporter reporter; @Before public void setup() { @@ -60,12 +66,20 @@ public class TracingOnScheduledTests { @Test public void should_have_span_set_after_scheduled_method_has_been_executed() { - await().atMost( 10, SECONDS).untilAsserted(() -> { + await().atMost(10, SECONDS).untilAsserted(() -> { then(this.beanWithScheduledMethod.isExecuted()).isTrue(); spanIsSetOnAScheduledMethod(); }); } + @Test + public void should_have_span_set_with_error_tag() { + await().atMost(10, SECONDS).untilAsserted(() -> { + then(this.throwsAnException.isExecuted()).isTrue(); + spanIsSetOnAScheduledMethodWithErrorTag(); + }); + } + @Test public void should_have_a_new_span_set_each_time_a_scheduled_method_has_been_executed() { final Span firstSpan = this.beanWithScheduledMethod.getSpan(); @@ -89,10 +103,28 @@ public class TracingOnScheduledTests { .getSpan(); then(storedSpan).isNotNull(); then(storedSpan.context().traceId()).isNotNull(); - then(this.reporter.getSpans().get(0).tags()) + zipkin2.Span foundSpan = this.reporter.getSpans().stream() + .filter(span -> !span.tags().containsKey("error")) + .findFirst().orElseThrow(() -> new AssertionError("Span is missing")); + then(foundSpan.tags()) .contains(new AbstractMap.SimpleEntry<>("class", "TestBeanWithScheduledMethod"), new AbstractMap.SimpleEntry<>("method", "scheduledMethod")); - then(this.reporter.getSpans().get(0).durationAsLong()).isGreaterThan(0L); + then(foundSpan.durationAsLong()).isGreaterThan(0L); + } + + private void spanIsSetOnAScheduledMethodWithErrorTag() { + Span storedSpan = TracingOnScheduledTests.this.beanWithScheduledMethod + .getSpan(); + then(storedSpan).isNotNull(); + then(storedSpan.context().traceId()).isNotNull(); + zipkin2.Span foundSpan = this.reporter.getSpans().stream() + .filter(span -> span.tags().containsKey("error")) + .findFirst().orElseThrow(() -> new AssertionError("Span is missing")); + then(foundSpan.tags()) + .contains(new AbstractMap.SimpleEntry<>("class", "TestBeanWithScheduledMethodThatThrowsAnException"), + new AbstractMap.SimpleEntry<>("method", "scheduledMethod")); + then(foundSpan.durationAsLong()).isGreaterThan(0L); + then(foundSpan.tags().get("error")).isNotEmpty(); } private void differentSpanHasBeenSetThan(final Span spanToCompare) { @@ -107,19 +139,28 @@ public class TracingOnScheduledTests { @EnableScheduling class ScheduledTestConfiguration { - @Bean Reporter testRepoter() { + @Bean + Reporter testRepoter() { return new ArrayListSpanReporter(); } - @Bean TestBeanWithScheduledMethod testBeanWithScheduledMethod(Tracing tracing) { + @Bean + TestBeanWithScheduledMethod testBeanWithScheduledMethod(Tracing tracing) { return new TestBeanWithScheduledMethod(tracing); } - @Bean TestBeanWithScheduledMethodToBeIgnored testBeanWithScheduledMethodToBeIgnored(Tracing tracing) { + @Bean + TestBeanWithScheduledMethodToBeIgnored testBeanWithScheduledMethodToBeIgnored(Tracing tracing) { return new TestBeanWithScheduledMethodToBeIgnored(tracing); } - @Bean Sampler alwaysSampler() { + @Bean + TestBeanWithScheduledMethodThatThrowsAnException throwsAnException(Tracing tracing) { + return new TestBeanWithScheduledMethodThatThrowsAnException(tracing); + } + + @Bean + Sampler alwaysSampler() { return Sampler.ALWAYS_SAMPLE; } @@ -161,6 +202,43 @@ class TestBeanWithScheduledMethod { } } +class TestBeanWithScheduledMethodThatThrowsAnException { + + private static final Log log = LogFactory.getLog(TestBeanWithScheduledMethod.class); + + private final Tracing tracing; + + Span span; + + AtomicBoolean executed = new AtomicBoolean(false); + + TestBeanWithScheduledMethodThatThrowsAnException(Tracing tracing) { + this.tracing = tracing; + } + + @Scheduled(fixedDelay = 1L) + public void scheduledMethod() { + log.info("Running the scheduled method"); + this.span = this.tracing.tracer().currentSpan(); + log.info("Stored the span " + this.span + " as current span"); + this.executed.set(true); + throw new RuntimeException("HELLO"); + } + + public Span getSpan() { + return this.span; + } + + public AtomicBoolean isExecuted() { + return this.executed; + } + + public void clear() { + this.span = null; + this.executed.set(false); + } +} + class TestBeanWithScheduledMethodToBeIgnored { private final Tracing tracing;