Changes v2 trace ID to hex

openzipkin · Aug 31, 2017 · 5bc51b1 · 5bc51b1
1 parent bbbc721
commit 5bc51b1
Show file tree

Hide file tree

Showing 28 changed files with 288 additions and 313 deletions.
diff --git a/benchmarks/src/main/java/zipkin/benchmarks/SpanBenchmarks.java b/benchmarks/src/main/java/zipkin/benchmarks/SpanBenchmarks.java
@@ -69,8 +69,10 @@ public zipkin.Span buildLocalSpan() {
         .build();
   }
 
-  static final long traceId = Util.lowerHexToUnsignedLong("86154a4ba6e91385");
-  static final long spanId = Util.lowerHexToUnsignedLong("4d1e00c0db9010db");
+  static final String traceIdHex = "86154a4ba6e91385";
+  static final String spanIdHex = "4d1e00c0db9010db";
+  static final long traceId = Util.lowerHexToUnsignedLong(traceIdHex);
+  static final long spanId = Util.lowerHexToUnsignedLong(spanIdHex);
   static final Endpoint frontend = Endpoint.create("frontend", 127 << 24 | 1);
   static final Endpoint backend = Endpoint.builder()
     .serviceName("backend")
@@ -113,9 +115,9 @@ public Span buildClientOnlySpan2() {
 
   static Span buildClientOnlySpan2(Span.Builder builder) {
     return builder
-      .traceId(traceId)
-      .parentId(traceId)
-      .id(spanId)
+      .traceId(traceIdHex)
+      .parentId(traceIdHex)
+      .id(spanIdHex)
       .name("get")
       .kind(Span.Kind.CLIENT)
       .localEndpoint(frontend)

diff --git a/zipkin-junit/src/main/java/zipkin/junit/ZipkinDispatcher.java b/zipkin-junit/src/main/java/zipkin/junit/ZipkinDispatcher.java
@@ -39,6 +39,7 @@
 import zipkin.storage.SpanStore;
 
 import static zipkin.internal.Util.lowerHexToUnsignedLong;
+import static zipkin.internal.v2.Span.normalizeTraceId;
 
 final class ZipkinDispatcher extends Dispatcher {
   static final long DEFAULT_LOOKBACK = 86400000L; // 1 day in millis
@@ -142,9 +143,7 @@ MockResponse queryV2(HttpUrl url) throws IOException {
       return jsonResponse(bout.toByteArray());
     } else if (url.encodedPath().startsWith("/api/v2/trace/")) {
       String traceIdHex = url.encodedPath().replace("/api/v2/trace/", "");
-      long traceIdHigh = traceIdHex.length() == 32 ? lowerHexToUnsignedLong(traceIdHex, 0) : 0L;
-      long traceIdLow = lowerHexToUnsignedLong(traceIdHex);
-      List<zipkin.internal.v2.Span> trace = store2.getTrace(traceIdHigh, traceIdLow).execute();
+      List<zipkin.internal.v2.Span> trace = store2.getTrace(normalizeTraceId(traceIdHex)).execute();
       if (!trace.isEmpty()) {
         ByteArrayOutputStream bout = new ByteArrayOutputStream();
         writeTrace(bout, trace);

diff --git a/zipkin-junit/src/test/java/zipkin/junit/v2/HttpV2SpanStore.java b/zipkin-junit/src/test/java/zipkin/junit/v2/HttpV2SpanStore.java
@@ -21,7 +21,6 @@
 import okhttp3.Request;
 import zipkin.Codec;
 import zipkin.DependencyLink;
-import zipkin.internal.Util;
 import zipkin.internal.v2.Call;
 import zipkin.internal.v2.Span;
 import zipkin.internal.v2.codec.Decoder;
@@ -50,10 +49,9 @@ final class HttpV2SpanStore implements SpanStore {
       content -> Decoder.JSON.decodeNestedList(content.readByteArray()));
   }
 
-  @Override public Call<List<Span>> getTrace(long traceIdHigh, long traceIdLow) {
-    String traceIdHex = Util.toLowerHex(traceIdHigh, traceIdLow);
+  @Override public Call<List<Span>> getTrace(String traceId) {
     return factory.newCall(new Request.Builder()
-      .url(factory.baseUrl.resolve("/api/v2/trace/" + traceIdHex))
+      .url(factory.baseUrl.resolve("/api/v2/trace/" + Span.normalizeTraceId(traceId)))
       .build(), content -> Decoder.JSON.decodeList(content.readByteArray()))
       .handleError(((error, callback) -> {
         if (error instanceof HttpException && ((HttpException) error).code == 404) {

diff --git a/zipkin-server/src/main/java/zipkin/server/ZipkinQueryApiV2.java b/zipkin-server/src/main/java/zipkin/server/ZipkinQueryApiV2.java
@@ -42,7 +42,6 @@
 import zipkin.storage.StorageComponent;
 
 import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE;
-import static zipkin.internal.Util.lowerHexToUnsignedLong;
 
 @RestController
 @RequestMapping("/api/v2")
@@ -147,12 +146,8 @@ public String getTraces(
   public String getTrace(@PathVariable String traceIdHex, WebRequest request) throws IOException {
     if (storage == null) throw new Version2StorageNotConfigured();
 
-    long traceIdHigh = traceIdHex.length() == 32 ? lowerHexToUnsignedLong(traceIdHex, 0) : 0L;
-    long traceIdLow = lowerHexToUnsignedLong(traceIdHex);
-    List<Span> trace = storage.v2SpanStore().getTrace(traceIdHigh, traceIdLow).execute();
-    if (trace.isEmpty()) {
-      throw new TraceNotFoundException(traceIdHex, traceIdHigh, traceIdLow);
-    }
+    List<Span> trace = storage.v2SpanStore().getTrace(traceIdHex).execute();
+    if (trace.isEmpty()) throw new TraceNotFoundException(traceIdHex);
     Buffer buffer = new Buffer();
     buffer.writeByte('[');
     for (int i = 0, length = trace.size(); i < length; ) {
@@ -181,9 +176,8 @@ public void notFound() {
   }
 
   static class TraceNotFoundException extends RuntimeException {
-    TraceNotFoundException(String traceIdHex, long traceIdHigh, long traceId) {
-      super(String.format("Cannot find trace for id=%s, parsed value=%s", traceIdHex,
-        traceIdHigh != 0 ? traceIdHigh + "," + traceId : traceId));
+    TraceNotFoundException(String traceIdHex) {
+      super("Cannot find trace " + traceIdHex);
     }
   }
 

diff --git a/...arch-http/src/main/java/zipkin/storage/elasticsearch/http/ElasticsearchHttpSpanStore.java b/...arch-http/src/main/java/zipkin/storage/elasticsearch/http/ElasticsearchHttpSpanStore.java
@@ -23,8 +23,6 @@
 import java.util.Locale;
 import java.util.Map;
 import zipkin.DependencyLink;
-import zipkin.internal.Pair;
-import zipkin.internal.Util;
 import zipkin.internal.v2.Call;
 import zipkin.internal.v2.Span;
 import zipkin.internal.v2.storage.QueryRequest;
@@ -92,14 +90,14 @@ final class ElasticsearchHttpSpanStore implements SpanStore {
     // be no significant difference in user experience since span start times are usually very
     // close to each other in human time.
     Aggregation traceIdTimestamp = Aggregation.terms("traceId", request.limit())
-        .addSubAggregation(Aggregation.min("timestamp_millis"))
-        .orderBy("timestamp_millis", "desc");
+      .addSubAggregation(Aggregation.min("timestamp_millis"))
+      .orderBy("timestamp_millis", "desc");
 
     List<String> indices = indexNameFormatter.formatTypeAndRange(SPAN, beginMillis, endMillis);
     if (indices.isEmpty()) return Call.emptyList();
 
     SearchRequest esRequest = SearchRequest.create(indices)
-        .filters(filters).addAggregation(traceIdTimestamp);
+      .filters(filters).addAggregation(traceIdTimestamp);
 
     HttpCall<List<String>> traceIdsCall = search.newCall(esRequest, BodyConverters.SORTED_KEYS);
 
@@ -111,7 +109,7 @@ final class ElasticsearchHttpSpanStore implements SpanStore {
       // Due to tokenization of the trace ID, our matches are imprecise on Span.traceIdHigh
       for (Iterator<List<Span>> trace = traces.iterator(); trace.hasNext(); ) {
         List<Span> next = trace.next();
-        if (next.get(0).traceIdHigh() != 0 && !request.test(next)) {
+        if (next.get(0).traceId().length() > 16 && !request.test(next)) {
           trace.remove();
         }
       }
@@ -126,12 +124,14 @@ final class ElasticsearchHttpSpanStore implements SpanStore {
     });
   }
 
-  @Override public Call<List<Span>> getTrace(long traceIdHigh, long traceIdLow) {
-    String traceIdHex = Util.toLowerHex(strictTraceId ? traceIdHigh : 0L, traceIdLow);
+  @Override public Call<List<Span>> getTrace(String traceId) {
+    // make sure we have a 16 or 32 character trace ID
+    traceId = Span.normalizeTraceId(traceId);
 
-    SearchRequest request = SearchRequest.create(asList(allSpanIndices))
-        .term("traceId", traceIdHex);
+    // Unless we are strict, truncate the trace ID to 64bit (encoded as 16 characters)
+    if (!strictTraceId && traceId.length() == 32) traceId = traceId.substring(16);
 
+    SearchRequest request = SearchRequest.create(asList(allSpanIndices)).term("traceId", traceId);
     return search.newCall(request, BodyConverters.SPANS);
   }
 
@@ -147,9 +147,9 @@ final class ElasticsearchHttpSpanStore implements SpanStore {
     SearchRequest.Filters filters = new SearchRequest.Filters();
     filters.addRange("timestamp_millis", beginMillis, endMillis);
     SearchRequest request = SearchRequest.create(indices)
-        .filters(filters)
-        .addAggregation(Aggregation.terms("localEndpoint.serviceName", Integer.MAX_VALUE))
-        .addAggregation(Aggregation.terms("remoteEndpoint.serviceName", Integer.MAX_VALUE));
+      .filters(filters)
+      .addAggregation(Aggregation.terms("localEndpoint.serviceName", Integer.MAX_VALUE))
+      .addAggregation(Aggregation.terms("remoteEndpoint.serviceName", Integer.MAX_VALUE));
     return search.newCall(request, BodyConverters.SORTED_KEYS);
   }
 
@@ -164,12 +164,12 @@ final class ElasticsearchHttpSpanStore implements SpanStore {
 
     // A span name is only valid on a local endpoint, as a span name is defined locally
     SearchRequest.Filters filters = new SearchRequest.Filters()
-        .addRange("timestamp_millis", beginMillis, endMillis)
-        .addTerm("localEndpoint.serviceName", serviceName.toLowerCase(Locale.ROOT));
+      .addRange("timestamp_millis", beginMillis, endMillis)
+      .addTerm("localEndpoint.serviceName", serviceName.toLowerCase(Locale.ROOT));
 
     SearchRequest request = SearchRequest.create(indices)
-        .filters(filters)
-        .addAggregation(Aggregation.terms("name", Integer.MAX_VALUE));
+      .filters(filters)
+      .addAggregation(Aggregation.terms("name", Integer.MAX_VALUE));
 
     return search.newCall(request, BodyConverters.SORTED_KEYS);
   }
@@ -188,9 +188,11 @@ final class ElasticsearchHttpSpanStore implements SpanStore {
   static List<List<Span>> groupByTraceId(Collection<Span> input, boolean strictTraceId) {
     if (input.isEmpty()) return Collections.emptyList();
 
-    Map<Pair<Long>, List<Span>> groupedByTraceId = new LinkedHashMap<>();
+    Map<String, List<Span>> groupedByTraceId = new LinkedHashMap<>();
     for (Span span : input) {
-      Pair<Long> traceId = Pair.create(strictTraceId ? span.traceIdHigh() : 0L, span.traceId());
+      String traceId = strictTraceId || span.traceId().length() == 16
+        ? span.traceId()
+        : span.traceId().substring(16);
       if (!groupedByTraceId.containsKey(traceId)) {
         groupedByTraceId.put(traceId, new LinkedList<>());
       }

diff --git a/...search-http/src/main/java/zipkin/storage/elasticsearch/http/ElasticsearchHttpStorage.java b/...search-http/src/main/java/zipkin/storage/elasticsearch/http/ElasticsearchHttpStorage.java
@@ -73,6 +73,8 @@ public static Builder builder() {
     return result;
   }
 
+  abstract Builder toBuilder();
+
   @AutoValue.Builder
   public static abstract class Builder implements zipkin.storage.StorageComponent.Builder {
     abstract Builder client(OkHttpClient client);

diff --git a/...tp/src/test/java/zipkin/storage/elasticsearch/http/ElasticsearchHttpSpanConsumerTest.java b/...tp/src/test/java/zipkin/storage/elasticsearch/http/ElasticsearchHttpSpanConsumerTest.java
@@ -62,7 +62,7 @@ public void close() throws IOException {
   @Test public void addsTimestamp_millisIntoJson() throws Exception {
     es.enqueue(new MockResponse());
 
-    Span span = Span.builder().traceId(20L).id(20L).name("get")
+    Span span = Span.builder().traceId("20").id("20").name("get")
       .timestamp(TODAY * 1000).build();
 
     accept(span);
@@ -72,7 +72,7 @@ public void close() throws IOException {
   }
 
   @Test public void prefixWithTimestampMillisAndQuery_skipsWhenNoData() throws Exception {
-    Span span = Span.builder().traceId(20L).id(22L).name("").parentId(21L).timestamp(0L)
+    Span span = Span.builder().traceId("20").id("22").name("").parentId("21").timestamp(0L)
       .localEndpoint(TestObjects.WEB_ENDPOINT)
       .kind(Kind.CLIENT)
       .build();
@@ -84,7 +84,7 @@ public void close() throws IOException {
   }
 
   @Test public void prefixWithTimestampMillisAndQuery_addsTimestampMillis() throws Exception {
-    Span span = Span.builder().traceId(20L).id(22L).name("").parentId(21L).timestamp(1L)
+    Span span = Span.builder().traceId("20").id("22").name("").parentId("21").timestamp(1L)
       .localEndpoint(TestObjects.WEB_ENDPOINT)
       .kind(Kind.CLIENT)
       .build();
@@ -96,7 +96,7 @@ public void close() throws IOException {
   }
 
   @Test public void prefixWithTimestampMillisAndQuery_addsAnnotationQuery() throws Exception {
-    Span span = Span.builder().traceId(20L).id(22L).name("").parentId(21L)
+    Span span = Span.builder().traceId("20").id("22").name("").parentId("21")
       .localEndpoint(TestObjects.WEB_ENDPOINT)
       .addAnnotation(1L, "\"foo")
       .build();
@@ -108,7 +108,7 @@ public void close() throws IOException {
   }
 
   @Test public void prefixWithTimestampMillisAndQuery_addsAnnotationQueryTags() throws Exception {
-    Span span = Span.builder().traceId(20L).id(22L).name("").parentId(21L)
+    Span span = Span.builder().traceId("20").id("22").name("").parentId("21")
       .localEndpoint(TestObjects.WEB_ENDPOINT)
       .putTag("\"foo", "\"bar")
       .build();
@@ -120,7 +120,7 @@ public void close() throws IOException {
   }
 
   @Test public void prefixWithTimestampMillisAndQuery_readable() throws Exception {
-    Span span = Span.builder().traceId(20L).id(20L).name("get")
+    Span span = Span.builder().traceId("20").id("20").name("get")
       .timestamp(TODAY * 1000).build();
 
     byte[] message = MessageEncoder.JSON_BYTES.encode(asList(
@@ -134,7 +134,7 @@ public void close() throws IOException {
   @Test public void doesntWriteDocumentId() throws Exception {
     es.enqueue(new MockResponse());
 
-    accept(Span.builder().traceId(1L).id(1L).name("foo").build());
+    accept(Span.builder().traceId("1").id("1").name("foo").build());
 
     RecordedRequest request = es.takeRequest();
     assertThat(request.getBody().readByteString().utf8())
@@ -144,8 +144,8 @@ public void close() throws IOException {
   @Test public void writesSpanNaturallyWhenNoTimestamp() throws Exception {
     es.enqueue(new MockResponse());
 
-    Span span = Span.builder().traceId(1L).id(1L).name("foo").build();
-    accept(Span.builder().traceId(1L).id(1L).name("foo").build());
+    Span span = Span.builder().traceId("1").id("1").name("foo").build();
+    accept(Span.builder().traceId("1").id("1").name("foo").build());
 
     assertThat(es.takeRequest().getBody().readByteString().utf8())
       .contains("\n" + new String(Encoder.JSON.encode(span), UTF_8) + "\n");
@@ -154,13 +154,13 @@ public void close() throws IOException {
   @Test public void traceIsSearchableByServerServiceName() throws Exception {
     es.enqueue(new MockResponse());
 
-    Span clientSpan = Span.builder().traceId(20L).id(22L).name("").parentId(21L)
+    Span clientSpan = Span.builder().traceId("20").id("22").name("").parentId("21")
       .timestamp(1000L)
       .kind(Kind.CLIENT)
       .localEndpoint(TestObjects.WEB_ENDPOINT)
       .build();
 
-    Span serverSpan = Span.builder().traceId(20L).id(22L).name("get").parentId(21L)
+    Span serverSpan = Span.builder().traceId("20").id("22").name("get").parentId("21")
       .timestamp(2000L)
       .kind(Kind.SERVER)
       .localEndpoint(TestObjects.APP_ENDPOINT)
@@ -185,7 +185,7 @@ public void close() throws IOException {
 
     es.enqueue(new MockResponse());
 
-    accept(Span.builder().traceId(1L).id(1L).name("foo").build());
+    accept(Span.builder().traceId("1").id("1").name("foo").build());
 
     RecordedRequest request = es.takeRequest();
     assertThat(request.getPath())
@@ -195,7 +195,7 @@ public void close() throws IOException {
   @Test public void choosesTypeSpecificIndex() throws Exception {
     es.enqueue(new MockResponse());
 
-    Span span = Span.builder().traceId(1L).id(2L).parentId(1L).name("s")
+    Span span = Span.builder().traceId("1").id("2").parentId("1").name("s")
       .localEndpoint(TestObjects.APP_ENDPOINT)
       .addAnnotation(TimeUnit.DAYS.toMicros(365) /* 1971-01-01 */, "foo")
       .build();

diff --git a/...-http/src/test/java/zipkin/storage/elasticsearch/http/ElasticsearchHttpSpanStoreTest.java b/...-http/src/test/java/zipkin/storage/elasticsearch/http/ElasticsearchHttpSpanStoreTest.java
@@ -41,6 +41,25 @@ public class ElasticsearchHttpSpanStoreTest {
     storage.close();
   }
 
+  @Test public void doesntTruncateTraceIdByDefault() throws Exception {
+    es.enqueue(new MockResponse());
+    spanStore.getTrace("48fec942f3e78b893041d36dc43227fd").execute();
+
+    assertThat(es.takeRequest().getBody().readUtf8())
+      .contains("\"traceId\":\"48fec942f3e78b893041d36dc43227fd\"");
+  }
+
+  @Test public void truncatesTraceIdTo16CharsWhenNotStrict() throws Exception {
+    storage = storage.toBuilder().strictTraceId(false).build();
+    spanStore = new ElasticsearchHttpSpanStore(storage);
+
+    es.enqueue(new MockResponse());
+    spanStore.getTrace("48fec942f3e78b893041d36dc43227fd").execute();
+
+    assertThat(es.takeRequest().getBody().readUtf8())
+      .contains("\"traceId\":\"3041d36dc43227fd\"");
+  }
+
   @Test public void serviceNames_defaultsTo24HrsAgo_6x() throws Exception {
     es.enqueue(new MockResponse().setBody(SERVICE_NAMES));
     spanStore.getServiceNames().execute();

diff --git a/.../elasticsearch-http/src/test/java/zipkin/storage/elasticsearch/http/JsonAdaptersTest.java b/.../elasticsearch-http/src/test/java/zipkin/storage/elasticsearch/http/JsonAdaptersTest.java
@@ -21,7 +21,6 @@
 import zipkin.Endpoint;
 import zipkin.TestObjects;
 import zipkin.internal.ApplyTimestampAndDuration;
-import zipkin.internal.Util;
 import zipkin.internal.V2SpanConverter;
 import zipkin.internal.v2.Span;
 import zipkin.internal.v2.codec.Encoder;
@@ -147,7 +146,7 @@ public void span_roundTrip() throws IOException {
   public void span_specialCharsInJson() throws IOException {
     // service name is surrounded by control characters
     Endpoint e = Endpoint.create(new String(new char[] {0, 'a', 1}), 0);
-    Span worstSpanInTheWorld = Span.builder().traceId(1L).id(1L)
+    Span worstSpanInTheWorld = Span.builder().traceId("1").id("1")
       // name is terrible
       .name(new String(new char[] {'"', '\\', '\t', '\b', '\n', '\r', '\f'}))
       .localEndpoint(e)
@@ -226,7 +225,7 @@ public void span_readsTraceIdHighFromTraceIdField() throws IOException {
 
     assertThat(JsonAdapters.SPAN_ADAPTER.fromJson(with128BitTraceId))
       .isEqualTo(JsonAdapters.SPAN_ADAPTER.fromJson(withLower64bitsTraceId).toBuilder()
-        .traceIdHigh(Util.lowerHexToUnsignedLong("48485a3953bb6124")).build());
+        .traceId("48485a3953bb61246b221d5bc9e6496c").build());
   }
 
   @Test