From 92d2dec5fafd6cdb74baf89b4137150ccd0cd35d Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Fri, 21 Jul 2017 10:56:44 +0800 Subject: [PATCH] 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 | 2 +- .../kafka/ZipkinKafkaCollectorProperties.java | 3 +- ...sticsearchAwsStorageAutoConfiguration.java | 1 - .../java/zipkin/junit/ZipkinRuleTest.java | 37 +++++++++++-------- .../zipkin/server/ZipkinHttpCollector.java | 3 -- .../cassandra/CassandraSpanStoreTest.java | 3 +- .../cassandra3/CassandraSpanStore.java | 1 - .../integration/CassandraSpanStoreTest.java | 4 -- .../CassandraStrictTraceIdFalseTest.java | 1 - .../cassandra3/integration/CassandraTest.java | 8 ++++ .../http/ElasticsearchHttpStorageTest.java | 1 - .../zipkin/storage/mysql/DSLContexts.java | 1 - zipkin/src/test/java/zipkin/TestObjects.java | 4 +- .../InMemorySpanStoreEvictionTest.java | 8 ++-- .../java/zipkin/storage/SpanStoreTest.java | 36 +++++++++--------- 15 files changed, 53 insertions(+), 60 deletions(-) diff --git a/pom.xml b/pom.xml index 67e0af497db..67f29922be7 100755 --- a/pom.xml +++ b/pom.xml @@ -70,7 +70,7 @@ 3.8.0 1.3 3.8.1 - 1.3.1 + 1.4.1 1.15 3.6.1 diff --git a/zipkin-autoconfigure/collector-kafka/src/main/java/zipkin/autoconfigure/collector/kafka/ZipkinKafkaCollectorProperties.java b/zipkin-autoconfigure/collector-kafka/src/main/java/zipkin/autoconfigure/collector/kafka/ZipkinKafkaCollectorProperties.java index 6d05b251ea9..d19446d6f4c 100644 --- a/zipkin-autoconfigure/collector-kafka/src/main/java/zipkin/autoconfigure/collector/kafka/ZipkinKafkaCollectorProperties.java +++ b/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; diff --git a/zipkin-autoconfigure/storage-elasticsearch-aws/src/main/java/zipkin/autoconfigure/storage/elasticsearch/aws/ZipkinElasticsearchAwsStorageAutoConfiguration.java b/zipkin-autoconfigure/storage-elasticsearch-aws/src/main/java/zipkin/autoconfigure/storage/elasticsearch/aws/ZipkinElasticsearchAwsStorageAutoConfiguration.java index dabec1151bb..84e201dc268 100644 --- a/zipkin-autoconfigure/storage-elasticsearch-aws/src/main/java/zipkin/autoconfigure/storage/elasticsearch/aws/ZipkinElasticsearchAwsStorageAutoConfiguration.java +++ b/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; diff --git a/zipkin-junit/src/test/java/zipkin/junit/ZipkinRuleTest.java b/zipkin-junit/src/test/java/zipkin/junit/ZipkinRuleTest.java index a9063149a8c..bbebf7e586b 100644 --- a/zipkin-junit/src/test/java/zipkin/junit/ZipkinRuleTest.java +++ b/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 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 trace = asList(TRACE.get(0).toBuilder().addAnnotation( - Annotation.create(System.currentTimeMillis(), new String(annotation2K), null)).build()); + List 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(); diff --git a/zipkin-server/src/main/java/zipkin/server/ZipkinHttpCollector.java b/zipkin-server/src/main/java/zipkin/server/ZipkinHttpCollector.java index 06eee61c1bb..57040aebedb 100644 --- a/zipkin-server/src/main/java/zipkin/server/ZipkinHttpCollector.java +++ b/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; diff --git a/zipkin-storage/cassandra/src/test/java/zipkin/storage/cassandra/CassandraSpanStoreTest.java b/zipkin-storage/cassandra/src/test/java/zipkin/storage/cassandra/CassandraSpanStoreTest.java index 1ce89143997..e2029768801 100644 --- a/zipkin-storage/cassandra/src/test/java/zipkin/storage/cassandra/CassandraSpanStoreTest.java +++ b/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; diff --git a/zipkin-storage/cassandra3/src/main/java/zipkin/storage/cassandra3/CassandraSpanStore.java b/zipkin-storage/cassandra3/src/main/java/zipkin/storage/cassandra3/CassandraSpanStore.java index 8ee0a3c7bbf..82312e3f8be 100644 --- a/zipkin-storage/cassandra3/src/main/java/zipkin/storage/cassandra3/CassandraSpanStore.java +++ b/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; diff --git a/zipkin-storage/cassandra3/src/test/java/zipkin/storage/cassandra3/integration/CassandraSpanStoreTest.java b/zipkin-storage/cassandra3/src/test/java/zipkin/storage/cassandra3/integration/CassandraSpanStoreTest.java index fc4943386ce..ef3da3e58dc 100644 --- a/zipkin-storage/cassandra3/src/test/java/zipkin/storage/cassandra3/integration/CassandraSpanStoreTest.java +++ b/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; diff --git a/zipkin-storage/cassandra3/src/test/java/zipkin/storage/cassandra3/integration/CassandraStrictTraceIdFalseTest.java b/zipkin-storage/cassandra3/src/test/java/zipkin/storage/cassandra3/integration/CassandraStrictTraceIdFalseTest.java index ce282557384..328f2492faa 100644 --- a/zipkin-storage/cassandra3/src/test/java/zipkin/storage/cassandra3/integration/CassandraStrictTraceIdFalseTest.java +++ b/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; diff --git a/zipkin-storage/cassandra3/src/test/java/zipkin/storage/cassandra3/integration/CassandraTest.java b/zipkin-storage/cassandra3/src/test/java/zipkin/storage/cassandra3/integration/CassandraTest.java index 5289a62963b..c731e97befd 100644 --- a/zipkin-storage/cassandra3/src/test/java/zipkin/storage/cassandra3/integration/CassandraTest.java +++ b/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(); + } + } } diff --git a/zipkin-storage/elasticsearch-http/src/test/java/zipkin/storage/elasticsearch/http/ElasticsearchHttpStorageTest.java b/zipkin-storage/elasticsearch-http/src/test/java/zipkin/storage/elasticsearch/http/ElasticsearchHttpStorageTest.java index d86019cf512..97f225c2176 100644 --- a/zipkin-storage/elasticsearch-http/src/test/java/zipkin/storage/elasticsearch/http/ElasticsearchHttpStorageTest.java +++ b/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; diff --git a/zipkin-storage/mysql/src/main/java/zipkin/storage/mysql/DSLContexts.java b/zipkin-storage/mysql/src/main/java/zipkin/storage/mysql/DSLContexts.java index d624ed14b5f..e976df3f73f 100644 --- a/zipkin-storage/mysql/src/main/java/zipkin/storage/mysql/DSLContexts.java +++ b/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; diff --git a/zipkin/src/test/java/zipkin/TestObjects.java b/zipkin/src/test/java/zipkin/TestObjects.java index 02af2266ab2..f26451df3ea 100644 --- a/zipkin/src/test/java/zipkin/TestObjects.java +++ b/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() diff --git a/zipkin/src/test/java/zipkin/storage/InMemorySpanStoreEvictionTest.java b/zipkin/src/test/java/zipkin/storage/InMemorySpanStoreEvictionTest.java index 0b02da358a4..b80db13c662 100644 --- a/zipkin/src/test/java/zipkin/storage/InMemorySpanStoreEvictionTest.java +++ b/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) diff --git a/zipkin/src/test/java/zipkin/storage/SpanStoreTest.java b/zipkin/src/test/java/zipkin/storage/SpanStoreTest.java index 2c1c280eefb..9a8143616bd 100755 --- a/zipkin/src/test/java/zipkin/storage/SpanStoreTest.java +++ b/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);