Merge pull request #85 from openzipkin/dethrift

Removes internal use of libthrift

interop/pom.xml

@@ -29,7 +29,9 @@
 
   <properties>
     <main.basedir>${project.basedir}/..</main.basedir>
-    <zipkin-scala.version>1.33.2</zipkin-scala.version>
+    <!-- override scrooge's thrift version so we don't need to declare a twitter-specific repo -->
+    <libthrift.version>0.9.3</libthrift.version>
+    <zipkin-scala.version>1.34.0</zipkin-scala.version>
     <scalatest.version>2.2.5</scalatest.version>
   </properties>
 
@@ -51,6 +53,18 @@
       <version>${zipkin-scala.version}</version>
     </dependency>
 
+    <dependency>
+      <groupId>io.zipkin</groupId>
+      <artifactId>zipkin-scrooge</artifactId>
+      <version>${zipkin-scala.version}</version>
+    </dependency>
+
+    <dependency>
+      <groupId>org.apache.thrift</groupId>
+      <artifactId>libthrift</artifactId>
+      <version>${libthrift.version}</version>
+    </dependency>
+
     <dependency>
       <groupId>${project.groupId}</groupId>
       <artifactId>spanstore-jdbc</artifactId>

interop/src/main/java/zipkin/interop/ScalaSpanStoreAdapter.java

@@ -13,17 +13,17 @@
  */
 package zipkin.interop;
 
-import com.fasterxml.jackson.core.JsonProcessingException;
-import com.fasterxml.jackson.core.type.TypeReference;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.twitter.util.Future;
 import com.twitter.zipkin.common.Span;
-import com.twitter.zipkin.json.JsonSpan;
+import com.twitter.zipkin.conversions.thrift$;
 import com.twitter.zipkin.json.ZipkinJson$;
 import com.twitter.zipkin.storage.QueryRequest;
-import java.io.IOException;
 import java.util.ArrayList;
-import java.util.stream.Collectors;
+import org.apache.thrift.protocol.TBinaryProtocol;
+import org.apache.thrift.protocol.TList;
+import org.apache.thrift.protocol.TType;
+import org.apache.thrift.transport.TMemoryBuffer;
 import scala.Tuple2;
 import scala.collection.Iterator;
 import scala.collection.JavaConversions;
@@ -40,11 +40,9 @@
  * Adapts {@link SpanStore} to a scala {@link com.twitter.zipkin.storage.SpanStore} in order to test
  * against its {@link com.twitter.zipkin.storage.SpanStoreSpec} for interoperability reasons.
  *
- * <p/> This implementation uses json to ensure structures are compatible.
+ * <p/> This implementation uses thrift TBinaryProtocol to ensure structures are compatible.
  */
 public final class ScalaSpanStoreAdapter extends com.twitter.zipkin.storage.SpanStore {
-  private static final ObjectMapper scalaCodec = ZipkinJson$.MODULE$;
-
   private final SpanStore spanStore;
 
   public ScalaSpanStoreAdapter(SpanStore spanStore) {
@@ -111,12 +109,11 @@ public void close() {
 
   @Nullable
   private static java.util.List<Span> convert(java.util.List<zipkin.Span> input) {
-    byte[] bytes = Codec.JSON.writeSpans(input);
+    byte[] bytes = Codec.THRIFT.writeSpans(input);
     try {
-      TypeReference<java.util.List<JsonSpan>> ref = new TypeReference<java.util.List<JsonSpan>>(){};
-      java.util.List<JsonSpan> read = scalaCodec.readValue(bytes, ref);
-      return read.stream().map(JsonSpan::invert).collect(Collectors.toList());
-    } catch (IOException e) {
+      List<Span> read = thrift$.MODULE$.thriftListToSpans(bytes);
+      return JavaConversions.seqAsJavaList(read);
+    } catch (RuntimeException e) {
       e.printStackTrace();
       return null;
     }
@@ -125,9 +122,19 @@ private static java.util.List<Span> convert(java.util.List<zipkin.Span> input) {
   @Nullable
   private static java.util.List<zipkin.Span> invert(Seq<Span> input) {
     try {
-      byte[] bytes = scalaCodec.writeValueAsBytes(input);
-      return Codec.JSON.readSpans(bytes);
-    } catch (JsonProcessingException e) {
+      TMemoryBuffer transport = new TMemoryBuffer(0);
+      TBinaryProtocol oproto = new TBinaryProtocol(transport);
+      oproto.writeListBegin(new TList(TType.STRUCT, input.size()));
+      Iterator<Span> iterator = input.iterator();
+      while (iterator.hasNext()) {
+        com.twitter.zipkin.thriftscala.Span thriftSpan =
+            thrift$.MODULE$.spanToThriftSpan(iterator.next()).toThrift();
+        thriftSpan.write(oproto);
+      }
+      oproto.writeListEnd();
+      byte[] bytes = transport.getArray();
+      return Codec.THRIFT.readSpans(bytes);
+    } catch (Exception e) {
       e.printStackTrace();
       return null;
     }

pom.xml

@@ -42,7 +42,6 @@
     <moshi.version>1.1.0</moshi.version>
     <okio.version>1.6.0</okio.version>
     <spring-boot.version>1.3.2.RELEASE</spring-boot.version>
-    <libthrift.version>0.9.3</libthrift.version>
     <!-- MySQL connector is GPL, even if it has an OSS exception.
          https://www.mysql.com/about/legal/licensing/foss-exception/
 
@@ -169,18 +168,6 @@
         <version>${assertj.version}</version>
       </dependency>
 
-      <dependency>
-        <groupId>org.apache.thrift</groupId>
-        <artifactId>libthrift</artifactId>
-        <version>${libthrift.version}</version>
-        <exclusions>
-          <exclusion>
-            <groupId>*</groupId>
-            <artifactId>*</artifactId>
-          </exclusion>
-        </exclusions>
-      </dependency>
-
       <dependency>
         <groupId>org.mariadb.jdbc</groupId>
         <artifactId>mariadb-java-client</artifactId>

zipkin/pom.xml

@@ -35,10 +35,6 @@
       <groupId>com.squareup.moshi</groupId>
       <artifactId>moshi</artifactId>
     </dependency>
-    <dependency>
-      <groupId>org.apache.thrift</groupId>
-      <artifactId>libthrift</artifactId>
-    </dependency>
   </dependencies>
 
   <build>
@@ -66,7 +62,7 @@
         <groupId>org.codehaus.mojo</groupId>
         <artifactId>animal-sniffer-maven-plugin</artifactId>
       </plugin>
-      <!-- Use of okio, moshi, and libthrift are internal only -->
+      <!-- Use of okio and moshi are internal only -->
       <plugin>
         <artifactId>maven-shade-plugin</artifactId>
         <executions>
@@ -87,10 +83,6 @@
                   <pattern>com.squareup.moshi</pattern>
                   <shadedPattern>zipkin.internal.moshi</shadedPattern>
                 </relocation>
-                <relocation>
-                  <pattern>org.apache.thrift</pattern>
-                  <shadedPattern>zipkin.internal.libthrift</shadedPattern>
-                </relocation>
               </relocations>
             </configuration>
           </execution>

zipkin/src/main/java/zipkin/Span.java

@@ -256,9 +256,9 @@ public Builder duration(@Nullable Long duration) {
      *
      * @see Span#annotations
      */
-    public Builder annotations(Annotation... annotations) {
+    public Builder annotations(Collection<Annotation> annotations) {
       this.annotations.clear();
-      Collections.addAll(this.annotations, annotations);
+      this.annotations.addAll(annotations);
       return this;
     }
 
@@ -273,9 +273,9 @@ public Builder addAnnotation(Annotation annotation) {
      *
      * @see Span#binaryAnnotations
      */
-    public Builder binaryAnnotations(BinaryAnnotation... binaryAnnotations) {
+    public Builder binaryAnnotations(Collection<BinaryAnnotation> binaryAnnotations) {
       this.binaryAnnotations.clear();
-      Collections.addAll(this.binaryAnnotations, binaryAnnotations);
+      this.binaryAnnotations.addAll(binaryAnnotations);
       return this;
     }
 

zipkin/src/main/java/zipkin/internal/CorrectForClockSkew.java

@@ -78,25 +78,25 @@ private static void adjust(Node<Span> node, @Nullable ClockSkew skewFromParent)
   }
 
   /** If any annotation has an IP with skew associated, adjust accordingly. */
-  private static Span adjustTimestamps(Span span, ClockSkew clockSkew) {
-    Annotation[] annotations = null;
+  private static Span adjustTimestamps(Span span, ClockSkew skew) {
+    List<Annotation> annotations = null;
     for (int i = 0, length = span.annotations.size(); i < length; i++) {
       Annotation a = span.annotations.get(i);
       if (a.endpoint == null) continue;
-      if (clockSkew.endpoint.ipv4 == a.endpoint.ipv4) {
-        if (annotations == null) annotations = span.annotations.toArray(new Annotation[length]);
-        annotations[i] = new Annotation.Builder(a).timestamp(a.timestamp - clockSkew.skew).build();
+      if (skew.endpoint.ipv4 == a.endpoint.ipv4) {
+        if (annotations == null) annotations = new ArrayList<>(span.annotations);
+        annotations.set(i, new Annotation.Builder(a).timestamp(a.timestamp - skew.skew).build());
       }
     }
     if (annotations != null) {
-      return new Span.Builder(span).timestamp(annotations[0].timestamp).annotations(annotations).build();
+      return new Span.Builder(span).timestamp(annotations.get(0).timestamp).annotations(annotations).build();
     }
     // Search for a local span on the skewed endpoint
     for (int i = 0, length = span.binaryAnnotations.size(); i < length; i++) {
       BinaryAnnotation b = span.binaryAnnotations.get(i);
       if (b.endpoint == null) continue;
-      if (b.key.equals(Constants.LOCAL_COMPONENT) && clockSkew.endpoint.ipv4 == b.endpoint.ipv4) {
-        return new Span.Builder(span).timestamp(span.timestamp - clockSkew.skew).build();
+      if (b.key.equals(Constants.LOCAL_COMPONENT) && skew.endpoint.ipv4 == b.endpoint.ipv4) {
+        return new Span.Builder(span).timestamp(span.timestamp - skew.skew).build();
       }
     }
     return span;