code review notes

Author: jaydeluca

instrumentation/play/play-mvc/play-mvc-2.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/play/v2_4/ActionInstrumentation.java

@@ -17,6 +17,7 @@
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
 import java.lang.reflect.Method;
+import javax.annotation.Nullable;
 import net.bytebuddy.asm.Advice;
 import net.bytebuddy.description.type.TypeDescription;
 import net.bytebuddy.matcher.ElementMatcher;
@@ -65,22 +66,16 @@ public static void stopTraceOnResponse(
         @Advice.Thrown Throwable throwable,
         @Advice.Argument(0) Request<?> req,
         @Advice.Return(readOnly = false) Future<Result> responseFuture,
-        @Advice.Enter AdviceScope actionScope) {
-      if (actionScope == null || actionScope.getScope() == null) {
+        @Advice.Enter @Nullable AdviceScope actionScope) {
+      if (actionScope == null) {
         return;
       }
       actionScope.closeScope();
 
       updateSpan(actionScope.getContext(), req);
 
       // span finished in RequestCompleteCallback
-      if (throwable == null) {
-        responseFuture.onComplete(
-            new RequestCompleteCallback(actionScope.getContext()),
-            ((Action<?>) thisAction).executionContext());
-      } else {
-        actionScope.end(throwable);
-      }
+      actionScope.end(throwable, responseFuture, (Action<?>) thisAction);
     }
   }
 }

instrumentation/play/play-mvc/play-mvc-2.4/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/play/v2_4/AdviceScope.java

@@ -9,6 +9,10 @@
 
 import io.opentelemetry.context.Context;
 import io.opentelemetry.context.Scope;
+import javax.annotation.Nullable;
+import play.api.mvc.Action;
+import play.api.mvc.Result;
+import scala.concurrent.Future;
 
 /** Container used to carry state between enter and exit advices */
 public final class AdviceScope {
@@ -27,10 +31,7 @@ public Context getContext() {
     return context;
   }
 
-  public Scope getScope() {
-    return scope;
-  }
-
+  @Nullable
   public static AdviceScope start(Context parentContext, ActionData actionData) {
     if (!instrumenter().shouldStart(parentContext, actionData)) {
       return null;
@@ -46,8 +47,14 @@ public void closeScope() {
     }
   }
 
-  public void end(Throwable throwable) {
+  public void end(Throwable throwable, Future<Result> responseFuture, Action<?> thisAction) {
     closeScope();
-    instrumenter().end(context, actionData, null, throwable);
+
+    if (throwable == null) {
+      responseFuture.onComplete(
+          new RequestCompleteCallback(getContext()), thisAction.executionContext());
+    } else {
+      instrumenter().end(context, actionData, null, throwable);
+    }
   }
 }