Merge pull request #431 from DataDog/gary/okhttp-log

Remove Util Logger and Refactor test

dd-java-agent/instrumentation/okhttp-3/src/main/java/datadog/trace/instrumentation/okhttp3/OkHttpClientSpanDecorator.java

@@ -4,8 +4,8 @@ import static io.opentracing.log.Fields.ERROR_OBJECT;
 
 import io.opentracing.Span;
 import io.opentracing.tag.Tags;
-import java.net.Inet4Address;
+import java.net.Inet6Address;
-import java.nio.ByteBuffer;
+import java.net.InetAddress;
 import java.util.Collections;
 import okhttp3.Connection;
 import okhttp3.Request;
@@ -71,16 +71,17 @@ public interface OkHttpClientSpanDecorator {
         @Override
         public void onResponse(
             final Connection connection, final Response response, final Span span) {
+          final InetAddress inetAddress = connection.socket().getInetAddress();
+
           Tags.HTTP_STATUS.set(span, response.code());
-          Tags.PEER_HOSTNAME.set(span, connection.socket().getInetAddress().getHostName());
+          Tags.PEER_HOSTNAME.set(span, inetAddress.getHostName());
           Tags.PEER_PORT.set(span, connection.socket().getPort());
 
-          if (connection.socket().getInetAddress() instanceof Inet4Address) {
+          String ipvKey = Tags.PEER_HOST_IPV4.getKey();
-            final byte[] address = connection.socket().getInetAddress().getAddress();
+          if (inetAddress instanceof Inet6Address) {
-            Tags.PEER_HOST_IPV4.set(span, ByteBuffer.wrap(address).getInt());
+            ipvKey = Tags.PEER_HOST_IPV6.getKey();
-          } else {
-            Tags.PEER_HOST_IPV6.set(span, connection.socket().getInetAddress().toString());
           }
+          span.setTag(ipvKey, inetAddress.getHostAddress());
         }
       };
 }

dd-java-agent/instrumentation/okhttp-3/src/main/java/datadog/trace/instrumentation/okhttp3/TracingInterceptor.java

@@ -12,7 +12,7 @@ import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.Executors;
-import java.util.logging.Logger;
+import lombok.extern.slf4j.Slf4j;
 import okhttp3.Dispatcher;
 import okhttp3.Interceptor;
 import okhttp3.OkHttpClient;
@@ -33,8 +33,8 @@ import okhttp3.Response;
  *
  * @author Pavol Loffay
  */
+@Slf4j
 public class TracingInterceptor implements Interceptor {
-  private static final Logger log = Logger.getLogger(TracingInterceptor.class.getName());
 
   private final Tracer tracer;
   private final List<OkHttpClientSpanDecorator> decorators;
@@ -120,7 +120,7 @@ public class TracingInterceptor implements Interceptor {
                     tracer, tagWrapper.getSpan().context(), decorators)
                 .intercept(chain);
       } else {
-        log.severe("tag is null or not an instance of TagWrapper, skipping decorator onResponse()");
+        log.error("tag is null or not an instance of TagWrapper, skipping decorator onResponse()");
       }
     }
 

dd-java-agent/instrumentation/okhttp-3/src/test/groovy/OkHttp3Test.groovy

@@ -1,6 +1,5 @@
 import datadog.trace.agent.test.AgentTestRunner
 import datadog.trace.api.DDSpanTypes
-import datadog.trace.api.DDTags
 import io.opentracing.tag.Tags
 import okhttp3.OkHttpClient
 import okhttp3.Request
@@ -8,6 +7,7 @@ import ratpack.http.Headers
 
 import java.util.concurrent.atomic.AtomicReference
 
+import static datadog.trace.agent.test.ListWriterAssert.assertTraces
 import static ratpack.groovy.test.embed.GroovyEmbeddedApp.ratpack
 
 class OkHttp3Test extends AgentTestRunner {
@@ -32,55 +32,45 @@ class OkHttp3Test extends AgentTestRunner {
 
     expect:
     response.body.string() == "pong"
-    TEST_WRITER.size() == 1
+    assertTraces(TEST_WRITER, 1) {
+      trace(0, 2) {
+        span(0) {
+          operationName "okhttp.http"
+          serviceName "okhttp"
+          resourceName "okhttp.http"
+          spanType DDSpanTypes.HTTP_CLIENT
+          errored false
+          parent()
+          tags {
+            "component" "okhttp"
+            "span.type" DDSpanTypes.HTTP_CLIENT
+            defaultTags()
+          }
+        }
+        span(1) {
+          operationName "okhttp.http"
+          serviceName "okhttp"
+          resourceName "GET /ping"
+          errored false
+          childOf(span(0))
+          tags {
+            defaultTags()
+            "component" "okhttp"
+            "span.type" DDSpanTypes.HTTP_CLIENT
+            "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_CLIENT
+            "$Tags.HTTP_METHOD.key" "GET"
+            "$Tags.HTTP_STATUS.key" 200
+            "$Tags.HTTP_URL.key" "http://localhost:$server.address.port/ping"
+            "$Tags.PEER_HOSTNAME.key" "localhost"
+            "$Tags.PEER_PORT.key" server.address.port
+            "$Tags.PEER_HOST_IPV4.key" "127.0.0.1"
+          }
+        }
+      }
+    }
 
-    def trace = TEST_WRITER.firstTrace()
+    receivedHeaders.get().get("x-datadog-trace-id") == TEST_WRITER[0][1].traceId
-    trace.size() == 2
+    receivedHeaders.get().get("x-datadog-parent-id") == TEST_WRITER[0][1].spanId
-
-    and: // span 0
-    def span1 = trace[0]
-
-    span1.context().operationName == "okhttp.http"
-    span1.serviceName == "okhttp"
-    span1.resourceName == "okhttp.http"
-    span1.type == DDSpanTypes.HTTP_CLIENT
-    !span1.context().getErrorFlag()
-    span1.context().parentId == "0"
-
-
-    def tags1 = span1.context().tags
-    tags1["component"] == "okhttp"
-    tags1["span.type"] == DDSpanTypes.HTTP_CLIENT
-    tags1["thread.name"] != null
-    tags1["thread.id"] != null
-    tags1.size() == 4
-
-    and: // span 1
-    def span2 = trace[1]
-
-    span2.context().operationName == "okhttp.http"
-    span2.serviceName == "okhttp"
-    span2.resourceName == "GET /ping"
-    span2.type == DDSpanTypes.HTTP_CLIENT
-    !span2.context().getErrorFlag()
-    span2.context().parentId == span1.spanId
-
-
-    def tags2 = span2.context().tags
-    tags2[Tags.COMPONENT.key] == "okhttp"
-    tags2[Tags.SPAN_KIND.key] == Tags.SPAN_KIND_CLIENT
-    tags2[DDTags.SPAN_TYPE] == DDSpanTypes.HTTP_CLIENT
-    tags2[Tags.HTTP_METHOD.key] == "GET"
-    tags2[Tags.HTTP_URL.key] == "http://localhost:$server.address.port/ping"
-    tags2[Tags.PEER_HOSTNAME.key] == "localhost"
-    tags2[Tags.PEER_PORT.key] == server.address.port
-    tags2[Tags.PEER_HOST_IPV4.key] != null
-    tags2[DDTags.THREAD_NAME] != null
-    tags2[DDTags.THREAD_ID] != null
-    tags2.size() == 11
-
-    receivedHeaders.get().get("x-datadog-trace-id") == "$span2.traceId"
-    receivedHeaders.get().get("x-datadog-parent-id") == "$span2.spanId"
 
     cleanup:
     server.close()