[cxf] branch 3.2.x-fixes updated (720a186 -> 1292ce0)

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[cxf] branch 3.2.x-fixes updated (720a186 -> 1292ce0)

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

reta pushed a change to branch 3.2.x-fixes
in repository https://gitbox.apache.org/repos/asf/cxf.git.


    from 720a186  Recording .gitmergeinfo Changes
     new f212b2a  CXF-8169: Span#finish is never called in case of client timeouts
     new 1292ce0  Recording .gitmergeinfo Changes

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .gitmergeinfo                                      |  2 ++
 .../tracing/brave/AbstractBraveClientProvider.java | 22 +++++++++++++
 .../tracing/brave/BraveClientStartInterceptor.java | 10 ++++++
 .../AbstractOpenTracingClientProvider.java         | 29 +++++++++++++++++
 .../OpenTracingClientStartInterceptor.java         | 11 ++++++-
 .../cxf/systest/jaxrs/tracing/BookStore.java       | 15 +++++++++
 .../jaxrs/tracing/brave/BraveTracingTest.java      | 37 ++++++++++++++++++++++
 .../jaxrs/tracing/opentracing/IsTagContaining.java |  4 +--
 .../opentracing/OpenTracingTracingTest.java        | 36 ++++++++++++++++++++-
 9 files changed, 162 insertions(+), 4 deletions(-)

Reply | Threaded
Open this post in threaded view
|

[cxf] 01/02: CXF-8169: Span#finish is never called in case of client timeouts

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

reta pushed a commit to branch 3.2.x-fixes
in repository https://gitbox.apache.org/repos/asf/cxf.git

commit f212b2ac0a569d428f07fe3ba2a557a38b8e9283
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
   
    (cherry picked from commit 9ad4ecac3c5444fc08e9d48cc5a1c60e4bb02b4c)
   
    # Conflicts:
    # systests/tracing/src/test/java/org/apache/cxf/systest/jaxrs/tracing/BookStore.java
    # systests/tracing/src/test/java/org/apache/cxf/systest/jaxrs/tracing/brave/BraveTracingTest.java
---
 .../tracing/brave/AbstractBraveClientProvider.java | 22 +++++++++++++
 .../tracing/brave/BraveClientStartInterceptor.java | 10 ++++++
 .../AbstractOpenTracingClientProvider.java         | 29 +++++++++++++++++
 .../OpenTracingClientStartInterceptor.java         | 11 ++++++-
 .../cxf/systest/jaxrs/tracing/BookStore.java       | 15 +++++++++
 .../jaxrs/tracing/brave/BraveTracingTest.java      | 37 ++++++++++++++++++++++
 .../jaxrs/tracing/opentracing/IsTagContaining.java |  4 +--
 .../opentracing/OpenTracingTracingTest.java        | 36 ++++++++++++++++++++-
 8 files changed, 160 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 0f8945e..bcbaff9 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;
@@ -96,4 +97,32 @@ public abstract class AbstractOpenTracingClientProvider extends AbstractTracingP
             span.close();
         }
     }
+    
+    protected void stopTraceSpan(final TraceScopeHolder<TraceScope> holder, final Throwable ex) {
+        if (holder == null) {
+            return;
+        }
+
+        final TraceScope scope = holder.getScope();
+        if (scope != null) {
+            ActiveSpan span = scope.getSpan();
+            
+            // 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()) {
+                span = scope.getContinuation().activate();
+            }
+
+
+            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.setTag(Tags.ERROR.getKey(), Boolean.TRUE);
+            span.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 9be6a02..3b5a4a0 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
@@ -164,4 +164,19 @@ 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()),
+                new Book("Mastering Apache CXF", 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 2ec7520..cfc3534 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;
@@ -69,10 +75,17 @@ import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.CoreMatchers.not;
 import static org.hamcrest.CoreMatchers.nullValue;
 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;
 
 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;
@@ -395,6 +408,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 d2cab4b..3c0dd4f 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;
@@ -31,6 +32,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,19 +50,24 @@ 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.opentracing.ActiveSpan;
 import io.opentracing.Tracer;
 import io.opentracing.propagation.Format.Builtin;
 import io.opentracing.propagation.TextMap;
+import io.opentracing.tag.Tags;
 
 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;
@@ -73,6 +80,9 @@ import static org.hamcrest.Matchers.empty;
 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 Random random;
@@ -94,7 +104,7 @@ public class OpenTracingTracingTest extends AbstractBusClientServerTestBase {
             sf.create();
         }
     }
-
+    
     @BeforeClass
     public static void startServers() throws Exception {
         AbstractResourceInfo.clearAllMaps();
@@ -348,6 +358,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))

Reply | Threaded
Open this post in threaded view
|

[cxf] 02/02: Recording .gitmergeinfo Changes

reta
In reply to this post by reta
This is an automated email from the ASF dual-hosted git repository.

reta pushed a commit to branch 3.2.x-fixes
in repository https://gitbox.apache.org/repos/asf/cxf.git

commit 1292ce0bf4e9498a6d95068de56319265b7fbd2f
Author: reta <[hidden email]>
AuthorDate: Mon Dec 2 20:06:48 2019 -0500

    Recording .gitmergeinfo Changes
---
 .gitmergeinfo | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/.gitmergeinfo b/.gitmergeinfo
index 3e6cc91..0ffbfa6 100644
--- a/.gitmergeinfo
+++ b/.gitmergeinfo
@@ -22,6 +22,7 @@ B 055a2ec152d9945b0817c2fac82af43d0692bfd5
 B 057f4dc521f02227d7b36bbb508835a9edb69e15
 B 07251d65ed30455dd982fdbe694aab419d2fbfcb
 B 07696f88f3d5bae43c4a248c26fdf24a916fd5f5
+B 07faf9e4b03a2e15d9aa7885dacc63d1797df38f
 B 0861dd6eefeb15cd3f642ff6144159d047445c97
 B 0883d9177d54fccfd76de51c9f8715f48a336d2e
 B 08f7177ac2746b70e41c21bcbe9fb62f47a3b996
@@ -797,6 +798,7 @@ M 961227f92b14856e5477f8f209ad9ced340c56ce
 M 99028cc48e200ea0c36fe2bee87bf813e513a513
 M 9a2f1212cd7031c8447ae4294c4c4ccc7322d9e5
 M 9ab0d2766695a2ba9f1ed1ca042b5d2a42eb4fd9
+M 9ad4ecac3c5444fc08e9d48cc5a1c60e4bb02b4c
 M 9bca6952c145b6ec6d2fb5726676ad7c3d67b7f7
 M 9c02f616f6aec1cf43f1d6bac810d606dab98a72
 M 9e1788a8e08264a2735ba5373a96f09215d60769