This is an automated email from the ASF dual-hosted git repository.
reta pushed a commit to branch 3.3.x-fixes
in repository
https://gitbox.apache.org/repos/asf/cxf.gitcommit 9ad4ecac3c5444fc08e9d48cc5a1c60e4bb02b4c
Author: reta <
[hidden email]>
AuthorDate: Sun Dec 1 17:16:38 2019 -0500
CXF-8169: Span#finish is never called in case of client timeouts
(cherry picked from commit 7b50181ebc445bb84d3463b284643432399bc0f8)
# Conflicts:
# systests/tracing/src/test/java/org/apache/cxf/systest/jaxrs/tracing/brave/BraveTracingTest.java
# systests/tracing/src/test/java/org/apache/cxf/systest/jaxrs/tracing/opentracing/OpenTracingTracingTest.java
---
.../tracing/brave/AbstractBraveClientProvider.java | 22 +++++++++++++
.../tracing/brave/BraveClientStartInterceptor.java | 10 ++++++
.../AbstractOpenTracingClientProvider.java | 30 ++++++++++++++++++
.../OpenTracingClientStartInterceptor.java | 11 ++++++-
.../cxf/systest/jaxrs/tracing/BookStore.java | 8 +++++
.../jaxrs/tracing/brave/BraveTracingTest.java | 34 ++++++++++++++++++++
.../jaxrs/tracing/opentracing/IsTagContaining.java | 4 +--
.../opentracing/OpenTracingTracingTest.java | 36 +++++++++++++++++++++-
8 files changed, 151 insertions(+), 4 deletions(-)
diff --git a/integration/tracing/tracing-brave/src/main/java/org/apache/cxf/tracing/brave/AbstractBraveClientProvider.java b/integration/tracing/tracing-brave/src/main/java/org/apache/cxf/tracing/brave/AbstractBraveClientProvider.java
index 4c4c5e2..2f4d7ed 100644
--- a/integration/tracing/tracing-brave/src/main/java/org/apache/cxf/tracing/brave/AbstractBraveClientProvider.java
+++ b/integration/tracing/tracing-brave/src/main/java/org/apache/cxf/tracing/brave/AbstractBraveClientProvider.java
@@ -108,4 +108,26 @@ public abstract class AbstractBraveClientProvider extends AbstractTracingProvide
}
}
}
+
+ protected void stopTraceSpan(final TraceScopeHolder<TraceScope> holder, final Throwable ex) {
+ if (holder == null) {
+ return;
+ }
+
+ final TraceScope scope = holder.getScope();
+ if (scope != null) {
+ try {
+ // If the client invocation was asynchronous , the trace span has been created
+ // in another thread and should be re-attached to the current one.
+ if (holder.isDetached()) {
+ brave.tracing().tracer().joinSpan(scope.getSpan().context());
+ }
+
+ final HttpClientHandler<?, Response> handler = HttpClientHandler.create(brave, null);
+ handler.handleReceive(null, ex, scope.getSpan());
+ } finally {
+ scope.close();
+ }
+ }
+ }
}
diff --git a/integration/tracing/tracing-brave/src/main/java/org/apache/cxf/tracing/brave/BraveClientStartInterceptor.java b/integration/tracing/tracing-brave/src/main/java/org/apache/cxf/tracing/brave/BraveClientStartInterceptor.java
index 2916c41..d33c4ad 100644
--- a/integration/tracing/tracing-brave/src/main/java/org/apache/cxf/tracing/brave/BraveClientStartInterceptor.java
+++ b/integration/tracing/tracing-brave/src/main/java/org/apache/cxf/tracing/brave/BraveClientStartInterceptor.java
@@ -44,4 +44,14 @@ public class BraveClientStartInterceptor extends AbstractBraveClientInterceptor
message.getExchange().put(TRACE_SPAN, holder);
}
}
+
+ @Override
+ public void handleFault(Message message) {
+ @SuppressWarnings("unchecked")
+ final TraceScopeHolder<TraceScope> holder =
+ (TraceScopeHolder<TraceScope>)message.getExchange().get(TRACE_SPAN);
+
+ final Exception ex = message.getContent(Exception.class);
+ super.stopTraceSpan(holder, ex);
+ }
}
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 ef9a526..2da43c5 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
@@ -19,6 +19,7 @@
package org.apache.cxf.tracing.opentracing;
import java.net.URI;
+import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.logging.Logger;
@@ -103,4 +104,33 @@ public abstract class AbstractOpenTracingClientProvider extends AbstractTracingP
scope.close();
}
}
+
+ protected void stopTraceSpan(final TraceScopeHolder<TraceScope> holder, final Throwable ex) {
+ if (holder == null) {
+ return;
+ }
+
+ final TraceScope traceScope = holder.getScope();
+ if (traceScope != null) {
+ Span span = traceScope.getSpan();
+ Scope scope = traceScope.getScope();
+
+ // If the client invocation was asynchronous , the trace span has been created
+ // in another thread and should be re-attached to the current one.
+ if (holder.isDetached()) {
+ scope = tracer.scopeManager().activate(span);
+ }
+
+ span.setTag(Tags.ERROR.getKey(), Boolean.TRUE);
+ if (ex != null) {
+ final Map<String, Object> logEvent = new HashMap<>(2);
+ logEvent.put("event", Tags.ERROR.getKey());
+ logEvent.put("message", ex.getMessage());
+ span.log(logEvent);
+ }
+ span.finish();
+
+ scope.close();
+ }
+ }
}
diff --git a/integration/tracing/tracing-opentracing/src/main/java/org/apache/cxf/tracing/opentracing/OpenTracingClientStartInterceptor.java b/integration/tracing/tracing-opentracing/src/main/java/org/apache/cxf/tracing/opentracing/OpenTracingClientStartInterceptor.java
index 011fef6..578fe49 100644
--- a/integration/tracing/tracing-opentracing/src/main/java/org/apache/cxf/tracing/opentracing/OpenTracingClientStartInterceptor.java
+++ b/integration/tracing/tracing-opentracing/src/main/java/org/apache/cxf/tracing/opentracing/OpenTracingClientStartInterceptor.java
@@ -47,5 +47,14 @@ public class OpenTracingClientStartInterceptor extends AbstractOpenTracingClient
message.getExchange().put(TRACE_SPAN, holder);
}
}
-
+
+ @Override
+ public void handleFault(Message message) {
+ @SuppressWarnings("unchecked")
+ final TraceScopeHolder<TraceScope> holder =
+ (TraceScopeHolder<TraceScope>)message.getExchange().get(TRACE_SPAN);
+
+ final Exception ex = message.getContent(Exception.class);
+ super.stopTraceSpan(holder, ex);
+ }
}
diff --git a/systests/tracing/src/test/java/org/apache/cxf/systest/jaxrs/tracing/BookStore.java b/systests/tracing/src/test/java/org/apache/cxf/systest/jaxrs/tracing/BookStore.java
index f460540..1f6d752 100644
--- a/systests/tracing/src/test/java/org/apache/cxf/systest/jaxrs/tracing/BookStore.java
+++ b/systests/tracing/src/test/java/org/apache/cxf/systest/jaxrs/tracing/BookStore.java
@@ -124,6 +124,14 @@ public class BookStore<T extends Closeable> {
return Response.ok().build();
}
+ @GET
+ @Path("/books/long")
+ @Produces(MediaType.APPLICATION_JSON)
+ public Collection<Book> getBooksLong() throws InterruptedException {
+ Thread.sleep(500);
+ return books();
+ }
+
private static Collection<Book> books() {
return Arrays.asList(
new Book("Apache CXF in Action", UUID.randomUUID().toString()),
diff --git a/systests/tracing/src/test/java/org/apache/cxf/systest/jaxrs/tracing/brave/BraveTracingTest.java b/systests/tracing/src/test/java/org/apache/cxf/systest/jaxrs/tracing/brave/BraveTracingTest.java
index 2629c03..7f053bd 100644
--- a/systests/tracing/src/test/java/org/apache/cxf/systest/jaxrs/tracing/brave/BraveTracingTest.java
+++ b/systests/tracing/src/test/java/org/apache/cxf/systest/jaxrs/tracing/brave/BraveTracingTest.java
@@ -21,6 +21,7 @@ package org.apache.cxf.systest.jaxrs.tracing.brave;
import java.net.MalformedURLException;
import java.util.Arrays;
import java.util.Collection;
+import java.util.Collections;
import java.util.Random;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Future;
@@ -29,6 +30,7 @@ import java.util.concurrent.TimeoutException;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
+import javax.ws.rs.ProcessingException;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.Response.Status;
@@ -48,15 +50,19 @@ import org.apache.cxf.systest.brave.TestSpanReporter;
import org.apache.cxf.systest.jaxrs.tracing.BookStore;
import org.apache.cxf.testutil.common.AbstractBusClientServerTestBase;
import org.apache.cxf.testutil.common.AbstractBusTestServerBase;
+import org.apache.cxf.tracing.brave.BraveClientFeature;
import org.apache.cxf.tracing.brave.TraceScope;
import org.apache.cxf.tracing.brave.jaxrs.BraveClientProvider;
import org.apache.cxf.tracing.brave.jaxrs.BraveFeature;
+import org.apache.cxf.transports.http.configuration.HTTPClientPolicy;
import org.awaitility.Duration;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Ignore;
+import org.junit.Rule;
import org.junit.Test;
+import org.junit.rules.ExpectedException;
import static org.apache.cxf.systest.brave.BraveTestSupport.PARENT_SPAN_ID_NAME;
import static org.apache.cxf.systest.brave.BraveTestSupport.SAMPLED_NAME;
@@ -70,6 +76,7 @@ import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.CoreMatchers.nullValue;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.collection.IsMapContaining.hasEntry;
+import static org.hamcrest.collection.IsMapContaining.hasKey;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
@@ -77,6 +84,9 @@ import static org.junit.Assert.assertTrue;
public class BraveTracingTest extends AbstractBusClientServerTestBase {
public static final String PORT = allocatePort(BraveTracingTest.class);
+ @Rule
+ public ExpectedException expectedException = ExpectedException.none();
+
private Tracing brave;
private BraveClientProvider braveClientProvider;
private Random random;
@@ -399,6 +409,30 @@ public class BraveTracingTest extends AbstractBusClientServerTestBase {
assertThatTraceHeadersArePresent(r, false);
}
+ @Test
+ public void testThatNewSpanIsCreatedOnClientTimeout() {
+ final WebClient client = WebClient
+ .create("
http://localhost:" + PORT + "/bookstore/books/long", Collections.emptyList(),
+ Arrays.asList(new BraveClientFeature(brave)), null)
+ .accept(MediaType.APPLICATION_JSON);
+
+ HTTPClientPolicy httpClientPolicy = new HTTPClientPolicy();
+ httpClientPolicy.setConnectionTimeout(100);
+ httpClientPolicy.setReceiveTimeout(100);
+ WebClient.getConfig(client).getHttpConduit().setClient(httpClientPolicy);
+
+ expectedException.expect(ProcessingException.class);
+ try {
+ client.get();
+ } finally {
+ await().atMost(Duration.ONE_SECOND).until(()-> TestSpanReporter.getAllSpans().size() == 2);
+ assertThat(TestSpanReporter.getAllSpans().size(), equalTo(2));
+ assertThat(TestSpanReporter.getAllSpans().get(0).name(), equalTo("get " + client.getCurrentURI()));
+ assertThat(TestSpanReporter.getAllSpans().get(0).tags(), hasKey("error"));
+ assertThat(TestSpanReporter.getAllSpans().get(1).name(), equalTo("get /bookstore/books/long"));
+ }
+ }
+
protected WebClient createWebClient(final String url, final Object ... providers) {
return WebClient
.create("
http://localhost:" + PORT + url, Arrays.asList(providers))
diff --git a/systests/tracing/src/test/java/org/apache/cxf/systest/jaxrs/tracing/opentracing/IsTagContaining.java b/systests/tracing/src/test/java/org/apache/cxf/systest/jaxrs/tracing/opentracing/IsTagContaining.java
index b9ac95a..8cad059 100644
--- a/systests/tracing/src/test/java/org/apache/cxf/systest/jaxrs/tracing/opentracing/IsTagContaining.java
+++ b/systests/tracing/src/test/java/org/apache/cxf/systest/jaxrs/tracing/opentracing/IsTagContaining.java
@@ -23,11 +23,11 @@ import org.hamcrest.collection.IsMapContaining;
import static org.hamcrest.CoreMatchers.equalTo;
public class IsTagContaining extends IsMapContaining<String, Object> {
- public IsTagContaining(final String key, final String value) {
+ public IsTagContaining(final String key, final Object value) {
super(equalTo(key), equalTo(value));
}
- public static IsTagContaining hasItem(final String key, final String value) {
+ public static IsTagContaining hasItem(final String key, final Object value) {
return new IsTagContaining(key, value);
}
}
\ No newline at end of file
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 6cc9227..91c8d41 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
@@ -21,6 +21,7 @@ package org.apache.cxf.systest.jaxrs.tracing.opentracing;
import java.net.MalformedURLException;
import java.util.Arrays;
import java.util.Collection;
+import java.util.Collections;
import java.util.Iterator;
import java.util.Map.Entry;
import java.util.Random;
@@ -32,6 +33,7 @@ import java.util.concurrent.TimeoutException;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
+import javax.ws.rs.ProcessingException;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
import javax.ws.rs.core.Response.Status;
@@ -46,8 +48,10 @@ import org.apache.cxf.systest.jaeger.TestSender;
import org.apache.cxf.systest.jaxrs.tracing.BookStore;
import org.apache.cxf.testutil.common.AbstractBusClientServerTestBase;
import org.apache.cxf.testutil.common.AbstractBusTestServerBase;
+import org.apache.cxf.tracing.opentracing.OpenTracingClientFeature;
import org.apache.cxf.tracing.opentracing.jaxrs.OpenTracingClientProvider;
import org.apache.cxf.tracing.opentracing.jaxrs.OpenTracingFeature;
+import org.apache.cxf.transports.http.configuration.HTTPClientPolicy;
import org.awaitility.Duration;
import io.jaegertracing.Configuration;
@@ -62,12 +66,15 @@ import io.opentracing.Span;
import io.opentracing.Tracer;
import io.opentracing.propagation.Format.Builtin;
import io.opentracing.propagation.TextMap;
+import io.opentracing.tag.Tags;
import org.junit.After;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Ignore;
+import org.junit.Rule;
import org.junit.Test;
+import org.junit.rules.ExpectedException;
import static org.apache.cxf.systest.jaxrs.tracing.opentracing.HasSpan.hasSpan;
import static org.apache.cxf.systest.jaxrs.tracing.opentracing.IsLogContaining.hasItem;
@@ -83,6 +90,9 @@ import static org.junit.Assert.assertTrue;
public class OpenTracingTracingTest extends AbstractBusClientServerTestBase {
public static final String PORT = allocatePort(OpenTracingTracingTest.class);
+ @Rule
+ public ExpectedException expectedException = ExpectedException.none();
+
private Tracer tracer;
private OpenTracingClientProvider openTracingClientProvider;
private final Random random = new Random();
@@ -111,7 +121,7 @@ public class OpenTracingTracingTest extends AbstractBusClientServerTestBase {
sf.create();
}
}
-
+
@BeforeClass
public static void startServers() throws Exception {
AbstractResourceInfo.clearAllMaps();
@@ -398,6 +408,30 @@ public class OpenTracingTracingTest extends AbstractBusClientServerTestBase {
assertThat(TestSender.getAllSpans().get(0).getOperationName(), equalTo("Processing books"));
}
+ @Test
+ public void testThatNewSpanIsCreatedOnClientTimeout() {
+ final WebClient client = WebClient
+ .create("
http://localhost:" + PORT + "/bookstore/books/long", Collections.emptyList(),
+ Arrays.asList(new OpenTracingClientFeature(tracer)), null)
+ .accept(MediaType.APPLICATION_JSON);
+
+ HTTPClientPolicy httpClientPolicy = new HTTPClientPolicy();
+ httpClientPolicy.setConnectionTimeout(100);
+ httpClientPolicy.setReceiveTimeout(100);
+ WebClient.getConfig(client).getHttpConduit().setClient(httpClientPolicy);
+
+ expectedException.expect(ProcessingException.class);
+ try {
+ client.get();
+ } finally {
+ await().atMost(Duration.ONE_SECOND).until(()-> TestSender.getAllSpans().size() == 2);
+ assertThat(TestSender.getAllSpans().toString(), TestSender.getAllSpans().size(), equalTo(2));
+ assertThat(TestSender.getAllSpans().get(0).getOperationName(), equalTo("GET " + client.getCurrentURI()));
+ assertThat(TestSender.getAllSpans().get(0).getTags(), hasItem(Tags.ERROR.getKey(), Boolean.TRUE));
+ assertThat(TestSender.getAllSpans().get(1).getOperationName(), equalTo("GET /bookstore/books/long"));
+ }
+ }
+
protected WebClient createWebClient(final String url, final Object ... providers) {
return WebClient
.create("
http://localhost:" + PORT + url, Arrays.asList(providers))
Locked
