From 0db0458e76ea2740c0cb3ee7fa18656ba9635971 Mon Sep 17 00:00:00 2001 From: Jason Plumb Date: Fri, 30 Sep 2022 15:20:31 -0700 Subject: [PATCH 1/5] guard against null HttpContext --- .../ApacheHttpAsyncClientInstrumentation.java | 15 ++++++++++----- .../ApacheHttpAsyncClientInstrumentation.java | 17 +++++++++++------ 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/instrumentation/apache-httpasyncclient-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpasyncclient/ApacheHttpAsyncClientInstrumentation.java b/instrumentation/apache-httpasyncclient-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpasyncclient/ApacheHttpAsyncClientInstrumentation.java index 849d449e1ddf..55e250293381 100644 --- a/instrumentation/apache-httpasyncclient-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpasyncclient/ApacheHttpAsyncClientInstrumentation.java +++ b/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; @@ -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); } } } diff --git a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpAsyncClientInstrumentation.java b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpAsyncClientInstrumentation.java index 85f8ae28c9bf..0244ba887ff6 100644 --- a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpAsyncClientInstrumentation.java +++ b/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; @@ -159,7 +160,7 @@ public static class WrappedFutureCallback implements FutureCallback { 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 delegate; private volatile Context context; @@ -182,7 +183,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 +205,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 +228,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 +258,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); } } } From 61b706b59ec77c79e542ab1738ebe5d26df8c019 Mon Sep 17 00:00:00 2001 From: Jason Plumb Date: Fri, 30 Sep 2022 15:58:14 -0700 Subject: [PATCH 2/5] add @Nullable --- .../ApacheHttpAsyncClientInstrumentation.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/apache-httpasyncclient-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpasyncclient/ApacheHttpAsyncClientInstrumentation.java b/instrumentation/apache-httpasyncclient-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpasyncclient/ApacheHttpAsyncClientInstrumentation.java index 55e250293381..53b9cffb58a7 100644 --- a/instrumentation/apache-httpasyncclient-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpasyncclient/ApacheHttpAsyncClientInstrumentation.java +++ b/instrumentation/apache-httpasyncclient-4.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpasyncclient/ApacheHttpAsyncClientInstrumentation.java @@ -149,7 +149,7 @@ public static class WrappedFutureCallback implements FutureCallback { 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 delegate; private volatile Context context; From 7cf172e321b300820be06b79debc3e13c008b97a Mon Sep 17 00:00:00 2001 From: Jason Plumb Date: Thu, 6 Oct 2022 09:18:36 -0700 Subject: [PATCH 3/5] default the http context --- .../v5_0/ApacheHttpAsyncClientInstrumentation.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpAsyncClientInstrumentation.java b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpAsyncClientInstrumentation.java index 0244ba887ff6..889202be6a32 100644 --- a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpAsyncClientInstrumentation.java +++ b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpAsyncClientInstrumentation.java @@ -33,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; @@ -68,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); From 8573b027994778ed52ed80f9c6641ae061b41ec2 Mon Sep 17 00:00:00 2001 From: Jason Plumb Date: Thu, 6 Oct 2022 13:21:44 -0700 Subject: [PATCH 4/5] remove null guard --- .../v5_0/ApacheHttpAsyncClientInstrumentation.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpAsyncClientInstrumentation.java b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpAsyncClientInstrumentation.java index 889202be6a32..b51f077ff745 100644 --- a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpAsyncClientInstrumentation.java +++ b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpAsyncClientInstrumentation.java @@ -264,9 +264,6 @@ private void cancelDelegate() { @Nullable private HttpResponse getResponseFromHttpContext() { - if (httpContext == null) { - return null; - } return (HttpResponse) httpContext.getAttribute(HttpCoreContext.HTTP_RESPONSE); } } From 0634baa7ae9c251389ea7ddf91b0532a277a281f Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Thu, 6 Oct 2022 13:48:26 -0700 Subject: [PATCH 5/5] Update instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpAsyncClientInstrumentation.java --- .../v5_0/ApacheHttpAsyncClientInstrumentation.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpAsyncClientInstrumentation.java b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpAsyncClientInstrumentation.java index b51f077ff745..5a6885710e0e 100644 --- a/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpAsyncClientInstrumentation.java +++ b/instrumentation/apache-httpclient/apache-httpclient-5.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachehttpclient/v5_0/ApacheHttpAsyncClientInstrumentation.java @@ -164,7 +164,7 @@ public static class WrappedFutureCallback implements FutureCallback { private static final Logger logger = Logger.getLogger(WrappedFutureCallback.class.getName()); private final Context parentContext; - @Nullable private final HttpContext httpContext; + private final HttpContext httpContext; private final FutureCallback delegate; private volatile Context context;