Guard against null HttpContext (open-telemetry#6792)

Resolves open-telemetry#6787

Co-authored-by: Trask Stalnaker <[email protected]>

Author: 2 people

instrumentation/apache-httpasyncclient-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpasyncclient/ApacheHttpAsyncClientInstrumentation.java

@@ -20,6 +20,7 @@
 import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
 import java.io.IOException;
 import java.util.logging.Logger;
+import javax.annotation.Nullable;
 import net.bytebuddy.asm.Advice;
 import net.bytebuddy.description.type.TypeDescription;
 import net.bytebuddy.matcher.ElementMatcher;
@@ -148,7 +149,7 @@ public static class WrappedFutureCallback<T> implements FutureCallback<T> {
     private static final Logger logger = Logger.getLogger(WrappedFutureCallback.class.getName());
 
     private final Context parentContext;
-    private final HttpContext httpContext;
+    @Nullable private final HttpContext httpContext;
     private final FutureCallback<T> delegate;
 
     private volatile Context context;
@@ -171,7 +172,7 @@ public void completed(T result) {
         return;
       }
 
-      instrumenter().end(context, otelRequest, getResponse(httpContext), null);
+      instrumenter().end(context, otelRequest, getResponseFromHttpContext(), null);
 
       if (parentContext == null) {
         completeDelegate(result);
@@ -193,7 +194,7 @@ public void failed(Exception ex) {
       }
 
       // end span before calling delegate
-      instrumenter().end(context, otelRequest, getResponse(httpContext), ex);
+      instrumenter().end(context, otelRequest, getResponseFromHttpContext(), ex);
 
       if (parentContext == null) {
         failDelegate(ex);
@@ -216,7 +217,7 @@ public void cancelled() {
 
       // TODO (trask) add "canceled" span attribute
       // end span before calling delegate
-      instrumenter().end(context, otelRequest, getResponse(httpContext), null);
+      instrumenter().end(context, otelRequest, getResponseFromHttpContext(), null);
 
       if (parentContext == null) {
         cancelDelegate();
@@ -246,8 +247,12 @@ private void cancelDelegate() {
       }
     }
 
-    private static HttpResponse getResponse(HttpContext context) {
-      return (HttpResponse) context.getAttribute(HttpCoreContext.HTTP_RESPONSE);
+    @Nullable
+    private HttpResponse getResponseFromHttpContext() {
+      if (httpContext == null) {
+        return null;
+      }
+      return (HttpResponse) httpContext.getAttribute(HttpCoreContext.HTTP_RESPONSE);
     }
   }
 }

instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpAsyncClientInstrumentation.java

@@ -21,6 +21,7 @@
 import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
 import java.io.IOException;
 import java.util.logging.Logger;
+import javax.annotation.Nullable;
 import net.bytebuddy.asm.Advice;
 import net.bytebuddy.description.type.TypeDescription;
 import net.bytebuddy.matcher.ElementMatcher;
@@ -32,6 +33,7 @@
 import org.apache.hc.core5.http.nio.AsyncRequestProducer;
 import org.apache.hc.core5.http.nio.DataStreamChannel;
 import org.apache.hc.core5.http.nio.RequestChannel;
+import org.apache.hc.core5.http.protocol.BasicHttpContext;
 import org.apache.hc.core5.http.protocol.HttpContext;
 import org.apache.hc.core5.http.protocol.HttpCoreContext;
 
@@ -67,10 +69,13 @@ public static class ClientAdvice {
     @Advice.OnMethodEnter(suppress = Throwable.class)
     public static void methodEnter(
         @Advice.Argument(value = 0, readOnly = false) AsyncRequestProducer requestProducer,
-        @Advice.Argument(3) HttpContext httpContext,
+        @Advice.Argument(value = 3, readOnly = false) HttpContext httpContext,
         @Advice.Argument(value = 4, readOnly = false) FutureCallback<?> futureCallback) {
 
       Context parentContext = currentContext();
+      if (httpContext == null) {
+        httpContext = new BasicHttpContext();
+      }
 
       WrappedFutureCallback<?> wrappedFutureCallback =
           new WrappedFutureCallback<>(parentContext, httpContext, futureCallback);
@@ -182,7 +187,7 @@ public void completed(T result) {
         return;
       }
 
-      instrumenter().end(context, httpRequest, getResponse(httpContext), null);
+      instrumenter().end(context, httpRequest, getResponseFromHttpContext(), null);
 
       if (parentContext == null) {
         completeDelegate(result);
@@ -204,7 +209,7 @@ public void failed(Exception ex) {
       }
 
       // end span before calling delegate
-      instrumenter().end(context, httpRequest, getResponse(httpContext), ex);
+      instrumenter().end(context, httpRequest, getResponseFromHttpContext(), ex);
 
       if (parentContext == null) {
         failDelegate(ex);
@@ -227,7 +232,7 @@ public void cancelled() {
 
       // TODO (trask) add "canceled" span attribute
       // end span before calling delegate
-      instrumenter().end(context, httpRequest, getResponse(httpContext), null);
+      instrumenter().end(context, httpRequest, getResponseFromHttpContext(), null);
 
       if (parentContext == null) {
         cancelDelegate();
@@ -257,8 +262,9 @@ private void cancelDelegate() {
       }
     }
 
-    private static HttpResponse getResponse(HttpContext context) {
-      return (HttpResponse) context.getAttribute(HttpCoreContext.HTTP_RESPONSE);
+    @Nullable
+    private HttpResponse getResponseFromHttpContext() {
+      return (HttpResponse) httpContext.getAttribute(HttpCoreContext.HTTP_RESPONSE);
     }
   }
 }