From 8856854002878e7736c908eb20afd1975dfb5ec5 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Tue, 7 Jun 2016 10:17:32 +0200 Subject: [PATCH] Fixed duplicated logs in tracefilter --- .../sleuth/instrument/web/TraceFilter.java | 23 ++++++++++++++----- .../instrument/web/client/WebClientTests.java | 8 +++++++ 2 files changed, 25 insertions(+), 6 deletions(-) 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 9e7c0d3ec..5b0f2b72c 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 @@ -115,6 +115,16 @@ public class TraceFilter extends OncePerRequestFilter { if (spanFromRequest != null) { this.tracer.continueSpan(spanFromRequest); } + // in case of a response with exception status a exception controller will close the span + if (!httpStatusSuccessful(response) && isSpanContinued(request)) { + // it means that the span was already detached once and we're processing an error + try { + filterChain.doFilter(request, response); + } finally { + this.tracer.close(spanFromRequest); + } + return; + } addToResponseIfNotPresent(response, Span.SAMPLED_NAME, skip ? Span.SPAN_NOT_SAMPLED : Span.SPAN_SAMPLED); String name = HTTP_COMPONENT + ":" + uri; spanFromRequest = createSpan(request, skip, spanFromRequest, name); @@ -148,12 +158,8 @@ public class TraceFilter extends OncePerRequestFilter { } else { spanFromRequest.logEvent(Span.SERVER_SEND); } - // in case of a response with exception status a exception controller will close the span - 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 + // in case of a response with exception status will close the span when exception dispatch is handled + if (httpStatusSuccessful(response)) { this.tracer.close(spanFromRequest); } else { this.tracer.detach(spanFromRequest); @@ -162,6 +168,11 @@ public class TraceFilter extends OncePerRequestFilter { } } + private boolean httpStatusSuccessful(HttpServletResponse response) { + HttpStatus httpStatus = HttpStatus.valueOf(response.getStatus()); + return httpStatus.is2xxSuccessful() || httpStatus.is3xxRedirection(); + } + private Span getSpanFromAttribute(HttpServletRequest request) { return (Span) request.getAttribute(TRACE_REQUEST_ATTR); } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/WebClientTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/WebClientTests.java index 75805f34f..b068de9e6 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/WebClientTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/client/WebClientTests.java @@ -23,6 +23,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Random; +import java.util.stream.Collectors; import com.netflix.loadbalancer.BaseLoadBalancer; import com.netflix.loadbalancer.ILoadBalancer; @@ -45,6 +46,7 @@ import org.springframework.cloud.client.loadbalancer.LoadBalanced; import org.springframework.cloud.netflix.feign.EnableFeignClients; import org.springframework.cloud.netflix.feign.FeignClient; import org.springframework.cloud.netflix.ribbon.RibbonClient; +import org.springframework.cloud.sleuth.Log; import org.springframework.cloud.sleuth.Sampler; import org.springframework.cloud.sleuth.Span; import org.springframework.cloud.sleuth.SpanReporter; @@ -213,6 +215,12 @@ public class WebClientTests { Optional storedSpan = this.listener.getSpans().stream() .filter(span -> "404".equals(span.tags().get("http.status_code"))).findFirst(); then(storedSpan.isPresent()).isTrue(); + this.listener.getSpans().stream() + .forEach(span -> { + int initialSize = span.logs().size(); + int distinctSize = span.logs().stream().map(Log::getEvent).distinct().collect(Collectors.toList()).size(); + then(initialSize).as("there are no duplicate log entries").isEqualTo(distinctSize); + }); then(this.testErrorController.getSpan()).isNotNull(); }