From d48eb4de777ba30edcb168bf6d2bcbd9bd2724ac Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Wed, 26 Sep 2018 12:55:18 -0400 Subject: [PATCH 1/4] Decouple Groovy version from Gradle And use newer Groovy and Spock --- .../classloading/ClassLoadingTest.groovy | 17 +++++++++-------- .../src/test/groovy/GrpcStreamingTest.groovy | 2 +- gradle/dependencies.gradle | 6 +++--- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/dd-java-agent-ittests/src/test/groovy/datadog/trace/agent/integration/classloading/ClassLoadingTest.groovy b/dd-java-agent-ittests/src/test/groovy/datadog/trace/agent/integration/classloading/ClassLoadingTest.groovy index 46b03e77f1..781248df2e 100644 --- a/dd-java-agent-ittests/src/test/groovy/datadog/trace/agent/integration/classloading/ClassLoadingTest.groovy +++ b/dd-java-agent-ittests/src/test/groovy/datadog/trace/agent/integration/classloading/ClassLoadingTest.groovy @@ -17,7 +17,7 @@ class ClassLoadingTest extends Specification { /** Assert that we can instrument classloaders which cannot resolve agent advice classes. */ def "instrument classloader without agent classes"() { setup: - final URLClassLoader loader = new URLClassLoader(classpath, (ClassLoader) null) + URLClassLoader loader = new URLClassLoader(classpath, (ClassLoader) null) when: loader.loadClass("datadog.agent.TracingAgent") @@ -25,15 +25,15 @@ class ClassLoadingTest extends Specification { thrown ClassNotFoundException when: - final Class instrumentedClass = loader.loadClass(ClassToInstrument.getName()) + Class instrumentedClass = loader.loadClass(ClassToInstrument.getName()) then: instrumentedClass.getClassLoader() == loader } def "make sure ByteBuddy does not hold strong references to ClassLoader"() { setup: - final URLClassLoader loader = new URLClassLoader(classpath, (ClassLoader) null) - final WeakReference ref = new WeakReference<>(loader) + URLClassLoader loader = new URLClassLoader(classpath, (ClassLoader) null) + WeakReference ref = new WeakReference<>(loader) when: loader.loadClass(ClassToInstrument.getName()) @@ -45,7 +45,7 @@ class ClassLoadingTest extends Specification { null == ref.get() } - // We are doing this because Grovy cannot properly resolve constructor argument types in anonymous classes + // We are doing this because Groovy cannot properly resolve constructor argument types in anonymous classes static class CountingClassLoader extends URLClassLoader { public int count = 0 @@ -62,9 +62,10 @@ class ClassLoadingTest extends Specification { def "make sure that ByteBuddy reads classes's bytes only once"() { setup: - final CountingClassLoader loader = new CountingClassLoader(classpath) + CountingClassLoader loader = new CountingClassLoader(classpath) when: + //loader.loadClass("aaa") loader.loadClass(ClassToInstrument.getName()) int countAfterFirstLoad = loader.count loader.loadClass(ClassToInstrumentChild.getName()) @@ -77,8 +78,8 @@ class ClassLoadingTest extends Specification { def "make sure that ByteBuddy doesn't resue cached type descriptions between different classloaders"() { setup: - final CountingClassLoader loader1 = new CountingClassLoader(classpath) - final CountingClassLoader loader2 = new CountingClassLoader(classpath) + CountingClassLoader loader1 = new CountingClassLoader(classpath) + CountingClassLoader loader2 = new CountingClassLoader(classpath) when: loader1.loadClass(ClassToInstrument.getName()) diff --git a/dd-java-agent/instrumentation/grpc-1.5/src/test/groovy/GrpcStreamingTest.groovy b/dd-java-agent/instrumentation/grpc-1.5/src/test/groovy/GrpcStreamingTest.groovy index 554712e73d..2b152190b1 100644 --- a/dd-java-agent/instrumentation/grpc-1.5/src/test/groovy/GrpcStreamingTest.groovy +++ b/dd-java-agent/instrumentation/grpc-1.5/src/test/groovy/GrpcStreamingTest.groovy @@ -24,7 +24,7 @@ class GrpcStreamingTest extends AgentTestRunner { def "test conversation #name"() { setup: - final msgCount = serverMessageCount + def msgCount = serverMessageCount def serverReceived = new CopyOnWriteArrayList<>() def clientReceived = new CopyOnWriteArrayList<>() def error = new AtomicReference() diff --git a/gradle/dependencies.gradle b/gradle/dependencies.gradle index 4b5cd30343..91fbf855b3 100644 --- a/gradle/dependencies.gradle +++ b/gradle/dependencies.gradle @@ -1,5 +1,5 @@ -def groovyVer = GroovySystem.version -def spockGroovyVer = GroovySystem.version.replaceAll(/\.\d+$/, '') +def groovyVer = "2.5.2" +def spockGroovyVer = groovyVer.replaceAll(/\.\d+$/, '') ext { versions = [ @@ -10,7 +10,7 @@ ext { jackson : "2.6.3", // This is a transitive dependency for the tracer. // Use an old version to not force an upgrade for others using tracer as a dependency. - spock : "1.1-groovy-$spockGroovyVer", + spock : "1.2-groovy-$spockGroovyVer", groovy : groovyVer, junit : "4.12", logback : "1.2.3", From 44ea493f531738f9a7aba740ab8aa88371bbab79 Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Wed, 26 Sep 2018 12:55:36 -0400 Subject: [PATCH 2/4] Download gradle source to make idea more helpful --- gradle/wrapper/gradle-wrapper.properties | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index 5c1b6c95b8..154557e47f 100644 --- a/gradle/wrapper/gradle-wrapper.properties +++ b/gradle/wrapper/gradle-wrapper.properties @@ -1,5 +1,6 @@ +#Wed Sep 26 11:49:09 EDT 2018 distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists -distributionUrl=https\://services.gradle.org/distributions/gradle-4.10.1-bin.zip zipStoreBase=GRADLE_USER_HOME zipStorePath=wrapper/dists +distributionUrl=https\://services.gradle.org/distributions/gradle-4.10.1-all.zip From 84bef236981f5e7415c5f472c83c99fab4a7607e Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Wed, 26 Sep 2018 12:55:57 -0400 Subject: [PATCH 3/4] Increase timeouts in Couchbase to make tests more stable --- .../src/test/groovy/util/AbstractCouchbaseTest.groovy | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/dd-java-agent/instrumentation/couchbase-2.0/src/test/groovy/util/AbstractCouchbaseTest.groovy b/dd-java-agent/instrumentation/couchbase-2.0/src/test/groovy/util/AbstractCouchbaseTest.groovy index edf8042b22..95c1324c87 100644 --- a/dd-java-agent/instrumentation/couchbase-2.0/src/test/groovy/util/AbstractCouchbaseTest.groovy +++ b/dd-java-agent/instrumentation/couchbase-2.0/src/test/groovy/util/AbstractCouchbaseTest.groovy @@ -115,7 +115,7 @@ abstract class AbstractCouchbaseTest extends AgentTestRunner { } private DefaultCouchbaseEnvironment.Builder envBuilder(BucketSettings bucketSettings) { - def timeout = TimeUnit.SECONDS.toMillis(5) + def timeout = TimeUnit.SECONDS.toMillis(10) return DefaultCouchbaseEnvironment.builder() .bootstrapCarrierDirectPort(mock.getCarrierPort(bucketSettings.name())) .bootstrapHttpDirectPort(port) @@ -124,9 +124,14 @@ abstract class AbstractCouchbaseTest extends AgentTestRunner { .networkLatencyMetricsCollectorConfig(DefaultLatencyMetricsCollectorConfig.create(0, TimeUnit.DAYS)) .computationPoolSize(1) .connectTimeout(timeout) + .disconnectTimeout(timeout) .kvTimeout(timeout) .managementTimeout(timeout) .queryTimeout(timeout) .viewTimeout(timeout) + .keepAliveTimeout(timeout) + .searchTimeout(timeout) + .analyticsTimeout(timeout) + .socketConnectTimeout(timeout.intValue()) } } From 6d5b06a2922375a6abd065621a278ae7575c6ad0 Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Wed, 26 Sep 2018 13:10:46 -0400 Subject: [PATCH 4/4] Upgrade gradle to 4.10.2 --- gradle/wrapper/gradle-wrapper.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index 154557e47f..63811edbbe 100644 --- a/gradle/wrapper/gradle-wrapper.properties +++ b/gradle/wrapper/gradle-wrapper.properties @@ -3,4 +3,4 @@ distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists zipStoreBase=GRADLE_USER_HOME zipStorePath=wrapper/dists -distributionUrl=https\://services.gradle.org/distributions/gradle-4.10.1-all.zip +distributionUrl=https\://services.gradle.org/distributions/gradle-4.10.2-all.zip