Tolerate reads of 128 bit X-B3-TraceId (#239)

The first step of transitioning to 128bit `X-B3-TraceId` is tolerantly reading 32 character long ids by throwing away the high bits (any characters left of 16 characters). This allows the tracing system to more flexibly introduce 128bit trace id support in the future. Ex. when `X-B3-TraceId: 463ac35c9f6413ad48485a3953bb6124` is received, parse the lower 64 bits (right most 16 characters ex48485a3953bb6124) as the trace id. See openzipkin/b3-propagation#6
thgoexpt · Sep 16, 2016 · 04ef577 · 04ef577
1 parent 0210d52
commit 04ef577
Show file tree

Hide file tree

Showing 4 changed files with 123 additions and 99 deletions.
diff --git a/brave-core/src/main/java/com/github/kristofa/brave/IdConversion.java b/brave-core/src/main/java/com/github/kristofa/brave/IdConversion.java
@@ -19,44 +19,45 @@
  * @author kristof
  */
 public class IdConversion {
-
-	/**
-	 * Converts long trace or span id to String.
-	 * 
-	 * @param id trace, span or parent span id.
-	 * @return String representation.
-	 */
-	public static String convertToString(final long id) {
-            return Long.toHexString(id);
-	}
-
-	/**
-	 * Converts String trace or span id to long.
-	 * 
-	 * @param id trace, span or parent span id.
-	 * @return Long representation.
-	 */
-	public static long convertToLong(final String id) {
-	  if (id.length() == 0 || id.length() > 16) {
-	    throw new NumberFormatException(
-		id + " should be a <=16 character lower-hex string with no prefix");
-	  }
 
-	  long result = 0;
+  /**
+   * Converts long trace or span id to String.
+   *
+   * @param id trace, span or parent span id.
+   * @return String representation.
+   */
+  public static String convertToString(final long id) {
+    return Long.toHexString(id);
+  }
 
-	  for (char c : id.toCharArray()) {
-	    result <<= 4;
+  /**
+   * 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 convertToLong(final String lowerHex) {
+    int length = lowerHex.length();
+    if (length < 1 || length > 32) throw isntLowerHexLong(lowerHex);
 
-	    if (c >= '0' && c <= '9') {
-	      result |= c - '0';
-	    } else if (c >= 'a' && c <= 'f') {
-	      result |= c - 'a' + 10;
-	    } else {
-	      throw new NumberFormatException("character " + c + " not lower hex in " + id);
-	    }
-	  }
+    // trim off any high bits
+    int i = length > 16 ? length - 16 : 0;
 
-	  return result;
-	}
+    long result = 0;
+    for (; i < length; i++) {
+      char c = lowerHex.charAt(i);
+      result <<= 4;
+      if (c >= '0' && c <= '9') {
+        result |= c - '0';
+      } else if (c >= 'a' && c <= 'f') {
+        result |= c - 'a' + 10;
+      } else {
+        throw isntLowerHexLong(lowerHex);
+      }
+    }
+    return result;
+  }
 
+  static NumberFormatException isntLowerHexLong(String lowerHex) {
+    throw new NumberFormatException(
+        lowerHex + " should be a 1 to 32 character lower-hex string with no prefix");
+  }
 }
diff --git a/brave-core/src/test/java/com/github/kristofa/brave/IdConversionTest.java b/brave-core/src/test/java/com/github/kristofa/brave/IdConversionTest.java
@@ -1,74 +1,80 @@
 package com.github.kristofa.brave;
 
-import static org.junit.Assert.*;
-
 import org.junit.Test;
 
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.Assert.assertEquals;
+
 public class IdConversionTest {
-
-	@Test
-	public void testPositiveId() {
-		final long longId = 8828218016717761634L;
-		// This id was generated using the zipkin code.
-		final String expectedId = "7a842183262a6c62";
-		assertEquals(expectedId, IdConversion.convertToString(longId));
-		assertEquals(longId, IdConversion.convertToLong(expectedId));
-	}
-
 
-	@Test
-	public void testNegativeId() {
-		final long longId = -4667777584646200191L;
-		// This id was generated using the zipkin code.
-		final String expectedId = "bf38b90488a1e481";
-		assertEquals(expectedId, IdConversion.convertToString(longId));
-		assertEquals(longId, IdConversion.convertToLong(expectedId));
-	}
-
-	@Test
-	public void testZeroId() {
-		final long longId = 0;
-		// Zipkin prepends 0's but conversion without those zeros also works.
-		final String expectedId = "0";
-		assertEquals(expectedId, IdConversion.convertToString(longId));
-		assertEquals(longId, IdConversion.convertToLong(expectedId));
-	}
-
-	@Test
-	public void testMinValueId() {
-		final long longId = Long.MIN_VALUE;
-		// This id was generated using the zipkin code.
-		final String expectedId = "8000000000000000";
-		assertEquals(expectedId, IdConversion.convertToString(longId));
-		assertEquals(longId, IdConversion.convertToLong(expectedId));
-	}
-
-	@Test
-	public void testMaxValueId() {
-		final long longId = Long.MAX_VALUE;
-		// This id was generated using the zipkin code.
-		final String expectedId = "7fffffffffffffff";
-		assertEquals(expectedId, IdConversion.convertToString(longId));
-		assertEquals(longId, IdConversion.convertToLong(expectedId));
-	}
+  @Test
+  public void testPositiveId() {
+    final long longId = 8828218016717761634L;
+    // This id was generated using the zipkin code.
+    final String expectedId = "7a842183262a6c62";
+    assertEquals(expectedId, IdConversion.convertToString(longId));
+    assertEquals(longId, IdConversion.convertToLong(expectedId));
+  }
+
+  @Test
+  public void testNegativeId() {
+    final long longId = -4667777584646200191L;
+    // This id was generated using the zipkin code.
+    final String expectedId = "bf38b90488a1e481";
+    assertEquals(expectedId, IdConversion.convertToString(longId));
+    assertEquals(longId, IdConversion.convertToLong(expectedId));
+  }
+
+  @Test
+  public void testZeroId() {
+    final long longId = 0;
+    // Zipkin prepends 0's but conversion without those zeros also works.
+    final String expectedId = "0";
+    assertEquals(expectedId, IdConversion.convertToString(longId));
+    assertEquals(longId, IdConversion.convertToLong(expectedId));
+  }
+
+  @Test
+  public void testMinValueId() {
+    final long longId = Long.MIN_VALUE;
+    // This id was generated using the zipkin code.
+    final String expectedId = "8000000000000000";
+    assertEquals(expectedId, IdConversion.convertToString(longId));
+    assertEquals(longId, IdConversion.convertToLong(expectedId));
+  }
+
+  @Test
+  public void testMaxValueId() {
+    final long longId = Long.MAX_VALUE;
+    // This id was generated using the zipkin code.
+    final String expectedId = "7fffffffffffffff";
+    assertEquals(expectedId, IdConversion.convertToString(longId));
+    assertEquals(longId, IdConversion.convertToLong(expectedId));
+  }
+
+  @Test(expected = NumberFormatException.class)
+  public void testIdTooLong() {
+    IdConversion.convertToLong("7ffffffffffffffff7ffffffffffffffff");
+  }
 
-	@Test(expected = NumberFormatException.class)
-	public void testIdTooLong() {
-	        IdConversion.convertToLong("7ffffffffffffffff");
-	}
+  @Test(expected = NumberFormatException.class)
+  public void testIdEmpty() {
+    IdConversion.convertToLong("");
+  }
 
-        @Test(expected = NumberFormatException.class)
-        public void testIdEmpty() {
-                IdConversion.convertToLong("");
-        }
+  @Test(expected = NumberFormatException.class)
+  public void testIdShouldntHavePrefix() {
+    IdConversion.convertToLong("0x7fffffffffffffff7fffffffffffffff");
+  }
 
-	@Test(expected = NumberFormatException.class)
-	public void testIdShouldntHavePrefix() {
-		IdConversion.convertToLong("0x7fffffffffffffff");
-	}
+  @Test(expected = NumberFormatException.class)
+  public void testIdShouldntBeUppercase() {
+    IdConversion.convertToLong("7FFFFFFFFFFFFFFF");
+  }
 
-	@Test(expected = NumberFormatException.class)
-	public void testIdShouldntBeUppercase() {
-	  	IdConversion.convertToLong("7FFFFFFFFFFFFFFF");
-	}
+  @Test
+  public void lowerHexToUnsignedLong_downgrades128bitIdsByDroppingHighBits() {
+    assertThat(IdConversion.convertToLong("463ac35c9f6413ad48485a3953bb6124"))
+        .isEqualTo(IdConversion.convertToLong("48485a3953bb6124"));
+  }
 }
diff --git a/brave-http/src/test/java/com/github/kristofa/brave/http/HttpServerRequestAdapterTest.java b/brave-http/src/test/java/com/github/kristofa/brave/http/HttpServerRequestAdapterTest.java
@@ -117,6 +117,23 @@ public void getTraceDataSampledOneNoParentId() {
         assertNull(spanId.nullableParentId());
     }
 
+    @Test
+    public void tolerates128BitTraceIdByDroppingHighBits() {
+        String hex128Bits = "463ac35c9f6413ad48485a3953bb6124";
+        String lower64Bits = "48485a3953bb6124";
+        when(serverRequest.getHttpHeaderValue(BraveHttpHeaders.Sampled.getName())).thenReturn("1");
+        when(serverRequest.getHttpHeaderValue(BraveHttpHeaders.TraceId.getName())).thenReturn(hex128Bits);
+        when(serverRequest.getHttpHeaderValue(BraveHttpHeaders.SpanId.getName())).thenReturn(lower64Bits);
+        TraceData traceData = adapter.getTraceData();
+        assertNotNull(traceData);
+        assertTrue(traceData.getSample());
+        SpanId spanId = traceData.getSpanId();
+        assertNotNull(spanId);
+        assertEquals(IdConversion.convertToLong(lower64Bits), spanId.traceId);
+        assertEquals(IdConversion.convertToLong(lower64Bits), spanId.spanId);
+        assertNull(spanId.nullableParentId());
+    }
+
     @Test
     public void getTraceDataSampledTrueWithParentId() {
         when(serverRequest.getHttpHeaderValue(BraveHttpHeaders.Sampled.getName())).thenReturn("true");

diff --git a/pom.xml b/pom.xml
@@ -47,7 +47,7 @@
 
     <spring.version>4.3.2.RELEASE</spring.version>
     <jetty.version>8.1.20.v20160902</jetty.version>
-    <zipkin.version>1.8.4</zipkin.version>
+    <zipkin.version>1.11.1</zipkin.version>
     <log4j.version>2.3</log4j.version>
     <httpcomponents.version>4.4.1</httpcomponents.version>