From 87ff492f2d47be8e4a97c82e5b24f8583d5c87df Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Sat, 7 Nov 2015 07:56:32 -0800 Subject: [PATCH] Updates to latest zipkin/brave Zipkin now has Span.timestamp/duration, so no more need for "aquire/release". This also corrects the host/endpoint logged for timeline annotations, as it should always be the local process. --- README.adoc | 2 - pom.xml | 4 +- spring-cloud-sleuth-zipkin/docker-compose.yml | 30 ++++++---- spring-cloud-sleuth-zipkin/pom.xml | 2 +- .../DiscoveryClientEndpointLocator.java | 2 +- .../cloud/sleuth/zipkin/EndpointLocator.java | 7 +-- .../ServerPropertiesEndpointLocator.java | 40 ++----------- .../zipkin/ZipkinAutoConfiguration.java | 10 ++-- .../cloud/sleuth/zipkin/ZipkinProperties.java | 4 +- .../sleuth/zipkin/ZipkinSpanListener.java | 59 ++++++++++++------- 10 files changed, 72 insertions(+), 88 deletions(-) diff --git a/README.adoc b/README.adoc index c738fa909..07aa62661 100644 --- a/README.adoc +++ b/README.adoc @@ -48,8 +48,6 @@ logging: 8. Hit `http://localhost:3380`, `http://localhost:3380/call`, `http://localhost:3380/async` for some interesting sample traces (the app callas back to itself). 9. Goto `http://localhost:8082` for zipkin web (8080 if running locally from source, the host is the docker host, so if you are using boot2docker it will be different) -WARNING: The docker images for zipkin are old and don't work very well (the UI in particular). Zipkin is in a state of flux, but it should settle down soon when there is an actual release. Best results actually come from building from source and running the jar files (the query and collector services need command line arguments, so check the zipkin README for updates). - NOTE: You can see the zipkin spans without the UI (in logs) if you run the sample with `sample.zipkin.enabled=false`. There are a few samples with slightly different features: diff --git a/pom.xml b/pom.xml index 3c54f3aa9..465050911 100644 --- a/pom.xml +++ b/pom.xml @@ -129,7 +129,7 @@ com.github.kristofa - brave-zipkin-spancollector + brave-spancollector-scribe ${brave.version} @@ -202,7 +202,7 @@ - 3.0.0 + 3.1.0 1.1.0.BUILD-SNAPSHOT 1.8.4 1.0-groovy-2.4 diff --git a/spring-cloud-sleuth-zipkin/docker-compose.yml b/spring-cloud-sleuth-zipkin/docker-compose.yml index ae9ea8b0d..85ea87952 100644 --- a/spring-cloud-sleuth-zipkin/docker-compose.yml +++ b/spring-cloud-sleuth-zipkin/docker-compose.yml @@ -1,35 +1,39 @@ -cassandra: - image: quay.io/openzipkin/zipkin-cassandra:1.9.0 +mysql: + image: openzipkin/zipkin-mysql:1.25.0 ports: - - 9042:9042 + - 3306:3306 collector: - image: quay.io/openzipkin/zipkin-collector:1.9.0 + image: openzipkin/zipkin-collector:1.25.0 environment: - - BLOCK_ON_CASSANDRA=true + - TRANSPORT_TYPE=scribe + - STORAGE_TYPE=mysql expose: - 9410 ports: - 9410:9410 - 9900:9900 links: - - cassandra:db + - mysql:storage query: - image: quay.io/openzipkin/zipkin-query:1.9.0 + image: openzipkin/zipkin-query:1.25.0 environment: - - BLOCK_ON_CASSANDRA=true + # Remove TRANSPORT_TYPE to disable tracing + - TRANSPORT_TYPE=http + - STORAGE_TYPE=mysql expose: - 9411 ports: - 9411:9411 - 9901:9901 links: - - cassandra:db - - collector + - mysql:storage web: - image: quay.io/openzipkin/zipkin-web:1.9.0 + image: openzipkin/zipkin-web:1.25.0 + environment: + # Remove TRANSPORT_TYPE to disable tracing + - TRANSPORT_TYPE=http ports: - 8080:8080 - 9990:9990 links: - - collector - - query + - query \ No newline at end of file diff --git a/spring-cloud-sleuth-zipkin/pom.xml b/spring-cloud-sleuth-zipkin/pom.xml index 38e92f145..610de99ac 100644 --- a/spring-cloud-sleuth-zipkin/pom.xml +++ b/spring-cloud-sleuth-zipkin/pom.xml @@ -43,7 +43,7 @@ com.github.kristofa - brave-zipkin-spancollector + brave-spancollector-scribe org.projectlombok diff --git a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/DiscoveryClientEndpointLocator.java b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/DiscoveryClientEndpointLocator.java index e816388d4..083facc0a 100644 --- a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/DiscoveryClientEndpointLocator.java +++ b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/DiscoveryClientEndpointLocator.java @@ -41,7 +41,7 @@ public class DiscoveryClientEndpointLocator implements EndpointLocator { } @Override - public Endpoint locate(Span span) { + public Endpoint local() { ServiceInstance instance = this.client.getLocalServiceInstance(); return new Endpoint(getIpAddress(instance), new Integer(instance.getPort()).shortValue(), instance.getServiceId()); diff --git a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/EndpointLocator.java b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/EndpointLocator.java index 2d3a1bf4f..8265d25a7 100644 --- a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/EndpointLocator.java +++ b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/EndpointLocator.java @@ -16,19 +16,16 @@ package org.springframework.cloud.sleuth.zipkin; -import org.springframework.cloud.sleuth.Span; - import com.twitter.zipkin.gen.Endpoint; /** - * Strategy for locating a Brave "Endpoin" from a Spring Cloud Span (and whatever other - * environment properties might be available). + * Strategy for locating a zipkin {@linkplain Endpoint} for the current process. * * @author Dave Syer * */ public interface EndpointLocator { - Endpoint locate(Span span); + Endpoint local(); } diff --git a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ServerPropertiesEndpointLocator.java b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ServerPropertiesEndpointLocator.java index fcf07e54f..7e54b93ac 100644 --- a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ServerPropertiesEndpointLocator.java +++ b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ServerPropertiesEndpointLocator.java @@ -16,14 +16,10 @@ package org.springframework.cloud.sleuth.zipkin; -import java.net.InetAddress; -import java.net.UnknownHostException; -import java.nio.ByteBuffer; - import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.autoconfigure.web.ServerProperties; import org.springframework.boot.context.embedded.EmbeddedServletContainerInitializedEvent; -import org.springframework.cloud.sleuth.Span; +import org.springframework.cloud.util.InetUtils; import org.springframework.context.event.EventListener; import com.twitter.zipkin.gen.Endpoint; @@ -46,11 +42,10 @@ public class ServerPropertiesEndpointLocator implements EndpointLocator { } @Override - public Endpoint locate(Span span) { - String serviceName = getServiceName(span); + public Endpoint local() { int address = getAddress(); Integer port = getPort(); - Endpoint ep = new Endpoint(address, port.shortValue(), serviceName); + Endpoint ep = new Endpoint(address, port.shortValue(), appName); return ep; } @@ -74,36 +69,11 @@ public class ServerPropertiesEndpointLocator implements EndpointLocator { } private int getAddress() { - String address; if (this.serverProperties!=null && this.serverProperties.getAddress() != null) { - address = this.serverProperties.getAddress().getHostAddress(); + return InetUtils.convert(this.serverProperties.getAddress()).getIpAddressAsInt(); } else { - address = "127.0.0.1"; + return 127 <<24|1; } - return ipAddressToInt(address); } - - private String getServiceName(Span span) { - String serviceName; - if (span.getProcessId() != null) { - serviceName = span.getProcessId().toLowerCase(); - } - else { - serviceName = this.appName; - } - return serviceName; - } - - private int ipAddressToInt(final String ip) { - InetAddress inetAddress = null; - try { - inetAddress = InetAddress.getByName(ip); - } - catch (final UnknownHostException e) { - throw new IllegalArgumentException(e); - } - return ByteBuffer.wrap(inetAddress.getAddress()).getInt(); - } - } diff --git a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ZipkinAutoConfiguration.java b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ZipkinAutoConfiguration.java index 82b401c2b..72e1095d0 100644 --- a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ZipkinAutoConfiguration.java +++ b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ZipkinAutoConfiguration.java @@ -28,22 +28,22 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import com.github.kristofa.brave.SpanCollector; -import com.github.kristofa.brave.zipkin.ZipkinSpanCollector; +import com.github.kristofa.brave.scribe.ScribeSpanCollector; /** * @author Spencer Gibb */ @Configuration @EnableConfigurationProperties -@ConditionalOnClass(ZipkinSpanCollector.class) +@ConditionalOnClass(ScribeSpanCollector.class) @ConditionalOnProperty(value = "spring.zipkin.enabled", matchIfMissing = true) public class ZipkinAutoConfiguration { @Bean @ConditionalOnMissingBean(SpanCollector.class) - public ZipkinSpanCollector spanCollector() { + public ScribeSpanCollector spanCollector() { ZipkinProperties zipkin = zipkinProperties(); - ZipkinSpanCollector collector = new ZipkinSpanCollector(zipkin.getHost(), + ScribeSpanCollector collector = new ScribeSpanCollector(zipkin.getHost(), zipkin.getPort(), zipkin.getCollector()); return collector; } @@ -55,7 +55,7 @@ public class ZipkinAutoConfiguration { @Bean public ZipkinSpanListener sleuthTracer(SpanCollector spanCollector, EndpointLocator endpointLocator) { - return new ZipkinSpanListener(spanCollector, endpointLocator); + return new ZipkinSpanListener(spanCollector, endpointLocator.local()); } @Configuration diff --git a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ZipkinProperties.java b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ZipkinProperties.java index 963d3d1c2..5d08cd217 100644 --- a/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ZipkinProperties.java +++ b/spring-cloud-sleuth-zipkin/src/main/java/org/springframework/cloud/sleuth/zipkin/ZipkinProperties.java @@ -18,7 +18,7 @@ package org.springframework.cloud.sleuth.zipkin; import org.springframework.boot.context.properties.ConfigurationProperties; -import com.github.kristofa.brave.zipkin.ZipkinSpanCollectorParams; +import com.github.kristofa.brave.scribe.ScribeSpanCollectorParams; import lombok.Data; @@ -33,5 +33,5 @@ public class ZipkinProperties { private String host = "localhost"; private int port = 9410; private boolean enabled = true; - private ZipkinSpanCollectorParams collector = new ZipkinSpanCollectorParams(); + private ScribeSpanCollectorParams collector = new ScribeSpanCollectorParams(); } 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 f0d4d74d6..5ada65904 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 @@ -49,16 +49,21 @@ import lombok.extern.apachecommons.CommonsLog; public class ZipkinSpanListener { private SpanCollector spanCollector; - private EndpointLocator endpointLocator; + /** + * Endpoint is the visible IP address of this service, the port it is + * listening on and the service name from discovery. + */ + private Endpoint localEndpoint; - public ZipkinSpanListener(SpanCollector spanCollector, EndpointLocator endpointLocator) { + public ZipkinSpanListener(SpanCollector spanCollector, Endpoint localEndpoint) { this.spanCollector = spanCollector; - this.endpointLocator = endpointLocator; + this.localEndpoint = localEndpoint; } @EventListener @Order(0) public void start(SpanAcquiredEvent event) { + // Starting a span in zipkin means adding: traceId, id, parentId(optional), and timestamp event.getSpan().addTimelineAnnotation("acquire"); } @@ -66,6 +71,9 @@ public class ZipkinSpanListener { @Order(0) public void serverReceived(ServerReceivedEvent event) { if (event.getParent() != null && event.getParent().isRemote()) { + // If an inbound RPC call, it should log a "sr" annotation. + // If possible, it should log a binary annotation of "ca", indicating the + // caller's address (ex X-Forwarded-For header) event.getParent().addTimelineAnnotation(zipkinCoreConstants.SERVER_RECV); } } @@ -73,6 +81,9 @@ public class ZipkinSpanListener { @EventListener @Order(0) public void clientSend(ClientSentEvent event) { + // For an outbound RPC call, it should log a "cs" annotation. + // If possible, it should log a binary annotation of "sa", indicating the + // destination address. event.getSpan().addTimelineAnnotation(zipkinCoreConstants.CLIENT_SEND); } @@ -94,6 +105,7 @@ public class ZipkinSpanListener { @EventListener @Order(0) public void release(SpanReleasedEvent event) { + // Ending a span in zipkin means adding duration and sending it out event.getSpan().addTimelineAnnotation("release"); this.spanCollector.collect(convert(event.getSpan())); } @@ -108,11 +120,8 @@ public class ZipkinSpanListener { */ public com.twitter.zipkin.gen.Span convert(Span span) { com.twitter.zipkin.gen.Span zipkinSpan = new com.twitter.zipkin.gen.Span(); - - Endpoint ep = this.endpointLocator.locate(span); - List annotationList = createZipkinAnnotations(span, ep); - List binaryAnnotationList = createZipkinBinaryAnnotations(span, - ep); + addZipkinAnnotations(zipkinSpan, span, localEndpoint); + List binaryAnnotationList = createZipkinBinaryAnnotations(span, localEndpoint); zipkinSpan.setTrace_id(hash(span.getTraceId())); if (span.getParents().size() > 0) { if (span.getParents().size() > 1) { @@ -125,7 +134,6 @@ public class ZipkinSpanListener { if (StringUtils.hasText(span.getName())) { zipkinSpan.setName(span.getName()); } - zipkinSpan.setAnnotations(annotationList); zipkinSpan.setBinary_annotations(binaryAnnotationList); return zipkinSpan; } @@ -134,14 +142,27 @@ public class ZipkinSpanListener { /** * Add annotations from the sleuth Span. */ - private List createZipkinAnnotations(Span span, Endpoint endpoint) { - List annotationList = new ArrayList<>(); + private void addZipkinAnnotations(com.twitter.zipkin.gen.Span zipkinSpan, + Span span, Endpoint endpoint) { + Long startTs = null; + Long endTs = null; for (TimelineAnnotation ta : span.getTimelineAnnotations()) { Annotation zipkinAnnotation = createZipkinAnnotation(ta.getMsg(), - ta.getTime(), endpoint, true); - annotationList.add(zipkinAnnotation); + ta.getTime(), endpoint); + if (zipkinAnnotation.getValue().equals("acquire")) { + startTs = zipkinAnnotation.getTimestamp(); + } else if (zipkinAnnotation.getValue().equals("release")) { + endTs = zipkinAnnotation.getTimestamp(); + } else { + zipkinSpan.addToAnnotations(zipkinAnnotation); + } + } + if (startTs != null) { + zipkinSpan.setTimestamp(startTs); + if (endTs != null) { + zipkinSpan.setDuration(endTs - zipkinSpan.getTimestamp()); + } } - return annotationList; } /** @@ -174,20 +195,14 @@ public class ZipkinSpanListener { * @param value Annotation value * @param time timestamp will be extracted * @param endpoint the endpoint this annotation will be associated with. - * @param sendRequest use the first or last timestamp. */ private static Annotation createZipkinAnnotation(String value, long time, - Endpoint endpoint, boolean sendRequest) { + Endpoint endpoint) { Annotation annotation = new Annotation(); annotation.setHost(endpoint); // Zipkin is in microseconds - if (sendRequest) { - annotation.setTimestamp(time * 1000); - } - else { - annotation.setTimestamp(time * 1000); - } + annotation.setTimestamp(time * 1000); annotation.setValue(value); return annotation; }