fix: code review cleanup

Author: dmarkwat

instrumentation/finagle-http-23.11/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/finaglehttp/v23_11/GenStreamingServerDispatcherInstrumentation.java

@@ -14,6 +14,7 @@
 import io.opentelemetry.context.Scope;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
+import javax.annotation.Nullable;
 import net.bytebuddy.asm.Advice;
 import net.bytebuddy.description.type.TypeDescription;
 import net.bytebuddy.matcher.ElementMatcher;
@@ -44,19 +45,20 @@ public void transform(TypeTransformer transformer) {
   public static class DispatchAdvice {
 
     @Advice.OnMethodEnter(suppress = Throwable.class, inline = false)
+    @Nullable
     public static Scope methodEnter(@Advice.Argument(0) Object req) {
       if (req instanceof Request) {
         // practically this will always be a Request, from HttpServerDispatcher
         Request request = (Request) req;
         // if this is null, there's a bug in the instrumentation
         Context context = request.ctx().apply(Helpers.OTEL_CONTEXT_KEY);
-        return context.makeCurrent();
+        return context != null ? context.makeCurrent() : null;
       }
       return null;
     }
 
     @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class, inline = false)
-    public static void methodExit(@Advice.Enter Scope scope) {
+    public static void methodExit(@Advice.Enter @Nullable Scope scope) {
       if (scope != null) {
         scope.close();
       }

instrumentation/finagle-http-23.11/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/finaglehttp/v23_11/Helpers.java

@@ -43,7 +43,7 @@ public class Helpers {
   public static final RecordSchema.Field<Context> OTEL_CONTEXT_KEY =
       Request$.MODULE$.Schema().newField();
 
-  public static final String OTEL_NETTY_HANDLER = "otelFinagleNettyHandler";
+  private static final String OTEL_NETTY_HANDLER = "otelFinagleNettyHandler";
 
   private Helpers() {}
 
@@ -163,8 +163,6 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception
                     FULL_HTTP_REQUEST_CONTEXT.set((FullHttpRequest) msg, Context.current());
                   } else if (msg instanceof HttpRequest) {
                     HTTP_REQUEST_CONTEXT.set((HttpRequest) msg, Context.current());
-                  } else {
-                    throw new IllegalArgumentException("unexpected request type: " + msg);
                   }
 
                   super.channelRead(ctx, msg);
@@ -177,15 +175,12 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception
   Part 2/3 of bridging the otel Context from netty to finagle.
    */
   public static void chainContextToFinagle(Object msg, Request request) {
-    Context context;
+    Context context = null;
     // type switch courtesy of com.twitter.finagle.netty4.http.Netty4ServerStreamTransport.read()
     if (msg instanceof FullHttpRequest) {
       context = FULL_HTTP_REQUEST_CONTEXT.get((FullHttpRequest) msg);
     } else if (msg instanceof HttpRequest) {
       context = HTTP_REQUEST_CONTEXT.get((HttpRequest) msg);
-    } else {
-      // shouldn't practically reach here
-      throw new IllegalArgumentException("unexpected request type: " + msg);
     }
 
     // hook the Context from netty's request up to finagle's request

instrumentation/finagle-http-23.11/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/finaglehttp/v23_11/PromiseInterruptibleInstrumentation.java

@@ -39,7 +39,7 @@ public static class ConstructorAdvice {
 
     @Advice.OnMethodExit(suppress = Throwable.class, inline = false)
     @Advice.AssignReturned.ToArguments(@ToArgument(1))
-    public static PartialFunction<Throwable, BoxedUnit> onEnter(
+    public static PartialFunction<Throwable, BoxedUnit> onExit(
         @Advice.Argument(1) PartialFunction<Throwable, BoxedUnit> handler) {
       if (handler instanceof TwitterUtilCoreHelpers.InterruptibleWithContext) {
         return handler;

instrumentation/finagle-http-23.11/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/finaglehttp/v23_11/PromiseKInstrumentation.java

@@ -15,6 +15,7 @@
 import io.opentelemetry.context.Scope;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
+import javax.annotation.Nullable;
 import net.bytebuddy.asm.Advice;
 import net.bytebuddy.description.type.TypeDescription;
 import net.bytebuddy.matcher.ElementMatcher;
@@ -55,15 +56,18 @@ public static void onExit(@Advice.This Promise.K thiz) {
   @SuppressWarnings("unused")
   public static class ApplyAdvice {
     @Advice.OnMethodEnter(suppress = Throwable.class, inline = false)
+    @Nullable
     public static Scope onApplyEnter(@Advice.This Promise.K thiz) {
       // if this is null, there's a bug in the instrumentation
       Context savedContext = TwitterUtilCoreHelpers.PROMISE_K_CONTEXT_FIELD.get(thiz);
-      return savedContext.makeCurrent();
+      return savedContext != null ? savedContext.makeCurrent() : null;
     }
 
     @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class, inline = false)
-    public static void onApplyExit(@Advice.Enter Scope scope) {
-      scope.close();
+    public static void onApplyExit(@Advice.Enter @Nullable Scope scope) {
+      if (scope != null) {
+        scope.close();
+      }
     }
   }
 }