Tolerate 128bit ids in json and http api by throwing out high bits (#…

…1296) This bridges support for 128bit trace ids by not failing on their receipt. Basically, this throws out any hex characters to the left of the 16 needed for the 64bit trace id.
openzipkin · Sep 15, 2016 · 90342b4 · 90342b4
1 parent f223b87
commit 90342b4
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 18 deletions.
diff --git a/zipkin-server/src/test/java/zipkin/server/ZipkinServerIntegrationTests.java b/zipkin-server/src/test/java/zipkin/server/ZipkinServerIntegrationTests.java
@@ -194,6 +194,22 @@ public void readsRawTrace() throws Exception {
         .andExpect(content().string(new String(Codec.JSON.writeSpans(asList(span, span)), UTF_8)));
   }
 
+  @Test
+  public void downgrades128BitTraceIdToLower64Bits() throws Exception {
+    Span span = TRACE.get(0);
+
+    mockMvc.perform(post("/api/v1/spans").content(Codec.JSON.writeSpans(asList(span))))
+        .andExpect(status().isAccepted());
+
+    // sleep as the the storage operation is async
+    Thread.sleep(1500);
+
+    // Tosses high bits
+    mockMvc.perform(get(format("/api/v1/trace/48485a3953bb6124%016x", span.traceId)))
+        .andExpect(status().isOk())
+        .andExpect(content().string(new String(Codec.JSON.writeSpans(asList(span)), UTF_8)));
+  }
+
   /** The zipkin-ui is a single-page app. This prevents reloading all resources on each click. */
   @Test
   public void setsMaxAgeOnUiResources() throws Exception {

diff --git a/zipkin/src/main/java/zipkin/internal/Util.java b/zipkin/src/main/java/zipkin/internal/Util.java
@@ -93,14 +93,20 @@ public static List<Date> getDays(long endTs, @Nullable Long lookback) {
     return days;
   }
 
-  /** Parses a 1 to 16 character lower-hex string with no prefix int an unsigned long. */
+  /**
+   * Parses a 1 to 32 character lower-hex string with no prefix into an unsigned long, tossing any
+   * bits higher than 64.
+   */
   public static long lowerHexToUnsignedLong(String lowerHex) {
-    char[] array = lowerHex.toCharArray();
-    if (array.length < 1 || array.length > 16) {
-      throw isntLowerHexLong(lowerHex);
-    }
+    int length = lowerHex.length();
+    if (length < 1 || length > 32) throw isntLowerHexLong(lowerHex);
+
+    // trim off any high bits
+    int i = length > 16 ? length - 16 : 0;
+
     long result = 0;
-    for (char c : array) {
+    for (; i < length; i++) {
+      char c = lowerHex.charAt(i);
       result <<= 4;
       if (c >= '0' && c <= '9') {
         result |= c - '0';
@@ -115,7 +121,7 @@ public static long lowerHexToUnsignedLong(String lowerHex) {
 
   static NumberFormatException isntLowerHexLong(String lowerHex) {
     throw new NumberFormatException(
-        lowerHex + " should be a 1 to 16 character lower-hex string with no prefix");
+        lowerHex + " should be a 1 to 32 character lower-hex string with no prefix");
   }
 
   /** Inspired by {@code okio.Buffer.writeLong} */

diff --git a/zipkin/src/test/java/zipkin/internal/JsonCodecTest.java b/zipkin/src/test/java/zipkin/internal/JsonCodecTest.java
@@ -24,6 +24,7 @@
 
 import static java.util.Arrays.asList;
 import static org.assertj.core.api.Assertions.assertThat;
+import static zipkin.internal.Util.UTF_8;
 
 public final class JsonCodecTest extends CodecTest {
 
@@ -48,6 +49,23 @@ public void stringsRoundTrip() throws IOException {
         .isEqualTo(strings);
   }
 
+  @Test
+  public void tolerates128bitTraceIds_byTossingHighBits() {
+    byte[] with128BitTraceId = ("{\n"
+        + "  \"traceId\": \"48485a3953bb61246b221d5bc9e6496c\",\n"
+        + "  \"name\": \"get-traces\",\n"
+        + "  \"id\": \"6b221d5bc9e6496c\"\n"
+        + "}").getBytes(UTF_8);
+    byte[] withLower64bitsTraceId = ("{\n"
+        + "  \"traceId\": \"6b221d5bc9e6496c\",\n"
+        + "  \"name\": \"get-traces\",\n"
+        + "  \"id\": \"6b221d5bc9e6496c\"\n"
+        + "}").getBytes(UTF_8);
+
+    assertThat(Codec.JSON.readSpan(with128BitTraceId))
+        .isEqualTo(Codec.JSON.readSpan(withLower64bitsTraceId));
+  }
+
   @Test
   public void ignoreNull_parentId() {
     String json = "{\n"
@@ -57,7 +75,7 @@ public void ignoreNull_parentId() {
         + "  \"parentId\": null\n"
         + "}";
 
-    Codec.JSON.readSpan(json.getBytes(Util.UTF_8));
+    Codec.JSON.readSpan(json.getBytes(UTF_8));
   }
 
   @Test
@@ -69,7 +87,7 @@ public void ignoreNull_timestamp() {
         + "  \"timestamp\": null\n"
         + "}";
 
-    Codec.JSON.readSpan(json.getBytes(Util.UTF_8));
+    Codec.JSON.readSpan(json.getBytes(UTF_8));
   }
 
   @Test
@@ -81,7 +99,7 @@ public void ignoreNull_duration() {
         + "  \"duration\": null\n"
         + "}";
 
-    Codec.JSON.readSpan(json.getBytes(Util.UTF_8));
+    Codec.JSON.readSpan(json.getBytes(UTF_8));
   }
 
   @Test
@@ -93,7 +111,7 @@ public void ignoreNull_debug() {
         + "  \"debug\": null\n"
         + "}";
 
-    Codec.JSON.readSpan(json.getBytes(Util.UTF_8));
+    Codec.JSON.readSpan(json.getBytes(UTF_8));
   }
 
   @Test
@@ -111,7 +129,7 @@ public void ignoreNull_annotation_endpoint() {
         + "  ]\n"
         + "}";
 
-    Codec.JSON.readSpan(json.getBytes(Util.UTF_8));
+    Codec.JSON.readSpan(json.getBytes(UTF_8));
   }
 
   @Test
@@ -129,7 +147,7 @@ public void ignoreNull_binaryAnnotation_endpoint() {
         + "  ]\n"
         + "}";
 
-    Codec.JSON.readSpan(json.getBytes(Util.UTF_8));
+    Codec.JSON.readSpan(json.getBytes(UTF_8));
   }
 
   @Test
@@ -143,7 +161,7 @@ public void niceErrorOnNull_traceId() {
         + "  \"id\": \"6b221d5bc9e6496c\"\n"
         + "}";
 
-    Codec.JSON.readSpan(json.getBytes(Util.UTF_8));
+    Codec.JSON.readSpan(json.getBytes(UTF_8));
   }
 
   @Test
@@ -157,7 +175,7 @@ public void niceErrorOnNull_id() {
         + "  \"id\": null\n"
         + "}";
 
-    Codec.JSON.readSpan(json.getBytes(Util.UTF_8));
+    Codec.JSON.readSpan(json.getBytes(UTF_8));
   }
 
   @Test
@@ -175,7 +193,7 @@ public void binaryAnnotation_long() {
         + "  ]\n"
         + "}";
 
-    Span span = Codec.JSON.readSpan(json.getBytes(Util.UTF_8));
+    Span span = Codec.JSON.readSpan(json.getBytes(UTF_8));
     assertThat(span.binaryAnnotations)
         .containsExactly(BinaryAnnotation.builder()
             .key("num")
@@ -202,7 +220,7 @@ public void binaryAnnotation_double() {
         + "  ]\n"
         + "}";
 
-    Span span = Codec.JSON.readSpan(json.getBytes(Util.UTF_8));
+    Span span = Codec.JSON.readSpan(json.getBytes(UTF_8));
     assertThat(span.binaryAnnotations)
         .containsExactly(BinaryAnnotation.builder()
             .key("num")

diff --git a/zipkin/src/test/java/zipkin/internal/UtilTest.java b/zipkin/src/test/java/zipkin/internal/UtilTest.java
@@ -53,14 +53,20 @@ public void midnightUTCTest() throws ParseException {
         .isEqualTo("2011-04-15T00:00:00Z");
   }
 
+  @Test
+  public void lowerHexToUnsignedLong_downgrades128bitIdsByDroppingHighBits() {
+    assertThat(lowerHexToUnsignedLong("463ac35c9f6413ad48485a3953bb6124"))
+        .isEqualTo(lowerHexToUnsignedLong("48485a3953bb6124"));
+  }
+
   @Test
   public void lowerHexToUnsignedLongTest() {
     assertThat(lowerHexToUnsignedLong("ffffffffffffffff")).isEqualTo(-1);
     assertThat(lowerHexToUnsignedLong("0")).isEqualTo(0);
     assertThat(lowerHexToUnsignedLong(Long.toHexString(Long.MAX_VALUE))).isEqualTo(Long.MAX_VALUE);
 
     try {
-      lowerHexToUnsignedLong("fffffffffffffffff"); // too long
+      lowerHexToUnsignedLong("fffffffffffffffffffffffffffffffff"); // too long
       failBecauseExceptionWasNotThrown(NumberFormatException.class);
     } catch (NumberFormatException e) {