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 ac561c4 Fixing an issue with pending spans (created by joinSpan call, should be discarded), stabilizing tracing tests ac561c4 is described below commit ac561c4a2b27bd8b427ff1f91d19c7929a8222ab Author: reta <drr...@gmail.com> AuthorDate: Wed Aug 29 21:29:01 2018 -0400 Fixing an issue with pending spans (created by joinSpan call, should be discarded), stabilizing tracing tests --- .../org/apache/cxf/tracing/brave/AbstractBraveProvider.java | 8 +++++++- .../java/org/apache/cxf/systest/brave/TestSpanReporter.java | 3 ++- .../java/org/apache/cxf/systest/htrace/TestSpanReceiver.java | 3 ++- .../test/java/org/apache/cxf/systest/jaeger/TestSender.java | 3 ++- .../cxf/systest/jaxrs/tracing/brave/BraveTracingTest.java | 10 +++++++++- 5 files changed, 22 insertions(+), 5 deletions(-) diff --git a/integration/tracing/tracing-brave/src/main/java/org/apache/cxf/tracing/brave/AbstractBraveProvider.java b/integration/tracing/tracing-brave/src/main/java/org/apache/cxf/tracing/brave/AbstractBraveProvider.java index 01dfa0c..d95db3e 100644 --- a/integration/tracing/tracing-brave/src/main/java/org/apache/cxf/tracing/brave/AbstractBraveProvider.java +++ b/integration/tracing/tracing-brave/src/main/java/org/apache/cxf/tracing/brave/AbstractBraveProvider.java @@ -92,13 +92,14 @@ public abstract class AbstractBraveProvider extends AbstractTracingProvider { } final TraceScope scope = holder.getScope(); + Span span = null; if (scope != null) { try { // If the service resource is using asynchronous processing mode, the trace // scope 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()); + span = brave.tracing().tracer().joinSpan(scope.getSpan().context()); } final Response response = HttpAdapterFactory.response(responseStatus); @@ -108,6 +109,11 @@ public abstract class AbstractBraveProvider extends AbstractTracingProvider { handler.handleSend(response, null, scope.getSpan()); } finally { scope.close(); + if (span != null) { + // We do not care about the span created by joinSpan, since it + // should be managed by the scope.getSpan() itself. + span.abandon(); + } } } } diff --git a/systests/tracing/src/test/java/org/apache/cxf/systest/brave/TestSpanReporter.java b/systests/tracing/src/test/java/org/apache/cxf/systest/brave/TestSpanReporter.java index fe2413f..4f9b5fa 100644 --- a/systests/tracing/src/test/java/org/apache/cxf/systest/brave/TestSpanReporter.java +++ b/systests/tracing/src/test/java/org/apache/cxf/systest/brave/TestSpanReporter.java @@ -19,13 +19,14 @@ package org.apache.cxf.systest.brave; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import zipkin2.Span; import zipkin2.reporter.Reporter; public class TestSpanReporter implements Reporter<Span> { - private static List<Span> spans = new ArrayList<>(); + private static List<Span> spans = Collections.synchronizedList(new ArrayList<>()); @Override public void report(Span span) { diff --git a/systests/tracing/src/test/java/org/apache/cxf/systest/htrace/TestSpanReceiver.java b/systests/tracing/src/test/java/org/apache/cxf/systest/htrace/TestSpanReceiver.java index fdc1158..a36cf62 100644 --- a/systests/tracing/src/test/java/org/apache/cxf/systest/htrace/TestSpanReceiver.java +++ b/systests/tracing/src/test/java/org/apache/cxf/systest/htrace/TestSpanReceiver.java @@ -21,6 +21,7 @@ package org.apache.cxf.systest.htrace; import java.io.IOException; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.List; import org.apache.htrace.core.HTraceConfiguration; @@ -31,7 +32,7 @@ import org.apache.htrace.core.SpanReceiver; * Test HTrace Span receiver */ public class TestSpanReceiver extends SpanReceiver { - private static List<Span> spans = new ArrayList<>(); + private static List<Span> spans = Collections.synchronizedList(new ArrayList<>()); public TestSpanReceiver(final HTraceConfiguration conf) { } diff --git a/systests/tracing/src/test/java/org/apache/cxf/systest/jaeger/TestSender.java b/systests/tracing/src/test/java/org/apache/cxf/systest/jaeger/TestSender.java index 7daa339..a7bea38 100644 --- a/systests/tracing/src/test/java/org/apache/cxf/systest/jaeger/TestSender.java +++ b/systests/tracing/src/test/java/org/apache/cxf/systest/jaeger/TestSender.java @@ -19,6 +19,7 @@ package org.apache.cxf.systest.jaeger; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import io.jaegertracing.internal.JaegerSpan; @@ -26,7 +27,7 @@ import io.jaegertracing.internal.exceptions.SenderException; import io.jaegertracing.spi.Sender; public class TestSender implements Sender { - private static List<JaegerSpan> spans = new ArrayList<>(); + private static List<JaegerSpan> spans = Collections.synchronizedList(new ArrayList<>()); @Override public int append(JaegerSpan span) throws SenderException { 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 30486f9..2ec7520 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 @@ -51,6 +51,7 @@ import org.apache.cxf.testutil.common.AbstractBusTestServerBase; 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.awaitility.Duration; import org.junit.Before; import org.junit.BeforeClass; @@ -63,6 +64,7 @@ import static org.apache.cxf.systest.brave.BraveTestSupport.SPAN_ID_NAME; import static org.apache.cxf.systest.brave.BraveTestSupport.TRACE_ID_NAME; import static org.apache.cxf.systest.jaxrs.tracing.brave.HasSpan.hasSpan; import static org.apache.cxf.systest.jaxrs.tracing.brave.IsAnnotationContaining.hasItem; +import static org.awaitility.Awaitility.await; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.CoreMatchers.nullValue; @@ -325,6 +327,9 @@ public class BraveTracingTest extends AbstractBusClientServerTestBase { } finally { span.finish(); } + + // Await till flush happens, usually a second is enough + await().atMost(Duration.ONE_SECOND).until(()-> TestSpanReporter.getAllSpans().size() == 4); assertThat(TestSpanReporter.getAllSpans().size(), equalTo(4)); assertThat(TestSpanReporter.getAllSpans().get(3).name(), equalTo("test span")); @@ -342,7 +347,7 @@ public class BraveTracingTest extends AbstractBusClientServerTestBase { final Response r = f.get(1, TimeUnit.SECONDS); assertEquals(Status.OK.getStatusCode(), r.getStatus()); assertThat(brave.tracer().currentSpan().context().spanId(), equalTo(span.context().spanId())); - + assertThat(TestSpanReporter.getAllSpans().size(), equalTo(3)); assertThat(TestSpanReporter.getAllSpans().get(0).name(), equalTo("get books")); assertThat(TestSpanReporter.getAllSpans().get(1).name(), equalTo("get /bookstore/books")); @@ -354,6 +359,9 @@ public class BraveTracingTest extends AbstractBusClientServerTestBase { span.finish(); } + // Await till flush happens, usually a second is enough + await().atMost(Duration.ONE_SECOND).until(()-> TestSpanReporter.getAllSpans().size() == 4); + assertThat(TestSpanReporter.getAllSpans().size(), equalTo(4)); assertThat(TestSpanReporter.getAllSpans().get(3).name(), equalTo("test span")); }