From 7ca2e8f94f3588c3fc84db45e8481ae069a3e8ea Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Tue, 24 Jul 2018 17:45:25 +0800 Subject: [PATCH] Adjusts expectations of data reporting of loopback spans (#1034) * Adjusts expectations of data reporting of loopback spans The latest version of Brave corrects a data merge problem where shared spans (created via Tracer.joinSpan) would lose some data. Along the way, this fixes some misuse of `Tracer.joinSpan` which should only be used for RPC server spans. See https://github.com/openzipkin/brave/pull/734 applied in Brave 5.1.3 * Makes the test more better --- .../rxjava/SleuthRxJavaSchedulersHook.java | 2 +- .../documentation/SpringCloudSleuthDocTests.java | 2 +- .../web/SpringDataInstrumentationTests.java | 13 +++++++++---- .../web/multiple/MultipleHopsIntegrationTests.java | 2 +- .../java/integration/MessagingApplicationTests.java | 2 +- 5 files changed, 13 insertions(+), 8 deletions(-) diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/rxjava/SleuthRxJavaSchedulersHook.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/rxjava/SleuthRxJavaSchedulersHook.java index 196395d30..57089deba 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/rxjava/SleuthRxJavaSchedulersHook.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/instrument/rxjava/SleuthRxJavaSchedulersHook.java @@ -128,7 +128,7 @@ class SleuthRxJavaSchedulersHook extends RxJavaSchedulersHook { Span span = this.parent; boolean created = false; if (span != null) { - span = this.tracer.joinSpan(this.parent.context()); + span = this.tracer.toSpan(this.parent.context()); } else { span = this.tracer.nextSpan().name(RXJAVA_COMPONENT).start(); span.tag(THREAD_NAME_KEY, Thread.currentThread().getName()); diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/documentation/SpringCloudSleuthDocTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/documentation/SpringCloudSleuthDocTests.java index a3ca39dbc..60a1a2065 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/documentation/SpringCloudSleuthDocTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/documentation/SpringCloudSleuthDocTests.java @@ -173,7 +173,7 @@ public class SpringCloudSleuthDocTests { // tag::manual_span_continuation[] // let's assume that we're in a thread Y and we've received // the `initialSpan` from thread X - Span continuedSpan = this.tracer.joinSpan(newSpan.context()); + Span continuedSpan = this.tracer.toSpan(newSpan.context()); try { // ... // You can tag a span diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/SpringDataInstrumentationTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/SpringDataInstrumentationTests.java index a080a6b30..d750e3cb2 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/SpringDataInstrumentationTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/SpringDataInstrumentationTests.java @@ -46,7 +46,9 @@ import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.ActiveProfiles; import org.springframework.test.context.junit4.SpringRunner; import org.springframework.web.client.RestTemplate; +import zipkin2.Span; +import static org.assertj.core.api.Assertions.tuple; import static org.assertj.core.api.BDDAssertions.then; /** @@ -81,10 +83,13 @@ public class SpringDataInstrumentationTests { then(noOfNames).isEqualTo(8); then(this.reporter.getSpans()).isNotEmpty(); Awaitility.await().untilAsserted(() -> { - then(this.reporter.getSpans()).hasSize(1); - zipkin2.Span storedSpan = this.reporter.getSpans().get(0); - then(storedSpan.name()).isEqualTo("http:/reservations"); - then(storedSpan.tags()).containsKey("mvc.controller.class"); + // Make sure the data is attached to the right side of the span + then(this.reporter.getSpans()) + .extracting(Span::kind, Span::name, s -> s.tags().get("mvc.controller.class")) + .containsExactlyInAnyOrder( + tuple(Span.Kind.CLIENT, "http:/reservations", null), + tuple(Span.Kind.SERVER, "http:/reservations", "RepositoryEntityController") + ); }); then(this.tracer.currentSpan()).isNull(); } diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/multiple/MultipleHopsIntegrationTests.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/multiple/MultipleHopsIntegrationTests.java index 6ff1cac3b..a34c3c6a5 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/multiple/MultipleHopsIntegrationTests.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/instrument/web/multiple/MultipleHopsIntegrationTests.java @@ -77,7 +77,7 @@ public class MultipleHopsIntegrationTests { this.restTemplate.getForObject("http://localhost:" + this.config.port + "/greeting", String.class); await().atMost(5, SECONDS).untilAsserted(() -> { - then(this.reporter.getSpans()).hasSize(13); + then(this.reporter.getSpans()).hasSize(14); }); then(this.reporter.getSpans().stream().map(zipkin2.Span::name) .collect(toList())).containsAll(asList("http:/greeting", "send")); 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 1fe53a086..edb21e68d 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 @@ -129,7 +129,7 @@ public class MessagingApplicationTests extends AbstractIntegrationTest { Optional lastHttpSpansParent = findLastHttpSpansParent(); // "http:/parent/" -> "message:messages" -> "http:/foo" (CS + CR) -> "http:/foo" (SS) thenAllSpansArePresent(firstHttpSpan, eventSpans, lastHttpSpansParent, eventSentSpan, producerSpan); - then(this.integrationTestSpanCollector.hashedSpans).as("There were 5 spans").hasSize(5); + then(this.integrationTestSpanCollector.hashedSpans).as("There were 6 spans").hasSize(6); log.info("Checking the parent child structure"); List> parentChild = this.integrationTestSpanCollector.hashedSpans.stream() .filter(span -> span.parentId() != null)