Skip to content

Commit

Permalink
Ports remaining scala SpanStoreSpec tests to SpanStoreTest
Browse files Browse the repository at this point in the history 
This ports remaining tests to java, and also ports code changes needed
to make them pass.
  • Loading branch information
Adrian Cole committed Mar 21, 2016
1 parent b776ac8 commit 9eef587
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 57 deletions.
8 changes: 5 additions & 3 deletions zipkin-spanstores/jdbc/src/main/java/zipkin/jdbc/JDBCSpanStore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ public void accept(List<Span> spans) {
List<Query> inserts = new ArrayList<>();

for (Span span : spans) {
Long authoritativeTimestamp = span.timestamp;
span = ApplyTimestampAndDuration.apply(span);
Long binaryAnnotationTimestamp = span.timestamp;
if (binaryAnnotationTimestamp == null) { // fallback if we have no timestamp, yet
Expand All @@ -109,8 +110,8 @@ public void accept(List<Span> spans) {
if (!span.name.equals("") && !span.name.equals("unknown")) {
updateFields.put(ZIPKIN_SPANS.NAME, span.name);
}
if (span.timestamp != null) {
updateFields.put(ZIPKIN_SPANS.START_TS, span.timestamp);
if (authoritativeTimestamp != null) {
updateFields.put(ZIPKIN_SPANS.START_TS, authoritativeTimestamp);
}
if (span.duration != null) {
updateFields.put(ZIPKIN_SPANS.DURATION, span.duration);
Expand Down Expand Up @@ -229,7 +230,8 @@ List<List<Span>> getTraces(@Nullable QueryRequest request, @Nullable Long traceI
}
}
}
trace.add(span.build());
Span rawSpan = span.build();
trace.add(raw ? rawSpan : ApplyTimestampAndDuration.apply(rawSpan));
}
if (!raw) trace = CorrectForClockSkew.apply(trace);
result.add(trace);
Expand Down
49 changes: 21 additions & 28 deletions zipkin/src/main/java/zipkin/internal/ApplyTimestampAndDuration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
*/
package zipkin.internal;

import java.util.List;
import zipkin.Annotation;
import zipkin.Constants;
import zipkin.Span;
Expand All @@ -30,39 +29,33 @@ public class ApplyTimestampAndDuration {
// For spans that core client annotations, the distance between "cs" and "cr" should be the
// authoritative duration. We are special-casing this to avoid setting incorrect duration
// when there's skew between the client and the server.
public static Span apply(Span s) {
if ((s.timestamp == null || s.duration == null) && !s.annotations.isEmpty()) {
Long ts = s.timestamp;
Long dur = s.duration;
ts = ts != null ? ts : getFirstTimestamp(s.annotations);
if (dur == null) {
long lastTs = getLastTimestamp(s.annotations);
if (ts != lastTs) {
dur = lastTs - ts;
}
}
return new Span.Builder(s).timestamp(ts).duration(dur).build();
public static Span apply(Span span) {
// Don't overwrite authoritatively set timestamp and duration!
if (span.timestamp != null && span.duration != null) {
return span;
}
return s;
}

static long getFirstTimestamp(List<Annotation> annotations) {
for (int i = 0, length = annotations.size(); i < length; i++) {
if (annotations.get(i).value.equals(Constants.CLIENT_SEND)) {
return annotations.get(i).timestamp;
}
// Only calculate span.timestamp and duration on complete spans. This avoids
// persisting an inaccurate timestamp due to a late arriving annotation.
if (span.annotations.size() < 2) {
return span;
}
return annotations.get(0).timestamp;
}

static long getLastTimestamp(List<Annotation> annotations) {
int length = annotations.size();
for (int i = 0; i < length; i++) {
if (annotations.get(i).value.equals(Constants.CLIENT_RECV)) {
return annotations.get(i).timestamp;
// For spans that core client annotations, the distance between "cs" and "cr" should be the
// authoritative duration. We are special-casing this to avoid setting incorrect duration
// when there's skew between the client and the server.
Long first = span.annotations.get(0).timestamp;
Long last = span.annotations.get(span.annotations.size() - 1).timestamp;
for (Annotation annotation : span.annotations) {
if (annotation.value.equals(Constants.CLIENT_SEND)) {
first = annotation.timestamp;
} else if (annotation.value.equals(Constants.CLIENT_RECV)) {
last = annotation.timestamp;
}
}
return annotations.get(length - 1).timestamp;
long ts = span.timestamp != null ? span.timestamp : first;
Long dur = span.duration != null ? span.duration : last.equals(first) ? null : last - first;
return new Span.Builder(span).timestamp(ts).duration(dur).build();
}

private ApplyTimestampAndDuration() {
Expand Down
102 changes: 76 additions & 26 deletions zipkin/src/test/java/zipkin/SpanStoreTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,11 @@
import static java.util.Arrays.asList;
import static java.util.Collections.emptyList;
import static org.assertj.core.api.Assertions.assertThat;
import static zipkin.Constants.CLIENT_RECV;
import static zipkin.Constants.CLIENT_SEND;
import static zipkin.Constants.LOCAL_COMPONENT;
import static zipkin.Constants.SERVER_RECV;
import static zipkin.Constants.SERVER_SEND;

/**
* Base test for {@link SpanStore} implementations. Subtypes should create a connection to a real
Expand Down Expand Up @@ -458,21 +462,21 @@ public void correctsClockSkew() {
.traceId(1)
.name("method1")
.id(666)
.addAnnotation(Annotation.create((today + 100) * 1000, Constants.CLIENT_SEND, client))
.addAnnotation(Annotation.create((today + 95) * 1000, Constants.SERVER_RECV, frontend)) // before client sends
.addAnnotation(Annotation.create((today + 120) * 1000, Constants.SERVER_SEND, frontend)) // before client receives
.addAnnotation(Annotation.create((today + 135) * 1000, Constants.CLIENT_RECV, client)).build();
.addAnnotation(Annotation.create((today + 100) * 1000, CLIENT_SEND, client))
.addAnnotation(Annotation.create((today + 95) * 1000, SERVER_RECV, frontend)) // before client sends
.addAnnotation(Annotation.create((today + 120) * 1000, SERVER_SEND, frontend)) // before client receives
.addAnnotation(Annotation.create((today + 135) * 1000, CLIENT_RECV, client)).build();

/** Intentionally not setting span.timestamp, duration */
Span remoteChild = new Span.Builder()
.traceId(1)
.name("method2")
.id(777)
.parentId(666L)
.addAnnotation(Annotation.create((today + 100) * 1000, Constants.CLIENT_SEND, frontend))
.addAnnotation(Annotation.create((today + 115) * 1000, Constants.SERVER_RECV, backend))
.addAnnotation(Annotation.create((today + 120) * 1000, Constants.SERVER_SEND, backend))
.addAnnotation(Annotation.create((today + 115) * 1000, Constants.CLIENT_RECV, frontend)) // before server sent
.addAnnotation(Annotation.create((today + 100) * 1000, CLIENT_SEND, frontend))
.addAnnotation(Annotation.create((today + 115) * 1000, SERVER_RECV, backend))
.addAnnotation(Annotation.create((today + 120) * 1000, SERVER_SEND, backend))
.addAnnotation(Annotation.create((today + 115) * 1000, CLIENT_RECV, frontend)) // before server sent
.build();

/** Local spans must explicitly set timestamp */
Expand Down Expand Up @@ -510,31 +514,77 @@ public void correctsClockSkew() {
assertThat(adjusted.get(2).duration).isEqualTo(skewed.get(2).duration);
}

/**
* This test shows that regardless of whether span.timestamp and duration are set directly or
* derived from annotations, the client wins vs the server. This is important because the client
* holds the critical path of a shared span.
*/
@Test
public void rawTrace(){
public void clientTimestampAndDurationWinInSharedSpan() {
Endpoint client = Endpoint.create("client", 192 << 24 | 168 << 16 | 1, 8080);
Endpoint server = Endpoint.create("server", 192 << 24 | 168 << 16 | 2, 8080);

long clientTimestamp = (today + 100) * 1000;
long clientDuration = 35 * 1000;

// both client and server set span.timestamp, duration
Span clientView = new Span.Builder().traceId(1).name("direct").id(666)
.timestamp(clientTimestamp).duration(clientDuration)
.addAnnotation(Annotation.create((today + 100) * 1000, CLIENT_SEND, client))
.addAnnotation(Annotation.create((today + 135) * 1000, CLIENT_RECV, client))
.build();

Span serverView = new Span.Builder().traceId(1).name("direct").id(666)
.timestamp((today + 105) * 1000).duration(25 * 1000L)
.addAnnotation(Annotation.create((today + 105) * 1000, SERVER_RECV, server))
.addAnnotation(Annotation.create((today + 130) * 1000, SERVER_SEND, server))
.build();

// neither client, nor server set span.timestamp, duration
Span clientViewDerived = new Span.Builder().traceId(1).name("derived").id(666)
.addAnnotation(Annotation.create(clientTimestamp, CLIENT_SEND, client))
.addAnnotation(Annotation.create(clientTimestamp + clientDuration, CLIENT_SEND, client))
.build();

Span serverViewDerived = new Span.Builder().traceId(1).name("derived").id(666)
.addAnnotation(Annotation.create((today + 105) * 1000, SERVER_RECV, server))
.addAnnotation(Annotation.create((today + 130) * 1000, SERVER_SEND, server))
.build();

store.accept(asList(serverView, serverViewDerived)); // server span hits the collection tier first
store.accept(asList(clientView, clientViewDerived)); // intentionally different collection event

for (Span span : store.getTrace(clientView.traceId)) {
assertThat(span.timestamp).isEqualTo(clientTimestamp);
assertThat(span.duration).isEqualTo(clientDuration);
}
}

// This supports the "raw trace" feature, which skips application-level data cleaning
@Test
public void rawTrace_doesntPerformQueryTimeAdjustment() {
Endpoint frontend = Endpoint.create("frontend", 192 << 24 | 168 << 16 | 2, 8080);
Annotation sr = Annotation.create((today + 95) * 1000, SERVER_RECV, frontend);
Annotation ss = Annotation.create((today + 100) * 1000, SERVER_SEND, frontend);

Span span = new Span.Builder()
.traceId(1)
.name("method1")
.id(666)
.timestamp((today + 95) * 1000)
.duration(20 * 1000L)
.addAnnotation(Annotation.create((today + 100) * 1000, Constants.CLIENT_SEND, client))
.addAnnotation(Annotation.create((today + 95) * 1000, Constants.SERVER_RECV, frontend)) // before client sends
.addAnnotation(Annotation.create((today + 120) * 1000, Constants.SERVER_SEND, frontend)) // before client receives
.addAnnotation(Annotation.create((today + 135) * 1000, Constants.CLIENT_RECV, client)).build();
Span span = new Span.Builder().traceId(1).name("method1").id(666).build();

store.accept(asList(span));
// Simulate instrumentation that sends annotations one at-a-time.
// This should prevent the collection tier from being able to calculate duration.
store.accept(asList(new Span.Builder(span).addAnnotation(sr).build()));
store.accept(asList(new Span.Builder(span).addAnnotation(ss).build()));

// clock skew adjustment will affect the span returned by default
// Normally, span store implementations will merge spans by id and add duration by query time
assertThat(store.getTrace(span.traceId))
.doesNotContain(span);

// the raw trace does not include any adjustments at query time, such as clock skew
assertThat(store.getRawTrace(span.traceId))
.containsExactly(span);
.containsExactly(new Span.Builder(span)
.timestamp(sr.timestamp)
.duration(ss.timestamp - sr.timestamp)
.annotations(asList(sr, ss)).build());

// Since a collector never saw both sides of the span, we'd not see duration in the raw trace.
for (Span raw : store.getRawTrace(span.traceId)) {
assertThat(raw.duration).isNull();
}
}

static long clientDuration(Span span) {
Expand Down

0 comments on commit 9eef587

Please sign in to comment.