You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cxf.apache.org by re...@apache.org on 2017/09/14 00:58:25 UTC

[cxf] branch master updated: CXF-7439: Support OpenTracing Tracer API. Adding more HTTP tags, aligning the client-side behavior with Brave.

This is an automated email from the ASF dual-hosted git repository.

reta pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cxf.git


The following commit(s) were added to refs/heads/master by this push:
     new e92dc70  CXF-7439: Support OpenTracing Tracer API. Adding more HTTP tags, aligning the client-side behavior with Brave.
e92dc70 is described below

commit e92dc70e49ebbf8e2548a14965ce9fed53563978
Author: reta <dr...@gmail.com>
AuthorDate: Wed Sep 13 20:57:59 2017 -0400

    CXF-7439: Support OpenTracing Tracer API. Adding more HTTP tags, aligning the client-side behavior with Brave.
---
 .../AbstractOpenTracingClientProvider.java         | 29 ++++++++++++----------
 .../opentracing/AbstractOpenTracingProvider.java   |  8 ++++--
 .../apache/cxf/tracing/opentracing/TraceScope.java | 10 --------
 .../opentracing/OpenTracingTracingTest.java        | 25 +++++++++++++------
 .../opentracing/OpenTracingTracingTest.java        | 13 +++++++---
 5 files changed, 48 insertions(+), 37 deletions(-)

diff --git a/integration/tracing/tracing-opentracing/src/main/java/org/apache/cxf/tracing/opentracing/AbstractOpenTracingClientProvider.java b/integration/tracing/tracing-opentracing/src/main/java/org/apache/cxf/tracing/opentracing/AbstractOpenTracingClientProvider.java
index fac0147..5dc94f8 100644
--- a/integration/tracing/tracing-opentracing/src/main/java/org/apache/cxf/tracing/opentracing/AbstractOpenTracingClientProvider.java
+++ b/integration/tracing/tracing-opentracing/src/main/java/org/apache/cxf/tracing/opentracing/AbstractOpenTracingClientProvider.java
@@ -32,11 +32,11 @@ import io.opentracing.ActiveSpan;
 import io.opentracing.ActiveSpan.Continuation;
 import io.opentracing.Tracer;
 import io.opentracing.propagation.Format.Builtin;
+import io.opentracing.tag.Tags;
 
 public abstract class AbstractOpenTracingClientProvider extends AbstractTracingProvider {
     protected static final Logger LOG = LogUtils.getL7dLogger(AbstractOpenTracingClientProvider.class);
     protected static final String TRACE_SPAN = "org.apache.cxf.tracing.client.opentracing.span";
-    protected static final String HTTP_STATUS_TAG = "http.status";
     
     private final Tracer tracer;
 
@@ -47,24 +47,29 @@ public abstract class AbstractOpenTracingClientProvider extends AbstractTracingP
     protected TraceScopeHolder<TraceScope> startTraceSpan(final Map<String, List<String>> requestHeaders,
             URI uri, String method) {
 
-        ActiveSpan span = tracer.activeSpan();
-        boolean managed = false;
-        if (span == null) {
+        final ActiveSpan parent = tracer.activeSpan();
+        ActiveSpan span = null; 
+        if (parent == null) {
             span = tracer.buildSpan(buildSpanDescription(uri.toString(), method)).startActive();
-            managed = true;
+        } else {
+            span = tracer.buildSpan(buildSpanDescription(uri.toString(), method)).asChildOf(parent).startActive();
         }
-        
-        tracer.inject(span.context(), Builtin.HTTP_HEADERS, new TextMapInjectAdapter(requestHeaders));
 
+        // Set additional tags 
+        span.setTag(Tags.HTTP_METHOD.getKey(), method);
+        span.setTag(Tags.HTTP_URL.getKey(), uri.toString());
+
+        tracer.inject(span.context(), Builtin.HTTP_HEADERS, new TextMapInjectAdapter(requestHeaders));
+        
         // In case of asynchronous client invocation, the span should be detached as JAX-RS
         // client request / response filters are going to be executed in different threads.
         Continuation continuation = null;
-        if (isAsyncInvocation() && managed) {
+        if (isAsyncInvocation()) {
             continuation = span.capture();
             span.deactivate();
         }
 
-        return new TraceScopeHolder<TraceScope>(new TraceScope(span, continuation, managed), 
+        return new TraceScopeHolder<TraceScope>(new TraceScope(span, continuation), 
             continuation != null /* detached */);
     }
 
@@ -87,10 +92,8 @@ public abstract class AbstractOpenTracingClientProvider extends AbstractTracingP
                 span = scope.getContinuation().activate();
             }
 
-            span.setTag(HTTP_STATUS_TAG, responseStatus);
-            if (scope.isManaged()) {
-                span.close();
-            }
+            span.setTag(Tags.HTTP_STATUS.getKey(), responseStatus);
+            span.close();
         }
     }
 }
diff --git a/integration/tracing/tracing-opentracing/src/main/java/org/apache/cxf/tracing/opentracing/AbstractOpenTracingProvider.java b/integration/tracing/tracing-opentracing/src/main/java/org/apache/cxf/tracing/opentracing/AbstractOpenTracingProvider.java
index 7d337f2..edaef29 100644
--- a/integration/tracing/tracing-opentracing/src/main/java/org/apache/cxf/tracing/opentracing/AbstractOpenTracingProvider.java
+++ b/integration/tracing/tracing-opentracing/src/main/java/org/apache/cxf/tracing/opentracing/AbstractOpenTracingProvider.java
@@ -34,11 +34,11 @@ import io.opentracing.SpanContext;
 import io.opentracing.Tracer;
 import io.opentracing.propagation.Format.Builtin;
 import io.opentracing.propagation.TextMapExtractAdapter;
+import io.opentracing.tag.Tags;
 
 public abstract class AbstractOpenTracingProvider extends AbstractTracingProvider {
     protected static final Logger LOG = LogUtils.getL7dLogger(AbstractOpenTracingProvider.class);
     protected static final String TRACE_SPAN = "org.apache.cxf.tracing.opentracing.span";
-    protected static final String HTTP_STATUS_TAG = "http.status";
     
     protected final Tracer tracer;
     
@@ -64,6 +64,10 @@ public abstract class AbstractOpenTracingProvider extends AbstractTracingProvide
             scope = tracer.buildSpan(buildSpanDescription(uri.getPath(), method)).asChildOf(parent).startActive();
         }
         
+        // Set additional tags
+        scope.setTag(Tags.HTTP_METHOD.getKey(), method);
+        scope.setTag(Tags.HTTP_URL.getKey(), uri.toString());
+        
         // If the service resource is using asynchronous processing mode, the trace
         // scope will be closed in another thread and as such should be detached.
         Continuation continuation = null;
@@ -98,7 +102,7 @@ public abstract class AbstractOpenTracingProvider extends AbstractTracingProvide
                 span = scope.getContinuation().activate();
             }
 
-            span.setTag(HTTP_STATUS_TAG, responseStatus);
+            span.setTag(Tags.HTTP_STATUS.getKey(), responseStatus);
             span.close();
         }
     }
diff --git a/integration/tracing/tracing-opentracing/src/main/java/org/apache/cxf/tracing/opentracing/TraceScope.java b/integration/tracing/tracing-opentracing/src/main/java/org/apache/cxf/tracing/opentracing/TraceScope.java
index 3f04515..7a9762d 100644
--- a/integration/tracing/tracing-opentracing/src/main/java/org/apache/cxf/tracing/opentracing/TraceScope.java
+++ b/integration/tracing/tracing-opentracing/src/main/java/org/apache/cxf/tracing/opentracing/TraceScope.java
@@ -24,16 +24,10 @@ import io.opentracing.ActiveSpan.Continuation;
 public class TraceScope {
     private final ActiveSpan span;
     private final Continuation continuation;
-    private final boolean managed;
     
     TraceScope(final ActiveSpan span, final Continuation continuation) {
-        this(span, continuation, true);
-    }
-    
-    TraceScope(final ActiveSpan span, final Continuation continuation, final boolean managed) {
         this.span = span;
         this.continuation = continuation;
-        this.managed = managed;
     }
     
     public ActiveSpan getSpan() {
@@ -43,8 +37,4 @@ public class TraceScope {
     public Continuation getContinuation() {
         return continuation;
     }
-    
-    public boolean isManaged() {
-        return managed;
-    }
 }
diff --git a/systests/tracing/src/test/java/org/apache/cxf/systest/jaxrs/tracing/opentracing/OpenTracingTracingTest.java b/systests/tracing/src/test/java/org/apache/cxf/systest/jaxrs/tracing/opentracing/OpenTracingTracingTest.java
index adaa08d..48a453c 100644
--- a/systests/tracing/src/test/java/org/apache/cxf/systest/jaxrs/tracing/opentracing/OpenTracingTracingTest.java
+++ b/systests/tracing/src/test/java/org/apache/cxf/systest/jaxrs/tracing/opentracing/OpenTracingTracingTest.java
@@ -290,17 +290,21 @@ public class OpenTracingTracingTest extends AbstractBusClientServerTestBase {
             final Response r = client.get();
             assertEquals(Status.OK.getStatusCode(), r.getStatus());
 
-            assertThat(TestSender.getAllSpans().size(), equalTo(2));
+            assertThat(TestSender.getAllSpans().size(), equalTo(3));
             assertThat(TestSender.getAllSpans().get(0).getOperationName(), equalTo("Get Books"));
             assertThat(TestSender.getAllSpans().get(0).getReferences(), not(empty()));
+            assertThat(TestSender.getAllSpans().get(1).getReferences(), not(empty()));
             assertThat(TestSender.getAllSpans().get(1).getOperationName(), equalTo("GET /bookstore/books"));
+            assertThat(TestSender.getAllSpans().get(2).getOperationName(), equalTo("GET " + client.getCurrentURI()));
+            assertThat(TestSender.getAllSpans().get(2).getReferences(), not(empty()));
         }
 
         // Await till flush happens, usually every second
-        await().atMost(Duration.ONE_SECOND).until(()-> TestSender.getAllSpans().size() == 3);
+        await().atMost(Duration.ONE_SECOND).until(()-> TestSender.getAllSpans().size() == 4);
         
-        assertThat(TestSender.getAllSpans().size(), equalTo(3));
-        assertThat(TestSender.getAllSpans().get(2).getOperationName(), equalTo("test span"));
+        assertThat(TestSender.getAllSpans().size(), equalTo(4));
+        assertThat(TestSender.getAllSpans().get(3).getOperationName(), equalTo("test span"));
+        assertThat(TestSender.getAllSpans().get(3).getReferences(), empty());
     }
 
     @Test
@@ -314,16 +318,21 @@ public class OpenTracingTracingTest extends AbstractBusClientServerTestBase {
             assertEquals(Status.OK.getStatusCode(), r.getStatus());
             assertThat(tracer.activeSpan().context(), equalTo(span.context()));
 
-            assertThat(TestSender.getAllSpans().size(), equalTo(2));
+            assertThat(TestSender.getAllSpans().size(), equalTo(3));
             assertThat(TestSender.getAllSpans().get(0).getOperationName(), equalTo("Get Books"));
+            assertThat(TestSender.getAllSpans().get(0).getReferences(), not(empty()));
             assertThat(TestSender.getAllSpans().get(1).getOperationName(), equalTo("GET /bookstore/books"));
+            assertThat(TestSender.getAllSpans().get(1).getReferences(), not(empty()));
+            assertThat(TestSender.getAllSpans().get(2).getOperationName(), equalTo("GET " + client.getCurrentURI()));
+            assertThat(TestSender.getAllSpans().get(2).getReferences(), not(empty()));
         }
 
         // Await till flush happens, usually every second
-        await().atMost(Duration.ONE_SECOND).until(()-> TestSender.getAllSpans().size() == 3);
+        await().atMost(Duration.ONE_SECOND).until(()-> TestSender.getAllSpans().size() == 4);
 
-        assertThat(TestSender.getAllSpans().size(), equalTo(3));
-        assertThat(TestSender.getAllSpans().get(2).getOperationName(), equalTo("test span"));
+        assertThat(TestSender.getAllSpans().size(), equalTo(4));
+        assertThat(TestSender.getAllSpans().get(3).getOperationName(), equalTo("test span"));
+        assertThat(TestSender.getAllSpans().get(3).getReferences(), empty());
     }
 
     @Test
diff --git a/systests/tracing/src/test/java/org/apache/cxf/systest/jaxws/tracing/opentracing/OpenTracingTracingTest.java b/systests/tracing/src/test/java/org/apache/cxf/systest/jaxws/tracing/opentracing/OpenTracingTracingTest.java
index e1413a3..3e86b7a 100644
--- a/systests/tracing/src/test/java/org/apache/cxf/systest/jaxws/tracing/opentracing/OpenTracingTracingTest.java
+++ b/systests/tracing/src/test/java/org/apache/cxf/systest/jaxws/tracing/opentracing/OpenTracingTracingTest.java
@@ -163,17 +163,22 @@ public class OpenTracingTracingTest extends AbstractBusClientServerTestBase {
             assertThat(service.getBooks().size(), equalTo(2));
             assertThat(tracer.activeSpan(), not(nullValue()));
 
-            assertThat(TestSender.getAllSpans().size(), equalTo(2));
+            assertThat(TestSender.getAllSpans().size(), equalTo(3));
             assertThat(TestSender.getAllSpans().get(0).getOperationName(), equalTo("Get Books"));
             assertThat(TestSender.getAllSpans().get(0).getReferences(), not(empty()));
             assertThat(TestSender.getAllSpans().get(1).getOperationName(), equalTo("POST /BookStore"));
+            assertThat(TestSender.getAllSpans().get(1).getReferences(), not(empty()));
+            assertThat(TestSender.getAllSpans().get(2).getOperationName(),
+                equalTo("POST http://localhost:" + PORT + "/BookStore"));
+            assertThat(TestSender.getAllSpans().get(2).getReferences(), not(empty()));
         }
 
         // Await till flush happens, usually every second
-        await().atMost(Duration.ONE_SECOND).until(()-> TestSender.getAllSpans().size() == 3);
+        await().atMost(Duration.ONE_SECOND).until(()-> TestSender.getAllSpans().size() == 4);
 
-        assertThat(TestSender.getAllSpans().size(), equalTo(3));
-        assertThat(TestSender.getAllSpans().get(2).getOperationName(), equalTo("test span"));
+        assertThat(TestSender.getAllSpans().size(), equalTo(4));
+        assertThat(TestSender.getAllSpans().get(3).getOperationName(), equalTo("test span"));
+        assertThat(TestSender.getAllSpans().get(3).getReferences(), empty());
     }
 
     @Test

-- 
To stop receiving notification emails like this one, please contact
['"commits@cxf.apache.org" <co...@cxf.apache.org>'].