diff --git a/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/AWSHttpClientInstrumentation.java b/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/AWSHttpClientInstrumentation.java new file mode 100644 index 0000000000..8dc1f804eb --- /dev/null +++ b/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/AWSHttpClientInstrumentation.java @@ -0,0 +1,110 @@ +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.isAbstract; +import static net.bytebuddy.matcher.ElementMatchers.isMethod; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.not; + +import com.amazonaws.AmazonClientException; +import com.amazonaws.Request; +import com.amazonaws.handlers.RequestHandler2; +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import io.opentracing.Scope; +import java.util.Map; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +/** + * This is additional 'helper' to catch cases when HTTP request throws exception different from + * {@link AmazonClientException} (for example an error thrown by another handler). In these cases + * {@link RequestHandler2#afterError} is not called. + */ +@AutoService(Instrumenter.class) +public class AWSHttpClientInstrumentation extends Instrumenter.Default { + + public AWSHttpClientInstrumentation() { + super("aws-sdk"); + } + + @Override + public ElementMatcher typeMatcher() { + return named("com.amazonaws.http.AmazonHttpClient"); + } + + @Override + public String[] helperClassNames() { + return new String[] { + "datadog.trace.agent.decorator.BaseDecorator", + "datadog.trace.agent.decorator.ClientDecorator", + "datadog.trace.agent.decorator.HttpClientDecorator", + packageName + ".AwsSdkClientDecorator", + packageName + ".TracingRequestHandler", + }; + } + + @Override + public Map, String> transformers() { + return singletonMap( + isMethod().and(not(isAbstract())).and(named("doExecute")), + HttpClientAdvice.class.getName()); + } + + 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 typeMatcher() { + return named("com.amazonaws.http.AmazonHttpClient$RequestExecutor"); + } + + @Override + public Map, 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, + @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(); + } + } + } + } + } +} diff --git a/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/TracingRequestHandler.java b/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/TracingRequestHandler.java index 51b2a85432..cdb0c24bac 100644 --- a/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/TracingRequestHandler.java +++ b/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/TracingRequestHandler.java @@ -16,7 +16,7 @@ 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. - private static final HandlerContextKey SCOPE_CONTEXT_KEY = + public static final HandlerContextKey SCOPE_CONTEXT_KEY = new HandlerContextKey<>("DatadogScope"); @Override @@ -35,16 +35,22 @@ public class TracingRequestHandler extends RequestHandler2 { @Override public void afterResponse(final Request request, final Response response) { final Scope scope = request.getHandlerContext(SCOPE_CONTEXT_KEY); - DECORATE.onResponse(scope.span(), response); - DECORATE.beforeFinish(scope.span()); - scope.close(); + if (scope != null) { + request.addHandlerContext(SCOPE_CONTEXT_KEY, null); + DECORATE.onResponse(scope.span(), response); + DECORATE.beforeFinish(scope.span()); + scope.close(); + } } @Override public void afterError(final Request request, final Response response, final Exception e) { final Scope scope = request.getHandlerContext(SCOPE_CONTEXT_KEY); - DECORATE.onError(scope.span(), e); - DECORATE.beforeFinish(scope.span()); - scope.close(); + if (scope != null) { + request.addHandlerContext(SCOPE_CONTEXT_KEY, null); + DECORATE.onError(scope.span(), e); + DECORATE.beforeFinish(scope.span()); + scope.close(); + } } } diff --git a/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/test/groovy/AWSClientTest.groovy b/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/test/groovy/AWSClientTest.groovy index da91adb2f4..f23c15edd4 100644 --- a/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/test/groovy/AWSClientTest.groovy +++ b/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/test/groovy/AWSClientTest.groovy @@ -1,6 +1,15 @@ +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 +import com.amazonaws.auth.EnvironmentVariableCredentialsProvider +import com.amazonaws.auth.InstanceProfileCredentialsProvider +import com.amazonaws.auth.SystemPropertiesCredentialsProvider +import com.amazonaws.auth.profile.ProfileCredentialsProvider import com.amazonaws.handlers.RequestHandler2 +import com.amazonaws.retry.PredefinedRetryPolicies import com.amazonaws.services.ec2.AmazonEC2Client import com.amazonaws.services.rds.AmazonRDSClient import com.amazonaws.services.rds.model.DeleteOptionGroupRequest @@ -8,15 +17,26 @@ import com.amazonaws.services.s3.AmazonS3Client import com.amazonaws.services.s3.S3ClientOptions import datadog.trace.agent.test.AgentTestRunner 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 { + + private static final CREDENTIALS_PROVIDER_CHAIN = new AWSCredentialsProviderChain( + new EnvironmentVariableCredentialsProvider(), + new SystemPropertiesCredentialsProvider(), + new ProfileCredentialsProvider(), + new InstanceProfileCredentialsProvider()) + def setupSpec() { System.setProperty(SDKGlobalConfiguration.ACCESS_KEY_SYSTEM_PROPERTY, "my-access-key") System.setProperty(SDKGlobalConfiguration.SECRET_KEY_SYSTEM_PROPERTY, "my-secret-key") @@ -63,9 +83,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 @@ -135,4 +157,175 @@ class AWSClientTest extends AgentTestRunner { """ | new AmazonRDSClient().withEndpoint("http://localhost:$server.address.port") } + + def "send #operation request to closed port"() { + setup: + responseBody.set(body) + + when: + call.call(client) + + then: + thrown AmazonClientException + + assertTraces(1) { + trace(0, 2) { + span(0) { + serviceName "java-aws-sdk" + operationName "aws.http" + resourceName "$service.$operation" + spanType DDSpanTypes.HTTP_CLIENT + errored true + parent() + tags { + "$Tags.COMPONENT.key" "java-aws-sdk" + "$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:${UNUSABLE_PORT}" + "aws.operation" "${operation}Request" + "aws.agent" "java-aws-sdk" + errorTags AmazonClientException, ~/Unable to execute HTTP request/ + defaultTags() + } + } + span(1) { + operationName "http.request" + resourceName "$method /$url" + spanType DDSpanTypes.HTTP_CLIENT + errored true + childOf(span(0)) + tags { + "$Tags.COMPONENT.key" "apache-httpclient" + "$Tags.HTTP_URL.key" "http://localhost:${UNUSABLE_PORT}/$url" + "$Tags.PEER_HOSTNAME.key" "localhost" + "$Tags.PEER_PORT.key" UNUSABLE_PORT + "$Tags.HTTP_METHOD.key" "$method" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + errorTags HttpHostConnectException, ~/Connection refused/ + defaultTags() + } + } + } + } + + 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:${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() + } } diff --git a/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/test_1_11_106/groovy/AWSClientTest.groovy b/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/test_1_11_106/groovy/AWSClientTest.groovy index 566fa3501c..8a42c34b28 100644 --- a/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/test_1_11_106/groovy/AWSClientTest.groovy +++ b/dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/test_1_11_106/groovy/AWSClientTest.groovy @@ -1,11 +1,21 @@ +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 import com.amazonaws.auth.AWSStaticCredentialsProvider import com.amazonaws.auth.AnonymousAWSCredentials import com.amazonaws.auth.BasicAWSCredentials +import com.amazonaws.auth.EnvironmentVariableCredentialsProvider +import com.amazonaws.auth.InstanceProfileCredentialsProvider +import com.amazonaws.auth.SystemPropertiesCredentialsProvider +import com.amazonaws.auth.profile.ProfileCredentialsProvider import com.amazonaws.client.builder.AwsClientBuilder import com.amazonaws.handlers.RequestHandler2 import com.amazonaws.regions.Regions +import com.amazonaws.retry.PredefinedRetryPolicies import com.amazonaws.services.ec2.AmazonEC2ClientBuilder import com.amazonaws.services.rds.AmazonRDSClientBuilder import com.amazonaws.services.rds.model.DeleteOptionGroupRequest @@ -13,15 +23,26 @@ import com.amazonaws.services.s3.AmazonS3Client import com.amazonaws.services.s3.AmazonS3ClientBuilder import datadog.trace.agent.test.AgentTestRunner 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 { + + private static final CREDENTIALS_PROVIDER_CHAIN = new AWSCredentialsProviderChain( + new EnvironmentVariableCredentialsProvider(), + new SystemPropertiesCredentialsProvider(), + new ProfileCredentialsProvider(), + new InstanceProfileCredentialsProvider()) + def setupSpec() { System.setProperty(SDKGlobalConfiguration.ACCESS_KEY_SYSTEM_PROPERTY, "my-access-key") System.setProperty(SDKGlobalConfiguration.SECRET_KEY_SYSTEM_PROPERTY, "my-secret-key") @@ -92,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 @@ -164,4 +187,179 @@ class AWSClientTest extends AgentTestRunner { """ | AmazonRDSClientBuilder.standard().withEndpointConfiguration(endpoint).withCredentials(credentialsProvider).build() } + + def "send #operation request to closed port"() { + setup: + responseBody.set(body) + + when: + call.call(client) + + then: + thrown SdkClientException + + assertTraces(1) { + trace(0, 2) { + span(0) { + serviceName "java-aws-sdk" + operationName "aws.http" + resourceName "$service.$operation" + spanType DDSpanTypes.HTTP_CLIENT + errored true + parent() + tags { + "$Tags.COMPONENT.key" "java-aws-sdk" + "$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:${UNUSABLE_PORT}" + "aws.operation" "${operation}Request" + "aws.agent" "java-aws-sdk" + errorTags SdkClientException, ~/Unable to execute HTTP request/ + defaultTags() + } + } + span(1) { + operationName "http.request" + resourceName "$method /$url" + spanType DDSpanTypes.HTTP_CLIENT + errored true + childOf(span(0)) + tags { + "$Tags.COMPONENT.key" "apache-httpclient" + "$Tags.HTTP_URL.key" "http://localhost:${UNUSABLE_PORT}/$url" + "$Tags.PEER_HOSTNAME.key" "localhost" + "$Tags.PEER_PORT.key" UNUSABLE_PORT + "$Tags.HTTP_METHOD.key" "$method" + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + errorTags HttpHostConnectException, ~/Connection refused/ + defaultTags() + } + } + } + } + + 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:${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() + } } diff --git a/dd-java-agent/instrumentation/aws-java-sdk-2.2/src/main/java/datadog/trace/instrumentation/aws/v2/AwsHttpClientInstrumentation.java b/dd-java-agent/instrumentation/aws-java-sdk-2.2/src/main/java/datadog/trace/instrumentation/aws/v2/AwsHttpClientInstrumentation.java index 800bf18d86..e7777ca723 100644 --- a/dd-java-agent/instrumentation/aws-java-sdk-2.2/src/main/java/datadog/trace/instrumentation/aws/v2/AwsHttpClientInstrumentation.java +++ b/dd-java-agent/instrumentation/aws-java-sdk-2.2/src/main/java/datadog/trace/instrumentation/aws/v2/AwsHttpClientInstrumentation.java @@ -43,6 +43,7 @@ public final class AwsHttpClientInstrumentation extends AbstractAwsClientInstrum } public static class AwsHttpClientAdvice { + // scope.close here doesn't actually close the span. /** * FIXME: This is a hack to prevent netty instrumentation from messing things up. diff --git a/dd-java-agent/instrumentation/aws-java-sdk-2.2/src/main/java8/datadog/trace/instrumentation/aws/v2/TracingExecutionInterceptor.java b/dd-java-agent/instrumentation/aws-java-sdk-2.2/src/main/java8/datadog/trace/instrumentation/aws/v2/TracingExecutionInterceptor.java index d1e3d58187..377c85e0bc 100644 --- a/dd-java-agent/instrumentation/aws-java-sdk-2.2/src/main/java8/datadog/trace/instrumentation/aws/v2/TracingExecutionInterceptor.java +++ b/dd-java-agent/instrumentation/aws-java-sdk-2.2/src/main/java8/datadog/trace/instrumentation/aws/v2/TracingExecutionInterceptor.java @@ -62,18 +62,26 @@ public class TracingExecutionInterceptor implements ExecutionInterceptor { public void afterExecution( final Context.AfterExecution context, final ExecutionAttributes executionAttributes) { final Span span = executionAttributes.getAttribute(SPAN_ATTRIBUTE); - // Call onResponse on both types of responses: - DECORATE.onResponse(span, context.response()); - DECORATE.onResponse(span, context.httpResponse()); - DECORATE.beforeFinish(span); - span.finish(); + if (span != null) { + executionAttributes.putAttribute(SPAN_ATTRIBUTE, null); + // Call onResponse on both types of responses: + DECORATE.onResponse(span, context.response()); + DECORATE.onResponse(span, context.httpResponse()); + DECORATE.beforeFinish(span); + span.finish(); + } } @Override public void onExecutionFailure( final Context.FailedExecution context, final ExecutionAttributes executionAttributes) { final Span span = executionAttributes.getAttribute(SPAN_ATTRIBUTE); - DECORATE.onError(span, context.exception()); + if (span != null) { + executionAttributes.putAttribute(SPAN_ATTRIBUTE, null); + DECORATE.onError(span, context.exception()); + DECORATE.beforeFinish(span); + span.finish(); + } } public static Consumer getOverrideConfigurationConsumer() { diff --git a/dd-java-agent/instrumentation/aws-java-sdk-2.2/src/test/groovy/AwsClientTest.groovy b/dd-java-agent/instrumentation/aws-java-sdk-2.2/src/test/groovy/AwsClientTest.groovy index 91e6735340..bd68934d38 100644 --- a/dd-java-agent/instrumentation/aws-java-sdk-2.2/src/test/groovy/AwsClientTest.groovy +++ b/dd-java-agent/instrumentation/aws-java-sdk-2.2/src/test/groovy/AwsClientTest.groovy @@ -4,6 +4,8 @@ import io.opentracing.tag.Tags import software.amazon.awssdk.auth.credentials.AwsBasicCredentials import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider import software.amazon.awssdk.core.async.AsyncResponseTransformer +import software.amazon.awssdk.core.exception.SdkClientException +import software.amazon.awssdk.http.apache.ApacheHttpClient import software.amazon.awssdk.regions.Region import software.amazon.awssdk.services.ec2.Ec2AsyncClient import software.amazon.awssdk.services.ec2.Ec2Client @@ -17,6 +19,7 @@ import software.amazon.awssdk.services.s3.model.GetObjectRequest import spock.lang.AutoCleanup import spock.lang.Shared +import java.time.Duration import java.util.concurrent.Future import java.util.concurrent.atomic.AtomicReference @@ -39,13 +42,11 @@ class AwsClientTest extends AgentTestRunner { } } } - @Shared - def serverUrl = new URI("http://localhost:$server.address.port") def "send #operation request with builder {#builder.class.getName()} mocked response"() { setup: def client = builder - .endpointOverride(serverUrl) + .endpointOverride(server.address) .region(Region.AP_NORTHEAST_1) .credentialsProvider(CREDENTIALS_PROVIDER) .build() @@ -131,7 +132,7 @@ class AwsClientTest extends AgentTestRunner { def "send #operation async request with builder {#builder.class.getName()} mocked response"() { setup: def client = builder - .endpointOverride(serverUrl) + .endpointOverride(server.address) .region(Region.AP_NORTHEAST_1) .credentialsProvider(CREDENTIALS_PROVIDER) .build() @@ -225,5 +226,75 @@ class AwsClientTest extends AgentTestRunner { """ | RdsAsyncClient.builder() } - // TODO: add timeout tests? + def "timeout and retry errors captured"() { + setup: + def server = httpServer { + handlers { + all { + Thread.sleep(500) + response.status(200).send() + } + } + } + def client = S3Client.builder() + .endpointOverride(server.address) + .region(Region.AP_NORTHEAST_1) + .credentialsProvider(CREDENTIALS_PROVIDER) + .httpClientBuilder(ApacheHttpClient.builder().socketTimeout(Duration.ofMillis(50))) + .build() + + when: + client.getObject(GetObjectRequest.builder().bucket("someBucket").key("someKey").build()) + + then: + thrown SdkClientException + + 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/someBucket/someKey" + "$Tags.HTTP_METHOD.key" "GET" + "$Tags.PEER_HOSTNAME.key" "localhost" + "$Tags.PEER_PORT.key" server.address.port + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT + "aws.service" "S3" + "aws.operation" "GetObject" + "aws.agent" "java-aws-sdk" + errorTags SdkClientException, "Unable to execute HTTP request: Read timed out" + 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 + errorTags SocketTimeoutException, "Read timed out" + defaultTags() + } + } + } + } + } + + cleanup: + server.close() + } }