From 9829ff99033a869dcaede89e3fa30f45e0efd1e4 Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Fri, 26 Jun 2015 12:37:05 -0600 Subject: [PATCH] Move to event driven interceptors. use Span{Started|Stopped}Event rather than SpanStartListener/SpanReceiver --- .../sleuth/ArrayListSpanAccumulator.java | 21 ++++++++ .../cloud/sleuth/DefaultTrace.java | 27 +++------- .../cloud/sleuth/SpanReceiver.java | 13 ----- .../cloud/sleuth/SpanStartListener.java | 9 ---- .../springframework/cloud/sleuth/Trace.java | 2 - .../cloud/sleuth/TraceAutoConfiguration.java | 8 +-- .../cloud/sleuth/TraceScope.java | 11 ++-- .../cloud/sleuth/event/SpanStartedEvent.java | 19 +++++++ .../cloud/sleuth/event/SpanStoppedEvent.java | 19 +++++++ .../receiver/ArrayListSpanReceiver.java | 29 ----------- .../slf4j/SleuthSlf4jAutoConfiguration.java | 8 +-- ...ner.java => Slf4jSpanStartedListener.java} | 8 +-- ...ver.java => Slf4jSpanStoppedListener.java} | 19 +++---- .../cloud/sleuth/DefaultTraceTests.java | 52 +++++++++---------- 14 files changed, 117 insertions(+), 128 deletions(-) create mode 100644 spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/ArrayListSpanAccumulator.java delete mode 100644 spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/SpanReceiver.java delete mode 100644 spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/SpanStartListener.java create mode 100644 spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/event/SpanStartedEvent.java create mode 100644 spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/event/SpanStoppedEvent.java delete mode 100644 spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/receiver/ArrayListSpanReceiver.java rename spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/slf4j/{Slf4jSpanStartListener.java => Slf4jSpanStartedListener.java} (58%) rename spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/slf4j/{Slf4jSpanReceiver.java => Slf4jSpanStoppedListener.java} (51%) diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/ArrayListSpanAccumulator.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/ArrayListSpanAccumulator.java new file mode 100644 index 000000000..543ac1d04 --- /dev/null +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/ArrayListSpanAccumulator.java @@ -0,0 +1,21 @@ +package org.springframework.cloud.sleuth; + +import java.util.ArrayList; + +import lombok.Value; + +import org.springframework.cloud.sleuth.event.SpanStoppedEvent; +import org.springframework.context.ApplicationListener; + +/** + * @author Spencer Gibb + */ +@Value +public class ArrayListSpanAccumulator implements ApplicationListener { + private final ArrayList spans = new ArrayList<>(); + + @Override + public void onApplicationEvent(SpanStoppedEvent event) { + spans.add(event.getSpan()); + } +} diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/DefaultTrace.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/DefaultTrace.java index 9b5507595..1da2c7d5b 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/DefaultTrace.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/DefaultTrace.java @@ -2,9 +2,11 @@ package org.springframework.cloud.sleuth; import static org.springframework.cloud.sleuth.Utils.error; -import java.util.Collection; import java.util.Collections; +import org.springframework.cloud.sleuth.event.SpanStartedEvent; +import org.springframework.context.ApplicationEventPublisher; + /** * @author Spencer Gibb */ @@ -14,16 +16,13 @@ public class DefaultTrace implements Trace { private final IdGenerator idGenerator; - private final Collection spanStartListeners; - private final Collection spanReceivers; + private final ApplicationEventPublisher publisher; public DefaultTrace(Sampler defaultSampler, IdGenerator idGenerator, - Collection spanStartListeners, - Collection spanReceivers) { + ApplicationEventPublisher publisher) { this.defaultSampler = defaultSampler; this.idGenerator = idGenerator; - this.spanStartListeners = spanStartListeners; - this.spanReceivers = spanReceivers; + this.publisher = publisher; } @Override @@ -102,9 +101,7 @@ public class DefaultTrace implements Trace { protected TraceScope doStart(Span span) { if (span != null) { - for (SpanStartListener listener : spanStartListeners) { - listener.startSpan(span); - } + publisher.publishEvent(new SpanStartedEvent(this, span)); } return continueSpan(span); } @@ -115,7 +112,7 @@ public class DefaultTrace implements Trace { if (span == null) return NullScope.INSTANCE; Span oldSpan = getCurrentSpan(); TraceContextHolder.setCurrentSpan(span); - return new TraceScope(this, span, oldSpan); + return new TraceScope(this.publisher, span, oldSpan); } protected Span getCurrentSpan() { @@ -129,12 +126,4 @@ public class DefaultTrace implements Trace { s.addKVAnnotation(key, value); } } - - //TODO: rename? this is the end of a Span lifecycle - @Override - public void deliver(Span span) { - for (SpanReceiver receiver : spanReceivers) { - receiver.receiveSpan(span); - } - } } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/SpanReceiver.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/SpanReceiver.java deleted file mode 100644 index 9d0d54702..000000000 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/SpanReceiver.java +++ /dev/null @@ -1,13 +0,0 @@ -package org.springframework.cloud.sleuth; - -import java.io.Closeable; - -/** - * The collector within a process that is the destination of Spans when a trace is running. - */ -public interface SpanReceiver extends Closeable { - /** - * Called when a Span is stopped and can now be stored. - */ - public void receiveSpan(Span span); -} diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/SpanStartListener.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/SpanStartListener.java deleted file mode 100644 index 2958cbe1c..000000000 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/SpanStartListener.java +++ /dev/null @@ -1,9 +0,0 @@ -package org.springframework.cloud.sleuth; - -/** - * @author Spencer Gibb - */ -//TODO: rename? -public interface SpanStartListener { - void startSpan(Span span); -} diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/Trace.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/Trace.java index 1a3ae11c4..35e5e8039 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/Trace.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/Trace.java @@ -76,6 +76,4 @@ public interface Trace { * Adds a data annotation to the current span if tracing is currently on. */ void addKVAnnotation(String key, String value); - - void deliver(Span span); } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/TraceAutoConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/TraceAutoConfiguration.java index a46b743b7..84ff1e3b6 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/TraceAutoConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/TraceAutoConfiguration.java @@ -2,11 +2,10 @@ package org.springframework.cloud.sleuth; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.cloud.sleuth.sampler.IsTracingSampler; +import org.springframework.context.ApplicationEventPublisher; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; -import java.util.Collection; - /** * @author Spencer Gibb */ @@ -27,7 +26,8 @@ public class TraceAutoConfiguration { @Bean @ConditionalOnMissingBean - public Trace trace(Sampler sampler, IdGenerator idGenerator, Collection listeners, Collection receivers) { - return new DefaultTrace(sampler, idGenerator, listeners, receivers); + public Trace trace(Sampler sampler, IdGenerator idGenerator, + ApplicationEventPublisher publisher) { + return new DefaultTrace(sampler, idGenerator, publisher); } } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/TraceScope.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/TraceScope.java index b33b1af4a..6ff9fc130 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/TraceScope.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/TraceScope.java @@ -4,6 +4,8 @@ import java.io.Closeable; import lombok.Data; import lombok.SneakyThrows; +import org.springframework.cloud.sleuth.event.SpanStoppedEvent; +import org.springframework.context.ApplicationEventPublisher; /** * @author Spencer Gibb @@ -11,7 +13,7 @@ import lombok.SneakyThrows; @Data public class TraceScope implements Closeable { - private final Trace trace; + private final ApplicationEventPublisher publisher; /** * the span for this scope @@ -25,8 +27,8 @@ public class TraceScope implements Closeable { private boolean detached = false; - public TraceScope(Trace trace, Span span, Span savedSpan) { - this.trace = trace; + public TraceScope(ApplicationEventPublisher publisher, Span span, Span savedSpan) { + this.publisher = publisher; this.span = span; this.savedSpan = savedSpan; } @@ -72,8 +74,7 @@ public class TraceScope implements Closeable { "probably forgotten to close or detach " + cur); } else { span.stop(); - //TODO: use ApplicationEvents here? - trace.deliver(span); + this.publisher.publishEvent(new SpanStoppedEvent(this, span)); TraceContextHolder.setCurrentSpan(savedSpan); } } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/event/SpanStartedEvent.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/event/SpanStartedEvent.java new file mode 100644 index 000000000..49e5a7cdb --- /dev/null +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/event/SpanStartedEvent.java @@ -0,0 +1,19 @@ +package org.springframework.cloud.sleuth.event; + +import lombok.Value; +import org.springframework.cloud.sleuth.Span; +import org.springframework.context.ApplicationEvent; + +/** + * @author Spencer Gibb + */ +@Value +public class SpanStartedEvent extends ApplicationEvent { + + private final Span span; + + public SpanStartedEvent(Object source, Span span) { + super(source); + this.span = span; + } +} diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/event/SpanStoppedEvent.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/event/SpanStoppedEvent.java new file mode 100644 index 000000000..5272bfc92 --- /dev/null +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/event/SpanStoppedEvent.java @@ -0,0 +1,19 @@ +package org.springframework.cloud.sleuth.event; + +import lombok.Value; +import org.springframework.cloud.sleuth.Span; +import org.springframework.context.ApplicationEvent; + +/** + * @author Spencer Gibb + */ +@Value +public class SpanStoppedEvent extends ApplicationEvent { + + private final Span span; + + public SpanStoppedEvent(Object source, Span span) { + super(source); + this.span = span; + } +} diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/receiver/ArrayListSpanReceiver.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/receiver/ArrayListSpanReceiver.java deleted file mode 100644 index 167ab3ee9..000000000 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/receiver/ArrayListSpanReceiver.java +++ /dev/null @@ -1,29 +0,0 @@ -package org.springframework.cloud.sleuth.receiver; - -import org.springframework.cloud.sleuth.Span; -import org.springframework.cloud.sleuth.SpanReceiver; - -import java.io.IOException; -import java.util.ArrayList; -import java.util.List; - -/** - * @author Spencer Gibb - */ -public class ArrayListSpanReceiver implements SpanReceiver { - private final ArrayList spans = new ArrayList<>(); - - @Override - public void receiveSpan(Span span) { - spans.add(span); - } - - public List getSpans() { - return spans; - } - - @Override - public void close() throws IOException { - - } -} diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/slf4j/SleuthSlf4jAutoConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/slf4j/SleuthSlf4jAutoConfiguration.java index ea0781be7..f17d0cba9 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/slf4j/SleuthSlf4jAutoConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/slf4j/SleuthSlf4jAutoConfiguration.java @@ -13,12 +13,12 @@ import org.springframework.context.annotation.Configuration; public class SleuthSlf4jAutoConfiguration { @Bean - public Slf4jSpanStartListener slf4jSpanStartListener() { - return new Slf4jSpanStartListener(); + public Slf4jSpanStartedListener slf4jSpanStartedListener() { + return new Slf4jSpanStartedListener(); } @Bean - public Slf4jSpanReceiver slf4jSpanReceiver() { - return new Slf4jSpanReceiver(); + public Slf4jSpanStoppedListener slf4jSpanStoppedListener() { + return new Slf4jSpanStoppedListener(); } } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/slf4j/Slf4jSpanStartListener.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/slf4j/Slf4jSpanStartedListener.java similarity index 58% rename from spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/slf4j/Slf4jSpanStartListener.java rename to spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/slf4j/Slf4jSpanStartedListener.java index 98ab81ace..6653a4115 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/slf4j/Slf4jSpanStartListener.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/slf4j/Slf4jSpanStartedListener.java @@ -4,17 +4,19 @@ import lombok.extern.slf4j.Slf4j; import org.slf4j.MDC; import org.springframework.cloud.sleuth.Span; -import org.springframework.cloud.sleuth.SpanStartListener; import org.springframework.cloud.sleuth.Trace; +import org.springframework.cloud.sleuth.event.SpanStartedEvent; +import org.springframework.context.ApplicationListener; /** * @author Spencer Gibb */ @Slf4j -public class Slf4jSpanStartListener implements SpanStartListener { +public class Slf4jSpanStartedListener implements ApplicationListener { @Override - public void startSpan(Span span) { + public void onApplicationEvent(SpanStartedEvent event) { + Span span = event.getSpan(); //TODO: what log level? log.info("Starting span: {}", span); MDC.put(Trace.SPAN_ID_NAME, span.getSpanId()); diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/slf4j/Slf4jSpanReceiver.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/slf4j/Slf4jSpanStoppedListener.java similarity index 51% rename from spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/slf4j/Slf4jSpanReceiver.java rename to spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/slf4j/Slf4jSpanStoppedListener.java index 078c24889..b5b12d053 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/slf4j/Slf4jSpanReceiver.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/slf4j/Slf4jSpanStoppedListener.java @@ -4,28 +4,21 @@ import static org.springframework.cloud.sleuth.Trace.SPAN_ID_NAME; import static org.springframework.cloud.sleuth.Trace.TRACE_ID_NAME; import lombok.extern.slf4j.Slf4j; -import org.slf4j.MDC; -import org.springframework.cloud.sleuth.Span; -import org.springframework.cloud.sleuth.SpanReceiver; -import javax.annotation.PostConstruct; -import java.io.IOException; +import org.slf4j.MDC; +import org.springframework.cloud.sleuth.event.SpanStoppedEvent; +import org.springframework.context.ApplicationListener; /** * @author Spencer Gibb */ @Slf4j -public class Slf4jSpanReceiver implements SpanReceiver { +public class Slf4jSpanStoppedListener implements ApplicationListener { @Override - public void receiveSpan(Span span) { + public void onApplicationEvent(SpanStoppedEvent event) { //TODO: what should this log level be? - log.info("Received span: {}", span); + log.info("Received span: {}", event.getSpan()); MDC.remove(SPAN_ID_NAME); MDC.remove(TRACE_ID_NAME); } - - @PostConstruct - @Override - public void close() throws IOException { - } } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/DefaultTraceTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/DefaultTraceTests.java index a85f21995..199215f39 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/DefaultTraceTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/DefaultTraceTests.java @@ -1,19 +1,24 @@ package org.springframework.cloud.sleuth; import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.notNullValue; import static org.junit.Assert.assertThat; +import static org.mockito.Mockito.atLeast; +import static org.mockito.Mockito.isA; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import java.util.ArrayList; -import java.util.Collections; import java.util.List; -import lombok.Data; - import org.junit.Test; -import org.springframework.cloud.sleuth.receiver.ArrayListSpanReceiver; +import org.mockito.ArgumentCaptor; +import org.springframework.cloud.sleuth.event.SpanStartedEvent; +import org.springframework.cloud.sleuth.event.SpanStoppedEvent; import org.springframework.cloud.sleuth.sampler.AlwaysSampler; import org.springframework.cloud.sleuth.sampler.IsTracingSampler; +import org.springframework.context.ApplicationEvent; +import org.springframework.context.ApplicationEventPublisher; /** * @author Spencer Gibb @@ -27,15 +32,10 @@ public class DefaultTraceTests { @Test public void tracingWorks() { - ArrayListSpanReceiver spanReceiver = new ArrayListSpanReceiver(); - ListSpanStartListener listener = new ListSpanStartListener(); - List startListeners = Collections - . singletonList(listener); - List spanReceivers = Collections - . singletonList(spanReceiver); + ApplicationEventPublisher publisher = mock(ApplicationEventPublisher.class); DefaultTrace trace = new DefaultTrace(new IsTracingSampler(), - new RandomUuidGenerator(), startListeners, spanReceivers); + new RandomUuidGenerator(), publisher); TraceScope scope = trace.startSpan(CREATE_SIMPLE_TRACE, new AlwaysSampler()); try { @@ -45,12 +45,20 @@ public class DefaultTraceTests { scope.close(); } - List startedSpans = listener.getSpans(); - assertThat("startedSpans was null", startedSpans, is(notNullValue())); - assertThat("startedSpans was wrong size", startedSpans.size(), is(NUM_SPANS)); + verify(publisher, times(NUM_SPANS)).publishEvent(isA(SpanStartedEvent.class)); + verify(publisher, times(NUM_SPANS)).publishEvent(isA(SpanStoppedEvent.class)); + + ArgumentCaptor captor = ArgumentCaptor + .forClass(ApplicationEvent.class); + verify(publisher, atLeast(NUM_SPANS)).publishEvent(captor.capture()); + + List spans = new ArrayList<>(); + for (ApplicationEvent event : captor.getAllValues()) { + if (event instanceof SpanStoppedEvent) { + spans.add(((SpanStoppedEvent) event).getSpan()); + } + } - List spans = spanReceiver.getSpans(); - assertThat("spans was null", spans, is(notNullValue())); assertThat("spans was wrong size", spans.size(), is(NUM_SPANS)); Span root = assertSpan(spans, null, CREATE_SIMPLE_TRACE); @@ -109,14 +117,4 @@ public class DefaultTraceTests { cur.close(); } } - - @Data - class ListSpanStartListener implements SpanStartListener { - private ArrayList spans = new ArrayList<>(); - - @Override - public void startSpan(Span span) { - spans.add(span); - } - } }