[cxf] branch 3.3.x-fixes updated (04454ed -> 07faf9e)

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

[cxf] branch 3.3.x-fixes updated (04454ed -> 07faf9e)

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

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


    from 04454ed  Recording .gitmergeinfo Changes
     new 9ad4eca  CXF-8169: Span#finish is never called in case of client timeouts
     new 07faf9e  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                                      |  1 +
 .../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 +++++++++++++++++++++-
 9 files changed, 152 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.3.x-fixes
in repository https://gitbox.apache.org/repos/asf/cxf.git

commit 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))

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.3.x-fixes
in repository https://gitbox.apache.org/repos/asf/cxf.git

commit 07faf9e4b03a2e15d9aa7885dacc63d1797df38f
Author: reta <[hidden email]>
AuthorDate: Sun Dec 1 21:32:47 2019 -0500

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

diff --git a/.gitmergeinfo b/.gitmergeinfo
index 56718d8..09bc259 100644
--- a/.gitmergeinfo
+++ b/.gitmergeinfo
@@ -55,6 +55,7 @@ M 589e321622b72ae57fab910d76b3e4fb0c160979
 M 59ba4821c92146723df063f9eb6e1c3ae0be5834
 M 5aa0660a9fe324e5c8f36e47e270971d0e85b6f8
 M 6b7e50b87d74dc6b7d1e830b373c7e30c04e14a2
+M 7b50181ebc445bb84d3463b284643432399bc0f8
 M 7ef814556d727c147d6f625cbb1170edfd24a752
 M 9c462e9bd614d529f0a2fd86490243ea1c858651
 M a4fc3a0462f09dd6c6c920385b67e621d7e36399