Merge pull request #113 from worldtiki/msb

Kapil Rastogi · web-flow · commit 2fe1bf07383e · 2020-08-05T15:29:21.000+05:30
Draft proposal to bring ids closer to b3 spec
diff --git a/core/src/main/java/com/expedia/www/haystack/client/Tracer.java b/core/src/main/java/com/expedia/www/haystack/client/Tracer.java
@@ -288,7 +288,7 @@ boolean isServerSpan() {
 
         protected SpanContext createNewContext() {
 
-            return createContext(tracer.idGenerator.generate(), tracer.idGenerator.generate(), null, Collections.emptyMap());
+            return createContext(tracer.idGenerator.generateTraceId(), tracer.idGenerator.generateSpanId(), null, Collections.emptyMap());
         }
 
         protected SpanContext createContext(UUID traceId, UUID spanId, UUID parentId, Map<String, String> baggage) {
@@ -331,7 +331,7 @@ protected SpanContext createDependentContext() {
             }
 
             return createContext(parent.getContext().getTraceId(),
-                    tracer.idGenerator.generate(),
+                    tracer.idGenerator.generateSpanId(),
                     parent.getContext().getSpanId(),
                     baggage);
         }
diff --git a/core/src/main/java/com/expedia/www/haystack/client/idgenerators/IdGenerator.java b/core/src/main/java/com/expedia/www/haystack/client/idgenerators/IdGenerator.java
@@ -17,5 +17,25 @@
 package com.expedia.www.haystack.client.idgenerators;
 
 public interface IdGenerator {
+
+    /**
+     * Generates a unique id
+     * @deprecated This method was deprecated in 0.3.1. Use {@link #generateTraceId()}  and {@link #generateSpanId()} instead
+     */
+    @Deprecated
     Object generate();
+
+    /**
+     * Generates a random unique identifier for a trace.
+     */
+    default Object generateTraceId() {
+        return generate();
+    }
+
+    /**
+     * Generates a random identifier for a span. It should be unique within a trace.
+     */
+    default Object generateSpanId() {
+        return generate();
+    }
 }
diff --git a/core/src/main/java/com/expedia/www/haystack/client/idgenerators/LongIdGenerator.java b/core/src/main/java/com/expedia/www/haystack/client/idgenerators/LongIdGenerator.java
@@ -16,11 +16,11 @@
  */
 package com.expedia.www.haystack.client.idgenerators;
 
-
-import com.expedia.www.haystack.client.idgenerators.IdGenerator;
-
 import java.util.concurrent.ThreadLocalRandom;
 
+/**
+ * Generates random and unique Longs as ids for Traces and Spans.
+ */
 public class LongIdGenerator implements IdGenerator {
 
     @Override
diff --git a/core/src/main/java/com/expedia/www/haystack/client/idgenerators/RandomUUIDGenerator.java b/core/src/main/java/com/expedia/www/haystack/client/idgenerators/RandomUUIDGenerator.java
@@ -19,6 +19,13 @@
 
 import java.util.UUID;
 
+/**
+ * Generates UUIDs as ids for Traces and Spans.
+ * <p/>
+ * Consider using the TimeBasedUUIDGenerator which is more performant.
+ *
+ * @see com.expedia.www.haystack.client.idgenerators.TimeBasedUUIDGenerator
+ */
 public class RandomUUIDGenerator implements IdGenerator {
 
     @Override
diff --git a/core/src/main/java/com/expedia/www/haystack/client/idgenerators/TimeBasedUUIDGenerator.java b/core/src/main/java/com/expedia/www/haystack/client/idgenerators/TimeBasedUUIDGenerator.java
@@ -16,16 +16,17 @@
  */
 package com.expedia.www.haystack.client.idgenerators;
 
-
-import com.fasterxml.uuid.Generators;
-
 import java.util.UUID;
 
+import static com.fasterxml.uuid.Generators.timeBasedGenerator;
 
+/**
+ * Generates UUIDs as ids for Traces and Spans.
+ */
 public class TimeBasedUUIDGenerator implements IdGenerator {
 
     @Override
     public UUID generate() {
-        return Generators.timeBasedGenerator().generate();
+        return timeBasedGenerator().generate();
     }
 }
diff --git a/core/src/main/java/com/expedia/www/haystack/client/propagation/Codex.java b/core/src/main/java/com/expedia/www/haystack/client/propagation/Codex.java
@@ -40,4 +40,19 @@ public interface Codex<R,T> {
      */
     T decode(R value);
 
+    default T decodeTraceId(R value) {
+        return decode(value);
+    }
+
+    default T decodeSpanId(R value) {
+        return decode(value);
+    }
+
+    default T decodeBaggage(R value) {
+        return decode(value);
+    }
+
+    default T decodeKey(R value) {
+        return decode(value);
+    }
 }
diff --git a/core/src/main/java/com/expedia/www/haystack/client/propagation/TextMapPropagator.java b/core/src/main/java/com/expedia/www/haystack/client/propagation/TextMapPropagator.java
@@ -78,18 +78,18 @@ public SpanContext extract(TextMap carrier) {
         final Map<String, String> baggage = new HashMap<>();
 
         for (Map.Entry<String, String> entry : carrier) {
-            final String decodedKey = keyCodex.decode(entry.getKey());
+            final String decodedKey = keyCodex.decodeKey(entry.getKey());
             final String decodedKeyLowerCase = decodedKey.toLowerCase();
 
             if (decodedKeyLowerCase.startsWith(convention.baggagePrefix().toLowerCase(Locale.ROOT))) {
                 baggage.put(decodedKey.substring(convention.baggagePrefix().length()),
-                            valueCodex.decode(entry.getValue()));
+                            valueCodex.decodeBaggage(entry.getValue()));
             } else if (containsIgnoreCase(convention.traceIdKeyAliases(), decodedKeyLowerCase)) {
-                traceId = valueCodex.decode(entry.getValue());
+                traceId = valueCodex.decodeTraceId(entry.getValue());
             } else if (containsIgnoreCase(convention.parentIdKeyAliases(), decodedKeyLowerCase)) {
-                parentId = valueCodex.decode(entry.getValue());
+                parentId = valueCodex.decodeSpanId(entry.getValue());
             } else if (containsIgnoreCase(convention.spanIdKeyAliases(), decodedKeyLowerCase)) {
-                spanId = valueCodex.decode(entry.getValue());
+                spanId = valueCodex.decodeSpanId(entry.getValue());
             }
         }
 
diff --git a/core/src/test/java/com/expedia/www/haystack/client/TracerPropagationTest.java b/core/src/test/java/com/expedia/www/haystack/client/TracerPropagationTest.java
@@ -43,9 +43,9 @@ public void setUp() {
     @Test(expected=IllegalArgumentException.class)
     public void testInjectInvalidFormat() {
         IdGenerator idGenerator = new RandomUUIDGenerator();
-        Object traceId = idGenerator.generate();
-        Object spanId = idGenerator.generate();
-        Object parentId = idGenerator.generate();
+        Object traceId = idGenerator.generateTraceId();
+        Object spanId = idGenerator.generateSpanId();
+        Object parentId = idGenerator.generateTraceId();
 
         String carrier = "";
 
@@ -58,9 +58,9 @@ public void testInjectInvalidFormat() {
     @Test
     public void testInject() {
         IdGenerator idGenerator = new RandomUUIDGenerator();
-        Object traceId = idGenerator.generate();
-        Object spanId = idGenerator.generate();
-        Object parentId = idGenerator.generate();
+        Object traceId = idGenerator.generateTraceId();
+        Object spanId = idGenerator.generateSpanId();
+        Object parentId = idGenerator.generateTraceId();
 
         Map<String, String> carrierValues = new HashMap<>();
         TextMapAdapter carrier = new TextMapAdapter(carrierValues);
@@ -80,9 +80,9 @@ public void testInject() {
     @Test
     public void testInjectURLEncoded() {
         IdGenerator idGenerator = new RandomUUIDGenerator();
-        Object traceId = idGenerator.generate();
-        Object spanId = idGenerator.generate();
-        Object parentId = idGenerator.generate();
+        Object traceId = idGenerator.generateTraceId();
+        Object spanId = idGenerator.generateSpanId();
+        Object parentId = idGenerator.generateTraceId();
 
         Map<String, String> carrierValues = new HashMap<>();
         TextMap carrier = new TextMapAdapter(carrierValues);
@@ -103,9 +103,9 @@ public void testInjectURLEncoded() {
     @Test
     public void testExtract() {
         IdGenerator idGenerator = new RandomUUIDGenerator();
-        Object traceId = idGenerator.generate();
-        Object spanId = idGenerator.generate();
-        Object parentId = idGenerator.generate();
+        Object traceId = idGenerator.generateTraceId();
+        Object spanId = idGenerator.generateSpanId();
+        Object parentId = idGenerator.generateTraceId();
 
         Map<String, String> carrierValues = new HashMap<>();
         carrierValues.put("Baggage-TEST", "TEXT");
@@ -128,9 +128,9 @@ public void testExtract() {
     @Test
     public void testExtractIgnoreUnknowns() {
         IdGenerator idGenerator = new RandomUUIDGenerator();
-        Object traceId = idGenerator.generate();
-        Object spanId = idGenerator.generate();
-        Object parentId = idGenerator.generate();
+        Object traceId = idGenerator.generateTraceId();
+        Object spanId = idGenerator.generateSpanId();
+        Object parentId = idGenerator.generateTraceId();
 
         Map<String, String> carrierValues = new HashMap<>();
         carrierValues.put("Trace-ID", traceId.toString());
@@ -153,8 +153,8 @@ public void testExtractIgnoreUnknowns() {
     @Test
     public void testExtractInvalid() {
         IdGenerator idGenerator = new RandomUUIDGenerator();
-        Object spanId = idGenerator.generate();
-        Object parentId = idGenerator.generate();
+        Object spanId = idGenerator.generateSpanId();
+        Object parentId = idGenerator.generateTraceId();
 
         Map<String, String> carrierValues = new HashMap<>();
         carrierValues.put("Span-ID", spanId.toString());
@@ -171,9 +171,9 @@ public void testExtractInvalid() {
     @Test
     public void testExtractURLEncoded() {
         IdGenerator idGenerator = new RandomUUIDGenerator();
-        Object traceId = idGenerator.generate();
-        Object spanId = idGenerator.generate();
-        Object parentId = idGenerator.generate();
+        Object traceId = idGenerator.generateTraceId();
+        Object spanId = idGenerator.generateSpanId();
+        Object parentId = idGenerator.generateTraceId();
 
         Map<String, String> carrierValues = new HashMap<>();
         carrierValues.put("Baggage-TEST", "!%40%23%23*%5E%20%25%5E%26%26(*");
diff --git a/core/src/test/java/com/expedia/www/haystack/client/dispatchers/clients/HttpCollectorClientTest.java b/core/src/test/java/com/expedia/www/haystack/client/dispatchers/clients/HttpCollectorClientTest.java
@@ -38,7 +38,6 @@
 import java.io.IOException;
 import java.util.HashMap;
 import java.util.Map;
-import java.util.UUID;
 
 import static org.junit.Assert.assertEquals;
 import static org.mockito.Mockito.*;
@@ -47,8 +46,8 @@ public class HttpCollectorClientTest {
 
     private Tracer tracer;
     private final static String serviceName = "dummy-service";
-    private final static Object traceId = new RandomUUIDGenerator().generate();
-    private final static Object spanId = new RandomUUIDGenerator().generate();
+    private final static Object traceId = new RandomUUIDGenerator().generateTraceId();
+    private final static Object spanId = new RandomUUIDGenerator().generateSpanId();
     private final static SpanContext spanContext = new SpanContext(traceId, spanId, null);
 
     @Before

Original file line number	Diff line number	Diff line change
`@@ -288,7 +288,7 @@ boolean isServerSpan() {`
`288`	`288`
`289`	`289`	`protected SpanContext createNewContext() {`
`290`	`290`
`291`		`- return createContext(tracer.idGenerator.generate(), tracer.idGenerator.generate(), null, Collections.emptyMap());`
	`291`	`+ return createContext(tracer.idGenerator.generateTraceId(), tracer.idGenerator.generateSpanId(), null, Collections.emptyMap());`
`292`	`292`	`}`
`293`	`293`
`294`	`294`	`protected SpanContext createContext(UUID traceId, UUID spanId, UUID parentId, Map<String, String> baggage) {`
`@@ -331,7 +331,7 @@ protected SpanContext createDependentContext() {`
`331`	`331`	`}`
`332`	`332`
`333`	`333`	`return createContext(parent.getContext().getTraceId(),`
`334`		`- tracer.idGenerator.generate(),`
	`334`	`+ tracer.idGenerator.generateSpanId(),`
`335`	`335`	`parent.getContext().getSpanId(),`
`336`	`336`	`baggage);`
`337`	`337`	`}`