Skip to content

Commit

Permalink
Removes internal use of libthrift
Browse files Browse the repository at this point in the history
This implements TBinaryProtocol directly in the ThriftCodec. By doing
the code, dependencies, and exceptions are simpler (as we are decoding
via a buffer not a network). This uses Okio Buffer, which also
simplified some code.

The payment was having to implement a couple utilities from libthrift,
namely the skip logic. Overall, the code is smaller and the binary
shrunk from 275k to 225k.
  • Loading branch information
Adrian Cole committed Feb 25, 2016
1 parent fe3a3c4 commit f5a93ea
Show file tree
Hide file tree
Showing 8 changed files with 398 additions and 505 deletions.
16 changes: 15 additions & 1 deletion interop/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand All @@ -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>
Expand Down
39 changes: 23 additions & 16 deletions interop/src/main/java/zipkin/interop/ScalaSpanStoreAdapter.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand Down
13 changes: 0 additions & 13 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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/
Expand Down Expand Up @@ -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>
Expand Down
10 changes: 1 addition & 9 deletions zipkin/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand All @@ -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>
Expand Down
8 changes: 4 additions & 4 deletions zipkin/src/main/java/zipkin/Span.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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;
}

Expand Down
16 changes: 8 additions & 8 deletions zipkin/src/main/java/zipkin/internal/CorrectForClockSkew.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading

0 comments on commit f5a93ea

Please sign in to comment.