Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Jul 21, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
37 changes: 21 additions & 16 deletions zipkin-junit/src/test/java/zipkin/junit/ZipkinRuleTest.java
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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 {

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

Expand All @@ -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);
Expand All @@ -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()
Expand Down Expand Up @@ -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);
}

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 1 addition & 3 deletions zipkin/src/test/java/zipkin/TestObjects.java
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is invalid to put a CLIENT_ADDR on a client span. Scrubbed this as it confuses other things

.addBinaryAnnotation(BinaryAnnotation.address(SERVER_ADDR, DB_ENDPOINT))
.addBinaryAnnotation(BinaryAnnotation.create(ERROR, "\uD83D\uDCA9", NO_IP_ENDPOINT))
.build()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,14 +144,12 @@ public void evict_detailed() {

@Test
public void evict_oneTraceMultipleSpans() {
Span testSpan1 = span1.toBuilder().traceIdHigh(1L).traceId(123).
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test was accidentally testing defaults related to ignoring traceIdHigh.

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)
Expand Down
36 changes: 17 additions & 19 deletions zipkin/src/test/java/zipkin/storage/SpanStoreTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this data had incorrectly aligned timestamps (which wasn't the point of the test)

.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();
Expand Down Expand Up @@ -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);
Expand Down