Capture exception on finatra controller span (#4669)

* Capture exception on finatra controller span

* add return type check

* store VirtualField in a static field

instrumentation/finatra-2.9/javaagent/src/latestDepTest/groovy/FinatraServerLatestTest.groovy

@@ -3,12 +3,15 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
+
 import com.twitter.app.lifecycle.Event
 import com.twitter.app.lifecycle.Observer
 import com.twitter.finatra.http.HttpServer
 import com.twitter.util.Await
 import com.twitter.util.Duration
 import com.twitter.util.Promise
+import io.opentelemetry.api.trace.StatusCode
 import io.opentelemetry.instrumentation.test.AgentTestTrait
 import io.opentelemetry.instrumentation.test.asserts.TraceAssert
 import io.opentelemetry.instrumentation.test.base.HttpServerTest
@@ -81,8 +84,10 @@ class FinatraServerLatestTest extends HttpServerTest<HttpServer> implements Agen
       name "FinatraController"
       kind INTERNAL
       childOf(parent as SpanData)
-      // Finatra doesn't propagate the stack trace or exception to the instrumentation
-      // so the normal errorAttributes() method can't be used
+      if (endpoint == EXCEPTION) {
+        status StatusCode.ERROR
+        errorEvent(Exception, EXCEPTION.body)
+      }
       attributes {
       }
     }

instrumentation/finatra-2.9/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/finatra/FinatraExceptionManagerInstrumentation.java

@@ -0,0 +1,53 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.finatra;
+
+import static net.bytebuddy.matcher.ElementMatchers.isMethod;
+import static net.bytebuddy.matcher.ElementMatchers.named;
+import static net.bytebuddy.matcher.ElementMatchers.returns;
+import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
+
+import com.twitter.finagle.http.Response;
+import io.opentelemetry.instrumentation.api.field.VirtualField;
+import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
+import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
+import net.bytebuddy.asm.Advice;
+import net.bytebuddy.description.type.TypeDescription;
+import net.bytebuddy.matcher.ElementMatcher;
+
+public class FinatraExceptionManagerInstrumentation implements TypeInstrumentation {
+  @Override
+  public ElementMatcher<TypeDescription> typeMatcher() {
+    return named("com.twitter.finatra.http.exceptions.ExceptionManager");
+  }
+
+  @Override
+  public void transform(TypeTransformer transformer) {
+    transformer.applyAdviceToMethod(
+        isMethod()
+            .and(named("toResponse"))
+            .and(takesArgument(1, Throwable.class))
+            .and(returns(named("com.twitter.finagle.http.Response"))),
+        this.getClass().getName() + "$HandleExceptionAdvice");
+  }
+
+  @SuppressWarnings("unused")
+  public static class HandleExceptionAdvice {
+
+    @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
+    public static void handleException(
+        @Advice.Return Response response, @Advice.Argument(1) Throwable throwable) {
+
+      if (throwable == null) {
+        return;
+      }
+
+      VirtualField<Response, Throwable> virtualField =
+          VirtualField.find(Response.class, Throwable.class);
+      virtualField.set(response, throwable);
+    }
+  }
+}

instrumentation/finatra-2.9/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/finatra/FinatraInstrumentationModule.java

@@ -5,7 +5,7 @@
 
 package io.opentelemetry.javaagent.instrumentation.finatra;
 
-import static java.util.Collections.singletonList;
+import static java.util.Arrays.asList;
 
 import com.google.auto.service.AutoService;
 import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
@@ -20,6 +20,6 @@ public class FinatraInstrumentationModule extends InstrumentationModule {
 
   @Override
   public List<TypeInstrumentation> typeInstrumentations() {
-    return singletonList(new FinatraRouteInstrumentation());
+    return asList(new FinatraRouteInstrumentation(), new FinatraExceptionManagerInstrumentation());
   }
 }

instrumentation/finatra-2.9/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/finatra/FinatraResponseListener.java

@@ -10,9 +10,13 @@ import static io.opentelemetry.javaagent.instrumentation.finatra.FinatraSingleto
 import com.twitter.finagle.http.Response;
 import com.twitter.util.FutureEventListener;
 import io.opentelemetry.context.Context;
+import io.opentelemetry.instrumentation.api.field.VirtualField;
 
 public final class FinatraResponseListener implements FutureEventListener<Response> {
 
+  private static final VirtualField<Response, Throwable> responseThrowableField =
+      VirtualField.find(Response.class, Throwable.class);
+
   private final Context context;
   private final Class<?> request;
 
@@ -23,7 +27,8 @@ public final class FinatraResponseListener implements FutureEventListener<Respon
 
   @Override
   public void onSuccess(Response response) {
-    instrumenter().end(context, request, null, null);
+    Throwable throwable = responseThrowableField.get(response);
+    instrumenter().end(context, request, null, throwable);
   }
 
   @Override

instrumentation/finatra-2.9/javaagent/src/test/groovy/FinatraServerTest.groovy

@@ -3,9 +3,12 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import static io.opentelemetry.instrumentation.test.base.HttpServerTest.ServerEndpoint.EXCEPTION
+
 import com.twitter.finatra.http.HttpServer
 import com.twitter.util.Await
 import com.twitter.util.Duration
+import io.opentelemetry.api.trace.StatusCode
 import io.opentelemetry.instrumentation.test.AgentTestTrait
 import io.opentelemetry.instrumentation.test.asserts.TraceAssert
 import io.opentelemetry.instrumentation.test.base.HttpServerTest
@@ -65,8 +68,10 @@ class FinatraServerTest extends HttpServerTest<HttpServer> implements AgentTestT
       name "FinatraController"
       kind INTERNAL
       childOf(parent as SpanData)
-      // Finatra doesn't propagate the stack trace or exception to the instrumentation
-      // so the normal errorAttributes() method can't be used
+      if (endpoint == EXCEPTION) {
+        status StatusCode.ERROR
+        errorEvent(Exception, EXCEPTION.body)
+      }
       attributes {
       }
     }