Added missing error setting on the span in zuul pre filters

without this change when an exception is thrown in a filter we're not setting the error message as a tag.

fixes #616
This commit is contained in:
Marcin Grzejszczak
2017-07-06 12:23:49 +02:00
parent a977e4ae0e
commit 050dfe9bdf
3 changed files with 39 additions and 23 deletions

View File

@@ -16,21 +16,22 @@
package org.springframework.cloud.sleuth.instrument.zuul;
import java.lang.invoke.MethodHandles;
import java.net.URI;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.springframework.cloud.sleuth.instrument.web.HttpSpanInjector;
import org.springframework.cloud.sleuth.Span;
import org.springframework.cloud.sleuth.Tracer;
import org.springframework.cloud.sleuth.instrument.web.HttpTraceKeysInjector;
import org.springframework.cloud.sleuth.instrument.web.TraceRequestAttributes;
import com.netflix.zuul.ExecutionStatus;
import com.netflix.zuul.ZuulFilter;
import com.netflix.zuul.ZuulFilterResult;
import com.netflix.zuul.context.RequestContext;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.springframework.cloud.sleuth.ErrorParser;
import org.springframework.cloud.sleuth.ExceptionMessageErrorParser;
import org.springframework.cloud.sleuth.Span;
import org.springframework.cloud.sleuth.Tracer;
import org.springframework.cloud.sleuth.instrument.web.HttpSpanInjector;
import org.springframework.cloud.sleuth.instrument.web.HttpTraceKeysInjector;
import org.springframework.cloud.sleuth.instrument.web.TraceRequestAttributes;
import java.lang.invoke.MethodHandles;
import java.net.URI;
/**
* A pre request {@link ZuulFilter} that sets tracing related headers on the request
@@ -48,12 +49,23 @@ public class TracePreZuulFilter extends ZuulFilter {
private final Tracer tracer;
private final HttpSpanInjector spanInjector;
private final HttpTraceKeysInjector httpTraceKeysInjector;
private final ErrorParser errorParser;
@Deprecated
public TracePreZuulFilter(Tracer tracer, HttpSpanInjector spanInjector,
HttpTraceKeysInjector httpTraceKeysInjector) {
this.tracer = tracer;
this.spanInjector = spanInjector;
this.httpTraceKeysInjector = httpTraceKeysInjector;
this.errorParser = new ExceptionMessageErrorParser();
}
public TracePreZuulFilter(Tracer tracer, HttpSpanInjector spanInjector,
HttpTraceKeysInjector httpTraceKeysInjector, ErrorParser errorParser) {
this.tracer = tracer;
this.spanInjector = spanInjector;
this.httpTraceKeysInjector = httpTraceKeysInjector;
this.errorParser = errorParser;
}
@Override
@@ -91,6 +103,7 @@ public class TracePreZuulFilter extends ZuulFilter {
log.debug("The result of Zuul filter execution was not successful thus "
+ "will close the current span " + newSpan);
}
this.errorParser.parseErrorTags(newSpan, result.getException());
this.tracer.close(newSpan);
}
return result;

View File

@@ -27,6 +27,7 @@ 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.support.RibbonRequestCustomizer;
import org.springframework.cloud.sleuth.ErrorParser;
import org.springframework.cloud.sleuth.instrument.web.HttpSpanInjector;
import org.springframework.cloud.sleuth.TraceKeys;
import org.springframework.cloud.sleuth.Tracer;
@@ -55,8 +56,9 @@ public class TraceZuulAutoConfiguration {
@Bean
@ConditionalOnMissingBean
public TracePreZuulFilter tracePreZuulFilter(Tracer tracer,
HttpSpanInjector spanInjector, HttpTraceKeysInjector httpTraceKeysInjector) {
return new TracePreZuulFilter(tracer, spanInjector, httpTraceKeysInjector);
HttpSpanInjector spanInjector, HttpTraceKeysInjector httpTraceKeysInjector,
ErrorParser errorParser) {
return new TracePreZuulFilter(tracer, spanInjector, httpTraceKeysInjector, errorParser);
}
@Bean

View File

@@ -16,10 +16,8 @@
package org.springframework.cloud.sleuth.instrument.zuul;
import java.util.Random;
import java.util.concurrent.atomic.AtomicReference;
import javax.servlet.http.HttpServletRequest;
import com.netflix.zuul.context.RequestContext;
import com.netflix.zuul.monitoring.MonitoringHelper;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
@@ -28,6 +26,7 @@ import org.mockito.BDDMockito;
import org.mockito.Mock;
import org.mockito.runners.MockitoJUnitRunner;
import org.springframework.cloud.sleuth.DefaultSpanNamer;
import org.springframework.cloud.sleuth.ExceptionMessageErrorParser;
import org.springframework.cloud.sleuth.NoOpSpanReporter;
import org.springframework.cloud.sleuth.Span;
import org.springframework.cloud.sleuth.TraceKeys;
@@ -39,8 +38,9 @@ import org.springframework.cloud.sleuth.sampler.NeverSampler;
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 javax.servlet.http.HttpServletRequest;
import java.util.Random;
import java.util.concurrent.atomic.AtomicReference;
import static org.springframework.cloud.sleuth.assertions.SleuthAssertions.then;
@@ -57,7 +57,7 @@ public class TracePreZuulFilterTests {
new DefaultSpanNamer(), new NoOpSpanLogger(), new NoOpSpanReporter(), new TraceKeys());
private TracePreZuulFilter filter = new TracePreZuulFilter(this.tracer, new ZipkinHttpSpanInjector(),
new HttpTraceKeysInjector(this.tracer, new TraceKeys()));
new HttpTraceKeysInjector(this.tracer, new TraceKeys()), new ExceptionMessageErrorParser());
@After
public void clean() {
@@ -108,18 +108,19 @@ public class TracePreZuulFilterTests {
final AtomicReference<Span> span = new AtomicReference<>();
new TracePreZuulFilter(this.tracer, new ZipkinHttpSpanInjector(),
new HttpTraceKeysInjector(this.tracer, new TraceKeys())) {
new HttpTraceKeysInjector(this.tracer, new TraceKeys()), new ExceptionMessageErrorParser()) {
@Override
public Object run() {
super.run();
span.set(TracePreZuulFilterTests.this.tracer.getCurrentSpan());
throw new RuntimeException();
throw new RuntimeException("foo");
}
}.runFilter();
then(startedSpan).isNotEqualTo(span.get());
then(span.get().logs()).extracting("event").contains(Span.CLIENT_SEND);
then(span.get()).hasATag("http.method", "GET");
then(span.get()).hasATag("error", "foo");
then(this.tracer.getCurrentSpan()).isEqualTo(startedSpan);
}
@@ -129,7 +130,7 @@ public class TracePreZuulFilterTests {
final AtomicReference<Span> span = new AtomicReference<>();
new TracePreZuulFilter(this.tracer, new ZipkinHttpSpanInjector(),
new HttpTraceKeysInjector(this.tracer, new TraceKeys())) {
new HttpTraceKeysInjector(this.tracer, new TraceKeys()), new ExceptionMessageErrorParser()) {
@Override
public Object run() {
span.set(TracePreZuulFilterTests.this.tracer.getCurrentSpan());