From b3c78c139dab25af6420d5ed43b5dc6c98221263 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Sat, 2 Apr 2016 00:04:28 +0200 Subject: [PATCH] Fixed closing of Zuul span, added local component span for zuul fixes #242 --- .../instrument/zuul/TracePostZuulFilter.java | 11 ++-- .../instrument/zuul/TracePreZuulFilter.java | 22 ++++++-- .../zuul/TraceZuulAutoConfiguration.java | 5 +- .../zuul/TracePostZuulFilterTests.java | 3 +- .../zuul/TracePreZuulFilterTests.java | 50 ++++++++++++++++++- 5 files changed, 76 insertions(+), 15 deletions(-) diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TracePostZuulFilter.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TracePostZuulFilter.java index b23379ccd..79863acdd 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TracePostZuulFilter.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TracePostZuulFilter.java @@ -17,7 +17,7 @@ package org.springframework.cloud.sleuth.instrument.zuul; import org.springframework.cloud.sleuth.Span; -import org.springframework.cloud.sleuth.SpanAccessor; +import org.springframework.cloud.sleuth.Tracer; import com.netflix.zuul.ZuulFilter; @@ -29,10 +29,10 @@ import com.netflix.zuul.ZuulFilter; */ public class TracePostZuulFilter extends ZuulFilter { - private final SpanAccessor spanAccessor; + private final Tracer tracer; - public TracePostZuulFilter(SpanAccessor spanAccessor) { - this.spanAccessor = spanAccessor; + public TracePostZuulFilter(Tracer tracer) { + this.tracer = tracer; } @Override @@ -44,6 +44,7 @@ public class TracePostZuulFilter extends ZuulFilter { public Object run() { // TODO: the client sent event should come from the client not the filter! getCurrentSpan().logEvent(Span.CLIENT_RECV); + this.tracer.close(getCurrentSpan()); return null; } @@ -58,6 +59,6 @@ public class TracePostZuulFilter extends ZuulFilter { } private Span getCurrentSpan() { - return this.spanAccessor.getCurrentSpan(); + return this.tracer.getCurrentSpan(); } } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TracePreZuulFilter.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TracePreZuulFilter.java index ceb641721..802b4d7e7 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TracePreZuulFilter.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TracePreZuulFilter.java @@ -20,7 +20,9 @@ import org.springframework.cloud.sleuth.Span; import org.springframework.cloud.sleuth.SpanInjector; import org.springframework.cloud.sleuth.Tracer; +import com.netflix.zuul.ExecutionStatus; import com.netflix.zuul.ZuulFilter; +import com.netflix.zuul.ZuulFilterResult; import com.netflix.zuul.context.RequestContext; /** @@ -32,6 +34,8 @@ import com.netflix.zuul.context.RequestContext; */ public class TracePreZuulFilter extends ZuulFilter { + private static final String ZUUL_COMPONENT = "zuul"; + private final Tracer tracer; private final SpanInjector spanInjector; @@ -47,12 +51,22 @@ public class TracePreZuulFilter extends ZuulFilter { @Override public Object run() { + getCurrentSpan().logEvent(Span.CLIENT_SEND); + return null; + } + + @Override + public ZuulFilterResult runFilter() { RequestContext ctx = RequestContext.getCurrentContext(); Span span = getCurrentSpan(); - this.spanInjector.inject(span, ctx); - // TODO: the client sent event should come from the client not the filter! - span.logEvent(Span.CLIENT_SEND); - return null; + Span newSpan = this.tracer.createSpan(span.getName(), span); + newSpan.tag(Span.SPAN_LOCAL_COMPONENT_TAG_NAME, ZUUL_COMPONENT); + this.spanInjector.inject(newSpan, ctx); + ZuulFilterResult result = super.runFilter(); + if (ExecutionStatus.SUCCESS != result.getStatus()) { + this.tracer.close(newSpan); + } + return result; } private Span getCurrentSpan() { diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TraceZuulAutoConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TraceZuulAutoConfiguration.java index efb6cdffc..f9cc68514 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TraceZuulAutoConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/zuul/TraceZuulAutoConfiguration.java @@ -22,7 +22,6 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication; import org.springframework.cloud.netflix.ribbon.SpringClientFactory; -import org.springframework.cloud.sleuth.SpanAccessor; import org.springframework.cloud.sleuth.SpanInjector; import org.springframework.cloud.sleuth.Tracer; import org.springframework.cloud.sleuth.autoconfig.TraceAutoConfiguration; @@ -63,8 +62,8 @@ public class TraceZuulAutoConfiguration { @Bean @ConditionalOnMissingBean - public TracePostZuulFilter tracePostZuulFilter(SpanAccessor accessor) { - return new TracePostZuulFilter(accessor); + public TracePostZuulFilter tracePostZuulFilter(Tracer tracer) { + return new TracePostZuulFilter(tracer); } @Bean diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/zuul/TracePostZuulFilterTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/zuul/TracePostZuulFilterTests.java index 01238c71f..a2bd48a6d 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/zuul/TracePostZuulFilterTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/zuul/TracePostZuulFilterTests.java @@ -52,10 +52,11 @@ public class TracePostZuulFilterTests { } @Test - public void filterPublishesEvent() throws Exception { + public void filterPublishesEventAndClosesSpan() throws Exception { Span span = this.tracer.createSpan("http:start"); this.filter.run(); then(span).hasLoggedAnEvent(Span.CLIENT_RECV); + then(this.tracer.getCurrentSpan()).isNull(); } } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/zuul/TracePreZuulFilterTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/zuul/TracePreZuulFilterTests.java index 428ee7229..5a26ec59d 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/zuul/TracePreZuulFilterTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/zuul/TracePreZuulFilterTests.java @@ -17,6 +17,7 @@ package org.springframework.cloud.sleuth.instrument.zuul; import java.util.Random; +import java.util.concurrent.atomic.AtomicReference; import org.junit.After; import org.junit.Before; @@ -31,6 +32,7 @@ import org.springframework.cloud.sleuth.trace.DefaultTracer; import org.springframework.cloud.sleuth.trace.TestSpanContextHolder; import com.netflix.zuul.context.RequestContext; +import com.netflix.zuul.monitoring.MonitoringHelper; import static org.assertj.core.api.BDDAssertions.then; @@ -45,6 +47,11 @@ public class TracePreZuulFilterTests { private TracePreZuulFilter filter = new TracePreZuulFilter(this.tracer, new RequestContextInjector()); + @Before + public void setup() { + MonitoringHelper.initMocks(); + } + @After @Before public void clean() { @@ -56,7 +63,7 @@ public class TracePreZuulFilterTests { public void filterAddsHeaders() throws Exception { this.tracer.createSpan("http:start"); - this.filter.run(); + this.filter.runFilter(); RequestContext ctx = RequestContext.getCurrentContext(); then(ctx.getZuulRequestHeaders().get(Span.TRACE_ID_NAME)) @@ -69,7 +76,7 @@ public class TracePreZuulFilterTests { public void notSampledIfNotExportable() throws Exception { this.tracer.createSpan("http:start", NeverSampler.INSTANCE); - this.filter.run(); + this.filter.runFilter(); RequestContext ctx = RequestContext.getCurrentContext(); then(ctx.getZuulRequestHeaders().get(Span.TRACE_ID_NAME)) @@ -78,4 +85,43 @@ public class TracePreZuulFilterTests { .isEqualTo(Span.SPAN_NOT_SAMPLED); } + + @Test + public void shouldCloseSpanWhenExceptionIsThrown() throws Exception { + Span startedSpan = this.tracer.createSpan("http:start", NeverSampler.INSTANCE); + final AtomicReference span = new AtomicReference<>(); + + new TracePreZuulFilter(this.tracer, new RequestContextInjector()) { + @Override + public Object run() { + super.run(); + span.set(TracePreZuulFilterTests.this.tracer.getCurrentSpan()); + throw new RuntimeException(); + } + }.runFilter(); + + then(startedSpan).isNotEqualTo(span.get()); + then(span.get().logs()).extracting("event").contains(Span.CLIENT_SEND); + then(this.tracer.getCurrentSpan()).isEqualTo(startedSpan); + } + + @Test + public void shouldNotCloseSpanWhenNoExceptionIsThrown() throws Exception { + Span startedSpan = this.tracer.createSpan("http:start", NeverSampler.INSTANCE); + final AtomicReference span = new AtomicReference<>(); + + new TracePreZuulFilter(this.tracer, new RequestContextInjector()) { + @Override + public Object run() { + span.set(TracePreZuulFilterTests.this.tracer.getCurrentSpan()); + return super.run(); + } + }.runFilter(); + + then(startedSpan).isNotEqualTo(span.get()); + then(span.get().logs()).extracting("event").contains(Span.CLIENT_SEND); + then(span.get().tags()).containsKey(Span.SPAN_LOCAL_COMPONENT_TAG_NAME); + then(this.tracer.getCurrentSpan()).isEqualTo(span.get()); + } + }