Add support single and dual span types (#67)

mchandramouli · ashishagg · commit 7dc7923d85fd · 2018-10-29T21:01:56.000+05:30
* adding support for single and dual span types

* merge conflicts

* renaming config to dualSpanMode instead of dualSpanType
diff --git a/.gitignore b/.gitignore
@@ -37,9 +37,11 @@ buildNumber.properties
 # virtual machine crash logs, see http://www.java.com/en/download/help/error_hotspot.xml
 hs_err_pid*
 
-# IntelliJ Idea project files
+# IDE
 .idea/
 *.iml
+*.ipr
+*.iws
 *.classpath
 *.project
 *.settings
diff --git a/core/src/main/java/com/expedia/www/haystack/client/SpanContext.java b/core/src/main/java/com/expedia/www/haystack/client/SpanContext.java
@@ -28,12 +28,22 @@ public class SpanContext implements io.opentracing.SpanContext {
     private final UUID traceId;
     private final UUID spanId;
     private final UUID parentId;
+    private boolean extractedContext;
 
     public SpanContext(UUID traceId, UUID spanId, UUID parentId) {
-        this(traceId, spanId, parentId, Collections.<String, String>emptyMap());
+        this(traceId, spanId, parentId, false);
     }
 
+    public SpanContext(UUID traceId, UUID spanId, UUID parentId, boolean extractedContext) {
+        this(traceId, spanId, parentId, Collections.emptyMap(), extractedContext);
+    }
+
+    @Deprecated
     SpanContext(UUID traceId, UUID spanId, UUID parentId, Map<String, String> baggage) {
+        this(traceId, spanId, parentId, baggage, false);
+    }
+
+    SpanContext(UUID traceId, UUID spanId, UUID parentId, Map<String, String> baggage, boolean extractedContext) {
         if (baggage == null) {
             throw new NullPointerException();
         }
@@ -42,6 +52,7 @@ public SpanContext(UUID traceId, UUID spanId, UUID parentId) {
         this.spanId = spanId;
         this.parentId = parentId;
         this.baggage = Collections.unmodifiableMap(baggage);
+        this.extractedContext = extractedContext;
     }
 
     @Override
@@ -67,13 +78,13 @@ public String toString() {
     }
 
     public SpanContext addBaggage(Map<String, String> newBaggage) {
-        return new SpanContext(traceId, spanId, parentId, newBaggage);
+        return new SpanContext(traceId, spanId, parentId, newBaggage, extractedContext);
     }
 
     public SpanContext addBaggage(String key, String value) {
         Map<String, String> newBaggage = new HashMap<>(this.baggage);
         newBaggage.put(key, value);
-        return new SpanContext(traceId, spanId, parentId, newBaggage);
+        return new SpanContext(traceId, spanId, parentId, newBaggage, extractedContext);
     }
 
     @Override
@@ -110,4 +121,8 @@ public UUID getSpanId() {
     public UUID getParentId() {
         return parentId;
     }
+
+    boolean isExtractedContext() {
+        return extractedContext;
+    }
 }
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
@@ -30,6 +30,7 @@
 import io.opentracing.Span;
 import io.opentracing.propagation.Format;
 import io.opentracing.propagation.TextMap;
+import io.opentracing.tag.Tags;
 import io.opentracing.util.ThreadLocalScopeManager;
 import org.apache.commons.lang3.builder.ReflectionToStringBuilder;
 import org.apache.commons.lang3.builder.ToStringStyle;
@@ -43,6 +44,7 @@ public class Tracer implements io.opentracing.Tracer {
     protected final PropagationRegistry registry;
     private final String serviceName;
     private final ScopeManager scopeManager;
+    private final boolean dualSpanMode;
 
     private final Counter spansCreatedCounter;
 
@@ -60,12 +62,14 @@ public class Tracer implements io.opentracing.Tracer {
     private final Timer extractTimer;
     private final Counter extractFailureCounter;
 
-    public Tracer(String serviceName, ScopeManager scopeManager, Clock clock, Dispatcher dispatcher, PropagationRegistry registry, Metrics metrics) {
+    public Tracer(String serviceName, ScopeManager scopeManager, Clock clock,
+                  Dispatcher dispatcher, PropagationRegistry registry, Metrics metrics, boolean dualSpanMode) {
         this.serviceName = serviceName;
         this.scopeManager = scopeManager;
         this.clock = clock;
         this.dispatcher = dispatcher;
         this.registry = registry;
+        this.dualSpanMode = dualSpanMode;
 
         this.dispatchTimer = Timer.builder("dispatch").register(metrics);
         this.closeTimer = Timer.builder("close").register(metrics);
@@ -250,21 +254,19 @@ public SpanBuilder withStartTimestamp(long microseconds) {
             return this;
         }
 
-        @Override
-        public Scope startActive(boolean finishSpanOnClose) {
-            return tracer.scopeManager().activate(start(), finishSpanOnClose);
+        boolean isServerSpan() {
+            return Tags.SPAN_KIND_SERVER.equals(tags.get(Tags.SPAN_KIND.getKey()));
         }
 
-        protected SpanContext createNewContext() {
-            UUID randomId = UUID.randomUUID();
-            return createContext(randomId, randomId, null, Collections.emptyMap());
+        SpanContext createNewContext() {
+            return createContext(UUID.randomUUID(), UUID.randomUUID(), null, Collections.emptyMap());
         }
 
-        protected SpanContext createContext(UUID traceId, UUID spanId, UUID parentId, Map<String, String> baggage) {
-            return new SpanContext(traceId, spanId, parentId, baggage);
+        SpanContext createContext(UUID traceId, UUID spanId, UUID parentId, Map<String, String> baggage) {
+            return new SpanContext(traceId, spanId, parentId, baggage, false);
         }
 
-        protected SpanContext createDependentContext() {
+        SpanContext createDependentContext() {
             Reference parent = references.get(0);
             for (Reference reference : references) {
                 if (References.CHILD_OF.equals(reference.getReferenceType())) {
@@ -279,13 +281,25 @@ protected SpanContext createDependentContext() {
                 baggage.putAll(reference.getContext().getBaggage());
             }
 
+            // This is a check to see if the tracer is configured to support single
+            // single span type (Zipkin style shared span id) or
+            // dual span type (client and server having their own span ids ).
+            // a. If tracer is not of dualSpanType and if it is a server span then we
+            // just return the parent context with the same shared span ids
+            // b. If tracer is not of dualSpanType and if the parent context is an extracted one from the wire
+            // then we assume this is the first span in the server and so just return the parent context
+            // with the same shared span ids
+            if (!tracer.dualSpanMode && (isServerSpan() || parent.getContext().isExtractedContext())) {
+                return parent.getContext();
+            }
+
             return createContext(parent.getContext().getTraceId(),
                     UUID.randomUUID(),
                     parent.getContext().getSpanId(),
                     baggage);
         }
 
-        protected SpanContext createContext() {
+        SpanContext createContext() {
             // handle active spans if needed
             if (references.isEmpty() && !ignoreActive && tracer.activeSpan() != null) {
                 asChildOf(tracer.activeSpan());
@@ -294,6 +308,7 @@ protected SpanContext createContext() {
             if (references.isEmpty()) {
                 return createNewContext();
             }
+
             return createDependentContext();
         }
 
@@ -304,6 +319,11 @@ private long calculateStartTime() {
             return startTime;
         }
 
+        @Override
+        public Scope startActive(boolean finishSpanOnClose) {
+            return tracer.scopeManager().activate(start(), finishSpanOnClose);
+        }
+
         @Override
         @Deprecated
         public com.expedia.www.haystack.client.Span startManual() {
@@ -324,6 +344,7 @@ public static class Builder {
         protected Dispatcher dispatcher;
         protected PropagationRegistry registry = new PropagationRegistry();
         protected Metrics metrics;
+        private boolean dualSpanMode;
 
         public Builder(MetricsRegistry registry, String serviceName, Dispatcher dispatcher) {
             this(new Metrics(registry, Tracer.class.getName(), Collections.emptyList()), serviceName, dispatcher);
@@ -341,7 +362,6 @@ public Builder(Metrics metrics, String serviceName, Dispatcher dispatcher) {
             TextMapPropagator httpPropagator = new TextMapPropagator.Builder().withURLCodex().build();
             withFormat(Format.Builtin.HTTP_HEADERS, (Injector<TextMap>) httpPropagator);
             withFormat(Format.Builtin.HTTP_HEADERS, (Extractor<TextMap>) httpPropagator);
-
         }
 
         public Builder withScopeManager(ScopeManager scope) {
@@ -375,8 +395,18 @@ public Builder clearAllFormats() {
             return this;
         }
 
+        /**
+         * Enables production of client and server spans with two different span-ids. If not enabled,
+         * this will cause the Tracer to produce client and server spans with shared span-ids (Zipkin style)
+         * @return this builder instance
+         */
+        public Builder withDualSpanMode() {
+            dualSpanMode = true;
+            return this;
+        }
+
         public Tracer build() {
-            return new Tracer(serviceName, scopeManager, clock, dispatcher, registry, metrics);
+            return new Tracer(serviceName, scopeManager, clock, dispatcher, registry, metrics, dualSpanMode);
         }
 
     }
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
@@ -103,8 +103,9 @@ public SpanContext extract(TextMap carrier) {
         }
 
         SpanContext context = new SpanContext(UUID.fromString(traceId),
-                UUID.fromString(spanId),
-                parentId == null ? null : UUID.fromString(parentId));
+                                              UUID.fromString(spanId),
+                                              parentId == null ? null : UUID.fromString(parentId),
+                                              true);
         return context.addBaggage(baggage);
     }
 
diff --git a/core/src/test/java/com/expedia/www/haystack/client/SpanBuilderTest.java b/core/src/test/java/com/expedia/www/haystack/client/SpanBuilderTest.java
@@ -16,17 +16,19 @@
  */
 package com.expedia.www.haystack.client;
 
-import java.util.Map;
-
-import org.junit.Assert;
-import org.junit.Before;
-import org.junit.Test;
-
 import com.expedia.www.haystack.client.dispatchers.Dispatcher;
 import com.expedia.www.haystack.client.dispatchers.NoopDispatcher;
 import com.expedia.www.haystack.client.metrics.NoopMetricsRegistry;
-
 import io.opentracing.References;
+import io.opentracing.propagation.Format;
+import io.opentracing.propagation.TextMap;
+import io.opentracing.tag.Tags;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.Map;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
 
 public class SpanBuilderTest {
 
@@ -53,23 +55,112 @@ public void testReferences() {
         Span following = tracer.buildSpan("following").start();
 
         Span child = tracer.buildSpan("child")
-            .asChildOf(parent)
-            .addReference(References.FOLLOWS_FROM, following.context())
-            .start();
-
+                .asChildOf(parent)
+                .addReference(References.FOLLOWS_FROM, following.context())
+                .start();
 
         Assert.assertEquals(2, child.getReferences().size());
         Assert.assertEquals(child.getReferences().get(0), new Reference(References.CHILD_OF, parent.context()));
         Assert.assertEquals(child.getReferences().get(1), new Reference(References.FOLLOWS_FROM, following.context()));
     }
 
+    @Test
+    public void testChildOfWithDualSpanType() {
+        //create a client span
+        final Tracer clientTracer = new Tracer.Builder(new NoopMetricsRegistry(),
+                                                       "ClientService",
+                                                       dispatcher).withDualSpanMode().build();
+        final Span clientSpan = clientTracer.buildSpan("Api_call")
+                .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_CLIENT)
+                .start();
+        final MapBackedTextMap wireData = new MapBackedTextMap();
+        clientTracer.inject(clientSpan.context(), Format.Builtin.TEXT_MAP, wireData);
+
+        //create a server
+        final Tracer serverTracer = new Tracer.Builder(new NoopMetricsRegistry(),
+                                                       "ServerService",
+                                                       dispatcher).withDualSpanMode().build();
+        final SpanContext wireContext = serverTracer.extract(Format.Builtin.TEXT_MAP, wireData);
+        final Span serverSpan = serverTracer.buildSpan("Api")
+                .asChildOf(wireContext)
+                .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_SERVER)
+                .start();
+
+        Assert.assertEquals("trace-ids are not matching",
+                            clientSpan.context().getTraceId().toString(),
+                            serverSpan.context().getTraceId().toString());
+        Assert.assertEquals("server's parent id - client's span id do not match",
+                            clientSpan.context().getSpanId().toString(),
+                            serverSpan.context().getParentId().toString());
+    }
+
+    @Test
+    public void testChildOfWithSingleSpanType() {
+        //create a client span
+        final Tracer clientTracer = new Tracer.Builder(new NoopMetricsRegistry(),
+                                                       "ClientService",
+                                                       dispatcher).build();
+        final Span clientSpan = clientTracer.buildSpan("Api_call")
+                .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_CLIENT)
+                .start();
+        final MapBackedTextMap wireData = new MapBackedTextMap();
+        clientTracer.inject(clientSpan.context(), Format.Builtin.TEXT_MAP, wireData);
+
+        //create a server
+        final Tracer serverTracer = new Tracer.Builder(new NoopMetricsRegistry(),
+                                                       "ServerService",
+                                                       dispatcher).build();
+        final SpanContext wireContext = serverTracer.extract(Format.Builtin.TEXT_MAP, wireData);
+        final Span serverSpan = serverTracer.buildSpan("Api")
+                .asChildOf(wireContext)
+                .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_SERVER)
+                .start();
+
+        Assert.assertEquals("trace-ids are not matching",
+                            clientSpan.context().getTraceId().toString(),
+                            serverSpan.context().getTraceId().toString());
+        Assert.assertEquals("server - client spans do not match",
+                            clientSpan.context().getSpanId().toString(),
+                            serverSpan.context().getSpanId().toString());
+    }
+
+
+    @Test
+    public void testChildOfWithSingleSpanTypeAndExtractedContext() {
+        //create a client span
+        final Tracer clientTracer = new Tracer.Builder(new NoopMetricsRegistry(),
+                                                       "ClientService",
+                                                       dispatcher).build();
+        final Span clientSpan = clientTracer.buildSpan("Api_call")
+                .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_CLIENT)
+                .start();
+        final MapBackedTextMap wireData = new MapBackedTextMap();
+        clientTracer.inject(clientSpan.context(), Format.Builtin.TEXT_MAP, wireData);
+
+        //create a server
+        final Tracer serverTracer = new Tracer.Builder(new NoopMetricsRegistry(),
+                                                       "ServerService",
+                                                       dispatcher).build();
+        final SpanContext wireContext = serverTracer.extract(Format.Builtin.TEXT_MAP, wireData);
+        final Span serverSpan = serverTracer.buildSpan("Api")
+                .asChildOf(wireContext)
+                .start();
+
+        Assert.assertEquals("trace-ids are not matching",
+                            clientSpan.context().getTraceId().toString(),
+                            serverSpan.context().getTraceId().toString());
+        Assert.assertEquals("server - client spans do not match",
+                            clientSpan.context().getSpanId().toString(),
+                            serverSpan.context().getSpanId().toString());
+    }
+
     @Test
     public void testWithTags() {
         Span child = tracer.buildSpan("child")
-            .withTag("string-key", "string-value")
-            .withTag("boolean-key", false)
-            .withTag("number-key", 1l)
-            .start();
+                .withTag("string-key", "string-value")
+                .withTag("boolean-key", false)
+                .withTag("number-key", 1l)
+                .start();
 
         Map<String, ?> tags = child.getTags();
 
@@ -82,4 +173,21 @@ public void testWithTags() {
         Assert.assertEquals(1l, tags.get("number-key"));
     }
 
+    private class MapBackedTextMap implements TextMap {
+        private final Map<String, String> map = new HashMap<>();
+
+        @Override
+        public Iterator<Map.Entry<String, String>> iterator() {
+            return map.entrySet().iterator();
+        }
+
+        @Override
+        public void put(String key, String value) {
+            map.put(key, value);
+        }
+
+        public Map<String, String> getMap() {
+            return map;
+        }
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -103,8 +103,9 @@ public SpanContext extract(TextMap carrier) {`
`103`	`103`	`}`
`104`	`104`
`105`	`105`	`SpanContext context = new SpanContext(UUID.fromString(traceId),`
`106`		`- UUID.fromString(spanId),`
`107`		`- parentId == null ? null : UUID.fromString(parentId));`
	`106`	`+ UUID.fromString(spanId),`
	`107`	`+ parentId == null ? null : UUID.fromString(parentId),`
	`108`	`+ true);`
`108`	`109`	`return context.addBaggage(baggage);`
`109`	`110`	`}`
`110`	`111`