zipkin-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <>
Subject [GitHub] [incubator-zipkin-brave] lambcode 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 16:07:14 GMT
lambcode commented on a change in pull request #921: Add tests for #920. "brave.fush" annotation
should not be reported for finished spans

 File path: brave/src/test/java/brave/
 @@ -693,6 +695,52 @@
+  @Test public void useSpanAfterFinished_doesNotCauseBraveFlush() throws InterruptedException
+    simulateInProcessPropagation(tracer, tracer);
+    blockOnGC();
+    tracer.newTrace().start().abandon(); //trigger orphaned span check
+    assertThat(spans).hasSize(1);
+    assertThat(
+      .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(
+      .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:
   I was not aware that the tracer/tracing was meant to be a singleton. The API lets one create
different instances via Tracing.newBuilder(). Is the intent that this builder is used only
once per application? If so, why does the API allow you to create multiple instances?
   As a little bit of background, we have different tracing instances each with a different
serviceName. As an example lets say I have webservice A<sub>ws</sub> and webservice
B<sub>ws</sub>. A<sub>ws</sub> requests information from B<sub>ws</sub>
through a library B<sub>client</sub> that makes http calls to B<sub>ws</sub>.
The B<sub>client</sub> library uses a tracing instance with serviceName "B" whereas
the A<sub>ws</sub> has a tracing instance with serviceName "A". A trace will start
in A<sub>ws</sub> with a span that has service name "A". This service will eventually
call into the B<sub>client</sub> library code which will create child spans (via
a different tracer) with serviceName "B".
   We've actually had some discussions internally recently on whether or not we should be
doing this, so some guidance on when different tracing instances should be used would be appreciated.

   As for the in-process propagation, I think you might be confused that the test does not
actually use multiple threads. I found that this is not strictly necessary because the behavior
is actually caused by using multiple tracing instances and scoped spans. I just found the
in-process propagation example to be an easy one to understand.

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:

With regards,
Apache Git Services

To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message