From f08b3057abbcfd8104f90ceb22fdddd7cc843ca2 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Thu, 6 Jul 2017 14:04:51 +0200 Subject: [PATCH] We no longer set SA for peer.service fixes #619 --- .../stream/ConvertToZipkinSpanList.java | 14 +++++------ .../stream/ConvertToZipkinSpanListTests.java | 21 ++++++++++++----- .../sleuth/zipkin/ZipkinSpanListener.java | 23 +++++++++---------- .../zipkin/ZipkinSpanListenerTests.java | 11 ++++++++- 4 files changed, 43 insertions(+), 26 deletions(-) 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 e03f246a8..bb9e50465 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 @@ -15,9 +15,6 @@ */ package org.springframework.cloud.sleuth.zipkin.stream; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; import org.apache.commons.logging.Log; import org.springframework.cloud.sleuth.Span; import org.springframework.cloud.sleuth.stream.Host; @@ -29,6 +26,10 @@ import zipkin.Constants; import zipkin.Endpoint; import zipkin.Span.Builder; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + /** * This converts sleuth spans to zipkin ones, skipping invalid or unsampled. * @@ -88,7 +89,7 @@ final class ConvertToZipkinSpanList { ZipkinMessageListener.addZipkinAnnotations(zipkinSpan, span, ep); ZipkinMessageListener.addZipkinBinaryAnnotations(zipkinSpan, span, ep); if (hasClientSend(span)) { - ensureServerAddr(span, zipkinSpan, ep); + ensureServerAddr(span, zipkinSpan); } // In the RPC span model, the client owns the timestamp and duration of the span. If we // were propagated an id, we can assume that we shouldn't report timestamp or duration, @@ -129,10 +130,9 @@ final class ConvertToZipkinSpanList { BinaryAnnotation.create(Constants.LOCAL_COMPONENT, processId, ep)); } - private static void ensureServerAddr(Span span, Builder zipkinSpan, - Endpoint ep) { + private static void ensureServerAddr(Span span, Builder zipkinSpan) { if (span.tags().containsKey(Span.SPAN_PEER_SERVICE_TAG_NAME)) { - Endpoint endpoint = ep.toBuilder().serviceName(span.tags().get( + Endpoint endpoint = Endpoint.builder().serviceName(span.tags().get( Span.SPAN_PEER_SERVICE_TAG_NAME)).build(); zipkinSpan.addBinaryAnnotation( BinaryAnnotation.address(Constants.SERVER_ADDR, endpoint)); diff --git a/spring-cloud-sleuth-zipkin-stream/src/test/java/org/springframework/cloud/sleuth/zipkin/stream/ConvertToZipkinSpanListTests.java b/spring-cloud-sleuth-zipkin-stream/src/test/java/org/springframework/cloud/sleuth/zipkin/stream/ConvertToZipkinSpanListTests.java index 6c5700e57..304a1b6eb 100644 --- a/spring-cloud-sleuth-zipkin-stream/src/test/java/org/springframework/cloud/sleuth/zipkin/stream/ConvertToZipkinSpanListTests.java +++ b/spring-cloud-sleuth-zipkin-stream/src/test/java/org/springframework/cloud/sleuth/zipkin/stream/ConvertToZipkinSpanListTests.java @@ -15,15 +15,18 @@ */ package org.springframework.cloud.sleuth.zipkin.stream; -import java.util.Arrays; -import java.util.Collections; -import java.util.List; -import java.util.Random; +import org.assertj.core.api.Condition; import org.junit.Test; import org.springframework.cloud.sleuth.Span; import org.springframework.cloud.sleuth.stream.Host; import org.springframework.cloud.sleuth.stream.Spans; import zipkin.Constants; +import zipkin.Endpoint; + +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Random; import static org.assertj.core.api.Assertions.assertThat; @@ -86,8 +89,14 @@ public class ConvertToZipkinSpanListTests { .hasSize(1) .flatExtracting(input1 -> input1.binaryAnnotations) .filteredOn("key", Constants.SERVER_ADDR) - .extracting(input -> input.endpoint.serviceName) - .contains("myservice"); + .extracting(input -> input.endpoint) + .hasSize(1) + .has(new Condition>() { + @Override public boolean matches(List value) { + Endpoint endpoint = value.get(0); + return endpoint.serviceName.equals("myservice") && endpoint.ipv4 == 0; + } + }); } @Test 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 a14722766..5e7b77744 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 @@ -16,6 +16,14 @@ package org.springframework.cloud.sleuth.zipkin; +import org.springframework.cloud.commons.util.IdUtils; +import org.springframework.cloud.sleuth.Log; +import org.springframework.cloud.sleuth.NoOpSpanAdjuster; +import org.springframework.cloud.sleuth.Span; +import org.springframework.cloud.sleuth.SpanAdjuster; +import org.springframework.cloud.sleuth.SpanReporter; +import org.springframework.core.env.Environment; +import org.springframework.util.StringUtils; import zipkin.Annotation; import zipkin.BinaryAnnotation; import zipkin.Constants; @@ -27,15 +35,6 @@ import java.util.Collections; import java.util.List; import java.util.Map; -import org.springframework.cloud.commons.util.IdUtils; -import org.springframework.cloud.sleuth.Log; -import org.springframework.cloud.sleuth.NoOpSpanAdjuster; -import org.springframework.cloud.sleuth.Span; -import org.springframework.cloud.sleuth.SpanAdjuster; -import org.springframework.cloud.sleuth.SpanReporter; -import org.springframework.core.env.Environment; -import org.springframework.util.StringUtils; - /** * Listener of Sleuth events. Reports to Zipkin via {@link ZipkinSpanReporter}. * @@ -149,10 +148,10 @@ public class ZipkinSpanListener implements SpanReporter { zipkinSpan.addBinaryAnnotation(component); } - private void ensureServerAddr(Span span, zipkin.Span.Builder zipkinSpan, Endpoint localEndpoint) { + private void ensureServerAddr(Span span, zipkin.Span.Builder zipkinSpan) { if (span.tags().containsKey(Span.SPAN_PEER_SERVICE_TAG_NAME)) { zipkinSpan.addBinaryAnnotation(BinaryAnnotation.address(Constants.SERVER_ADDR, - localEndpoint.toBuilder().serviceName( + Endpoint.builder().serviceName( span.tags().get(Span.SPAN_PEER_SERVICE_TAG_NAME)).build())); } } @@ -178,7 +177,7 @@ public class ZipkinSpanListener implements SpanReporter { ensureLocalComponent(span, zipkinSpan, endpoint); } if (hasClientSend) { - ensureServerAddr(span, zipkinSpan, endpoint); + ensureServerAddr(span, zipkinSpan); } if (instanceIdToTag && this.environment != null) { setInstanceIdIfPresent(zipkinSpan, endpoint, Span.INSTANCEID); diff --git a/spring-cloud-sleuth-zipkin/src/test/java/org/springframework/cloud/sleuth/zipkin/ZipkinSpanListenerTests.java b/spring-cloud-sleuth-zipkin/src/test/java/org/springframework/cloud/sleuth/zipkin/ZipkinSpanListenerTests.java index c14d60aad..8c5498b57 100644 --- a/spring-cloud-sleuth-zipkin/src/test/java/org/springframework/cloud/sleuth/zipkin/ZipkinSpanListenerTests.java +++ b/spring-cloud-sleuth-zipkin/src/test/java/org/springframework/cloud/sleuth/zipkin/ZipkinSpanListenerTests.java @@ -16,6 +16,7 @@ package org.springframework.cloud.sleuth.zipkin; +import org.assertj.core.api.Condition; import zipkin.Constants; import java.util.ArrayList; @@ -41,6 +42,7 @@ import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Primary; import org.springframework.mock.env.MockEnvironment; import org.springframework.test.context.junit4.SpringRunner; +import zipkin.Endpoint; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertEquals; @@ -219,7 +221,14 @@ public class ZipkinSpanListenerTests { assertThat(result.binaryAnnotations) .filteredOn("key", Constants.SERVER_ADDR) - .isNotEmpty(); + .extracting(input -> input.endpoint) + .hasSize(1) + .has(new Condition>() { + @Override public boolean matches(List value) { + Endpoint endpoint = value.get(0); + return endpoint.serviceName.equals("fooservice") && endpoint.ipv4 == 0; + } + }); } @Test