misc cleanup

tylerbenson · tylerbenson · commit 2e6ff449762c · 2017-12-07T12:06:58.000-08:00
diff --git a/dd-java-agent-ittests/src/test/groovy/com/datadoghq/agent/integration/TestHttpServer.groovy b/dd-java-agent-ittests/src/test/groovy/com/datadoghq/agent/integration/TestHttpServer.groovy
@@ -1,21 +1,18 @@
 package com.datadoghq.agent.integration
 
-import io.opentracing.ActiveSpan;
-import io.opentracing.SpanContext;
-import io.opentracing.propagation.Format;
-import io.opentracing.propagation.TextMap;
-import io.opentracing.util.GlobalTracer;
-
-import ratpack.http.MutableHeaders
-import ratpack.http.Response
+import io.opentracing.ActiveSpan
+import io.opentracing.SpanContext
+import io.opentracing.propagation.Format
+import io.opentracing.propagation.TextMap
+import io.opentracing.util.GlobalTracer
 import ratpack.groovy.test.embed.GroovyEmbeddedApp
 import ratpack.handling.Context
 
 import static ratpack.groovy.test.embed.GroovyEmbeddedApp.ratpack
 
 /**
  * A simple http server used for testing.<br>
- * Binds locally to {@link #PORT}.
+ * Binds locally to {@link #port}.
  *
  * <p>To start: {@link #startServer()}<br>
  * to stop: {@link #stopServer()}
@@ -27,24 +24,24 @@ class TestHttpServer {
    */
   public static final String IS_DD_SERVER = "is-dd-server"
 
-  private static GroovyEmbeddedApp INSTANCE = null
-  private static int PORT = 0
+  private static GroovyEmbeddedApp server = null
+  private static int port = 0
 
   static int getPort() {
-    if (PORT == 0) {
+    if (port == 0) {
       throw new RuntimeException("Server not started")
     }
-    return PORT;
+    return port
   }
 
   /**
    * Start the server. Has no effect if already started.
    *
    * @throws IOException
    */
-  static synchronized def startServer() {
-    if (null == INSTANCE) {
-      INSTANCE = ratpack {
+  static synchronized startServer() {
+    if (null == server) {
+      server = ratpack {
         handlers {
           get {
             String msg = "<html><body><h1>Hello test.</h1>\n"
@@ -55,46 +52,46 @@ class TestHttpServer {
             if (isDDServer) {
               final SpanContext extractedContext =
                 GlobalTracer.get()
-                .extract(Format.Builtin.HTTP_HEADERS, new RatpackResponseAdapter(context))
+                  .extract(Format.Builtin.HTTP_HEADERS, new RatpackResponseAdapter(context))
               ActiveSpan span =
                 GlobalTracer.get()
-                .buildSpan("test-http-server")
-                .asChildOf(extractedContext)
-                .startActive()
+                  .buildSpan("test-http-server")
+                  .asChildOf(extractedContext)
+                  .startActive()
               span.deactivate()
             }
 
             response.status(200).send(msg)
           }
         }
       }
-      PORT = INSTANCE.address.port
+      port = server.address.port
     }
   }
 
   /** Stop the server. Has no effect if already stopped. */
   static synchronized void stopServer() {
-    if (null != INSTANCE) {
-      INSTANCE.close()
-      INSTANCE = null
-      PORT = 0
+    if (null != server) {
+      server.close()
+      server = null
+      port = 0
     }
   }
 
   private static class RatpackResponseAdapter implements TextMap {
-    final Context context;
+    final Context context
 
-    public RatpackResponseAdapter(Context context) {
+    RatpackResponseAdapter(Context context) {
       this.context = context
     }
 
     @Override
-    public void put(String key, String value) {
+    void put(String key, String value) {
       context.response.set(key, value)
     }
 
     @Override
-    public Iterator<Map.Entry<String, String>> iterator() {
+    Iterator<Map.Entry<String, String>> iterator() {
       return context.request.getHeaders().asMultiValueMap().entrySet().iterator()
     }
   }
diff --git a/dd-java-agent-ittests/src/test/groovy/com/datadoghq/agent/integration/httpclient/ApacheHttpClientTest.groovy b/dd-java-agent-ittests/src/test/groovy/com/datadoghq/agent/integration/httpclient/ApacheHttpClientTest.groovy
@@ -17,9 +17,9 @@ import spock.lang.Specification
 class ApacheHttpClientTest extends Specification {
 
   @Shared
-  def ListWriter writer = new ListWriter()
+  def writer = new ListWriter()
   @Shared
-  def DDTracer tracer = new DDTracer(writer)
+  def tracer = new DDTracer(writer)
 
   def setupSpec() {
     TestUtils.registerOrReplaceGlobalTracer(tracer)
diff --git a/dd-java-agent/integrations/jms-2/src/test/groovy/JMS2Test.groovy b/dd-java-agent/integrations/jms-2/src/test/groovy/JMS2Test.groovy
@@ -156,7 +156,7 @@ class JMS2Test extends Specification {
     def producer = session.createProducer(destination)
     def consumer = session.createConsumer(destination)
     consumer.setMessageListener { message ->
-      Thread.sleep(5) // Slow things down a bit.
+      Thread.sleep(10) // Slow things down a bit.
       messageRef.set(message)
     }
 
diff --git a/dd-java-agent/integrations/okhttp-3/src/main/java/dd/inst/okhttp3/OkHttp3Instrumentation.java b/dd-java-agent/integrations/okhttp-3/src/main/java/dd/inst/okhttp3/OkHttp3Instrumentation.java
@@ -14,6 +14,7 @@
 import java.util.Collections;
 import net.bytebuddy.agent.builder.AgentBuilder;
 import net.bytebuddy.asm.Advice;
+import okhttp3.Interceptor;
 import okhttp3.OkHttpClient;
 
 @AutoService(Instrumenter.class)
@@ -38,6 +39,11 @@ public static class OkHttp3Advice {
     @Advice.OnMethodEnter(suppress = Throwable.class)
     public static void addTracingInterceptor(
         @Advice.Argument(0) final OkHttpClient.Builder builder) {
+      for (final Interceptor interceptor : builder.interceptors()) {
+        if (interceptor instanceof TracingInterceptor) {
+          return;
+        }
+      }
       final TracingInterceptor interceptor =
           new TracingInterceptor(GlobalTracer.get(), Collections.singletonList(STANDARD_TAGS));
       builder.addInterceptor(interceptor);
diff --git a/dd-java-agent/integrations/okhttp-3/src/test/groovy/OkHttp3Test.groovy b/dd-java-agent/integrations/okhttp-3/src/test/groovy/OkHttp3Test.groovy
@@ -40,10 +40,10 @@ class OkHttp3Test extends Specification {
         }
       }
     }
-    def client = new OkHttpClient();
+    def client = new OkHttpClient()
     def request = new Request.Builder()
       .url("http://localhost:$server.address.port/ping")
-      .build();
+      .build()
 
     def response = client.newCall(request).execute()
 
diff --git a/dd-java-agent/tooling/src/test/groovy/dd/test/TestUtils.groovy b/dd-java-agent/tooling/src/test/groovy/dd/test/TestUtils.groovy
@@ -50,11 +50,11 @@ class TestUtils {
   }
 
   static runUnderTrace(final String rootOperationName, Callable r) {
-    ActiveSpan rootSpan = GlobalTracer.get().buildSpan(rootOperationName).startActive();
+    ActiveSpan rootSpan = GlobalTracer.get().buildSpan(rootOperationName).startActive()
     try {
-      return r.call();
+      return r.call()
     } finally {
-      rootSpan.deactivate();
+      rootSpan.deactivate()
     }
   }
 }

Original file line number	Diff line number	Diff line change
`@@ -156,7 +156,7 @@ class JMS2Test extends Specification {`
`156`	`156`	`def producer = session.createProducer(destination)`
`157`	`157`	`def consumer = session.createConsumer(destination)`
`158`	`158`	`consumer.setMessageListener { message ->`
`159`		`- Thread.sleep(5) // Slow things down a bit.`
	`159`	`+ Thread.sleep(10) // Slow things down a bit.`
`160`	`160`	`messageRef.set(message)`
`161`	`161`	`}`
`162`	`162`
Original file line number	Diff line number	Diff line change
`@@ -40,10 +40,10 @@ class OkHttp3Test extends Specification {`
`40`	`40`	`}`
`41`	`41`	`}`
`42`	`42`	`}`
`43`		`- def client = new OkHttpClient();`
	`43`	`+ def client = new OkHttpClient()`
`44`	`44`	`def request = new Request.Builder()`
`45`	`45`	`.url("http://localhost:$server.address.port/ping")`
`46`		`- .build();`
	`46`	`+ .build()`
`47`	`47`
`48`	`48`	`def response = client.newCall(request).execute()`
`49`	`49`
Original file line number	Diff line number	Diff line change
`@@ -50,11 +50,11 @@ class TestUtils {`
`50`	`50`	`}`
`51`	`51`
`52`	`52`	`static runUnderTrace(final String rootOperationName, Callable r) {`
`53`		`- ActiveSpan rootSpan = GlobalTracer.get().buildSpan(rootOperationName).startActive();`
	`53`	`+ ActiveSpan rootSpan = GlobalTracer.get().buildSpan(rootOperationName).startActive()`
`54`	`54`	`try {`
`55`		`- return r.call();`
	`55`	`+ return r.call()`
`56`	`56`	`} finally {`
`57`		`- rootSpan.deactivate();`
	`57`	`+ rootSpan.deactivate()`
`58`	`58`	`}`
`59`	`59`	`}`
`60`	`60`	`}`