Allow span decoration when exception thrown reading/writing

opentracing-contrib · Sep 15, 2021 · 9162858 · 9162858
1 parent 55fc4c9
commit 9162858
Show file tree

Hide file tree

Showing 4 changed files with 117 additions and 9 deletions.
diff --git a/...t/common/src/main/java/io/opentracing/contrib/jaxrs2/itest/common/AbstractServerTest.java b/...t/common/src/main/java/io/opentracing/contrib/jaxrs2/itest/common/AbstractServerTest.java
@@ -212,6 +212,43 @@ public void testExceptionInHandler() {
 //        Assert.assertEquals(500, mockSpan.tags().get(Tags.HTTP_STATUS.getKey()));
     }
 
+    @Test
+    public void testExceptionInWriteInterceptor() {
+        Client client = ClientBuilder.newClient();
+        Response response = client.target(url("/failToSerialize"))
+                .request()
+                .get();
+        response.close();
+        Assert.assertEquals(500, response.getStatus());
+        await().until(finishedSpansSizeEquals(2));
+
+        List<MockSpan> mockSpans = mockTracer.finishedSpans();
+        Assert.assertEquals(2, mockSpans.size());
+        MockSpan mockSpan = mockSpans.get(0);
+        Assert.assertEquals(3, mockSpan.tags().size());
+        Assert.assertEquals("serialize", mockSpan.operationName());
+        Assert.assertEquals(true, mockSpan.tags().get(Tags.ERROR.getKey()));
+    }
+
+    @Test
+    public void testExceptionInReadInterceptor() {
+        Client client = ClientBuilder.newClient();
+        Response response = client.target(url("/failToDeserialize"))
+                .request()
+                .post(Entity.text("hello!"));
+        response.close();
+        // cxf returns 500 while jersey and resteasy returns 415
+        Assert.assertTrue(response.getStatus() > 400);
+        await().until(finishedSpansSizeEquals(2));
+
+        List<MockSpan> mockSpans = mockTracer.finishedSpans();
+        Assert.assertEquals(2, mockSpans.size());
+        MockSpan mockSpan = mockSpans.get(0);
+        Assert.assertEquals(3, mockSpan.tags().size());
+        Assert.assertEquals("deserialize", mockSpan.operationName());
+        Assert.assertEquals(true, mockSpan.tags().get(Tags.ERROR.getKey()));
+    }
+
     @Test
     public void testMappedExceptionInHandler() {
         Client client = ClientBuilder.newClient();

diff --git a/...est/common/src/main/java/io/opentracing/contrib/jaxrs2/itest/common/rest/TestHandler.java b/...est/common/src/main/java/io/opentracing/contrib/jaxrs2/itest/common/rest/TestHandler.java
@@ -1,6 +1,5 @@
 package io.opentracing.contrib.jaxrs2.itest.common.rest;
 
-import io.opentracing.Scope;
 import io.opentracing.Span;
 import io.opentracing.SpanContext;
 import io.opentracing.Tracer;
@@ -24,6 +23,10 @@
 import javax.ws.rs.core.Response;
 import org.eclipse.microprofile.opentracing.Traced;
 import org.junit.Assert;
+import javax.ws.rs.core.StreamingOutput;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.util.Map;
 
 /**
  * @author Pavol Loffay
@@ -94,6 +97,30 @@ public Response postWithBody(@Context HttpServletRequest request, String body) {
         return Response.ok().entity(body).build();
     }
 
+    @GET
+    @Path("/failToSerialize")
+    @Produces(MediaType.TEXT_PLAIN)
+    @Consumes(MediaType.TEXT_PLAIN)
+    public Response failToSerialize(@Context HttpServletRequest request) {
+        assertActiveSpan();
+        StreamingOutput output = new StreamingOutput() {
+            @Override
+            public void write(OutputStream output) throws IOException {
+                throw new IOException("Fail to serialize");
+            }
+        };
+        return Response.ok().entity(output).build();
+    }
+
+    @POST
+    @Path("/failToDeserialize")
+    @Produces(MediaType.TEXT_PLAIN)
+    @Consumes(MediaType.TEXT_PLAIN)
+    public Response failToDeserialize(@Context HttpServletRequest request, Map<String, String> body) {
+        assertActiveSpan();
+        return Response.ok().build();
+    }
+
     @GET
     @Path("/path/{pathParam}")
     public Response pathParam(@PathParam("pathParam") String pathParam1) {

diff --git a/...2/src/main/java/io/opentracing/contrib/jaxrs2/serialization/InterceptorSpanDecorator.java b/...2/src/main/java/io/opentracing/contrib/jaxrs2/serialization/InterceptorSpanDecorator.java
@@ -1,12 +1,16 @@
 package io.opentracing.contrib.jaxrs2.serialization;
 
 import io.opentracing.Span;
+import io.opentracing.tag.Tags;
+
 import javax.ws.rs.ext.InterceptorContext;
+import javax.ws.rs.ext.ReaderInterceptorContext;
+import javax.ws.rs.ext.WriterInterceptorContext;
 
 public interface InterceptorSpanDecorator {
 
     /**
-     * Decorate spans by outgoing object.
+     * Decorate spans by incoming object.
      *
      * @param context
      * @param span
@@ -21,6 +25,24 @@ public interface InterceptorSpanDecorator {
      */
     void decorateWrite(InterceptorContext context, Span span);
 
+    /**
+     * Decorate spans by error throw when processing incoming object.
+     *
+     * @param e
+     * @param context
+     * @param span
+     */
+    void decorateReadException(Exception e, ReaderInterceptorContext context, Span span);
+
+    /**
+     * Decorate spans by error thrown when processing outgoing object.
+     *
+     * @param e
+     * @param context
+     * @param span
+     */
+    void decorateWriteException(Exception e, WriterInterceptorContext context, Span span);
+
     /**
      * Adds tags: \"media.type\", \"entity.type\"
      */
@@ -35,5 +57,15 @@ public void decorateRead(InterceptorContext context, Span span) {
         public void decorateWrite(InterceptorContext context, Span span) {
             decorateRead(context, span);
         }
+
+        @Override
+        public void decorateReadException(Exception e, ReaderInterceptorContext context, Span span) {
+            Tags.ERROR.set(span, true);
+        }
+
+        @Override
+        public void decorateWriteException(Exception e, WriterInterceptorContext context, Span span) {
+            Tags.ERROR.set(span, true);
+        }
     };
 }
diff --git a/...-jaxrs2/src/main/java/io/opentracing/contrib/jaxrs2/serialization/TracingInterceptor.java b/...-jaxrs2/src/main/java/io/opentracing/contrib/jaxrs2/serialization/TracingInterceptor.java
@@ -6,7 +6,6 @@
 import io.opentracing.Tracer;
 import io.opentracing.contrib.jaxrs2.internal.SpanWrapper;
 import io.opentracing.noop.NoopSpan;
-import io.opentracing.tag.Tags;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -40,8 +39,7 @@ public Object aroundReadFrom(ReaderInterceptorContext context)
             try {
                 return context.proceed();
             } catch (Exception e) {
-                //TODO add exception logs in case they are not added by the filter.
-                Tags.ERROR.set(span, true);
+                decorateReadException(e, context, span);
                 throw e;
             }
         } finally {
@@ -55,10 +53,12 @@ public void aroundWriteTo(WriterInterceptorContext context)
         Span span = buildSpan(context, "serialize");
         try (Scope scope = tracer.activateSpan(span)) {
             decorateWrite(context, span);
-            context.proceed();
-        } catch (Exception e) {
-            Tags.ERROR.set(span, true);
-            throw e;
+            try {
+                context.proceed();
+            } catch (Exception e) {
+                decorateWriteException(e, context, span);
+                throw e;
+            }
         } finally {
             span.finish();
         }
@@ -106,4 +106,16 @@ private void decorateWrite(InterceptorContext context, Span span) {
             decorator.decorateWrite(context, span);
         }
     }
+
+    private void decorateReadException(Exception e, ReaderInterceptorContext context, Span span) {
+        for (InterceptorSpanDecorator decorator : spanDecorators) {
+            decorator.decorateReadException(e, context, span);
+        }
+    }
+
+    private void decorateWriteException(Exception e, WriterInterceptorContext context, Span span) {
+        for (InterceptorSpanDecorator decorator : spanDecorators) {
+            decorator.decorateWriteException(e, context, span);
+        }
+    }
 }