diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/ExceptionToIgnoreInTraceFilter.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/ExceptionToIgnoreInTraceFilter.java new file mode 100644 index 000000000..b6fec4c9e --- /dev/null +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/ExceptionToIgnoreInTraceFilter.java @@ -0,0 +1,24 @@ +/* + * Copyright 2013-2018 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.sleuth.instrument.web; + +/** + * Provide a exceptionClassName to ignore in trace. + */ +interface ExceptionToIgnoreInTraceFilter { + String exceptionClassName(); +} diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/ExceptionToIgnoreInTraceFilterProvider.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/ExceptionToIgnoreInTraceFilterProvider.java new file mode 100644 index 000000000..5e627378e --- /dev/null +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/ExceptionToIgnoreInTraceFilterProvider.java @@ -0,0 +1,31 @@ +/* + * Copyright 2013-2018 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.sleuth.instrument.web; + +import java.util.List; + +class ExceptionToIgnoreInTraceFilterProvider { + final List exceptionsToIgnoreInTraceFilters; + + public ExceptionToIgnoreInTraceFilterProvider(List exceptionsToIgnoreInTraceFilter) { + this.exceptionsToIgnoreInTraceFilters = exceptionsToIgnoreInTraceFilter; + } + + public List exceptionsToIgnoreInTraceFilters() { + return this.exceptionsToIgnoreInTraceFilters; + } +} \ No newline at end of file diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceFilter.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceFilter.java index ea6d886ad..f3bbca62d 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceFilter.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceFilter.java @@ -27,7 +27,6 @@ import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; - import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.beans.factory.BeanFactory; @@ -106,6 +105,7 @@ public class TraceFilter extends GenericFilterBean { private ErrorParser errorParser; private final BeanFactory beanFactory; private Boolean hasErrorController; + private ExceptionToIgnoreInTraceFilterProvider exceptionToIgnoreInTraceFilterProvider; private final UrlPathHelper urlPathHelper = new UrlPathHelper(); @@ -241,6 +241,9 @@ public class TraceFilter extends GenericFilterBean { if (exception == null || !hasErrorController()) { tracer().close(span); clearTraceAttribute(request); + } else if(exception != null && needIgnoreException(exception)){ + tracer().close(span); + clearTraceAttribute(request); } } else if (errorAlreadyHandled(request) && tracer().isTracing() && !shouldCloseSpan(request)) { if (log.isDebugEnabled()) { @@ -271,6 +274,16 @@ public class TraceFilter extends GenericFilterBean { } } + + private boolean needIgnoreException(Throwable exception) { + for (ExceptionToIgnoreInTraceFilter filter : exceptionToIgnoreInTraceFilterProvider().exceptionsToIgnoreInTraceFilters()) { + if(exception.getClass().getName().equals(filter.exceptionClassName())){ + return true; + } + } + return false; + } + // null check is only for tests private boolean hasErrorController() { if (this.hasErrorController == null) { @@ -502,5 +515,12 @@ public class TraceFilter extends GenericFilterBean { } return this.errorParser; } + + ExceptionToIgnoreInTraceFilterProvider exceptionToIgnoreInTraceFilterProvider() { + if (this.exceptionToIgnoreInTraceFilterProvider == null) { + this.exceptionToIgnoreInTraceFilterProvider = this.beanFactory.getBean(ExceptionToIgnoreInTraceFilterProvider.class); + } + return this.exceptionToIgnoreInTraceFilterProvider; + } } diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebAutoConfiguration.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebAutoConfiguration.java index f6b1913de..554292326 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebAutoConfiguration.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebAutoConfiguration.java @@ -15,7 +15,11 @@ */ package org.springframework.cloud.sleuth.instrument.web; +import org.apache.catalina.connector.ClientAbortException; +import org.apache.catalina.connector.Request; +import org.apache.catalina.connector.Response; import org.springframework.beans.factory.BeanFactory; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.boot.autoconfigure.AutoConfigureAfter; import org.springframework.boot.autoconfigure.condition.ConditionalOnBean; @@ -34,6 +38,9 @@ import org.springframework.context.annotation.Import; import org.springframework.context.annotation.Role; import org.springframework.web.servlet.config.annotation.WebMvcConfigurerAdapter; +import java.util.ArrayList; +import java.util.List; + import static javax.servlet.DispatcherType.ASYNC; import static javax.servlet.DispatcherType.ERROR; import static javax.servlet.DispatcherType.FORWARD; @@ -98,4 +105,32 @@ public class TraceWebAutoConfiguration { return new TraceFilter(beanFactory, skipPatternProvider.skipPattern()); } + + @Configuration + @ConditionalOnClass(name = "org.apache.catalina.connector.ClientAbortException") + protected static class ClientAbortExceptionToIgnoreInTraceFilterConfig{ + /** + * Ignore the name of {@link ClientAbortException} when use tomcat. Because the tomcat will ignore this exception + * in {@link org.apache.catalina.core.StandardHostValve#throwable(Request, Response, Throwable)}, Causes the current span to not close. + * More detail see #1038. + */ + @Bean + public ExceptionToIgnoreInTraceFilter clientAbortExceptionToIgnoreInTraceFilter(){ + return new ExceptionToIgnoreInTraceFilter() { + @Override + public String exceptionClassName() { + return ClientAbortException.class.getName(); + } + }; + } + } + + @Autowired(required=false) + List exceptionsToIgnoreInTraceFilter = new ArrayList<>(); + + @Bean + ExceptionToIgnoreInTraceFilterProvider exceptionToIgnoreInTraceFilterProvider() { + return new ExceptionToIgnoreInTraceFilterProvider(this.exceptionsToIgnoreInTraceFilter); + } + } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterTests.java index aa987ea40..fb6de94f5 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterTests.java @@ -16,10 +16,14 @@ package org.springframework.cloud.sleuth.instrument.web; +import java.io.IOException; import java.util.ArrayList; +import java.util.List; import java.util.Random; import java.util.regex.Pattern; +import org.apache.catalina.connector.ClientAbortException; +import org.assertj.core.util.Lists; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -28,6 +32,7 @@ import org.mockito.Mock; import org.mockito.Mockito; import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.NoSuchBeanDefinitionException; +import org.springframework.boot.autoconfigure.web.ErrorController; import org.springframework.cloud.sleuth.DefaultSpanNamer; import org.springframework.cloud.sleuth.ErrorParser; import org.springframework.cloud.sleuth.ExceptionMessageErrorParser; @@ -54,6 +59,12 @@ import org.springframework.mock.web.MockHttpServletResponse; import org.springframework.mock.web.MockServletContext; import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder; +import javax.servlet.ServletException; +import javax.servlet.ServletOutputStream; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; +import javax.servlet.WriteListener; + import static org.junit.Assert.assertEquals; import static org.mockito.MockitoAnnotations.initMocks; import static org.springframework.cloud.sleuth.assertions.SleuthAssertions.assertThat; @@ -375,6 +386,98 @@ public class TraceFilterTests { then(TestSpanContextHolder.getCurrentSpan()).isNull(); } + @Test + public void closesSpanWhenResponseStatusIs2xxAndExceptionIsClientAbortException() throws Exception { + this.request = builder().header(Span.SPAN_ID_NAME, PARENT_ID) + .header(Span.TRACE_ID_NAME, 20L).buildRequest(new MockServletContext()); + TraceFilter filter = new TraceFilter(beanFactory()); + BDDMockito.given(beanFactory.getBean(ErrorController.class)).willReturn(() -> "/error"); + List filters = Lists.newArrayList(getClientAbortExpcetionToIgnoreInTraceFilter()); + BDDMockito.given(beanFactory.getBean(ExceptionToIgnoreInTraceFilterProvider.class)) + .willReturn(getExceptionToIgnoreInTraceFilterProvider(filters)); + this.response = new MockHttpServletResponse(){ + @Override + public ServletOutputStream getOutputStream() { + ServletOutputStream outputStream = super.getOutputStream(); + return new ServletOutputStream() { + @Override + public boolean isReady() { + return outputStream.isReady(); + } + + @Override + public void setWriteListener(WriteListener listener) { + outputStream.setWriteListener(listener); + } + + @Override + public void write(int b) throws IOException { + outputStream.write(b); + } + + @Override + public void flush() throws IOException { + throw new ClientAbortException("Broken pipe"); + } + }; + } + }; + response.setStatus(200); + this.filterChain = new MockFilterChain(){ + @Override + public void doFilter(ServletRequest request, ServletResponse response) throws IOException, ServletException { + ServletOutputStream outputStream = response.getOutputStream(); + outputStream.write(1); + outputStream.flush(); + } + }; + try { + filter.doFilter(this.request, this.response, this.filterChain); + }catch (ClientAbortException e){ + // ig + } + then(TestSpanContextHolder.getCurrentSpan()).isNull(); + then(spanReporter.getSpans()).hasSize(1); + } + @Test + public void closesSpanWhenResponseStatusIs2xxAndClientAbortExceptionThrowAfterTraceFilter() throws Exception { + this.request = builder().header(Span.SPAN_ID_NAME, PARENT_ID) + .header(Span.TRACE_ID_NAME, 20L).buildRequest(new MockServletContext()); + TraceFilter filter = new TraceFilter(beanFactory()); + BDDMockito.given(beanFactory.getBean(ErrorController.class)).willReturn(() -> "/error"); + List filters = Lists.newArrayList(getClientAbortExpcetionToIgnoreInTraceFilter()); + BDDMockito.given(beanFactory.getBean(ExceptionToIgnoreInTraceFilterProvider.class)) + .willReturn(getExceptionToIgnoreInTraceFilterProvider(filters)); + this.response = new MockHttpServletResponse(); + response.setStatus(200); + this.filterChain = new MockFilterChain(){ + @Override + public void doFilter(ServletRequest request, ServletResponse response) throws IOException, ServletException { + throw new ClientAbortException(); + } + }; + try { + filter.doFilter(this.request, this.response, this.filterChain); + }catch (ClientAbortException e){ + // ig + } + then(TestSpanContextHolder.getCurrentSpan()).isNull(); + then(spanReporter.getSpans()).hasSize(1); + } + + private ExceptionToIgnoreInTraceFilter getClientAbortExpcetionToIgnoreInTraceFilter() { + return new ExceptionToIgnoreInTraceFilter(){ + @Override + public String exceptionClassName() { + return ClientAbortException.class.getName(); + } + }; + } + + private ExceptionToIgnoreInTraceFilterProvider getExceptionToIgnoreInTraceFilterProvider(List filters) { + return new ExceptionToIgnoreInTraceFilterProvider(filters); + } + @Test public void closesSpanWhenResponseStatusIs3xx() throws Exception { this.request = builder().header(Span.SPAN_ID_NAME, PARENT_ID)