Refactors project around v2 storage apis

This does the following towards unifying the storage layer: * Removes zipkin-guava storage adapters as they aren't used anymore * Removes the "legacy read" of our first Elasticsearch model * Refactors artifact and directory names wher -v1 implies legacy * Refactors README to discourage v1 storage components * Makes the server only depend on zipkin2.storage.StorageComponent Follow-up work can refactor more once a v2 collector component is available.
tianmafly · Jun 7, 2018 · 8e14ac1 · 8e14ac1
1 parent 67e7774
commit 8e14ac1
Show file tree

Hide file tree

Showing 243 changed files with 3,473 additions and 5,992 deletions.
diff --git a/README.md b/README.md
@@ -50,7 +50,10 @@ bytes = SpanBytesEncoder.JSON_V2.encode(span);
 Note: The above is just an example, most likely you'll want to use an existing tracing library like [Brave](https://github.com/openzipkin/brave)
 
 ## Storage Component
-Zipkin includes a [StorageComponent](zipkin2/src/main/java/zipkin2/storage/StorageComponent.java), used to store and query spans and dependency links. This is used by the server and those making custom servers, collectors, or span reporters. For this reason, storage components have minimal dependencies; many run on Java 7.
+Zipkin includes a [StorageComponent](zipkin2/src/main/java/zipkin2/storage/StorageComponent.java), used to store and query spans and
+dependency links. This is used by the server and those making custom
+servers, collectors, or span reporters. For this reason, storage
+components have minimal dependencies, but most require Java 8+
 
 Ex.
 ```java
@@ -69,19 +72,25 @@ storage.close();
 ```
 
 ### In-Memory
-The [InMemoryStorage](zipkin2/src/main/java/zipkin2/storage/InMemoryStorage.java) component is packaged in zipkin's core library. It is not persistent, nor viable for realistic work loads. Its purpose is for testing, for example starting a server on your laptop without any database needed.
+The [InMemoryStorage](zipkin2/src/main/java/zipkin2/storage/InMemoryStorage.java) component is packaged in zipkin's core library. It
+is neither persistent, nor viable for realistic work loads. Its purpose
+is for testing, for example starting a server on your laptop without any
+database needed.
 
-### MySQL
-The [MySQLStorage](zipkin-storage/mysql) component currently is only tested with MySQL 5.6-7. It is designed to be easy to understand, and get started with. For example, it deconstructs spans into columns, so you can perform ad-hoc queries using SQL. However, this component has [known performance issues](https://github.com/openzipkin/zipkin/issues/1233): queries will eventually take seconds to return if you put a lot of data into it.
+### Cassandra
+The [Cassandra](zipkin-storage/cassandra) component is tested against
+Cassandra 3.11+. It stores spans using UDTs, such that they appear like
+the v2 Zipkin model in cqlsh. It is designed for scale. For example, it
+uses a combination of SASI and manually implemented indexes to make
+querying larger data more performant.
 
-### Cassandra v3
-The [Cassandra v3](zipkin-storage/zipkin2_cassandra) component is tested against Cassandra 3.11+. It stores spans using UDTs, such that they appear like the v2 Zipkin model in cqlsh. It is designed for scale. For example, it uses a combination of SASI and manually implemented indexes to make querying larger data more performant. This store requires a [spark job](https://github.com/openzipkin/zipkin-dependencies) to aggregate dependency links.
-
-### Cassandra Legacy
-The [Cassandra Legacy](zipkin-storage/cassandra) component is tested against Cassandra 2.2+. It stores spans as opaque thrifts which means you can't read them in cqlsh. However, it is designed for scale. For example, it has manually implemented indexes to make querying larger data more performant. This store requires a [spark job](https://github.com/openzipkin/zipkin-dependencies) to aggregate dependency links.
+Note: This store requires a [spark job](https://github.com/openzipkin/zipkin-dependencies) to aggregate dependency links.
 
 ### Elasticsearch
-The [ElasticsearchHttpStorage](zipkin-storage/elasticsearch-http) component is tested against Elasticsearch 2.x and 5.x. It stores spans as json and has been designed for larger scale. This store requires a [spark job](https://github.com/openzipkin/zipkin-dependencies) to aggregate dependency links.
+The [Elasticsearch](zipkin-storage/elasticsearch) component is tested against Elasticsearch 2-6.x.
+It stores spans as json and has been designed for larger scale.
+
+Note: This store requires a [spark job](https://github.com/openzipkin/zipkin-dependencies) to aggregate dependency links.
 
 ### Disabling search
 Search is enabled by default, primarily in support of the `GET /traces`,
@@ -95,11 +104,34 @@ disable search to reduce storage costs or increase write throughput.
 `StorageComponent.Builder.searchEnabled(false)` is implied when a zipkin
 is run with the env variable `SEARCH_ENABLED=false`.
 
+### Legacy (v1) components
+The following components are no longer encouraged, but exist to help aid
+transition to supported ones. These are indicated as "v1" as they use
+data layouts based on Zipkin's V1 Thrift model, as opposed to the
+simpler v2 data model currently used.
+
+#### MySQL
+The [MySQL v1](zipkin-storage/mysql-v1) component currently is only
+tested with MySQL 5.6-7. It is designed to be easy to understand, and
+get started with. For example, it deconstructs spans into columns, so
+you can perform ad-hoc queries using SQL. However, this component has
+[known performance issues](https://github.com/openzipkin/zipkin/issues/1233): queries will eventually take seconds to return
+if you put a lot of data into it.
+
+#### Cassandra
+The [Cassandra v1](zipkin-storage/cassandra-v1) component is tested
+against Cassandra 2.2+. It stores spans as opaque thrifts which means
+you can't read them in cqlsh. However, it is designed for scale. For
+example, it has manually implemented indexes to make querying larger
+data more performant. This store requires a [spark job](https://github.com/openzipkin/zipkin-dependencies) to aggregate
+dependency links.
+
 ## Running the server from source
-The [zipkin server](zipkin-server)
-receives spans via HTTP POST and respond to queries from its UI. It can also run collectors, such as Scribe or Kafka.
+The [Zipkin server](zipkin-server) receives spans via HTTP POST and respond to queries
+from its UI. It can also run collectors, such as RabbitMQ or Kafka.
 
-To run the server from the currently checked out source, enter the following. JDK 8 is required.
+To run the server from the currently checked out source, enter the
+following. JDK 8 is required.
 ```bash
 # Build the server and also make its dependencies
 $ ./mvnw -DskipTests --also-make -pl zipkin-server clean install

diff --git a/pom.xml b/pom.xml
@@ -27,7 +27,6 @@
     <module>zipkin</module>
     <module>zipkin-ui</module>
     <module>zipkin-junit</module>
-    <module>zipkin-guava</module>
     <module>benchmarks</module>
     <module>zipkin-storage</module>
     <module>zipkin-collector</module>
@@ -188,8 +187,8 @@
       </dependency>
 
       <dependency>
-        <groupId>${project.groupId}</groupId>
-        <artifactId>zipkin-autoconfigure-storage-cassandra</artifactId>
+        <groupId>io.zipkin.zipkin2</groupId>
+        <artifactId>zipkin-autoconfigure-storage-cassandra-v1</artifactId>
         <version>${project.version}</version>
       </dependency>
 
@@ -200,8 +199,8 @@
       </dependency>
 
       <dependency>
-        <groupId>${project.groupId}</groupId>
-        <artifactId>zipkin-autoconfigure-storage-cassandra3</artifactId>
+        <groupId>io.zipkin.zipkin2</groupId>
+        <artifactId>zipkin-autoconfigure-storage-cassandra</artifactId>
         <version>${project.version}</version>
       </dependency>
 
@@ -212,25 +211,19 @@
       </dependency>
 
       <dependency>
-        <groupId>${project.groupId}</groupId>
-        <artifactId>zipkin-autoconfigure-storage-mysql</artifactId>
-        <version>${project.version}</version>
-      </dependency>
-
-      <dependency>
-        <groupId>${project.groupId}</groupId>
-        <artifactId>zipkin-storage-elasticsearch-http</artifactId>
+        <groupId>io.zipkin.zipkin2</groupId>
+        <artifactId>zipkin-autoconfigure-storage-mysql-v1</artifactId>
         <version>${project.version}</version>
       </dependency>
 
       <dependency>
-        <groupId>${project.groupId}</groupId>
-        <artifactId>zipkin-autoconfigure-storage-elasticsearch-http</artifactId>
+        <groupId>io.zipkin.zipkin2</groupId>
+        <artifactId>zipkin-autoconfigure-storage-elasticsearch</artifactId>
         <version>${project.version}</version>
       </dependency>
 
       <dependency>
-        <groupId>${project.groupId}</groupId>
+        <groupId>io.zipkin.zipkin2</groupId>
         <artifactId>zipkin-autoconfigure-storage-elasticsearch-aws</artifactId>
         <version>${project.version}</version>
       </dependency>

diff --git a/zipkin-autoconfigure/pom.xml b/zipkin-autoconfigure/pom.xml
@@ -40,10 +40,10 @@
     <module>collector-rabbitmq</module>
     <module>collector-scribe</module>
     <module>storage-cassandra</module>
-    <module>storage-cassandra3</module>
-    <module>storage-elasticsearch-http</module>
+    <module>storage-cassandra-v1</module>
+    <module>storage-elasticsearch</module>
     <module>storage-elasticsearch-aws</module>
-    <module>storage-mysql</module>
+    <module>storage-mysql-v1</module>
     <module>metrics-prometheus</module>
   </modules>
 

diff --git a/...-autoconfigure/storage-cassandra3/pom.xml → ...utoconfigure/storage-cassandra-v1/pom.xml b/...-autoconfigure/storage-cassandra3/pom.xml → ...utoconfigure/storage-cassandra-v1/pom.xml
@@ -14,7 +14,9 @@
     the License.
 
 -->
-<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+  xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
   <modelVersion>4.0.0</modelVersion>
 
   <parent>
@@ -23,23 +25,20 @@
     <version>2.8.5-SNAPSHOT</version>
   </parent>
 
-  <artifactId>zipkin-autoconfigure-storage-cassandra3</artifactId>
-  <name>Auto Configuration: Cassandra 3 Storage</name>
+  <groupId>io.zipkin.zipkin2</groupId>
+  <artifactId>zipkin-autoconfigure-storage-cassandra-v1</artifactId>
+  <name>Auto Configuration: Cassandra Storage (v1)</name>
 
   <properties>
     <main.basedir>${project.basedir}/../..</main.basedir>
   </properties>
 
   <dependencies>
-    <dependency>
-      <groupId>${project.groupId}</groupId>
-      <artifactId>zipkin</artifactId>
-    </dependency>
-
     <dependency>
       <groupId>io.zipkin.zipkin2</groupId>
-      <artifactId>zipkin-storage-cassandra</artifactId>
+      <artifactId>zipkin-storage-cassandra-v1</artifactId>
     </dependency>
+
     <dependency>
       <groupId>io.zipkin.brave</groupId>
       <artifactId>brave</artifactId>

diff --git a/...ure/storage/cassandra/TracingSession.java → ...ure/storage/cassandra/TracingSession.java b/...ure/storage/cassandra/TracingSession.java → ...ure/storage/cassandra/TracingSession.java
@@ -38,6 +38,7 @@
 
 /**
  * Differs from https://github.com/openzipkin/brave-cassandra in the following ways:
+ *
  * <pre><ul>
  *   <li>Doesn't propagate (due to cassandra 3.11.1 java.nio.BufferUnderflowException: null)</li>
  *   <li>Doesn't trace unless there's a current span (to prevent write amplification)</li>
@@ -58,7 +59,8 @@ final class TracingSession extends AbstractSession {
     keyspace = delegate.getLoggedKeyspace();
   }
 
-  @Override public ResultSetFuture executeAsync(Statement statement) {
+  @Override
+  public ResultSetFuture executeAsync(Statement statement) {
     // don't start new traces (to prevent amplifying writes to local storage)
     if (tracer.currentSpan() == null) {
       return delegate.executeAsync(statement);
@@ -85,66 +87,80 @@ final class TracingSession extends AbstractSession {
       throw e;
     }
     if (span.isNoop()) return result; // don't add callback on noop
-    Futures.addCallback(result, new FutureCallback<ResultSet>() {
-      @Override public void onSuccess(ResultSet result) {
-        InetSocketAddress host = result.getExecutionInfo().getQueriedHost().getSocketAddress();
-        span.remoteEndpoint(Endpoint.newBuilder()
-          .serviceName(remoteServiceName)
-          .ip(host.getAddress())
-          .port(host.getPort())
-          .build()
-        );
-        span.finish();
-      }
-
-      @Override public void onFailure(Throwable e) {
-        addError(e, span);
-        span.finish();
-      }
-    });
+    Futures.addCallback(
+        result,
+        new FutureCallback<ResultSet>() {
+          @Override
+          public void onSuccess(ResultSet result) {
+            InetSocketAddress host = result.getExecutionInfo().getQueriedHost().getSocketAddress();
+            span.remoteEndpoint(
+                Endpoint.newBuilder()
+                    .serviceName(remoteServiceName)
+                    .ip(host.getAddress())
+                    .port(host.getPort())
+                    .build());
+            span.finish();
+          }
+
+          @Override
+          public void onFailure(Throwable e) {
+            addError(e, span);
+            span.finish();
+          }
+        });
     return result;
   }
 
-  @Override protected ListenableFuture<PreparedStatement> prepareAsync(String query,
-    Map<String, ByteBuffer> customPayload) {
+  @Override
+  protected ListenableFuture<PreparedStatement> prepareAsync(
+      String query, Map<String, ByteBuffer> customPayload) {
     SimpleStatement statement = new SimpleStatement(query);
     statement.setOutgoingPayload(customPayload);
     return prepareAsync(statement);
   }
 
-  @Override public ListenableFuture<PreparedStatement> prepareAsync(String query) {
+  @Override
+  public ListenableFuture<PreparedStatement> prepareAsync(String query) {
     return delegate.prepareAsync(query);
   }
 
-  @Override public String getLoggedKeyspace() {
+  @Override
+  public String getLoggedKeyspace() {
     return delegate.getLoggedKeyspace();
   }
 
-  @Override public Session init() {
+  @Override
+  public Session init() {
     return delegate.init();
   }
 
-  @Override public ListenableFuture<Session> initAsync() {
+  @Override
+  public ListenableFuture<Session> initAsync() {
     return delegate.initAsync();
   }
 
-  @Override public ListenableFuture<PreparedStatement> prepareAsync(RegularStatement statement) {
+  @Override
+  public ListenableFuture<PreparedStatement> prepareAsync(RegularStatement statement) {
     return delegate.prepareAsync(statement);
   }
 
-  @Override public CloseFuture closeAsync() {
+  @Override
+  public CloseFuture closeAsync() {
     return delegate.closeAsync();
   }
 
-  @Override public boolean isClosed() {
+  @Override
+  public boolean isClosed() {
     return delegate.isClosed();
   }
 
-  @Override public Cluster getCluster() {
+  @Override
+  public Cluster getCluster() {
     return delegate.getCluster();
   }
 
-  @Override public State getState() {
+  @Override
+  public State getState() {
     return delegate.getState();
   }
 

diff --git a/...kinCassandraStorageAutoConfiguration.java → ...kinCassandraStorageAutoConfiguration.java b/...kinCassandraStorageAutoConfiguration.java → ...kinCassandraStorageAutoConfiguration.java
@@ -34,7 +34,8 @@ class TracingZipkinCassandraStorageAutoConfiguration {
 
   // NOTE: this doesn't yet trace span consumption commands because the trace context
   // is lost when indirected with SpanConsumer.accept().enqueue(). We'll fix this later
-  @Bean SessionFactory tracingSessionFactory() {
+  @Bean
+  SessionFactory tracingSessionFactory() {
     return storage -> new TracingSession(tracing, delegate.create(storage));
   }
 }
diff --git a/...kinCassandraStorageAutoConfiguration.java → ...kinCassandraStorageAutoConfiguration.java b/...kinCassandraStorageAutoConfiguration.java → ...kinCassandraStorageAutoConfiguration.java
@@ -21,7 +21,6 @@
 import org.springframework.boot.context.properties.EnableConfigurationProperties;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
-import zipkin.internal.V2StorageComponent;
 import zipkin2.storage.StorageComponent;
 import zipkin2.storage.cassandra.v1.CassandraStorage;
 import zipkin2.storage.cassandra.v1.SessionFactory;
@@ -43,21 +42,14 @@ class ZipkinCassandraStorageAutoConfiguration {
 
   @Bean
   @ConditionalOnMissingBean
-  V2StorageComponent storage(
+  StorageComponent storage(
       ZipkinCassandraStorageProperties properties,
       @Value("${zipkin.storage.strict-trace-id:true}") boolean strictTraceId,
       @Value("${zipkin.storage.search-enabled:true}") boolean searchEnabled) {
     CassandraStorage.Builder builder =
         properties.toBuilder().strictTraceId(strictTraceId).searchEnabled(searchEnabled);
-    CassandraStorage result =
-        tracingSessionFactory == null
-            ? builder.build()
-            : builder.sessionFactory(tracingSessionFactory).build();
-    return V2StorageComponent.create(result);
-  }
-
-  @Bean
-  CassandraStorage v2Storage(V2StorageComponent component) {
-    return (CassandraStorage) component.delegate();
+    return tracingSessionFactory == null
+        ? builder.build()
+        : builder.sessionFactory(tracingSessionFactory).build();
   }
 }