Fix instrumentation and add tests to verify.
MODIFY
dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/AWSHttpClientInstrumentation.java
dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/AWSHttpClientInstrumentation.java
@@ -2,7 +2,6 @@ package datadog.trace.instrumentation.aws.v0;
import static datadog.trace.instrumentation.aws.v0.AwsSdkClientDecorator.DECORATE;
import static java.util.Collections.singletonMap;
-import static net.bytebuddy.matcher.ElementMatchers.declaresField;
import static net.bytebuddy.matcher.ElementMatchers.isAbstract;
import static net.bytebuddy.matcher.ElementMatchers.isMethod;
import static net.bytebuddy.matcher.ElementMatchers.named;
@@ -22,12 +21,11 @@ import net.bytebuddy.matcher.ElementMatcher;
/**
* This is additional 'helper' to catch cases when HTTP request throws exception different from
- * {@link AmazonClientException}. In these cases {@link RequestHandler2#afterError} is not called.
- *
- * <p>FIXME: come up with tests for this - maybe some test that mimics timeout?
+ * {@link AmazonClientException} (for example an error thrown by another handler). In these cases
+ * {@link RequestHandler2#afterError} is not called.
*/
@AutoService(Instrumenter.class)
-public final class AWSHttpClientInstrumentation extends Instrumenter.Default {
+public class AWSHttpClientInstrumentation extends Instrumenter.Default {
public AWSHttpClientInstrumentation() {
super("aws-sdk");
@@ -35,8 +33,7 @@ public final class AWSHttpClientInstrumentation extends Instrumenter.Default {
@Override
public ElementMatcher<TypeDescription> typeMatcher() {
- return named("com.amazonaws.http.AmazonHttpClient.RequestExecutor")
- .and(declaresField(named("request")));
+ return named("com.amazonaws.http.AmazonHttpClient");
}
@Override
@@ -58,6 +55,42 @@ public final class AWSHttpClientInstrumentation extends Instrumenter.Default {
}
public static class HttpClientAdvice {
+ @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
+ public static void methodExit(
+ @Advice.Argument(value = 0, optional = true) final Request<?> request,
+ @Advice.Thrown final Throwable throwable) {
+ if (throwable != null) {
+ final Scope scope = request.getHandlerContext(TracingRequestHandler.SCOPE_CONTEXT_KEY);
+ if (scope != null) {
+ request.addHandlerContext(TracingRequestHandler.SCOPE_CONTEXT_KEY, null);
+ DECORATE.onError(scope.span(), throwable);
+ DECORATE.beforeFinish(scope.span());
+ scope.close();
+ }
+ }
+ }
+ }
+
+ /**
+ * Due to a change in the AmazonHttpClient class, this instrumentation is needed to support newer
+ * versions. The above class should cover older versions.
+ */
+ @AutoService(Instrumenter.class)
+ public static final class RequestExecutorInstrumentation extends AWSHttpClientInstrumentation {
+
+ @Override
+ public ElementMatcher<TypeDescription> typeMatcher() {
+ return named("com.amazonaws.http.AmazonHttpClient$RequestExecutor");
+ }
+
+ @Override
+ public Map<? extends ElementMatcher<? super MethodDescription>, String> transformers() {
+ return singletonMap(
+ isMethod().and(not(isAbstract())).and(named("doExecute")),
+ RequestExecutorAdvice.class.getName());
+ }
+
+ public static class RequestExecutorAdvice {
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
public static void methodExit(
@Advice.FieldValue("request") final Request<?> request,
@@ -74,3 +107,4 @@ public final class AWSHttpClientInstrumentation extends Instrumenter.Default {
}
}
}
+}

MODIFY
dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/TracingRequestHandler.java
dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/TracingRequestHandler.java
@@ -16,7 +16,8 @@ public class TracingRequestHandler extends RequestHandler2 {
// Note: aws1.x sdk doesn't have any truly async clients so we can store scope in request context
// safely.
- static final HandlerContextKey<Scope> SCOPE_CONTEXT_KEY = new HandlerContextKey<>("DatadogScope");
+ public static final HandlerContextKey<Scope> SCOPE_CONTEXT_KEY =
+ new HandlerContextKey<>("DatadogScope");
@Override
public AmazonWebServiceRequest beforeMarshalling(final AmazonWebServiceRequest request) {

MODIFY
dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/test/groovy/AWSClientTest.groovy
dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/test/groovy/AWSClientTest.groovy
import com.amazonaws.AmazonClientException
import com.amazonaws.ClientConfiguration
+import com.amazonaws.Request
import com.amazonaws.SDKGlobalConfiguration
import com.amazonaws.auth.AWSCredentialsProviderChain
import com.amazonaws.auth.BasicAWSCredentials
@@ -15,16 +16,18 @@ import com.amazonaws.services.rds.model.DeleteOptionGroupRequest
import com.amazonaws.services.s3.AmazonS3Client
import com.amazonaws.services.s3.S3ClientOptions
import datadog.trace.agent.test.AgentTestRunner
-import datadog.trace.agent.test.utils.PortUtils
import datadog.trace.api.DDSpanTypes
+import io.opentracing.Tracer
import io.opentracing.tag.Tags
import org.apache.http.conn.HttpHostConnectException
+import org.apache.http.impl.execchain.RequestAbortedException
import spock.lang.AutoCleanup
import spock.lang.Shared
import java.util.concurrent.atomic.AtomicReference
import static datadog.trace.agent.test.server.http.TestHttpServer.httpServer
+import static datadog.trace.agent.test.utils.PortUtils.UNUSABLE_PORT
class AWSClientTest extends AgentTestRunner {
@@ -32,7 +35,7 @@ class AWSClientTest extends AgentTestRunner {
new EnvironmentVariableCredentialsProvider(),
new SystemPropertiesCredentialsProvider(),
new ProfileCredentialsProvider(),
- new InstanceProfileCredentialsProvider());
+ new InstanceProfileCredentialsProvider())
def setupSpec() {
System.setProperty(SDKGlobalConfiguration.ACCESS_KEY_SYSTEM_PROPERTY, "my-access-key")
@@ -176,11 +179,11 @@ class AWSClientTest extends AgentTestRunner {
parent()
tags {
"$Tags.COMPONENT.key" "java-aws-sdk"
- "$Tags.HTTP_URL.key" "http://localhost:${PortUtils.UNUSABLE_PORT}"
+ "$Tags.HTTP_URL.key" "http://localhost:${UNUSABLE_PORT}"
"$Tags.HTTP_METHOD.key" "$method"
"$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
"aws.service" { it.contains(service) }
- "aws.endpoint" "http://localhost:${PortUtils.UNUSABLE_PORT}"
+ "aws.endpoint" "http://localhost:${UNUSABLE_PORT}"
"aws.operation" "${operation}Request"
"aws.agent" "java-aws-sdk"
errorTags AmazonClientException, ~/Unable to execute HTTP request/
@@ -195,9 +198,9 @@ class AWSClientTest extends AgentTestRunner {
childOf(span(0))
tags {
"$Tags.COMPONENT.key" "apache-httpclient"
- "$Tags.HTTP_URL.key" "http://localhost:${PortUtils.UNUSABLE_PORT}/$url"
+ "$Tags.HTTP_URL.key" "http://localhost:${UNUSABLE_PORT}/$url"
"$Tags.PEER_HOSTNAME.key" "localhost"
- "$Tags.PEER_PORT.key" PortUtils.UNUSABLE_PORT
+ "$Tags.PEER_PORT.key" UNUSABLE_PORT
"$Tags.HTTP_METHOD.key" "$method"
"$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
errorTags HttpHostConnectException, ~/Connection refused/
@@ -209,6 +212,120 @@ class AWSClientTest extends AgentTestRunner {
where:
service | operation | method | url | call | body | client
- "S3" | "GetObject" | "GET" | "someBucket/someKey" | { client -> client.getObject("someBucket", "someKey") } | "" | new AmazonS3Client(CREDENTIALS_PROVIDER_CHAIN, new ClientConfiguration().withRetryPolicy(PredefinedRetryPolicies.getDefaultRetryPolicyWithCustomMaxRetries(0))).withEndpoint("http://localhost:${PortUtils.UNUSABLE_PORT}")
+ "S3" | "GetObject" | "GET" | "someBucket/someKey" | { client -> client.getObject("someBucket", "someKey") } | "" | new AmazonS3Client(CREDENTIALS_PROVIDER_CHAIN, new ClientConfiguration().withRetryPolicy(PredefinedRetryPolicies.getDefaultRetryPolicyWithCustomMaxRetries(0))).withEndpoint("http://localhost:${UNUSABLE_PORT}")
+ }
+
+ def "naughty request handler doesn't break the trace"() {
+ setup:
+ def client = new AmazonS3Client(CREDENTIALS_PROVIDER_CHAIN)
+ client.addRequestHandler(new RequestHandler2() {
+ void beforeRequest(Request<?> request) {
+ throw new RuntimeException("bad handler")
+ }
+ })
+
+ when:
+ client.getObject("someBucket", "someKey")
+
+ then:
+ ((Tracer) TEST_TRACER).activeSpan() == null
+ thrown RuntimeException
+
+ assertTraces(1) {
+ trace(0, 1) {
+ span(0) {
+ serviceName "java-aws-sdk"
+ operationName "aws.http"
+ resourceName "S3.GetObject"
+ spanType DDSpanTypes.HTTP_CLIENT
+ errored true
+ parent()
+ tags {
+ "$Tags.COMPONENT.key" "java-aws-sdk"
+ "$Tags.HTTP_URL.key" "https://s3.amazonaws.com"
+ "$Tags.HTTP_METHOD.key" "GET"
+ "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
+ "aws.service" "Amazon S3"
+ "aws.endpoint" "https://s3.amazonaws.com"
+ "aws.operation" "GetObjectRequest"
+ "aws.agent" "java-aws-sdk"
+ errorTags RuntimeException, "bad handler"
+ defaultTags()
+ }
+ }
+ }
+ }
+ }
+
+ def "timeout and retry errors captured"() {
+ setup:
+ def server = httpServer {
+ handlers {
+ all {
+ Thread.sleep(500)
+ response.status(200).send()
+ }
+ }
+ }
+ AmazonS3Client client = new AmazonS3Client(new ClientConfiguration().withRequestTimeout(50 /* ms */))
+ .withEndpoint("http://localhost:$server.address.port")
+
+ when:
+ client.getObject("someBucket", "someKey")
+
+ then:
+ ((Tracer) TEST_TRACER).activeSpan() == null
+ thrown AmazonClientException
+
+ assertTraces(1) {
+ trace(0, 5) {
+ span(0) {
+ serviceName "java-aws-sdk"
+ operationName "aws.http"
+ resourceName "S3.GetObject"
+ spanType DDSpanTypes.HTTP_CLIENT
+ errored true
+ parent()
+ tags {
+ "$Tags.COMPONENT.key" "java-aws-sdk"
+ "$Tags.HTTP_URL.key" "http://localhost:$server.address.port"
+ "$Tags.HTTP_METHOD.key" "GET"
+ "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
+ "aws.service" "Amazon S3"
+ "aws.endpoint" "http://localhost:$server.address.port"
+ "aws.operation" "GetObjectRequest"
+ "aws.agent" "java-aws-sdk"
+ errorTags AmazonClientException, ~/Unable to execute HTTP request/
+ defaultTags()
+ }
+ }
+ (1..4).each {
+ span(it) {
+ operationName "http.request"
+ resourceName "GET /someBucket/someKey"
+ spanType DDSpanTypes.HTTP_CLIENT
+ errored true
+ childOf(span(0))
+ tags {
+ "$Tags.COMPONENT.key" "apache-httpclient"
+ "$Tags.HTTP_URL.key" "http://localhost:$server.address.port/someBucket/someKey"
+ "$Tags.PEER_HOSTNAME.key" "localhost"
+ "$Tags.PEER_PORT.key" server.address.port
+ "$Tags.HTTP_METHOD.key" "GET"
+ "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
+ try {
+ errorTags SocketException, "Socket closed"
+ } catch (AssertionError e) {
+ errorTags RequestAbortedException, "Request aborted"
+ }
+ defaultTags()
+ }
+ }
+ }
+ }
+ }
+
+ cleanup:
+ server.close()
}
}

MODIFY
dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/test_1_11_106/groovy/AWSClientTest.groovy
dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/test_1_11_106/groovy/AWSClientTest.groovy
+import com.amazonaws.AmazonClientException
import com.amazonaws.AmazonWebServiceClient
import com.amazonaws.ClientConfiguration
+import com.amazonaws.Request
import com.amazonaws.SDKGlobalConfiguration
import com.amazonaws.SdkClientException
import com.amazonaws.auth.AWSCredentialsProviderChain
@@ -20,16 +22,18 @@ import com.amazonaws.services.rds.model.DeleteOptionGroupRequest
import com.amazonaws.services.s3.AmazonS3Client
import com.amazonaws.services.s3.AmazonS3ClientBuilder
import datadog.trace.agent.test.AgentTestRunner
-import datadog.trace.agent.test.utils.PortUtils
import datadog.trace.api.DDSpanTypes
+import io.opentracing.Tracer
import io.opentracing.tag.Tags
import org.apache.http.conn.HttpHostConnectException
+import org.apache.http.impl.execchain.RequestAbortedException
import spock.lang.AutoCleanup
import spock.lang.Shared
import java.util.concurrent.atomic.AtomicReference
import static datadog.trace.agent.test.server.http.TestHttpServer.httpServer
+import static datadog.trace.agent.test.utils.PortUtils.UNUSABLE_PORT
class AWSClientTest extends AgentTestRunner {
@@ -37,7 +41,7 @@ class AWSClientTest extends AgentTestRunner {
new EnvironmentVariableCredentialsProvider(),
new SystemPropertiesCredentialsProvider(),
new ProfileCredentialsProvider(),
- new InstanceProfileCredentialsProvider());
+ new InstanceProfileCredentialsProvider())
def setupSpec() {
System.setProperty(SDKGlobalConfiguration.ACCESS_KEY_SYSTEM_PROPERTY, "my-access-key")
@@ -109,9 +113,11 @@ class AWSClientTest extends AgentTestRunner {
def "send #operation request with mocked response"() {
setup:
responseBody.set(body)
+
+ when:
def response = call.call(client)
- expect:
+ then:
response != null
client.requestHandler2s != null
@@ -203,11 +209,11 @@ class AWSClientTest extends AgentTestRunner {
parent()
tags {
"$Tags.COMPONENT.key" "java-aws-sdk"
- "$Tags.HTTP_URL.key" "http://localhost:${PortUtils.UNUSABLE_PORT}"
+ "$Tags.HTTP_URL.key" "http://localhost:${UNUSABLE_PORT}"
"$Tags.HTTP_METHOD.key" "$method"
"$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
"aws.service" { it.contains(service) }
- "aws.endpoint" "http://localhost:${PortUtils.UNUSABLE_PORT}"
+ "aws.endpoint" "http://localhost:${UNUSABLE_PORT}"
"aws.operation" "${operation}Request"
"aws.agent" "java-aws-sdk"
errorTags SdkClientException, ~/Unable to execute HTTP request/
@@ -222,9 +228,9 @@ class AWSClientTest extends AgentTestRunner {
childOf(span(0))
tags {
"$Tags.COMPONENT.key" "apache-httpclient"
- "$Tags.HTTP_URL.key" "http://localhost:${PortUtils.UNUSABLE_PORT}/$url"
+ "$Tags.HTTP_URL.key" "http://localhost:${UNUSABLE_PORT}/$url"
"$Tags.PEER_HOSTNAME.key" "localhost"
- "$Tags.PEER_PORT.key" PortUtils.UNUSABLE_PORT
+ "$Tags.PEER_PORT.key" UNUSABLE_PORT
"$Tags.HTTP_METHOD.key" "$method"
"$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
errorTags HttpHostConnectException, ~/Connection refused/
@@ -236,6 +242,124 @@ class AWSClientTest extends AgentTestRunner {
where:
service | operation | method | url | call | body | client
- "S3" | "GetObject" | "GET" | "someBucket/someKey" | { client -> client.getObject("someBucket", "someKey") } | "" | new AmazonS3Client(CREDENTIALS_PROVIDER_CHAIN, new ClientConfiguration().withRetryPolicy(PredefinedRetryPolicies.getDefaultRetryPolicyWithCustomMaxRetries(0))).withEndpoint("http://localhost:${PortUtils.UNUSABLE_PORT}")
+ "S3" | "GetObject" | "GET" | "someBucket/someKey" | { client -> client.getObject("someBucket", "someKey") } | "" | new AmazonS3Client(CREDENTIALS_PROVIDER_CHAIN, new ClientConfiguration().withRetryPolicy(PredefinedRetryPolicies.getDefaultRetryPolicyWithCustomMaxRetries(0))).withEndpoint("http://localhost:${UNUSABLE_PORT}")
+ }
+
+ def "naughty request handler doesn't break the trace"() {
+ setup:
+ def client = new AmazonS3Client(CREDENTIALS_PROVIDER_CHAIN)
+ client.addRequestHandler(new RequestHandler2() {
+ void beforeRequest(Request<?> request) {
+ throw new RuntimeException("bad handler")
+ }
+ })
+
+ when:
+ client.getObject("someBucket", "someKey")
+
+ then:
+ ((Tracer) TEST_TRACER).activeSpan() == null
+ thrown RuntimeException
+
+ assertTraces(1) {
+ trace(0, 1) {
+ span(0) {
+ serviceName "java-aws-sdk"
+ operationName "aws.http"
+ resourceName "S3.HeadBucket"
+ spanType DDSpanTypes.HTTP_CLIENT
+ errored true
+ parent()
+ tags {
+ "$Tags.COMPONENT.key" "java-aws-sdk"
+ "$Tags.HTTP_URL.key" "https://s3.amazonaws.com"
+ "$Tags.HTTP_METHOD.key" "HEAD"
+ "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
+ "aws.service" "Amazon S3"
+ "aws.endpoint" "https://s3.amazonaws.com"
+ "aws.operation" "HeadBucketRequest"
+ "aws.agent" "java-aws-sdk"
+ errorTags RuntimeException, "bad handler"
+ defaultTags()
+ }
+ }
+ }
+ }
+ }
+
+ def "timeout and retry errors captured"() {
+ setup:
+ def server = httpServer {
+ handlers {
+ all {
+ Thread.sleep(500)
+ response.status(200).send()
+ }
+ }
+ }
+ AmazonS3Client client = new AmazonS3Client(new ClientConfiguration().withRequestTimeout(50 /* ms */))
+ .withEndpoint("http://localhost:$server.address.port")
+
+ when:
+ client.getObject("someBucket", "someKey")
+
+ then:
+ ((Tracer) TEST_TRACER).activeSpan() == null
+ thrown AmazonClientException
+
+ assertTraces(1) {
+ trace(0, 5) {
+ span(0) {
+ serviceName "java-aws-sdk"
+ operationName "aws.http"
+ resourceName "S3.GetObject"
+ spanType DDSpanTypes.HTTP_CLIENT
+ errored true
+ parent()
+ tags {
+ "$Tags.COMPONENT.key" "java-aws-sdk"
+ "$Tags.HTTP_URL.key" "http://localhost:$server.address.port"
+ "$Tags.HTTP_METHOD.key" "GET"
+ "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
+ "aws.service" "Amazon S3"
+ "aws.endpoint" "http://localhost:$server.address.port"
+ "aws.operation" "GetObjectRequest"
+ "aws.agent" "java-aws-sdk"
+ try {
+ errorTags AmazonClientException, ~/Unable to execute HTTP request/
+ } catch (AssertionError e) {
+ errorTags SdkClientException, "Unable to execute HTTP request: Request did not complete before the request timeout configuration."
+ }
+ defaultTags()
+ }
+ }
+ (1..4).each {
+ span(it) {
+ operationName "http.request"
+ resourceName "GET /someBucket/someKey"
+ spanType DDSpanTypes.HTTP_CLIENT
+ errored true
+ childOf(span(0))
+ tags {
+ "$Tags.COMPONENT.key" "apache-httpclient"
+ "$Tags.HTTP_URL.key" "http://localhost:$server.address.port/someBucket/someKey"
+ "$Tags.PEER_HOSTNAME.key" "localhost"
+ "$Tags.PEER_PORT.key" server.address.port
+ "$Tags.HTTP_METHOD.key" "GET"
+ "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
+ try {
+ errorTags SocketException, "Socket closed"
+ } catch (AssertionError e) {
+ errorTags RequestAbortedException, "Request aborted"
+ }
+ defaultTags()
+ }
+ }
+ }
+ }
+ }
+
+ cleanup:
+ server.close()
}
}

Fix instrumentation and add tests to verify.