zipkin-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [incubator-zipkin-brave] adriancole commented on a change in pull request #921: Add tests for #920. "brave.fush" annotation should not be reported for finished spans
Date Wed, 12 Jun 2019 05:52:32 GMT
adriancole commented on a change in pull request #921: Add tests for #920. "brave.fush" annotation
should not be reported for finished spans
URL: https://github.com/apache/incubator-zipkin-brave/pull/921#discussion_r292754908
 
 

 ##########
 File path: brave/src/test/java/brave/TracerTest.java
 ##########
 @@ -693,6 +695,52 @@
         .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) {
 
 Review comment:
   this is an odd use case. the tracer (tracing etc) is a singleton and meant to be used as
such.
   
   In-process propagation involves moving the Span or TraceContext across threads, not switching
tracer instances per thread.
   
   Can you elaborate why you are doing this? It might cause problems other than the scenario
noted here.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@zipkin.apache.org
For additional commands, e-mail: dev-help@zipkin.apache.org


Mime
View raw message