Special-cases noop for zipkin 2 reporting (openzipkin#497)

sagarl · Sep 18, 2017 · 2014931 · 2014931
1 parent ecff17a
commit 2014931
Show file tree

Hide file tree

Showing 8 changed files with 49 additions and 57 deletions.
diff --git a/archive/brave-core/src/test/java/brave/interop/MixedBraveVersionsExample.java b/archive/brave-core/src/test/java/brave/interop/MixedBraveVersionsExample.java
@@ -19,13 +19,12 @@
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
-import zipkin.Endpoint;
-import zipkin.storage.InMemoryStorage;
+import zipkin2.Endpoint;
+import zipkin2.storage.InMemoryStorage;
 
+import static brave.internal.HexCodec.toLowerHex;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.mock;
-import static zipkin.Constants.CLIENT_SEND;
-import static zipkin.Constants.SERVER_RECV;
 
 /**
  * This is an example of interop between Brave 3 and Brave 4.
@@ -43,17 +42,17 @@
 public class MixedBraveVersionsExample {
   @Rule public MockWebServer server = new MockWebServer();
 
-  InMemoryStorage storage = new InMemoryStorage();
+  InMemoryStorage storage = InMemoryStorage.newBuilder().build();
 
   /** Use different tracers for client and server as usually they are on different hosts. */
   Tracing brave4Client = Tracing.newBuilder()
-      .localEndpoint(Endpoint.builder().serviceName("client").build())
-      .reporter(s -> storage.spanConsumer().accept(Collections.singletonList(s)))
+      .localEndpoint(Endpoint.newBuilder().serviceName("client").build())
+      .spanReporter(s -> storage.spanConsumer().accept(Collections.singletonList(s)))
       .build();
   Brave brave3Client = TracerAdapter.newBrave(brave4Client.tracer());
   Tracing brave4Server = Tracing.newBuilder()
-      .localEndpoint(Endpoint.builder().serviceName("server").build())
-      .reporter(s -> storage.spanConsumer().accept(Collections.singletonList(s)))
+      .localEndpoint(Endpoint.newBuilder().serviceName("server").build())
+      .spanReporter(s -> storage.spanConsumer().accept(Collections.singletonList(s)))
       .build();
   Brave brave3Server = TracerAdapter.newBrave(brave4Server.tracer());
 
@@ -95,10 +94,11 @@ public void createTraceWithBrave3AndBrave4() throws Exception {
     // * root server span created with Brave 3
     // * one-way child span created with Brave 4
     // * local grandchild span created with Brave 3
-    List<zipkin.Span> trace = storage.spanStore().getTrace(parent.context().traceId());
-    assertThat(trace).hasSize(3);
-    assertThat(trace.get(0).id).isEqualTo(trace.get(1).parentId);
-    assertThat(trace.get(1).id).isEqualTo(trace.get(2).parentId);
+    List<zipkin2.Span>
+        trace = storage.spanStore().getTrace(toLowerHex(parent.context().traceId())).execute();
+    assertThat(trace).hasSize(4);
+    assertThat(trace.get(2).id()).isEqualTo(trace.get(0).parentId());
+    assertThat(trace.get(0).id()).isEqualTo(trace.get(3).parentId());
   }
 
   /**
@@ -124,7 +124,7 @@ void createAndPropagateOneWaySpan(Span parent) {
 
     // fire off the request asynchronously, totally dropping any response
     new OkHttpClient().newCall(request.build()).enqueue(mock(Callback.class));
-    span.annotate(CLIENT_SEND).flush(); // record the timestamp of the client send and flush
+    span.kind(Span.Kind.CLIENT).flush(); // record the timestamp of the client send and flush
   }
 
   /**
@@ -138,7 +138,7 @@ Span joinOneWaySpan(RecordedRequest recordedRequest) {
     // in real life, we'd guard result.context was set and start a new trace if not
     Span serverSpan = brave4Server.tracer().joinSpan(result.context())
         .name(recordedRequest.getMethod())
-        .annotate(SERVER_RECV);
+        .kind(Span.Kind.SERVER);
     serverSpan.flush(); // record the timestamp of the server receive and flush
     return serverSpan;
   }

diff --git a/archive/brave-core/src/test/java/brave/interop/TracerAdapterTest.java b/archive/brave-core/src/test/java/brave/interop/TracerAdapterTest.java
@@ -6,28 +6,26 @@
 import com.github.kristofa.brave.SpanId;
 import com.github.kristofa.brave.TracerAdapter;
 import com.twitter.zipkin.gen.Span;
-import java.nio.charset.Charset;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.atomic.AtomicLong;
 import org.junit.After;
 import org.junit.Test;
 import zipkin.Constants;
+import zipkin2.Annotation;
 
 import static com.github.kristofa.brave.TracerAdapter.getServerSpan;
 import static com.github.kristofa.brave.TracerAdapter.setServerSpan;
 import static com.github.kristofa.brave.TracerAdapter.toSpan;
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.tuple;
+import static org.assertj.core.api.Assertions.entry;
 
 public class TracerAdapterTest {
-  static final Charset UTF_8 = Charset.forName("UTF-8");
-
-  List<zipkin.Span> spans = new ArrayList<>();
+  List<zipkin2.Span> spans = new ArrayList<>();
   AtomicLong epochMicros = new AtomicLong();
   Tracer brave4 = Tracing.newBuilder()
       .clock(epochMicros::incrementAndGet)
-      .reporter(spans::add)
+      .spanReporter(spans::add)
       .build()
       .tracer();
   Brave brave3 = TracerAdapter.newBrave(brave4);
@@ -136,39 +134,33 @@ public class TracerAdapterTest {
 
   void checkLocalSpanReportedToZipkin() {
     assertThat(spans).first().satisfies(s -> {
-          assertThat(s.name).isEqualTo("encode");
-          assertThat(s.timestamp).isEqualTo(1L);
-          assertThat(s.annotations).extracting(a -> a.timestamp, a -> a.value)
-              .containsExactly(tuple(2L, "pump fake"));
-          assertThat(s.binaryAnnotations).extracting(b -> b.key, b -> new String(b.value, UTF_8))
-              .containsExactly(tuple(Constants.LOCAL_COMPONENT, "codec"));
-          assertThat(s.duration).isEqualTo(2L);
+          assertThat(s.name()).isEqualTo("encode");
+          assertThat(s.timestamp()).isEqualTo(1L);
+          assertThat(s.annotations())
+              .containsExactly(Annotation.create(2L, "pump fake"));
+          assertThat(s.tags())
+              .containsExactly(entry(Constants.LOCAL_COMPONENT, "codec"));
+          assertThat(s.duration()).isEqualTo(2L);
         }
     );
   }
 
   void checkClientSpanReportedToZipkin() {
     assertThat(spans).first().satisfies(s -> {
-          assertThat(s.name).isEqualTo("get");
-          assertThat(s.timestamp).isEqualTo(1L);
-          assertThat(s.duration).isEqualTo(1L);
-          assertThat(s.annotations).extracting(a -> a.timestamp, a -> a.value).containsExactly(
-              tuple(1L, "cs"),
-              tuple(2L, "cr")
-          );
+          assertThat(s.name()).isEqualTo("get");
+          assertThat(s.timestamp()).isEqualTo(1L);
+          assertThat(s.duration()).isEqualTo(1L);
+          assertThat(s.kind()).isEqualTo(zipkin2.Span.Kind.CLIENT);
         }
     );
   }
 
   void checkServerSpanReportedToZipkin() {
     assertThat(spans).first().satisfies(s -> {
-          assertThat(s.name).isEqualTo("get");
-          assertThat(s.timestamp).isEqualTo(1L);
-          assertThat(s.duration).isEqualTo(1L);
-          assertThat(s.annotations).extracting(a -> a.timestamp, a -> a.value).containsExactly(
-              tuple(1L, "sr"),
-              tuple(2L, "ss")
-          );
+          assertThat(s.name()).isEqualTo("get");
+          assertThat(s.timestamp()).isEqualTo(1L);
+          assertThat(s.duration()).isEqualTo(1L);
+          assertThat(s.kind()).isEqualTo(zipkin2.Span.Kind.SERVER);
         }
     );
   }

diff --git a/brave/src/main/java/brave/Tracing.java b/brave/src/main/java/brave/Tracing.java
@@ -171,6 +171,11 @@ public Builder spanReporter(Reporter<zipkin2.Span> reporter) {
     @Deprecated
     public Builder reporter(final Reporter<zipkin.Span> reporter) {
       if (reporter == null) throw new NullPointerException("reporter == null");
+      if (reporter == Reporter.NOOP) {
+        this.reporter = s -> {
+        };
+        return this;
+      }
       this.reporter = new Reporter<zipkin2.Span>() {
         @Override public void report(zipkin2.Span span) {
           reporter.report(V2SpanConverter.toSpan(span));

diff --git a/instrumentation/benchmarks/src/main/java/brave/http/HttpClientBenchmarks.java b/instrumentation/benchmarks/src/main/java/brave/http/HttpClientBenchmarks.java
@@ -18,7 +18,6 @@
 import org.openjdk.jmh.annotations.TearDown;
 import org.openjdk.jmh.annotations.Threads;
 import org.openjdk.jmh.annotations.Warmup;
-import zipkin.reporter.Reporter;
 
 import static io.undertow.util.Headers.CONTENT_TYPE;
 
@@ -61,10 +60,10 @@ protected String baseUrl() {
 
     client = newClient();
     tracedClient = newClient(HttpTracing.create(
-        Tracing.newBuilder().reporter(Reporter.NOOP).build()
+        Tracing.newBuilder().spanReporter(s -> {}).build()
     ));
     unsampledClient = newClient(HttpTracing.create(
-        Tracing.newBuilder().sampler(Sampler.NEVER_SAMPLE).reporter(Reporter.NOOP).build()
+        Tracing.newBuilder().sampler(Sampler.NEVER_SAMPLE).spanReporter(s -> {}).build()
     ));
   }
 

diff --git a/instrumentation/benchmarks/src/main/java/brave/jaxrs2/JaxRs2ServerBenchmarks.java b/instrumentation/benchmarks/src/main/java/brave/jaxrs2/JaxRs2ServerBenchmarks.java
@@ -23,7 +23,6 @@
 import org.openjdk.jmh.runner.RunnerException;
 import org.openjdk.jmh.runner.options.Options;
 import org.openjdk.jmh.runner.options.OptionsBuilder;
-import zipkin.reporter.Reporter;
 
 public class JaxRs2ServerBenchmarks extends HttpServerBenchmarks {
 
@@ -45,7 +44,7 @@ public static class App extends Application {
   public static class Unsampled extends Application {
     @Override public Set<Object> getSingletons() {
       return new LinkedHashSet<>(Arrays.asList(new Resource(), TracingFeature.create(
-          Tracing.newBuilder().sampler(Sampler.NEVER_SAMPLE).reporter(Reporter.NOOP).build()
+          Tracing.newBuilder().sampler(Sampler.NEVER_SAMPLE).spanReporter(s -> {}).build()
       )));
     }
   }
@@ -54,7 +53,7 @@ public static class Unsampled extends Application {
   public static class TracedApp extends Application {
     @Override public Set<Object> getSingletons() {
       return new LinkedHashSet<>(Arrays.asList(new Resource(), TracingFeature.create(
-          Tracing.newBuilder().reporter(Reporter.NOOP).build()
+          Tracing.newBuilder().spanReporter(s -> {}).build()
       )));
     }
   }

diff --git a/instrumentation/benchmarks/src/main/java/brave/servlet/ServletBenchmarks.java b/instrumentation/benchmarks/src/main/java/brave/servlet/ServletBenchmarks.java
@@ -20,7 +20,6 @@
 import org.openjdk.jmh.runner.RunnerException;
 import org.openjdk.jmh.runner.options.Options;
 import org.openjdk.jmh.runner.options.OptionsBuilder;
-import zipkin.reporter.Reporter;
 
 import static javax.servlet.DispatcherType.REQUEST;
 
@@ -37,14 +36,14 @@ static class HelloServlet extends HttpServlet {
   public static class Unsampled extends ForwardingTracingFilter {
     public Unsampled() {
       super(TracingFilter.create(
-          Tracing.newBuilder().sampler(Sampler.NEVER_SAMPLE).reporter(Reporter.NOOP).build()
+          Tracing.newBuilder().sampler(Sampler.NEVER_SAMPLE).spanReporter(s -> {}).build()
       ));
     }
   }
 
   public static class Traced extends ForwardingTracingFilter {
     public Traced() {
-      super(TracingFilter.create(Tracing.newBuilder().reporter(Reporter.NOOP).build()));
+      super(TracingFilter.create(Tracing.newBuilder().spanReporter(s -> {}).build()));
     }
   }
 

diff --git a/instrumentation/benchmarks/src/main/java/brave/sparkjava/SparkBenchmarks.java b/instrumentation/benchmarks/src/main/java/brave/sparkjava/SparkBenchmarks.java
@@ -14,7 +14,6 @@
 import spark.Spark;
 import spark.servlet.SparkApplication;
 import spark.servlet.SparkFilter;
-import zipkin.reporter.Reporter;
 
 import static javax.servlet.DispatcherType.REQUEST;
 
@@ -29,7 +28,7 @@ public void init() {
 
   public static class Unsampled implements SparkApplication {
     SparkTracing sparkTracing = SparkTracing.create(
-        Tracing.newBuilder().sampler(Sampler.NEVER_SAMPLE).reporter(Reporter.NOOP).build()
+        Tracing.newBuilder().sampler(Sampler.NEVER_SAMPLE).spanReporter(s -> {}).build()
     );
 
     @Override
@@ -42,7 +41,7 @@ public void init() {
 
   public static class Traced implements SparkApplication {
     SparkTracing sparkTracing = SparkTracing.create(
-        Tracing.newBuilder().reporter(Reporter.NOOP).build()
+        Tracing.newBuilder().spanReporter(s -> {}).build()
     );
 
     @Override

diff --git a/instrumentation/benchmarks/src/main/java/brave/spring/webmvc/WebMvcBenchmarks.java b/instrumentation/benchmarks/src/main/java/brave/spring/webmvc/WebMvcBenchmarks.java
@@ -20,7 +20,6 @@
 import org.springframework.web.servlet.config.annotation.EnableWebMvc;
 import org.springframework.web.servlet.config.annotation.InterceptorRegistry;
 import org.springframework.web.servlet.config.annotation.WebMvcConfigurerAdapter;
-import zipkin.reporter.Reporter;
 
 public class WebMvcBenchmarks extends HttpServerBenchmarks {
 
@@ -47,10 +46,10 @@ public ResponseEntity<String> traced() throws IOException {
   static class SpringConfig extends WebMvcConfigurerAdapter {
     @Override public void addInterceptors(InterceptorRegistry registry) {
       registry.addInterceptor(TracingHandlerInterceptor.create(
-          Tracing.newBuilder().sampler(Sampler.NEVER_SAMPLE).reporter(Reporter.NOOP).build()
+          Tracing.newBuilder().sampler(Sampler.NEVER_SAMPLE).spanReporter(s -> {}).build()
       )).addPathPatterns("/unsampled");
       registry.addInterceptor(TracingHandlerInterceptor.create(
-          Tracing.newBuilder().reporter(Reporter.NOOP).build()
+          Tracing.newBuilder().spanReporter(s -> {}).build()
       )).addPathPatterns("/traced");
     }
   }