[incubator-zipkin-brave] branch master updated: Add tests for "brave.flush" annotation should not be reported for finished spans (#921)

[ad...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

adriancole pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-zipkin-brave.git


The following commit(s) were added to refs/heads/master by this push:
     new 7dbb53f  Add tests for "brave.flush" annotation should not be reported for finished spans (#921)
7dbb53f is described below

commit 7dbb53f7f6dd540d2d7fd0ad37b4c6a424af5176
Author: lambcode <58...@users.noreply.github.com>
AuthorDate: Thu Jun 13 01:12:06 2019 -0600

    Add tests for "brave.flush" annotation should not be reported for finished spans (#921)
---
 .../java/brave/internal/recorder/PendingSpans.java |  2 +-
 brave/src/test/java/brave/TracerTest.java          | 53 ++++++++++++++++++++++
 2 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/brave/src/main/java/brave/internal/recorder/PendingSpans.java b/brave/src/main/java/brave/internal/recorder/PendingSpans.java
index 2fb21cf..26e1354 100644
--- a/brave/src/main/java/brave/internal/recorder/PendingSpans.java
+++ b/brave/src/main/java/brave/internal/recorder/PendingSpans.java
@@ -46,7 +46,7 @@ import java.util.logging.Logger;
 public final class PendingSpans extends ReferenceQueue<TraceContext> {
   private static final Logger LOG = Logger.getLogger(PendingSpans.class.getName());
 
-  // Eventhough we only put by RealKey, we allow get and remove by LookupKey
+  // Even though we only put by RealKey, we allow get and remove by LookupKey
   final ConcurrentMap<Object, PendingSpan> delegate = new ConcurrentHashMap<>(64);
   final Clock clock;
   final FinishedSpanHandler zipkinHandler; // Used when flushing spans
diff --git a/brave/src/test/java/brave/TracerTest.java b/brave/src/test/java/brave/TracerTest.java
index dd20494..353f275 100644
--- a/brave/src/test/java/brave/TracerTest.java
+++ b/brave/src/test/java/brave/TracerTest.java
@@ -34,8 +34,10 @@ import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.function.Function;
+import java.util.stream.Collectors;
 import org.junit.After;
 import org.junit.Test;
+import zipkin2.Annotation;
 import zipkin2.Endpoint;
 import zipkin2.reporter.Reporter;
 
@@ -693,6 +695,52 @@ public class TracerTest {
         .isPositive();
   }
 
+  @Test public void useSpanAfterFinished_doesNotCauseBraveFlush() throws InterruptedException {
+    simulateInProcessPropagation(tracer, tracer);
+    blockOnGC();
+    tracer.newTrace().start().abandon(); //trigger orphaned span check
+    assertThat(spans).hasSize(1);
+    assertThat(spans.stream()
+      .flatMap(span -> span.annotations().stream())
+      .map(Annotation::value)
+      .collect(Collectors.toList())).doesNotContain("brave.flush");
+  }
+
+  @Test public void useSpanAfterFinishedInOtherTracer_doesNotCauseBraveFlush()
+    throws InterruptedException {
+    Tracer noOpTracer = Tracing.newBuilder()
+      .build().tracer();
+    simulateInProcessPropagation(noOpTracer, tracer);
+    blockOnGC();
+    tracer.newTrace().start().abandon(); //trigger orphaned span check
+
+    // We expect the span to be reported to the NOOP reporter, and nothing to be reported to "spans"
+    assertThat(spans).hasSize(0);
+    assertThat(spans.stream()
+      .flatMap(span -> span.annotations().stream())
+      .map(Annotation::value)
+      .collect(Collectors.toList())).doesNotContain("brave.flush");
+  }
+
+  /**
+   * Must be a separate method from the test method to allow for local variables to be garbage
+   * collected
+   */
+  private static void simulateInProcessPropagation(Tracer tracer1, Tracer tracer2) {
+    Span span1 = tracer1.newTrace();
+    span1.start();
+
+    // Pretend we're on child thread
+    Tracer.SpanInScope spanInScope = tracer2.withSpanInScope(span1);
+
+    // Back on original thread
+    span1.finish();
+
+    // Pretend we're on child thread
+    Span span2 = tracer2.currentSpan();
+    spanInScope.close();
+  }
+
   @Test public void localRootId_joinSpan_notYetSampled() {
     TraceContext context1 = TraceContext.newBuilder().traceId(1).spanId(2).build();
     TraceContext context2 = TraceContext.newBuilder().traceId(1).spanId(3).build();
@@ -826,4 +874,9 @@ public class TracerTest {
     }).spanReporter(Reporter.NOOP).build().tracer();
     return reportedNames;
   }
+
+  static void blockOnGC() throws InterruptedException {
+    System.gc();
+    Thread.sleep(200L);
+  }
 }