Migrated to brave's span customizing handler interceptors

This commit is contained in:
Marcin Grzejszczak
2018-03-11 20:45:53 +01:00
parent 1753d44ab9
commit 171daaf280
9 changed files with 23 additions and 173 deletions

View File

@@ -272,7 +272,7 @@
<spring-cloud-stream.version>Elmhurst.BUILD-SNAPSHOT</spring-cloud-stream.version>
<spring-cloud-netflix.version>2.0.0.BUILD-SNAPSHOT</spring-cloud-netflix.version>
<spring-cloud-openfeign.version>2.0.0.BUILD-SNAPSHOT</spring-cloud-openfeign.version>
<brave.version>4.17.2</brave.version>
<brave.version>4.17.3-SNAPSHOT</brave.version>
<!-- Version set until zipkin-junit gets defined in Brave BOM -->
<zipkin.version>2.5.1</zipkin.version>
<spring-security-boot-autoconfigure.version>2.0.0.RELEASE</spring-security-boot-autoconfigure.version>

View File

@@ -47,8 +47,11 @@ import org.springframework.boot.context.properties.ConfigurationProperties;
* what's you are storing.
*
* @since 1.0.0
*
* @deprecated the Brave's defaults are suggested to be used
*/
@ConfigurationProperties("spring.sleuth.keys")
@Deprecated
public class TraceKeys {
private Http http = new Http();

View File

@@ -1,122 +0,0 @@
/*
* 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 javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import brave.SpanCustomizer;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.springframework.beans.factory.BeanFactory;
import org.springframework.cloud.sleuth.ErrorParser;
import org.springframework.cloud.sleuth.TraceKeys;
import org.springframework.web.method.HandlerMethod;
import org.springframework.web.servlet.handler.HandlerInterceptorAdapter;
/**
* {@link org.springframework.web.servlet.HandlerInterceptor} that wraps handling of a
* adds tags related to the class and method name.
*
* @author Marcin Grzejszczak
* @since 1.0.3
*/
class SleuthTraceHandlerInterceptor extends HandlerInterceptorAdapter {
private static final Log log = LogFactory.getLog(SleuthTraceHandlerInterceptor.class);
private final BeanFactory beanFactory;
private SpanCustomizer spanCustomizer;
private TraceKeys traceKeys;
private ErrorParser errorParser;
public SleuthTraceHandlerInterceptor(BeanFactory beanFactory) {
this.beanFactory = beanFactory;
}
@Override
public boolean preHandle(HttpServletRequest request, HttpServletResponse response,
Object handler) {
SpanCustomizer span = spanCustomizer();
if (log.isDebugEnabled()) {
log.debug("Adding tags to span " + span);
}
addClassMethodTag(handler, span);
addClassNameTag(handler, span);
return true;
}
private void addClassMethodTag(Object handler, SpanCustomizer span) {
if (handler instanceof HandlerMethod) {
String methodName = ((HandlerMethod) handler).getMethod().getName();
span.tag(traceKeys().getMvc().getControllerMethod(), methodName);
if (log.isDebugEnabled()) {
log.debug("Adding a method tag with value [" + methodName + "] to a span " + span);
}
}
}
private void addClassNameTag(Object handler, SpanCustomizer span) {
String className;
if (handler instanceof HandlerMethod) {
className = ((HandlerMethod) handler).getBeanType().getSimpleName();
} else {
className = handler.getClass().getSimpleName();
}
if (log.isDebugEnabled()) {
log.debug("Adding a class tag with value [" + className + "] to a span " + span);
}
span.tag(traceKeys().getMvc().getControllerClass(), className);
}
@Override
public void afterConcurrentHandlingStarted(HttpServletRequest request,
HttpServletResponse response, Object handler) {
}
@Override
public void afterCompletion(HttpServletRequest request, HttpServletResponse response,
Object handler, Exception ex) {
SpanCustomizer span = spanCustomizer();
if (ex != null && span != null) {
errorParser().parseErrorTags(span, ex);
}
}
private SpanCustomizer spanCustomizer() {
if (this.spanCustomizer == null) {
this.spanCustomizer = this.beanFactory.getBean(SpanCustomizer.class);
}
return this.spanCustomizer;
}
private TraceKeys traceKeys() {
if (this.traceKeys == null) {
this.traceKeys = this.beanFactory.getBean(TraceKeys.class);
}
return this.traceKeys;
}
private ErrorParser errorParser() {
if (this.errorParser == null) {
this.errorParser = this.beanFactory.getBean(ErrorParser.class);
}
return this.errorParser;
}
}

View File

@@ -19,7 +19,7 @@ package org.springframework.cloud.sleuth.instrument.web;
import java.util.Collections;
import javax.servlet.http.HttpServletRequest;
import brave.spring.webmvc.TracingHandlerInterceptor;
import brave.spring.webmvc.SpanCustomizingAsyncHandlerInterceptor;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.springframework.beans.BeansException;
@@ -89,12 +89,7 @@ class TraceSpringDataBeanPostProcessor implements BeanPostProcessor {
if (handlerExecutionChain == null) {
return null;
}
handlerExecutionChain.addInterceptor(this.applicationContext.getBean(TracingHandlerInterceptor.class));
String legacyEnabled = this.applicationContext.getEnvironment()
.getProperty("spring.sleuth.http.legacy.enabled", "false");
if (Boolean.parseBoolean(legacyEnabled)) {
handlerExecutionChain.addInterceptor(this.applicationContext.getBean(SleuthTraceHandlerInterceptor.class));
}
handlerExecutionChain.addInterceptor(this.applicationContext.getBean(SpanCustomizingAsyncHandlerInterceptor.class));
return handlerExecutionChain;
}
}

View File

@@ -16,46 +16,28 @@
package org.springframework.cloud.sleuth.instrument.web;
import brave.http.HttpTracing;
import brave.spring.webmvc.TracingHandlerInterceptor;
import org.springframework.beans.factory.BeanFactory;
import brave.spring.webmvc.SpanCustomizingAsyncHandlerInterceptor;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
import org.springframework.context.ApplicationContext;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.context.annotation.Import;
import org.springframework.web.servlet.config.annotation.InterceptorRegistry;
import org.springframework.web.servlet.config.annotation.WebMvcConfigurer;
/**
* MVC Adapter that adds the {@link TracingHandlerInterceptor}
* MVC Adapter that adds the {@link SpanCustomizingAsyncHandlerInterceptor}
*
* @author Marcin Grzejszczak
*
* @since 1.0.3
*/
@Configuration
@Import(SpanCustomizingAsyncHandlerInterceptor.class)
class TraceWebMvcConfigurer implements WebMvcConfigurer {
@Autowired ApplicationContext applicationContext;
@Bean
public TracingHandlerInterceptor tracingHandlerInterceptor(HttpTracing tracing) {
return (TracingHandlerInterceptor) TracingHandlerInterceptor.create(tracing);
}
@Bean
@ConditionalOnProperty("spring.sleuth.http.legacy.enabled")
public SleuthTraceHandlerInterceptor legacySleuthTraceHandlerInterceptor(BeanFactory beanFactory) {
return new SleuthTraceHandlerInterceptor(beanFactory);
}
@Override
public void addInterceptors(InterceptorRegistry registry) {
registry.addInterceptor(this.applicationContext.getBean(TracingHandlerInterceptor.class));
String legacyEnabled = this.applicationContext.getEnvironment()
.getProperty("spring.sleuth.http.legacy.enabled", "false");
if (Boolean.parseBoolean(legacyEnabled)) {
registry.addInterceptor(this.applicationContext.getBean(SleuthTraceHandlerInterceptor.class));
}
registry.addInterceptor(this.applicationContext.getBean(SpanCustomizingAsyncHandlerInterceptor.class));
}
}

View File

@@ -19,6 +19,7 @@ package org.springframework.cloud.sleuth.instrument.web;
import brave.Tracer;
import brave.http.HttpTracing;
import brave.servlet.TracingFilter;
import brave.spring.webmvc.SpanCustomizingAsyncHandlerInterceptor;
import org.springframework.boot.autoconfigure.AutoConfigureAfter;
import org.springframework.boot.autoconfigure.condition.ConditionalOnBean;
import org.springframework.boot.autoconfigure.condition.ConditionalOnClass;
@@ -54,6 +55,7 @@ import static javax.servlet.DispatcherType.REQUEST;
@ConditionalOnWebApplication(type = ConditionalOnWebApplication.Type.SERVLET)
@ConditionalOnBean(HttpTracing.class)
@AutoConfigureAfter(TraceHttpAutoConfiguration.class)
@Import(SpanCustomizingAsyncHandlerInterceptor.class)
public class TraceWebServletAutoConfiguration {
public static final int TRACING_FILTER_ORDER = Ordered.HIGHEST_PRECEDENCE + 5;

View File

@@ -18,7 +18,7 @@ package org.springframework.cloud.sleuth.instrument.zuul;
import java.lang.invoke.MethodHandles;
import brave.spring.webmvc.TracingHandlerInterceptor;
import brave.spring.webmvc.SpanCustomizingAsyncHandlerInterceptor;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.springframework.beans.BeansException;
@@ -58,7 +58,7 @@ class TraceZuulHandlerMappingBeanPostProcessor implements BeanPostProcessor {
}
ZuulHandlerMapping zuulHandlerMapping = (ZuulHandlerMapping) bean;
zuulHandlerMapping.setInterceptors(
this.beanFactory.getBean(TracingHandlerInterceptor.class));
this.beanFactory.getBean(SpanCustomizingAsyncHandlerInterceptor.class));
}
return bean;
}

View File

@@ -96,7 +96,7 @@ public class TraceFilterIntegrationTests extends AbstractMvcIntegrationTest {
public void should_create_a_trace() throws Exception {
whenSentPingWithoutTracingData();
then(this.reporter.getSpans()).hasSize(2);
then(this.reporter.getSpans()).hasSize(1);
zipkin2.Span span = this.reporter.getSpans().get(0);
then(span.tags())
.containsKey(new TraceKeys().getMvc().getControllerClass())
@@ -175,14 +175,11 @@ public class TraceFilterIntegrationTests extends AbstractMvcIntegrationTest {
whenSentToNonExistentEndpointWithTraceId(expectedTraceId);
// it's a span with the same ids
then(this.reporter.getSpans()).hasSize(2);
then(this.reporter.getSpans()).hasSize(1);
zipkin2.Span serverSpan = this.reporter.getSpans().get(0);
then(serverSpan.tags())
.containsEntry("custom", "tag")
.containsEntry("http.status_code", "404");
zipkin2.Span handlerSpan = this.reporter.getSpans().get(0);
then(handlerSpan.tags())
.containsEntry("http.status_code", "404");
then(this.tracer.currentSpan()).isNull();
}
@@ -199,12 +196,8 @@ public class TraceFilterIntegrationTests extends AbstractMvcIntegrationTest {
// we need to dump the span cause it's not in TraceFilter since TF
// has also error dispatch and the ErrorController would report the span
then(this.reporter.getSpans()).hasSize(2);
// server
then(this.reporter.getSpans()).hasSize(1);
then(this.reporter.getSpans().get(0).tags())
.containsEntry("error", "java.lang.RuntimeException");
// handler
then(this.reporter.getSpans().get(1).tags())
.containsEntry("error", "Request processing failed; nested exception is java.lang.RuntimeException");
}

View File

@@ -84,19 +84,15 @@ public class TraceFilterWebIntegrationTests {
}
then(Tracing.current().tracer().currentSpan()).isNull();
then(this.accumulator.getSpans()).hasSize(2);
then(this.accumulator.getSpans()).hasSize(1);
Span fromFirstTraceFilterFlow = this.accumulator.getSpans().get(0);
then(fromFirstTraceFilterFlow.tags())
.containsEntry("http.status_code", "500")
.containsEntry("http.method", "GET")
.containsEntry("error", "Throwing exception")
.containsEntry("mvc.controller.class", "ExceptionThrowingController");
Span fromErrorController = this.accumulator.getSpans().get(1);
then(fromErrorController.tags())
.containsEntry("http.status_code", "500")
.containsEntry("mvc.controller.class", "ExceptionThrowingController")
.containsEntry("error", "Request processing failed; nested exception is java.lang.RuntimeException: Throwing exception");
// issue#714
String hex = fromErrorController.traceId();
String hex = fromFirstTraceFilterFlow.traceId();
String[] split = capture.toString().split("\n");
List<String> list = Arrays.stream(split).filter(s -> s.contains(
"Uncaught exception thrown"))
@@ -114,9 +110,10 @@ public class TraceFilterWebIntegrationTests {
}
then(Tracing.current().tracer().currentSpan()).isNull();
then(this.accumulator.getSpans()).hasSize(2).as("spans with same id, one from server, one from handler");
then(this.accumulator.getSpans()).hasSize(1);
then(this.accumulator.getSpans().get(0).kind().ordinal()).isEqualTo(Span.Kind.SERVER.ordinal());
then(this.accumulator.getSpans().get(0).tags()).containsEntry("http.status_code", "400");
then(this.accumulator.getSpans().get(0).tags()).containsEntry("http.path", "/test_bad_request");
}
@Test