Merge branch 'master' into Add_BaggageRestrictionManager

CHANGELOG.rst

@@ -1,6 +1,11 @@
 Changes by Version
 ==================
 
+0.21.0
+------
+- Use okhttp for HTTP sender (https://github.com/uber/jaeger-client-java/pull/224)
+
+
 0.20.0 (2017-06-23)
 -------------------
 - Upgrade to OpenTracing Java 0.30.0 with in-process propagation support (https://github.com/uber/jaeger-client-java/pull/188)

README.md

@@ -136,7 +136,7 @@ This allows using Jaeger UI to find the trace by this tag.
 
 ## Developing
 
- 1. `git submodule init update`
+ 1. `git submodule update --init`
  2. `./gradlew clean check`
 
 ### Code Style

build.gradle

@@ -5,7 +5,7 @@ plugins {
     id "jacoco"
     id "com.github.hierynomus.license" version "0.14.0"
     id 'com.github.sherter.google-java-format' version '0.3.2'
-    id "com.github.johnrengelman.shadow" version "1.2.3"
+    id "com.github.johnrengelman.shadow" version "2.0.1"
     id "net.ltgt.errorprone" version "0.0.10"
     id 'ru.vyarus.animalsniffer' version '1.3.0'
 }
@@ -133,7 +133,7 @@ task codeCoverageReport(type: JacocoReport, group: 'Coverage reports') {
     reports {
         html.enabled = true
         xml.enabled = true
-        xml.destination = "${buildDir}/reports/jacoco/report.xml"
+        xml.destination = file("${buildDir}/reports/jacoco/report.xml")
     }
 
     afterEvaluate {
@@ -170,4 +170,4 @@ def getVersionForBuild() {
     }
 
     return ext.developmentVersion
-}
+}

jaeger-b3/build.gradle

@@ -1,7 +1,7 @@
 description = 'Integration library for B3 Propagation'
 
 dependencies {
-    compile project(':jaeger-core')
+    compile project(path: ':jaeger-core', configuration: 'shadow')
 
     testCompile group: 'junit', name: 'junit', version: junitVersion
     testCompile group: 'io.zipkin.brave', name: 'brave-http', version: '4.1.1'

jaeger-context/src/main/java/com/uber/jaeger/context/ThreadLocalTraceContext.java

@@ -27,13 +27,7 @@
 import java.util.Stack;
 
 public class ThreadLocalTraceContext implements TraceContext {
-  private final ThreadLocal<Stack<Span>> threadLocal =
-      new ThreadLocal<Stack<Span>>() {
-        @Override
-        public Stack<Span> initialValue() {
-          return new Stack<Span>();
-        }
-      };
+  private final ThreadLocal<Stack<Span>> threadLocal = new SpanStackThreadLocal();
 
   @Override
   public void push(Span span) {
@@ -59,4 +53,12 @@ public Span pop() throws EmptyStackException {
   public Span getCurrentSpan() throws EmptyStackException {
     return threadLocal.get().peek();
   }
+
+  private static class SpanStackThreadLocal extends ThreadLocal<Stack<Span>> {
+
+    @Override
+    public Stack<Span> initialValue() {
+      return new Stack<Span>();
+    }
+  }
 }

jaeger-core/build.gradle

@@ -6,6 +6,7 @@ dependencies {
     compile group: 'io.opentracing', name: 'opentracing-util', version: opentracingVersion
     compile group: 'com.google.code.gson', name: 'gson', version: gsonVersion
     compile group: 'org.slf4j', name: 'slf4j-api', version: slf4jVersion
+    compile group: 'com.squareup.okhttp3', name: 'okhttp', version: '3.8.1'
 
     // Testing frameworks
     // Jersey dependencies for unit tests
@@ -19,6 +20,18 @@ dependencies {
     signature 'org.codehaus.mojo.signature:java16:1.1@signature'
 }
 
+apply plugin: 'com.github.johnrengelman.shadow'
+shadowJar {
+    relocate 'okhttp', 'jaeger.okhttp'
+    classifier 'okhttp381'
+}
+
+artifacts {
+    archives(shadowJar.archivePath) {
+        builtBy shadowJar
+    }
+}
+
 task jaegerVersion {
     doLast {
         def dirPath = 'src/main/resources/com/uber/jaeger/'

jaeger-core/src/main/java/com/uber/jaeger/senders/HttpSender.java

@@ -24,61 +24,92 @@
 
 import com.uber.jaeger.thriftjava.Batch;
 import com.uber.jaeger.thriftjava.Process;
+import com.uber.jaeger.thriftjava.Span;
 import java.io.IOException;
-import java.net.URI;
-import java.net.URISyntaxException;
 import java.util.List;
-import org.apache.http.HttpResponse;
-import org.apache.http.HttpStatus;
-import org.apache.http.client.HttpClient;
-import org.apache.http.client.methods.HttpPost;
-import org.apache.http.entity.ByteArrayEntity;
-import org.apache.http.impl.client.DefaultHttpClient;
+import okhttp3.HttpUrl;
+import okhttp3.MediaType;
+import okhttp3.OkHttpClient;
+import okhttp3.Request;
+import okhttp3.RequestBody;
+import okhttp3.Response;
 import org.apache.thrift.TException;
 import org.apache.thrift.TSerializer;
-import org.apache.thrift.protocol.TBinaryProtocol.Factory;
+import org.apache.thrift.protocol.TBinaryProtocol;
 import org.apache.thrift.protocol.TProtocolFactory;
 
 public class HttpSender extends ThriftSender {
+
   private static final String HTTP_COLLECTOR_JAEGER_THRIFT_FORMAT_PARAM = "format=jaeger.thrift";
-  private static final TProtocolFactory PROTOCOL_FACTORY = new Factory();
+  private static final TProtocolFactory PROTOCOL_FACTORY = new TBinaryProtocol.Factory();
   private static final TSerializer SERIALIZER = new TSerializer(PROTOCOL_FACTORY);
+  private static final int ONE_MB_IN_BYTES = 1048576;
+  private static final MediaType MEDIA_TYPE_THRIFT = MediaType.parse("application/x-thrift");
+  private final OkHttpClient httpClient;
+  private final Request.Builder requestBuilder;
 
-  private final HttpClient httpClient = new DefaultHttpClient();
-  private final URI collectorUri;
+  /**
+   * @param endpoint Jaeger REST endpoint consuming jaeger.thrift, e.g
+   * http://localhost:14268/api/traces
+   *
+   * Uses the default {@link okhttp3.OkHttpClient} which uses {@link okhttp3.ConnectionPool#ConnectionPool()}.
+   * Use {@link HttpSender#HttpSender(java.lang.String, int, okhttp3.OkHttpClient)} to adjust parameters.
+   */
+  public HttpSender(String endpoint) {
+    this(endpoint, ONE_MB_IN_BYTES);
+  }
+
+  /**
+   * @param endpoint Jaeger REST endpoint consuming jaeger.thrift, e.g
+   * http://localhost:14268/api/traces
+   * @param maxPayloadBytes max bytes to serialize as payload
+   */
+  public HttpSender(String endpoint, int maxPayloadBytes) {
+    this(endpoint, maxPayloadBytes, new OkHttpClient());
+  }
 
   /**
-   * @param endpoint Jaeger REST endpoint consuming jaeger.thrift, e.g http://localhost:14268/api/traces
-   * @param maxPacketSize max packet size
+   * @param endpoint Jaeger REST endpoint consuming jaeger.thrift, e.g
+   * http://localhost:14268/api/traces
+   * @param maxPayloadBytes max bytes to serialize as payload
+   * @param client a client used to make http requests
    */
-  public HttpSender(String endpoint, int maxPacketSize) {
-    super(PROTOCOL_FACTORY, maxPacketSize);
-    this.collectorUri = constructCollectorUri(endpoint);
+  private HttpSender(String endpoint, int maxPayloadBytes, OkHttpClient client) {
+    super(PROTOCOL_FACTORY, maxPayloadBytes);
+    HttpUrl collectorUrl = HttpUrl
+        .parse(String.format("%s?%s", endpoint, HTTP_COLLECTOR_JAEGER_THRIFT_FORMAT_PARAM));
+    if (collectorUrl == null) {
+      throw new IllegalArgumentException("Could not parse url.");
+    }
+    this.httpClient = client;
+    this.requestBuilder = new Request.Builder().url(collectorUrl);
   }
 
   @Override
-  public void send(Process process, List<com.uber.jaeger.thriftjava.Span> spans) throws TException {
+  public void send(Process process, List<Span> spans) throws TException {
     Batch batch = new Batch(process, spans);
     byte[] bytes = SERIALIZER.serialize(batch);
 
+    RequestBody body = RequestBody.create(MEDIA_TYPE_THRIFT, bytes);
+    Request request = requestBuilder.post(body).build();
+    Response response;
     try {
-      HttpPost httpPost = new HttpPost(this.collectorUri);
-      httpPost.setEntity(new ByteArrayEntity(bytes));
-      HttpResponse response = httpClient.execute(httpPost);
-      if (response.getStatusLine().getStatusCode() != HttpStatus.SC_OK) {
-        throw new TException("Could not send " + spans.size() + " spans, response "
-            + response.getStatusLine().getStatusCode());
-      }
+      response = httpClient.newCall(request).execute();
     } catch (IOException e) {
-      throw new TException("Could not send " + spans.size() + ", spans", e);
+      throw new TException(String.format("Could not send %d spans", spans.size()), e);
     }
-  }
 
-  private URI constructCollectorUri(String endpoint) {
-    try {
-      return new URI(String.format("%s?%s", endpoint, HTTP_COLLECTOR_JAEGER_THRIFT_FORMAT_PARAM));
-    } catch (URISyntaxException e) {
-      throw new IllegalArgumentException("Wrong collector host", e);
+    if (!response.isSuccessful()) {
+      String responseBody;
+      try {
+        responseBody = response.body() != null ? response.body().string() : "null";
+      } catch (IOException e) {
+        responseBody = "unable to read response";
+      }
+
+      String exceptionMessage = String.format("Could not send %d spans, response %d: %s",
+          spans.size(), response.code(), responseBody);
+      throw new TException(exceptionMessage);
     }
   }
 }

jaeger-core/src/test/java/com/uber/jaeger/senders/HttpSenderTest.java

@@ -0,0 +1,97 @@
+/*
+ * Copyright (c) 2017, Uber Technologies, Inc
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+package com.uber.jaeger.senders;
+
+import com.uber.jaeger.thriftjava.Process;
+import com.uber.jaeger.thriftjava.Span;
+import java.util.ArrayList;
+import java.util.List;
+import javax.ws.rs.POST;
+import javax.ws.rs.Path;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.core.Application;
+import javax.ws.rs.core.Response;
+import org.apache.thrift.TException;
+import org.glassfish.jersey.server.ResourceConfig;
+import org.glassfish.jersey.test.JerseyTest;
+import org.junit.Test;
+
+/**
+ * This class tests that HttpSender can be configured to make requests
+ * and that exceptions are handled correctly.
+ * See  {@link com.uber.jaeger.crossdock.JerseyServer} for integration tests.
+ */
+public class HttpSenderTest extends JerseyTest {
+
+  @Override
+  protected Application configure() {
+    return new ResourceConfig(TraceAccepter.class);
+  }
+
+  @Test
+  public void sendHappy() throws Exception {
+    HttpSender sender = new HttpSender(target("/api/traces").getUri().toString());
+    sender.send(new Process("robotrock"), generateSpans());
+  }
+
+  @Test(expected = TException.class)
+  public void sendServerError() throws Exception {
+    HttpSender sender = new HttpSender(target("/api/tracesErr").getUri().toString());
+    sender.send(new Process("robotrock"), generateSpans());
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void misconfiguredUrl() throws Exception {
+    new HttpSender("misconfiguredUrl");
+  }
+
+  @Test(expected = TException.class)
+  public void serverDoesntExist() throws Exception {
+    HttpSender sender = new HttpSender("http://some-server/api/traces");
+    sender.send(new Process("robotrock"), generateSpans());
+  }
+
+  private List<Span> generateSpans() {
+    ArrayList<Span> spans = new ArrayList<>();
+    Span span = new Span();
+    span.setOperationName("boomerang");
+    spans.add(span);
+    return spans;
+  }
+
+  @Path("api")
+  public static class TraceAccepter {
+
+    @Path("traces")
+    @POST()
+    public void postHappy(@QueryParam("format") String format, String data) {
+    }
+
+    @Path("tracesErr")
+    @POST()
+    public Response postErr(@QueryParam("format") String format, String data) {
+      return Response.serverError().build();
+    }
+  }
+
+}

jaeger-crossdock/src/main/java/com/uber/jaeger/crossdock/JerseyServer.java

@@ -153,7 +153,7 @@ public static void main(String[] args) throws Exception {
   private static Sender senderFromEnv(String jaegerHost) {
     String senderEnvVar = System.getenv(Constants.ENV_PROP_SENDER_TYPE);
     if ("http".equalsIgnoreCase(senderEnvVar)) {
-      return new HttpSender(String.format("http://%s:14268/api/traces", jaegerHost), 0);
+      return new HttpSender(String.format("http://%s:14268/api/traces", jaegerHost));
     } else if ("udp".equalsIgnoreCase(senderEnvVar) || senderEnvVar == null || senderEnvVar.isEmpty()) {
       return new UdpSender(jaegerHost, 0, 0);
     }

jaeger-zipkin/build.gradle

@@ -4,7 +4,7 @@ dependencies {
     compile group: 'io.zipkin.reporter', name: 'zipkin-reporter', version: '0.6.13'
     compile group: 'io.zipkin.reporter', name: 'zipkin-sender-urlconnection', version: '0.6.13'
     compile project(':jaeger-b3')
-    compile project(':jaeger-core')
+    compile project(path: ':jaeger-core', configuration: 'shadow')
 
     testCompile group: 'junit', name: 'junit', version: junitVersion
     testCompile group: 'io.zipkin.java', name: 'zipkin-junit', version: '1.16.2'