diff --git a/dd-java-agent/instrumentation/aws-java/aws-java-common/src/main/java/datadog/trace/instrumentation/aws/AwsAccountIdentity.java b/dd-java-agent/instrumentation/aws-java/aws-java-common/src/main/java/datadog/trace/instrumentation/aws/AwsAccountIdentity.java new file mode 100644 index 00000000000..e6948d70acf --- /dev/null +++ b/dd-java-agent/instrumentation/aws-java/aws-java-common/src/main/java/datadog/trace/instrumentation/aws/AwsAccountIdentity.java @@ -0,0 +1,37 @@ +package datadog.trace.instrumentation.aws; + +/** + * Account identity of resources addressed by AWS SDK requests. + * + *

Two sources are trustworthy and used by the SDK instrumentations: + * + *

+ */ +public final class AwsAccountIdentity { + + private AwsAccountIdentity() {} + + /** {@code true} for exactly twelve ASCII digits. */ + public static boolean isAccountId(final String value) { + if (value == null || value.length() != 12) { + return false; + } + for (int i = 0; i < 12; i++) { + char c = value.charAt(i); + if (c < '0' || c > '9') { + return false; + } + } + return true; + } +} diff --git a/dd-java-agent/instrumentation/aws-java/aws-java-common/src/main/java/datadog/trace/instrumentation/aws/AwsArn.java b/dd-java-agent/instrumentation/aws-java/aws-java-common/src/main/java/datadog/trace/instrumentation/aws/AwsArn.java new file mode 100644 index 00000000000..abb6053d596 --- /dev/null +++ b/dd-java-agent/instrumentation/aws-java/aws-java-common/src/main/java/datadog/trace/instrumentation/aws/AwsArn.java @@ -0,0 +1,117 @@ +package datadog.trace.instrumentation.aws; + +/** + * A parsed Amazon Resource Name: {@code arn:::::}. + * + *

Parsed once so callers that need several fields do not re-scan the string. The AWS SDK ships + * its own parser ({@code software.amazon.awssdk.arns.Arn}) but it lives in a module that neither + * the SDK v2 2.2.0 floor these instrumentations compile against nor SDK v1 provide, so a minimal + * one is kept here. + */ +public final class AwsArn { + + private static final String TABLE_PREFIX = "table/"; + + private final String raw; + private final String partition; + private final String service; + private final String region; + private final String account; + private final String resource; + + private AwsArn( + final String raw, + final String partition, + final String service, + final String region, + final String account, + final String resource) { + this.raw = raw; + this.partition = partition; + this.service = service; + this.region = region; + this.account = account; + this.resource = resource; + } + + /** + * Parses an ARN. + * + * @return the parsed ARN, or {@code null} when the value is not of the form {@code + * arn:partition:service:region:account:resource} (partition and service non-empty). + */ + public static AwsArn parse(final String value) { + if (value == null || !value.startsWith("arn:")) { + return null; + } + int c1 = value.indexOf(':', 4); + if (c1 < 0) { + return null; + } + int c2 = value.indexOf(':', c1 + 1); + if (c2 < 0) { + return null; + } + int c3 = value.indexOf(':', c2 + 1); + if (c3 < 0) { + return null; + } + int c4 = value.indexOf(':', c3 + 1); + if (c4 < 0) { + return null; + } + if (c1 == 4 || c2 == c1 + 1 || c4 == value.length() - 1) { + // empty partition, empty service or empty resource + return null; + } + return new AwsArn( + value, + value.substring(4, c1), + value.substring(c1 + 1, c2), + emptyToNull(value.substring(c2 + 1, c3)), + emptyToNull(value.substring(c3 + 1, c4)), + value.substring(c4 + 1)); + } + + /** The original string. */ + public String raw() { + return raw; + } + + /** {@code aws}, {@code aws-cn}, {@code aws-us-gov}, ... */ + public String partition() { + return partition; + } + + /** {@code dynamodb}, {@code sns}, {@code s3}, ... */ + public String service() { + return service; + } + + /** The Region, or {@code null} for global services (S3, IAM). */ + public String region() { + return region; + } + + /** The 12-digit account, or {@code null} when absent (S3 ARNs) or not exactly twelve digits. */ + public String account() { + return AwsAccountIdentity.isAccountId(account) ? account : null; + } + + /** Everything after the fifth colon. Never empty. */ + public String resource() { + return resource; + } + + /** Whether this ARN identifies a DynamoDB table or one of its sub-resources. */ + public boolean isDynamoDbTable() { + return "dynamodb".equals(service) + && resource.startsWith(TABLE_PREFIX) + && resource.length() > TABLE_PREFIX.length() + && resource.charAt(TABLE_PREFIX.length()) != '/'; + } + + private static String emptyToNull(final String value) { + return value.isEmpty() ? null : value; + } +} diff --git a/dd-java-agent/instrumentation/aws-java/aws-java-common/src/test/java/datadog/trace/instrumentation/aws/AwsAccountIdentityTest.java b/dd-java-agent/instrumentation/aws-java/aws-java-common/src/test/java/datadog/trace/instrumentation/aws/AwsAccountIdentityTest.java new file mode 100644 index 00000000000..6f53271b95e --- /dev/null +++ b/dd-java-agent/instrumentation/aws-java/aws-java-common/src/test/java/datadog/trace/instrumentation/aws/AwsAccountIdentityTest.java @@ -0,0 +1,25 @@ +package datadog.trace.instrumentation.aws; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.NullAndEmptySource; +import org.junit.jupiter.params.provider.ValueSource; + +class AwsAccountIdentityTest { + + @ParameterizedTest(name = "{0} is an account id") + @ValueSource(strings = {"123456789012", "000000000000", "999999999999"}) + void acceptsTwelveDigits(String value) { + assertTrue(AwsAccountIdentity.isAccountId(value)); + } + + @ParameterizedTest(name = "{0} is not an account id") + @NullAndEmptySource + @ValueSource( + strings = {"12345", "1234567890123", "12345678901a", " 23456789012", "arn:aws:s3:::b"}) + void rejectsNonAccountIds(String value) { + assertFalse(AwsAccountIdentity.isAccountId(value)); + } +} diff --git a/dd-java-agent/instrumentation/aws-java/aws-java-common/src/test/java/datadog/trace/instrumentation/aws/AwsArnTest.java b/dd-java-agent/instrumentation/aws-java/aws-java-common/src/test/java/datadog/trace/instrumentation/aws/AwsArnTest.java new file mode 100644 index 00000000000..97ad122362c --- /dev/null +++ b/dd-java-agent/instrumentation/aws-java/aws-java-common/src/test/java/datadog/trace/instrumentation/aws/AwsArnTest.java @@ -0,0 +1,91 @@ +package datadog.trace.instrumentation.aws; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; + +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.NullAndEmptySource; +import org.junit.jupiter.params.provider.ValueSource; +import org.tabletest.junit.TableTest; + +class AwsArnTest { + + @ParameterizedTest(name = "parses {0}") + @CsvSource( + nullValues = "NULL", + value = { + "arn:aws:dynamodb:us-east-1:123456789012:table/orders, aws, dynamodb, us-east-1, 123456789012, table/orders", + "arn:aws:dynamodb:us-east-1:123456789012:table/orders/index/by-user, aws, dynamodb, us-east-1, 123456789012, table/orders/index/by-user", + "arn:aws-cn:dynamodb:cn-north-1:123456789012:table/orders, aws-cn, dynamodb, cn-north-1, 123456789012, table/orders", + "arn:aws-us-gov:sns:us-gov-west-1:123456789012:alerts, aws-us-gov, sns, us-gov-west-1, 123456789012, alerts", + "arn:aws:s3:::my-bucket, aws, s3, NULL, NULL, my-bucket", + "arn:aws:iam::123456789012:role/app, aws, iam, NULL, 123456789012, role/app", + "arn:aws:states:us-east-1:123456789012:stateMachine:orders:extra:colons, aws, states, us-east-1, 123456789012, stateMachine:orders:extra:colons", + }) + void parsesFields( + String value, + String partition, + String service, + String region, + String account, + String resource) { + AwsArn arn = AwsArn.parse(value); + assertNotNull(arn); + assertEquals(value, arn.raw()); + assertEquals(partition, arn.partition()); + assertEquals(service, arn.service()); + assertEquals(region, arn.region()); + assertEquals(account, arn.account()); + assertEquals(resource, arn.resource()); + } + + @ParameterizedTest(name = "rejects {0}") + @NullAndEmptySource + @ValueSource( + strings = { + "orders", + "arn:", + "arn:aws", + "arn:aws:dynamodb", + "arn:aws:dynamodb:us-east-1", + "arn:aws:dynamodb:us-east-1:123456789012", + "arn:aws:dynamodb:us-east-1:123456789012:", + "arn::dynamodb:us-east-1:123456789012:table/orders", + "arn:aws::us-east-1:123456789012:table/orders", + "table/orders", + "https://sqs.us-east-1.amazonaws.com/123456789012/q", + }) + void rejectsNonArn(String value) { + assertNull(AwsArn.parse(value)); + } + + @ParameterizedTest(name = "account of {0} is null") + @ValueSource( + strings = { + "arn:aws:dynamodb:us-east-1:12345:table/orders", + "arn:aws:dynamodb:us-east-1:12345678901a:table/orders", + "arn:aws:dynamodb:us-east-1::table/orders", + }) + void accountMustBeTwelveDigits(String value) { + AwsArn arn = AwsArn.parse(value); + assertNotNull(arn); + assertNull(arn.account()); + } + + @TableTest({ + "Scenario | Value | Expected", + "Table | arn:aws:dynamodb:us-east-1:123456789012:table/orders | true ", + "Index | arn:aws:dynamodb:us-east-1:123456789012:table/orders/index/by-user | true ", + "Stream | arn:aws:dynamodb:us-east-1:123456789012:table/orders/stream/2024-01-01T00:00:00.000 | true ", + "Missing name | arn:aws:dynamodb:us-east-1:123456789012:table/ | false ", + "Empty name | arn:aws:dynamodb:us-east-1:123456789012:table//index/by-user | false ", + "Non-table | arn:aws:dynamodb:us-east-1:123456789012:backup/orders | false ", + "Other service | arn:aws:sns:us-east-1:123456789012:table/orders | false ", + "S3 bucket | arn:aws:s3:::my-bucket | false " + }) + void identifiesDynamoDbTables(String value, boolean expected) { + assertEquals(expected, AwsArn.parse(value).isDynamoDbTable()); + } +} diff --git a/dd-java-agent/instrumentation/aws-java/aws-java-sdk-1.11/build.gradle b/dd-java-agent/instrumentation/aws-java/aws-java-sdk-1.11/build.gradle index e8173d6afe0..42c803ea271 100644 --- a/dd-java-agent/instrumentation/aws-java/aws-java-sdk-1.11/build.gradle +++ b/dd-java-agent/instrumentation/aws-java/aws-java-sdk-1.11/build.gradle @@ -37,7 +37,9 @@ addTestSuiteForDir('latestDsmTest', 'dsmTest') addTestSuiteExtendingForDir('latestDsmForkedTest', 'latestDsmTest', 'dsmTest') dependencies { + testImplementation libs.bundles.mockito compileOnly group: 'com.amazonaws', name: 'aws-java-sdk-core', version: '1.11.0' + implementation project(':dd-java-agent:instrumentation:aws-java:aws-java-common') // Include httpclient instrumentation for testing because it is a dependency for aws-sdk. testImplementation project(':dd-java-agent:instrumentation:apache-httpclient:apache-httpclient-4.0') diff --git a/dd-java-agent/instrumentation/aws-java/aws-java-sdk-1.11/src/main/java/datadog/trace/instrumentation/aws/v0/AwsSdkClientDecorator.java b/dd-java-agent/instrumentation/aws-java/aws-java-sdk-1.11/src/main/java/datadog/trace/instrumentation/aws/v0/AwsSdkClientDecorator.java index 07027e0f4f6..93f5d247e56 100644 --- a/dd-java-agent/instrumentation/aws-java/aws-java-sdk-1.11/src/main/java/datadog/trace/instrumentation/aws/v0/AwsSdkClientDecorator.java +++ b/dd-java-agent/instrumentation/aws-java/aws-java-sdk-1.11/src/main/java/datadog/trace/instrumentation/aws/v0/AwsSdkClientDecorator.java @@ -25,6 +25,8 @@ import datadog.trace.bootstrap.instrumentation.api.Tags; import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString; import datadog.trace.bootstrap.instrumentation.decorator.HttpClientDecorator; +import datadog.trace.instrumentation.aws.AwsAccountIdentity; +import datadog.trace.instrumentation.aws.AwsArn; import java.net.URI; import java.util.List; import java.util.Locale; @@ -179,6 +181,23 @@ protected void doOnRequest(final AgentSpan span, final Request request) { } String tableName = access.getTableName(originalRequest); if (null != tableName) { + // A DynamoDB TableName given as an ARN carries the owning account. SDK v1 does not expose + // the signing credentials to request handlers, so a bare name yields no account here. The + // existing table name tags keep the value as given, ARN or not. Other services with a + // TableName member (Timestream, Keyspaces, ...) only get the plain table name tags. + AwsArn arn = + awsSimplifiedServiceName != null && awsSimplifiedServiceName.startsWith("dynamodb") + ? AwsArn.parse(tableName) + : null; + if (arn != null) { + if (arn.isDynamoDbTable()) { + span.setTag(InstrumentationTags.AWS_TABLE_ARN, arn.raw()); + } + String account = arn.account(); + if (account != null) { + span.setTag(InstrumentationTags.AWS_ACCOUNT, account); + } + } span.setTag(InstrumentationTags.AWS_TABLE_NAME, tableName); span.setTag(InstrumentationTags.TABLE_NAME, tableName); bestPrecursor = InstrumentationTags.AWS_TABLE_NAME; @@ -247,6 +266,19 @@ protected void doOnRequest(final AgentSpan span, final Request request) { } } + /** Tags the expected bucket owner only after S3 has accepted the request. */ + public void onSuccessfulRequest(final AgentSpan span, final Request request) { + if (!"s3".equalsIgnoreCase(simplifyServiceName(request.getServiceName()))) { + return; + } + final AmazonWebServiceRequest originalRequest = request.getOriginalRequest(); + final String expectedBucketOwner = + GetterAccess.of(originalRequest).getExpectedBucketOwner(originalRequest); + if (AwsAccountIdentity.isAccountId(expectedBucketOwner)) { + span.setTag(InstrumentationTags.AWS_ACCOUNT, expectedBucketOwner); + } + } + public void onServiceResponse( final AgentSpan span, final String awsService, final Response response) { if ("s3".equalsIgnoreCase(simplifyServiceName(awsService)) diff --git a/dd-java-agent/instrumentation/aws-java/aws-java-sdk-1.11/src/main/java/datadog/trace/instrumentation/aws/v0/AwsSdkModule.java b/dd-java-agent/instrumentation/aws-java/aws-java-sdk-1.11/src/main/java/datadog/trace/instrumentation/aws/v0/AwsSdkModule.java index 85e69c37492..7dc3af4f7f8 100644 --- a/dd-java-agent/instrumentation/aws-java/aws-java-sdk-1.11/src/main/java/datadog/trace/instrumentation/aws/v0/AwsSdkModule.java +++ b/dd-java-agent/instrumentation/aws-java/aws-java-sdk-1.11/src/main/java/datadog/trace/instrumentation/aws/v0/AwsSdkModule.java @@ -21,6 +21,13 @@ protected AwsSdkModule(String namespace, String instrumentationName) { this.namespace = namespace; } + @Override + public boolean isHelperClass(String className) { + // These helpers are compiled in aws-java-common, outside this module's output. + return className.equals("datadog.trace.instrumentation.aws.AwsAccountIdentity") + || className.equals("datadog.trace.instrumentation.aws.AwsArn"); + } + @Override public Map contextStore() { Map map = new java.util.HashMap<>(); diff --git a/dd-java-agent/instrumentation/aws-java/aws-java-sdk-1.11/src/main/java/datadog/trace/instrumentation/aws/v0/GetterAccess.java b/dd-java-agent/instrumentation/aws-java/aws-java-sdk-1.11/src/main/java/datadog/trace/instrumentation/aws/v0/GetterAccess.java index ed87408c1bd..37e69fa18ca 100644 --- a/dd-java-agent/instrumentation/aws-java/aws-java-sdk-1.11/src/main/java/datadog/trace/instrumentation/aws/v0/GetterAccess.java +++ b/dd-java-agent/instrumentation/aws-java/aws-java-sdk-1.11/src/main/java/datadog/trace/instrumentation/aws/v0/GetterAccess.java @@ -39,6 +39,7 @@ static GetterAccess of(final Object request) { private final MethodHandle getPublishBatchRequestEntries; private final MethodHandle getApproximateArrivalTimestamp; private final MethodHandle getTableName; + private final MethodHandle getExpectedBucketOwner; private GetterAccess(final Class objectType) { operationName = @@ -55,6 +56,7 @@ private GetterAccess(final Class objectType) { getApproximateArrivalTimestamp = findGetter(objectType, "getApproximateArrivalTimestamp", Date.class); getTableName = findStringGetter(objectType, "getTableName"); + getExpectedBucketOwner = findStringGetter(objectType, "getExpectedBucketOwner"); } String getOperationNameFromType() { @@ -101,6 +103,10 @@ String getTableName(final Object object) { return invokeForString(getTableName, object); } + String getExpectedBucketOwner(final Object object) { + return invokeForString(getExpectedBucketOwner, object); + } + Date getApproximateArrivalTimestamp(final Object object) { return invoke(getApproximateArrivalTimestamp, object); } diff --git a/dd-java-agent/instrumentation/aws-java/aws-java-sdk-1.11/src/main/java/datadog/trace/instrumentation/aws/v0/TracingRequestHandler.java b/dd-java-agent/instrumentation/aws-java/aws-java-sdk-1.11/src/main/java/datadog/trace/instrumentation/aws/v0/TracingRequestHandler.java index 84e6f7ad142..afa65f9cd32 100644 --- a/dd-java-agent/instrumentation/aws-java/aws-java-sdk-1.11/src/main/java/datadog/trace/instrumentation/aws/v0/TracingRequestHandler.java +++ b/dd-java-agent/instrumentation/aws-java/aws-java-sdk-1.11/src/main/java/datadog/trace/instrumentation/aws/v0/TracingRequestHandler.java @@ -96,6 +96,7 @@ public void afterResponse(final Request request, final Response response) span = AgentSpan.fromContext(context); if (span != null) { DECORATE.onResponse(span, response); + DECORATE.onSuccessfulRequest(span, request); DECORATE.onServiceResponse(span, request.getServiceName(), response); DECORATE.beforeFinish(span); span.finish(); diff --git a/dd-java-agent/instrumentation/aws-java/aws-java-sdk-1.11/src/test/groovy/AWS1ClientTest.groovy b/dd-java-agent/instrumentation/aws-java/aws-java-sdk-1.11/src/test/groovy/AWS1ClientTest.groovy index c5ea6ca23cf..2a574b898f3 100644 --- a/dd-java-agent/instrumentation/aws-java/aws-java-sdk-1.11/src/test/groovy/AWS1ClientTest.groovy +++ b/dd-java-agent/instrumentation/aws-java/aws-java-sdk-1.11/src/test/groovy/AWS1ClientTest.groovy @@ -210,6 +210,7 @@ abstract class AWS1ClientTest extends VersionedNamingTestBase { "S3" | "GetObject" | "GET" | "/someBucket/someKey" | AmazonS3ClientBuilder.standard().withPathStyleAccessEnabled(true).withEndpointConfiguration(endpoint).withCredentials(credentialsProvider).build() | { c -> c.getObject("someBucket", "someKey") } | ["aws.bucket.name": "someBucket", "bucketname": "someBucket", "aws.object.key": "someKey"] | "" | "aws.bucket.name" | null "DynamoDBv2" | "CreateTable" | "POST" | "/" | AmazonDynamoDBClientBuilder.standard().withEndpointConfiguration(endpoint).withCredentials(credentialsProvider).build() | { c -> c.createTable(new CreateTableRequest("sometable", null)) } | ["aws.table.name": "sometable", "tablename": "sometable"] | "" | "aws.table.name" | null "DynamoDBv2" | "GetItem" | "POST" | "/" | AmazonDynamoDBClientBuilder.standard().withEndpointConfiguration(endpoint).withCredentials(credentialsProvider).build() | { c -> c.getItem(new GetItemRequest("sometable", ["attribute": new AttributeValue("somevalue")])) } | ["aws.table.name": "sometable", "tablename": "sometable"] | "" | "aws.table.name" | null + "DynamoDBv2" | "GetItem" | "POST" | "/" | AmazonDynamoDBClientBuilder.standard().withEndpointConfiguration(endpoint).withCredentials(credentialsProvider).build() | { c -> c.getItem(new GetItemRequest("arn:aws:dynamodb:us-east-1:123456789012:table/sometable", ["attribute": new AttributeValue("somevalue")])) } | ["aws.table.name": "arn:aws:dynamodb:us-east-1:123456789012:table/sometable", "tablename": "arn:aws:dynamodb:us-east-1:123456789012:table/sometable", "aws.table.arn": "arn:aws:dynamodb:us-east-1:123456789012:table/sometable", "aws_account": "123456789012"] | "" | "aws.table.name" | null "Kinesis" | "DeleteStream" | "POST" | "/" | AmazonKinesisClientBuilder.standard().withEndpointConfiguration(endpoint).withCredentials(credentialsProvider).build() | { c -> c.deleteStream(new DeleteStreamRequest().withStreamName("somestream")) } | ["aws.stream.name": "somestream", "streamname": "somestream"] | "" | "aws.stream.name" | null "SQS" | "CreateQueue" | "POST" | "/" | AmazonSQSClientBuilder.standard().withEndpointConfiguration(endpoint).withCredentials(credentialsProvider).build() | { c -> c.createQueue(new CreateQueueRequest("somequeue")) } | ["aws.queue.name": "somequeue", "queuename": "somequeue"] | """ diff --git a/dd-java-agent/instrumentation/aws-java/aws-java-sdk-1.11/src/test/java/datadog/trace/instrumentation/aws/v0/AwsSdkClientDecoratorTest.java b/dd-java-agent/instrumentation/aws-java/aws-java-sdk-1.11/src/test/java/datadog/trace/instrumentation/aws/v0/AwsSdkClientDecoratorTest.java new file mode 100644 index 00000000000..8dfdf855853 --- /dev/null +++ b/dd-java-agent/instrumentation/aws-java/aws-java-sdk-1.11/src/test/java/datadog/trace/instrumentation/aws/v0/AwsSdkClientDecoratorTest.java @@ -0,0 +1,77 @@ +package datadog.trace.instrumentation.aws.v0; + +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.RETURNS_SELF; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.amazonaws.DefaultRequest; +import com.amazonaws.http.HttpMethodName; +import com.amazonaws.services.dynamodbv2.model.GetItemRequest; +import datadog.trace.api.TraceConfig; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import java.net.URI; +import org.tabletest.junit.TableTest; + +class AwsSdkClientDecoratorTest { + + @TableTest({ + "Scenario | Service | Enrich", + "DynamoDB | AmazonDynamoDB | true ", + "DynamoDB v2 | AmazonDynamoDBv2 | true ", + "Other table model | AmazonTimestreamWrite | false ", + "Athena | AmazonAthena | false " + }) + void onlyDynamoDbTablesReceiveOwnership(String service, boolean enrich) { + AgentSpan span = mock(AgentSpan.class, RETURNS_SELF); + when(span.traceConfig()).thenReturn(mock(TraceConfig.class)); + String tableArn = "arn:aws:dynamodb:us-east-1:123456789012:table/orders"; + // Reuse a TableName-bearing model to verify that the service controls enrichment. + DefaultRequest request = + new DefaultRequest<>(new GetItemRequest().withTableName(tableArn), service); + request.setEndpoint(URI.create("http://localhost")); + request.setHttpMethod(HttpMethodName.POST); + + AwsSdkClientDecorator.DECORATE.onRequest(span, request); + + verify(span).setTag("aws.table.name", tableArn); + verify(span).setTag("tablename", tableArn); + if (enrich) { + verify(span).setTag("aws_account", "123456789012"); + verify(span).setTag("aws.table.arn", tableArn); + } else { + verify(span, never()).setTag(eq("aws_account"), anyString()); + verify(span, never()).setTag(eq("aws.table.arn"), anyString()); + } + } + + @TableTest({ + "Scenario | Table Name | Table Arn", + "Table | arn:aws:dynamodb:us-east-1:123456789012:table/orders | true ", + "Non-table | arn:aws:dynamodb:us-east-1:123456789012:backup/orders | false ", + "Empty name | arn:aws:dynamodb:us-east-1:123456789012:table/ | false ", + "SNS ARN | arn:aws:sns:us-east-1:123456789012:table/orders | false ", + "S3 ARN | arn:aws:s3:::my-bucket | false " + }) + void preservesTableNamesAndOnlyTagsTableArns(String tableName, boolean tableArn) { + AgentSpan span = mock(AgentSpan.class, RETURNS_SELF); + when(span.traceConfig()).thenReturn(mock(TraceConfig.class)); + DefaultRequest request = + new DefaultRequest<>(new GetItemRequest().withTableName(tableName), "AmazonDynamoDBv2"); + request.setEndpoint(URI.create("http://localhost")); + request.setHttpMethod(HttpMethodName.POST); + + AwsSdkClientDecorator.DECORATE.onRequest(span, request); + + verify(span).setTag("aws.table.name", tableName); + verify(span).setTag("tablename", tableName); + if (tableArn) { + verify(span).setTag("aws.table.arn", tableName); + } else { + verify(span, never()).setTag(eq("aws.table.arn"), anyString()); + } + } +} diff --git a/dd-java-agent/instrumentation/aws-java/aws-java-sdk-1.11/src/test/java/datadog/trace/instrumentation/aws/v0/S3BucketOwnerTest.java b/dd-java-agent/instrumentation/aws-java/aws-java-sdk-1.11/src/test/java/datadog/trace/instrumentation/aws/v0/S3BucketOwnerTest.java new file mode 100644 index 00000000000..c8e9ad617f1 --- /dev/null +++ b/dd-java-agent/instrumentation/aws-java/aws-java-sdk-1.11/src/test/java/datadog/trace/instrumentation/aws/v0/S3BucketOwnerTest.java @@ -0,0 +1,83 @@ +package datadog.trace.instrumentation.aws.v0; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.RETURNS_SELF; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.amazonaws.AmazonWebServiceRequest; +import com.amazonaws.DefaultRequest; +import com.amazonaws.Response; +import com.amazonaws.http.HttpMethodName; +import com.amazonaws.http.HttpResponse; +import datadog.context.Context; +import datadog.trace.api.TraceConfig; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import java.io.IOException; +import java.net.URI; +import org.tabletest.junit.TableTest; + +public class S3BucketOwnerTest { + + @TableTest({ + "Scenario | Owner | Status | Expected owner", + "Success | 123456789012 | 200 | 123456789012 ", + "Owner rejected | 999999999999 | 403 | ", + "Server failure | 123456789012 | 500 | ", + "Transport failure | 123456789012 | 0 | ", + "Malformed owner | invalid | 200 | ", + "Missing owner | | 200 | " + }) + void tagsOwnerOnlyAfterSuccess(String owner, int status, String expectedOwner) { + AgentSpan span = mock(AgentSpan.class, RETURNS_SELF); + when(span.traceConfig()).thenReturn(mock(TraceConfig.class)); + Context context = mock(Context.class); + when(context.get(any())).thenReturn(span); + DefaultRequest request = + new DefaultRequest<>(new BucketRequest(owner), "Amazon S3"); + request.setEndpoint(URI.create("http://localhost")); + request.setHttpMethod(HttpMethodName.GET); + request.addHandlerContext(TracingRequestHandler.CONTEXT_CONTEXT_KEY, context); + + AwsSdkClientDecorator.DECORATE.onRequest(span, request); + verify(span, never()).setTag(eq("aws_account"), anyString()); + + HttpResponse httpResponse = mock(HttpResponse.class); + when(httpResponse.getStatusCode()).thenReturn(status); + Response response = new Response<>(new Object(), httpResponse); + TracingRequestHandler handler = new TracingRequestHandler(null, null); + if (status == 200) { + handler.afterResponse(request, response); + } else { + handler.afterError(request, status == 0 ? null : response, new IOException("request failed")); + } + + if (expectedOwner == null) { + verify(span, never()).setTag(eq("aws_account"), anyString()); + } else { + verify(span).setTag("aws_account", expectedOwner); + } + verify(span).finish(); + } + + /** Exposes the newer request getter while compiling against the SDK 1.11 baseline. */ + public static class BucketRequest extends AmazonWebServiceRequest { + private final String owner; + + public BucketRequest(String owner) { + this.owner = owner; + } + + public String getExpectedBucketOwner() { + return owner; + } + + public String getBucketName() { + return "somebucket"; + } + } +} diff --git a/dd-java-agent/instrumentation/aws-java/aws-java-sdk-2.2/build.gradle b/dd-java-agent/instrumentation/aws-java/aws-java-sdk-2.2/build.gradle index cfdc266d8ec..39e248d88f8 100644 --- a/dd-java-agent/instrumentation/aws-java/aws-java-sdk-2.2/build.gradle +++ b/dd-java-agent/instrumentation/aws-java/aws-java-sdk-2.2/build.gradle @@ -34,9 +34,10 @@ configurations.named('latestPayloadTaggingTestContainerImage') { def fixedSdkVersion = '2.20.33' // 2.20.34 is missing and breaks IDEA import dependencies { + testImplementation libs.bundles.mockito compileOnly group: 'software.amazon.awssdk', name: 'aws-core', version: '2.2.0' payloadTaggingTestContainerImage(image('localstack/localstack:4.2.0', 'test.localstack.image')) - testImplementation project(':dd-java-agent:instrumentation:aws-java:aws-java-common') + implementation project(':dd-java-agent:instrumentation:aws-java:aws-java-common') // Include httpclient instrumentation for testing because it is a dependency for aws-sdk. testImplementation project(':dd-java-agent:instrumentation:apache-httpclient:apache-httpclient-4.0') diff --git a/dd-java-agent/instrumentation/aws-java/aws-java-sdk-2.2/src/main/java/datadog/trace/instrumentation/aws/v2/AwsSdkClientDecorator.java b/dd-java-agent/instrumentation/aws-java/aws-java-sdk-2.2/src/main/java/datadog/trace/instrumentation/aws/v2/AwsSdkClientDecorator.java index 7c1a1a85b90..ce50b771d55 100644 --- a/dd-java-agent/instrumentation/aws-java/aws-java-sdk-2.2/src/main/java/datadog/trace/instrumentation/aws/v2/AwsSdkClientDecorator.java +++ b/dd-java-agent/instrumentation/aws-java/aws-java-sdk-2.2/src/main/java/datadog/trace/instrumentation/aws/v2/AwsSdkClientDecorator.java @@ -14,6 +14,7 @@ import datadog.trace.api.Config; import datadog.trace.api.ConfigDefaults; import datadog.trace.api.DDTags; +import datadog.trace.api.GenericClassValue; import datadog.trace.api.cache.DDCache; import datadog.trace.api.cache.DDCaches; import datadog.trace.api.datastreams.AgentDataStreamsMonitoring; @@ -28,7 +29,12 @@ import datadog.trace.bootstrap.instrumentation.api.Tags; import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString; import datadog.trace.bootstrap.instrumentation.decorator.HttpClientDecorator; +import datadog.trace.instrumentation.aws.AwsAccountIdentity; +import datadog.trace.instrumentation.aws.AwsArn; import datadog.trace.payloadtags.PayloadTagsData; +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.MethodType; import java.net.URI; import java.time.Instant; import java.util.ArrayList; @@ -41,6 +47,8 @@ import java.util.Set; import javax.annotation.Nonnull; import javax.annotation.ParametersAreNonnullByDefault; +import software.amazon.awssdk.auth.credentials.AwsCredentials; +import software.amazon.awssdk.auth.signer.AwsSignerExecutionAttribute; import software.amazon.awssdk.awscore.AwsResponse; import software.amazon.awssdk.core.SdkBytes; import software.amazon.awssdk.core.SdkField; @@ -186,7 +194,16 @@ public void onSdkRequest( }); // DynamoDB - request.getValueForField("TableName", String.class).ifPresent(name -> setTableName(span, name)); + request + .getValueForField("TableName", String.class) + .ifPresent( + name -> { + if ("dynamodb".equalsIgnoreCase(awsServiceName)) { + onDynamoDbTable(span, name, attributes); + } else { + setTableName(span, name); + } + }); // DSM if (traceConfig().isDataStreamsEnabled()) { @@ -274,6 +291,90 @@ private static void setPeerService( } } + /** + * Tags the owning account of the table. A TableName given as an ARN carries it (and is tagged as + * aws.table.arn). A bare name is, by DynamoDB's documented contract, resolved in the requestor's + * own account, so the account owning the signing credentials is the table owner. The existing + * aws.table.name, tablename and peer.service tags keep the TableName value as given, ARN or not, + * so nothing changes for spans that already carry an ARN there. + */ + private static void onDynamoDbTable( + final AgentSpan span, final String tableName, final ExecutionAttributes attributes) { + setTableName(span, tableName); + AwsArn arn = AwsArn.parse(tableName); + String account; + if (arn != null) { + account = arn.account(); + if (arn.isDynamoDbTable()) { + span.setTag(InstrumentationTags.AWS_TABLE_ARN, arn.raw()); + } + } else { + account = callerAccount(attributes); + } + if (account != null) { + span.setTag(InstrumentationTags.AWS_ACCOUNT, account); + } + } + + /** + * Account owning the credentials that sign this request, when the SDK exposes it + * (AwsCredentialsIdentity.accountId(), SDK 2.26+, populated by the STS, SSO, profile, process and + * container providers). Absent on older SDKs and for providers that do not resolve it. + */ + private static String callerAccount(final ExecutionAttributes attributes) { + AwsCredentials credentials = + attributes.getAttribute(AwsSignerExecutionAttribute.AWS_CREDENTIALS); + return credentials == null ? null : credentialsAccountId(credentials); + } + + // accountId() is absent at the 2.2.0 floor, so look it up per credentials class. ClassValue + // lets application classes unload even though each cached MethodHandle references its class. + private static final MethodHandle NO_ACCOUNT_ID_GETTER = + MethodHandles.constant(Optional.class, Optional.empty()); + private static final ClassValue ACCOUNT_ID_GETTERS = + GenericClassValue.of(AwsSdkClientDecorator::accountIdGetter); + + private static MethodHandle accountIdGetter(final Class type) { + try { + return MethodHandles.publicLookup() + .findVirtual(type, "accountId", MethodType.methodType(Optional.class)); + } catch (NoSuchMethodException | IllegalAccessException e) { + return NO_ACCOUNT_ID_GETTER; + } + } + + static String credentialsAccountId(final AwsCredentials credentials) { + try { + MethodHandle getter = ACCOUNT_ID_GETTERS.get(credentials.getClass()); + if (getter == NO_ACCOUNT_ID_GETTER) { + return null; + } + Object value = getter.invoke(credentials); + if (value instanceof Optional) { + Object account = ((Optional) value).orElse(null); + if (account instanceof String && AwsAccountIdentity.isAccountId((String) account)) { + return (String) account; + } + } + } catch (VirtualMachineError | ThreadDeath fatal) { + throw fatal; + } catch (Throwable ignored) { + // Lookup or provider failures, including LinkageError, mean the account is unavailable. + } + return null; + } + + /** Tags the expected bucket owner only after S3 has accepted the request. */ + public void onSdkRequestSuccess( + final AgentSpan span, final SdkRequest request, final ExecutionAttributes attributes) { + if ("s3".equalsIgnoreCase(attributes.getAttribute(SdkExecutionAttribute.SERVICE_NAME))) { + request + .getValueForField("ExpectedBucketOwner", String.class) + .filter(AwsAccountIdentity::isAccountId) + .ifPresent(owner -> span.setTag(InstrumentationTags.AWS_ACCOUNT, owner)); + } + } + private static void setBucketName(AgentSpan span, String name) { span.setTag(InstrumentationTags.AWS_BUCKET_NAME, name); span.setTag(InstrumentationTags.BUCKET_NAME, name); diff --git a/dd-java-agent/instrumentation/aws-java/aws-java-sdk-2.2/src/main/java/datadog/trace/instrumentation/aws/v2/AwsSdkModule.java b/dd-java-agent/instrumentation/aws-java/aws-java-sdk-2.2/src/main/java/datadog/trace/instrumentation/aws/v2/AwsSdkModule.java index d2202c1f730..82a183aa889 100644 --- a/dd-java-agent/instrumentation/aws-java/aws-java-sdk-2.2/src/main/java/datadog/trace/instrumentation/aws/v2/AwsSdkModule.java +++ b/dd-java-agent/instrumentation/aws-java/aws-java-sdk-2.2/src/main/java/datadog/trace/instrumentation/aws/v2/AwsSdkModule.java @@ -16,6 +16,13 @@ public AwsSdkModule() { super("aws-sdk"); } + @Override + public boolean isHelperClass(String className) { + // These helpers are compiled in aws-java-common, outside this module's output. + return className.equals("datadog.trace.instrumentation.aws.AwsAccountIdentity") + || className.equals("datadog.trace.instrumentation.aws.AwsArn"); + } + @Override public Map contextStore() { return Collections.singletonMap( diff --git a/dd-java-agent/instrumentation/aws-java/aws-java-sdk-2.2/src/main/java/datadog/trace/instrumentation/aws/v2/TracingExecutionInterceptor.java b/dd-java-agent/instrumentation/aws-java/aws-java-sdk-2.2/src/main/java/datadog/trace/instrumentation/aws/v2/TracingExecutionInterceptor.java index 366c67675e3..a9a551c64e4 100644 --- a/dd-java-agent/instrumentation/aws-java/aws-java-sdk-2.2/src/main/java/datadog/trace/instrumentation/aws/v2/TracingExecutionInterceptor.java +++ b/dd-java-agent/instrumentation/aws-java/aws-java-sdk-2.2/src/main/java/datadog/trace/instrumentation/aws/v2/TracingExecutionInterceptor.java @@ -135,6 +135,7 @@ public void afterExecution( final AgentSpan span = fromContext(ddContext); if (span != null) { executionAttributes.putAttribute(CONTEXT_ATTRIBUTE, null); + DECORATE.onSdkRequestSuccess(span, context.request(), executionAttributes); // Call onResponse on both types of responses: DECORATE.onSdkResponse(span, context.response(), context.httpResponse(), executionAttributes); DECORATE.onResponse(span, context.httpResponse()); diff --git a/dd-java-agent/instrumentation/aws-java/aws-java-sdk-2.2/src/payloadTaggingTest/java/datadog/trace/instrumentation/aws/v2/S3BucketOwnerForkedTest.java b/dd-java-agent/instrumentation/aws-java/aws-java-sdk-2.2/src/payloadTaggingTest/java/datadog/trace/instrumentation/aws/v2/S3BucketOwnerForkedTest.java new file mode 100644 index 00000000000..b1bad3498d7 --- /dev/null +++ b/dd-java-agent/instrumentation/aws-java/aws-java-sdk-2.2/src/payloadTaggingTest/java/datadog/trace/instrumentation/aws/v2/S3BucketOwnerForkedTest.java @@ -0,0 +1,121 @@ +package datadog.trace.instrumentation.aws.v2; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import com.sun.net.httpserver.HttpServer; +import datadog.trace.agent.test.AbstractInstrumentationTest; +import datadog.trace.core.DDSpan; +import java.io.IOException; +import java.net.InetSocketAddress; +import java.net.URI; +import java.util.List; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import software.amazon.awssdk.auth.credentials.AwsBasicCredentials; +import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider; +import software.amazon.awssdk.regions.Region; +import software.amazon.awssdk.services.s3.S3Client; +import software.amazon.awssdk.services.s3.S3Configuration; +import software.amazon.awssdk.services.s3.model.GetObjectRequest; +import software.amazon.awssdk.services.s3.model.S3Exception; + +/** + * Lives in this source set because its S3 model (2.18.40) has {@code ExpectedBucketOwner}; the base + * test suite pins s3 2.2.0 which predates the field. + */ +class S3BucketOwnerForkedTest extends AbstractInstrumentationTest { + + private static HttpServer server; + private static S3Client client; + + @BeforeAll + static void startServer() throws IOException { + server = HttpServer.create(new InetSocketAddress("localhost", 0), 0); + server.createContext( + "/", + exchange -> { + int status = exchange.getRequestURI().getPath().endsWith("/rejected") ? 403 : 200; + exchange.sendResponseHeaders(status, -1); + exchange.close(); + }); + server.start(); + client = + S3Client.builder() + .endpointOverride(URI.create("http://localhost:" + server.getAddress().getPort())) + // Keep the bucket in the path to avoid resolving somebucket.localhost. + .serviceConfiguration(S3Configuration.builder().pathStyleAccessEnabled(true).build()) + .region(Region.US_EAST_1) + .credentialsProvider( + StaticCredentialsProvider.create(AwsBasicCredentials.create("test", "test"))) + .build(); + } + + @AfterAll + static void stopServer() { + client.close(); + server.stop(0); + } + + @Test + void expectedBucketOwnerTagsTheOwningAccount() throws Exception { + client.getObject( + GetObjectRequest.builder() + .bucket("somebucket") + .key("somekey") + .expectedBucketOwner("123456789012") + .build()); + + DDSpan span = firstSpan(); + assertEquals("somebucket", span.getTag("aws.bucket.name")); + assertEquals("123456789012", span.getTag("aws_account")); + } + + @Test + void noExpectedBucketOwnerMeansNoAccountTag() throws Exception { + client.getObject(GetObjectRequest.builder().bucket("somebucket").key("somekey").build()); + + DDSpan span = firstSpan(); + assertEquals("somebucket", span.getTag("aws.bucket.name")); + assertFalse(span.getTags().containsKey("aws_account")); + } + + @Test + void malformedExpectedBucketOwnerIsIgnored() throws Exception { + client.getObject( + GetObjectRequest.builder() + .bucket("somebucket") + .key("somekey") + .expectedBucketOwner("not-an-account") + .build()); + + assertFalse(firstSpan().getTags().containsKey("aws_account")); + } + + @Test + void rejectedExpectedBucketOwnerIsNotTagged() throws Exception { + S3Exception error = + assertThrows( + S3Exception.class, + () -> + client.getObject( + GetObjectRequest.builder() + .bucket("somebucket") + .key("rejected") + .expectedBucketOwner("999999999999") + .build())); + + assertEquals(403, error.statusCode()); + DDSpan span = firstSpan(); + assertEquals("somebucket", span.getTag("aws.bucket.name")); + assertFalse(span.getTags().containsKey("aws_account")); + } + + private static DDSpan firstSpan() throws Exception { + writer.waitForTraces(1); + List trace = writer.firstTrace(); + return trace.get(0); + } +} diff --git a/dd-java-agent/instrumentation/aws-java/aws-java-sdk-2.2/src/test/groovy/Aws2ClientTest.groovy b/dd-java-agent/instrumentation/aws-java/aws-java-sdk-2.2/src/test/groovy/Aws2ClientTest.groovy index aa7ecf0cd24..7723aeec1c3 100644 --- a/dd-java-agent/instrumentation/aws-java/aws-java-sdk-2.2/src/test/groovy/Aws2ClientTest.groovy +++ b/dd-java-agent/instrumentation/aws-java/aws-java-sdk-2.2/src/test/groovy/Aws2ClientTest.groovy @@ -433,6 +433,78 @@ abstract class Aws2ClientTest extends VersionedNamingTestBase { """ } + def "DynamoDb request with a table ARN tags the owning account and table ARN"() { + setup: + def client = DynamoDbClient.builder() + .endpointOverride(server.address) + .region(Region.AP_NORTHEAST_1) + .credentialsProvider(CREDENTIALS_PROVIDER) + .build() + responseBody.set("") + def tableArn = "arn:aws:dynamodb:ap-northeast-1:123456789012:table/sometable" + + when: + client.getItem(GetItemRequest.builder().tableName(tableArn).key(["attribute": AttributeValue.builder().s("somevalue").build()]).build()) + TEST_WRITER.waitForTraces(1) + + then: + assertTraces(1) { + trace(1) { + span { + serviceName expectedService("DynamoDb", "GetItem") + operationName expectedOperation("DynamoDb", "GetItem") + resourceName "DynamoDb.GetItem" + spanType DDSpanTypes.HTTP_CLIENT + errored false + measured true + parent() + tags { + "$Tags.COMPONENT" "java-aws-sdk" + "$Tags.SPAN_KIND" Tags.SPAN_KIND_CLIENT + "$Tags.PEER_HOSTNAME" "localhost" + "$Tags.PEER_PORT" server.address.port + "$Tags.HTTP_METHOD" "POST" + "$Tags.HTTP_STATUS" 200 + "aws.service" "DynamoDb" + "aws_service" "DynamoDb" + "aws.operation" "GetItem" + "aws.agent" "java-aws-sdk" + "aws.requestId" "UNKNOWN" + // the name tags (and so peer.service) keep the TableName as given; the ARN and + // account are additions, not replacements + "aws.table.name" tableArn + "tablename" tableArn + "aws.table.arn" tableArn + "aws_account" "123456789012" + peerServiceFrom("aws.table.name") + urlTags("${server.address}/", ExpectedQueryParams.getExpectedQueryParams("GetItem")) + defaultTags(false, true) + } + } + } + } + } + + def "DynamoDb request with a bare table name has no account when the credentials carry none"() { + setup: + def client = DynamoDbClient.builder() + .endpointOverride(server.address) + .region(Region.AP_NORTHEAST_1) + .credentialsProvider(CREDENTIALS_PROVIDER) + .build() + responseBody.set("") + + when: + client.getItem(GetItemRequest.builder().tableName("sometable").key(["attribute": AttributeValue.builder().s("somevalue").build()]).build()) + TEST_WRITER.waitForTraces(1) + + then: + def tags = TEST_WRITER.firstTrace().first().tags + tags["aws.table.name"] == "sometable" + !tags.containsKey("aws_account") + !tags.containsKey("aws.table.arn") + } + def "timeout and retry errors captured"() { setup: def server = httpServer { diff --git a/dd-java-agent/instrumentation/aws-java/aws-java-sdk-2.2/src/test/java/datadog/trace/instrumentation/aws/v2/AwsSdkClientDecoratorCredentialsTest.java b/dd-java-agent/instrumentation/aws-java/aws-java-sdk-2.2/src/test/java/datadog/trace/instrumentation/aws/v2/AwsSdkClientDecoratorCredentialsTest.java new file mode 100644 index 00000000000..83e583b09cc --- /dev/null +++ b/dd-java-agent/instrumentation/aws-java/aws-java-sdk-2.2/src/test/java/datadog/trace/instrumentation/aws/v2/AwsSdkClientDecoratorCredentialsTest.java @@ -0,0 +1,150 @@ +package datadog.trace.instrumentation.aws.v2; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import datadog.trace.agent.test.AbstractInstrumentationTest; +import java.util.Optional; +import org.junit.jupiter.api.Test; +import software.amazon.awssdk.auth.credentials.AwsBasicCredentials; +import software.amazon.awssdk.auth.credentials.AwsCredentials; + +/** + * {@code AwsCredentialsIdentity.accountId()} does not exist in the SDK this suite compiles against, + * so the lookup is exercised with credentials classes that declare the same method. + */ +public class AwsSdkClientDecoratorCredentialsTest extends AbstractInstrumentationTest { + + @Test + void readsTheAccountFromCredentialsThatExposeIt() { + assertEquals( + "123456789012", + AwsSdkClientDecorator.credentialsAccountId(new AccountCredentials("123456789012"))); + } + + @Test + void ignoresCredentialsWithoutAnAccountIdMethod() { + // AwsBasicCredentials at 2.2.0 predates accountId() + assertNull( + AwsSdkClientDecorator.credentialsAccountId(AwsBasicCredentials.create("key", "secret"))); + } + + @Test + void ignoresAnEmptyOrMalformedAccount() { + assertNull(AwsSdkClientDecorator.credentialsAccountId(new AccountCredentials(null))); + assertNull(AwsSdkClientDecorator.credentialsAccountId(new AccountCredentials("12345"))); + assertNull( + AwsSdkClientDecorator.credentialsAccountId(new AccountCredentials("arn:aws:iam::1:root"))); + } + + @Test + void treatsAFailingLookupAsAbsent() { + assertNull(AwsSdkClientDecorator.credentialsAccountId(new FailingCredentials())); + } + + @Test + void doesNotSwallowFatalErrors() { + OutOfMemoryError error = + assertThrows( + OutOfMemoryError.class, + () -> AwsSdkClientDecorator.credentialsAccountId(new ExhaustedCredentials())); + assertSame(ExhaustedCredentials.ERROR, error); + } + + @Test + void doesNotSwallowThreadDeath() { + ThreadDeath error = new ThreadDeath(); + assertSame( + error, + assertThrows( + ThreadDeath.class, + () -> AwsSdkClientDecorator.credentialsAccountId(new ErrorCredentials(error)))); + } + + @Test + void treatsLinkageErrorsAsAbsent() { + assertNull( + AwsSdkClientDecorator.credentialsAccountId( + new ErrorCredentials(new NoClassDefFoundError("missing provider dependency")))); + } + + public static final class ErrorCredentials implements AwsCredentials { + private final Error error; + + ErrorCredentials(Error error) { + this.error = error; + } + + public Optional accountId() { + throw error; + } + + @Override + public String accessKeyId() { + return "key"; + } + + @Override + public String secretAccessKey() { + return "secret"; + } + } + + public static final class AccountCredentials implements AwsCredentials { + private final String account; + + AccountCredentials(String account) { + this.account = account; + } + + public Optional accountId() { + return Optional.ofNullable(account); + } + + @Override + public String accessKeyId() { + return "key"; + } + + @Override + public String secretAccessKey() { + return "secret"; + } + } + + public static final class FailingCredentials implements AwsCredentials { + public Optional accountId() { + throw new IllegalStateException("provider could not resolve the account"); + } + + @Override + public String accessKeyId() { + return "key"; + } + + @Override + public String secretAccessKey() { + return "secret"; + } + } + + public static final class ExhaustedCredentials implements AwsCredentials { + static final OutOfMemoryError ERROR = new OutOfMemoryError("simulated"); + + public Optional accountId() { + throw ERROR; + } + + @Override + public String accessKeyId() { + return "key"; + } + + @Override + public String secretAccessKey() { + return "secret"; + } + } +} diff --git a/dd-java-agent/instrumentation/aws-java/aws-java-sdk-2.2/src/test/java/datadog/trace/instrumentation/aws/v2/AwsSdkClientDecoratorTest.java b/dd-java-agent/instrumentation/aws-java/aws-java-sdk-2.2/src/test/java/datadog/trace/instrumentation/aws/v2/AwsSdkClientDecoratorTest.java new file mode 100644 index 00000000000..c876e1561c9 --- /dev/null +++ b/dd-java-agent/instrumentation/aws-java/aws-java-sdk-2.2/src/test/java/datadog/trace/instrumentation/aws/v2/AwsSdkClientDecoratorTest.java @@ -0,0 +1,104 @@ +package datadog.trace.instrumentation.aws.v2; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.RETURNS_SELF; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import datadog.context.Context; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import java.util.Optional; +import org.tabletest.junit.TableTest; +import software.amazon.awssdk.auth.credentials.AwsCredentials; +import software.amazon.awssdk.auth.signer.AwsSignerExecutionAttribute; +import software.amazon.awssdk.awscore.AwsExecutionAttribute; +import software.amazon.awssdk.core.SdkRequest; +import software.amazon.awssdk.core.interceptor.ExecutionAttributes; +import software.amazon.awssdk.core.interceptor.SdkExecutionAttribute; +import software.amazon.awssdk.http.SdkHttpRequest; +import software.amazon.awssdk.regions.Region; + +public class AwsSdkClientDecoratorTest { + + @TableTest({ + "Scenario | Service | Enrich", + "DynamoDB | DynamoDb | true ", + "Timestream | TimestreamWrite | false ", + "Athena | Athena | false " + }) + void onlyDynamoDbTablesReceiveOwnership(String service, boolean enrich) { + AgentSpan span = mock(AgentSpan.class, RETURNS_SELF); + Context context = mock(Context.class); + when(context.get(any())).thenReturn(span); + SdkRequest request = mock(SdkRequest.class); + when(request.getValueForField("TableName", String.class)).thenReturn(Optional.of("orders")); + ExecutionAttributes attributes = new ExecutionAttributes(); + attributes.putAttribute(SdkExecutionAttribute.SERVICE_NAME, service); + attributes.putAttribute(SdkExecutionAttribute.OPERATION_NAME, "WriteRecords"); + attributes.putAttribute(AwsExecutionAttribute.AWS_REGION, Region.US_EAST_1); + attributes.putAttribute(AwsSignerExecutionAttribute.AWS_CREDENTIALS, new AccountCredentials()); + + AwsSdkClientDecorator.DECORATE.onSdkRequest( + context, request, mock(SdkHttpRequest.class), attributes); + + verify(span).setTag("aws.table.name", "orders"); + verify(span).setTag("tablename", "orders"); + if (enrich) { + verify(span).setTag("aws_account", "123456789012"); + } else { + verify(span, never()).setTag(eq("aws_account"), anyString()); + } + verify(span, never()).setTag(eq("aws.table.arn"), anyString()); + } + + @TableTest({ + "Scenario | Table Name | Table Arn", + "Table | arn:aws:dynamodb:us-east-1:123456789012:table/orders | true ", + "Non-table | arn:aws:dynamodb:us-east-1:123456789012:backup/orders | false ", + "Empty name | arn:aws:dynamodb:us-east-1:123456789012:table/ | false ", + "SNS ARN | arn:aws:sns:us-east-1:123456789012:table/orders | false ", + "S3 ARN | arn:aws:s3:::my-bucket | false " + }) + void preservesTableNamesAndOnlyTagsTableArns(String tableName, boolean tableArn) { + AgentSpan span = mock(AgentSpan.class, RETURNS_SELF); + Context context = mock(Context.class); + when(context.get(any())).thenReturn(span); + SdkRequest request = mock(SdkRequest.class); + when(request.getValueForField("TableName", String.class)).thenReturn(Optional.of(tableName)); + ExecutionAttributes attributes = new ExecutionAttributes(); + attributes.putAttribute(SdkExecutionAttribute.SERVICE_NAME, "DynamoDb"); + attributes.putAttribute(SdkExecutionAttribute.OPERATION_NAME, "GetItem"); + + AwsSdkClientDecorator.DECORATE.onSdkRequest( + context, request, mock(SdkHttpRequest.class), attributes); + + verify(span).setTag("aws.table.name", tableName); + verify(span).setTag("tablename", tableName); + if (tableArn) { + verify(span).setTag("aws.table.arn", tableName); + } else { + verify(span, never()).setTag(eq("aws.table.arn"), anyString()); + } + } + + /** Exposes the newer credentials API while compiling against the SDK 2.2.0 baseline. */ + public static class AccountCredentials implements AwsCredentials { + public Optional accountId() { + return Optional.of("123456789012"); + } + + @Override + public String accessKeyId() { + return "test"; + } + + @Override + public String secretAccessKey() { + return "test"; + } + } +} diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/InstrumentationTags.java b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/InstrumentationTags.java index 0c1054e7776..5a84543e55d 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/InstrumentationTags.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/instrumentation/api/InstrumentationTags.java @@ -37,6 +37,11 @@ public class InstrumentationTags { public static final String AWS_REQUEST_ID = "aws.requestId"; public static final String AWS_STORAGE_CLASS = "aws.storage.class"; + // Owning account of the addressed resource. aws_account matches the tag dd-trace-py sets and the + // dimension tag on the AWS integration metrics. + public static final String AWS_ACCOUNT = "aws_account"; + public static final String AWS_TABLE_ARN = "aws.table.arn"; + // These are temporary keys used for span pointer hash calculation public static final String S3_ETAG = "s3.eTag"; public static final String DYNAMO_PRIMARY_KEY_1 = "dynamodb.primary_key_1";