Handle null callback in HttpAsyncApache instrumentation
MODIFY
dd-java-agent/instrumentation/apache-httpasyncclient-4/src/main/java/datadog/trace/instrumentation/apachehttpasyncclient/ApacheHttpAsyncClientInstrumentation.java
dd-java-agent/instrumentation/apache-httpasyncclient-4/src/main/java/datadog/trace/instrumentation/apachehttpasyncclient/ApacheHttpAsyncClientInstrumentation.java
@@ -180,6 +180,7 @@ public class ApacheHttpAsyncClientInstrumentation extends Instrumenter.Default {
}
this.clientSpan = clientSpan;
this.context = context;
+ // Note: this can be null in real life, so we have to handle this carefully
this.delegate = delegate;
}
@@ -190,11 +191,11 @@ public class ApacheHttpAsyncClientInstrumentation extends Instrumenter.Default {
clientSpan.finish(); // Finish span before calling delegate
if (parentContinuation == null) {
- delegate.completed(result);
+ completeDelegate(result);
} else {
try (final TraceScope scope = parentContinuation.activate()) {
scope.setAsyncPropagation(true);
- delegate.completed(result);
+ completeDelegate(result);
}
}
}
@@ -207,11 +208,11 @@ public class ApacheHttpAsyncClientInstrumentation extends Instrumenter.Default {
clientSpan.finish(); // Finish span before calling delegate
if (parentContinuation == null) {
- delegate.failed(ex);
+ failDelegate(ex);
} else {
try (final TraceScope scope = parentContinuation.activate()) {
scope.setAsyncPropagation(true);
- delegate.failed(ex);
+ failDelegate(ex);
}
}
}
@@ -223,12 +224,30 @@ public class ApacheHttpAsyncClientInstrumentation extends Instrumenter.Default {
clientSpan.finish(); // Finish span before calling delegate
if (parentContinuation == null) {
- delegate.cancelled();
+ cancelDelegate();
} else {
try (final TraceScope scope = parentContinuation.activate()) {
scope.setAsyncPropagation(true);
- delegate.cancelled();
+ cancelDelegate();
+ }
+ }
+ }
+
+ private void completeDelegate(final T result) {
+ if (delegate != null) {
+ delegate.completed(result);
+ }
}
+
+ private void failDelegate(final Exception ex) {
+ if (delegate != null) {
+ delegate.failed(ex);
+ }
+ }
+
+ private void cancelDelegate() {
+ if (delegate != null) {
+ delegate.cancelled();
}
}
}

ADD
dd-java-agent/instrumentation/apache-httpasyncclient-4/src/test/groovy/ApacheHttpAsyncClientNullCallbackTest.groovy
+import datadog.trace.agent.test.base.HttpClientTest
+import datadog.trace.instrumentation.apachehttpasyncclient.ApacheHttpAsyncClientDecorator
+import org.apache.http.client.methods.HttpGet
+import org.apache.http.impl.nio.client.HttpAsyncClients
+import org.apache.http.message.BasicHeader
+import spock.lang.AutoCleanup
+import spock.lang.Shared
+
+import java.util.concurrent.Future
+
+class ApacheHttpAsyncClientNullCallbackTest extends HttpClientTest<ApacheHttpAsyncClientDecorator> {
+
+ @AutoCleanup
+ @Shared
+ def client = HttpAsyncClients.createDefault()
+
+ def setupSpec() {
+ client.start()
+ }
+
+ @Override
+ int doRequest(String method, URI uri, Map<String, String> headers, Closure callback) {
+ assert method == "GET"
+
+ HttpGet request = new HttpGet(uri)
+ headers.entrySet().each {
+ request.addHeader(new BasicHeader(it.key, it.value))
+ }
+
+ // The point here is to test case when callback is null - fire-and-forget style
+ // So to make sure request is done we start request, wait for future to finish
+ // and then call callback if present.
+ Future future = client.execute(request, null)
+ future.get()
+ if (callback != null) {
+ callback()
+ }
+ return 200
+ }
+
+ @Override
+ ApacheHttpAsyncClientDecorator decorator() {
+ return ApacheHttpAsyncClientDecorator.DECORATE
+ }
+
+ @Override
+ Integer statusOnRedirectError() {
+ return 302
+ }
+}

Handle null callback in HttpAsyncApache instrumentation