Test cleanup including being less sensitive to single-host span storage

There were tests accidentally assuming merge semantics when testing
unrelated things. This scrubs some of the tests to focus on what they
are testing, and in doing so help pave forward single-host span storage.

pom.xml

@@ -70,7 +70,7 @@
     <assertj.version>3.8.0</assertj.version>
     <hamcrest.version>1.3</hamcrest.version>
     <okhttp.version>3.8.1</okhttp.version>
-    <testcontainers.version>1.3.1</testcontainers.version>
+    <testcontainers.version>1.4.1</testcontainers.version>
 
     <animal-sniffer-maven-plugin.version>1.15</animal-sniffer-maven-plugin.version>
     <maven-compiler-plugin.version>3.6.1</maven-compiler-plugin.version>

zipkin-autoconfigure/collector-kafka/src/main/java/zipkin/autoconfigure/collector/kafka/ZipkinKafkaCollectorProperties.java

@@ -1,5 +1,5 @@
 /**
- * Copyright 2015-2016 The OpenZipkin Authors
+ * Copyright 2015-2017 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
@@ -15,7 +15,6 @@
 
 import java.util.LinkedHashMap;
 import java.util.Map;
-import java.util.Properties;
 import org.springframework.boot.context.properties.ConfigurationProperties;
 import zipkin.collector.kafka.KafkaCollector;
 

zipkin-autoconfigure/storage-elasticsearch-aws/src/main/java/zipkin/autoconfigure/storage/elasticsearch/aws/ZipkinElasticsearchAwsStorageAutoConfiguration.java

@@ -39,7 +39,6 @@
 import org.springframework.core.type.AnnotatedTypeMetadata;
 import zipkin.autoconfigure.storage.elasticsearch.http.ZipkinElasticsearchHttpStorageProperties;
 import zipkin.storage.StorageComponent;
-import zipkin.storage.elasticsearch.http.ElasticsearchHttpStorage;
 
 import static java.lang.String.format;
 import static zipkin.internal.Util.checkArgument;

zipkin-junit/src/test/java/zipkin/junit/ZipkinRuleTest.java

@@ -1,5 +1,5 @@
 /**
- * Copyright 2015-2016 The OpenZipkin Authors
+ * Copyright 2015-2017 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
@@ -35,7 +35,10 @@
 import static java.util.Arrays.asList;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.failBecauseExceptionWasNotThrown;
+import static zipkin.TestObjects.LOTS_OF_SPANS;
+import static zipkin.TestObjects.TODAY;
 import static zipkin.TestObjects.TRACE;
+import static zipkin.TestObjects.WEB_ENDPOINT;
 
 public class ZipkinRuleTest {
 
@@ -44,22 +47,21 @@ public class ZipkinRuleTest {
 
   OkHttpClient client = new OkHttpClient();
 
-  long traceId = TRACE.get(0).traceId;
-
   @Test
   public void getTraces_storedViaPost() throws IOException {
+    List<Span> trace = asList(TRACE.get(0));
     // write the span to the zipkin using http
-    assertThat(postSpans(TRACE).code()).isEqualTo(202);
+    assertThat(postSpans(trace).code()).isEqualTo(202);
 
     // read the traces directly
     assertThat(zipkin.getTraces())
-        .containsOnly(TRACE);
+        .containsOnly(trace);
   }
 
   /** The rule is here to help debugging. Even partial spans should be returned */
   @Test
   public void getTraces_whenMissingTimestamps() throws IOException {
-    Span span = Span.builder().traceId(traceId).id(traceId).name("foo").build();
+    Span span = Span.builder().traceId(1L).id(1L).name("foo").build();
     // write the span to the zipkin using http
     assertThat(postSpans(asList(span)).code()).isEqualTo(202);
 
@@ -85,7 +87,7 @@ public void storeSpans_readbackHttp() throws IOException {
 
     // read trace id using the the http api
     Response getResponse = client.newCall(new Request.Builder()
-        .url(format("%s/api/v1/trace/%016x", zipkin.httpUrl(), traceId)).build()
+        .url(format("%s/api/v1/trace/%016x", zipkin.httpUrl(), TRACE.get(0).traceId)).build()
     ).execute();
 
     assertThat(getResponse.code()).isEqualTo(200);
@@ -94,28 +96,32 @@ public void storeSpans_readbackHttp() throws IOException {
   /** The raw query can show affects like redundant rows in the data store. */
   @Test
   public void storeSpans_readbackRaw() throws IOException {
+    long traceId = LOTS_OF_SPANS[0].traceId;
+
     // write the span to zipkin directly
-    zipkin.storeSpans(TRACE);
-    zipkin.storeSpans(TRACE);
+    zipkin.storeSpans(asList(LOTS_OF_SPANS[0]));
+    zipkin.storeSpans(asList(LOTS_OF_SPANS[0]));
 
     // Default will merge by span id
     Response defaultResponse = client.newCall(new Request.Builder()
         .url(format("%s/api/v1/trace/%016x", zipkin.httpUrl(), traceId)).build()
     ).execute();
 
-    assertThat(Codec.JSON.readSpans(defaultResponse.body().bytes())).hasSize(TRACE.size());
+    assertThat(Codec.JSON.readSpans(defaultResponse.body().bytes())).hasSize(1);
 
     // In the in-memory (or cassandra) stores, a raw read will show duplicate span rows.
     Response rawResponse = client.newCall(new Request.Builder()
         .url(format("%s/api/v1/trace/%016x?raw", zipkin.httpUrl(), traceId)).build()
     ).execute();
 
-    assertThat(Codec.JSON.readSpans(rawResponse.body().bytes())).hasSize(TRACE.size() * 2);
+    assertThat(Codec.JSON.readSpans(rawResponse.body().bytes())).hasSize(2);
   }
 
   @Test
   public void getBy128BitTraceId() throws Exception {
-    Span span = TRACE.get(0).toBuilder().traceIdHigh(traceId).build();
+    long traceId = LOTS_OF_SPANS[0].traceId;
+
+    Span span = LOTS_OF_SPANS[0].toBuilder().traceIdHigh(traceId).build();
     zipkin.storeSpans(asList(span));
 
     Response getResponse = client.newCall(new Request.Builder()
@@ -194,7 +200,6 @@ public void gzippedSpans() throws IOException {
         .post(RequestBody.create(MediaType.parse("application/json"), gzippedJson)).build()
     ).execute();
 
-    assertThat(zipkin.getTraces()).containsOnly(TRACE);
     assertThat(zipkin.collectorMetrics().bytes()).isEqualTo(spansInJson.length);
   }
 
@@ -214,13 +219,13 @@ public void readSpans_gzippedResponse() throws Exception {
     char[] annotation2K = new char[2048];
     Arrays.fill(annotation2K, 'a');
 
-    List<Span> trace = asList(TRACE.get(0).toBuilder().addAnnotation(
-            Annotation.create(System.currentTimeMillis(), new String(annotation2K), null)).build());
+    List<Span> trace = asList(TRACE.get(0).toBuilder()
+      .addAnnotation(Annotation.create(TODAY, new String(annotation2K), WEB_ENDPOINT)).build());
 
     zipkin.storeSpans(trace);
 
     Response response = client.newCall(new Request.Builder()
-            .url(format("%s/api/v1/trace/%016x", zipkin.httpUrl(), traceId))
+            .url(format("%s/api/v1/trace/%016x", zipkin.httpUrl(), trace.get(0).traceId))
             .addHeader("Accept-Encoding", "gzip").build()
     ).execute();
 

zipkin-server/src/main/java/zipkin/server/ZipkinHttpCollector.java

@@ -19,16 +19,13 @@
 import java.util.zip.GZIPInputStream;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
-import org.springframework.http.HttpStatus;
 import org.springframework.http.ResponseEntity;
 import org.springframework.util.concurrent.ListenableFuture;
 import org.springframework.util.concurrent.SettableListenableFuture;
 import org.springframework.web.bind.annotation.CrossOrigin;
-import org.springframework.web.bind.annotation.ExceptionHandler;
 import org.springframework.web.bind.annotation.RequestBody;
 import org.springframework.web.bind.annotation.RequestHeader;
 import org.springframework.web.bind.annotation.RequestMapping;
-import org.springframework.web.bind.annotation.ResponseStatus;
 import org.springframework.web.bind.annotation.RestController;
 import zipkin.Codec;
 import zipkin.collector.Collector;

zipkin-storage/cassandra/src/test/java/zipkin/storage/cassandra/CassandraSpanStoreTest.java

@@ -1,5 +1,5 @@
 /**
- * Copyright 2015-2016 The OpenZipkin Authors
+ * Copyright 2015-2017 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
@@ -18,7 +18,6 @@
 import java.util.stream.IntStream;
 
 import org.junit.AssumptionViolatedException;
-import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;

zipkin-storage/cassandra3/src/main/java/zipkin/storage/cassandra3/CassandraSpanStore.java

@@ -42,7 +42,6 @@
 import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
-import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;

zipkin-storage/cassandra3/src/test/java/zipkin/storage/cassandra3/integration/CassandraSpanStoreTest.java

@@ -15,24 +15,20 @@
 
 import java.util.ArrayList;
 import java.util.List;
-import java.util.concurrent.TimeUnit;
 import java.util.stream.IntStream;
 import org.junit.Test;
-import org.testcontainers.shaded.com.google.common.util.concurrent.Uninterruptibles;
 import zipkin.Annotation;
 import zipkin.BinaryAnnotation;
 import zipkin.Endpoint;
 import zipkin.Span;
 import zipkin.TestObjects;
 import zipkin.internal.ApplyTimestampAndDuration;
-import zipkin.internal.CallbackCaptor;
 import zipkin.internal.Util;
 import zipkin.storage.QueryRequest;
 import zipkin.storage.SpanStoreTest;
 import zipkin.storage.cassandra3.Cassandra3Storage;
 import zipkin.storage.cassandra3.InternalForTests;
 
-import static java.util.Arrays.asList;
 import static java.util.stream.Collectors.toList;
 import static org.assertj.core.api.Assertions.assertThat;
 

zipkin-storage/cassandra3/src/test/java/zipkin/storage/cassandra3/integration/CassandraStrictTraceIdFalseTest.java

@@ -15,7 +15,6 @@
 
 import java.util.ArrayList;
 import java.util.List;
-import org.junit.ClassRule;
 import org.junit.Test;
 import zipkin.Span;
 import zipkin.TestObjects;

zipkin-storage/cassandra3/src/test/java/zipkin/storage/cassandra3/integration/CassandraTest.java

@@ -16,6 +16,7 @@
 import com.datastax.driver.core.Session;
 import java.io.IOException;
 import org.junit.ClassRule;
+import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.experimental.runners.Enclosed;
@@ -79,4 +80,11 @@ public static class EnsureSchemaTest extends CassandraEnsureSchemaTest {
       return storage.get().session();
     }
   }
+
+  @Ignore("TODO: get this working or explain why not")
+  public static class StrictTraceIdFalseTest extends CassandraStrictTraceIdFalseTest {
+    @Override protected Cassandra3Storage storage() {
+      return storage.get();
+    }
+  }
 }

zipkin-storage/elasticsearch-http/src/test/java/zipkin/storage/elasticsearch/http/ElasticsearchHttpStorageTest.java

@@ -17,7 +17,6 @@
 import java.util.concurrent.TimeUnit;
 import okhttp3.mockwebserver.MockResponse;
 import okhttp3.mockwebserver.MockWebServer;
-import okhttp3.mockwebserver.RecordedRequest;
 import org.junit.After;
 import org.junit.Rule;
 import org.junit.Test;

zipkin-storage/mysql/src/main/java/zipkin/storage/mysql/DSLContexts.java

@@ -20,7 +20,6 @@
 import org.jooq.conf.Settings;
 import org.jooq.impl.DSL;
 import org.jooq.impl.DefaultConfiguration;
-import org.jooq.tools.jdbc.JDBCUtils;
 import zipkin.internal.Nullable;
 
 import static zipkin.internal.Util.checkNotNull;

zipkin/src/test/java/zipkin/TestObjects.java

@@ -1,5 +1,5 @@
 /**
- * Copyright 2015-2016 The OpenZipkin Authors
+ * Copyright 2015-2017 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
@@ -28,7 +28,6 @@
 import static zipkin.Constants.SERVER_ADDR;
 import static zipkin.Constants.SERVER_RECV;
 import static zipkin.Constants.SERVER_SEND;
-import static zipkin.internal.Util.UTF_8;
 import static zipkin.internal.Util.midnightUTC;
 
 public final class TestObjects {
@@ -72,7 +71,6 @@ public final class TestObjects {
           .addAnnotation(Annotation.create((TODAY + 150) * 1000, CLIENT_SEND, APP_ENDPOINT))
           .addAnnotation(Annotation.create((TODAY + 200) * 1000, CLIENT_RECV, APP_ENDPOINT))
           .addAnnotation(Annotation.create((TODAY + 190) * 1000, "⻩", NO_IP_ENDPOINT))
-          .addBinaryAnnotation(BinaryAnnotation.address(CLIENT_ADDR, APP_ENDPOINT))
           .addBinaryAnnotation(BinaryAnnotation.address(SERVER_ADDR, DB_ENDPOINT))
           .addBinaryAnnotation(BinaryAnnotation.create(ERROR, "\uD83D\uDCA9", NO_IP_ENDPOINT))
           .build()

zipkin/src/test/java/zipkin/storage/InMemorySpanStoreEvictionTest.java

@@ -144,14 +144,12 @@ public void evict_detailed() {
 
   @Test
   public void evict_oneTraceMultipleSpans() {
-    Span testSpan1 = span1.toBuilder().traceIdHigh(1L).traceId(123).
-      timestamp(ann1.timestamp).build();
-    Span testSpan2 = span2.toBuilder().traceIdHigh(2L).traceId(123).
-      timestamp(ann2.timestamp).build();
+    Span testSpan1 = span1;
+    Span testSpan2 = span2.toBuilder().traceId(span1.traceId).build();
 
     consumer.accept(asList(testSpan1, testSpan2));
     assertThat(store.getTraces(QueryRequest.builder().build()))
-      .containsOnly(asList(testSpan1), asList(testSpan2));
+      .containsOnly(asList(testSpan1, testSpan2));
 
     // Since both spans are a part of the same trace, getting down to
     // only one span means deleting both (the whole trace)

zipkin/src/test/java/zipkin/storage/SpanStoreTest.java

@@ -519,11 +519,11 @@ public void getTraces_differentiateOnServiceName() {
 
     Span trace2 = Span.builder().traceId(2).name("get").id(2)
         .timestamp((today + 2) * 1000)
-        .addAnnotation(Annotation.create((today + 1) * 1000, CLIENT_SEND, APP_ENDPOINT))
-        .addAnnotation(Annotation.create((today + 1) * 1000, SERVER_RECV, WEB_ENDPOINT))
-        .addAnnotation(Annotation.create((today + 1) * 1000, SERVER_SEND, WEB_ENDPOINT))
-        .addAnnotation(Annotation.create((today + 1) * 1000, CLIENT_RECV, APP_ENDPOINT))
-        .addAnnotation(Annotation.create((today + 1) * 1000, "app", APP_ENDPOINT))
+        .addAnnotation(Annotation.create((today + 2) * 1000, CLIENT_SEND, APP_ENDPOINT))
+        .addAnnotation(Annotation.create((today + 2) * 1000, SERVER_RECV, WEB_ENDPOINT))
+        .addAnnotation(Annotation.create((today + 2) * 1000, SERVER_SEND, WEB_ENDPOINT))
+        .addAnnotation(Annotation.create((today + 2) * 1000, CLIENT_RECV, APP_ENDPOINT))
+        .addAnnotation(Annotation.create((today + 2) * 1000, "app", APP_ENDPOINT))
         .addBinaryAnnotation(BinaryAnnotation.create("local", "app", APP_ENDPOINT))
         .addBinaryAnnotation(BinaryAnnotation.create("app-b", "app", APP_ENDPOINT))
         .build();
@@ -816,23 +816,21 @@ public void clientTimestampAndDurationWinInSharedSpan() {
   @Test
   public void traceWithManySpans() {
     Span[] trace = new Span[101];
-    trace[0] = TestObjects.TRACE.get(0);
-
-    IntStream.range(0, 100).forEach(i -> {
-      Span s = TestObjects.TRACE.get(1);
-      trace[i + 1] = s.toBuilder()
-          .id(s.id + i)
-          .timestamp(s.timestamp + i)
-          .annotations(s.annotations.stream()
-              .map(a -> Annotation.create(a.timestamp + i, a.value, a.endpoint))
-              .collect(toList()))
-          .build();
-    });
+    trace[0] = Span.builder().traceId(0xf66529c8cc356aa0L).id(0x93288b4644570496L).name("get")
+      .timestamp(TODAY * 1000).duration(350 * 1000L)
+      .addAnnotation(Annotation.create(TODAY * 1000, SERVER_RECV, WEB_ENDPOINT))
+      .addAnnotation(Annotation.create((TODAY + 350) * 1000, SERVER_SEND, WEB_ENDPOINT))
+      .build();
+
+    IntStream.range(1, trace.length).forEach(i ->
+      trace[i] = Span.builder().traceId(trace[0].traceId).parentId(trace[0].id).id(i).name("foo")
+        .timestamp((TODAY + i) * 1000).duration(10L)
+        .addBinaryAnnotation(BinaryAnnotation.create(LOCAL_COMPONENT, "", WEB_ENDPOINT))
+        .build());
 
     accept(trace);
 
-    String serviceName = trace[1].annotations.get(0).endpoint.serviceName;
-    assertThat(store().getTraces(QueryRequest.builder().serviceName(serviceName).build()))
+    assertThat(store().getTraces(QueryRequest.builder().build()))
         .containsExactly(asList(trace));
     assertThat(store().getTrace(trace[0].traceIdHigh, trace[0].traceId))
         .containsExactly(trace);