Upgrade tracing libraries (#1230)

* Upgrade of libraries related to tracing. (#1169) Signed-off-by: Tomas Langer <tomas.langer@oracle.com> * Upgrades OpenTracing version, Jaeger and Zipkin tracers and fixed backward incompatible calls. Signed-off-by: Tomas Langer <tomas.langer@oracle.com> * Upgrade to latest Zipkin sender. Signed-off-by: Tomas Langer <tomas.langer@oracle.com> * Updated CHANGELOG.md Signed-off-by: Tomas Langer <tomas.langer@oracle.com>
helidon-io · Dec 16, 2019 · 5d69b7d · 5d69b7d
1 parent fc8badd
commit 5d69b7d
Show file tree

Hide file tree

Showing 23 changed files with 116 additions and 86 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,16 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 ### Backward incompatible changes
 #### Common
 - Flow API is now used from `java.util.concurrent.Flow`, Helidon specific `Flow` class is now removed
+#### Tracing
+- We have upgraded to OpenTracing version 0.33.0 that is not backward compatible, the following breaking changes exist
+    (these are OpenTracing changes, not Helidon changes):
+    1. `TextMapExtractAdapter` and `TextMapInjectAdapter` are now `TextMapAdapter`
+    2. module name changed from `opentracing.api` to `io.opentracing.api` (same for `noop` and `util`)
+    3. `SpanBuilder` no longer has `startActive` method, you need to use `Tracer.activateSpan(Span)`
+    4. `ScopeManager.activate(Span, boolean)` is replaced by `ScopeManager.activate(Span)` - second parameter is now always 
+            `false`
+    5. `Scope ScopeManager.active()` is removed - replaced by `Span Tracer.activeSpan()`
+- If you use the `TracerBuilder` abstraction in Helidon and have no custom Spans, there is no change required    
 
 ## [1.4.0] 
 

diff --git a/dependencies/pom.xml b/dependencies/pom.xml
@@ -39,7 +39,7 @@
         <version.lib.activation-api>1.2.0</version.lib.activation-api>
         <version.lib.annotation-api>1.3.1</version.lib.annotation-api>
         <version.lib.apache-httpclient>4.5.10</version.lib.apache-httpclient>
-        <version.lib.brave-opentracing>0.31.0</version.lib.brave-opentracing>
+        <version.lib.brave-opentracing>0.35.0</version.lib.brave-opentracing>
         <version.lib.cdi-api>2.0</version.lib.cdi-api>
         <version.lib.eclipselink>2.7.4</version.lib.eclipselink>
         <version.lib.el-api>3.0.0</version.lib.el-api>
@@ -55,7 +55,7 @@
         <version.lib.hikaricp>2.7.8</version.lib.hikaricp>
         <version.lib.inject>1</version.lib.inject>
         <version.lib.jackson>2.10.0</version.lib.jackson>
-        <version.lib.jaegertracing>0.34.0</version.lib.jaegertracing>
+        <version.lib.jaegertracing>1.1.0</version.lib.jaegertracing>
         <version.lib.jakarta-persistence-api>2.2.2</version.lib.jakarta-persistence-api>
         <version.lib.jandex>2.1.1.Final</version.lib.jandex>
         <version.lib.jaxb-api>2.3.0</version.lib.jaxb-api>
@@ -84,8 +84,8 @@
         <version.lib.netty>4.1.42.Final</version.lib.netty>
         <version.lib.oci-java-sdk-objectstorage>1.5.2</version.lib.oci-java-sdk-objectstorage>
         <version.lib.ojdbc8>19.3.0.0</version.lib.ojdbc8>
-        <version.lib.opentracing>0.31.0</version.lib.opentracing>
-        <version.lib.opentracing.grpc>0.0.12</version.lib.opentracing.grpc>
+        <version.lib.opentracing>0.33.0</version.lib.opentracing>
+        <version.lib.opentracing.grpc>0.2.1</version.lib.opentracing.grpc>
         <version.lib.persistence-api>2.2</version.lib.persistence-api>
         <version.lib.prometheus>0.6.0</version.lib.prometheus>
         <version.lib.protobuf.java>3.5.1</version.lib.protobuf.java>
@@ -99,7 +99,7 @@
         <version.lib.ucp>19.3.0.0</version.lib.ucp>
         <version.lib.validation-api>2.0.1.Final</version.lib.validation-api>
         <version.lib.weld>3.1.1.Final</version.lib.weld>
-        <version.lib.zipkin>2.6.0</version.lib.zipkin>
+        <version.lib.zipkin>2.12.0</version.lib.zipkin>
         <version.lib.hystrix>1.5.18</version.lib.hystrix>
         <version.lib.failsafe>2.3.1</version.lib.failsafe>
         <version.lib.yasson>1.0.3</version.lib.yasson>

diff --git a/grpc/client/src/main/java/module-info.java b/grpc/client/src/main/java/module-info.java
@@ -22,6 +22,6 @@
 
     requires transitive io.helidon.grpc.core;
 
-    requires opentracing.api;
+    requires io.opentracing.api;
     requires opentracing.grpc;
 }
diff --git a/grpc/server/src/main/java/io/helidon/grpc/server/GrpcTracing.java b/grpc/server/src/main/java/io/helidon/grpc/server/GrpcTracing.java
@@ -39,7 +39,7 @@
 import io.opentracing.contrib.grpc.OpenTracingContextKey;
 import io.opentracing.contrib.grpc.OperationNameConstructor;
 import io.opentracing.propagation.Format;
-import io.opentracing.propagation.TextMapExtractAdapter;
+import io.opentracing.propagation.TextMapAdapter;
 
 /**
  * A {@link ServerInterceptor} that adds tracing to gRPC service calls.
@@ -147,7 +147,7 @@ private Span getSpanFromHeaders(Map<String, String> headers, String operationNam
 
         try {
             SpanContext parentSpanCtx = tracer.extract(Format.Builtin.HTTP_HEADERS,
-                                                       new TextMapExtractAdapter(headers));
+                                                       new TextMapAdapter(headers));
             if (parentSpanCtx == null) {
                 span = tracer.buildSpan(operationName)
                         .start();

diff --git a/grpc/server/src/main/java/module-info.java b/grpc/server/src/main/java/module-info.java
@@ -30,7 +30,7 @@
 
     requires transitive grpc.services;
     requires transitive microprofile.health.api;
-    requires transitive opentracing.api;
+    requires transitive io.opentracing.api;
     requires transitive opentracing.grpc;
 
     requires java.annotation;

diff --git a/microprofile/tracing/src/main/java/io/helidon/microprofile/tracing/MpTracingInterceptor.java b/microprofile/tracing/src/main/java/io/helidon/microprofile/tracing/MpTracingInterceptor.java
@@ -85,8 +85,9 @@ private <E extends Member & AnnotatedElement> Object trace(InvocationContext con
 
             parentSpan.ifPresent(spanBuilder::asChildOf);
 
-            Scope scope = spanBuilder.startActive(false);
-            Span span = scope.span();
+            Span span = spanBuilder.start();
+            Scope scope = tracer.scopeManager().activate(span);
+
             try {
                 return context.proceed();
             } catch (Exception e) {

diff --git a/microprofile/tracing/src/main/java/module-info.java b/microprofile/tracing/src/main/java/module-info.java
@@ -23,7 +23,7 @@
 
     requires java.ws.rs;
     requires jersey.common;
-    requires opentracing.api;
+    requires io.opentracing.api;
 
     requires static cdi.api;
     requires static javax.inject;

diff --git a/microprofile/tracing/src/test/java/io/helidon/microprofile/tracing/TestTracerProvider.java b/microprofile/tracing/src/test/java/io/helidon/microprofile/tracing/TestTracerProvider.java
@@ -31,11 +31,11 @@
 import io.opentracing.SpanContext;
 import io.opentracing.Tracer;
 import io.opentracing.propagation.Format;
-import io.opentracing.propagation.TextMapInjectAdapter;
+import io.opentracing.propagation.TextMapAdapter;
 import io.opentracing.util.GlobalTracer;
 
 /**
- * TODO javadoc.
+ * Testing tracer provider.
  */
 public class TestTracerProvider implements TracerProvider {
     @Override
@@ -112,27 +112,24 @@ static class TestTracer implements Tracer {
         public ScopeManager scopeManager() {
             return new ScopeManager() {
                 private Scope active;
+                private Span activeSpan;
+
                 @Override
-                public Scope activate(Span span, boolean finishSpanOnClose) {
+                public Scope activate(Span span) {
                     active = new Scope() {
                         @Override
                         public void close() {
-                            if (finishSpanOnClose) {
-                                span.finish();
-                            }
+                            activeSpan = null;
                         }
 
-                        @Override
-                        public Span span() {
-                            return span;
-                        }
                     };
+                    activeSpan = span;
                     return active;
                 }
 
                 @Override
-                public Scope active() {
-                    return active;
+                public Span activeSpan() {
+                    return activeSpan;
                 }
             };
         }
@@ -142,6 +139,16 @@ public Span activeSpan() {
             return null;
         }
 
+        @Override
+        public Scope activateSpan(Span span) {
+            return null;
+        }
+
+        @Override
+        public void close() {
+
+        }
+
         @Override
         public SpanBuilder buildSpan(String operationName) {
             return new TestSpanBuilder(operationName);
@@ -151,9 +158,9 @@ public SpanBuilder buildSpan(String operationName) {
 
         @Override
         public <C> void inject(SpanContext spanContext, Format<C> format, C carrier) {
-            TestSpan span = ((TestSpanContext)spanContext).testSpan;
+            TestSpan span = ((TestSpanContext) spanContext).testSpan;
 
-            TextMapInjectAdapter adapter = (TextMapInjectAdapter) carrier;
+            TextMapAdapter adapter = (TextMapAdapter) carrier;
             adapter.put(OPERATION_NAME_HEADER, span.operationName);
         }
 
@@ -169,7 +176,6 @@ static class TestSpanBuilder implements Tracer.SpanBuilder {
 
         private SpanContext parent;
 
-
         public TestSpanBuilder(String operationName) {
             this.operationName = operationName;
         }
@@ -219,26 +225,22 @@ public Tracer.SpanBuilder withStartTimestamp(long microseconds) {
             return this;
         }
 
-        @Override
-        public Scope startActive(boolean finishSpanOnClose) {
-             return new TestScope((TestSpan) start());
-        }
-
-        @Override
-        public Span startManual() {
-            return start();
-        }
-
         @Override
         public Span start() {
-            TestSpan result = new TestSpan(this);;
+            TestSpan result = new TestSpan(this);
+            ;
 
             if (null != parent) {
-                ((TestSpanContext)parent).testSpan.addChild(result);
+                ((TestSpanContext) parent).testSpan.addChild(result);
             }
             return result;
 
         }
+
+        @Override
+        public <T> Tracer.SpanBuilder withTag(io.opentracing.tag.Tag<T> tag, T value) {
+            return this;
+        }
     }
 
     static final class TestSpan implements Span {
@@ -262,6 +264,11 @@ public SpanContext context() {
             return new TestSpanContext(this);
         }
 
+        @Override
+        public <T> Span setTag(io.opentracing.tag.Tag<T> tag, T value) {
+            return this;
+        }
+
         @Override
         public Span setTag(String key, String value) {
             return this;
@@ -336,22 +343,15 @@ public Iterable<Map.Entry<String, String>> baggageItems() {
 
             return map.entrySet();
         }
-    }
-
-    static class TestScope implements Scope {
-        private final TestSpan testSpan;
-
-        TestScope(TestSpan testSpan) {
-            this.testSpan = testSpan;
-        }
 
         @Override
-        public void close() {
+        public String toTraceId() {
+            return testSpan.parent.toTraceId();
         }
 
         @Override
-        public Span span() {
-            return testSpan;
+        public String toSpanId() {
+            return testSpan.toString();
         }
     }
 }
diff --git a/security/security/src/main/java/module-info.java b/security/security/src/main/java/module-info.java
@@ -27,14 +27,14 @@
     requires transitive io.helidon.common.configurable;
     requires transitive io.helidon.common.reactive;
     requires transitive io.helidon.config;
-    requires transitive opentracing.api;
+    requires transitive io.opentracing.api;
     // noop and api expose the same package :(
-    // requires opentracing.noop;
-    requires opentracing.util;
+    // requires io.opentracing.noop;
+    requires io.opentracing.util;
 
     requires io.helidon.security.util;
     requires io.helidon.common.context;
-    requires opentracing.noop;
+    requires io.opentracing.noop;
 
     exports io.helidon.security;
     exports io.helidon.security.spi;

diff --git a/tracing/jaeger/src/main/java/module-info.java b/tracing/jaeger/src/main/java/module-info.java
@@ -23,9 +23,10 @@
     requires io.helidon.tracing;
 
     requires java.logging;
-    requires opentracing.util;
+    requires io.opentracing.util;
     requires jaeger.client;
     requires jaeger.core;
+    requires io.opentracing.noop;
 
     exports io.helidon.tracing.jaeger;
 

diff --git a/...ing/jersey-client/src/main/java/io/helidon/tracing/jersey/client/ClientTracingFilter.java b/...ing/jersey-client/src/main/java/io/helidon/tracing/jersey/client/ClientTracingFilter.java
@@ -45,7 +45,7 @@
 import io.opentracing.SpanContext;
 import io.opentracing.Tracer;
 import io.opentracing.propagation.Format;
-import io.opentracing.propagation.TextMapInjectAdapter;
+import io.opentracing.propagation.TextMapAdapter;
 import io.opentracing.tag.Tags;
 import io.opentracing.util.GlobalTracer;
 
@@ -325,7 +325,7 @@ private Map<String, List<String>> tracingHeaders(Tracer tracer, Span currentSpan
 
         tracer.inject(currentSpan.context(),
                       Format.Builtin.HTTP_HEADERS,
-                      new TextMapInjectAdapter(tracerHeaders));
+                      new TextMapAdapter(tracerHeaders));
 
         return new HashMap<>(tracerHeaders.entrySet()
                                      .stream()

diff --git a/tracing/jersey-client/src/main/java/module-info.java b/tracing/jersey-client/src/main/java/module-info.java
@@ -28,8 +28,8 @@
     requires jersey.client;
     requires jersey.common;
 
-    requires opentracing.api;
-    requires opentracing.util;
+    requires io.opentracing.api;
+    requires io.opentracing.util;
 
     requires io.helidon.tracing;
     requires io.helidon.tracing.config;

diff --git a/tracing/jersey/src/main/java/io/helidon/tracing/jersey/AbstractTracingFilter.java b/tracing/jersey/src/main/java/io/helidon/tracing/jersey/AbstractTracingFilter.java
@@ -83,7 +83,7 @@ public void filter(ContainerRequestContext requestContext) {
             configureSpan(spanBuilder);
 
             Span span = spanBuilder.start();
-            Scope spanScope = tracer.scopeManager().activate(span, false);
+            Scope spanScope = tracer.scopeManager().activate(span);
 
             requestContext.setProperty(SPAN_PROPERTY, span);
             requestContext.setProperty(SPAN_SCOPE_PROPERTY, spanScope);

diff --git a/tracing/jersey/src/main/java/module-info.java b/tracing/jersey/src/main/java/module-info.java
@@ -23,7 +23,7 @@
 
     requires java.ws.rs;
     requires jersey.server;
-    requires opentracing.api;
+    requires io.opentracing.api;
     requires io.helidon.common;
     requires io.helidon.common.context;
     requires io.helidon.jersey.common;

diff --git a/tracing/tracing/src/main/java/module-info.java b/tracing/tracing/src/main/java/module-info.java
@@ -23,9 +23,9 @@
     requires io.helidon.common;
     requires io.helidon.common.serviceloader;
     requires io.helidon.config;
-    requires transitive opentracing.api;
-    requires opentracing.noop;
-    requires opentracing.util;
+    requires transitive io.opentracing.api;
+    requires io.opentracing.noop;
+    requires io.opentracing.util;
 
     exports io.helidon.tracing;
     exports io.helidon.tracing.spi;

diff --git a/tracing/zipkin/src/main/java/io/helidon/tracing/zipkin/ZipkinScopeManager.java b/tracing/zipkin/src/main/java/io/helidon/tracing/zipkin/ZipkinScopeManager.java
@@ -32,17 +32,22 @@ class ZipkinScopeManager implements ScopeManager {
     }
 
     @Override
-    public Scope activate(Span span, boolean finishSpanOnClose) {
-        Span toActivate = span;
+    public Scope activate(Span span) {
+        return scopeManager.activate(unwrap(span));
+    }
+
+    private Span unwrap(Span span) {
+        Span unwrapped = span;
 
         if (span instanceof ZipkinSpan) {
-            toActivate = ((ZipkinSpan) span).unwrap();
+            unwrapped = ((ZipkinSpan) span).unwrap();
         }
-        return scopeManager.activate(toActivate, finishSpanOnClose);
+
+        return unwrapped;
     }
 
     @Override
-    public Scope active() {
-        return scopeManager.active();
+    public Span activeSpan() {
+        return scopeManager.activeSpan();
     }
 }