Merge pull request #13 from openzipkin/zipkin-scala-tests

Adds integration tests from scala project and addresses drift

zipkin-java-core/src/main/java/io/zipkin/Constants.java

@@ -13,6 +13,11 @@
  */
 package io.zipkin;
 
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.LinkedHashSet;
+import java.util.Set;
+
 public final class Constants {
   /* Common annotation values */
   public static final String CLIENT_RECV = "cr";
@@ -27,12 +32,32 @@ public final class Constants {
    * Annotation#host()}
    */
   public static final String CLIENT_ADDR = "ca";
+
   /**
    * The endpoint associated with "SERVER_" annotations is not necessarily {@link
    * Annotation#host()}
    */
   public static final String SERVER_ADDR = "sa";
 
+  /**
+   * Zipkin Span Storage implementations should not expect users to search by core annotations.
+   *
+   * When storage implementations separate annotation indexing from storage, filter out these
+   * values.
+   */
+  public static final Set<String> CORE_ANNOTATIONS = Collections.unmodifiableSet(
+      new LinkedHashSet<>(Arrays.asList(
+          CLIENT_RECV,
+          CLIENT_SEND,
+          SERVER_RECV,
+          SERVER_SEND,
+          WIRE_SEND,
+          WIRE_RECV,
+          CLIENT_ADDR,
+          SERVER_ADDR
+      ))
+  );
+
   private Constants() {
   }
 }

zipkin-java-core/src/main/java/io/zipkin/Span.java

@@ -18,9 +18,7 @@
 import com.facebook.swift.codec.ThriftStruct;
 import com.google.auto.value.AutoValue;
 import io.zipkin.internal.Nullable;
-import java.util.LinkedHashSet;
 import java.util.List;
-import java.util.Set;
 
 import static com.facebook.swift.codec.ThriftField.Requiredness.OPTIONAL;
 
@@ -59,31 +57,6 @@ public static Builder builder(Span source) {
   @ThriftField(value = 9, requiredness = OPTIONAL)
   public abstract Boolean debug();
 
-  /**
-   * Assuming this is an RPC span, is it from the client side?
-   */
-  public boolean isClientSide() {
-    for (Annotation a : annotations()) {
-      if (a.value().equals(Constants.CLIENT_SEND) || a.value().equals(Constants.CLIENT_RECV)) {
-        return true;
-      }
-    }
-    return false;
-  }
-
-  /**
-   * Returns the lower-cased set of service names this span has annotations for.
-   */
-  public Set<String> serviceNames() {
-    Set<String> result = new LinkedHashSet<>();
-    for (Annotation a : annotations()) {
-      if (a.host() != null) {
-        result.add(a.host().serviceName().toLowerCase());
-      }
-    }
-    return result;
-  }
-
   @AutoValue.Builder
   public interface Builder {
 

zipkin-java-query/pom.xml

@@ -29,6 +29,12 @@
   <name>Zipkin Java Query</name>
   <description>Zipkin Java Query</description>
 
+  <properties>
+    <!-- Dependencies for SpanStoreSpec -->
+    <zipkin-scala.version>1.9.0</zipkin-scala.version>
+    <scalatest.version>2.2.5</scalatest.version>
+  </properties>
+
   <dependencies>
     <dependency>
       <groupId>${project.groupId}</groupId>
@@ -39,5 +45,59 @@
       <groupId>com.facebook.swift</groupId>
       <artifactId>swift-annotations</artifactId>
     </dependency>
+
+    <!-- for ZipkinQuerySpanStoreAdapter -->
+    <dependency>
+      <groupId>${project.groupId}</groupId>
+      <artifactId>swift-codec</artifactId>
+    </dependency>
+
+    <!-- Dependencies for SpanStoreSpec -->
+    <dependency>
+      <groupId>io.zipkin</groupId>
+      <artifactId>zipkin-common</artifactId>
+      <version>${zipkin-scala.version}</version>
+      <scope>test</scope>
+      <exclusions>
+          <exclusion>
+            <groupId>org.apache.thrift</groupId>
+            <artifactId>libthrift</artifactId>
+          </exclusion>
+      </exclusions>
+    </dependency>
+
+    <dependency>
+      <groupId>io.zipkin</groupId>
+      <artifactId>zipkin-scrooge</artifactId>
+      <version>${zipkin-scala.version}</version>
+      <scope>test</scope>
+      <exclusions>
+          <exclusion>
+            <groupId>org.apache.thrift</groupId>
+            <artifactId>libthrift</artifactId>
+          </exclusion>
+      </exclusions>
+    </dependency>
+
+    <dependency>
+      <groupId>io.zipkin</groupId>
+      <artifactId>zipkin-common</artifactId>
+      <version>${zipkin-scala.version}</version>
+      <classifier>test</classifier>
+      <scope>test</scope>
+      <exclusions>
+          <exclusion>
+            <groupId>org.apache.thrift</groupId>
+            <artifactId>libthrift</artifactId>
+          </exclusion>
+      </exclusions>
+    </dependency>
+
+    <dependency>
+      <groupId>org.scalatest</groupId>
+      <artifactId>scalatest_2.10</artifactId>
+      <version>${scalatest.version}</version>
+      <scope>test</scope>
+    </dependency>
   </dependencies>
 </project>

zipkin-java-query/src/main/java/io/zipkin/query/InMemoryZipkinQuery.java

@@ -15,6 +15,7 @@
 
 import io.zipkin.Annotation;
 import io.zipkin.BinaryAnnotation;
+import io.zipkin.Constants;
 import io.zipkin.Span;
 import io.zipkin.Trace;
 import io.zipkin.internal.Nullable;
@@ -28,18 +29,20 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.function.Consumer;
 import java.util.function.Predicate;
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
 
-public final class InMemoryZipkinQuery implements ZipkinQuery {
+public final class InMemoryZipkinQuery implements ZipkinQuery, Consumer<List<Span>> {
   private static final Charset UTF_8 = Charset.forName("UTF-8");
 
   private final Multimap<Long, Span> traceIdToSpans = new Multimap<>(LinkedList::new);
   private final Multimap<String, Long> serviceToTraceIds = new Multimap<>(LinkedHashSet::new);
   private final Multimap<String, String> serviceToSpanNames = new Multimap<>(LinkedHashSet::new);
 
-  public synchronized void accept(Iterable<Span> spans) {
+  @Override
+  public synchronized void accept(List<Span> spans) {
     spans.forEach(span -> {
       long traceId = span.traceId();
       traceIdToSpans.put(span.traceId(), span);
@@ -125,14 +128,16 @@ static Predicate<Span> spanPredicate(QueryRequest request) {
       spanPredicate = spanPredicate.and(s -> s.name().equals(request.spanName()));
     }
     if (request.annotations() != null && !request.annotations().isEmpty()) {
-      spanPredicate = spanPredicate.and(s -> s.annotations().stream().map(
-          Annotation::value).allMatch(v -> request.annotations().contains(v)));
+      spanPredicate = spanPredicate.and(s -> s.annotations().stream()
+          .map(Annotation::value)
+          .filter(v -> !Constants.CORE_ANNOTATIONS.contains(v)) // don't return core annotations
+          .anyMatch(v -> request.annotations().contains(v)));
     }
     if (request.binaryAnnotations() != null && !request.binaryAnnotations().isEmpty()) {
       spanPredicate = spanPredicate.and(s -> s.binaryAnnotations().stream()
           .filter(b -> b.type() == BinaryAnnotation.Type.STRING)
           .filter(b -> request.binaryAnnotations().containsKey(b.key()))
-          .allMatch(
+          .anyMatch(
               b -> request.binaryAnnotations().get(b.key()).equals(new String(b.value(), UTF_8))));
     }
     return spanPredicate;

zipkin-java-query/src/main/java/io/zipkin/query/QueryRequest.java

@@ -17,6 +17,7 @@
 import com.facebook.swift.codec.ThriftField;
 import com.facebook.swift.codec.ThriftStruct;
 import com.google.auto.value.AutoValue;
+import io.zipkin.BinaryAnnotation;
 import io.zipkin.internal.Nullable;
 import java.util.List;
 import java.util.Map;
@@ -38,10 +39,16 @@ public static Builder builder() {
   @ThriftField(value = 2, requiredness = OPTIONAL)
   public abstract String spanName();
 
+  /**
+   * Custom-defined annotation values to search for.
+   */
   @Nullable
   @ThriftField(value = 3, requiredness = OPTIONAL)
   public abstract List<String> annotations();
 
+  /**
+   * Binary annotations of type {@link BinaryAnnotation.Type#STRING} to search for.
+   */
   @Nullable
   @ThriftField(value = 8, requiredness = OPTIONAL)
   public abstract Map<String, String> binaryAnnotations();

zipkin-java-query/src/test/java/io/zipkin/query/InMemoryZipkinQuerySpanStoreTest.java

@@ -0,0 +1,29 @@
+/**
+ * Copyright 2015 The OpenZipkin Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
+ * in compliance with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package io.zipkin.query;
+
+import com.twitter.zipkin.storage.SpanStore;
+import com.twitter.zipkin.storage.SpanStoreSpec;
+
+public class InMemoryZipkinQuerySpanStoreTest extends SpanStoreSpec {
+  private ZipkinQuerySpanStoreAdapter<InMemoryZipkinQuery> mem;
+
+  public SpanStore store() {
+    return mem;
+  }
+
+  public void clear() {
+    mem = new ZipkinQuerySpanStoreAdapter<>(new InMemoryZipkinQuery());
+  }
+}

zipkin-java-query/src/test/java/io/zipkin/query/InMemoryZipkinQueryTest.java

@@ -14,6 +14,7 @@
 package io.zipkin.query;
 
 import io.zipkin.Span;
+import java.util.List;
 
 public class InMemoryZipkinQueryTest extends ZipkinQueryTest {
   InMemoryZipkinQuery query = new InMemoryZipkinQuery();
@@ -22,7 +23,7 @@ public class InMemoryZipkinQueryTest extends ZipkinQueryTest {
     return query;
   }
 
-  @Override protected void reload(Iterable<Span> spans) {
+  @Override protected void reload(List<Span> spans) {
     query.clear();
     query.accept(spans);
   }