SpringCloud/spring-cloud-sleuth
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Close span when Exception is ClientAbortException (#1040)

Browse Source 
* close span when Exception is ClientAbortException
* Support custom to ignore exception in trace.
* explicit import statements
* Add test to verify that the span got reported. Fix typo.

Fixes gh-1038
This commit is contained in:
Alvin
2018-07-30 21:30:39 +08:00
committed by Marcin Grzejszczak
parent 3511b1621b
commit 707f3714d6
5 changed files with 214 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

24
spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/ExceptionToIgnoreInTraceFilter.java Normal file
View File

@@ -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();
}

31
spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/ExceptionToIgnoreInTraceFilterProvider.java Normal file
View File

@@ -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<ExceptionToIgnoreInTraceFilter> exceptionsToIgnoreInTraceFilters;
public ExceptionToIgnoreInTraceFilterProvider(List<ExceptionToIgnoreInTraceFilter> exceptionsToIgnoreInTraceFilter) {
this.exceptionsToIgnoreInTraceFilters = exceptionsToIgnoreInTraceFilter;
}
public List<ExceptionToIgnoreInTraceFilter> exceptionsToIgnoreInTraceFilters() {
return this.exceptionsToIgnoreInTraceFilters;
}
}

22
spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceFilter.java
View File

@@ -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;
}
}

35
spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/web/TraceWebAutoConfiguration.java
View File

@@ -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<ExceptionToIgnoreInTraceFilter> exceptionsToIgnoreInTraceFilter = new ArrayList<>();
@Bean
ExceptionToIgnoreInTraceFilterProvider exceptionToIgnoreInTraceFilterProvider() {
return new ExceptionToIgnoreInTraceFilterProvider(this.exceptionsToIgnoreInTraceFilter);
}
}

103
spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/TraceFilterTests.java
View File

@@ -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<ExceptionToIgnoreInTraceFilter> 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<ExceptionToIgnoreInTraceFilter> 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<ExceptionToIgnoreInTraceFilter> filters) {
return new ExceptionToIgnoreInTraceFilterProvider(filters);
}
@Test
public void closesSpanWhenResponseStatusIs3xx() throws Exception {
this.request = builder().header(Span.SPAN_ID_NAME, PARENT_ID)