Support AWS SDK v1 request object subclasses. (#5231)

open-telemetry · Jan 26, 2022 · d79c26f · d79c26f
1 parent 5f4fc25
commit d79c26f
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 14 deletions.
diff --git a/...rc/main/java/io/opentelemetry/instrumentation/awssdk/v1_11/AwsSdkInstrumenterFactory.java b/...rc/main/java/io/opentelemetry/instrumentation/awssdk/v1_11/AwsSdkInstrumenterFactory.java
@@ -38,7 +38,8 @@ final class AwsSdkInstrumenterFactory {
               rpcAttributesExtractor,
               netAttributesExtractor,
               experimentalAttributesExtractor);
-  private static final AwsSdkSpanNameExtractor spanName = new AwsSdkSpanNameExtractor();
+  private static final AwsSdkSpanNameExtractor spanName =
+      new AwsSdkSpanNameExtractor(rpcAttributesExtractor);
 
   static Instrumenter<Request<?>, Response<?>> requestInstrumenter(
       OpenTelemetry openTelemetry, boolean captureExperimentalSpanAttributes) {

diff --git a/...main/java/io/opentelemetry/instrumentation/awssdk/v1_11/AwsSdkRpcAttributesExtractor.java b/...main/java/io/opentelemetry/instrumentation/awssdk/v1_11/AwsSdkRpcAttributesExtractor.java
@@ -16,7 +16,13 @@ final class AwsSdkRpcAttributesExtractor extends RpcAttributesExtractor<Request<
         @Override
         protected String computeValue(Class<?> type) {
           String ret = type.getSimpleName();
-          ret = ret.substring(0, ret.length() - 7); // remove 'Request'
+          if (!ret.endsWith("Request")) {
+            // Best effort check one parent to support implicit subclasses
+            ret = type.getSuperclass().getSimpleName();
+          }
+          if (ret.endsWith("Request")) {
+            ret = ret.substring(0, ret.length() - 7); // remove 'Request'
+          }
           return ret;
         }
       };

diff --git a/.../src/main/java/io/opentelemetry/instrumentation/awssdk/v1_11/AwsSdkSpanNameExtractor.java b/.../src/main/java/io/opentelemetry/instrumentation/awssdk/v1_11/AwsSdkSpanNameExtractor.java
@@ -10,23 +10,25 @@
 import java.util.concurrent.ConcurrentHashMap;
 
 class AwsSdkSpanNameExtractor implements SpanNameExtractor<Request<?>> {
+
+  private final AwsSdkRpcAttributesExtractor rpcAttributes;
   private final NamesCache namesCache = new NamesCache();
 
+  AwsSdkSpanNameExtractor(AwsSdkRpcAttributesExtractor rpcAttributes) {
+    this.rpcAttributes = rpcAttributes;
+  }
+
   @Override
   public String extract(Request<?> request) {
-    String awsServiceName = request.getServiceName();
-    Class<?> awsOperation = request.getOriginalRequest().getClass();
-    return qualifiedOperation(awsServiceName, awsOperation);
+    return qualifiedOperation(
+        rpcAttributes.service(request),
+        rpcAttributes.method(request),
+        request.getOriginalRequest().getClass());
   }
 
-  private String qualifiedOperation(String service, Class<?> operation) {
-    ConcurrentHashMap<String, String> cache = namesCache.get(operation);
-    return cache.computeIfAbsent(
-        service,
-        s ->
-            s.replace("Amazon", "").trim()
-                + '.'
-                + operation.getSimpleName().replace("Request", ""));
+  private String qualifiedOperation(String service, String operation, Class<?> requestClass) {
+    ConcurrentHashMap<String, String> cache = namesCache.get(requestClass);
+    return cache.computeIfAbsent(service, s -> s.replace("Amazon", "").trim() + '.' + operation);
   }
 
   static final class NamesCache extends ClassValue<ConcurrentHashMap<String, String>> {

diff --git a/...c/main/groovy/io/opentelemetry/instrumentation/awssdk/v1_11/AbstractAws1ClientTest.groovy b/...c/main/groovy/io/opentelemetry/instrumentation/awssdk/v1_11/AbstractAws1ClientTest.groovy
@@ -94,7 +94,7 @@ abstract class AbstractAws1ClientTest extends InstrumentationSpecification {
     response != null
 
     client.requestHandler2s != null
-    client.requestHandler2s.find{it.getClass().getSimpleName() == "TracingRequestHandler"} != null
+    client.requestHandler2s.find { it.getClass().getSimpleName() == "TracingRequestHandler" } != null
 
     assertTraces(1) {
       trace(0, 1) {
@@ -133,6 +133,11 @@ abstract class AbstractAws1ClientTest extends InstrumentationSpecification {
     "S3"         | "GetObject"         | "GET"  | "/someBucket/someKey" | AmazonS3ClientBuilder.standard().withPathStyleAccessEnabled(true) | { c -> c.getObject("someBucket", "someKey") }                                   | ["aws.bucket.name": "someBucket"] | ""
     "DynamoDBv2" | "CreateTable"       | "POST" | "/"                   | AmazonDynamoDBClientBuilder.standard()                            | { c -> c.createTable(new CreateTableRequest("sometable", null)) }               | ["aws.table.name": "sometable"]   | ""
     "Kinesis"    | "DeleteStream"      | "POST" | "/"                   | AmazonKinesisClientBuilder.standard()                             | { c -> c.deleteStream(new DeleteStreamRequest().withStreamName("somestream")) } | ["aws.stream.name": "somestream"] | ""
+    // Some users may implicitly subclass the request object to mimic a fluent style
+    "Kinesis"    | "DeleteStream"      | "POST" | "/"                   | AmazonKinesisClientBuilder.standard()                             | { c ->
+      c.deleteStream(new DeleteStreamRequest() {
+        { withStreamName("somestream") }
+      }) }                                                                                                                                                                                                                    | ["aws.stream.name": "somestream"] | ""
     "EC2"        | "AllocateAddress"   | "POST" | "/"                   | AmazonEC2ClientBuilder.standard()                                 | { c -> c.allocateAddress() }                                                    | [:]                               | """
         <AllocateAddressResponse xmlns="http://ec2.amazonaws.com/doc/2016-11-15/">
            <requestId>59dbff89-35bd-4eac-99ed-be587EXAMPLE</requestId>