Adds brave.Tracing an object graph that holds things you need (#381)

Before, we added some helpers like `Tracer.clock()` because when instrumenting, you sometimes need more than just the Tracer. Recently, we needed to decide how to expose the propagation format.. should we add another convenience method? Taking experience from Zipkin (which took its experience from Dagger and Spring), another way is to expose an object graph.. a component that provides types often used together. This adds `brave.Tracing` which can be passed around or implemented differently as needed. Instrumentation would consume this to get common types they need. For example, `JaxRs2Tracing` would accept an instance of tracing, so that it has what it needs, a tracer and a propagation handler. Instances of this can be wired up in Spring (Dagger, Guice or CDI), making instrumentation wiring far simpler than chaining builders. Thanks to @bogdandrutu and @llinder for discussions leading to this.
openzipkin · Apr 22, 2017 · 7fd9bae · 7fd9bae
1 parent 849c4d1
commit 7fd9bae
Show file tree

Hide file tree

Showing 29 changed files with 412 additions and 229 deletions.
diff --git a/brave-benchmarks/src/main/java/brave/SpanCreationBenchmarks.java b/brave-benchmarks/src/main/java/brave/SpanCreationBenchmarks.java
@@ -40,9 +40,9 @@ public class SpanCreationBenchmarks {
   @Setup
   public void setup() {
     // real everything except reporting
-    tracer = Tracer.newBuilder()
+    tracer = Tracing.newBuilder()
         .reporter(Reporter.NOOP)
-        .build();
+        .build().tracer();
     recorder = tracer.recorder;
     clock = tracer.clock;
     brave = new Brave.Builder()

diff --git a/brave-core/src/test/java/brave/interop/MixedBraveVersionsExample.java b/brave-core/src/test/java/brave/interop/MixedBraveVersionsExample.java
@@ -2,6 +2,7 @@
 
 import brave.Span;
 import brave.Tracer;
+import brave.Tracing;
 import brave.propagation.Propagation;
 import brave.propagation.TraceContextOrSamplingFlags;
 import com.github.kristofa.brave.Brave;
@@ -46,15 +47,15 @@ public class MixedBraveVersionsExample {
   InMemoryStorage storage = new InMemoryStorage();
 
   /** Use different tracers for client and server as usually they are on different hosts. */
-  Tracer brave4Client = Tracer.newBuilder()
+  Tracer brave4Client = Tracing.newBuilder()
       .localEndpoint(Endpoint.builder().serviceName("client").build())
       .reporter(s -> storage.spanConsumer().accept(Collections.singletonList(s)))
-      .build();
+      .build().tracer();
   Brave brave3Client = TracerAdapter.newBrave(brave4Client);
-  Tracer brave4Server = Tracer.newBuilder()
+  Tracer brave4Server = Tracing.newBuilder()
       .localEndpoint(Endpoint.builder().serviceName("server").build())
       .reporter(s -> storage.spanConsumer().accept(Collections.singletonList(s)))
-      .build();
+      .build().tracer();
   Brave brave3Server = TracerAdapter.newBrave(brave4Server);
 
   CountDownLatch flushedIncomingRequest = new CountDownLatch(1);

diff --git a/brave-core/src/test/java/brave/interop/TracerAdapterTest.java b/brave-core/src/test/java/brave/interop/TracerAdapterTest.java
@@ -1,6 +1,7 @@
 package brave.interop; // intentionally in a different package
 
 import brave.Tracer;
+import brave.Tracing;
 import com.github.kristofa.brave.Brave;
 import com.github.kristofa.brave.SpanId;
 import com.github.kristofa.brave.TracerAdapter;
@@ -22,8 +23,11 @@ public class TracerAdapterTest {
 
   List<zipkin.Span> spans = new ArrayList<>();
   AtomicLong epochMicros = new AtomicLong();
-  Tracer brave4 =
-      Tracer.newBuilder().clock(epochMicros::incrementAndGet).reporter(spans::add).build();
+  Tracer brave4 = Tracing.newBuilder()
+      .clock(epochMicros::incrementAndGet)
+      .reporter(spans::add)
+      .build()
+      .tracer();
   Brave brave3 = TracerAdapter.newBrave(brave4);
 
   @Test public void startWithLocalTracerAndFinishWithTracer() {

diff --git a/brave-core/src/test/java/com/github/kristofa/brave/Brave4ClientTracerTest.java b/brave-core/src/test/java/com/github/kristofa/brave/Brave4ClientTracerTest.java
@@ -1,13 +1,13 @@
 package com.github.kristofa.brave;
 
-import brave.Tracer;
+import brave.Tracing;
 
 public class Brave4ClientTracerTest extends ClientTracerTest {
   @Override Brave newBrave() {
-    return TracerAdapter.newBrave(Tracer.newBuilder()
+    return TracerAdapter.newBrave(Tracing.newBuilder()
         .clock(new AnnotationSubmitter.DefaultClock()::currentTimeMicroseconds)
         .localEndpoint(ZIPKIN_ENDPOINT)
         .clock(clock::currentTimeMicroseconds)
-        .reporter(spans::add).build());
+        .reporter(spans::add).build().tracer());
   }
 }
diff --git a/brave-core/src/test/java/com/github/kristofa/brave/Brave4LocalTracerTest.java b/brave-core/src/test/java/com/github/kristofa/brave/Brave4LocalTracerTest.java
@@ -1,19 +1,19 @@
 package com.github.kristofa.brave;
 
-import brave.Tracer;
+import brave.Tracing;
 
 public class Brave4LocalTracerTest extends LocalTracerTest {
   @Override Brave newBrave() {
-    return TracerAdapter.newBrave(Tracer.newBuilder()
+    return TracerAdapter.newBrave(Tracing.newBuilder()
         .clock(clock::currentTimeMicroseconds)
         .localEndpoint(ZIPKIN_ENDPOINT)
-        .reporter(spans::add).build());
+        .reporter(spans::add).build().tracer());
   }
 
   @Override Brave newBrave(ServerClientAndLocalSpanState state) {
-    return TracerAdapter.newBrave(Tracer.newBuilder()
+    return TracerAdapter.newBrave(Tracing.newBuilder()
         .clock(clock::currentTimeMicroseconds)
         .localEndpoint(ZIPKIN_ENDPOINT)
-        .reporter(spans::add).build(), state);
+        .reporter(spans::add).build().tracer(), state);
   }
 }
diff --git a/brave-core/src/test/java/com/github/kristofa/brave/Brave4ServerRequestInterceptorTest.java b/brave-core/src/test/java/com/github/kristofa/brave/Brave4ServerRequestInterceptorTest.java
@@ -1,11 +1,11 @@
 package com.github.kristofa.brave;
 
-import brave.Tracer;
+import brave.Tracing;
 
 public class Brave4ServerRequestInterceptorTest extends ServerRequestInterceptorTest {
   @Override Brave newBrave() {
-    return TracerAdapter.newBrave(Tracer.newBuilder()
+    return TracerAdapter.newBrave(Tracing.newBuilder()
         .localEndpoint(ZIPKIN_ENDPOINT)
-        .reporter(spans::add).build());
+        .reporter(spans::add).build().tracer());
   }
 }
diff --git a/brave-core/src/test/java/com/github/kristofa/brave/Brave4ServerTracerTest.java b/brave-core/src/test/java/com/github/kristofa/brave/Brave4ServerTracerTest.java
@@ -1,12 +1,12 @@
 package com.github.kristofa.brave;
 
-import brave.Tracer;
+import brave.Tracing;
 
 public class Brave4ServerTracerTest extends ServerTracerTest {
   @Override Brave newBrave() {
-    return TracerAdapter.newBrave(Tracer.newBuilder()
+    return TracerAdapter.newBrave(Tracing.newBuilder()
         .clock(clock::currentTimeMicroseconds)
         .localEndpoint(ZIPKIN_ENDPOINT)
-        .reporter(spans::add).build());
+        .reporter(spans::add).build().tracer());
   }
 }
diff --git a/brave-core/src/test/java/com/github/kristofa/brave/Brave4Test.java b/brave-core/src/test/java/com/github/kristofa/brave/Brave4Test.java
@@ -1,22 +1,22 @@
 package com.github.kristofa.brave;
 
-import brave.Tracer;
+import brave.Tracing;
 
 public class Brave4Test extends BraveTest {
 
   @Override protected Brave newBrave() {
-    return TracerAdapter.newBrave(Tracer.newBuilder().build());
+    return TracerAdapter.newBrave(Tracing.newBuilder().build().tracer());
   }
 
   @Override protected Brave newBrave(Sampler sampler) {
-    return TracerAdapter.newBrave(Tracer.newBuilder().sampler(new brave.sampler.Sampler() {
+    return TracerAdapter.newBrave(Tracing.newBuilder().sampler(new brave.sampler.Sampler() {
       @Override public boolean isSampled(long traceId) {
         return sampler.isSampled(traceId);
       }
-    }).build());
+    }).build().tracer());
   }
 
   @Override protected Brave newBraveWith128BitTraceIds() {
-    return TracerAdapter.newBrave(Tracer.newBuilder().traceId128Bit(true).build());
+    return TracerAdapter.newBrave(Tracing.newBuilder().traceId128Bit(true).build().tracer());
   }
 }
diff --git a/...ore/src/test/java/com/github/kristofa/brave/internal/Brave4MaybeAddClientAddressTest.java b/...ore/src/test/java/com/github/kristofa/brave/internal/Brave4MaybeAddClientAddressTest.java
@@ -1,10 +1,10 @@
 package com.github.kristofa.brave.internal;
 
-import brave.Tracer;
+import brave.Tracing;
 import com.github.kristofa.brave.TracerAdapter;
 
 public class Brave4MaybeAddClientAddressTest extends MaybeAddClientAddressTest {
   public Brave4MaybeAddClientAddressTest() {
-    brave = TracerAdapter.newBrave(Tracer.newBuilder().reporter(spans::add).build());
+    brave = TracerAdapter.newBrave(Tracing.newBuilder().reporter(spans::add).build().tracer());
   }
 }
diff --git a/brave/README.md b/brave/README.md
@@ -23,15 +23,18 @@ http (as opposed to Kafka).
 sender = OkHttpSender.create("http://127.0.0.1:9411/api/v1/spans");
 reporter = AsyncReporter.builder(sender).build();
 
-// Now, create a tracer with the service name you want to see in Zipkin.
-tracer = Tracer.newBuilder()
-              .localServiceName("my-service")
-              .reporter(reporter)
-              .build();
+// Create a tracing component with the service name you want to see in Zipkin.
+tracing = Tracing.newBuilder()
+                 .localServiceName("my-service")
+                 .reporter(reporter)
+                 .build();
 
-// When all tracing tasks are complete, close the tracer and reporter
+// Tracing exposes objects you might need, most importantly the tracer
+tracer = tracing.tracer();
+
+// When all tracing tasks are complete, close the tracing component and reporter
 // This might be a shutdown hook for some users
-tracer.close();
+tracing.close();
 reporter.close();
 ```
 
@@ -235,14 +238,16 @@ span = contextOrFlags.context() != null
     : tracer.newTrace(contextOrFlags.samplingFlags());
 ```
 
-## Current Tracer
-Brave supports a "current tracer" concept which should only be used when
-you have no other means to get a reference to a tracer. This was made
-for JDBC connections, as they often initialize prior to the tracer.
+## Current Tracing Component
+Brave supports a "current tracing component" concept which should only
+be used when you have no other means to get a reference. This was made
+for JDBC connections, as they often initialize prior to the tracing
+component.
 
-The most recent tracer instantiated is available via `Tracer.current()`.
-If you use `Tracer.current()` do not cache the result. Instead, lookup
-`Tracer.current()` prior to creating a new span.
+The most recent tracing component instantiated is available via
+`Tracing.current()`. You there's also a shortcut to get only the tracer
+via `Tracing.currentTracer()`. If you use either of these methods, do
+noot cache the result. Instead, look them up each time you need them.
 
 ## Current Span
 
@@ -260,10 +265,10 @@ can use to decorate executors.
 
 ```java
 CurrentTraceContext currentTraceContext = new CurrentTraceContext.Default();
-tracer = Tracer.newBuilder()
-               .currentTraceContext(currentTraceContext)
-               ...
-               .build();
+tracing = Tracing.newBuilder()
+                 .currentTraceContext(currentTraceContext)
+                 ...
+                 .build();
 
 Client c = Client.create();
 c.setExecutorService(currentTraceContext.executorService(realExecutorService));
@@ -380,8 +385,8 @@ If your code uses Brave 3 apis, all you need to do is use `TracerAdapter`
 to create a (Brave 3) .. Brave. You don't have to change anything else.
 
 ```java
-Tracer brave4 = Tracer.newBuilder()...build();
-Brave brave3 = TracerAdapter.newBrave(brave4);
+Tracing brave4 = Tracing.newBuilder()...build();
+Brave brave3 = TracerAdapter.newBrave(brave4.tracer());
 ```
 
 ### Converting between types