Enable exception tests on vert.x (#2914)

open-telemetry · May 6, 2021 · 5670024 · 5670024
1 parent 9e9fe11
commit 5670024
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 26 deletions.
diff --git a/...-reactive-3.5/javaagent/src/test/groovy/server/VertxRxCircuitBreakerHttpServerTest.groovy b/...-reactive-3.5/javaagent/src/test/groovy/server/VertxRxCircuitBreakerHttpServerTest.groovy
@@ -126,4 +126,9 @@ class VertxRxCircuitBreakerHttpServerTest extends VertxRxHttpServerTest {
         .listen(port) { startFuture.complete() }
     }
   }
+
+  @Override
+  boolean hasExceptionOnServerSpan(HttpServerTest.ServerEndpoint endpoint) {
+    return endpoint != EXCEPTION && super.hasExceptionOnServerSpan(endpoint)
+  }
 }
diff --git a/...entation/vertx-reactive-3.5/javaagent/src/test/groovy/server/VertxRxHttpServerTest.groovy b/...entation/vertx-reactive-3.5/javaagent/src/test/groovy/server/VertxRxHttpServerTest.groovy
@@ -7,6 +7,7 @@ package server
 
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.ERROR
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
+import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.NOT_FOUND
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.PATH_PARAM
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.QUERY_PARAM
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.REDIRECT
@@ -53,25 +54,21 @@ class VertxRxHttpServerTest extends HttpServerTest<Vertx> implements AgentTestTr
     server.close()
   }
 
-  @Override
-  boolean testException() {
-    // https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/807
-    return false
-  }
-
   @Override
   boolean testPathParam() {
     return true
   }
 
-  @Override
-  boolean testNotFound() {
-    return false
-  }
-
   @Override
   String expectedServerSpanName(ServerEndpoint endpoint) {
-    return endpoint == PATH_PARAM ? "/path/:id/param" : endpoint.getPath()
+    switch (endpoint) {
+      case PATH_PARAM:
+        return "/path/:id/param"
+      case NOT_FOUND:
+        return "HTTP GET"
+      default:
+        return endpoint.getPath()
+    }
   }
 
   protected Class<AbstractVerticle> verticle() {

diff --git a/...n/java/io/opentelemetry/javaagent/instrumentation/vertx/RoutingContextHandlerWrapper.java b/...n/java/io/opentelemetry/javaagent/instrumentation/vertx/RoutingContextHandlerWrapper.java
@@ -10,6 +10,10 @@
 import io.opentelemetry.instrumentation.api.tracer.ServerSpan;
 import io.vertx.core.Handler;
 import io.vertx.ext.web.RoutingContext;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.UndeclaredThrowableException;
+import java.util.concurrent.CompletionException;
+import java.util.concurrent.ExecutionException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -26,8 +30,8 @@ public RoutingContextHandlerWrapper(Handler<RoutingContext> handler) {
 
   @Override
   public void handle(RoutingContext context) {
+    Span serverSpan = ServerSpan.fromContextOrNull(Context.current());
     try {
-      Span serverSpan = ServerSpan.fromContextOrNull(Context.current());
       if (serverSpan != null) {
         // TODO should update only SERVER span using
         // https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/465
@@ -36,6 +40,24 @@ public void handle(RoutingContext context) {
     } catch (Exception ex) {
       log.error("Failed to update server span name with vert.x route", ex);
     }
-    handler.handle(context);
+    try {
+      handler.handle(context);
+    } catch (Throwable throwable) {
+      if (serverSpan != null) {
+        serverSpan.recordException(unwrapThrowable(throwable));
+      }
+      throw throwable;
+    }
+  }
+
+  private Throwable unwrapThrowable(Throwable throwable) {
+    if (throwable.getCause() != null
+        && (throwable instanceof ExecutionException
+            || throwable instanceof CompletionException
+            || throwable instanceof InvocationTargetException
+            || throwable instanceof UndeclaredThrowableException)) {
+      return unwrapThrowable(throwable.getCause());
+    }
+    return throwable;
   }
 }
diff --git a/instrumentation/vertx-web-3.0/javaagent/src/test/groovy/server/VertxHttpServerTest.groovy b/instrumentation/vertx-web-3.0/javaagent/src/test/groovy/server/VertxHttpServerTest.groovy
@@ -5,6 +5,7 @@
 
 package server
 
+import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.NOT_FOUND
 import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.PATH_PARAM
 
 import io.opentelemetry.instrumentation.test.AgentTestTrait
@@ -48,30 +49,26 @@ class VertxHttpServerTest extends HttpServerTest<Vertx> implements AgentTestTrai
     server.close()
   }
 
-  @Override
-  boolean testException() {
-    // TODO(anuraaga): https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/807
-    return false
-  }
-
   @Override
   boolean testPathParam() {
     return true
   }
 
-  @Override
-  boolean testNotFound() {
-    return false
-  }
-
   @Override
   boolean testConcurrency() {
     return true
   }
 
   @Override
   String expectedServerSpanName(ServerEndpoint endpoint) {
-    return endpoint == PATH_PARAM ? "/path/:id/param" : endpoint.getPath()
+    switch (endpoint) {
+      case PATH_PARAM:
+        return "/path/:id/param"
+      case NOT_FOUND:
+        return "HTTP GET"
+      default:
+        return endpoint.getPath()
+    }
   }
 
 }