Fix AWS tests and change span type to http-client

There seems to be a bug.  I would expect the aws span to be the parent of the apache http span, but that is not the case.

Removed the now unused HTTPComponent decorator.

dd-java-agent/instrumentation/apache-httpclient-4/src/test/groovy/ApacheHttpClientTest.groovy

@@ -129,7 +129,7 @@ class ApacheHttpClientTest extends AgentTestRunner {
         "$Tags.HTTP_STATUS.key" status
         "$Tags.HTTP_URL.key" "http://localhost:$port/$route"
         "$Tags.PEER_HOSTNAME.key" "localhost"
-        "$Tags.PEER_PORT.key" server.getAddress().port
+        "$Tags.PEER_PORT.key" server.address.port
         "$Tags.HTTP_METHOD.key" "GET"
         "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
         "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT

dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/main/java/datadog/trace/instrumentation/aws/v0/SpanDecorator.java

@@ -13,21 +13,16 @@
  */
 package datadog.trace.instrumentation.aws.v0;
 
-import static io.opentracing.log.Fields.ERROR_KIND;
 import static io.opentracing.log.Fields.ERROR_OBJECT;
-import static io.opentracing.log.Fields.EVENT;
-import static io.opentracing.log.Fields.MESSAGE;
-import static io.opentracing.log.Fields.STACK;
 
 import com.amazonaws.AmazonWebServiceResponse;
 import com.amazonaws.Request;
 import com.amazonaws.Response;
+import datadog.trace.api.DDSpanTypes;
 import datadog.trace.api.DDTags;
 import io.opentracing.Span;
 import io.opentracing.tag.Tags;
-import java.io.PrintWriter;
+import java.util.Collections;
-import java.io.StringWriter;
-import java.util.HashMap;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 
@@ -50,9 +45,11 @@ class SpanDecorator {
     span.setTag("aws.operation", awsOperation.getSimpleName());
     span.setTag("aws.endpoint", request.getEndpoint().toString());
 
+    span.setTag(DDTags.SERVICE_NAME, COMPONENT_NAME);
     span.setTag(
         DDTags.RESOURCE_NAME,
         remapServiceName(awsServiceName) + "." + remapOperationName(awsOperation));
+    span.setTag(DDTags.SPAN_TYPE, DDSpanTypes.HTTP_CLIENT);
   }
 
   static void onResponse(final Response response, final Span span) {
@@ -65,22 +62,7 @@ class SpanDecorator {
 
   static void onError(final Throwable throwable, final Span span) {
     Tags.ERROR.set(span, Boolean.TRUE);
-    span.log(errorLogs(throwable));
+    span.log(Collections.singletonMap(ERROR_OBJECT, throwable));
-  }
-
-  private static Map<String, Object> errorLogs(final Throwable throwable) {
-    final Map<String, Object> errorLogs = new HashMap<>(4);
-    errorLogs.put(EVENT, Tags.ERROR.getKey());
-    errorLogs.put(ERROR_KIND, throwable.getClass().getName());
-    errorLogs.put(ERROR_OBJECT, throwable);
-
-    errorLogs.put(MESSAGE, throwable.getMessage());
-
-    final StringWriter sw = new StringWriter();
-    throwable.printStackTrace(new PrintWriter(sw));
-    errorLogs.put(STACK, sw.toString());
-
-    return errorLogs;
   }
 
   private static String remapServiceName(final String serviceName) {

dd-java-agent/instrumentation/aws-java-sdk-1.11.0/src/test/groovy/AWSClientTest.groovy

@@ -7,6 +7,7 @@ import com.amazonaws.services.rds.model.DeleteOptionGroupRequest
 import com.amazonaws.services.s3.AmazonS3Client
 import com.amazonaws.services.s3.S3ClientOptions
 import datadog.trace.agent.test.AgentTestRunner
+import datadog.trace.api.DDSpanTypes
 import datadog.trace.api.DDTags
 import io.opentracing.tag.Tags
 import spock.lang.AutoCleanup
@@ -14,6 +15,7 @@ import spock.lang.Shared
 
 import java.util.concurrent.atomic.AtomicReference
 
+import static datadog.trace.agent.test.asserts.ListWriterAssert.assertTraces
 import static datadog.trace.agent.test.server.http.TestHttpServer.httpServer
 
 class AWSClientTest extends AgentTestRunner {
@@ -72,81 +74,52 @@ class AWSClientTest extends AgentTestRunner {
     client.requestHandler2s.size() == handlerCount
     client.requestHandler2s.get(0).getClass().getSimpleName() == "TracingRequestHandler"
 
-    TEST_WRITER.size() == 2
+    assertTraces(TEST_WRITER, 2) {
-
+      trace(0, 1) {
-    def trace = TEST_WRITER.get(0)
+        span(0) {
-    trace.size() == 2
+          operationName "apache.http.request"
-
+          resourceName "$method /$url"
-    and: // span 0 - from apache-httpclient instrumentation
+          errored false
-    def span1 = trace[0]
+          parent() // FIXME: This should be a child of the aws.http call.
-
+          tags {
-    span1.context().operationName == "apache.http"
+            "$Tags.COMPONENT.key" "apache-httpclient"
-    span1.serviceName == "unnamed-java-app"
+            "$Tags.HTTP_STATUS.key" 200
-    span1.resourceName == "apache.http"
+            "$Tags.HTTP_URL.key" "$server.address/$url"
-    span1.type == null
+            "$Tags.PEER_HOSTNAME.key" "localhost"
-    !span1.context().getErrorFlag()
+            "$Tags.PEER_PORT.key" server.address.port
-    span1.context().parentId == "0"
+            "$Tags.HTTP_METHOD.key" "$method"
-
+            "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
-
+            "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT
-    def tags1 = span1.context().tags
+            defaultTags()
-    tags1["component"] == "apache-httpclient"
+          }
-    tags1["thread.name"] != null
+        }
-    tags1["thread.id"] != null
+      }
-    tags1.size() == 3
+      trace(1, 1) {
-
+        span(0) {
-    and: // span 1 - from apache-httpclient instrumentation
+          serviceName "java-aws-sdk"
-    def span2 = trace[1]
+          operationName "aws.http"
-
+          resourceName "$service.$operation"
-    span2.context().operationName == "http.request"
+          errored false
-    span2.serviceName == "unnamed-java-app"
+          parent()
-    span2.resourceName == "$method /$url"
+          tags {
-    span2.type == "http"
+            "$Tags.COMPONENT.key" "java-aws-sdk"
-    !span2.context().getErrorFlag()
+            "$Tags.HTTP_STATUS.key" 200
-    span2.context().parentId == span1.spanId
+            "$Tags.HTTP_URL.key" "$server.address"
-
+            "$Tags.HTTP_METHOD.key" "$method"
-
+            "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
-    def tags2 = span2.context().tags
+            "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT
-    tags2[Tags.SPAN_KIND.key] == Tags.SPAN_KIND_CLIENT
+            "aws.service" String
-    tags2[Tags.HTTP_METHOD.key] == "$method"
+            "aws.endpoint" "$server.address"
-    tags2[Tags.HTTP_URL.key] == "http://localhost:$server.address.port/$url"
+            "aws.operation" "${operation}Request"
-    tags2[Tags.PEER_HOSTNAME.key] == "localhost"
+            "aws.agent" "java-aws-sdk"
-    tags2[Tags.PEER_PORT.key] == server.address.port
+            defaultTags()
-    tags2[DDTags.THREAD_NAME] != null
+          }
-    tags2[DDTags.THREAD_ID] != null
+        }
-    tags2.size() == 9
+      }
-
+    }
-    and:
+    // Not sure why these are children of the aws.http span:
-
+    server.lastRequest.headers.get("x-datadog-trace-id") == TEST_WRITER[1][0].traceId
-    def trace2 = TEST_WRITER.get(1)
+    server.lastRequest.headers.get("x-datadog-parent-id") == TEST_WRITER[1][0].spanId
-    trace2.size() == 1
-
-    and: // span 0 - from aws instrumentation
-    def span = trace2[0]
-
-    span.context().operationName == "aws.http"
-    span.serviceName == "java-aws-sdk"
-    span.resourceName == "$service.$operation"
-    span.type == "web"
-    !span.context().getErrorFlag()
-    span.context().parentId == "0"
-
-    def tags = span.context().tags
-    tags[Tags.COMPONENT.key] == "java-aws-sdk"
-    tags[Tags.SPAN_KIND.key] == Tags.SPAN_KIND_CLIENT
-    tags[Tags.HTTP_METHOD.key] == "$method"
-    tags[Tags.HTTP_URL.key] == "http://localhost:$server.address.port"
-    tags[Tags.HTTP_STATUS.key] == 200
-    tags["aws.service"] == "Amazon $service" || tags["aws.service"] == "Amazon$service"
-    tags["aws.endpoint"] == "http://localhost:$server.address.port"
-    tags["aws.operation"] == "${operation}Request"
-    tags["aws.agent"] == "java-aws-sdk"
-    tags["span.type"] == "web"
-    tags["thread.name"] != null
-    tags["thread.id"] != null
-    tags.size() == 12
-
-    server.lastRequest.headers.get("x-datadog-trace-id") == "$span.traceId"
-    server.lastRequest.headers.get("x-datadog-parent-id") == "$span.spanId"
 
     where:
     service | operation           | method | url                  | handlerCount | call                                                                                                                                   | body               | client

dd-java-agent/instrumentation/aws-java-sdk-1.11.106/src/main/java/datadog/trace/instrumentation/aws/v106/SpanDecorator.java

@@ -13,21 +13,16 @@
  */
 package datadog.trace.instrumentation.aws.v106;
 
-import static io.opentracing.log.Fields.ERROR_KIND;
 import static io.opentracing.log.Fields.ERROR_OBJECT;
-import static io.opentracing.log.Fields.EVENT;
-import static io.opentracing.log.Fields.MESSAGE;
-import static io.opentracing.log.Fields.STACK;
 
 import com.amazonaws.AmazonWebServiceResponse;
 import com.amazonaws.Request;
 import com.amazonaws.Response;
+import datadog.trace.api.DDSpanTypes;
 import datadog.trace.api.DDTags;
 import io.opentracing.Span;
 import io.opentracing.tag.Tags;
-import java.io.PrintWriter;
+import java.util.Collections;
-import java.io.StringWriter;
-import java.util.HashMap;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 
@@ -50,9 +45,11 @@ class SpanDecorator {
     span.setTag("aws.operation", awsOperation.getSimpleName());
     span.setTag("aws.endpoint", request.getEndpoint().toString());
 
+    span.setTag(DDTags.SERVICE_NAME, COMPONENT_NAME);
     span.setTag(
         DDTags.RESOURCE_NAME,
         remapServiceName(awsServiceName) + "." + remapOperationName(awsOperation));
+    span.setTag(DDTags.SPAN_TYPE, DDSpanTypes.HTTP_CLIENT);
   }
 
   static void onResponse(final Response response, final Span span) {
@@ -65,22 +62,7 @@ class SpanDecorator {
 
   static void onError(final Throwable throwable, final Span span) {
     Tags.ERROR.set(span, Boolean.TRUE);
-    span.log(errorLogs(throwable));
+    span.log(Collections.singletonMap(ERROR_OBJECT, throwable));
-  }
-
-  private static Map<String, Object> errorLogs(final Throwable throwable) {
-    final Map<String, Object> errorLogs = new HashMap<>(4);
-    errorLogs.put(EVENT, Tags.ERROR.getKey());
-    errorLogs.put(ERROR_KIND, throwable.getClass().getName());
-    errorLogs.put(ERROR_OBJECT, throwable);
-
-    errorLogs.put(MESSAGE, throwable.getMessage());
-
-    final StringWriter sw = new StringWriter();
-    throwable.printStackTrace(new PrintWriter(sw));
-    errorLogs.put(STACK, sw.toString());
-
-    return errorLogs;
   }
 
   private static String remapServiceName(final String serviceName) {

dd-java-agent/instrumentation/aws-java-sdk-1.11.106/src/test/groovy/AWSClientTest.groovy

@@ -12,6 +12,7 @@ import com.amazonaws.services.rds.model.DeleteOptionGroupRequest
 import com.amazonaws.services.s3.AmazonS3Client
 import com.amazonaws.services.s3.AmazonS3ClientBuilder
 import datadog.trace.agent.test.AgentTestRunner
+import datadog.trace.api.DDSpanTypes
 import datadog.trace.api.DDTags
 import io.opentracing.tag.Tags
 import spock.lang.AutoCleanup
@@ -19,6 +20,7 @@ import spock.lang.Shared
 
 import java.util.concurrent.atomic.AtomicReference
 
+import static datadog.trace.agent.test.asserts.ListWriterAssert.assertTraces
 import static datadog.trace.agent.test.server.http.TestHttpServer.httpServer
 
 class AWSClientTest extends AgentTestRunner {
@@ -101,81 +103,52 @@ class AWSClientTest extends AgentTestRunner {
     client.requestHandler2s.size() == handlerCount
     client.requestHandler2s.get(0).getClass().getSimpleName() == "TracingRequestHandler"
 
-    TEST_WRITER.size() == 2
+    assertTraces(TEST_WRITER, 2) {
-
+      trace(0, 1) {
-    def trace = TEST_WRITER.get(0)
+        span(0) {
-    trace.size() == 2
+          operationName "apache.http.request"
-
+          resourceName "$method /$url"
-    and: // span 0 - from apache-httpclient instrumentation
+          errored false
-    def span1 = trace[0]
+          parent() // FIXME: This should be a child of the aws.http call.
-
+          tags {
-    span1.context().operationName == "apache.http"
+            "$Tags.COMPONENT.key" "apache-httpclient"
-    span1.serviceName == "unnamed-java-app"
+            "$Tags.HTTP_STATUS.key" 200
-    span1.resourceName == "apache.http"
+            "$Tags.HTTP_URL.key" "$server.address/$url"
-    span1.type == null
+            "$Tags.PEER_HOSTNAME.key" "localhost"
-    !span1.context().getErrorFlag()
+            "$Tags.PEER_PORT.key" server.address.port
-    span1.context().parentId == "0"
+            "$Tags.HTTP_METHOD.key" "$method"
-
+            "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
-
+            "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT
-    def tags1 = span1.context().tags
+            defaultTags()
-    tags1["component"] == "apache-httpclient"
+          }
-    tags1["thread.name"] != null
+        }
-    tags1["thread.id"] != null
+      }
-    tags1.size() == 3
+      trace(1, 1) {
-
+        span(0) {
-    and: // span 1 - from apache-httpclient instrumentation
+          serviceName "java-aws-sdk"
-    def span2 = trace[1]
+          operationName "aws.http"
-
+          resourceName "$service.$operation"
-    span2.context().operationName == "http.request"
+          errored false
-    span2.serviceName == "unnamed-java-app"
+          parent()
-    span2.resourceName == "$method /$url"
+          tags {
-    span2.type == "http"
+            "$Tags.COMPONENT.key" "java-aws-sdk"
-    !span2.context().getErrorFlag()
+            "$Tags.HTTP_STATUS.key" 200
-    span2.context().parentId == span1.spanId
+            "$Tags.HTTP_URL.key" "$server.address"
-
+            "$Tags.HTTP_METHOD.key" "$method"
-
+            "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
-    def tags2 = span2.context().tags
+            "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT
-    tags2[Tags.SPAN_KIND.key] == Tags.SPAN_KIND_CLIENT
+            "aws.service" String
-    tags2[Tags.HTTP_METHOD.key] == "$method"
+            "aws.endpoint" "$server.address"
-    tags2[Tags.HTTP_URL.key] == "http://localhost:$server.address.port/$url"
+            "aws.operation" "${operation}Request"
-    tags2[Tags.PEER_HOSTNAME.key] == "localhost"
+            "aws.agent" "java-aws-sdk"
-    tags2[Tags.PEER_PORT.key] == server.address.port
+            defaultTags()
-    tags2[DDTags.THREAD_NAME] != null
+          }
-    tags2[DDTags.THREAD_ID] != null
+        }
-    tags2.size() == 9
+      }
-
+    }
-    and:
+    // Not sure why these are children of the aws.http span:
-
+    server.lastRequest.headers.get("x-datadog-trace-id") == TEST_WRITER[1][0].traceId
-    def trace2 = TEST_WRITER.get(1)
+    server.lastRequest.headers.get("x-datadog-parent-id") == TEST_WRITER[1][0].spanId
-    trace2.size() == 1
-
-    and: // span 0 - from aws instrumentation
-    def span = trace2[0]
-
-    span.context().operationName == "aws.http"
-    span.serviceName == "java-aws-sdk"
-    span.resourceName == "$service.$operation"
-    span.type == "web"
-    !span.context().getErrorFlag()
-    span.context().parentId == "0"
-
-    def tags = span.context().tags
-    tags[Tags.COMPONENT.key] == "java-aws-sdk"
-    tags[Tags.SPAN_KIND.key] == Tags.SPAN_KIND_CLIENT
-    tags[Tags.HTTP_METHOD.key] == "$method"
-    tags[Tags.HTTP_URL.key] == "http://localhost:$server.address.port"
-    tags[Tags.HTTP_STATUS.key] == 200
-    tags["aws.service"] == "Amazon $service" || tags["aws.service"] == "Amazon$service"
-    tags["aws.endpoint"] == "http://localhost:$server.address.port"
-    tags["aws.operation"] == "${operation}Request"
-    tags["aws.agent"] == "java-aws-sdk"
-    tags["span.type"] == "web"
-    tags["thread.name"] != null
-    tags["thread.id"] != null
-    tags.size() == 12
-
-    server.lastRequest.headers.get("x-datadog-trace-id") == "$span.traceId"
-    server.lastRequest.headers.get("x-datadog-parent-id") == "$span.spanId"
 
     where:
     service | operation           | method | url                  | handlerCount | call                                                                   | body               | client

dd-trace-ot/src/main/java/datadog/opentracing/decorators/DDDecoratorsFactory.java

@@ -6,15 +6,11 @@ import java.util.List;
 /** Create DDSpanDecorators */
 public class DDDecoratorsFactory {
   public static List<AbstractDecorator> createBuiltinDecorators() {
-    final HTTPComponent httpDecorator = new HTTPComponent();
-    httpDecorator.setMatchingTag("component");
-    httpDecorator.setMatchingValue("java-aws-sdk");
 
     return Arrays.asList(
         new DBStatementAsResourceName(),
         new DBTypeDecorator(),
         new ErrorFlag(),
-        httpDecorator,
         new OperationDecorator(),
         new PeerServiceDecorator(),
         new ResourceNameDecorator(),

dd-trace-ot/src/main/java/datadog/opentracing/decorators/HTTPComponent.java

@@ -1,29 +0,0 @@
-package datadog.opentracing.decorators;
-
-import datadog.opentracing.DDSpanContext;
-import datadog.trace.api.DDTags;
-import io.opentracing.tag.Tags;
-
-/**
- * This span decorator leverages HTTP tags. It allows the dev to define a custom service name and
- * retrieves some HTTP meta such as the request path
- */
-public class HTTPComponent extends AbstractDecorator {
-
-  public HTTPComponent() {
-    super();
-    this.setMatchingTag(Tags.COMPONENT.getKey());
-    this.setReplacementTag(DDTags.SERVICE_NAME);
-  }
-
-  @Override
-  public boolean shouldSetTag(final DDSpanContext context, final String tag, final Object value) {
-    if (getMatchingValue().equals(value)) {
-      // Assign service name
-      super.shouldSetTag(context, tag, value);
-      // Assign span type to WEB
-      context.setSpanType("web");
-    }
-    return true;
-  }
-}