Not closing a span if exception happened and error controller is present; fixes gh-859

This commit is contained in:
Marcin Grzejszczak
2018-02-15 13:24:17 +01:00
parent 3224cc6b38
commit 0a5fa8bdad
2 changed files with 30 additions and 10 deletions

View File

@@ -32,6 +32,7 @@ import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.springframework.beans.factory.BeanFactory;
import org.springframework.beans.factory.NoSuchBeanDefinitionException;
import org.springframework.boot.autoconfigure.web.ErrorController;
import org.springframework.cloud.sleuth.ErrorParser;
import org.springframework.cloud.sleuth.Span;
import org.springframework.cloud.sleuth.SpanReporter;
@@ -104,6 +105,7 @@ public class TraceFilter extends GenericFilterBean {
private HttpTraceKeysInjector httpTraceKeysInjector;
private ErrorParser errorParser;
private final BeanFactory beanFactory;
private Boolean hasErrorController;
private final UrlPathHelper urlPathHelper = new UrlPathHelper();
@@ -236,8 +238,10 @@ public class TraceFilter extends GenericFilterBean {
if (log.isDebugEnabled()) {
log.debug("Closing the span " + span + " since the response was successful");
}
tracer().close(span);
clearTraceAttribute(request);
if (exception == null || !hasErrorController()) {
tracer().close(span);
clearTraceAttribute(request);
}
} else if (errorAlreadyHandled(request) && tracer().isTracing() && !shouldCloseSpan(request)) {
if (log.isDebugEnabled()) {
log.debug(
@@ -248,17 +252,33 @@ public class TraceFilter extends GenericFilterBean {
log.debug("Will close span " + span + " since " + (shouldCloseSpan(request) ? "some component marked it for closure" : "response was unsuccessful for the root span"));
}
tracer().close(span);
clearTraceAttribute(request);
if (exception == null || !hasErrorController()) {
clearTraceAttribute(request);
}
} else if (tracer().isTracing()) {
if (log.isDebugEnabled()) {
log.debug("Detaching the span " + span + " since the response was unsuccessful");
}
tracer().detach(span);
clearTraceAttribute(request);
if (exception == null || !hasErrorController()) {
tracer().detach(span);
}
}
}
}
// null check is only for tests
private boolean hasErrorController() {
if (this.hasErrorController == null) {
try {
this.hasErrorController = this.beanFactory.getBean(ErrorController.class) != null;
} catch (NoSuchBeanDefinitionException e) {
this.hasErrorController = false;
}
}
return this.hasErrorController;
}
private void addResponseTagsForSpanWithoutParent(HttpServletRequest request,
HttpServletResponse response) {
if (spanWithoutParent(request) && response.getStatus() >= 100) {

View File

@@ -16,6 +16,11 @@
package org.springframework.cloud.sleuth.instrument.web;
import java.lang.reflect.Field;
import java.util.concurrent.Callable;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.apache.commons.logging.Log;
import org.aspectj.lang.ProceedingJoinPoint;
import org.aspectj.lang.annotation.Around;
@@ -29,11 +34,6 @@ import org.springframework.cloud.sleuth.Tracer;
import org.springframework.cloud.sleuth.instrument.async.SpanContinuingTraceCallable;
import org.springframework.web.context.request.async.WebAsyncTask;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.lang.reflect.Field;
import java.util.concurrent.Callable;
/**
* Aspect that adds tracing to
* <p/>
@@ -154,7 +154,7 @@ public class TraceWebAspect {
return pjp.proceed();
} finally {
if (log.isDebugEnabled()) {
log.debug("Marking span " + currentSpan + " for closure by Trace Filter");
log.debug("Marking span " + currentSpan + " for closure by Trace Filter or Error Controller");
}
request.setAttribute(TraceFilter.TRACE_CLOSE_SPAN_REQUEST_ATTR, true);
}