diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/integration/TraceChannelInterceptor.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/integration/TraceChannelInterceptor.java index 605bf390c..6db8f276f 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/integration/TraceChannelInterceptor.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/integration/TraceChannelInterceptor.java @@ -16,6 +16,8 @@ package org.springframework.cloud.sleuth.instrument.integration; +import java.util.Random; + import org.springframework.cloud.sleuth.Span; import org.springframework.cloud.sleuth.Tracer; import org.springframework.cloud.sleuth.instrument.TraceKeys; @@ -23,8 +25,6 @@ import org.springframework.cloud.sleuth.sampler.IsTracingSampler; import org.springframework.messaging.Message; import org.springframework.messaging.MessageChannel; -import java.util.Random; - /** * @author Dave Syer * @@ -40,19 +40,15 @@ public class TraceChannelInterceptor extends AbstractTraceChannelInterceptor { @Override public void postSend(Message message, MessageChannel channel, boolean sent) { Span trace = this.traceHolder.get(); - // Double close to clean up the parent (remote span as well) - getTracer().close(getTracer().close(trace)); + getTracer().close(trace); this.traceHolder.remove(); } @Override public Message preSend(Message message, MessageChannel channel) { - if (getTracer().isTracing()) { - return SpanMessageHeaders.addSpanHeaders(getTraceKeys(), message, - getTracer().getCurrentSpan()); - } + Span parentSpan = getTracer().isTracing() ? getTracer().getCurrentSpan() : buildSpan(message); String name = getMessageChannelName(channel); - Span span = startSpan(buildSpan(message), name, message); + Span span = startSpan(parentSpan, name, message); this.traceHolder.set(span); return SpanMessageHeaders.addSpanHeaders(getTraceKeys(), message, span); } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/integration/TraceChannelInterceptorTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/integration/TraceChannelInterceptorTests.java index c72057cbb..2deaf5669 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/integration/TraceChannelInterceptorTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/integration/TraceChannelInterceptorTests.java @@ -16,12 +16,6 @@ package org.springframework.cloud.sleuth.instrument.integration; -import static org.assertj.core.api.BDDAssertions.then; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; - import java.util.ArrayList; import java.util.List; @@ -52,6 +46,12 @@ import org.springframework.messaging.MessagingException; import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; +import static org.assertj.core.api.BDDAssertions.then; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; + /** * @author Dave Syer */ @@ -120,7 +120,6 @@ public class TraceChannelInterceptorTests implements MessageHandler { .fromHex(this.message.getHeaders().get(Span.TRACE_ID_NAME, String.class)); then(traceId).isEqualTo(10L); then(spanId).isNotEqualTo(20L); - assertNull(SpanContextHolder.getCurrentSpan()); assertEquals(1, this.app.events.size()); } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/integration/TraceContextPropagationChannelInterceptorTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/integration/TraceContextPropagationChannelInterceptorTests.java index 978e9e023..d615efa8e 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/integration/TraceContextPropagationChannelInterceptorTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/integration/TraceContextPropagationChannelInterceptorTests.java @@ -38,7 +38,7 @@ import org.springframework.messaging.PollableChannel; import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; -import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; /** @@ -76,7 +76,7 @@ public class TraceContextPropagationChannelInterceptorTests { Long spanId = Span .fromHex(message.getHeaders().get(Span.SPAN_ID_NAME, String.class)); - assertEquals("spanId was wrong", expectedSpanId, spanId); + assertNotEquals("spanId was equal to parent's id", expectedSpanId, spanId); long traceId = Span .fromHex(message.getHeaders().get(Span.TRACE_ID_NAME, String.class)); diff --git a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-messaging/src/test/java/integration/MessagingApplicationTests.java b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-messaging/src/test/java/integration/MessagingApplicationTests.java index b0d16d0a3..85180dac6 100644 --- a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-messaging/src/test/java/integration/MessagingApplicationTests.java +++ b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-messaging/src/test/java/integration/MessagingApplicationTests.java @@ -15,6 +15,12 @@ */ package integration; +import java.util.Collection; +import java.util.List; +import java.util.Optional; +import java.util.Random; +import java.util.stream.Collectors; + import integration.MessagingApplicationTests.IntegrationSpanCollectorConfig; import org.junit.After; import org.junit.Test; @@ -31,10 +37,6 @@ import sample.SampleMessagingApplication; import tools.AbstractIntegrationTest; import zipkin.Span; -import java.util.Collection; -import java.util.Optional; -import java.util.Random; - import static org.assertj.core.api.BDDAssertions.then; @RunWith(SpringJUnit4ClassRunner.class) @@ -43,6 +45,8 @@ import static org.assertj.core.api.BDDAssertions.then; @TestPropertySource(properties="sample.zipkin.enabled=true") public class MessagingApplicationTests extends AbstractIntegrationTest { + private static final String SERVER_SENT = "ss"; + private static final String CLIENT_RECEIVED = "cr"; private static int port = 3381; private static String sampleAppUrl = "http://localhost:" + port; @Autowired IntegrationTestZipkinSpanReporter integrationTestSpanCollector; @@ -72,7 +76,7 @@ public class MessagingApplicationTests extends AbstractIntegrationTest { await().until(() -> { thenAllSpansHaveTraceIdEqualTo(traceId); - thenTheLastSpansParentHasIdEqualToFirstSpansId(); + thenTheSpansHaveProperParentStructure(); }); } @@ -99,14 +103,56 @@ public class MessagingApplicationTests extends AbstractIntegrationTest { then(this.integrationTestSpanCollector.hashedSpans.stream().allMatch(span -> span.traceId == traceId)).isTrue(); } - private void thenTheLastSpansParentHasIdEqualToFirstSpansId() { - Optional firstSpan = this.integrationTestSpanCollector.hashedSpans.stream() - .filter(span -> "http/".equals(span.name) && span.parentId != null).findFirst(); - Optional lastSpan = this.integrationTestSpanCollector.hashedSpans.stream() + private void thenTheSpansHaveProperParentStructure() { + Optional firstHttpSpan = findFirstHttpRequestSpan(); + List eventSpans = findAllEventRelatedSpans(); + Optional eventSentSpan = findSpanWithServerSentAnnotation(eventSpans); + Optional eventReceivedSpan = findSpanWithClientReceivedAnnotation(eventSpans); + Optional lastHttpSpan = findLastHttpSpan(); + thenAllSpansArePresent(firstHttpSpan, eventSpans, lastHttpSpan, eventSentSpan, eventReceivedSpan); + then(lastHttpSpan.get().parentId).isEqualTo(eventSentSpan.get().id); + then(eventSentSpan.get().parentId).isEqualTo(firstHttpSpan.get().id); + then(eventSentSpan.get()).isNotEqualTo(eventReceivedSpan.get()); + } + + private Optional findLastHttpSpan() { + return this.integrationTestSpanCollector.hashedSpans.stream() .filter(span -> "http/foo".equals(span.name)).findFirst(); - then(firstSpan.isPresent()).isTrue(); - then(lastSpan.isPresent()).isTrue(); - then(lastSpan.get().parentId).isEqualTo(firstSpan.get().id); + } + + private Optional findSpanWithClientReceivedAnnotation(List eventSpans) { + return eventSpans.stream() + .filter(span -> span.annotations.stream().filter(annotation -> CLIENT_RECEIVED + .equals(annotation.value)).findFirst().isPresent()) + .findFirst(); + } + + private Optional findSpanWithServerSentAnnotation(List eventSpans) { + return eventSpans.stream() + .filter(span -> span.annotations.stream().filter(annotation -> SERVER_SENT + .equals(annotation.value)).findFirst().isPresent()) + .findFirst(); + } + + private List findAllEventRelatedSpans() { + return this.integrationTestSpanCollector.hashedSpans.stream() + .filter(span -> "message/messages".equals(span.name) && span.parentId != null).collect( + Collectors.toList()); + } + + private Optional findFirstHttpRequestSpan() { + return this.integrationTestSpanCollector.hashedSpans.stream() + .filter(span -> "http/".equals(span.name) && span.parentId != null).findFirst(); + } + + private void thenAllSpansArePresent(Optional firstHttpSpan, + List eventSpans, Optional lastHttpSpan, + Optional eventSentSpan, Optional eventReceivedSpan) { + then(firstHttpSpan.isPresent()).isTrue(); + then(eventSpans).isNotEmpty(); + then(eventSentSpan.isPresent()).isTrue(); + then(eventReceivedSpan.isPresent()).isTrue(); + then(lastHttpSpan.isPresent()).isTrue(); } @Configuration diff --git a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-zipkin-stream/src/test/java/integration/ZipkinStreamTests.java b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-zipkin-stream/src/test/java/integration/ZipkinStreamTests.java index 4381584bc..c2a2e7a52 100644 --- a/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-zipkin-stream/src/test/java/integration/ZipkinStreamTests.java +++ b/spring-cloud-sleuth-samples/spring-cloud-sleuth-sample-zipkin-stream/src/test/java/integration/ZipkinStreamTests.java @@ -62,12 +62,12 @@ public class ZipkinStreamTests extends AbstractIntegrationTest { @Test public void should_propagate_spans_to_zipkin() { - Span span = Span.builder().traceId(traceId).spanId(spanId).name("test").build(); + Span span = Span.builder().traceId(this.traceId).spanId(this.spanId).name("test").build(); span.tag(getRequiredBinaryAnnotationName(), "10131"); this.input.send(messageWithSpan(span)); - await().until(allSpansWereRegisteredInZipkinWithTraceIdEqualTo(traceId)); + await().until(allSpansWereRegisteredInZipkinWithTraceIdEqualTo(this.traceId)); } private Message messageWithSpan(Span span) {