Do not strip leading zeros from trace IDs (#746)

* Do not strip leading zeros from trace IDs

Signed-off-by: Yuri Shkuro <[email protected]>

* Reuse util function

Signed-off-by: Yuri Shkuro <[email protected]>

* fix

Signed-off-by: Yuri Shkuro <[email protected]>

Author: yurishkuro

jaeger-core/src/main/java/io/jaegertracing/internal/JaegerSpanContext.java

@@ -16,6 +16,7 @@
 package io.jaegertracing.internal;
 
 import io.jaegertracing.internal.propagation.TextMapCodec;
+import io.jaegertracing.internal.utils.Utils;
 import io.opentracing.SpanContext;
 
 import java.util.Collections;
@@ -71,7 +72,7 @@ protected JaegerSpanContext(
     this.debugId = debugId;
     this.objectFactory = objectFactory;
     this.traceIdAsString = convertTraceId();
-    this.spanIdAsString = Long.toHexString(spanId);
+    this.spanIdAsString = Utils.to16HexString(spanId);
   }
 
   @Override
@@ -89,16 +90,10 @@ Map<String, String> baggage() {
 
   private String convertTraceId() {
     if (traceIdHigh == 0L) {
-      return Long.toHexString(traceIdLow);
-    }
-    final String hexStringHigh = Long.toHexString(traceIdHigh);
-    final String hexStringLow = Long.toHexString(traceIdLow);
-    if (hexStringLow.length() < 16) {
-      // left pad low trace id with '0'.
-      // In theory, only 12.5% of all possible long values will be padded.
-      // In practice, using Random.nextLong(), only 6% will need padding
-      return hexStringHigh + "0000000000000000".substring(hexStringLow.length()) + hexStringLow;
+      return Utils.to16HexString(traceIdLow);
     }
+    final String hexStringHigh = Utils.to16HexString(traceIdHigh);
+    final String hexStringLow = Utils.to16HexString(traceIdLow);
     return hexStringHigh + hexStringLow;
   }
 

jaeger-core/src/main/java/io/jaegertracing/internal/propagation/TextMapCodec.java

@@ -21,6 +21,7 @@
 import io.jaegertracing.internal.exceptions.EmptyTracerStateStringException;
 import io.jaegertracing.internal.exceptions.MalformedTracerStateStringException;
 import io.jaegertracing.internal.exceptions.TraceIdOutOfBoundException;
+import io.jaegertracing.internal.utils.Utils;
 import io.jaegertracing.spi.Codec;
 import io.opentracing.propagation.TextMap;
 
@@ -118,8 +119,9 @@ public static String contextAsString(JaegerSpanContext context) {
     int intFlag = context.getFlags() & 0xFF;
     return new StringBuilder()
         .append(context.getTraceId()).append(":")
-        .append(Long.toHexString(context.getSpanId())).append(":")
-        .append(Long.toHexString(context.getParentId())).append(":")
+        .append(Utils.to16HexString(context.getSpanId())).append(":")
+        // parent=0 is special, no need to encode as full 16 characters, and more readable this way
+        .append(context.getParentId() == 0 ? "0" : Utils.to16HexString(context.getParentId())).append(":")
         .append(Integer.toHexString(intFlag))
         .toString();
   }

jaeger-core/src/main/java/io/jaegertracing/internal/utils/Utils.java

@@ -60,5 +60,13 @@ public static boolean equals(Object a, Object b) {
     return (a == b) || (a != null && a.equals(b));
   }
 
+  public static String to16HexString(long id) {
+    final String hex = Long.toHexString(id);
+    if (hex.length() == 16) {
+      return hex;
+    }
+    return "0000000000000000".substring(hex.length()) + hex;
+  }
+
   private Utils() {}
 }

jaeger-core/src/test/java/io/jaegertracing/internal/JaegerSpanTest.java

@@ -31,6 +31,7 @@
 import io.jaegertracing.internal.metrics.Metrics;
 import io.jaegertracing.internal.reporters.InMemoryReporter;
 import io.jaegertracing.internal.samplers.ConstSampler;
+import io.jaegertracing.internal.utils.Utils;
 import io.jaegertracing.spi.BaggageRestrictionManager;
 import io.opentracing.References;
 import io.opentracing.Span;
@@ -190,7 +191,7 @@ public void testToTraceId() {
 
   @Test
   public void testToSpanId() {
-    assertEquals(Long.toHexString(jaegerSpan.context().getSpanId()), jaegerSpan.context().toSpanId());
+    assertEquals(Utils.to16HexString(jaegerSpan.context().getSpanId()), jaegerSpan.context().toSpanId());
   }
 
   @Test
@@ -295,7 +296,9 @@ public void testSpanToString() {
         false,
         Collections.emptyMap(),
         Collections.emptyList());
-    assertEquals("1:2:3:4 - test-operation", span.toString());
+    assertEquals(
+        "0000000000000001:0000000000000002:0000000000000003:4 - test-operation",
+        span.toString());
     span.finish();
   }
 
@@ -317,7 +320,9 @@ public void testSpanToStringWith128BitTraceId() {
         false,
         Collections.emptyMap(),
         Collections.emptyList());
-    assertEquals("20000000000000001:3:4:4 - test-operation", span.toString());
+    assertEquals(
+        "00000000000000020000000000000001:0000000000000003:0000000000000004:4 - test-operation",
+        span.toString());
     span.finish();
   }
 

jaeger-core/src/test/java/io/jaegertracing/internal/propagation/TextMapCodecTest.java

@@ -104,7 +104,8 @@ public void testContextAsStringWith128BitTraceId() {
 
     JaegerSpanContext context = new JaegerSpanContext(traceIdHigh, traceIdLow, spanId, parentId, flags);
     assertEquals(
-            "20000000000000001:3:4:5", TextMapCodec.contextAsString(context));
+            "00000000000000020000000000000001:0000000000000003:0000000000000004:5",
+            TextMapCodec.contextAsString(context));
   }
 
   @Test
@@ -169,7 +170,7 @@ public void testInjectDoNotEncodeSpanContext() {
     codec.inject(new JaegerSpanContext(0L, traceIdLow, spanId, parentId, (byte)1), new TextMapAdapter(headers));
 
     String traceId = headers.get("uber-trace-id");
-    assertEquals("2a:1:0:1", traceId);
+    assertEquals("000000000000002a:0000000000000001:0:1", traceId);
   }
 
   @Test

jaeger-core/src/test/java/io/jaegertracing/internal/propagation/TraceContextCodecTest.java

@@ -113,7 +113,7 @@ public void testInjectWith64bit() {
     String traceParent = carrier.get(TRACE_PARENT);
     assertEquals(EXAMPLE_TRACE_PARENT, traceParent);
     JaegerSpanContext extractedContext = traceContextCodec.extract(textMap);
-    assertEquals("1:2:0:0", extractedContext.toString());
+    assertEquals("0000000000000001:0000000000000002:0:0", extractedContext.toString());
   }
 
   @Test
@@ -123,7 +123,7 @@ public void testExtractWithCapitalizedTraceHeaders() {
     textMap.put("Traceparent", EXAMPLE_TRACE_PARENT);
     textMap.put("Tracestate", "whatever");
     JaegerSpanContext spanContext = traceContextCodec.extract(textMap);
-    assertEquals("1:2:0:0", spanContext.toString());
+    assertEquals("0000000000000001:0000000000000002:0:0", spanContext.toString());
     assertEquals("whatever", spanContext.getTraceState());
   }
 
@@ -201,7 +201,7 @@ public void testDebugIdWithTraceHeader() {
     textMap.put(Constants.DEBUG_ID_HEADER_KEY, EXAMPLE_DEBUG_ID);
     JaegerSpanContext spanContext = traceContextCodec.extract(textMap);
     JaegerTracer tracer = new JaegerTracer.Builder("service").withReporter(new InMemoryReporter()).build();
-    assertEquals("1", spanContext.getTraceId());
+    assertEquals("0000000000000001", spanContext.getTraceId());
     JaegerSpan child = tracer.buildSpan("span").asChildOf(spanContext).start();
     assertFalse(child.context().isDebug());
     child.finish();

jaeger-zipkin/src/test/java/io/jaegertracing/zipkin/internal/V2SpanConverterTest.java

@@ -303,7 +303,10 @@ public void testSpanLogsCreateAnnotations() {
 
   @Test
   public void testConvertSpanWith128BitTraceId() {
-    JaegerSpan span = tracer128.buildSpan("operation-name").start();
+    // zipkinSpan.traceId() always returns full length ID (padded with 0s).
+    // To make the test stable, use a short idHigh portion.
+    JaegerSpanContext c = new JaegerSpanContext(1L, 1L, 1L, 0L, (byte)0x01);
+    JaegerSpan span = tracer128.buildSpan("operation-name").asChildOf(c).start();
 
     zipkin2.Span zipkinSpan = V2SpanConverter.convertSpan(span);
     assertNotEquals(0, span.context().getTraceIdHigh());