From 0f427cbeb9f8ed5c80856805e2c752abdd3c8840 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Wed, 10 Aug 2016 17:18:20 +0200 Subject: [PATCH] Polish --- .../springframework/cloud/sleuth/Span.java | 4 +-- .../sleuth/instrument/web/TraceFilter.java | 33 ++++++++++--------- .../sleuth/assertions/ListOfSpansAssert.java | 9 +++-- .../instrument/hystrix/TraceCommandTests.java | 4 +-- .../instrument/web/client/WebClientTests.java | 12 +++---- .../stream/ConvertToZipkinSpanList.java | 2 +- .../sleuth/zipkin/ZipkinSpanListener.java | 10 +++--- 7 files changed, 34 insertions(+), 40 deletions(-) diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/Span.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/Span.java index 250f7a11d..3cbc67531 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/Span.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/Span.java @@ -462,9 +462,7 @@ public class Span { Span other = (Span) obj; if (this.spanId != other.spanId) return false; - if (this.traceId != other.traceId) - return false; - return true; + return this.traceId == other.traceId; } public static class SpanBuilder { 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 a593922a4..d2806fdc2 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 @@ -15,18 +15,18 @@ */ package org.springframework.cloud.sleuth.instrument.web; -import javax.servlet.FilterChain; -import javax.servlet.ServletException; -import javax.servlet.ServletRequest; -import javax.servlet.ServletResponse; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; import java.io.IOException; import java.lang.invoke.MethodHandles; import java.util.ArrayList; import java.util.Collections; import java.util.Enumeration; import java.util.regex.Pattern; +import javax.servlet.FilterChain; +import javax.servlet.ServletException; +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; @@ -204,30 +204,31 @@ public class TraceFilter extends GenericFilterBean { private void detachOrCloseSpans(HttpServletRequest request, HttpServletResponse response, Span spanFromRequest, Throwable exception) { - if (spanFromRequest != null) { + Span span = spanFromRequest; + if (span != null) { addResponseTags(response, exception); - if (spanFromRequest.hasSavedSpan() && requestHasAlreadyBeenHandled(request)) { - recordParentSpan(spanFromRequest.getSavedSpan()); + if (span.hasSavedSpan() && requestHasAlreadyBeenHandled(request)) { + recordParentSpan(span.getSavedSpan()); } else if (!requestHasAlreadyBeenHandled(request)) { - spanFromRequest = this.tracer.close(spanFromRequest); + span = this.tracer.close(span); } - recordParentSpan(spanFromRequest); + recordParentSpan(span); // in case of a response with exception status will close the span when exception dispatch is handled if (httpStatusSuccessful(response)) { if (log.isDebugEnabled()) { - log.debug("Closing the span " + spanFromRequest + " since the response was successful"); + log.debug("Closing the span " + span + " since the response was successful"); } - this.tracer.close(spanFromRequest); + this.tracer.close(span); } else if (errorAlreadyHandled(request)) { if (log.isDebugEnabled()) { log.debug( - "Won't detach the span " + spanFromRequest + " since error has already been handled"); + "Won't detach the span " + span + " since error has already been handled"); } } else { if (log.isDebugEnabled()) { - log.debug("Detaching the span " + spanFromRequest + " since the response was unsuccessful"); + log.debug("Detaching the span " + span + " since the response was unsuccessful"); } - this.tracer.detach(spanFromRequest); + this.tracer.detach(span); } } } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/assertions/ListOfSpansAssert.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/assertions/ListOfSpansAssert.java index 9f460d24f..06fb23942 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/assertions/ListOfSpansAssert.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/assertions/ListOfSpansAssert.java @@ -20,16 +20,15 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.stream.Collectors; - -import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.databind.ObjectMapper; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.assertj.core.api.AbstractAssert; import org.springframework.cloud.sleuth.Span; +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; + import static java.util.stream.Collectors.joining; import static java.util.stream.Collectors.toList; import static org.assertj.core.api.Assertions.assertThat; @@ -114,7 +113,7 @@ public class ListOfSpansAssert extends AbstractAssert "\nSPAN: " + span.toString() + " with name [" + span.getName() + "] " + - "\nwith tags " + span.tags() + "\nwith logs " + span.logs()).collect(Collectors.joining("\n")); + "\nwith tags " + span.tags() + "\nwith logs " + span.logs()).collect(joining("\n")); } public ListOfSpansAssert doesNotHaveASpanWithName(String name) { diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/hystrix/TraceCommandTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/hystrix/TraceCommandTests.java index 45254cada..1ab54bf9f 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/hystrix/TraceCommandTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/hystrix/TraceCommandTests.java @@ -32,7 +32,6 @@ import org.springframework.cloud.sleuth.trace.DefaultTracer; import org.springframework.cloud.sleuth.trace.TestSpanContextHolder; import com.netflix.hystrix.HystrixCommand; -import com.netflix.hystrix.HystrixCommandGroupKey; import com.netflix.hystrix.HystrixCommandKey; import com.netflix.hystrix.HystrixCommandProperties; import com.netflix.hystrix.HystrixThreadPoolProperties; @@ -103,8 +102,7 @@ public class TraceCommandTests { Tracer tracer = new DefaultTracer(new AlwaysSampler(), new Random(), new DefaultSpanNamer(), new NoOpSpanLogger(), new NoOpSpanReporter()); TraceKeys traceKeys = new TraceKeys(); - HystrixCommand.Setter setter = HystrixCommand.Setter - .withGroupKey(HystrixCommandGroupKey.Factory.asKey("group")) + HystrixCommand.Setter setter = withGroupKey(asKey("group")) .andCommandKey(HystrixCommandKey.Factory.asKey("command")); // tag::hystrix_command[] HystrixCommand hystrixCommand = new HystrixCommand(setter) { 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 03066f786..43bfe2de4 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 @@ -16,7 +16,6 @@ package org.springframework.cloud.sleuth.instrument.web.client; -import javax.servlet.http.HttpServletRequest; import java.lang.invoke.MethodHandles; import java.util.ArrayList; import java.util.Collections; @@ -26,10 +25,7 @@ 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; -import com.netflix.loadbalancer.Server; +import javax.servlet.http.HttpServletRequest; import org.apache.commons.logging.LogFactory; import org.junit.After; @@ -43,8 +39,6 @@ import org.springframework.boot.autoconfigure.EnableAutoConfiguration; import org.springframework.boot.autoconfigure.web.BasicErrorController; import org.springframework.boot.autoconfigure.web.ErrorAttributes; import org.springframework.boot.autoconfigure.web.ServerProperties; -import org.springframework.boot.test.SpringApplicationConfiguration; -import org.springframework.boot.test.WebIntegrationTest; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.cloud.client.loadbalancer.LoadBalanced; import org.springframework.cloud.netflix.feign.EnableFeignClients; @@ -73,6 +67,10 @@ import org.springframework.web.bind.annotation.RestController; import org.springframework.web.client.HttpClientErrorException; import org.springframework.web.client.RestTemplate; +import com.netflix.loadbalancer.BaseLoadBalancer; +import com.netflix.loadbalancer.ILoadBalancer; +import com.netflix.loadbalancer.Server; + import junitparams.JUnitParamsRunner; import junitparams.Parameters; diff --git a/spring-cloud-sleuth-zipkin-stream/src/main/java/org/springframework/cloud/sleuth/zipkin/stream/ConvertToZipkinSpanList.java b/spring-cloud-sleuth-zipkin-stream/src/main/java/org/springframework/cloud/sleuth/zipkin/stream/ConvertToZipkinSpanList.java index 930f5a5ef..1eac99aec 100644 --- a/spring-cloud-sleuth-zipkin-stream/src/main/java/org/springframework/cloud/sleuth/zipkin/stream/ConvertToZipkinSpanList.java +++ b/spring-cloud-sleuth-zipkin-stream/src/main/java/org/springframework/cloud/sleuth/zipkin/stream/ConvertToZipkinSpanList.java @@ -118,7 +118,7 @@ final class ConvertToZipkinSpanList { BinaryAnnotation.create(Constants.LOCAL_COMPONENT, processId, ep)); } - private static void ensureServerAddr(Span span, zipkin.Span.Builder zipkinSpan, + private static void ensureServerAddr(Span span, Builder zipkinSpan, Endpoint ep) { String serviceName = span.tags().containsKey(Span.SPAN_PEER_SERVICE_TAG_NAME) ? span.tags().get(Span.SPAN_PEER_SERVICE_TAG_NAME) : ep.serviceName; diff --git a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ZipkinSpanListener.java b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ZipkinSpanListener.java index ad63f007b..636b88a6b 100644 --- a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ZipkinSpanListener.java +++ b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ZipkinSpanListener.java @@ -136,7 +136,7 @@ public class ZipkinSpanListener implements SpanReporter { } private boolean hasClientSend(Span span) { - for (org.springframework.cloud.sleuth.Log log : span.logs()) { + for (Log log : span.logs()) { if (Constants.CLIENT_SEND.equals(log.getEvent())) { return !span.tags().containsKey(Constants.SERVER_ADDR); } @@ -183,16 +183,16 @@ public class ZipkinSpanListener implements SpanReporter { * duration (client receive - send) */ private long calculateDurationInMicros(Span span) { - org.springframework.cloud.sleuth.Log clientSend = hasLog(Span.CLIENT_SEND, span); - org.springframework.cloud.sleuth.Log clientReceived = hasLog(Span.CLIENT_RECV, span); + Log clientSend = hasLog(Span.CLIENT_SEND, span); + Log clientReceived = hasLog(Span.CLIENT_RECV, span); if (clientSend != null && clientReceived != null) { return (clientReceived.getTimestamp() - clientSend.getTimestamp()) * 1000; } return span.getAccumulatedMicros(); } - private org.springframework.cloud.sleuth.Log hasLog(String logName, Span span) { - for (org.springframework.cloud.sleuth.Log log : span.logs()) { + private Log hasLog(String logName, Span span) { + for (Log log : span.logs()) { if (logName.equals(log.getEvent())) { return log; }