HTRACE-111. HTrace Java client API fixes for 3.2 (cmccabe)
diff --git a/htrace-core/src/main/java/org/apache/htrace/Span.java b/htrace-core/src/main/java/org/apache/htrace/Span.java
index df28630..c6ec37b 100644
--- a/htrace-core/src/main/java/org/apache/htrace/Span.java
+++ b/htrace-core/src/main/java/org/apache/htrace/Span.java
@@ -35,8 +35,6 @@
*/
@JsonSerialize(using = Span.SpanSerializer.class)
public interface Span {
- public static final long ROOT_SPAN_ID = 0x74ace;
-
/**
* The block has completed, stop the clock
*/
@@ -88,10 +86,10 @@
String toString();
/**
- * Return the pseudo-unique (random) number of the first parent span, returns
- * ROOT_SPAN_ID if there are no parents.
+ * Returns the parents of the span.
+ * The array will be empty if there are no parents.
*/
- long getParentId();
+ long[] getParents();
/**
* Add a data annotation associated with this span
@@ -142,8 +140,8 @@
jgen.writeStringField("d", span.getDescription());
jgen.writeStringField("r", span.getProcessId());
jgen.writeArrayFieldStart("p");
- if (span.getParentId() != ROOT_SPAN_ID) {
- jgen.writeString(String.format("%016x", span.getParentId()));
+ for (long parent : span.getParents()) {
+ jgen.writeString(String.format("%016x", parent));
}
jgen.writeEndArray();
Map<String, String> traceInfoMap = span.getKVAnnotations();
diff --git a/htrace-core/src/main/java/org/apache/htrace/Trace.java b/htrace-core/src/main/java/org/apache/htrace/Trace.java
index 353be25..08e215a 100644
--- a/htrace-core/src/main/java/org/apache/htrace/Trace.java
+++ b/htrace-core/src/main/java/org/apache/htrace/Trace.java
@@ -23,8 +23,6 @@
import org.apache.htrace.wrappers.TraceCallable;
import org.apache.htrace.wrappers.TraceRunnable;
-import java.security.SecureRandom;
-import java.util.Random;
import java.util.concurrent.Callable;
/**
@@ -62,7 +60,6 @@
*/
public class Trace {
private static final Log LOG = LogFactory.getLog(Trace.class);
- private final static Random random = new SecureRandom();
/**
* Creates a new trace scope.
@@ -78,6 +75,19 @@
return startSpan(description, TrueIfTracingSampler.INSTANCE);
}
+ public static TraceScope startSpan(String description, TraceInfo tinfo) {
+ if (tinfo == null) return continueSpan(null);
+ Span newSpan = new MilliSpan.Builder().
+ begin(System.currentTimeMillis()).
+ end(0).
+ description(description).
+ traceId(tinfo.traceId).
+ parents(new long[] { tinfo.spanId }).
+ processId(Tracer.getProcessId()).
+ build();
+ return continueSpan(newSpan);
+ }
+
/**
* Creates a new trace scope.
*
diff --git a/htrace-core/src/main/java/org/apache/htrace/TraceTree.java b/htrace-core/src/main/java/org/apache/htrace/TraceTree.java
index 7d0c368..1f90c7f 100644
--- a/htrace-core/src/main/java/org/apache/htrace/TraceTree.java
+++ b/htrace-core/src/main/java/org/apache/htrace/TraceTree.java
@@ -21,11 +21,14 @@
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
+import java.util.Arrays;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
+import java.util.HashMap;
import java.util.Iterator;
+import java.util.LinkedList;
import java.util.List;
import java.util.TreeSet;
@@ -35,16 +38,16 @@
public class TraceTree {
public static final Log LOG = LogFactory.getLog(Tracer.class);
+
public static class SpansByParent {
+ /**
+ * Compare two spans by span ID.
+ */
private static Comparator<Span> COMPARATOR =
new Comparator<Span>() {
@Override
public int compare(Span a, Span b) {
- if (a.getParentId() < b.getParentId()) {
- return -1;
- } else if (a.getParentId() > b.getParentId()) {
- return 1;
- } else if (a.getSpanId() < b.getSpanId()) {
+ if (a.getSpanId() < b.getSpanId()) {
return -1;
} else if (a.getSpanId() > b.getSpanId()) {
return 1;
@@ -56,27 +59,37 @@
private final TreeSet<Span> treeSet;
+ private final HashMap<Long, LinkedList<Span>> parentToSpans;
+
SpansByParent(Collection<Span> spans) {
TreeSet<Span> treeSet = new TreeSet<Span>(COMPARATOR);
+ parentToSpans = new HashMap<Long, LinkedList<Span>>();
for (Span span : spans) {
treeSet.add(span);
+ for (long parent : span.getParents()) {
+ LinkedList<Span> list = parentToSpans.get(Long.valueOf(parent));
+ if (list == null) {
+ list = new LinkedList<Span>();
+ parentToSpans.put(Long.valueOf(parent), list);
+ }
+ list.add(span);
+ }
+ if (span.getParents().length == 0) {
+ LinkedList<Span> list = parentToSpans.get(Long.valueOf(0L));
+ if (list == null) {
+ list = new LinkedList<Span>();
+ parentToSpans.put(Long.valueOf(0L), list);
+ }
+ list.add(span);
+ }
}
this.treeSet = treeSet;
}
public List<Span> find(long parentId) {
- List<Span> spans = new ArrayList<Span>();
- Span span = new MilliSpan("", Long.MIN_VALUE,
- parentId, Long.MIN_VALUE, "");
- while (true) {
- span = treeSet.higher(span);
- if (span == null) {
- break;
- }
- if (span.getParentId() != parentId) {
- break;
- }
- spans.add(span);
+ LinkedList<Span> spans = parentToSpans.get(parentId);
+ if (spans == null) {
+ return new LinkedList<Span>();
}
return spans;
}
@@ -87,6 +100,9 @@
}
public static class SpansByProcessId {
+ /**
+ * Compare two spans by process ID, and then by span ID.
+ */
private static Comparator<Span> COMPARATOR =
new Comparator<Span>() {
@Override
@@ -116,8 +132,11 @@
public List<Span> find(String processId) {
List<Span> spans = new ArrayList<Span>();
- Span span = new MilliSpan("", Long.MIN_VALUE,
- Long.MIN_VALUE, Long.MIN_VALUE, processId);
+ Span span = new MilliSpan.Builder().
+ traceId(Long.MIN_VALUE).
+ spanId(Long.MIN_VALUE).
+ processId(processId).
+ build();
while (true) {
span = treeSet.higher(span);
if (span == null) {
@@ -143,8 +162,7 @@
* Create a new TraceTree
*
* @param spans The collection of spans to use to create this TraceTree. Should
- * have at least one root span (span with parentId =
- * Span.ROOT_SPAN_ID
+ * have at least one root span.
*/
public TraceTree(Collection<Span> spans) {
this.spansByParent = new SpansByParent(spans);
diff --git a/htrace-core/src/main/java/org/apache/htrace/Tracer.java b/htrace-core/src/main/java/org/apache/htrace/Tracer.java
index a214e5a..ddfd25c 100644
--- a/htrace-core/src/main/java/org/apache/htrace/Tracer.java
+++ b/htrace-core/src/main/java/org/apache/htrace/Tracer.java
@@ -20,7 +20,6 @@
import org.apache.commons.logging.LogFactory;
import org.apache.htrace.impl.MilliSpan;
-import java.security.SecureRandom;
import java.util.List;
import java.util.Random;
import java.util.concurrent.CopyOnWriteArrayList;
@@ -31,7 +30,16 @@
*/
public class Tracer {
public static final Log LOG = LogFactory.getLog(Tracer.class);
- private final static Random random = new SecureRandom();
+ private final static Random random = new Random();
+
+ private static long random64() {
+ long id;
+ do {
+ id = random.nextLong();
+ } while (id == 0);
+ return id;
+ }
+
private final List<SpanReceiver> receivers = new CopyOnWriteArrayList<SpanReceiver>();
private static final ThreadLocal<Span> currentSpan = new ThreadLocal<Span>() {
@Override
@@ -40,6 +48,7 @@
}
};
public static final TraceInfo DONT_TRACE = new TraceInfo(-1, -1);
+ private static final long EMPTY_PARENT_ARRAY[] = new long[0];
protected static String processId = null;
/**
@@ -68,11 +77,15 @@
protected Span createNew(String description) {
Span parent = currentSpan.get();
if (parent == null) {
- return new MilliSpan(description,
- /* traceId = */ random.nextLong(),
- /* parentSpanId = */ Span.ROOT_SPAN_ID,
- /* spanId = */ random.nextLong(),
- getProcessId());
+ return new MilliSpan.Builder().
+ begin(System.currentTimeMillis()).
+ end(0).
+ description(description).
+ traceId(random64()).
+ parents(EMPTY_PARENT_ARRAY).
+ spanId(random64()).
+ processId(getProcessId()).
+ build();
} else {
return parent.child(description);
}
diff --git a/htrace-core/src/main/java/org/apache/htrace/impl/MilliSpan.java b/htrace-core/src/main/java/org/apache/htrace/impl/MilliSpan.java
index 4467208..9babfc1 100644
--- a/htrace-core/src/main/java/org/apache/htrace/impl/MilliSpan.java
+++ b/htrace-core/src/main/java/org/apache/htrace/impl/MilliSpan.java
@@ -47,8 +47,6 @@
*/
@JsonDeserialize(using = MilliSpan.MilliSpanDeserializer.class)
public class MilliSpan implements Span {
-
- private static Random rand = new Random();
private static ObjectWriter JSON_WRITER = new ObjectMapper().writer();
private static final long EMPTY_PARENT_ARRAY[] = new long[0];
@@ -61,10 +59,27 @@
private Map<String, String> traceInfo = null;
private final String processId;
private List<TimelineAnnotation> timeline = null;
+ private final static Random random = new Random();
+
+ private static long random64() {
+ long id;
+ do {
+ id = random.nextLong();
+ } while (id == 0);
+ return id;
+ }
@Override
- public Span child(String description) {
- return new MilliSpan(description, traceId, spanId, rand.nextLong(), processId);
+ public Span child(String childDescription) {
+ return new MilliSpan.Builder().
+ begin(System.currentTimeMillis()).
+ end(0).
+ description(childDescription).
+ traceId(traceId).
+ parents(new long[] {spanId}).
+ spanId(random64()).
+ processId(processId).
+ build();
}
/**
@@ -155,20 +170,6 @@
this.timeline = builder.timeline;
}
- public MilliSpan(String description, long traceId, long parentSpanId, long spanId, String processId) {
- this.description = description;
- this.traceId = traceId;
- if (parentSpanId == Span.ROOT_SPAN_ID) {
- this.parents = new long[0];
- } else {
- this.parents = new long[] { parentSpanId };
- }
- this.spanId = spanId;
- this.begin = System.currentTimeMillis();
- this.end = 0;
- this.processId = processId;
- }
-
@Override
public synchronized void stop() {
if (end == 0) {
@@ -213,14 +214,9 @@
return spanId;
}
- // TODO: Fix API callers to deal with multiple parents, and get rid of
- // Span.ROOT_SPAN_ID.
@Override
- public long getParentId() {
- if (parents.length == 0) {
- return Span.ROOT_SPAN_ID;
- }
- return parents[0];
+ public long[] getParents() {
+ return parents;
}
@Override
diff --git a/htrace-core/src/test/java/org/apache/htrace/TestHTrace.java b/htrace-core/src/test/java/org/apache/htrace/TestHTrace.java
index 7b058d7..cd4eb28 100644
--- a/htrace-core/src/test/java/org/apache/htrace/TestHTrace.java
+++ b/htrace-core/src/test/java/org/apache/htrace/TestHTrace.java
@@ -77,7 +77,7 @@
Collection<Span> spans = psr.getSpans();
TraceTree traceTree = new TraceTree(spans);
- Collection<Span> roots = traceTree.getSpansByParent().find(Span.ROOT_SPAN_ID);
+ Collection<Span> roots = traceTree.getSpansByParent().find(0);
Assert.assertTrue("Trace tree must have roots", !roots.isEmpty());
Assert.assertEquals(numTraces, roots.size());
diff --git a/htrace-core/src/test/java/org/apache/htrace/impl/TestMilliSpan.java b/htrace-core/src/test/java/org/apache/htrace/impl/TestMilliSpan.java
index 7ad0482..857e9ac 100644
--- a/htrace-core/src/test/java/org/apache/htrace/impl/TestMilliSpan.java
+++ b/htrace-core/src/test/java/org/apache/htrace/impl/TestMilliSpan.java
@@ -19,12 +19,14 @@
import com.fasterxml.jackson.databind.ObjectMapper;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
import org.apache.htrace.Span;
import org.apache.htrace.TimelineAnnotation;
import org.junit.Test;
import java.security.SecureRandom;
+import java.util.Arrays;
import java.util.HashMap;
import java.util.Iterator;
import java.util.LinkedList;
@@ -40,7 +42,7 @@
assertEquals(expected.getTraceId(), got.getTraceId());
assertEquals(expected.getSpanId(), got.getSpanId());
assertEquals(expected.getProcessId(), got.getProcessId());
- assertEquals(expected.getParentId(), got.getParentId());
+ assertTrue(Arrays.equals(expected.getParents(), got.getParents()));
Map<String, String> expectedT = expected.getKVAnnotations();
Map<String, String> gotT = got.getKVAnnotations();
if (expectedT == null) {
diff --git a/htrace-flume/src/test/java/org/apache/htrace/impl/TestFlumeSpanReceiver.java b/htrace-flume/src/test/java/org/apache/htrace/impl/TestFlumeSpanReceiver.java
index a825690..f7d3840 100644
--- a/htrace-flume/src/test/java/org/apache/htrace/impl/TestFlumeSpanReceiver.java
+++ b/htrace-flume/src/test/java/org/apache/htrace/impl/TestFlumeSpanReceiver.java
@@ -72,7 +72,13 @@
startReceiver(null, avroHandler);
spans = new ArrayList<Span>();
- Span rootSpan = new MilliSpan(ROOT_SPAN_DESC, 1, Span.ROOT_SPAN_ID, 100, "test");
+ Span rootSpan = new MilliSpan.Builder().
+ description(ROOT_SPAN_DESC).
+ traceId(1).
+ spanId(100).
+ processId("test").
+ begin(System.currentTimeMillis()).
+ build();
Span innerOne = rootSpan.child("Some good work");
Span innerTwo = innerOne.child("Some more good work");
innerTwo.stop();
diff --git a/htrace-hbase/src/main/java/org/apache/htrace/impl/HBaseSpanReceiver.java b/htrace-hbase/src/main/java/org/apache/htrace/impl/HBaseSpanReceiver.java
index bb9c3a8..7a99366 100644
--- a/htrace-hbase/src/main/java/org/apache/htrace/impl/HBaseSpanReceiver.java
+++ b/htrace-hbase/src/main/java/org/apache/htrace/impl/HBaseSpanReceiver.java
@@ -207,12 +207,22 @@
for (Span span : dequeuedSpans) {
sbuilder.clear()
.setTraceId(span.getTraceId())
- .setParentId(span.getParentId())
.setStart(span.getStartTimeMillis())
.setStop(span.getStopTimeMillis())
.setSpanId(span.getSpanId())
.setProcessId(span.getProcessId())
.setDescription(span.getDescription());
+
+ if (span.getParents().length == 0) {
+ sbuilder.setParentId(0);
+ } else if (span.getParents().length > 0) {
+ sbuilder.setParentId(span.getParents()[0]);
+ if (span.getParents().length > 1) {
+ LOG.error("error: HBaseSpanReceiver does not support spans " +
+ "with multiple parents. Ignoring multiple parents for " +
+ span);
+ }
+ }
for (TimelineAnnotation ta : span.getTimelineAnnotations()) {
sbuilder.addTimeline(tlbuilder.clear()
.setTime(ta.getTime())
@@ -223,7 +233,7 @@
put.add(HBaseSpanReceiver.this.cf,
sbuilder.build().toByteArray(),
null);
- if (span.getParentId() == Span.ROOT_SPAN_ID) {
+ if (span.getParents().length == 0) {
put.add(HBaseSpanReceiver.this.icf,
INDEX_TIME_QUAL,
Bytes.toBytes(span.getStartTimeMillis()));
diff --git a/htrace-hbase/src/test/java/org/apache/htrace/impl/TestHBaseSpanReceiver.java b/htrace-hbase/src/test/java/org/apache/htrace/impl/TestHBaseSpanReceiver.java
index c990c18..79d6e9b 100644
--- a/htrace-hbase/src/test/java/org/apache/htrace/impl/TestHBaseSpanReceiver.java
+++ b/htrace-hbase/src/test/java/org/apache/htrace/impl/TestHBaseSpanReceiver.java
@@ -94,7 +94,7 @@
TraceTree traceTree = new TraceTree(spans);
Collection<Span> roots =
- traceTree.getSpansByParent().find(Span.ROOT_SPAN_ID);
+ traceTree.getSpansByParent().find(0);
Assert.assertTrue("Trace tree must have roots", !roots.isEmpty());
Assert.assertEquals(3, roots.size());
@@ -127,8 +127,7 @@
InputStream in = new ByteArrayInputStream(cell.getValueArray(),
cell.getValueOffset(),
cell.getValueLength());
- Assert.assertEquals(SpanProtos.Span.parseFrom(in).getParentId(),
- Span.ROOT_SPAN_ID);
+ Assert.assertEquals(SpanProtos.Span.parseFrom(in).getParentId(), 0);
}
}
} catch (IOException e) {
@@ -149,8 +148,10 @@
}
@Override
- public long getParentId() {
- return span.getParentId();
+ public long[] getParents() {
+ return (span.getParentId() == 0L) ?
+ (new long[] {}) :
+ (new long[] { span.getParentId() });
}
@Override
@@ -181,7 +182,7 @@
@Override
public String toString() {
return String.format("Span{Id:0x%16x,parentId:0x%16x,pid:%s,desc:%s}",
- getSpanId(), getParentId(),
+ getSpanId(), span.getParentId(),
getProcessId(), getDescription());
}
diff --git a/htrace-zipkin/src/main/java/org/apache/htrace/zipkin/HTraceToZipkinConverter.java b/htrace-zipkin/src/main/java/org/apache/htrace/zipkin/HTraceToZipkinConverter.java
index 8bd6442..4154d92 100644
--- a/htrace-zipkin/src/main/java/org/apache/htrace/zipkin/HTraceToZipkinConverter.java
+++ b/htrace-zipkin/src/main/java/org/apache/htrace/zipkin/HTraceToZipkinConverter.java
@@ -111,8 +111,12 @@
List<Annotation> annotationList = createZipkinAnnotations(hTraceSpan, ep);
List<BinaryAnnotation> binaryAnnotationList = createZipkinBinaryAnnotations(hTraceSpan, ep);
zipkinSpan.setTrace_id(hTraceSpan.getTraceId());
- if (hTraceSpan.getParentId() != org.apache.htrace.Span.ROOT_SPAN_ID) {
- zipkinSpan.setParent_id(hTraceSpan.getParentId());
+ if (hTraceSpan.getParents().length > 0) {
+ if (hTraceSpan.getParents().length > 1) {
+ LOG.error("zipkin doesn't support spans with multiple parents. Omitting " +
+ "other parents for " + hTraceSpan);
+ }
+ zipkinSpan.setParent_id(hTraceSpan.getParents()[0]);
}
zipkinSpan.setId(hTraceSpan.getSpanId());
zipkinSpan.setName(hTraceSpan.getDescription());
diff --git a/htrace-zipkin/src/test/java/org/apache/htrace/TestHTraceSpanToZipkinSpan.java b/htrace-zipkin/src/test/java/org/apache/htrace/TestHTraceSpanToZipkinSpan.java
index 07a9cea..de4a8cd 100644
--- a/htrace-zipkin/src/test/java/org/apache/htrace/TestHTraceSpanToZipkinSpan.java
+++ b/htrace-zipkin/src/test/java/org/apache/htrace/TestHTraceSpanToZipkinSpan.java
@@ -45,7 +45,14 @@
POJOSpanReceiver psr = new POJOSpanReceiver(HTraceConfiguration.EMPTY);
Trace.addReceiver(psr);
- Span rootSpan = new MilliSpan(ROOT_SPAN_DESC, 1, Span.ROOT_SPAN_ID, 100, "test");
+ Span rootSpan = new MilliSpan.Builder().
+ description(ROOT_SPAN_DESC).
+ traceId(1).
+ parents(new long[] { } ).
+ spanId(100).
+ processId("test").
+ begin(System.currentTimeMillis()).
+ build();
Span innerOne = rootSpan.child("Some good work");
Span innerTwo = innerOne.child("Some more good work");
innerTwo.stop();
@@ -66,7 +73,10 @@
String traceName = "testHTraceAnnotationTimestamp";
long startTime = System.currentTimeMillis() * 1000;
- Span ms = new MilliSpan(traceName, 1, Span.ROOT_SPAN_ID, 2, traceName);
+ Span ms = new MilliSpan.Builder().
+ description(traceName).traceId(1).parents(new long[] { }).
+ spanId(2).processId(traceName).begin(System.currentTimeMillis()).
+ build();
Thread.sleep(500);
long annoStartTime = System.currentTimeMillis() * 1000;
@@ -105,13 +115,20 @@
@Test
public void testHTraceDefaultPort() throws IOException {
- MilliSpan ms = new MilliSpan("test", 1, 2, 3, "hmaster");
+ MilliSpan ms = new MilliSpan.Builder().description("test").
+ traceId(1).parents(new long[] { 2 }).
+ spanId(3).processId("hmaster").
+ begin(System.currentTimeMillis()).build();
com.twitter.zipkin.gen.Span zs = new HTraceToZipkinConverter(12345, (short) -1).convert(ms);
for (com.twitter.zipkin.gen.Annotation annotation:zs.getAnnotations()) {
assertEquals((short)60000, annotation.getHost().getPort());
}
- ms = new MilliSpan("test", 1, 2, 3, "HregIonServer"); // make sure it's all lower cased
+ // make sure it's all lower cased
+ ms = new MilliSpan.Builder().description("test").traceId(1).
+ parents(new long[] {2}).spanId(3).
+ processId("HregIonServer").
+ begin(System.currentTimeMillis()).build();
zs = new HTraceToZipkinConverter(12345, (short) -1).convert(ms);
for (com.twitter.zipkin.gen.Annotation annotation:zs.getAnnotations()) {
assertEquals((short)60020, annotation.getHost().getPort());
@@ -120,8 +137,10 @@
private void assertSpansAreEquivalent(Span s, com.twitter.zipkin.gen.Span zs) {
assertEquals(s.getTraceId(), zs.getTrace_id());
- if (s.getParentId() != Span.ROOT_SPAN_ID) {
- assertEquals(s.getParentId(), zs.getParent_id());
+ assertTrue("zipkin doesn't support multiple parents to a single span.",
+ s.getParents().length <= 1);
+ if (s.getParents().length == 1) {
+ assertEquals(s.getParents()[0], zs.getParent_id());
}
assertEquals(s.getSpanId(), zs.getId());
Assert.assertNotNull(zs.getAnnotations());
