Merge branch 'master' into br/TACO-466

gradle.properties

@@ -19,8 +19,8 @@ curator_version=4.0.0
 zookeeper_version=3.4.10
 slf4j_log4j_over_slf4j_version=1.7.25
 libthrift_version=0.9.3
-zipkin_brave_brave_context_slf4j_version=4.5.1
-zipkin_reporter_zipkin_sender_okhttp3_version=1.0.0
+zipkin_brave_brave_context_slf4j_version=4.19.3
+zipkin_reporter_zipkin_sender_okhttp3_version=2.7.7
 auto_value_version=1.5.2
 lombok_version=1.16.20
 hamcrest_version=1.3
@@ -30,14 +30,14 @@ metrics_version=4.0.0
 # Test Dependencies
 grpc_version=1.7.0
 eclipse_jdt_core_compiler_ecj_version=4.5.1
-zipkin_brave_brave_instrumentation_http_version=4.4.0
+zipkin_brave_brave_instrumentation_http_version=4.19.3
 logback_version=1.1.7
 groovy_version=2.4.1
 okhttp_version=3.9.1
 eclipse_jetty_server_version=9.3.1.v20150714
 mockito_version=2.18.0
 powermock_version=2.0.0-beta.5
-zipkin_brave_brave_instrumentation_http_tests_version=4.4.0
+zipkin_brave_brave_instrumentation_http_tests_version=4.19.3
 curator_test_version=2.12.0
 junit_version=4.12
 concurrentunit_version=0.4.3

xio-core/build.gradle

@@ -43,7 +43,7 @@ dependencies {
   implementation group: 'org.apache.thrift', name: 'libthrift', version: libthrift_version
   implementation group: 'io.zipkin.brave', name: 'brave-instrumentation-http', version: zipkin_brave_brave_instrumentation_http_version
   implementation group: 'io.zipkin.brave', name: 'brave-context-slf4j', version: zipkin_brave_brave_context_slf4j_version
-  implementation group: 'io.zipkin.reporter', name: 'zipkin-sender-okhttp3', version: zipkin_reporter_zipkin_sender_okhttp3_version
+  implementation group: 'io.zipkin.reporter2', name: 'zipkin-sender-okhttp3', version: zipkin_reporter_zipkin_sender_okhttp3_version
 
   implementation group: 'io.dropwizard.metrics', name: 'metrics-core', version: metrics_version
   implementation group: 'io.dropwizard.metrics', name: 'metrics-jmx', version: metrics_version

xio-core/src/main/java/com/xjeffrose/xio/tracing/XioHttpClientAdapter.java

@@ -5,7 +5,7 @@
 import com.xjeffrose.xio.http.Response;
 import io.netty.handler.codec.http.HttpHeaderNames;
 import javax.annotation.ParametersAreNonnullByDefault;
-import zipkin.Endpoint;
+import zipkin2.Endpoint;
 
 @ParametersAreNonnullByDefault
 class XioHttpClientAdapter extends HttpClientAdapter<Request, Response> {

xio-core/src/main/java/com/xjeffrose/xio/tracing/XioHttpServerAdapter.java

@@ -5,7 +5,7 @@
 import com.xjeffrose.xio.http.Response;
 import io.netty.handler.codec.http.HttpHeaderNames;
 import javax.annotation.ParametersAreNonnullByDefault;
-import zipkin.Endpoint;
+import zipkin2.Endpoint;
 
 @ParametersAreNonnullByDefault
 class XioHttpServerAdapter extends HttpServerAdapter<Request, Response> {
@@ -52,6 +52,6 @@ public boolean parseClientAddress(Request request, Endpoint.Builder builder) {
       return true;
     }
     String remoteIp = request.headers().get("x-remote-ip");
-    return remoteIp != null && builder.parseIp(remoteIp);
+    return builder.parseIp(remoteIp);
   }
 }

xio-core/src/main/java/com/xjeffrose/xio/tracing/XioTracing.java

@@ -7,10 +7,10 @@
 import com.typesafe.config.Config;
 import lombok.NonNull;
 import lombok.val;
-import zipkin.Span;
-import zipkin.reporter.AsyncReporter;
-import zipkin.reporter.Reporter;
-import zipkin.reporter.okhttp3.OkHttpSender;
+import zipkin2.Span;
+import zipkin2.reporter.AsyncReporter;
+import zipkin2.reporter.Reporter;
+import zipkin2.reporter.okhttp3.OkHttpSender;
 
 public class XioTracing {
 
@@ -28,7 +28,7 @@ Tracing buildTracing(@NonNull String name, @NonNull String zipkinUrl, float samp
         .localServiceName(name)
         // puts trace IDs into logs
         .currentTraceContext(MDCCurrentTraceContext.create())
-        .reporter(buildReporter(zipkinUrl))
+        .spanReporter(buildReporter(zipkinUrl))
         .sampler(Sampler.create(samplingRate))
         .build();
   }

xio-core/src/test/java/com/xjeffrose/xio/pipeline/XioHttp1_1PipelineFunctionalTest.java

@@ -35,7 +35,7 @@ public void testProxyServer() throws IOException {
               + "\r\n"
               + "HEADER: Connection = Keep-Alive\r\n"
               + "HEADER: Accept-Encoding = gzip\r\n"
-              + "HEADER: User-Agent = okhttp/3.9.1\r\n"
+              + "HEADER: User-Agent = okhttp/3.11.0\r\n"
               + "\r\n"
               + "END OF CONTENT\r\n";
 

xio-core/src/test/java/com/xjeffrose/xio/tracing/HttpClientTracingHandlerIntegrationTest.java

@@ -15,7 +15,7 @@
 import io.netty.buffer.ByteBufUtil;
 import io.netty.channel.*;
 import java.util.ArrayList;
-import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.CompletableFuture;
 import java.util.logging.*;
@@ -27,58 +27,29 @@
 import okhttp3.mockwebserver.SocketPolicy;
 import org.apache.commons.codec.binary.Hex;
 import org.junit.*;
-import org.junit.rules.TestWatcher;
-import org.junit.runner.Description;
-import zipkin.Span;
-import zipkin.reporter.Reporter;
-
-// TODO(CK): These brave integration tests are flaky and stall out sometimes
-// Turn them back on when they are fixed
-public class HttpClientTracingHandlerIntegrationTest { // extends ITHttpClient<XioClient> {
-  String expectedResponse = "hello, world";
-  Response response;
-  List<Span> reportedSpans;
-  Logger hush = disableJavaLogging();
-  CompletableFuture<Response> local = new CompletableFuture<Response>();
-  MockWebServer server;
+import zipkin2.Span;
+import zipkin2.reporter.Reporter;
+
+public class HttpClientTracingHandlerIntegrationTest {
+  private String expectedResponse = "hello, world";
+  private Response response;
+  private List<Span> reportedSpans;
+  private CompletableFuture<Response> local = new CompletableFuture<>();
+  private MockWebServer server;
 
   private class FakeTracer extends XioTracing {
-    public FakeTracer(Config config) {
+    FakeTracer(Config config) {
       super(config);
     }
 
     @Override
-    Reporter<zipkin.Span> buildReporter(@NonNull String zipkinUrl) {
-      return new Reporter<Span>() {
-        @Override
-        public void report(Span span) {
-          reportedSpans.add(span);
-        }
-      };
+    Reporter<zipkin2.Span> buildReporter(@NonNull String zipkinUrl) {
+      return span -> reportedSpans.add(span);
     }
   }
 
-  @BeforeClass
-  public static void setupJul() {
-    JulBridge.initialize();
-  }
-
-  @Rule
-  public TestWatcher testWatcher =
-      new TestWatcher() {
-        @Override
-        protected void starting(final Description description) {
-          String methodName = description.getMethodName();
-          String className = description.getClassName();
-          className = className.substring(className.lastIndexOf('.') + 1);
-          // System.out.println("Starting JUnit-test: " + className + " " + methodName);
-        }
-      };
-
-  static Logger disableJavaLogging() {
-    Logger logger = Logger.getLogger("okhttp3.mockwebserver.MockWebServer");
-    logger.setLevel(Level.WARNING);
-    return logger;
+  private static void disableJavaLogging() {
+    Logger.getLogger("okhttp3.mockwebserver.MockWebServer").setLevel(Level.WARNING);
   }
 
   public class ApplicationHandler extends SimpleChannelInboundHandler<Response> {
@@ -99,38 +70,47 @@ public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws E
 
   private Config config() {
     // TODO(CK): this creates global state across tests we should do something smarter
-    System.setProperty("xio.baseClient.remotePort", Integer.toString(server.getPort()));
+    System.setProperty(
+        "xio.baseClient.remotePort",
+        Integer.toString(
+            server
+                .getPort())); // Maybe just pick a port, then run the test by starting the mock web server with that port?
     System.setProperty("xio.proxyRouteTemplate.proxyPath", "/");
     ConfigFactory.invalidateCaches();
     Config root = ConfigFactory.load();
     return root.getConfig("xio.tracingHandlerClientIntegrationTest");
   }
 
-  Client newClient(int port, XioTracing tracing) {
-    val channelConfig = ChannelConfiguration.clientConfig(1, "worker");
+  private Client newClient(XioTracing tracing) {
+    val channelConfig = ChannelConfiguration.clientConfig(1, "client-tracing-test-worker");
     val clientConfig = ClientConfig.from(ConfigFactory.load().getConfig("xio.baseClient"));
     val clientState = new ClientState(channelConfig, clientConfig);
     ClientChannelInitializer clientChannelInit =
-        new ClientChannelInitializer(clientState, () -> new ApplicationHandler(), tracing);
+        new ClientChannelInitializer(clientState, ApplicationHandler::new, tracing);
     ClientConnectionManager connManager =
         new ClientConnectionManager(clientState, clientChannelInit);
-    val client = new Client(clientState, connManager);
 
-    return client;
+    return new Client(clientState, connManager);
   }
 
-  MockResponse buildResponse() {
+  private MockResponse buildResponse() {
     return new MockResponse().setBody(expectedResponse).setSocketPolicy(SocketPolicy.KEEP_OPEN);
   }
 
+  @BeforeClass
+  public static void setupJul() {
+    disableJavaLogging();
+    JulBridge.initialize();
+  }
+
   @Before
   public void setUp() throws Exception {
-    reportedSpans = new ArrayList<Span>();
+    reportedSpans = new ArrayList<>();
 
     TlsConfig tlsConfig =
         TlsConfig.fromConfig("xio.h2BackendServer.settings.tls", ConfigFactory.load());
     server = OkHttpUnsafe.getSslMockWebServer(getKeyManagers(tlsConfig));
-    server.setProtocols(Arrays.asList(Protocol.HTTP_1_1));
+    server.setProtocols(Collections.singletonList(Protocol.HTTP_1_1));
     server.enqueue(buildResponse());
     server.start();
   }
@@ -142,7 +122,7 @@ public void tearDown() throws Exception {
 
   @Test
   public void testOutBoundAndInboundSpan() throws Exception {
-    val client = newClient(server.getPort(), new FakeTracer(config()));
+    val client = newClient(new FakeTracer(config()));
 
     val request =
         DefaultSegmentedRequest.builder()
@@ -153,31 +133,12 @@ public void testOutBoundAndInboundSpan() throws Exception {
 
     client.write(request);
     // We wait on the local future because this signals the full roundtrip between outbound and
-    // return trip from
-    // the Application Handler out and then back in.
+    // return trip from the Application Handler out and then back in.
     local.get();
     assertEquals(reportedSpans.size(), 1);
 
     val responseHex = ByteBufUtil.hexDump(((SegmentedData) response).content());
     byte[] bytes = Hex.decodeHex(responseHex.toCharArray());
     assertEquals(expectedResponse, new String(bytes, "UTF-8"));
   }
-
-  // @Override
-  @Test(expected = ComparisonFailure.class)
-  public void redirect() throws Exception {
-    throw new AssumptionViolatedException("client does not support redirect");
-  }
-
-  // @Override
-  @Test(expected = ComparisonFailure.class)
-  public void addsStatusCodeWhenNotOk() throws Exception {
-    throw new AssumptionViolatedException("test is flaky");
-  }
-
-  // @Override
-  @Test(expected = ComparisonFailure.class)
-  public void httpPathTagExcludesQueryParams() throws Exception {
-    throw new AssumptionViolatedException("test is flaky");
-  }
 }

xio-core/src/test/java/com/xjeffrose/xio/tracing/HttpServerTracingHandlerIntegrationTest.java

@@ -1,8 +1,8 @@
 package com.xjeffrose.xio.tracing;
 
 import brave.Tracing;
-import brave.internal.StrictCurrentTraceContext;
 import brave.propagation.CurrentTraceContext;
+import brave.propagation.StrictCurrentTraceContext;
 import brave.sampler.Sampler;
 import com.typesafe.config.Config;
 import com.xjeffrose.xio.application.Application;
@@ -31,8 +31,8 @@
 
 public class HttpServerTracingHandlerIntegrationTest extends Assert {
 
-  Application application = null;
-  CountDownLatch latch;
+  private Application application = null;
+  private CountDownLatch latch;
 
   @Before
   public void before() throws Exception {
@@ -69,16 +69,20 @@ public void testSpanDispatchedH2() throws Exception {
 
   private class XioTracingDecorator extends XioTracing {
 
-    CurrentTraceContext currentTraceContext = new StrictCurrentTraceContext();
+    private CurrentTraceContext currentTraceContext;
 
-    public XioTracingDecorator(Config config) {
+    XioTracingDecorator(Config config) {
       super(config);
     }
 
     @Override
     protected Tracing buildTracing(String name, String zipkinUrl, float samplingRate) {
+      if (currentTraceContext == null) {
+        currentTraceContext = new StrictCurrentTraceContext();
+      }
+
       return Tracing.newBuilder()
-          .reporter(ignored -> latch.countDown())
+          .spanReporter(ignored -> latch.countDown())
           .currentTraceContext(currentTraceContext)
           .sampler(Sampler.ALWAYS_SAMPLE)
           .build();

xio-core/src/test/java/com/xjeffrose/xio/tracing/HttpServerTracingHandlerTest.java

@@ -8,8 +8,8 @@
 import brave.Tracer;
 import brave.Tracing;
 import brave.http.HttpTracing;
-import brave.internal.StrictCurrentTraceContext;
 import brave.propagation.CurrentTraceContext;
+import brave.propagation.StrictCurrentTraceContext;
 import brave.propagation.TraceContext;
 import brave.sampler.Sampler;
 import com.xjeffrose.xio.http.DefaultFullResponse;
@@ -66,20 +66,20 @@ public void channelRead0(ChannelHandlerContext ctx, Request request) throws Exce
     }
   }
 
-  ConcurrentLinkedDeque<zipkin.Span> spans = new ConcurrentLinkedDeque<>();
+  ConcurrentLinkedDeque<zipkin2.Span> spans = new ConcurrentLinkedDeque<>();
 
   CurrentTraceContext currentTraceContext = new StrictCurrentTraceContext();
   HttpTracing httpTracing;
   HttpServerTracingDispatch httpTracingState;
 
   Tracing.Builder tracingBuilder(Sampler sampler) {
     return Tracing.newBuilder()
-        .reporter(
+        .spanReporter(
             s -> {
               // make sure the context was cleared prior to finish.. no leaks!
               TraceContext current = httpTracing.tracing().currentTraceContext().get();
               if (current != null) {
-                Assert.assertNotEquals(current.spanId(), s.id);
+                Assert.assertNotEquals(current.spanId(), s.id());
               }
               spans.add(s);
             })