From 97ae6151e2866caa8ffe705f24a3765ebffc08f4 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Tue, 28 Jul 2015 17:40:27 +0100 Subject: [PATCH] Use listeners instead of custom callbacks --- .../cloud/sleuth/TraceScope.java | 20 --------- .../cloud/sleuth/event/SpanStartedEvent.java | 4 ++ .../cloud/sleuth/event/SpanStoppedEvent.java | 4 ++ .../TraceWebClientAutoConfiguration.java | 9 ++-- .../slf4j/Slf4jSpanStartedListener.java | 4 +- .../cloud/sleuth/sample/SampleController.java | 17 ++++---- .../zipkin/ZipkinAutoConfiguration.java | 7 +--- ...pkinTrace.java => ZipkinSpanListener.java} | 41 ++++++++----------- 8 files changed, 42 insertions(+), 64 deletions(-) rename spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/{ZipkinTrace.java => ZipkinSpanListener.java} (73%) 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 f657fa3f7..61374d30c 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 @@ -1,8 +1,6 @@ package org.springframework.cloud.sleuth; import java.io.Closeable; -import java.util.ArrayList; -import java.util.List; import lombok.SneakyThrows; import lombok.Value; @@ -33,11 +31,6 @@ public class TraceScope implements Closeable { */ private final Span savedSpan; - /** - * List of callbacks to run on close. - */ - private List callbacks = new ArrayList(); - @NonFinal private boolean detached = false; @@ -73,12 +66,6 @@ public class TraceScope implements Closeable { return this.span; } - public void register(Runnable callback) { - if (!this.callbacks .contains(callback)) { - this.callbacks.add(callback); - } - } - @Override @SneakyThrows public void close() { @@ -86,13 +73,6 @@ public class TraceScope implements Closeable { return; } this.detached = true; - for (Runnable callback : this.callbacks) { - try { - callback.run(); - } catch (Throwable e) { - log.error("Error with callback on close", e); - } - } Span cur = TraceContextHolder.getCurrentSpan(); if (cur != this.span) { ExceptionUtils.error("Tried to close trace span " + this.span + " but " + 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 index 49e5a7cdb..8a0f9314b 100644 --- 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 @@ -1,6 +1,8 @@ package org.springframework.cloud.sleuth.event; +import lombok.EqualsAndHashCode; import lombok.Value; + import org.springframework.cloud.sleuth.Span; import org.springframework.context.ApplicationEvent; @@ -8,6 +10,8 @@ import org.springframework.context.ApplicationEvent; * @author Spencer Gibb */ @Value +@EqualsAndHashCode(callSuper=false) +@SuppressWarnings("serial") public class SpanStartedEvent extends ApplicationEvent { private final 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 index 5272bfc92..1b45c066e 100644 --- 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 @@ -1,6 +1,8 @@ package org.springframework.cloud.sleuth.event; +import lombok.EqualsAndHashCode; import lombok.Value; + import org.springframework.cloud.sleuth.Span; import org.springframework.context.ApplicationEvent; @@ -8,6 +10,8 @@ import org.springframework.context.ApplicationEvent; * @author Spencer Gibb */ @Value +@EqualsAndHashCode(callSuper=false) +@SuppressWarnings("serial") public class SpanStoppedEvent extends ApplicationEvent { private final Span span; diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceWebClientAutoConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceWebClientAutoConfiguration.java index c45cabd06..98ab01ac8 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceWebClientAutoConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/client/TraceWebClientAutoConfiguration.java @@ -1,7 +1,8 @@ package org.springframework.cloud.sleuth.instrument.web.client; +import javax.annotation.PostConstruct; + import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.boot.autoconfigure.AutoConfigureAfter; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; @@ -9,8 +10,6 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.web.client.RestTemplate; -import javax.annotation.PostConstruct; - /** * @author Spencer Gibb */ @@ -42,8 +41,8 @@ public class TraceWebClientAutoConfiguration { @PostConstruct public void init() { - if (restTemplate != null) { - restTemplate.getInterceptors().add(traceRestTemplateInterceptor); + if (this.restTemplate != null) { + this.restTemplate.getInterceptors().add(this.traceRestTemplateInterceptor); } } } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/slf4j/Slf4jSpanStartedListener.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/slf4j/Slf4jSpanStartedListener.java index 6653a4115..7708340e0 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/slf4j/Slf4jSpanStartedListener.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/slf4j/Slf4jSpanStartedListener.java @@ -17,9 +17,9 @@ public class Slf4jSpanStartedListener implements ApplicationListener { +ApplicationListener { @Autowired private RestTemplate restTemplate; @Autowired @@ -36,7 +37,7 @@ class SampleController implements final Random random = new Random(); Thread.sleep(random.nextInt(1000)); - String s = restTemplate.getForObject("http://localhost:" + port + "/hi2", + String s = this.restTemplate.getForObject("http://localhost:" + this.port + "/hi2", String.class); return "hi/" + s; } @@ -64,19 +65,19 @@ class SampleController implements @SneakyThrows @RequestMapping("/traced") public String traced() { - TraceScope scope = trace.startSpan("customTraceEndpoint", new AlwaysSampler()); + TraceScope scope = this.trace.startSpan("customTraceEndpoint", new AlwaysSampler()); final Random random = new Random(); int millis = random.nextInt(1000); log.info("Sleeping for {} millis", millis); Thread.sleep(millis); - String s = restTemplate.getForObject("http://localhost:" + port + "/hi2", String.class); + String s = this.restTemplate.getForObject("http://localhost:" + this.port + "/hi2", String.class); scope.close(); return "hi/" + s; } @Override public void onApplicationEvent(EmbeddedServletContainerInitializedEvent event) { - port = event.getEmbeddedServletContainer().getPort(); + this.port = event.getEmbeddedServletContainer().getPort(); } } diff --git a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ZipkinAutoConfiguration.java b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ZipkinAutoConfiguration.java index 6589318e4..92cb234d7 100644 --- a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ZipkinAutoConfiguration.java +++ b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ZipkinAutoConfiguration.java @@ -7,9 +7,6 @@ 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.context.properties.EnableConfigurationProperties; -import org.springframework.cloud.sleuth.IdGenerator; -import org.springframework.cloud.sleuth.Sampler; -import org.springframework.context.ApplicationEventPublisher; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; @@ -67,8 +64,8 @@ public class ZipkinAutoConfiguration { } @Bean - public ZipkinTrace zipkinTrace(ServerTracer serverTracer, Sampler sampler, IdGenerator idGenerator, ApplicationEventPublisher publisher) { - return new ZipkinTrace(serverTracer, sampler, idGenerator, publisher); + public ZipkinSpanListener zipkinTrace(ServerTracer serverTracer) { + return new ZipkinSpanListener(serverTracer); } @Configuration diff --git a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ZipkinTrace.java b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ZipkinSpanListener.java similarity index 73% rename from spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ZipkinTrace.java rename to spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ZipkinSpanListener.java index c703d8721..aa544f709 100644 --- a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ZipkinTrace.java +++ b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ZipkinSpanListener.java @@ -3,12 +3,10 @@ package org.springframework.cloud.sleuth.zipkin; import lombok.Data; import lombok.extern.apachecommons.CommonsLog; -import org.springframework.cloud.sleuth.IdGenerator; -import org.springframework.cloud.sleuth.Sampler; import org.springframework.cloud.sleuth.Span; -import org.springframework.cloud.sleuth.TraceScope; -import org.springframework.cloud.sleuth.trace.DefaultTrace; -import org.springframework.context.ApplicationEventPublisher; +import org.springframework.cloud.sleuth.event.SpanStartedEvent; +import org.springframework.cloud.sleuth.event.SpanStoppedEvent; +import org.springframework.context.event.EventListener; import com.github.kristofa.brave.ServerTracer; @@ -16,30 +14,25 @@ import com.github.kristofa.brave.ServerTracer; * @author Spencer Gibb */ @CommonsLog -public class ZipkinTrace extends DefaultTrace { +public class ZipkinSpanListener { private final ServerTracer serverTracer; - @Override - protected TraceScope doStart(final Span span) { - preTrace(span); - TraceScope scope = super.doStart(span); - scope.register(new Runnable() { - @Override - public void run() { - postTrace(span); - } - }); - return scope; - } - - public ZipkinTrace(ServerTracer serverTracer, Sampler defaultSampler, - IdGenerator idGenerator, ApplicationEventPublisher publisher) { - super(defaultSampler, idGenerator, publisher); + public ZipkinSpanListener(ServerTracer serverTracer) { this.serverTracer = serverTracer; } - public void preTrace(Span context) { + @EventListener + public void start(SpanStartedEvent event) { + preTrace(event.getSpan()); + } + + @EventListener + public void start(SpanStoppedEvent event) { + postTrace(event.getSpan()); + } + + protected void preTrace(Span context) { final TraceData traceData = getTraceData(context); this.serverTracer.clearCurrentSpan(); @@ -79,7 +72,7 @@ public class ZipkinTrace extends DefaultTrace { return context.getName(); } - public void postTrace(Span context) { + protected void postTrace(Span context) { // We can submit this in any case. When server state is not set or // we should not trace this request nothing will happen. log.debug("Sending server send.");