Fix AWS instrumentation destination.service.resource handling

CHANGELOG.asciidoc

@@ -25,10 +25,16 @@ endif::[]
 
 [float]
 ===== Bug fixes
+
 * Use `127.0.0.1` as defaut for `server_url` to prevent ipv6 ambiguity - {pull}2927[#2927]
 * Fix some span-compression concurrency issues - {pull}2865[#2865]
 * Add warning when agent is accidentally started on a JVM/JDK command-line tool - {pull}2924[#2924]
 
+===== Potentially breaking changes
+
+* The `context.destination.service.resource` span field has been changed for AWS S3, SQS and DynamoDB.
+As a result, the history of metrics relying on this field might break. - {pull}2947[#2947]
+
 [[release-notes-1.x]]
 === Java Agent version 1.x
 
@@ -37,6 +43,7 @@ endif::[]
 
 [float]
 ===== Features
+
 * Add support for log correlation for `java.util.logging` (JUL) - {pull}2724[#2724]
 * Add support for spring-kafka batch listeners - {pull}2815[#2815]
 * Improved instrumentation for legacy Apache HttpClient (when not using an `HttpUriRequest`, such as `BasicHttpRequest`)

apm-agent-plugins/apm-aws-sdk/apm-aws-sdk-1-plugin/src/test/java/co/elastic/apm/agent/awssdk/v1/DynamoDbClientIT.java

@@ -43,6 +43,7 @@
 import org.junit.jupiter.api.Test;
 import org.testcontainers.containers.localstack.LocalStackContainer;
 
+import javax.annotation.Nullable;
 import java.util.List;
 import java.util.Map;
 import java.util.function.Consumer;
@@ -168,6 +169,17 @@ protected String type() {
         return "db";
     }
 
+    @Override
+    protected String subtype() {
+        return "dynamodb";
+    }
+
+    @Nullable
+    @Override
+    protected String expectedTargetName(@Nullable String entityName) {
+        return localstack.getRegion();
+    }
+
     @Override
     protected LocalStackContainer.Service localstackService() {
         return LocalStackContainer.Service.DYNAMODB;

apm-agent-plugins/apm-aws-sdk/apm-aws-sdk-1-plugin/src/test/java/co/elastic/apm/agent/awssdk/v1/S3ClientIT.java

@@ -31,6 +31,8 @@
 import org.junit.jupiter.api.Test;
 import org.testcontainers.containers.localstack.LocalStackContainer;
 
+import javax.annotation.Nullable;
+
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.testcontainers.containers.localstack.LocalStackContainer.Service.S3;
 
@@ -86,6 +88,17 @@ protected String type() {
         return "storage";
     }
 
+    @Override
+    protected String subtype() {
+        return "s3";
+    }
+
+    @Nullable
+    @Override
+    protected String expectedTargetName(@Nullable String entityName) {
+        return entityName; //entityName is BUCKET_NAME
+    }
+
     @Override
     protected LocalStackContainer.Service localstackService() {
         return S3;

apm-agent-plugins/apm-aws-sdk/apm-aws-sdk-1-plugin/src/test/java/co/elastic/apm/agent/awssdk/v1/SQSClientIT.java

@@ -40,6 +40,7 @@
 import org.junit.jupiter.api.Test;
 import org.testcontainers.containers.localstack.LocalStackContainer;
 
+import javax.annotation.Nullable;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
@@ -242,6 +243,17 @@ protected String type() {
         return "messaging";
     }
 
+    @Override
+    protected String subtype() {
+        return "sqs";
+    }
+
+    @Nullable
+    @Override
+    protected String expectedTargetName(@Nullable String entityName) {
+        return entityName; //entityName is queue name
+    }
+
     @Override
     protected LocalStackContainer.Service localstackService() {
         return LocalStackContainer.Service.SQS;

apm-agent-plugins/apm-aws-sdk/apm-aws-sdk-1-plugin/src/test/java/co/elastic/apm/agent/awssdk/v1/SQSJmsClientIT.java

@@ -41,6 +41,7 @@
 import org.junit.jupiter.api.Test;
 import org.testcontainers.containers.localstack.LocalStackContainer;
 
+import javax.annotation.Nullable;
 import javax.jms.JMSException;
 import javax.jms.Message;
 import javax.jms.MessageConsumer;
@@ -274,6 +275,17 @@ protected String type() {
         return "messaging";
     }
 
+    @Override
+    protected String subtype() {
+        return "sqs";
+    }
+
+    @Nullable
+    @Override
+    protected String expectedTargetName(@Nullable String entityName) {
+        return entityName; //entityName is queue name
+    }
+
     @Override
     protected LocalStackContainer.Service localstackService() {
         return LocalStackContainer.Service.SQS;

apm-agent-plugins/apm-aws-sdk/apm-aws-sdk-2-plugin/src/test/java/co/elastic/apm/agent/awssdk/v2/DynamoDbClientIT.java

@@ -42,6 +42,7 @@
 import software.amazon.awssdk.services.dynamodb.model.ResourceNotFoundException;
 import software.amazon.awssdk.services.dynamodb.model.ScalarAttributeType;
 
+import javax.annotation.Nullable;
 import java.util.AbstractMap;
 import java.util.Arrays;
 import java.util.Collections;
@@ -198,6 +199,17 @@ protected String type() {
         return "db";
     }
 
+    @Override
+    protected String subtype() {
+        return "dynamodb";
+    }
+
+    @Nullable
+    @Override
+    protected String expectedTargetName(@Nullable String entityName) {
+        return localstack.getRegion();
+    }
+
     @Override
     protected LocalStackContainer.Service localstackService() {
         return LocalStackContainer.Service.DYNAMODB;

apm-agent-plugins/apm-aws-sdk/apm-aws-sdk-2-plugin/src/test/java/co/elastic/apm/agent/awssdk/v2/S3ClientIT.java

@@ -40,6 +40,8 @@
 import software.amazon.awssdk.services.s3.model.ListObjectsRequest;
 import software.amazon.awssdk.services.s3.model.PutObjectRequest;
 
+import javax.annotation.Nullable;
+
 import static org.assertj.core.api.Assertions.assertThat;
 
 
@@ -114,6 +116,17 @@ protected String type() {
         return "storage";
     }
 
+    @Override
+    protected String subtype() {
+        return "s3";
+    }
+
+    @Nullable
+    @Override
+    protected String expectedTargetName(@Nullable String entityName) {
+        return entityName; //entityName is bucket name
+    }
+
     @Override
     protected LocalStackContainer.Service localstackService() {
         return LocalStackContainer.Service.S3;

apm-agent-plugins/apm-aws-sdk/apm-aws-sdk-2-plugin/src/test/java/co/elastic/apm/agent/awssdk/v2/SQSClientIT.java

@@ -45,6 +45,7 @@
 import software.amazon.awssdk.services.sqs.model.SendMessageBatchRequestEntry;
 import software.amazon.awssdk.services.sqs.model.SendMessageRequest;
 
+import javax.annotation.Nullable;
 import java.util.Collections;
 import java.util.function.Consumer;
 
@@ -225,6 +226,17 @@ protected String type() {
         return "messaging";
     }
 
+    @Override
+    protected String subtype() {
+        return "sqs";
+    }
+
+    @Nullable
+    @Override
+    protected String expectedTargetName(@Nullable String entityName) {
+        return entityName; //entityName is queue name
+    }
+
     @Override
     protected LocalStackContainer.Service localstackService() {
         return LocalStackContainer.Service.SQS;

apm-agent-plugins/apm-aws-sdk/apm-aws-sdk-2-plugin/src/test/java/co/elastic/apm/agent/awssdk/v2/SQSJmsClientIT.java

@@ -41,6 +41,7 @@
 import software.amazon.awssdk.services.sqs.SqsClient;
 import software.amazon.awssdk.services.sqs.model.MessageAttributeValue;
 
+import javax.annotation.Nullable;
 import javax.jms.JMSException;
 import javax.jms.Message;
 import javax.jms.MessageConsumer;
@@ -279,6 +280,17 @@ protected String type() {
         return "messaging";
     }
 
+    @Override
+    protected String subtype() {
+        return "sqs";
+    }
+
+    @Nullable
+    @Override
+    protected String expectedTargetName(@Nullable String entityName) {
+        return entityName; //entityName is queue name
+    }
+
     @Override
     protected LocalStackContainer.Service localstackService() {
         return LocalStackContainer.Service.SQS;

apm-agent-plugins/apm-aws-sdk/apm-aws-sdk-common/src/main/java/co/elastic/apm/agent/awssdk/common/AbstractAwsSdkInstrumentationHelper.java

@@ -49,8 +49,7 @@ protected void setDestinationContext(Span span, @Nullable URI uri, R sdkRequest,
 
         span.getContext().getServiceTarget()
             .withType(type)
-            .withName(name)
-            .withNameOnlyDestinationResource();
+            .withName(name);
 
         span.getContext().getDestination()
             .getCloud()

apm-agent-plugins/apm-aws-sdk/apm-aws-sdk-common/src/main/java/co/elastic/apm/agent/awssdk/common/AbstractDynamoDBInstrumentationHelper.java

@@ -58,7 +58,7 @@ public void enrichSpan(Span span, R sdkRequest, URI httpURI, C context) {
             }
         }
 
-        setDestinationContext(span, httpURI, sdkRequest, context, DYNAMO_DB_TYPE, null);
+        setDestinationContext(span, httpURI, sdkRequest, context, DYNAMO_DB_TYPE, region);
     }
 
     @Nullable

apm-agent-plugins/apm-aws-sdk/apm-aws-sdk-common/src/test/java/co/elastic/apm/agent/awssdk/common/AbstractAwsClientIT.java

@@ -19,6 +19,7 @@
 package co.elastic.apm.agent.awssdk.common;
 
 import co.elastic.apm.agent.AbstractInstrumentationTest;
+import co.elastic.apm.agent.impl.context.ServiceTarget;
 import co.elastic.apm.agent.impl.transaction.Outcome;
 import co.elastic.apm.agent.impl.transaction.Span;
 import org.testcontainers.containers.localstack.LocalStackContainer;
@@ -44,11 +45,7 @@ public abstract class AbstractAwsClientIT extends AbstractInstrumentationTest {
     private static final DockerImageName localstackImage = DockerImageName.parse("localstack/localstack:0.14.2");
     protected static final String BUCKET_NAME = "some-test-bucket";
     protected static final String SQS_QUEUE_NAME = "some-test-sqs-queue";
-    protected static final String SQS_MESSAGE_PROCESSING_SPAN_NAME = "Process SQS message from " + SQS_QUEUE_NAME;
     protected static final String SQS_IGNORED_QUEUE_NAME = "ignored-queue";
-    protected static final String SQS_TYPE = "sqs";
-    protected static final String SQS_MESSAGING_TYPE = "messaging";
-    protected static final String SQS_MESSAGE_PROCESSING_ACTION = "processing";
     protected static final String MESSAGE_BODY = "some-test-sqs-message-body";
     protected static final String NEW_BUCKET_NAME = "new-test-bucket";
     protected static final String OBJECT_KEY = "some-object-key";
@@ -62,6 +59,11 @@ public abstract class AbstractAwsClientIT extends AbstractInstrumentationTest {
 
     protected abstract String type();
 
+    protected abstract String subtype();
+
+    @Nullable
+    protected abstract String expectedTargetName(@Nullable String entityName);
+
     protected abstract LocalStackContainer.Service localstackService();
 
     protected void executeTest(String operationName, @Nullable String entityName, Supplier<?> test) {
@@ -96,6 +98,17 @@ protected void executeTest(String operationName, String action, @Nullable String
         assertThat(span.getType()).isEqualTo(type());
         assertThat(span.getSubtype()).isEqualTo(localstackService().getLocalStackName());
         assertThat(span.getAction()).isEqualTo(action);
+        ServiceTarget serviceTarget = span.getContext().getServiceTarget();
+        assertThat(serviceTarget.getType()).isEqualTo(subtype());
+        String expectedTargetName = expectedTargetName(entityName);
+        if (expectedTargetName == null) {
+            assertThat(serviceTarget.getName()).isNull();
+            assertThat(serviceTarget.getDestinationResource().toString()).isEqualTo(subtype());
+        } else {
+            assertThat(serviceTarget.getName()).isNotNull();
+            assertThat(serviceTarget.getName().toString()).isEqualTo(expectedTargetName);
+            assertThat(serviceTarget.getDestinationResource().toString()).isEqualTo(subtype() + "/" + expectedTargetName);
+        }
         assertThat(span.getContext().getDestination().getAddress().toString())
             .isEqualTo(localstack.getEndpointOverride(LocalStackContainer.Service.S3).getHost());
         if (assertions != null) {
@@ -122,4 +135,5 @@ protected void executeTestWithException(Class<? extends Exception> exceptionType
             assertions.accept(span);
         }
     }
+
 }