From 6f05cff0237b287d4408c45537e47070fb3c5f94 Mon Sep 17 00:00:00 2001
From: Nikolay Martynov <mar.kolya@gmail.com>
Date: Thu, 15 Nov 2018 12:31:42 -0500
Subject: [PATCH 1/2] Add test for UrlConnection error on file protocol

---
 .../UrlInstrumentation.java                   |  4 +-
 .../src/test/groovy/UrlConnectionTest.groovy  | 44 +++++++++++++++++++
 2 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/UrlInstrumentation.java b/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/UrlInstrumentation.java
index 3b95ffabaee..397a49e94b0 100644
--- a/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/UrlInstrumentation.java
+++ b/dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/UrlInstrumentation.java
@@ -25,6 +25,8 @@
 @AutoService(Instrumenter.class)
 public class UrlInstrumentation extends Instrumenter.Default {
 
+  public static final String COMPONENT = "UrlConnection";
+
   public UrlInstrumentation() {
     super("urlconnection", "httpurlconnection");
   }
@@ -67,7 +69,7 @@ public static void errorSpan(
                 .buildSpan(protocol + ".request")
                 .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_CLIENT)
                 .withTag(DDTags.SPAN_TYPE, DDSpanTypes.HTTP_CLIENT)
-                .withTag(Tags.COMPONENT.getKey(), "UrlConnection")
+                .withTag(Tags.COMPONENT.getKey(), COMPONENT)
                 .startActive(true);
 
         final Span span = scope.span();
diff --git a/dd-java-agent/instrumentation/http-url-connection/src/test/groovy/UrlConnectionTest.groovy b/dd-java-agent/instrumentation/http-url-connection/src/test/groovy/UrlConnectionTest.groovy
index 6d2462b77a3..83f3a410e74 100644
--- a/dd-java-agent/instrumentation/http-url-connection/src/test/groovy/UrlConnectionTest.groovy
+++ b/dd-java-agent/instrumentation/http-url-connection/src/test/groovy/UrlConnectionTest.groovy
@@ -2,6 +2,7 @@ import datadog.trace.agent.test.AgentTestRunner
 import datadog.trace.agent.test.TestUtils
 import datadog.trace.api.DDSpanTypes
 import datadog.trace.api.DDTags
+import datadog.trace.instrumentation.http_url_connection.UrlInstrumentation
 import io.opentracing.tag.Tags
 import io.opentracing.util.GlobalTracer
 
@@ -65,4 +66,47 @@ class UrlConnectionTest extends AgentTestRunner {
 
     url = new URI("$scheme://localhost:$INVALID_PORT").toURL()
   }
+
+  def "trace request with connection failure to a local file with broken url path"() {
+    setup:
+    def url = new URI("file:/some-random-file%abc").toURL()
+
+    when:
+    runUnderTrace("someTrace") {
+      url.openConnection()
+    }
+
+    then:
+    thrown IllegalArgumentException
+
+    expect:
+    assertTraces(1) {
+      trace(0, 2) {
+        span(0) {
+          operationName "someTrace"
+          parent()
+          errored true
+          tags {
+            errorTags IllegalArgumentException, String
+            defaultTags()
+          }
+        }
+        span(1) {
+          operationName "file.request"
+          childOf span(0)
+          errored true
+          tags {
+            "$Tags.COMPONENT.key" UrlInstrumentation.COMPONENT
+            "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
+            // FIXME: These tags really make no sense for non-http connections, why do we set them?
+            "$DDTags.SPAN_TYPE" DDSpanTypes.HTTP_CLIENT
+            "$Tags.HTTP_URL.key" "$url"
+            "$Tags.PEER_PORT.key" 80
+            errorTags IllegalArgumentException, String
+            defaultTags()
+          }
+        }
+      }
+    }
+  }
 }

From 465e2375222da7d25dbac893ca55b98a39d94f52 Mon Sep 17 00:00:00 2001
From: Nikolay Martynov <mar.kolya@gmail.com>
Date: Thu, 15 Nov 2018 12:32:11 -0500
Subject: [PATCH 2/2] Do not set component in AWS instrumentation that later
 get rewritten anyway

---
 .../trace/instrumentation/aws/v0/TracingRequestHandler.java | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

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 8f9be7fc60e..cf3d3d87637 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
@@ -56,11 +56,9 @@ public AmazonWebServiceRequest beforeMarshalling(final AmazonWebServiceRequest r
   /** {@inheritDoc} */
   @Override
   public void beforeRequest(final Request<?> request) {
+    // Note: not setting Component tag here because it is always set by SpanDecorator
     final Tracer.SpanBuilder spanBuilder =
-        tracer
-            .buildSpan("aws.command")
-            .withTag(Tags.COMPONENT.getKey(), "aws-sdk")
-            .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_CLIENT);
+        tracer.buildSpan("aws.command").withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_CLIENT);
 
     if (parentContext != null) {
       spanBuilder.asChildOf(parentContext);