Improve comments and add synchronization.

dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/HttpUrlConnectionInstrumentation.java

@@ -75,13 +75,11 @@ public class HttpUrlConnectionInstrumentation extends Instrumenter.Default {
           if (connected) {
             return null;
           }
-          // We get here in cases where connect() is called first.
+          /*
-          // We need to inject headers now because after connected=true no more headers can be
+           * Ideally, we would like to only have a single span for each of the output and input streams,
-          // added.
+           * but since headers are also sent on connect(), there wouldn't be a span to mark as parent if
-
+           * we don't create a span here.
-          // In total there will be two spans:
+           */
-          // - one for the connect() which does propagation
-          // - one for the input or output stream (presumably called after connect())
           operationName += ".connect";
           break;
 
@@ -102,10 +100,11 @@ public class HttpUrlConnectionInstrumentation extends Instrumenter.Default {
           break;
       }
 
-      // AgentWriter uses HttpURLConnection to report to the trace-agent. We don't want to trace
+      /*
-      // those requests.
+       * AgentWriter uses HttpURLConnection to report to the trace-agent. We don't want to trace
-      // Check after the connected test above because getRequestProperty will throw an exception if
+       * those requests. Check after the connected test above because getRequestProperty will
-      // already connected.
+       * throw an exception if already connected.
+       */
       final boolean isTraceRequest =
           Thread.currentThread().getName().equals("dd-agent-writer")
               || (!connected && thiz.getRequestProperty("Datadog-Meta-Lang") != null);
@@ -115,7 +114,7 @@ public class HttpUrlConnectionInstrumentation extends Instrumenter.Default {
 
       final Tracer tracer = GlobalTracer.get();
       if (tracer.activeSpan() == null) {
-        // httpurlconnection doesn't play nicely with top-level spans
+        // We don't want this as a top level span.
         return null;
       }
 
@@ -163,9 +162,11 @@ public class HttpUrlConnectionInstrumentation extends Instrumenter.Default {
         Tags.ERROR.set(span, true);
         span.log(Collections.singletonMap(ERROR_OBJECT, throwable));
       } else if (responseCode > 0) {
-        // responseCode field cache is sometimes not populated.
+        /*
-        // We can't call getResponseCode() due to some unwanted side-effects (e.g. breaks
+         * responseCode field cache is sometimes not populated.
-        // getOutputStream).
+         * We can't call getResponseCode() due to some unwanted side-effects
+         * (e.g. breaks getOutputStream).
+         */
         Tags.HTTP_STATUS.set(span, responseCode);
       }
       scope.close();
@@ -180,10 +181,15 @@ public class HttpUrlConnectionInstrumentation extends Instrumenter.Default {
     public static HttpURLState get(final HttpURLConnection connection) {
       HttpURLState state = STATE_MAP.get(connection);
       if (state == null) {
-        // not thread-safe, but neither is HttpURLConnection
+        synchronized (connection) {
+          // might not be a good idea to synchronize on a method parameter...
+          state = STATE_MAP.get(connection);
+          if (state == null) {
             state = new HttpURLState();
             STATE_MAP.put(connection, state);
           }
+        }
+      }
       return state;
     }
 

dd-java-agent/instrumentation/http-url-connection/src/main/java/datadog/trace/instrumentation/http_url_connection/SunHttpClientInstrumentation.java

@@ -64,8 +64,6 @@ public class SunHttpClientInstrumentation extends Instrumenter.Default {
         @Advice.Argument(0) final MessageHeader header, @Advice.This final HttpClient client) {
       final Tracer tracer = GlobalTracer.get();
       final Span span = tracer.activeSpan();
-      // Use the active span, not the one created in SpanStartAdvice,
-      // since we aren't sure that span will complete.
 
       if (span != null) {
         tracer.inject(