Changed aspect into filter with custom dispatch (#297)
This commit is contained in:
@@ -111,7 +111,7 @@ public class TraceFilter extends OncePerRequestFilter {
|
||||
String uri = this.urlPathHelper.getPathWithinApplication(request);
|
||||
boolean skip = this.skipPattern.matcher(uri).matches()
|
||||
|| Span.SPAN_NOT_SAMPLED.equals(ServletUtils.getHeader(request, response, Span.SAMPLED_NAME));
|
||||
Span spanFromRequest = (Span) request.getAttribute(TRACE_REQUEST_ATTR);
|
||||
Span spanFromRequest = getSpanFromAttribute(request);
|
||||
if (spanFromRequest != null) {
|
||||
this.tracer.continueSpan(spanFromRequest);
|
||||
}
|
||||
@@ -152,11 +152,24 @@ public class TraceFilter extends OncePerRequestFilter {
|
||||
HttpStatus httpStatus = HttpStatus.valueOf(response.getStatus());
|
||||
if (httpStatus.is2xxSuccessful() || httpStatus.is3xxRedirection()) {
|
||||
this.tracer.close(spanFromRequest);
|
||||
} else if(isSpanContinued(request)) {
|
||||
// it means that the span was already detached once and we're processing an error
|
||||
this.tracer.close(spanFromRequest);
|
||||
} else {
|
||||
this.tracer.detach(spanFromRequest);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private Span getSpanFromAttribute(HttpServletRequest request) {
|
||||
return (Span) request.getAttribute(TRACE_REQUEST_ATTR);
|
||||
}
|
||||
|
||||
private boolean isSpanContinued(HttpServletRequest request) {
|
||||
return getSpanFromAttribute(request) != null;
|
||||
}
|
||||
|
||||
private void addRequestTagsForParentSpan(HttpServletRequest request, Span spanFromRequest) {
|
||||
if (spanFromRequest.getName().contains("parent")) {
|
||||
addRequestTags(spanFromRequest, request);
|
||||
@@ -247,4 +260,9 @@ public class TraceFilter extends OncePerRequestFilter {
|
||||
return requestURI.append('?').append(queryString).toString();
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
protected boolean shouldNotFilterErrorDispatch() {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -21,7 +21,6 @@ import java.util.concurrent.Callable;
|
||||
|
||||
import org.apache.commons.logging.Log;
|
||||
import org.aspectj.lang.ProceedingJoinPoint;
|
||||
import org.aspectj.lang.annotation.After;
|
||||
import org.aspectj.lang.annotation.Around;
|
||||
import org.aspectj.lang.annotation.Aspect;
|
||||
import org.aspectj.lang.annotation.Pointcut;
|
||||
@@ -84,9 +83,6 @@ public class TraceWebAspect {
|
||||
@Pointcut("@within(org.springframework.stereotype.Controller)")
|
||||
private void anyControllerAnnotated() { } // NOSONAR
|
||||
|
||||
@Pointcut("target(org.springframework.boot.autoconfigure.web.ErrorController+)")
|
||||
private void implementingErrorController() { } // NOSONAR
|
||||
|
||||
@Pointcut("execution(public java.util.concurrent.Callable *(..))")
|
||||
private void anyPublicMethodReturningCallable() { } // NOSONAR
|
||||
|
||||
@@ -99,9 +95,6 @@ public class TraceWebAspect {
|
||||
@Pointcut("(anyRestControllerAnnotated() || anyControllerAnnotated()) && anyPublicMethodReturningWebAsyncTask()")
|
||||
private void anyControllerOrRestControllerWithPublicWebAsyncTaskMethod() { } // NOSONAR
|
||||
|
||||
@Pointcut("(anyRestControllerAnnotated() || anyControllerAnnotated()) && implementingErrorController()")
|
||||
private void anyControllerOrRestControllerImplementingErrorController() { } // NOSONAR
|
||||
|
||||
@Around("anyControllerOrRestControllerWithPublicAsyncMethod()")
|
||||
@SuppressWarnings("unchecked")
|
||||
public Object wrapWithCorrelationId(ProceedingJoinPoint pjp) throws Throwable {
|
||||
@@ -134,11 +127,4 @@ public class TraceWebAspect {
|
||||
return webAsyncTask;
|
||||
}
|
||||
|
||||
@After("anyControllerOrRestControllerImplementingErrorController()")
|
||||
public void wrapErrorController() throws Throwable {
|
||||
if (this.tracer.isTracing()) {
|
||||
this.tracer.close(this.tracer.getCurrentSpan());
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@@ -30,6 +30,7 @@ import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean
|
||||
import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingClass;
|
||||
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
|
||||
import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication;
|
||||
import org.springframework.boot.context.embedded.FilterRegistrationBean;
|
||||
import org.springframework.boot.context.properties.EnableConfigurationProperties;
|
||||
import org.springframework.cloud.sleuth.SpanInjector;
|
||||
import org.springframework.cloud.sleuth.SpanExtractor;
|
||||
@@ -42,6 +43,12 @@ import org.springframework.context.annotation.Bean;
|
||||
import org.springframework.context.annotation.Configuration;
|
||||
import org.springframework.util.StringUtils;
|
||||
|
||||
import static javax.servlet.DispatcherType.ASYNC;
|
||||
import static javax.servlet.DispatcherType.ERROR;
|
||||
import static javax.servlet.DispatcherType.FORWARD;
|
||||
import static javax.servlet.DispatcherType.INCLUDE;
|
||||
import static javax.servlet.DispatcherType.REQUEST;
|
||||
|
||||
/**
|
||||
* {@link org.springframework.boot.autoconfigure.EnableAutoConfiguration Auto-configuration}
|
||||
* enables tracing to HTTP requests.
|
||||
@@ -72,7 +79,17 @@ public class TraceWebAutoConfiguration {
|
||||
}
|
||||
|
||||
@Bean
|
||||
@ConditionalOnMissingBean
|
||||
public FilterRegistrationBean traceWebFilter(Tracer tracer, TraceKeys traceKeys,
|
||||
SkipPatternProvider skipPatternProvider, SpanReporter spanReporter,
|
||||
SpanExtractor<HttpServletRequest> spanExtractor,
|
||||
SpanInjector<HttpServletResponse> spanInjector,
|
||||
HttpTraceKeysInjector httpTraceKeysInjector, TraceFilter traceFilter) {
|
||||
FilterRegistrationBean filterRegistrationBean = new FilterRegistrationBean(traceFilter);
|
||||
filterRegistrationBean.setDispatcherTypes(ASYNC, ERROR, FORWARD, INCLUDE, REQUEST);
|
||||
return filterRegistrationBean;
|
||||
}
|
||||
|
||||
@Bean
|
||||
public TraceFilter traceFilter(Tracer tracer, TraceKeys traceKeys,
|
||||
SkipPatternProvider skipPatternProvider, SpanReporter spanReporter,
|
||||
SpanExtractor<HttpServletRequest> spanExtractor,
|
||||
|
||||
@@ -117,6 +117,7 @@ public class TraceFilterIntegrationTests extends AbstractMvcIntegrationTest {
|
||||
MvcResult mvcResult = whenSentToNonExistentEndpointWithTraceId(expectedTraceId);
|
||||
|
||||
then(tracingHeaderFrom(mvcResult)).isEqualTo(expectedTraceId);
|
||||
then(this.tracer.getCurrentSpan()).isNull();
|
||||
}
|
||||
|
||||
@Override
|
||||
|
||||
@@ -260,7 +260,7 @@ public class TraceFilterTests {
|
||||
}
|
||||
|
||||
@Test
|
||||
public void doesNotCloseSpanWhenResponseStatusIsNot2xx() throws Exception {
|
||||
public void detachesSpanWhenResponseStatusIsNot2xx() throws Exception {
|
||||
this.request = builder().header(Span.SPAN_ID_NAME, 10L)
|
||||
.header(Span.TRACE_ID_NAME, 20L).buildRequest(new MockServletContext());
|
||||
TraceFilter filter = new TraceFilter(this.tracer, this.traceKeys, this.spanReporter,
|
||||
@@ -269,7 +269,7 @@ public class TraceFilterTests {
|
||||
|
||||
filter.doFilter(this.request, this.response, this.filterChain);
|
||||
|
||||
then(TestSpanContextHolder.getCurrentSpan()).isNotNull();
|
||||
then(TestSpanContextHolder.getCurrentSpan()).isNull();
|
||||
}
|
||||
|
||||
public void verifyParentSpanHttpTags() {
|
||||
|
||||
@@ -17,7 +17,6 @@
|
||||
package org.springframework.cloud.sleuth.instrument.web.client;
|
||||
|
||||
import javax.servlet.http.HttpServletRequest;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collections;
|
||||
import java.util.HashMap;
|
||||
import java.util.List;
|
||||
@@ -52,6 +51,7 @@ import org.springframework.cloud.sleuth.SpanReporter;
|
||||
import org.springframework.cloud.sleuth.Tracer;
|
||||
import org.springframework.cloud.sleuth.sampler.AlwaysSampler;
|
||||
import org.springframework.cloud.sleuth.trace.TestSpanContextHolder;
|
||||
import org.springframework.cloud.sleuth.util.ArrayListSpanAccumulator;
|
||||
import org.springframework.context.annotation.Bean;
|
||||
import org.springframework.context.annotation.Configuration;
|
||||
import org.springframework.http.HttpHeaders;
|
||||
@@ -84,7 +84,7 @@ public class WebClientTests {
|
||||
|
||||
@Autowired TestFeignInterface testFeignInterface;
|
||||
@Autowired @LoadBalanced RestTemplate template;
|
||||
@Autowired Listener listener;
|
||||
@Autowired ArrayListSpanAccumulator listener;
|
||||
@Autowired Tracer tracer;
|
||||
@Autowired TestErrorController testErrorController;
|
||||
|
||||
@@ -210,6 +210,9 @@ public class WebClientTests {
|
||||
} catch (HttpClientErrorException e) { }
|
||||
|
||||
then(this.tracer.getCurrentSpan()).isNull();
|
||||
Optional<Span> storedSpan = this.listener.getSpans().stream()
|
||||
.filter(span -> "404".equals(span.tags().get("http.status_code"))).findFirst();
|
||||
then(storedSpan.isPresent()).isTrue();
|
||||
then(this.testErrorController.getSpan()).isNotNull();
|
||||
}
|
||||
|
||||
@@ -261,11 +264,6 @@ public class WebClientTests {
|
||||
return new FooController();
|
||||
}
|
||||
|
||||
@Bean
|
||||
Listener listener() {
|
||||
return new Listener();
|
||||
}
|
||||
|
||||
@LoadBalanced
|
||||
@Bean
|
||||
public RestTemplate restTemplate() {
|
||||
@@ -282,6 +280,10 @@ public class WebClientTests {
|
||||
return new TestErrorController(errorAttributes, tracer);
|
||||
}
|
||||
|
||||
@Bean
|
||||
SpanReporter spanReporter() {
|
||||
return new ArrayListSpanAccumulator();
|
||||
}
|
||||
}
|
||||
|
||||
public static class TestErrorController extends BasicErrorController {
|
||||
@@ -310,20 +312,6 @@ public class WebClientTests {
|
||||
}
|
||||
}
|
||||
|
||||
@Component
|
||||
public static class Listener implements SpanReporter {
|
||||
private List<Span> events = new ArrayList<>();
|
||||
|
||||
public List<Span> getSpans() {
|
||||
return this.events;
|
||||
}
|
||||
|
||||
@Override
|
||||
public void report(Span span) {
|
||||
this.events.add(span);
|
||||
}
|
||||
}
|
||||
|
||||
@RestController
|
||||
public static class FooController {
|
||||
|
||||
|
||||
Reference in New Issue
Block a user