Upgrading the client to be OpenTracing 0.33 compliant (#110)

* Upgrading the client to be OpenTracing 0.33 compliant

* revving up versions and info

Author: eleduardo

README.md

@@ -9,6 +9,12 @@ Haystack client java is an [OpenTracing](https://github.com/opentracing/opentrac
 
 Opentracing API usage has been documented at [https://github.com/opentracing/opentracing-java](https://github.com/opentracing/opentracing-java). 
 
+### OpenTracing version compatibility
+
+The current OpenTracing version 0.33 has introduced backwards incompatible changes with all previous versions. Please make sure 
+that your dependencies on OpenTracing align to the 0.33 version to avoid runtime issues. 
+
+## Examples
 
 You can look at our sample projects for how to trace a simple web application
 
@@ -53,7 +59,7 @@ In the usual case, you just need to include the following dependency to your pro
 
 In case your apps are already wired in with some kind of instrumentation library other than OpenTracing you could refer to the below links based on your usecase
 
-* [Opencencus](https://github.com/ExpediaDotCom/haystack-opencensus-exporter-java)
+* [Opencensus](https://github.com/ExpediaDotCom/haystack-opencensus-exporter-java)
 * [Brave-Zipkin](https://github.com/HotelsDotCom/pitchfork)
 
 

ReleaseNotes.md

@@ -1,5 +1,9 @@
 # Release Notes
 
+## 0.3.0/ 2019-11-
+
+* Upgrading OpenTracing to version 0.33
+
 ## 0.2.3/ 2019-01-02 
 
 * Adding opentracing-spring-haystack-web-starter in integrations for Spring Boot and Web applications

core/src/main/java/com/expedia/www/haystack/client/Span.java

@@ -21,6 +21,8 @@
 
 import java.util.*;
 
+import io.opentracing.tag.Tag;
+
 public final class Span implements io.opentracing.Span {
   private final Tracer tracer;
   private final Clock clock;
@@ -171,6 +173,14 @@ public Span setTag(String key, Number value) {
     return addTag(key, value);
   }
 
+  @Override
+  public <T> io.opentracing.Span setTag(Tag<T> tag, T value) {
+    if (tag != null) {
+      return addTag(tag.getKey(), value);
+    }
+    return this;
+  }
+
   @Override
   public Span setTag(String key, boolean value) {
     return addTag(key, value);

core/src/main/java/com/expedia/www/haystack/client/SpanContext.java

@@ -104,6 +104,16 @@ public SpanContext addBaggage(String key, String value) {
         return new SpanContext(traceId, spanId, parentId, newBaggage, extractedContext);
     }
 
+    @Override
+    public String toTraceId() {
+        return this.traceId.toString();
+    }
+
+    @Override
+    public String toSpanId() {
+        return this.spanId.toString();
+    }
+
     @Override
     public Iterable<Entry<String, String>> baggageItems() {
         return baggage.entrySet();

core/src/main/java/com/expedia/www/haystack/client/Tracer.java

@@ -98,11 +98,7 @@ public Tracer(String serviceName, ScopeManager scopeManager, Clock clock,
         this.extractFailureCounter = Counter.builder("extract").tag(new Tag("state", "exception")).register(metrics);
 
         Runtime.getRuntime().addShutdownHook(new Thread(() -> {
-            try {
                 Tracer.this.close();
-            } catch (IOException e) {
-                /* skip logging any error */
-            }
         }));
     }
 
@@ -113,12 +109,11 @@ public String toString() {
                 .toString();
     }
 
-    public void close() throws IOException {
+    public void close(){
         try (Sample timer = closeTimer.start()) {
             dispatcher.close();
         } catch (IOException e) {
             closeExceptionCounter.increment();
-            throw e;
         }
     }
 
@@ -188,8 +183,12 @@ public <C> SpanContext extract(Format<C> format, C carrier) {
 
     @Override
     public Span activeSpan() {
-        final Scope scope = scopeManager.active();
-        return (scope == null ? null : scope.span());
+        return scopeManager.activeSpan();
+    }
+
+    @Override
+    public Scope activateSpan(Span span) {
+        return scopeManager.activate(span);
     }
 
     @Override
@@ -269,6 +268,14 @@ public SpanBuilder withTag(String key, Number value) {
             return this;
         }
 
+        @Override
+        public <T> io.opentracing.Tracer.SpanBuilder withTag(io.opentracing.tag.Tag<T> tag, T value) {
+            if (tag != null && tag.getKey() != null) {
+                this.tags.put(tag.getKey(), value);
+            }
+            return this;
+        }
+
         @Override
         public SpanBuilder withStartTimestamp(long microseconds) {
             this.startTime = microseconds;
@@ -349,17 +356,6 @@ 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() {
-            return start();
-        }
-
         @Override
         public com.expedia.www.haystack.client.Span start() {
             return new com.expedia.www.haystack.client.Span(tracer, clock, operationName, createContext(), calculateStartTime(), tags, references);

core/src/test/java/com/expedia/www/haystack/client/SpanPropagationTest.java

@@ -43,25 +43,29 @@ public void setUp() throws Exception {
     @Test
     public void testActiveSpan() {
         Assert.assertNull(tracer.activeSpan());
-
-        try (Scope scope = tracer.buildSpan("active-span").startActive(true)) {
-            Assert.assertEquals(tracer.scopeManager().active().span(), tracer.activeSpan());
-            Assert.assertEquals(scope.span(), tracer.activeSpan());
+        Span span = tracer.buildSpan("active-span").start();
+        try (Scope scope = tracer.scopeManager().activate(span)) {
+            Assert.assertEquals(tracer.scopeManager().activeSpan(), tracer.activeSpan());
+            Assert.assertEquals(span, tracer.activeSpan());
+        }finally {
+            //auto-close behavior based on the scope has been removed in 0.33
+            span.finish();
         }
-
         Assert.assertNull(tracer.activeSpan());
     }
 
     @Test
     public void testActiveSpanPropagation() {
         Assert.assertNull(tracer.activeSpan());
-
-        try (Scope scope = tracer.buildSpan("active-span").startActive(true)) {
+        Span span = tracer.buildSpan("active-span").start();
+        try (Scope scope = tracer.scopeManager().activate(span)) {
             tracer.buildSpan("child-active-span").start().finish();
-
-            Assert.assertEquals(tracer.scopeManager().active().span(), tracer.activeSpan());
-            Assert.assertEquals(scope.span(), tracer.activeSpan());
+            Assert.assertEquals(tracer.scopeManager().activeSpan(), tracer.activeSpan());
+            Assert.assertEquals(span, tracer.activeSpan());
             Assert.assertEquals("Haven't closed the parent", 1, dispatcher.getReportedSpans().size());
+        }finally {
+            //auto-close behavior based on the scope has been removed in 0.33
+            span.finish();
         }
 
         Assert.assertEquals("Parent closed", 2, dispatcher.getReportedSpans().size());
@@ -77,9 +81,12 @@ public void testActiveSpanPropagation() {
 
     @Test
     public void testActiveSpanIgnorePropagation() {
-        try (Scope scope = tracer.buildSpan("active-span").startActive(true)) {
+        Span span = tracer.buildSpan("active-span").start();
+        try (Scope scope = tracer.scopeManager().activate(span)) {
             Span seperateSpan = tracer.buildSpan("independent-active-span").ignoreActiveSpan().start();
             seperateSpan.finish();
+        }finally {
+            span.finish();
         }
 
         Assert.assertEquals(2, dispatcher.getReportedSpans().size());
@@ -96,10 +103,11 @@ public void testActiveSpanIgnorePropagation() {
     @Test
     public void testActiveSpanPreSeeded() {
         Span parentSpan = tracer.buildSpan("parent").start();
-
-        try (Scope scope = tracer.buildSpan("active").startActive(true)) {
+        Span activeSpan = tracer.buildSpan("active").start();
+        try (Scope scope = tracer.scopeManager().activate(activeSpan)) {
             tracer.buildSpan("child").asChildOf(parentSpan).start().finish();
         }
+        activeSpan.finish();
         parentSpan.finish();
 
         Assert.assertEquals("All spans closed", 3, dispatcher.getReportedSpans().size());

core/src/test/java/com/expedia/www/haystack/client/SpanTest.java

@@ -29,6 +29,9 @@
 import java.util.Collections;
 import java.util.List;
 
+import io.opentracing.tag.StringTag;
+import io.opentracing.tag.Tag;
+
 public class SpanTest {
 
   private Tracer tracer;
@@ -105,15 +108,24 @@ public void testTagForString() {
     Assert.assertEquals(value, span.getTags().get(key));
   }
 
+  @Test
+  public void testTagForTagType() {
+    String key = "typed-key-name";
+    String value = "typed-tag-value-value";
+    StringTag stringTag = new StringTag(key);
+    stringTag.set(span, value);
+    Assert.assertEquals(value, span.getTags().get(key));
+  }
+
   @Test
   public void testTagForEmptyConditons() {
     String key = "key-name";
     String stringValue = "value-value";
     Long longValue = 3L;
     final boolean boolValue = false;
 
-    span.setTag(null, stringValue);
-    span.setTag(null, longValue);
+    span.setTag((Tag<String>) null, stringValue);
+    span.setTag((Tag<Long>) null, longValue);
     span.setTag(null, boolValue);
     span.setTag(key, (Number) null);
     span.setTag(key, (String) null);

core/src/test/java/com/expedia/www/haystack/client/TracerPropagationTest.java

@@ -30,8 +30,7 @@
 
 import io.opentracing.propagation.Format;
 import io.opentracing.propagation.TextMap;
-import io.opentracing.propagation.TextMapExtractAdapter;
-import io.opentracing.propagation.TextMapInjectAdapter;
+import io.opentracing.propagation.TextMapAdapter;
 
 public class TracerPropagationTest {
     private Tracer tracer;
@@ -64,7 +63,7 @@ public void testInject() {
         Object parentId = idGenerator.generate();
 
         Map<String, String> carrierValues = new HashMap<>();
-        TextMap carrier = new TextMapInjectAdapter(carrierValues);
+        TextMapAdapter carrier = new TextMapAdapter(carrierValues);
 
         SpanContext context = new SpanContext(traceId, spanId, parentId).addBaggage("TEST", "TEXT");
 
@@ -86,7 +85,7 @@ public void testInjectURLEncoded() {
         Object parentId = idGenerator.generate();
 
         Map<String, String> carrierValues = new HashMap<>();
-        TextMap carrier = new TextMapInjectAdapter(carrierValues);
+        TextMap carrier = new TextMapAdapter(carrierValues);
 
         SpanContext context = new SpanContext(traceId, spanId, parentId).addBaggage("TEST", "!@##*^ %^&&(*").addBaggage("!@##*^ %^&&(*", "TEXT");
 
@@ -114,7 +113,7 @@ public void testExtract() {
         carrierValues.put("Span-ID", spanId.toString());
         carrierValues.put("Parent-ID", parentId.toString());
 
-        TextMap carrier = new TextMapExtractAdapter(carrierValues);
+        TextMap carrier = new TextMapAdapter(carrierValues);
 
         SpanContext context = tracer.extract(Format.Builtin.TEXT_MAP, carrier);
 
@@ -141,7 +140,7 @@ public void testExtractIgnoreUnknowns() {
         carrierValues.put("JunkKey", parentId.toString());
         carrierValues.put("JunkKey2", parentId.toString());
 
-        TextMap carrier = new TextMapExtractAdapter(carrierValues);
+        TextMap carrier = new TextMapAdapter(carrierValues);
 
         SpanContext context = tracer.extract(Format.Builtin.HTTP_HEADERS, carrier);
 
@@ -161,7 +160,7 @@ public void testExtractInvalid() {
         carrierValues.put("Span-ID", spanId.toString());
         carrierValues.put("Parent-ID", parentId.toString());
 
-        TextMap carrier = new TextMapExtractAdapter(carrierValues);
+        TextMap carrier = new TextMapAdapter(carrierValues);
 
         SpanContext context = tracer.extract(Format.Builtin.HTTP_HEADERS, carrier);
 
@@ -183,7 +182,7 @@ public void testExtractURLEncoded() {
         carrierValues.put("Span-ID", spanId.toString());
         carrierValues.put("Parent-ID", parentId.toString());
 
-        TextMap carrier = new TextMapExtractAdapter(carrierValues);
+        TextMap carrier = new TextMapAdapter(carrierValues);
 
         SpanContext context = tracer.extract(Format.Builtin.HTTP_HEADERS, carrier);
 

pom.xml

@@ -4,7 +4,7 @@
 
   <groupId>com.expedia.www</groupId>
   <artifactId>haystack-client-java-parent</artifactId>
-  <version>0.2.9-SNAPSHOT</version>
+  <version>0.3.0-SNAPSHOT</version>
   <packaging>pom</packaging>
   <name>Haystack Java Client Parent POM</name>
   <description>Haystack Java Client Parent POM</description>
@@ -43,7 +43,7 @@
     <java-version>1.8</java-version>
     <encoding>UTF-8</encoding>
 
-    <opentracing.version>0.31.0</opentracing.version>
+    <opentracing.version>0.33.0</opentracing.version>
     <protobuf.version>3.3.1</protobuf.version>
     <slf4j.version>1.7.25</slf4j.version>
     <junit.version>4.12</junit.version>
@@ -52,7 +52,7 @@
     <commonslang3.version>3.0</commonslang3.version>
     <grpc.version>1.9.0</grpc.version>
     <jaxrs.version>2.1</jaxrs.version>
-    <micrometer.version>1.1.1</micrometer.version>
+    <micrometer.version>1.3.1</micrometer.version>
     <haystack-idl-java.version>1.0.64</haystack-idl-java.version>
     <http-client.vesion>4.5.3</http-client.vesion>