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 c28f7c57d5e0335035a88293a7023811b9e8a180 Author: reta <[email protected]> AuthorDate: Wed Feb 26 21:36:48 2020 -0500 CXF-8220: The tag http.status_code is always 200 in server side tracing span, no matter what situation (cherry picked from commit 67643a4fe883f5d6067d4ce07f48cf4de10be9f7) # 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 # systests/tracing/src/test/java/org/apache/cxf/systest/jaxws/tracing/brave/BraveTracingTest.java # systests/tracing/src/test/java/org/apache/cxf/systest/jaxws/tracing/opentracing/OpenTracingTracingTest.java --- .../cxf/tracing/brave/AbstractBraveProvider.java | 16 -- .../cxf/tracing/brave/BraveStopInterceptor.java | 2 +- .../opentracing/OpenTracingStopInterceptor.java | 2 +- .../cxf/systest/jaxrs/tracing/BookStore.java | 24 ++- .../tracing/NullPointerExceptionMapper.java} | 19 +- .../jaxrs/tracing/brave/BraveTracingTest.java | 93 +++++----- .../opentracing/OpenTracingTracingTest.java | 32 ++++ .../systest/jaxws/tracing/BookStoreService.java | 1 + .../cxf/systest/jaxws/tracing/brave/BookStore.java | 12 ++ .../jaxws/tracing/brave/BraveTracingTest.java | 204 ++++++++++----------- .../jaxws/tracing/opentracing/BookStore.java | 14 +- .../opentracing/OpenTracingTracingTest.java | 20 ++ 12 files changed, 253 insertions(+), 186 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 d95db3e..d991f78 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 @@ -29,7 +29,6 @@ import brave.http.HttpServerAdapter; import brave.http.HttpServerHandler; import brave.http.HttpTracing; import org.apache.cxf.common.logging.LogUtils; -import org.apache.cxf.helpers.CastUtils; import org.apache.cxf.phase.PhaseInterceptorChain; import org.apache.cxf.tracing.AbstractTracingProvider; import org.apache.cxf.tracing.brave.internal.HttpAdapterFactory; @@ -79,14 +78,6 @@ public abstract class AbstractBraveProvider extends AbstractTracingProvider { final Map<String, List<Object>> responseHeaders, final int responseStatus, final TraceScopeHolder<TraceScope> holder) { - - // Transfer tracing headers into the response headers - brave - .tracing() - .propagation() - .keys() - .forEach(key -> transferRequestHeader(requestHeaders, responseHeaders, key)); - if (holder == null) { return; } @@ -125,11 +116,4 @@ public abstract class AbstractBraveProvider extends AbstractTracingProvider { private void propagateContinuationSpan(final Span continuationScope) { PhaseInterceptorChain.getCurrentMessage().put(Span.class, continuationScope); } - - private void transferRequestHeader(final Map<String, List<String>> requestHeaders, - final Map<String, List<Object>> responseHeaders, final String header) { - if (requestHeaders.containsKey(header)) { - responseHeaders.put(header, CastUtils.cast(requestHeaders.get(header))); - } - } } diff --git a/integration/tracing/tracing-brave/src/main/java/org/apache/cxf/tracing/brave/BraveStopInterceptor.java b/integration/tracing/tracing-brave/src/main/java/org/apache/cxf/tracing/brave/BraveStopInterceptor.java index 44d072a..7dc4c0a 100644 --- a/integration/tracing/tracing-brave/src/main/java/org/apache/cxf/tracing/brave/BraveStopInterceptor.java +++ b/integration/tracing/tracing-brave/src/main/java/org/apache/cxf/tracing/brave/BraveStopInterceptor.java @@ -33,7 +33,7 @@ import org.apache.cxf.phase.Phase; @NoJSR250Annotations public class BraveStopInterceptor extends AbstractBraveInterceptor { public BraveStopInterceptor(final HttpTracing brave) { - super(Phase.PRE_MARSHAL, brave); + super(Phase.POST_MARSHAL, brave); } @Override diff --git a/integration/tracing/tracing-opentracing/src/main/java/org/apache/cxf/tracing/opentracing/OpenTracingStopInterceptor.java b/integration/tracing/tracing-opentracing/src/main/java/org/apache/cxf/tracing/opentracing/OpenTracingStopInterceptor.java index 9ecc0c4..5aa61ae 100644 --- a/integration/tracing/tracing-opentracing/src/main/java/org/apache/cxf/tracing/opentracing/OpenTracingStopInterceptor.java +++ b/integration/tracing/tracing-opentracing/src/main/java/org/apache/cxf/tracing/opentracing/OpenTracingStopInterceptor.java @@ -32,7 +32,7 @@ import io.opentracing.Tracer; public class OpenTracingStopInterceptor extends AbstractOpenTracingInterceptor { public OpenTracingStopInterceptor(final Tracer tracer) { - super(Phase.PRE_MARSHAL, tracer); + super(Phase.POST_MARSHAL, tracer); } @Override 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 1f6d752..f02142f 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 @@ -28,6 +28,7 @@ import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; import javax.ws.rs.GET; +import javax.ws.rs.InternalServerErrorException; import javax.ws.rs.PUT; import javax.ws.rs.Path; import javax.ws.rs.PathParam; @@ -131,7 +132,28 @@ public class BookStore<T extends Closeable> { Thread.sleep(500); return books(); } - + + @GET + @Path("/books/exception") + @Produces(MediaType.APPLICATION_JSON) + public Collection<Book> getBooksException() { + throw new InternalServerErrorException("Simulated failure"); + } + + @GET + @Path("/books/error") + @Produces(MediaType.APPLICATION_JSON) + public Response getBooksError() { + return Response.status(503).build(); + } + + @GET + @Path("/books/mapper") + @Produces(MediaType.APPLICATION_JSON) + public Response getNpe() { + throw new NullPointerException("Simulated failure"); + } + 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/jaxws/tracing/BookStoreService.java b/systests/tracing/src/test/java/org/apache/cxf/systest/jaxrs/tracing/NullPointerExceptionMapper.java similarity index 65% copy from systests/tracing/src/test/java/org/apache/cxf/systest/jaxws/tracing/BookStoreService.java copy to systests/tracing/src/test/java/org/apache/cxf/systest/jaxrs/tracing/NullPointerExceptionMapper.java index e2c5ab8..ef81184 100644 --- a/systests/tracing/src/test/java/org/apache/cxf/systest/jaxws/tracing/BookStoreService.java +++ b/systests/tracing/src/test/java/org/apache/cxf/systest/jaxrs/tracing/NullPointerExceptionMapper.java @@ -16,16 +16,17 @@ * specific language governing permissions and limitations * under the License. */ -package org.apache.cxf.systest.jaxws.tracing; -import java.util.Collection; +package org.apache.cxf.systest.jaxrs.tracing; -import javax.jws.WebService; +import javax.ws.rs.core.Response; +import javax.ws.rs.core.Response.Status; +import javax.ws.rs.ext.ExceptionMapper; +import javax.ws.rs.ext.Provider; -import org.apache.cxf.systest.Book; - -@WebService -public interface BookStoreService { - Collection< Book > getBooks(); - int removeBooks(); +@Provider +public class NullPointerExceptionMapper implements ExceptionMapper<NullPointerException> { + public Response toResponse(NullPointerException exception) { + return Response.status(Status.NOT_FOUND).build(); + } } 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 97c3012..6e49dcf 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 @@ -49,6 +49,7 @@ import org.apache.cxf.jaxrs.model.AbstractResourceInfo; import org.apache.cxf.systest.brave.BraveTestSupport.SpanId; import org.apache.cxf.systest.brave.TestSpanReporter; import org.apache.cxf.systest.jaxrs.tracing.BookStore; +import org.apache.cxf.systest.jaxrs.tracing.NullPointerExceptionMapper; import org.apache.cxf.testutil.common.AbstractBusClientServerTestBase; import org.apache.cxf.testutil.common.AbstractBusTestServerBase; import org.apache.cxf.tracing.brave.BraveClientFeature; @@ -106,6 +107,7 @@ public class BraveTracingTest extends AbstractBusClientServerTestBase { sf.setAddress("http://localhost:"; + PORT); sf.setProvider(new JacksonJsonProvider()); sf.setProvider(new BraveFeature(brave)); + sf.setProvider(new NullPointerExceptionMapper()); sf.create(); } } @@ -157,8 +159,6 @@ public class BraveTracingTest extends AbstractBusClientServerTestBase { assertThat(TestSpanReporter.getAllSpans().size(), equalTo(2)); assertThat(TestSpanReporter.getAllSpans().get(0).name(), equalTo("get books")); assertThat(TestSpanReporter.getAllSpans().get(1).name(), equalTo("get /bookstore/books")); - - assertThatTraceIsPresent(r, spanId); } @Test @@ -171,8 +171,6 @@ public class BraveTracingTest extends AbstractBusClientServerTestBase { assertThat(TestSpanReporter.getAllSpans().size(), equalTo(1)); assertThat(TestSpanReporter.getAllSpans().get(0).name(), equalTo("get /bookstore/book/1")); assertThat(TestSpanReporter.getAllSpans().get(0).tags(), hasEntry("book-id", "1")); - - assertThatTraceIsPresent(r, spanId); } @Test @@ -185,8 +183,6 @@ public class BraveTracingTest extends AbstractBusClientServerTestBase { assertThat(TestSpanReporter.getAllSpans().size(), equalTo(2)); assertThat(TestSpanReporter.getAllSpans(), hasSpan("processing books", hasItem("Processing started"))); assertThat(TestSpanReporter.getAllSpans(), hasSpan("put /bookstore/process")); - - assertThatTraceIsPresent(r, spanId); } @Test @@ -197,8 +193,6 @@ public class BraveTracingTest extends AbstractBusClientServerTestBase { assertThat(TestSpanReporter.getAllSpans().size(), equalTo(3)); assertThat(TestSpanReporter.getAllSpans().get(0).name(), equalTo("get books")); assertThat(TestSpanReporter.getAllSpans().get(0).parentId(), not(nullValue())); - - assertThatTraceHeadersArePresent(r, false); } @Test @@ -214,8 +208,6 @@ public class BraveTracingTest extends AbstractBusClientServerTestBase { assertThat(TestSpanReporter.getAllSpans().get(0).parentId(), not(nullValue())); assertThat(TestSpanReporter.getAllSpans().get(0).parentId(), equalTo(TestSpanReporter.getAllSpans().get(1).id())); - - assertThatTraceIsPresent(r, spanId); } @Test @@ -227,8 +219,6 @@ public class BraveTracingTest extends AbstractBusClientServerTestBase { assertThat(TestSpanReporter.getAllSpans().size(), equalTo(1)); assertThat(TestSpanReporter.getAllSpans().get(0).name(), equalTo("get /bookstore/books/async/notrace")); - - assertThatTraceIsPresent(r, spanId); } @Test @@ -253,8 +243,6 @@ public class BraveTracingTest extends AbstractBusClientServerTestBase { assertThat(TestSpanReporter.getAllSpans().get(0).name(), equalTo("get books")); assertThat(TestSpanReporter.getAllSpans().get(1).name(), equalTo("get /bookstore/books")); assertThat(TestSpanReporter.getAllSpans().get(2).name(), equalTo("get " + client.getCurrentURI())); - - assertThatTraceHeadersArePresent(r, false); } @Test @@ -271,7 +259,6 @@ public class BraveTracingTest extends AbstractBusClientServerTestBase { for (final Response r: responses) { assertEquals(Status.OK.getStatusCode(), r.getStatus()); - assertThatTraceHeadersArePresent(r, false); } assertThat(TestSpanReporter.getAllSpans().size(), equalTo(12)); @@ -302,7 +289,6 @@ public class BraveTracingTest extends AbstractBusClientServerTestBase { for (final Response r: responses) { assertEquals(Status.OK.getStatusCode(), r.getStatus()); - assertThatTraceHeadersArePresent(r, false); } assertThat(TestSpanReporter.getAllSpans().size(), equalTo(12)); @@ -335,8 +321,6 @@ public class BraveTracingTest extends AbstractBusClientServerTestBase { assertThat(TestSpanReporter.getAllSpans().get(0).parentId(), not(nullValue())); assertThat(TestSpanReporter.getAllSpans().get(1).name(), equalTo("get /bookstore/books")); assertThat(TestSpanReporter.getAllSpans().get(2).name(), equalTo("get " + client.getCurrentURI())); - - assertThatTraceHeadersArePresent(r, true); } } finally { span.finish(); @@ -366,8 +350,6 @@ public class BraveTracingTest extends AbstractBusClientServerTestBase { assertThat(TestSpanReporter.getAllSpans().get(0).name(), equalTo("get books")); assertThat(TestSpanReporter.getAllSpans().get(1).name(), equalTo("get /bookstore/books")); assertThat(TestSpanReporter.getAllSpans().get(2).name(), equalTo("get " + client.getCurrentURI())); - - assertThatTraceHeadersArePresent(r, true); } } finally { span.finish(); @@ -390,8 +372,6 @@ public class BraveTracingTest extends AbstractBusClientServerTestBase { assertThat(TestSpanReporter.getAllSpans().size(), equalTo(2)); assertThat(TestSpanReporter.getAllSpans().get(1).name(), equalTo("get /bookstore/books/pseudo-async")); assertThat(TestSpanReporter.getAllSpans().get(0).name(), equalTo("processing books")); - - assertThatTraceIsPresent(r, spanId); } @Test @@ -406,7 +386,6 @@ public class BraveTracingTest extends AbstractBusClientServerTestBase { assertEquals(Status.OK.getStatusCode(), r.getStatus()); assertThat(TestSpanReporter.getAllSpans().size(), equalTo(0)); - assertThatTraceHeadersArePresent(r, false); } @Test @@ -433,6 +412,51 @@ public class BraveTracingTest extends AbstractBusClientServerTestBase { } } + @Test + public void testThatErrorSpanIsCreatedOnExceptionWhenNotProvided() { + final Response r = createWebClient("/bookstore/books/exception").get(); + assertEquals(Status.INTERNAL_SERVER_ERROR.getStatusCode(), r.getStatus()); + + assertThat(TestSpanReporter.getAllSpans().size(), equalTo(1)); + assertThat(TestSpanReporter.getAllSpans().get(0).name(), equalTo("get /bookstore/books/exception")); + assertThat(TestSpanReporter.getAllSpans().get(0).tags(), hasEntry("http.status_code", "500")); + + assertFalse(r.getHeaders().containsKey(SPAN_ID_NAME)); + assertFalse(r.getHeaders().containsKey(TRACE_ID_NAME)); + assertFalse(r.getHeaders().containsKey(SAMPLED_NAME)); + assertFalse(r.getHeaders().containsKey(PARENT_SPAN_ID_NAME)); + } + + @Test + public void testThatErrorSpanIsCreatedOnErrorWhenNotProvided() { + final Response r = createWebClient("/bookstore/books/error").get(); + assertEquals(Status.SERVICE_UNAVAILABLE.getStatusCode(), r.getStatus()); + + assertThat(TestSpanReporter.getAllSpans().size(), equalTo(1)); + assertThat(TestSpanReporter.getAllSpans().get(0).name(), equalTo("get /bookstore/books/error")); + assertThat(TestSpanReporter.getAllSpans().get(0).tags(), hasEntry("http.status_code", "503")); + + assertFalse(r.getHeaders().containsKey(SPAN_ID_NAME)); + assertFalse(r.getHeaders().containsKey(TRACE_ID_NAME)); + assertFalse(r.getHeaders().containsKey(SAMPLED_NAME)); + assertFalse(r.getHeaders().containsKey(PARENT_SPAN_ID_NAME)); + } + + @Test + public void testThatErrorSpanIsCreatedOnMappedExceptionWhenNotProvided() { + final Response r = createWebClient("/bookstore/books/mapper").get(); + assertEquals(Status.NOT_FOUND.getStatusCode(), r.getStatus()); + + assertThat(TestSpanReporter.getAllSpans().size(), equalTo(1)); + assertThat(TestSpanReporter.getAllSpans().get(0).name(), equalTo("get /bookstore/books/mapper")); + assertThat(TestSpanReporter.getAllSpans().get(0).tags(), hasEntry("http.status_code", "404")); + + assertFalse(r.getHeaders().containsKey(SPAN_ID_NAME)); + assertFalse(r.getHeaders().containsKey(TRACE_ID_NAME)); + assertFalse(r.getHeaders().containsKey(SAMPLED_NAME)); + assertFalse(r.getHeaders().containsKey(PARENT_SPAN_ID_NAME)); + } + protected WebClient createWebClient(final String url, final Object ... providers) { return WebClient .create("http://localhost:"; + PORT + url, Arrays.asList(providers)) @@ -447,29 +471,6 @@ public class BraveTracingTest extends AbstractBusClientServerTestBase { .header(PARENT_SPAN_ID_NAME, spanId.parentId()); } - private void assertThatTraceIsPresent(final Response r, final SpanId spanId) { - assertThat((String)r.getHeaders().getFirst(SPAN_ID_NAME), - equalTo(Long.toString(spanId.spanId()))); - assertThat((String)r.getHeaders().getFirst(TRACE_ID_NAME), - equalTo(Long.toString(spanId.traceId()))); - assertThat((String)r.getHeaders().getFirst(SAMPLED_NAME), - equalTo(Boolean.toString(spanId.sampled()))); - assertThat((String)r.getHeaders().getFirst(PARENT_SPAN_ID_NAME), - equalTo(Long.toString(spanId.parentId()))); - } - - private void assertThatTraceHeadersArePresent(final Response r, final boolean expectParent) { - assertTrue(r.getHeaders().containsKey(SPAN_ID_NAME)); - assertTrue(r.getHeaders().containsKey(TRACE_ID_NAME)); - assertTrue(r.getHeaders().containsKey(SAMPLED_NAME)); - - if (expectParent) { - assertTrue(r.getHeaders().containsKey(PARENT_SPAN_ID_NAME)); - } else { - assertFalse(r.getHeaders().containsKey(PARENT_SPAN_ID_NAME)); - } - } - private<T> T get(final Future<T> future) { try { return future.get(1, TimeUnit.HOURS); 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 c39dad9..b8ccbd9 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 @@ -47,6 +47,7 @@ import org.apache.cxf.jaxrs.lifecycle.SingletonResourceProvider; import org.apache.cxf.jaxrs.model.AbstractResourceInfo; import org.apache.cxf.systest.jaeger.TestSender; import org.apache.cxf.systest.jaxrs.tracing.BookStore; +import org.apache.cxf.systest.jaxrs.tracing.NullPointerExceptionMapper; import org.apache.cxf.testutil.common.AbstractBusClientServerTestBase; import org.apache.cxf.testutil.common.AbstractBusTestServerBase; import org.apache.cxf.tracing.opentracing.OpenTracingClientFeature; @@ -118,6 +119,7 @@ public class OpenTracingTracingTest extends AbstractBusClientServerTestBase { sf.setAddress("http://localhost:"; + PORT); sf.setProvider(new JacksonJsonProvider()); sf.setProvider(new OpenTracingFeature(tracer)); + sf.setProvider(new NullPointerExceptionMapper()); sf.create(); } } @@ -431,7 +433,37 @@ public class OpenTracingTracingTest extends AbstractBusClientServerTestBase { assertThat(TestSender.getAllSpans().get(1).getOperationName(), equalTo("GET /bookstore/books/long")); } } + + @Test + public void testThatErrorSpanIsCreatedOnExceptionWhenNotProvided() { + final Response r = createWebClient("/bookstore/books/exception").get(); + assertEquals(Status.INTERNAL_SERVER_ERROR.getStatusCode(), r.getStatus()); + + assertThat(TestSender.getAllSpans().toString(), TestSender.getAllSpans().size(), equalTo(1)); + assertThat(TestSender.getAllSpans().get(0).getOperationName(), equalTo("GET /bookstore/books/exception")); + assertThat(TestSender.getAllSpans().get(0).getTags(), hasItem(Tags.HTTP_STATUS.getKey(), 500)); + } + @Test + public void testThatErrorSpanIsCreatedOnErrorWhenNotProvided() { + final Response r = createWebClient("/bookstore/books/error").get(); + assertEquals(Status.SERVICE_UNAVAILABLE.getStatusCode(), r.getStatus()); + + assertThat(TestSender.getAllSpans().toString(), TestSender.getAllSpans().size(), equalTo(1)); + assertThat(TestSender.getAllSpans().get(0).getOperationName(), equalTo("GET /bookstore/books/error")); + assertThat(TestSender.getAllSpans().get(0).getTags(), hasItem(Tags.HTTP_STATUS.getKey(), 503)); + } + + @Test + public void testThatErrorSpanIsCreatedOnMappedExceptionWhenNotProvided() { + final Response r = createWebClient("/bookstore/books/mapper").get(); + assertEquals(Status.NOT_FOUND.getStatusCode(), r.getStatus()); + + assertThat(TestSender.getAllSpans().toString(), TestSender.getAllSpans().size(), equalTo(1)); + assertThat(TestSender.getAllSpans().get(0).getOperationName(), equalTo("GET /bookstore/books/mapper")); + assertThat(TestSender.getAllSpans().get(0).getTags(), hasItem(Tags.HTTP_STATUS.getKey(), 404)); + } + 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/jaxws/tracing/BookStoreService.java b/systests/tracing/src/test/java/org/apache/cxf/systest/jaxws/tracing/BookStoreService.java index e2c5ab8..c6a58cf 100644 --- a/systests/tracing/src/test/java/org/apache/cxf/systest/jaxws/tracing/BookStoreService.java +++ b/systests/tracing/src/test/java/org/apache/cxf/systest/jaxws/tracing/BookStoreService.java @@ -28,4 +28,5 @@ import org.apache.cxf.systest.Book; public interface BookStoreService { Collection< Book > getBooks(); int removeBooks(); + void addBooks(); } diff --git a/systests/tracing/src/test/java/org/apache/cxf/systest/jaxws/tracing/brave/BookStore.java b/systests/tracing/src/test/java/org/apache/cxf/systest/jaxws/tracing/brave/BookStore.java index d239a6a..2a1d254 100644 --- a/systests/tracing/src/test/java/org/apache/cxf/systest/jaxws/tracing/brave/BookStore.java +++ b/systests/tracing/src/test/java/org/apache/cxf/systest/jaxws/tracing/brave/BookStore.java @@ -22,8 +22,11 @@ import java.util.Arrays; import java.util.Collection; import java.util.UUID; +import javax.annotation.Resource; import javax.jws.WebMethod; import javax.jws.WebService; +import javax.xml.ws.WebServiceContext; +import javax.xml.ws.handler.MessageContext; import brave.Span; import brave.Tracer.SpanInScope; @@ -36,6 +39,9 @@ import org.apache.cxf.systest.jaxws.tracing.BookStoreService; public class BookStore implements BookStoreService { private final Tracing brave; + @Resource + private WebServiceContext context; + public BookStore() { brave = Tracing.newBuilder() .localServiceName("book-store") @@ -60,4 +66,10 @@ public class BookStore implements BookStoreService { public int removeBooks() { throw new RuntimeException("Unable to remove books"); } + + @WebMethod + public void addBooks() { + final MessageContext ctx = context.getMessageContext(); + ctx.put(MessageContext.HTTP_RESPONSE_CODE, 305); + } } diff --git a/systests/tracing/src/test/java/org/apache/cxf/systest/jaxws/tracing/brave/BraveTracingTest.java b/systests/tracing/src/test/java/org/apache/cxf/systest/jaxws/tracing/brave/BraveTracingTest.java index 884bbad..aff45cf 100644 --- a/systests/tracing/src/test/java/org/apache/cxf/systest/jaxws/tracing/brave/BraveTracingTest.java +++ b/systests/tracing/src/test/java/org/apache/cxf/systest/jaxws/tracing/brave/BraveTracingTest.java @@ -56,10 +56,10 @@ import static org.apache.cxf.systest.brave.BraveTestSupport.SAMPLED_NAME; 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.hamcrest.CoreMatchers.equalTo; -import static org.hamcrest.CoreMatchers.hasItem; 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.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -138,75 +138,60 @@ public class BraveTracingTest extends AbstractBusClientServerTestBase { assertThat(TestSpanReporter.getAllSpans().size(), equalTo(2)); assertThat(TestSpanReporter.getAllSpans().get(0).name(), equalTo("get books")); assertThat(TestSpanReporter.getAllSpans().get(1).name(), equalTo("post /bookstore")); - - final Map<String, List<String>> response = getResponseHeaders(service); - assertThatTraceIsPresent(response, spanId); } @Test - public void testThatNewChildSpanIsCreatedWhenParentIsProvided() throws MalformedURLException { - final Tracing brave = Tracing.newBuilder() - .localServiceName("book-store") - .spanReporter(new TestSpanReporter()) - .build(); - - final BookStoreService service = createJaxWsService(new Configurator() { - @Override - public void configure(final JaxWsProxyFactoryBean factory) { - factory.getFeatures().add(new BraveClientFeature(brave)); - } - }); - assertThat(service.getBooks().size(), equalTo(2)); - - assertThat(TestSpanReporter.getAllSpans().size(), equalTo(3)); - assertThat(TestSpanReporter.getAllSpans().get(0).name(), equalTo("get books")); - assertThat(TestSpanReporter.getAllSpans().get(0).parentId(), not(nullValue())); - assertThat(TestSpanReporter.getAllSpans().get(1).name(), equalTo("post /bookstore")); - assertThat(TestSpanReporter.getAllSpans().get(2).name(), - equalTo("post http://localhost:"; + PORT + "/bookstore")); - - final Map<String, List<String>> response = getResponseHeaders(service); - assertThatTraceHeadersArePresent(response, false); + public void testThatNewChildSpanIsCreatedWhenParentIsProvided() throws Exception { + try (Tracing brave = createTracer()) { + final BookStoreService service = createJaxWsService(new Configurator() { + @Override + public void configure(final JaxWsProxyFactoryBean factory) { + factory.getFeatures().add(new BraveClientFeature(brave)); + } + }); + assertThat(service.getBooks().size(), equalTo(2)); + + assertThat(TestSpanReporter.getAllSpans().size(), equalTo(3)); + assertThat(TestSpanReporter.getAllSpans().get(0).name(), equalTo("get books")); + assertThat(TestSpanReporter.getAllSpans().get(0).parentId(), not(nullValue())); + assertThat(TestSpanReporter.getAllSpans().get(1).name(), equalTo("post /bookstore")); + assertThat(TestSpanReporter.getAllSpans().get(2).name(), + equalTo("post http://localhost:"; + PORT + "/bookstore")); + } } @Test - public void testThatProvidedSpanIsNotClosedWhenActive() throws MalformedURLException { - final Tracing brave = Tracing.newBuilder() - .localServiceName("book-store") - .spanReporter(new TestSpanReporter()) - .build(); - - final BookStoreService service = createJaxWsService(new Configurator() { - @Override - public void configure(final JaxWsProxyFactoryBean factory) { - factory.getFeatures().add(new BraveClientFeature(brave)); - } - }); - - final Span span = brave.tracer().nextSpan().name("test span").start(); - try { - try (SpanInScope scope = brave.tracer().withSpanInScope(span)) { - assertThat(service.getBooks().size(), equalTo(2)); - assertThat(brave.tracer().currentSpan(), not(nullValue())); - - assertThat(TestSpanReporter.getAllSpans().size(), equalTo(3)); - assertThat(TestSpanReporter.getAllSpans().get(0).name(), equalTo("get books")); - assertThat(TestSpanReporter.getAllSpans().get(0).parentId(), not(nullValue())); - assertThat(TestSpanReporter.getAllSpans().get(1).name(), equalTo("post /bookstore")); - assertThat(TestSpanReporter.getAllSpans().get(2).name(), - equalTo("post http://localhost:"; + PORT + "/bookstore")); - - final Map<String, List<String>> response = getResponseHeaders(service); - assertThatTraceHeadersArePresent(response, true); - } - } finally { - if (span != null) { - span.finish(); + public void testThatProvidedSpanIsNotClosedWhenActive() throws Exception { + try (Tracing brave = createTracer()) { + final BookStoreService service = createJaxWsService(new Configurator() { + @Override + public void configure(final JaxWsProxyFactoryBean factory) { + factory.getFeatures().add(new BraveClientFeature(brave)); + } + }); + + final Span span = brave.tracer().nextSpan().name("test span").start(); + try { + try (SpanInScope scope = brave.tracer().withSpanInScope(span)) { + assertThat(service.getBooks().size(), equalTo(2)); + assertThat(brave.tracer().currentSpan(), not(nullValue())); + + assertThat(TestSpanReporter.getAllSpans().size(), equalTo(3)); + assertThat(TestSpanReporter.getAllSpans().get(0).name(), equalTo("get books")); + assertThat(TestSpanReporter.getAllSpans().get(0).parentId(), not(nullValue())); + assertThat(TestSpanReporter.getAllSpans().get(1).name(), equalTo("post /bookstore")); + assertThat(TestSpanReporter.getAllSpans().get(2).name(), + equalTo("post http://localhost:"; + PORT + "/bookstore")); + } + } finally { + if (span != null) { + span.finish(); + } } + + assertThat(TestSpanReporter.getAllSpans().size(), equalTo(4)); + assertThat(TestSpanReporter.getAllSpans().get(3).name(), equalTo("test span")); } - - assertThat(TestSpanReporter.getAllSpans().size(), equalTo(4)); - assertThat(TestSpanReporter.getAllSpans().get(3).name(), equalTo("test span")); } @Test @@ -231,35 +216,48 @@ public class BraveTracingTest extends AbstractBusClientServerTestBase { } @Test - public void testThatNewChildSpanIsCreatedWhenParentIsProvidedInCaseOfFault() throws MalformedURLException { - final Tracing brave = Tracing.newBuilder() - .localServiceName("book-store") - .spanReporter(new TestSpanReporter()) - .build(); - - final BookStoreService service = createJaxWsService(new Configurator() { - @Override - public void configure(final JaxWsProxyFactoryBean factory) { - factory.getFeatures().add(new BraveClientFeature(brave)); - factory.getOutInterceptors().add(new LoggingOutInterceptor()); - factory.getInInterceptors().add(new LoggingInInterceptor()); + public void testThatNewChildSpanIsCreatedWhenParentIsProvidedInCaseOfFault() throws Exception { + try (Tracing brave = createTracer()) { + final BookStoreService service = createJaxWsService(new Configurator() { + @Override + public void configure(final JaxWsProxyFactoryBean factory) { + factory.getFeatures().add(new BraveClientFeature(brave)); + } + }); + + try { + service.removeBooks(); + fail("Expected SOAPFaultException to be raised"); + } catch (final SOAPFaultException ex) { + /* expected exception */ } - }); - - try { - service.removeBooks(); - fail("Expected SOAPFaultException to be raised"); - } catch (final SOAPFaultException ex) { - /* expected exception */ + + assertThat(TestSpanReporter.getAllSpans().size(), equalTo(2)); + assertThat(TestSpanReporter.getAllSpans().get(0).name(), equalTo("post /bookstore")); + assertThat(TestSpanReporter.getAllSpans().get(1).name(), + equalTo("post http://localhost:"; + PORT + "/bookstore")); + } + } + + @Test + public void testThatNewChildSpanIsCreatedWhenParentIsProvidedAndCustomStatusCodeReturned() throws Exception { + try (Tracing brave = createTracer()) { + final BookStoreService service = createJaxWsService(new Configurator() { + @Override + public void configure(final JaxWsProxyFactoryBean factory) { + factory.getFeatures().add(new BraveClientFeature(brave)); + } + }); + + service.addBooks(); + + assertThat(TestSpanReporter.getAllSpans().size(), equalTo(2)); + assertThat(TestSpanReporter.getAllSpans().get(0).name(), equalTo("post /bookstore")); + assertThat(TestSpanReporter.getAllSpans().get(0).parentId(), nullValue()); + assertThat(TestSpanReporter.getAllSpans().get(0).tags(), hasEntry("http.status_code", "305")); + assertThat(TestSpanReporter.getAllSpans().get(1).name(), + equalTo("post http://localhost:"; + PORT + "/bookstore")); } - - assertThat(TestSpanReporter.getAllSpans().size(), equalTo(2)); - assertThat(TestSpanReporter.getAllSpans().get(0).name(), equalTo("post /bookstore")); - assertThat(TestSpanReporter.getAllSpans().get(1).name(), - equalTo("post http://localhost:"; + PORT + "/bookstore")); - - final Map<String, List<String>> response = getResponseHeaders(service); - assertThatTraceHeadersArePresent(response, false); } private BookStoreService createJaxWsService() throws MalformedURLException { @@ -298,27 +296,11 @@ public class BraveTracingTest extends AbstractBusClientServerTestBase { final Client proxy = ClientProxy.getClient(service); return CastUtils.cast((Map<?, ?>)proxy.getResponseContext().get(Message.PROTOCOL_HEADERS)); } - - private void assertThatTraceIsPresent(Map<String, List<String>> headers, SpanId spanId) { - assertThat(headers.get(SPAN_ID_NAME), - hasItem(Long.toString(spanId.spanId()))); - assertThat(headers.get(TRACE_ID_NAME), - hasItem(Long.toString(spanId.traceId()))); - assertThat(headers.get(SAMPLED_NAME), - hasItem(Boolean.toString(spanId.sampled()))); - assertThat(headers.get(PARENT_SPAN_ID_NAME), - hasItem(Long.toString(spanId.parentId()))); - } - - private void assertThatTraceHeadersArePresent(Map<String, List<String>> headers, boolean expectParent) { - assertTrue(headers.containsKey(SPAN_ID_NAME)); - assertTrue(headers.containsKey(TRACE_ID_NAME)); - assertTrue(headers.containsKey(SAMPLED_NAME)); - - if (expectParent) { - assertTrue(headers.containsKey(PARENT_SPAN_ID_NAME)); - } else { - assertFalse(headers.containsKey(PARENT_SPAN_ID_NAME)); - } + + private static Tracing createTracer() { + return Tracing.newBuilder() + .localServiceName("book-store") + .spanReporter(new TestSpanReporter()) + .build(); } } diff --git a/systests/tracing/src/test/java/org/apache/cxf/systest/jaxws/tracing/opentracing/BookStore.java b/systests/tracing/src/test/java/org/apache/cxf/systest/jaxws/tracing/opentracing/BookStore.java index af58418..5ffa70e 100644 --- a/systests/tracing/src/test/java/org/apache/cxf/systest/jaxws/tracing/opentracing/BookStore.java +++ b/systests/tracing/src/test/java/org/apache/cxf/systest/jaxws/tracing/opentracing/BookStore.java @@ -22,8 +22,11 @@ import java.util.Arrays; import java.util.Collection; import java.util.UUID; +import javax.annotation.Resource; import javax.jws.WebMethod; import javax.jws.WebService; +import javax.xml.ws.WebServiceContext; +import javax.xml.ws.handler.MessageContext; import org.apache.cxf.systest.Book; import org.apache.cxf.systest.jaxws.tracing.BookStoreService; @@ -37,12 +40,15 @@ import io.opentracing.util.GlobalTracer; public class BookStore implements BookStoreService { private final Tracer tracer; + @Resource + private WebServiceContext context; + public BookStore() { tracer = GlobalTracer.get(); } @WebMethod - public Collection< Book > getBooks() { + public Collection<Book> getBooks() { final Span span = tracer.buildSpan("Get Books").start(); try (Scope scope = tracer.activateSpan(span)) { return Arrays.asList( @@ -58,4 +64,10 @@ public class BookStore implements BookStoreService { public int removeBooks() { throw new RuntimeException("Unable to remove books"); } + + @WebMethod + public void addBooks() { + final MessageContext ctx = context.getMessageContext(); + ctx.put(MessageContext.HTTP_RESPONSE_CODE, 202); + } } 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 80ecded..724117d 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 @@ -53,6 +53,7 @@ import io.opentracing.Scope; import io.opentracing.Span; import io.opentracing.Tracer; import io.opentracing.propagation.Format.Builtin; +import io.opentracing.tag.Tags; import io.opentracing.util.GlobalTracer; import org.junit.Before; @@ -60,6 +61,7 @@ import org.junit.BeforeClass; import org.junit.Ignore; import org.junit.Test; +import static org.apache.cxf.systest.jaxrs.tracing.opentracing.IsTagContaining.hasItem; import static org.awaitility.Awaitility.await; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.not; @@ -241,9 +243,27 @@ public class OpenTracingTracingTest extends AbstractBusClientServerTestBase { assertThat(TestSender.getAllSpans().size(), equalTo(2)); assertThat(TestSender.getAllSpans().get(0).getOperationName(), equalTo("POST /BookStore")); + assertThat(TestSender.getAllSpans().get(0).getTags(), hasItem(Tags.HTTP_STATUS.getKey(), 500)); assertThat(TestSender.getAllSpans().get(1).getOperationName(), equalTo("POST http://localhost:"; + PORT + "/BookStore")); } + + @Test + public void testThatNewChildSpanIsCreatedWhenParentIsProvidedAndCustomStatusCodeReturned() throws Exception { + final BookStoreService service = createJaxWsService(new Configurator() { + @Override + public void configure(final JaxWsProxyFactoryBean factory) { + factory.getFeatures().add(new OpenTracingClientFeature(tracer)); + factory.getOutInterceptors().add(new LoggingOutInterceptor()); + factory.getInInterceptors().add(new LoggingInInterceptor()); + } + }); + service.addBooks(); + + assertThat(TestSender.getAllSpans().size(), equalTo(1)); + assertThat(TestSender.getAllSpans().get(0).getOperationName(), equalTo("POST /BookStore")); + assertThat(TestSender.getAllSpans().get(0).getTags(), hasItem(Tags.HTTP_STATUS.getKey(), 202)); + } private BookStoreService createJaxWsService() throws MalformedURLException { return createJaxWsService(new HashMap<String, List<String>>());
