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 <drr...@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" <commits@cxf.apache.org>'].