From e69edaec1451fac736d737e7e80f8b074f3b5b58 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Wed, 21 Aug 2019 16:20:22 -0700 Subject: [PATCH 1/3] Remove project config centrally defined and other misc cleanup --- buildSrc/src/main/groovy/MuzzlePlugin.groovy | 1 + .../agent-tooling/agent-tooling.gradle | 5 ++- .../akka-http-10.0/akka-http-10.0.gradle | 4 --- .../aws-java-sdk-2.2/aws-java-sdk-2.2.gradle | 20 ----------- .../instrumentation/instrumentation.gradle | 12 +++++++ .../akka-2.5-testing/akka-2.5-testing.gradle | 1 - .../akka-testing/akka-testing.gradle | 1 - .../java-concurrent/java-concurrent.gradle | 1 - .../kotlin-testing/kotlin-testing.gradle | 4 +-- .../scala-testing/scala-testing.gradle | 1 - .../lettuce-5/lettuce-5.gradle | 23 +----------- .../instrumentation/mongo/mongo.gradle | 4 --- .../ratpack-1.4/ratpack-1.4.gradle | 35 ------------------ .../reactor-core-3.1/reactor-core-3.1.gradle | 36 ------------------- .../spring-webflux/spring-webflux.gradle | 34 ------------------ dd-trace-java.gradle | 2 +- gradle/java.gradle | 34 +++++++++++++++++- 17 files changed, 53 insertions(+), 165 deletions(-) diff --git a/buildSrc/src/main/groovy/MuzzlePlugin.groovy b/buildSrc/src/main/groovy/MuzzlePlugin.groovy index 6112bdf5ee..00c3cd1001 100644 --- a/buildSrc/src/main/groovy/MuzzlePlugin.groovy +++ b/buildSrc/src/main/groovy/MuzzlePlugin.groovy @@ -335,6 +335,7 @@ class MuzzlePlugin implements Plugin { version.contains("beta") || version.contains("-b") || version.contains(".m") || + version.contains("-m") || version.contains("-dev") || version.contains("public_draft") } diff --git a/dd-java-agent/agent-tooling/agent-tooling.gradle b/dd-java-agent/agent-tooling/agent-tooling.gradle index 50e6ce3b6f..bef4b949c7 100644 --- a/dd-java-agent/agent-tooling/agent-tooling.gradle +++ b/dd-java-agent/agent-tooling/agent-tooling.gradle @@ -9,7 +9,10 @@ configurations { } dependencies { - compile project(':dd-java-agent:agent-bootstrap') + compile(project(':dd-java-agent:agent-bootstrap')) { + // This only needs to exist in the bootstrap, not the instrumentation jar. + exclude group: 'org.slf4j', module: 'slf4j-simple' + } compile group: 'com.blogspot.mydailyjava', name: 'weak-lock-free', version: '0.15' compile deps.bytebuddy compile deps.bytebuddyagent diff --git a/dd-java-agent/instrumentation/akka-http-10.0/akka-http-10.0.gradle b/dd-java-agent/instrumentation/akka-http-10.0/akka-http-10.0.gradle index da3d24b604..ff797058e6 100644 --- a/dd-java-agent/instrumentation/akka-http-10.0/akka-http-10.0.gradle +++ b/dd-java-agent/instrumentation/akka-http-10.0/akka-http-10.0.gradle @@ -79,12 +79,10 @@ dependencies { annotationProcessor deps.autoservice testCompile group: 'com.typesafe.akka', name: 'akka-http_2.11', version: '10.0.0' - testCompile project(':dd-java-agent:testing') testCompile group: 'com.squareup.okhttp3', name: 'okhttp', version: '3.6.0' testCompile project(':dd-java-agent:instrumentation:java-concurrent') testCompile project(':dd-java-agent:instrumentation:trace-annotation') - lagomTestCompile project(':dd-java-agent:testing') lagomTestCompile project(':dd-java-agent:instrumentation:akka-http-10.0') lagomTestCompile project(':dd-java-agent:instrumentation:trace-annotation') lagomTestCompile project(':dd-java-agent:instrumentation:java-concurrent') @@ -94,13 +92,11 @@ dependencies { // There are some internal API changes in 10.1 that we would like to test separately for version101TestCompile group: 'com.typesafe.akka', name: 'akka-http_2.11', version: '10.1.0' version101TestCompile group: 'com.typesafe.akka', name: 'akka-stream_2.11', version: '2.5.11' - version101TestCompile project(':dd-java-agent:testing') version101TestCompile project(':dd-java-agent:instrumentation:java-concurrent') version101TestCompile project(':dd-java-agent:instrumentation:trace-annotation') latestDepTestCompile group: 'com.typesafe.akka', name: 'akka-http_2.11', version: '+' latestDepTestCompile group: 'com.typesafe.akka', name: 'akka-stream_2.11', version: '+' - latestDepTestCompile project(':dd-java-agent:testing') latestDepTestCompile project(':dd-java-agent:instrumentation:java-concurrent') latestDepTestCompile project(':dd-java-agent:instrumentation:trace-annotation') } diff --git a/dd-java-agent/instrumentation/aws-java-sdk-2.2/aws-java-sdk-2.2.gradle b/dd-java-agent/instrumentation/aws-java-sdk-2.2/aws-java-sdk-2.2.gradle index acb3ed994a..3a98d56cf5 100644 --- a/dd-java-agent/instrumentation/aws-java-sdk-2.2/aws-java-sdk-2.2.gradle +++ b/dd-java-agent/instrumentation/aws-java-sdk-2.2/aws-java-sdk-2.2.gradle @@ -19,28 +19,9 @@ testSets { latestDepTest } -sourceSets { - main_java8 { - java.srcDirs "${project.projectDir}/src/main/java8" - } -} - -compileMain_java8Java { - sourceCompatibility = 1.8 - targetCompatibility = 1.8 -} - dependencies { main_java8CompileOnly group: 'software.amazon.awssdk', name: 'aws-core', version: '2.2.0' - main_java8Compile project(':dd-java-agent:agent-tooling') - - main_java8Compile deps.bytebuddy - main_java8Compile deps.opentracing - - compileOnly sourceSets.main_java8.compileClasspath - compile sourceSets.main_java8.output - // Include httpclient instrumentation for testing because it is a dependency for aws-sdk. testCompile project(':dd-java-agent:instrumentation:apache-httpclient-4') // Also include netty instrumentation because it is used by aws async client @@ -52,7 +33,6 @@ dependencies { testCompile group: 'software.amazon.awssdk', name: 'rds', version: '2.2.0' testCompile group: 'software.amazon.awssdk', name: 'ec2', version: '2.2.0' - latestDepTestCompile project(':dd-java-agent:testing') latestDepTestCompile project(':dd-java-agent:instrumentation:apache-httpclient-4') latestDepTestCompile project(':dd-java-agent:instrumentation:netty-4.1') latestDepTestCompile project(':dd-java-agent:instrumentation:java-concurrent') diff --git a/dd-java-agent/instrumentation/instrumentation.gradle b/dd-java-agent/instrumentation/instrumentation.gradle index f08ac512f1..7725097754 100644 --- a/dd-java-agent/instrumentation/instrumentation.gradle +++ b/dd-java-agent/instrumentation/instrumentation.gradle @@ -37,12 +37,24 @@ subprojects {Project subProj -> } } + String jdkCompile = null + if (project.hasProperty('minJavaVersionForTests') && project.getProperty('minJavaVersionForTests') != JavaVersion.VERSION_1_7) { + def version = JavaVersion.toVersion(project.getProperty('minJavaVersionForTests')) + def name = "java$version.majorVersion" + jdkCompile = "main_${name}Compile" + } dependencies { // Apply common dependencies for instrumentation. compile project(':dd-trace-api') compile project(':dd-java-agent:agent-tooling') compile deps.bytebuddy compile deps.opentracing + if(jdkCompile) { + "$jdkCompile" project(':dd-trace-api') + "$jdkCompile" project(':dd-java-agent:agent-tooling') + "$jdkCompile" deps.bytebuddy + "$jdkCompile" deps.opentracing + } annotationProcessor deps.autoservice implementation deps.autoservice diff --git a/dd-java-agent/instrumentation/java-concurrent/akka-2.5-testing/akka-2.5-testing.gradle b/dd-java-agent/instrumentation/java-concurrent/akka-2.5-testing/akka-2.5-testing.gradle index f37737124e..ea3a871dfd 100644 --- a/dd-java-agent/instrumentation/java-concurrent/akka-2.5-testing/akka-2.5-testing.gradle +++ b/dd-java-agent/instrumentation/java-concurrent/akka-2.5-testing/akka-2.5-testing.gradle @@ -15,7 +15,6 @@ dependencies { testCompile deps.scala testCompile group: 'com.typesafe.akka', name: 'akka-actor_2.11', version: '2.5.0' - testCompile project(':dd-java-agent:testing') testCompile project(':dd-java-agent:instrumentation:java-concurrent') testCompile project(':dd-java-agent:instrumentation:trace-annotation') } diff --git a/dd-java-agent/instrumentation/java-concurrent/akka-testing/akka-testing.gradle b/dd-java-agent/instrumentation/java-concurrent/akka-testing/akka-testing.gradle index 9e2ea34a78..30cb8ba25d 100644 --- a/dd-java-agent/instrumentation/java-concurrent/akka-testing/akka-testing.gradle +++ b/dd-java-agent/instrumentation/java-concurrent/akka-testing/akka-testing.gradle @@ -15,7 +15,6 @@ dependencies { testCompile group: 'com.typesafe.akka', name: 'akka-actor_2.11', version: '2.3.16' testCompile group: 'com.typesafe.akka', name: 'akka-testkit_2.11', version: '2.3.16' - testCompile project(':dd-java-agent:testing') testCompile project(':dd-java-agent:instrumentation:java-concurrent') testCompile project(':dd-java-agent:instrumentation:trace-annotation') } diff --git a/dd-java-agent/instrumentation/java-concurrent/java-concurrent.gradle b/dd-java-agent/instrumentation/java-concurrent/java-concurrent.gradle index ec380b6b7e..507a782af5 100644 --- a/dd-java-agent/instrumentation/java-concurrent/java-concurrent.gradle +++ b/dd-java-agent/instrumentation/java-concurrent/java-concurrent.gradle @@ -27,7 +27,6 @@ dependencies { testCompile project(':dd-java-agent:instrumentation:trace-annotation') - slickTestCompile project(':dd-java-agent:testing') slickTestCompile project(':dd-java-agent:instrumentation:java-concurrent') slickTestCompile project(':dd-java-agent:instrumentation:trace-annotation') slickTestCompile project(':dd-java-agent:instrumentation:jdbc') diff --git a/dd-java-agent/instrumentation/java-concurrent/kotlin-testing/kotlin-testing.gradle b/dd-java-agent/instrumentation/java-concurrent/kotlin-testing/kotlin-testing.gradle index bbb025b62c..3a67069d72 100644 --- a/dd-java-agent/instrumentation/java-concurrent/kotlin-testing/kotlin-testing.gradle +++ b/dd-java-agent/instrumentation/java-concurrent/kotlin-testing/kotlin-testing.gradle @@ -3,12 +3,10 @@ apply from: "${rootDir}/gradle/test-with-kotlin.gradle" dependencies { testCompile project(':dd-trace-api') - - + testCompile deps.kotlin testCompile deps.coroutines - testCompile project(':dd-java-agent:testing') testCompile project(':dd-java-agent:instrumentation:java-concurrent') testCompile project(':dd-java-agent:instrumentation:trace-annotation') } diff --git a/dd-java-agent/instrumentation/java-concurrent/scala-testing/scala-testing.gradle b/dd-java-agent/instrumentation/java-concurrent/scala-testing/scala-testing.gradle index 084d541e1b..4c6a34c862 100644 --- a/dd-java-agent/instrumentation/java-concurrent/scala-testing/scala-testing.gradle +++ b/dd-java-agent/instrumentation/java-concurrent/scala-testing/scala-testing.gradle @@ -13,7 +13,6 @@ dependencies { testCompile project(':dd-trace-api') testCompile deps.scala - testCompile project(':dd-java-agent:testing') testCompile project(':dd-java-agent:instrumentation:java-concurrent') testCompile project(':dd-java-agent:instrumentation:trace-annotation') } diff --git a/dd-java-agent/instrumentation/lettuce-5/lettuce-5.gradle b/dd-java-agent/instrumentation/lettuce-5/lettuce-5.gradle index d249ed26cb..26b14e19bd 100644 --- a/dd-java-agent/instrumentation/lettuce-5/lettuce-5.gradle +++ b/dd-java-agent/instrumentation/lettuce-5/lettuce-5.gradle @@ -15,17 +15,6 @@ muzzle { apply from: "${rootDir}/gradle/java.gradle" -sourceSets { - main_java8 { - java.srcDirs "${project.projectDir}/src/main/java8" - } -} - -compileMain_java8Java { - sourceCompatibility = 1.8 - targetCompatibility = 1.8 -} - apply plugin: 'org.unbroken-dome.test-sets' testSets { @@ -35,18 +24,8 @@ testSets { } dependencies { - main_java8CompileOnly group: 'io.lettuce', name: 'lettuce-core', version: '5.0.0.RELEASE' - - main_java8Compile project(':dd-java-agent:agent-tooling') - - main_java8Compile deps.bytebuddy - main_java8Compile deps.opentracing - - compileOnly sourceSets.main_java8.compileClasspath - - compile sourceSets.main_java8.output - compileOnly group: 'io.lettuce', name: 'lettuce-core', version: '5.0.0.RELEASE' + main_java8CompileOnly group: 'io.lettuce', name: 'lettuce-core', version: '5.0.0.RELEASE' testCompile group: 'com.github.kstyrc', name: 'embedded-redis', version: '0.6' testCompile group: 'io.lettuce', name: 'lettuce-core', version: '5.0.0.RELEASE' diff --git a/dd-java-agent/instrumentation/mongo/mongo.gradle b/dd-java-agent/instrumentation/mongo/mongo.gradle index f91b66b0cc..612664bb7c 100644 --- a/dd-java-agent/instrumentation/mongo/mongo.gradle +++ b/dd-java-agent/instrumentation/mongo/mongo.gradle @@ -1,10 +1,6 @@ apply from: "${rootDir}/gradle/java.gradle" dependencies { - testAnnotationProcessor deps.autoservice - testImplementation deps.autoservice - - testCompile project(':dd-java-agent:testing') testCompile group: 'de.flapdoodle.embed', name: 'de.flapdoodle.embed.mongo', version: '1.50.5' } diff --git a/dd-java-agent/instrumentation/ratpack-1.4/ratpack-1.4.gradle b/dd-java-agent/instrumentation/ratpack-1.4/ratpack-1.4.gradle index 623a8161be..6bb358afb4 100644 --- a/dd-java-agent/instrumentation/ratpack-1.4/ratpack-1.4.gradle +++ b/dd-java-agent/instrumentation/ratpack-1.4/ratpack-1.4.gradle @@ -20,28 +20,6 @@ muzzle { apply from: "${rootDir}/gradle/java.gradle" -/* -Here we introduce a sourceSet for the java 8 code which needs to be compiled with a source and target of 1.8 -The instrumentation classes must be compiled with java 7 and do nothing when ratpack is not on the classpath. The -java 8 classes are used lazily so there is no direct linking between the 1.7 and 1.8 bytecode. -*/ -sourceSets { - main_java8 { - java.srcDirs "${project.projectDir}/src/main/java8" - } -} - -compileMain_java8Java { - sourceCompatibility = 1.8 - targetCompatibility = 1.8 -} -// Note: ideally lombok plugin would do this for us, but currently it doesn't support custom -// source sets. See https://github.com/franzbecker/gradle-lombok/issues/17. -dependencies { - main_java8CompileOnly "org.projectlombok:lombok:${project.lombok.version}" transitive false - main_java8AnnotationProcessor "org.projectlombok:lombok:${project.lombok.version}" transitive false -} - apply plugin: 'org.unbroken-dome.test-sets' testSets { @@ -53,19 +31,6 @@ testSets { dependencies { main_java8CompileOnly group: 'io.ratpack', name: 'ratpack-core', version: '1.4.0' - main_java8Compile project(':dd-java-agent:agent-tooling') - - main_java8Compile deps.bytebuddy - main_java8Compile deps.opentracing - - annotationProcessor deps.autoservice - implementation deps.autoservice - - compileOnly sourceSets.main_java8.compileClasspath - - compile sourceSets.main_java8.output - - testCompile project(':dd-java-agent:testing') testCompile project(':dd-java-agent:instrumentation:java-concurrent') testCompile project(':dd-java-agent:instrumentation:netty-4.1') testCompile group: 'io.ratpack', name: 'ratpack-groovy-test', version: '1.4.0' diff --git a/dd-java-agent/instrumentation/reactor-core-3.1/reactor-core-3.1.gradle b/dd-java-agent/instrumentation/reactor-core-3.1/reactor-core-3.1.gradle index ba3e13c422..6c4a6e1e5a 100644 --- a/dd-java-agent/instrumentation/reactor-core-3.1/reactor-core-3.1.gradle +++ b/dd-java-agent/instrumentation/reactor-core-3.1/reactor-core-3.1.gradle @@ -15,23 +15,6 @@ muzzle { apply from: "${rootDir}/gradle/java.gradle" -sourceSets { - main_java8 { - java.srcDirs "${project.projectDir}/src/main/java8" - } -} - -compileMain_java8Java { - sourceCompatibility = 1.8 - targetCompatibility = 1.8 -} -// Note: ideally lombok plugin would do this for us, but currently it doesn't support custom -// source sets. See https://github.com/franzbecker/gradle-lombok/issues/17. -dependencies { - main_java8CompileOnly "org.projectlombok:lombok:${project.lombok.version}" transitive false - main_java8AnnotationProcessor "org.projectlombok:lombok:${project.lombok.version}" transitive false -} - apply plugin: 'org.unbroken-dome.test-sets' testSets { @@ -39,29 +22,10 @@ testSets { dirName = 'test' } } -compileTestJava { - sourceCompatibility = "1.8" - targetCompatibility = "1.8" -} - -compileLatestDepTestJava { - sourceCompatibility = "1.8" - targetCompatibility = "1.8" -} dependencies { - main_java8CompileOnly group: 'io.projectreactor', name: 'reactor-core', version: '3.1.0.RELEASE' - main_java8Compile project(':dd-java-agent:agent-tooling') - - main_java8Compile deps.bytebuddy - main_java8Compile deps.opentracing - - compileOnly sourceSets.main_java8.compileClasspath - - compile sourceSets.main_java8.output - testCompile project(':dd-java-agent:instrumentation:trace-annotation') testCompile project(':dd-java-agent:instrumentation:java-concurrent') diff --git a/dd-java-agent/instrumentation/spring-webflux/spring-webflux.gradle b/dd-java-agent/instrumentation/spring-webflux/spring-webflux.gradle index 120159c746..b395043ac7 100644 --- a/dd-java-agent/instrumentation/spring-webflux/spring-webflux.gradle +++ b/dd-java-agent/instrumentation/spring-webflux/spring-webflux.gradle @@ -15,23 +15,6 @@ muzzle { apply from: "${rootDir}/gradle/java.gradle" -sourceSets { - main_java8 { - java.srcDirs "${project.projectDir}/src/main/java8" - } -} - -compileMain_java8Java { - sourceCompatibility = 1.8 - targetCompatibility = 1.8 -} -// Note: ideally lombok plugin would do this for us, but currently it doesn't support custom -// source sets. See https://github.com/franzbecker/gradle-lombok/issues/17. -dependencies { - main_java8CompileOnly "org.projectlombok:lombok:${project.lombok.version}" transitive false - main_java8AnnotationProcessor "org.projectlombok:lombok:${project.lombok.version}" transitive false -} - apply plugin: 'org.unbroken-dome.test-sets' testSets { @@ -39,29 +22,12 @@ testSets { dirName = 'test' } } -compileTestJava { - sourceCompatibility = "1.8" - targetCompatibility = "1.8" -} - -compileLatestDepTestJava { - sourceCompatibility = "1.8" - targetCompatibility = "1.8" -} dependencies { // We use helpers from this project main_java8CompileOnly project(':dd-java-agent:instrumentation:reactor-core-3.1') main_java8CompileOnly group: 'org.springframework', name: 'spring-webflux', version: '5.0.0.RELEASE' - main_java8Compile project(':dd-java-agent:agent-tooling') - - main_java8Compile deps.bytebuddy - main_java8Compile deps.opentracing - - compileOnly sourceSets.main_java8.compileClasspath - compile sourceSets.main_java8.output - // We are using utils class from reactor-core instrumentation. // TODO: It is unclear why we need to use `compile` here (instead of 'compileOnly') compile project(':dd-java-agent:instrumentation:reactor-core-3.1') diff --git a/dd-trace-java.gradle b/dd-trace-java.gradle index 6e68a380b6..3b789aa8f3 100644 --- a/dd-trace-java.gradle +++ b/dd-trace-java.gradle @@ -1,5 +1,5 @@ plugins { - id 'io.franzbecker.gradle-lombok' version '1.14' + id 'io.franzbecker.gradle-lombok' version '1.14' // Last to support Java 7 id 'com.jfrog.artifactory' version '4.8.1' id 'com.jfrog.bintray' version '1.8.4' id 'org.unbroken-dome.test-sets' version '2.1.1' diff --git a/gradle/java.gradle b/gradle/java.gradle index 48a9270441..9b16d4f21c 100644 --- a/gradle/java.gradle +++ b/gradle/java.gradle @@ -16,6 +16,38 @@ if (applyCodeCoverage) { sourceCompatibility = 1.7 targetCompatibility = 1.7 +if (project.hasProperty('minJavaVersionForTests') && project.getProperty('minJavaVersionForTests') != JavaVersion.VERSION_1_7) { + def version = JavaVersion.toVersion(project.getProperty('minJavaVersionForTests')) + def name = "java$version.majorVersion" + sourceSets { + "main_$name" { + java.srcDirs "${project.projectDir}/src/main/$name" + } + } + + "compileMain_${name}Java" { + sourceCompatibility = version + targetCompatibility = version + } + + // Note: ideally lombok plugin would do this for us, but currently it doesn't support custom + // source sets. See https://github.com/franzbecker/gradle-lombok/issues/17. + dependencies { + compileOnly sourceSets."main_$name".compileClasspath + compile sourceSets."main_$name".output + + "main_${name}CompileOnly" "org.projectlombok:lombok:${project.lombok.version}" transitive false + "main_${name}AnnotationProcessor" "org.projectlombok:lombok:${project.lombok.version}" transitive false + } + + tasks.withType(JavaCompile).configureEach { + if (it.name.toLowerCase().contains("test")) { + sourceCompatibility = version + targetCompatibility = version + } + } +} + java { // See https://docs.gradle.org/current/userguide/upgrading_version_5.html, Automatic target JVM version disableAutoTargetJvm() @@ -310,7 +342,7 @@ for (def env : System.getenv().entrySet()) { tasks.withType(Test).configureEach { // All tests must complete within 2 minutes. timeout = Duration.ofMinutes(2) - + // Disable all tests if skipTests property was specified onlyIf { !project.rootProject.hasProperty("skipTests") } } From e74167adf2ccd07a7c2cc86cbd4df0b44a57b75f Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Wed, 21 Aug 2019 16:26:33 -0700 Subject: [PATCH 2/3] Update Play instrumentation to work with 2.7 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (Don’t be surprised when things break with 2.8… They’re religious about removing deprecated methods on minor release versions. If they followed standard convention, they’d likely be on at least 11.x.) Add client test for 2.4-2.5 http library. 2.6+ won’t work because the underlying frameworks we instrument are shaded. Also add server tests. We could do a lot more testing since it seems play still supports using Netty as the backing server even though it’s not the default. It’s difficult to do extensive testing though because they have so many breaking changes between versions. --- .../AkkaHttpClientInstrumentationTest.groovy | 9 +- .../src/test/groovy/Netty40ClientTest.groovy | 2 + .../instrumentation/play-2.4/play-2.4.gradle | 64 +++--- .../latestDepTest/groovy/Play26Test.groovy | 114 --------- .../latestDepTest/scala/Play26TestUtils.scala | 68 ------ .../play/PlayInstrumentation.java | 216 ------------------ .../play24/PlayInstrumentation.java | 48 ++++ .../instrumentation/play24/PlayAdvice.java | 82 +++++++ .../instrumentation/play24/PlayHeaders.java | 33 +++ .../play24}/PlayHttpServerDecorator.java | 18 +- .../play24/RequestCompleteCallback.java | 39 ++++ .../src/test/groovy/Play24Test.groovy | 95 -------- .../groovy/client/PlayWSClientTest.groovy | 53 +++++ .../NettyServerTestInstrumentation.java | 22 ++ .../groovy/server/PlayAsyncServerTest.groovy | 52 +++++ .../test/groovy/server/PlayServerTest.groovy | 105 +++++++++ .../src/test/scala/Play24TestUtils.scala | 82 ------- .../instrumentation/play-2.6/.gitignore | 1 + .../instrumentation/play-2.6/play-2.6.gradle | 59 +++++ .../play26/PlayInstrumentation.java | 48 ++++ .../instrumentation/play26/PlayAdvice.java | 77 +++++++ .../instrumentation/play26/PlayHeaders.java | 33 +++ .../play26/PlayHttpServerDecorator.java | 94 ++++++++ .../play26/RequestCompleteCallback.java | 39 ++++ .../server/AkkaHttpTestInstrumentation.java | 19 ++ .../groovy/server/PlayAsyncServerTest.groovy | 64 ++++++ .../test/groovy/server/PlayServerTest.groovy | 137 +++++++++++ .../agent/test/base/HttpServerTest.groovy | 24 +- settings.gradle | 1 + 29 files changed, 1086 insertions(+), 612 deletions(-) delete mode 100644 dd-java-agent/instrumentation/play-2.4/src/latestDepTest/groovy/Play26Test.groovy delete mode 100644 dd-java-agent/instrumentation/play-2.4/src/latestDepTest/scala/Play26TestUtils.scala delete mode 100644 dd-java-agent/instrumentation/play-2.4/src/main/java/datadog/trace/instrumentation/play/PlayInstrumentation.java create mode 100644 dd-java-agent/instrumentation/play-2.4/src/main/java/datadog/trace/instrumentation/play24/PlayInstrumentation.java create mode 100644 dd-java-agent/instrumentation/play-2.4/src/main/java8/datadog/trace/instrumentation/play24/PlayAdvice.java create mode 100644 dd-java-agent/instrumentation/play-2.4/src/main/java8/datadog/trace/instrumentation/play24/PlayHeaders.java rename dd-java-agent/instrumentation/play-2.4/src/main/{java/datadog/trace/instrumentation/play => java8/datadog/trace/instrumentation/play24}/PlayHttpServerDecorator.java (74%) create mode 100644 dd-java-agent/instrumentation/play-2.4/src/main/java8/datadog/trace/instrumentation/play24/RequestCompleteCallback.java delete mode 100644 dd-java-agent/instrumentation/play-2.4/src/test/groovy/Play24Test.groovy create mode 100644 dd-java-agent/instrumentation/play-2.4/src/test/groovy/client/PlayWSClientTest.groovy create mode 100644 dd-java-agent/instrumentation/play-2.4/src/test/groovy/server/NettyServerTestInstrumentation.java create mode 100644 dd-java-agent/instrumentation/play-2.4/src/test/groovy/server/PlayAsyncServerTest.groovy create mode 100644 dd-java-agent/instrumentation/play-2.4/src/test/groovy/server/PlayServerTest.groovy delete mode 100644 dd-java-agent/instrumentation/play-2.4/src/test/scala/Play24TestUtils.scala create mode 100644 dd-java-agent/instrumentation/play-2.6/.gitignore create mode 100644 dd-java-agent/instrumentation/play-2.6/play-2.6.gradle create mode 100644 dd-java-agent/instrumentation/play-2.6/src/main/java/datadog/trace/instrumentation/play26/PlayInstrumentation.java create mode 100644 dd-java-agent/instrumentation/play-2.6/src/main/java8/datadog/trace/instrumentation/play26/PlayAdvice.java create mode 100644 dd-java-agent/instrumentation/play-2.6/src/main/java8/datadog/trace/instrumentation/play26/PlayHeaders.java create mode 100644 dd-java-agent/instrumentation/play-2.6/src/main/java8/datadog/trace/instrumentation/play26/PlayHttpServerDecorator.java create mode 100644 dd-java-agent/instrumentation/play-2.6/src/main/java8/datadog/trace/instrumentation/play26/RequestCompleteCallback.java create mode 100644 dd-java-agent/instrumentation/play-2.6/src/test/groovy/server/AkkaHttpTestInstrumentation.java create mode 100644 dd-java-agent/instrumentation/play-2.6/src/test/groovy/server/PlayAsyncServerTest.groovy create mode 100644 dd-java-agent/instrumentation/play-2.6/src/test/groovy/server/PlayServerTest.groovy diff --git a/dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpClientInstrumentationTest.groovy b/dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpClientInstrumentationTest.groovy index 696d153a67..b37d0c169d 100644 --- a/dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpClientInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/akka-http-10.0/src/test/groovy/AkkaHttpClientInstrumentationTest.groovy @@ -27,10 +27,10 @@ class AkkaHttpClientInstrumentationTest extends HttpClientTest - // FIXME: Callback should be here instead. - // callback?.call() - //} + //.whenComplete { result, error -> + // FIXME: Callback should be here instead. + // callback?.call() + //} .toCompletableFuture() .get() } finally { @@ -51,6 +51,7 @@ class AkkaHttpClientInstrumentationTest extends HttpClientTest { @Shared def clientConfig = DefaultAsyncHttpClientConfig.Builder.newInstance().setRequestTimeout(TimeUnit.SECONDS.toMillis(10).toInteger()) @Shared + @AutoCleanup AsyncHttpClient asyncHttpClient = asyncHttpClient(clientConfig) @Override diff --git a/dd-java-agent/instrumentation/play-2.4/play-2.4.gradle b/dd-java-agent/instrumentation/play-2.4/play-2.4.gradle index 3b6ec2e4c3..a5dbe8dbe5 100644 --- a/dd-java-agent/instrumentation/play-2.4/play-2.4.gradle +++ b/dd-java-agent/instrumentation/play-2.4/play-2.4.gradle @@ -1,50 +1,58 @@ -// Set properties before any plugins get loaded ext { minJavaVersionForTests = JavaVersion.VERSION_1_8 -} - -apply from: "${rootDir}/gradle/java.gradle" -apply from: "${rootDir}/gradle/test-with-scala.gradle" - -apply plugin: 'org.unbroken-dome.test-sets' - -testSets { - latestDepTest + // Play doesn't work with Java 9+ until 2.6.12 + maxJavaVersionForTests = JavaVersion.VERSION_1_8 } muzzle { pass { group = 'com.typesafe.play' module = 'play_2.11' - versions = '[2.4.0,2.7.0-M1)' + versions = '[2.4.0,2.6)' + assertInverse = true } pass { group = 'com.typesafe.play' module = 'play_2.12' - versions = '[2.4.0,2.7.0-M1)' + versions = '[2.4.0,2.6)' + assertInverse = true + } + pass { + group = 'com.typesafe.play' + module = 'play_2.13' + versions = '[2.4.0,2.6)' + assertInverse = true + } +} + +apply from: "${rootDir}/gradle/java.gradle" + +apply plugin: 'org.unbroken-dome.test-sets' + +testSets { + latestDepTest { + dirName = 'test' } } dependencies { - compileOnly group: 'com.typesafe.play', name: 'play_2.11', version: '2.4.0' - - testCompile deps.scala - testCompile group: 'com.typesafe.play', name: 'play_2.11', version: '2.4.0' - testCompile group: 'com.typesafe.play', name: 'play-test_2.11', version: '2.4.0' - testCompile group: 'com.typesafe.play', name: 'play-ws_2.11', version: '2.4.0' + main_java8Compile group: 'com.typesafe.play', name: 'play_2.11', version: '2.4.0' testCompile project(':dd-java-agent:instrumentation:java-concurrent') - testCompile project(':dd-java-agent:instrumentation:trace-annotation') + testCompile project(':dd-java-agent:instrumentation:netty-4.0') + testCompile project(':dd-java-agent:instrumentation:netty-4.1') testCompile project(':dd-java-agent:instrumentation:akka-http-10.0') - testCompile group: 'com.squareup.okhttp3', name: 'okhttp', version: '3.6.0' - latestDepTestCompile deps.scala - latestDepTestCompile group: 'com.typesafe.play', name: 'play_2.11', version: '2.6.+' - latestDepTestCompile group: 'com.typesafe.play', name: 'play-test_2.11', version: '2.6.+' - latestDepTestCompile group: 'com.typesafe.play', name: 'play-ws_2.11', version: '2.6.+' -} + // Before 2.5, play used netty 3.x which isn't supported, so for better test consistency, we test with just 2.5 + testCompile group: 'com.typesafe.play', name: 'play-java_2.11', version: '2.5.0' + testCompile group: 'com.typesafe.play', name: 'play-java-ws_2.11', version: '2.5.0' + testCompile (group: 'com.typesafe.play', name: 'play-test_2.11', version: '2.5.0') { + exclude group: 'org.eclipse.jetty.websocket', module: 'websocket-client' + } -compileLatestDepTestGroovy { - classpath = classpath.plus(files(compileLatestDepTestScala.destinationDir)) - dependsOn compileLatestDepTestScala + latestDepTestCompile group: 'com.typesafe.play', name: 'play-java_2.11', version: '2.5.+' + latestDepTestCompile group: 'com.typesafe.play', name: 'play-java-ws_2.11', version: '2.5.+' + latestDepTestCompile (group: 'com.typesafe.play', name: 'play-test_2.11', version: '2.5.+') { + exclude group: 'org.eclipse.jetty.websocket', module: 'websocket-client' + } } diff --git a/dd-java-agent/instrumentation/play-2.4/src/latestDepTest/groovy/Play26Test.groovy b/dd-java-agent/instrumentation/play-2.4/src/latestDepTest/groovy/Play26Test.groovy deleted file mode 100644 index 60eec3f825..0000000000 --- a/dd-java-agent/instrumentation/play-2.4/src/latestDepTest/groovy/Play26Test.groovy +++ /dev/null @@ -1,114 +0,0 @@ -import datadog.trace.agent.test.AgentTestRunner -import datadog.trace.agent.test.utils.OkHttpUtils -import datadog.trace.agent.test.utils.PortUtils -import datadog.trace.api.DDSpanTypes -import okhttp3.Request -import play.api.test.TestServer -import play.test.Helpers -import spock.lang.Shared - -class Play26Test extends AgentTestRunner { - @Shared - int port = PortUtils.randomOpenPort() - @Shared - TestServer testServer - - @Shared - def client = OkHttpUtils.client() - - def setupSpec() { - testServer = Helpers.testServer(port, Play26TestUtils.buildTestApp()) - testServer.start() - } - - def cleanupSpec() { - testServer.stop() - } - - def "request traces"() { - setup: - def request = new Request.Builder() - .url("http://localhost:$port/$path") - .header("x-datadog-trace-id", "123") - .header("x-datadog-parent-id", "456") - .get() - .build() - def response = client.newCall(request).execute() - - expect: - testServer != null - response.code() == status - if (body instanceof Class) { - body.isInstance(response.body()) - } else { - response.body().string() == body - } - - assertTraces(1) { - trace(0, extraSpans ? 3 : 2) { - span(0) { - traceId "123" - parentId "456" - serviceName "unnamed-java-app" - operationName "akka-http.request" - resourceName status == 404 ? "404" : "GET $route" - spanType DDSpanTypes.HTTP_SERVER - errored isError - tags { - "http.status_code" status - "http.url" "http://localhost:$port/$path" - "http.method" "GET" - "span.kind" "server" - "component" "akka-http-server" - if (isError) { - "error" true - } - defaultTags(true) - } - } - span(1) { - operationName "play.request" - resourceName status == 404 ? "404" : "GET $route" - spanType DDSpanTypes.HTTP_SERVER - childOf(span(0)) - errored isError - tags { - "http.status_code" status - "http.url" "http://localhost:$port/$path" - "http.method" "GET" - "peer.ipv4" "127.0.0.1" - "span.kind" "server" - "component" "play-action" - if (isError) { - if (exception) { - errorTags(exception.class, exception.message) - } else { - "error" true - } - } - defaultTags() - } - } - if (extraSpans) { - span(2) { - operationName "TracedWork\$.doWork" - childOf(span(1)) - tags { - "component" "trace" - defaultTags() - } - } - } - } - } - - where: - path | route | body | status | isError | exception - "helloplay/spock" | "/helloplay/:from" | "hello spock" | 200 | false | null - "make-error" | "/make-error" | "Really sorry..." | 500 | true | null - "exception" | "/exception" | String | 500 | true | new RuntimeException("oh no") - "nowhere" | "/nowhere" | "Really sorry..." | 404 | false | null - - extraSpans = !isError && status != 404 - } -} diff --git a/dd-java-agent/instrumentation/play-2.4/src/latestDepTest/scala/Play26TestUtils.scala b/dd-java-agent/instrumentation/play-2.4/src/latestDepTest/scala/Play26TestUtils.scala deleted file mode 100644 index d24b3f7c07..0000000000 --- a/dd-java-agent/instrumentation/play-2.4/src/latestDepTest/scala/Play26TestUtils.scala +++ /dev/null @@ -1,68 +0,0 @@ -import java.lang.reflect.Field - -import datadog.trace.api.Trace -import play.api.mvc.request.RequestAttrKey -import play.api.mvc.{Action, _} -import play.api.routing.sird._ -import play.api.routing.{HandlerDef, Router} - -import scala.concurrent.duration._ -import scala.concurrent.{Await, Future} - -object Play26TestUtils { - def buildTestApp(): play.Application = { - // build play.api.Application with desired setting and pass into play.Application for testing - val apiApp: play.api.Application = new play.api.inject.guice.GuiceApplicationBuilder() - .requireAtInjectOnConstructors(true) - .router( - Router.from { - case GET(p"/helloplay/$from") => Action { req: RequestHeader => - HandlerSetter.setHandler(req, "/helloplay/:from") - // FIXME: Add WS request for testing. - // implicit val application = Play.current - // val wsRequest = WS.url("http://localhost:" + port).get() - val f: Future[String] = Future[String] { - TracedWork.doWork() - from - }(Action.executionContext) - Results.Ok(s"hello " + Await.result(f, 5 seconds)) - } - case GET(p"/make-error") => Action { req: RequestHeader => - HandlerSetter.setHandler(req, "/make-error") - Results.InternalServerError("Really sorry...") - } - case GET(p"/exception") => Action { req: RequestHeader => - HandlerSetter.setHandler(req, "/exception") - if (System.currentTimeMillis() > 0) { - throw new RuntimeException("oh no") - } - Results.Ok("hello") - } - case _ => Action { - Results.NotFound("Sorry..") - } - }) - .build() - - return new play.DefaultApplication(apiApp, new play.inject.guice.GuiceApplicationBuilder().build().injector()) - } -} - -object TracedWork { - @Trace - def doWork(): Unit = { - } -} - -object HandlerSetter { - def setHandler(req: RequestHeader, path: String): Unit = { - val f: Field = req.getClass().getDeclaredField("attrs") - f.setAccessible(true) - f.set(req, req.attrs - .updated(play.routing.Router.Attrs.HANDLER_DEF.underlying(), new HandlerDef(null, null, null, null, null, null, path, null, null)) - .updated(RequestAttrKey.Tags, Map(play.routing.Router.Tags.ROUTE_PATTERN -> path))) - f.setAccessible(false) - } -} - -class Play26TestUtils {} diff --git a/dd-java-agent/instrumentation/play-2.4/src/main/java/datadog/trace/instrumentation/play/PlayInstrumentation.java b/dd-java-agent/instrumentation/play-2.4/src/main/java/datadog/trace/instrumentation/play/PlayInstrumentation.java deleted file mode 100644 index 661f1b77b8..0000000000 --- a/dd-java-agent/instrumentation/play-2.4/src/main/java/datadog/trace/instrumentation/play/PlayInstrumentation.java +++ /dev/null @@ -1,216 +0,0 @@ -package datadog.trace.instrumentation.play; - -import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; -import static datadog.trace.instrumentation.play.PlayHttpServerDecorator.DECORATE; -import static java.util.Collections.singletonMap; -import static net.bytebuddy.matcher.ElementMatchers.named; -import static net.bytebuddy.matcher.ElementMatchers.returns; -import static net.bytebuddy.matcher.ElementMatchers.takesArgument; - -import akka.japi.JavaPartialFunction; -import com.google.auto.service.AutoService; -import datadog.trace.agent.tooling.Instrumenter; -import datadog.trace.api.DDTags; -import datadog.trace.context.TraceScope; -import io.opentracing.Scope; -import io.opentracing.Span; -import io.opentracing.SpanContext; -import io.opentracing.propagation.Format; -import io.opentracing.propagation.TextMap; -import io.opentracing.tag.Tags; -import io.opentracing.util.GlobalTracer; -import java.util.HashMap; -import java.util.Iterator; -import java.util.Map; -import lombok.extern.slf4j.Slf4j; -import net.bytebuddy.asm.Advice; -import net.bytebuddy.description.method.MethodDescription; -import net.bytebuddy.description.type.TypeDescription; -import net.bytebuddy.matcher.ElementMatcher; -import play.api.mvc.Action; -import play.api.mvc.Request; -import play.api.mvc.Result; -import scala.Option; -import scala.Tuple2; -import scala.concurrent.Future; - -@AutoService(Instrumenter.class) -public final class PlayInstrumentation extends Instrumenter.Default { - - public PlayInstrumentation() { - super("play"); - } - - @Override - public ElementMatcher typeMatcher() { - return safeHasSuperType(named("play.api.mvc.Action")); - } - - @Override - public String[] helperClassNames() { - return new String[] { - "datadog.trace.agent.decorator.BaseDecorator", - "datadog.trace.agent.decorator.ServerDecorator", - "datadog.trace.agent.decorator.HttpServerDecorator", - packageName + ".PlayHttpServerDecorator", - PlayInstrumentation.class.getName() + "$RequestCallback", - PlayInstrumentation.class.getName() + "$RequestError", - PlayInstrumentation.class.getName() + "$PlayHeaders" - }; - } - - @Override - public Map, String> transformers() { - return singletonMap( - named("apply") - .and(takesArgument(0, named("play.api.mvc.Request"))) - .and(returns(named("scala.concurrent.Future"))), - PlayAdvice.class.getName()); - } - - public static class PlayAdvice { - @Advice.OnMethodEnter(suppress = Throwable.class) - public static Scope startSpan(@Advice.Argument(0) final Request req) { - final Scope scope; - if (GlobalTracer.get().activeSpan() == null) { - final SpanContext extractedContext; - if (GlobalTracer.get().scopeManager().active() == null) { - extractedContext = - GlobalTracer.get().extract(Format.Builtin.HTTP_HEADERS, new PlayHeaders(req)); - } else { - extractedContext = null; - } - scope = - GlobalTracer.get() - .buildSpan("play.request") - .asChildOf(extractedContext) - .startActive(false); - } else { - // An upstream framework (e.g. akka-http, netty) has already started the span. - // Do not extract the context. - scope = GlobalTracer.get().buildSpan("play.request").startActive(false); - } - DECORATE.afterStart(scope); - DECORATE.onConnection(scope.span(), req); - - if (GlobalTracer.get().scopeManager().active() instanceof TraceScope) { - ((TraceScope) GlobalTracer.get().scopeManager().active()).setAsyncPropagation(true); - } - return scope; - } - - @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) - public static void stopTraceOnResponse( - @Advice.Enter final Scope playControllerScope, - @Advice.This final Object thisAction, - @Advice.Thrown final Throwable throwable, - @Advice.Argument(0) final Request req, - @Advice.Return(readOnly = false) Future responseFuture) { - final Span playControllerSpan = playControllerScope.span(); - - // Call onRequest on return after tags are populated. - DECORATE.onRequest(playControllerSpan, req); - - if (throwable == null) { - responseFuture.onFailure( - new RequestError(playControllerSpan), ((Action) thisAction).executionContext()); - responseFuture = - responseFuture.map( - new RequestCallback(playControllerSpan), ((Action) thisAction).executionContext()); - } else { - DECORATE.onError(playControllerSpan, throwable); - Tags.HTTP_STATUS.set(playControllerSpan, 500); - DECORATE.beforeFinish(playControllerSpan); - playControllerSpan.finish(); - } - playControllerScope.close(); - - final Span rootSpan = GlobalTracer.get().activeSpan(); - - // more about routes here: - // https://github.com/playframework/playframework/blob/master/documentation/manual/releases/release26/migration26/Migration26.md - - final Option pathOption = req.tags().get("ROUTE_PATTERN"); - if (rootSpan != null && !pathOption.isEmpty()) { - // set the resource name on the upstream akka/netty span - final String path = (String) pathOption.get(); - rootSpan.setTag(DDTags.RESOURCE_NAME, req.method() + " " + path); - } - } - } - - public static class PlayHeaders implements TextMap { - private final Request request; - - public PlayHeaders(final Request request) { - this.request = request; - } - - @Override - public Iterator> iterator() { - final scala.collection.Map scalaMap = request.headers().toSimpleMap(); - final Map javaMap = new HashMap<>(scalaMap.size()); - final scala.collection.Iterator> scalaIterator = scalaMap.iterator(); - while (scalaIterator.hasNext()) { - final Tuple2 tuple = scalaIterator.next(); - javaMap.put(tuple._1(), tuple._2()); - } - return javaMap.entrySet().iterator(); - } - - @Override - public void put(final String s, final String s1) { - throw new IllegalStateException("play headers can only be extracted"); - } - } - - @Slf4j - public static class RequestError extends JavaPartialFunction { - private final Span span; - - public RequestError(final Span span) { - this.span = span; - } - - @Override - public Object apply(final Throwable t, final boolean isCheck) throws Exception { - try { - DECORATE.onError(span, t); - DECORATE.beforeFinish(span); - if (GlobalTracer.get().scopeManager().active() instanceof TraceScope) { - ((TraceScope) GlobalTracer.get().scopeManager().active()).setAsyncPropagation(false); - } - } catch (final Throwable t2) { - log.debug("error in play instrumentation", t); - } finally { - span.finish(); - } - return null; - } - } - - @Slf4j - public static class RequestCallback extends scala.runtime.AbstractFunction1 { - private final Span span; - - public RequestCallback(final Span span) { - this.span = span; - } - - @Override - public Result apply(final Result result) { - if (GlobalTracer.get().scopeManager().active() instanceof TraceScope) { - ((TraceScope) GlobalTracer.get().scopeManager().active()).setAsyncPropagation(false); - } - try { - DECORATE.onResponse(span, result); - DECORATE.beforeFinish(span); - } catch (final Throwable t) { - log.debug("error in play instrumentation", t); - } finally { - span.finish(); - } - return result; - } - } -} diff --git a/dd-java-agent/instrumentation/play-2.4/src/main/java/datadog/trace/instrumentation/play24/PlayInstrumentation.java b/dd-java-agent/instrumentation/play-2.4/src/main/java/datadog/trace/instrumentation/play24/PlayInstrumentation.java new file mode 100644 index 0000000000..950fe72d60 --- /dev/null +++ b/dd-java-agent/instrumentation/play-2.4/src/main/java/datadog/trace/instrumentation/play24/PlayInstrumentation.java @@ -0,0 +1,48 @@ +package datadog.trace.instrumentation.play24; + +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; +import static java.util.Collections.singletonMap; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.returns; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import java.util.Map; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +@AutoService(Instrumenter.class) +public final class PlayInstrumentation extends Instrumenter.Default { + + public PlayInstrumentation() { + super("play"); + } + + @Override + public ElementMatcher typeMatcher() { + return safeHasSuperType(named("play.api.mvc.Action")); + } + + @Override + public String[] helperClassNames() { + return new String[] { + "datadog.trace.agent.decorator.BaseDecorator", + "datadog.trace.agent.decorator.ServerDecorator", + "datadog.trace.agent.decorator.HttpServerDecorator", + packageName + ".PlayHttpServerDecorator", + packageName + ".RequestCompleteCallback", + packageName + ".PlayHeaders", + }; + } + + @Override + public Map, String> transformers() { + return singletonMap( + named("apply") + .and(takesArgument(0, named("play.api.mvc.Request"))) + .and(returns(named("scala.concurrent.Future"))), + packageName + ".PlayAdvice"); + } +} diff --git a/dd-java-agent/instrumentation/play-2.4/src/main/java8/datadog/trace/instrumentation/play24/PlayAdvice.java b/dd-java-agent/instrumentation/play-2.4/src/main/java8/datadog/trace/instrumentation/play24/PlayAdvice.java new file mode 100644 index 0000000000..ca5ca34d24 --- /dev/null +++ b/dd-java-agent/instrumentation/play-2.4/src/main/java8/datadog/trace/instrumentation/play24/PlayAdvice.java @@ -0,0 +1,82 @@ +package datadog.trace.instrumentation.play24; + +import static datadog.trace.instrumentation.play24.PlayHttpServerDecorator.DECORATE; + +import datadog.trace.context.TraceScope; +import io.opentracing.Scope; +import io.opentracing.Span; +import io.opentracing.SpanContext; +import io.opentracing.propagation.Format; +import io.opentracing.tag.Tags; +import io.opentracing.util.GlobalTracer; +import net.bytebuddy.asm.Advice; +import play.api.mvc.Action; +import play.api.mvc.Request; +import play.api.mvc.Result; +import scala.concurrent.Future; + +public class PlayAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static Scope startSpan(@Advice.Argument(0) final Request req) { + final Scope scope; + if (GlobalTracer.get().activeSpan() == null) { + final SpanContext extractedContext; + if (GlobalTracer.get().scopeManager().active() == null) { + extractedContext = + GlobalTracer.get().extract(Format.Builtin.HTTP_HEADERS, new PlayHeaders(req)); + } else { + extractedContext = null; + } + scope = + GlobalTracer.get() + .buildSpan("play.request") + .asChildOf(extractedContext) + .startActive(false); + } else { + // An upstream framework (e.g. akka-http, netty) has already started the span. + // Do not extract the context. + scope = GlobalTracer.get().buildSpan("play.request").startActive(false); + } + DECORATE.afterStart(scope); + DECORATE.onConnection(scope.span(), req); + + if (GlobalTracer.get().scopeManager().active() instanceof TraceScope) { + ((TraceScope) GlobalTracer.get().scopeManager().active()).setAsyncPropagation(true); + } + return scope; + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void stopTraceOnResponse( + @Advice.Enter final Scope playControllerScope, + @Advice.This final Object thisAction, + @Advice.Thrown final Throwable throwable, + @Advice.Argument(0) final Request req, + @Advice.Return(readOnly = false) final Future responseFuture) { + final Span playControllerSpan = playControllerScope.span(); + + // Call onRequest on return after tags are populated. + DECORATE.onRequest(playControllerSpan, req); + + if (throwable == null) { + responseFuture.onComplete( + new RequestCompleteCallback(playControllerSpan), + ((Action) thisAction).executionContext()); + } else { + DECORATE.onError(playControllerSpan, throwable); + Tags.HTTP_STATUS.set(playControllerSpan, 500); + DECORATE.beforeFinish(playControllerSpan); + playControllerSpan.finish(); + } + playControllerScope.close(); + + final Span rootSpan = GlobalTracer.get().activeSpan(); + // set the resource name on the upstream akka/netty span + DECORATE.onRequest(rootSpan, req); + } + + // Unused method for muzzle to allow only 2.4-2.5 + public static void muzzleCheck() { + play.libs.Akka.system(); + } +} diff --git a/dd-java-agent/instrumentation/play-2.4/src/main/java8/datadog/trace/instrumentation/play24/PlayHeaders.java b/dd-java-agent/instrumentation/play-2.4/src/main/java8/datadog/trace/instrumentation/play24/PlayHeaders.java new file mode 100644 index 0000000000..5c20eeb2c9 --- /dev/null +++ b/dd-java-agent/instrumentation/play-2.4/src/main/java8/datadog/trace/instrumentation/play24/PlayHeaders.java @@ -0,0 +1,33 @@ +package datadog.trace.instrumentation.play24; + +import io.opentracing.propagation.TextMap; +import java.util.HashMap; +import java.util.Iterator; +import java.util.Map; +import play.api.mvc.Request; +import scala.Tuple2; + +public class PlayHeaders implements TextMap { + private final Request request; + + public PlayHeaders(final Request request) { + this.request = request; + } + + @Override + public Iterator> iterator() { + final scala.collection.Map scalaMap = request.headers().toSimpleMap(); + final Map javaMap = new HashMap<>(scalaMap.size()); + final scala.collection.Iterator> scalaIterator = scalaMap.iterator(); + while (scalaIterator.hasNext()) { + final Tuple2 tuple = scalaIterator.next(); + javaMap.put(tuple._1(), tuple._2()); + } + return javaMap.entrySet().iterator(); + } + + @Override + public void put(final String s, final String s1) { + throw new IllegalStateException("play headers can only be extracted"); + } +} diff --git a/dd-java-agent/instrumentation/play-2.4/src/main/java/datadog/trace/instrumentation/play/PlayHttpServerDecorator.java b/dd-java-agent/instrumentation/play-2.4/src/main/java8/datadog/trace/instrumentation/play24/PlayHttpServerDecorator.java similarity index 74% rename from dd-java-agent/instrumentation/play-2.4/src/main/java/datadog/trace/instrumentation/play/PlayHttpServerDecorator.java rename to dd-java-agent/instrumentation/play-2.4/src/main/java8/datadog/trace/instrumentation/play24/PlayHttpServerDecorator.java index 5bbfed9a85..e6e0468490 100644 --- a/dd-java-agent/instrumentation/play-2.4/src/main/java/datadog/trace/instrumentation/play/PlayHttpServerDecorator.java +++ b/dd-java-agent/instrumentation/play-2.4/src/main/java8/datadog/trace/instrumentation/play24/PlayHttpServerDecorator.java @@ -1,9 +1,11 @@ -package datadog.trace.instrumentation.play; +package datadog.trace.instrumentation.play24; import datadog.trace.agent.decorator.HttpServerDecorator; import datadog.trace.api.DDTags; import io.opentracing.Span; import io.opentracing.tag.Tags; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.UndeclaredThrowableException; import java.net.URI; import java.net.URISyntaxException; import lombok.extern.slf4j.Slf4j; @@ -64,7 +66,6 @@ public class PlayHttpServerDecorator extends HttpServerDecorator, Object> { + private final Span span; + + public RequestCompleteCallback(final Span span) { + this.span = span; + } + + @Override + public Object apply(final Try result) { + try { + if (result.isFailure()) { + DECORATE.onError(span, result.failed().get()); + } else { + DECORATE.onResponse(span, result.get()); + } + DECORATE.beforeFinish(span); + if (GlobalTracer.get().scopeManager().active() instanceof TraceScope) { + ((TraceScope) GlobalTracer.get().scopeManager().active()).setAsyncPropagation(false); + } + } catch (final Throwable t) { + log.debug("error in play instrumentation", t); + } finally { + span.finish(); + } + return null; + } +} diff --git a/dd-java-agent/instrumentation/play-2.4/src/test/groovy/Play24Test.groovy b/dd-java-agent/instrumentation/play-2.4/src/test/groovy/Play24Test.groovy deleted file mode 100644 index e10450885e..0000000000 --- a/dd-java-agent/instrumentation/play-2.4/src/test/groovy/Play24Test.groovy +++ /dev/null @@ -1,95 +0,0 @@ -import datadog.trace.agent.test.AgentTestRunner -import datadog.trace.agent.test.utils.OkHttpUtils -import datadog.trace.agent.test.utils.PortUtils -import datadog.trace.api.DDSpanTypes -import okhttp3.Request -import play.api.test.TestServer -import play.test.Helpers -import spock.lang.Shared - -class Play24Test extends AgentTestRunner { - @Shared - int port = PortUtils.randomOpenPort() - @Shared - TestServer testServer - - @Shared - def client = OkHttpUtils.client() - - def setupSpec() { - testServer = Helpers.testServer(port, Play24TestUtils.buildTestApp(port)) - testServer.start() - } - - def cleanupSpec() { - testServer.stop() - } - - def "request traces"() { - setup: - def request = new Request.Builder() - .url("http://localhost:$port/$path") - .header("x-datadog-trace-id", "123") - .header("x-datadog-parent-id", "456") - .get() - .build() - def response = client.newCall(request).execute() - - expect: - testServer != null - response.code() == status - if (body instanceof Class) { - body.isInstance(response.body()) - } else { - response.body().string() == body - } - - assertTraces(1) { - trace(0, extraSpans ? 2 : 1) { - span(0) { - traceId "123" - parentId "456" - operationName "play.request" - resourceName status == 404 ? "404" : "GET $route" - spanType DDSpanTypes.HTTP_SERVER - errored isError - tags { - "http.status_code" status - "http.url" "http://localhost:$port/$path" - "http.method" "GET" - "peer.ipv4" "127.0.0.1" - "span.kind" "server" - "component" "play-action" - if (isError) { - if (exception) { - errorTags(exception.class, exception.message) - } else { - "error" true - } - } - defaultTags(true) - } - } - if (extraSpans) { - span(1) { - operationName "TracedWork\$.doWork" - childOf(span(0)) - tags { - "component" "trace" - defaultTags() - } - } - } - } - } - - where: - path | route | body | status | isError | exception - "helloplay/spock" | "/helloplay/:from" | "hello spock" | 200 | false | null - "make-error" | "/make-error" | "Really sorry..." | 500 | true | null - "exception" | "/exception" | String | 500 | true | new RuntimeException("oh no") - "nowhere" | "/nowhere" | "Really sorry..." | 404 | false | null - - extraSpans = !isError && status != 404 - } -} diff --git a/dd-java-agent/instrumentation/play-2.4/src/test/groovy/client/PlayWSClientTest.groovy b/dd-java-agent/instrumentation/play-2.4/src/test/groovy/client/PlayWSClientTest.groovy new file mode 100644 index 0000000000..86768658bf --- /dev/null +++ b/dd-java-agent/instrumentation/play-2.4/src/test/groovy/client/PlayWSClientTest.groovy @@ -0,0 +1,53 @@ +package client + +import datadog.trace.agent.test.base.HttpClientTest +import datadog.trace.instrumentation.netty40.client.NettyHttpClientDecorator +import play.libs.ws.WS +import spock.lang.AutoCleanup +import spock.lang.Shared +import spock.lang.Subject + +// Play 2.6+ uses a separately versioned client that shades the underlying dependency +// This means our built in instrumentation won't work. +class PlayWSClientTest extends HttpClientTest { + @Subject + @Shared + @AutoCleanup + def client = WS.newClient(-1) + + @Override + int doRequest(String method, URI uri, Map headers, Closure callback) { + def request = client.url(uri.toString()) + headers.entrySet().each { + request.setHeader(it.key, it.value) + } + + def status = request.execute(method).thenApply { + callback?.call() + it + }.thenApply { + it.status + } + return status.toCompletableFuture().get() + } + + @Override + NettyHttpClientDecorator decorator() { + return NettyHttpClientDecorator.DECORATE + } + + @Override + String expectedOperationName() { + return "netty.client.request" + } + + @Override + boolean testRedirects() { + false + } + + @Override + boolean testConnectionFailure() { + false + } +} diff --git a/dd-java-agent/instrumentation/play-2.4/src/test/groovy/server/NettyServerTestInstrumentation.java b/dd-java-agent/instrumentation/play-2.4/src/test/groovy/server/NettyServerTestInstrumentation.java new file mode 100644 index 0000000000..61cb150a54 --- /dev/null +++ b/dd-java-agent/instrumentation/play-2.4/src/test/groovy/server/NettyServerTestInstrumentation.java @@ -0,0 +1,22 @@ +package server; + +import static net.bytebuddy.matcher.ElementMatchers.named; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.test.base.HttpServerTestAdvice; +import datadog.trace.agent.tooling.Instrumenter; +import net.bytebuddy.agent.builder.AgentBuilder; + +@AutoService(Instrumenter.class) +public class NettyServerTestInstrumentation implements Instrumenter { + + @Override + public AgentBuilder instrument(final AgentBuilder agentBuilder) { + return agentBuilder + .type(named("io.netty.handler.codec.ByteToMessageDecoder")) + .transform( + new AgentBuilder.Transformer.ForAdvice() + .advice( + named("channelRead"), HttpServerTestAdvice.ServerEntryAdvice.class.getName())); + } +} diff --git a/dd-java-agent/instrumentation/play-2.4/src/test/groovy/server/PlayAsyncServerTest.groovy b/dd-java-agent/instrumentation/play-2.4/src/test/groovy/server/PlayAsyncServerTest.groovy new file mode 100644 index 0000000000..a89348d1a6 --- /dev/null +++ b/dd-java-agent/instrumentation/play-2.4/src/test/groovy/server/PlayAsyncServerTest.groovy @@ -0,0 +1,52 @@ +package server + +import play.libs.concurrent.HttpExecution +import play.mvc.Results +import play.routing.RoutingDsl +import play.server.Server + +import java.util.concurrent.CompletableFuture +import java.util.function.Supplier + +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS + +class PlayAsyncServerTest extends PlayServerTest { + @Override + Server startServer(int port) { + def router = + new RoutingDsl() + .GET(SUCCESS.getPath()).routeAsync({ + CompletableFuture.supplyAsync({ + controller(SUCCESS) { + Results.status(SUCCESS.getStatus(), SUCCESS.getBody()) + } + }, HttpExecution.defaultContext()) + } as Supplier) + .GET(REDIRECT.getPath()).routeAsync({ + CompletableFuture.supplyAsync({ + controller(REDIRECT) { + Results.found(REDIRECT.getBody()) + } + }, HttpExecution.defaultContext()) + } as Supplier) + .GET(ERROR.getPath()).routeAsync({ + CompletableFuture.supplyAsync({ + controller(ERROR) { + Results.status(ERROR.getStatus(), ERROR.getBody()) + } + }, HttpExecution.defaultContext()) + } as Supplier) + .GET(EXCEPTION.getPath()).routeAsync({ + CompletableFuture.supplyAsync({ + controller(EXCEPTION) { + throw new Exception(EXCEPTION.getBody()) + } + }, HttpExecution.defaultContext()) + } as Supplier) + + return Server.forRouter(router.build(), port) + } +} diff --git a/dd-java-agent/instrumentation/play-2.4/src/test/groovy/server/PlayServerTest.groovy b/dd-java-agent/instrumentation/play-2.4/src/test/groovy/server/PlayServerTest.groovy new file mode 100644 index 0000000000..334b2478fd --- /dev/null +++ b/dd-java-agent/instrumentation/play-2.4/src/test/groovy/server/PlayServerTest.groovy @@ -0,0 +1,105 @@ +package server + +import datadog.opentracing.DDSpan +import datadog.trace.agent.test.asserts.TraceAssert +import datadog.trace.agent.test.base.HttpServerTest +import datadog.trace.api.DDSpanTypes +import datadog.trace.instrumentation.netty40.server.NettyHttpServerDecorator +import datadog.trace.instrumentation.play24.PlayHttpServerDecorator +import io.opentracing.tag.Tags +import play.mvc.Results +import play.routing.RoutingDsl +import play.server.Server + +import java.util.function.Supplier + +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS + +class PlayServerTest extends HttpServerTest { + @Override + Server startServer(int port) { + def router = + new RoutingDsl() + .GET(SUCCESS.getPath()).routeTo({ + controller(SUCCESS) { + Results.status(SUCCESS.getStatus(), SUCCESS.getBody()) + } + } as Supplier) + .GET(REDIRECT.getPath()).routeTo({ + controller(REDIRECT) { + Results.found(REDIRECT.getBody()) + } + } as Supplier) + .GET(ERROR.getPath()).routeTo({ + controller(ERROR) { + Results.status(ERROR.getStatus(), ERROR.getBody()) + } + } as Supplier) + .GET(EXCEPTION.getPath()).routeTo({ + controller(EXCEPTION) { + throw new Exception(EXCEPTION.getBody()) + } + } as Supplier) + + return Server.forRouter(router.build(), port) + } + + @Override + void stopServer(Server server) { + server.stop() + } + + @Override + NettyHttpServerDecorator decorator() { + return NettyHttpServerDecorator.DECORATE + } + + @Override + String expectedOperationName() { + return "netty.request" + } + + @Override + boolean hasHandlerSpan() { + true + } + + @Override + // Return the handler span's name + String reorderHandlerSpan() { + "play.request" + } + + boolean testExceptionBody() { + // I can't figure out how to set a proper exception handler to customize the response body. + false + } + + @Override + void handlerSpan(TraceAssert trace, int index, Object parent, ServerEndpoint endpoint = SUCCESS) { + trace.span(index) { + serviceName expectedServiceName() + operationName "play.request" + spanType DDSpanTypes.HTTP_SERVER + errored endpoint == ERROR || endpoint == EXCEPTION + childOf(parent as DDSpan) + tags { + "$Tags.COMPONENT.key" PlayHttpServerDecorator.DECORATE.component() + "$Tags.HTTP_STATUS.key" Integer + "$Tags.HTTP_URL.key" String + "$Tags.PEER_HOST_IPV4.key" { it == null || it == "127.0.0.1" } // Optional + "$Tags.HTTP_METHOD.key" String + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + defaultTags() + if (endpoint == ERROR) { + "$Tags.ERROR.key" true + } else if (endpoint == EXCEPTION) { + errorTags(Exception, EXCEPTION.body) + } + } + } + } +} diff --git a/dd-java-agent/instrumentation/play-2.4/src/test/scala/Play24TestUtils.scala b/dd-java-agent/instrumentation/play-2.4/src/test/scala/Play24TestUtils.scala deleted file mode 100644 index 44045973c5..0000000000 --- a/dd-java-agent/instrumentation/play-2.4/src/test/scala/Play24TestUtils.scala +++ /dev/null @@ -1,82 +0,0 @@ -import java.lang.reflect.Field - -import datadog.trace.api.Trace -import play.api.inject.bind -import play.api.mvc.{Action, _} -import play.api.routing.Router -import play.api.routing.sird._ -import play.inject.DelegateInjector - -import scala.concurrent.ExecutionContext.Implicits.global -import scala.concurrent.duration._ -import scala.concurrent.{Await, Future} - -object Play24TestUtils { - def buildTestApp(port: Int): play.Application = { - // build play.api.Application with desired setting and pass into play.Application for testing - val apiApp: play.api.Application = new play.api.inject.guice.GuiceApplicationBuilder() - .overrides(bind[Router].toInstance(Router.from { - case GET(p"/helloplay/$from") => Action { req: RequestHeader => - HandlerSetter.setHandler(req, "/helloplay/:from") - // FIXME: Add WS request for testing. - // implicit val application = Play.current - // val wsRequest = WS.url("http://localhost:" + port).get() - val f: Future[String] = Future[String] { - TracedWork.doWork() - from - } - // Await.result(wsRequest, 5 seconds) - Results.Ok(s"hello " + Await.result(f, 5 seconds)) - } - case GET(p"/") => Action { req: RequestHeader => - TracedWork.doWork() - Results.Ok(s"hello") - } - case GET(p"/make-error") => Action { req: RequestHeader => - HandlerSetter.setHandler(req, "/make-error") - Results.InternalServerError("Really sorry...") - } - case GET(p"/exception") => Action { req: RequestHeader => - HandlerSetter.setHandler(req, "/exception") - if (System.currentTimeMillis() > 0) { - throw new RuntimeException("oh no") - } - Results.Ok("hello") - } - case _ => Action { - Results.NotFound("Sorry..") - } - })) - .build() - - - return new play.DefaultApplication(apiApp, new DelegateInjector(apiApp.injector)) - } -} - -object TracedWork { - @Trace - def doWork(): Unit = { - } -} - -object HandlerSetter { - def setHandler(req: RequestHeader, path: String): Unit = { - val rh = getField(req, "rh$1") - val newTags: Map[String, String] = Map(Router.Tags.RoutePattern -> path) - val f: Field = rh.getClass().getDeclaredField("tags") - f.setAccessible(true) - f.set(rh, newTags) - f.setAccessible(false) - } - - private def getField(o: Object, fieldName: String): Object = { - val f: Field = o.getClass().getDeclaredField(fieldName) - f.setAccessible(true) - val result: Object = f.get(o) - f.setAccessible(false) - return result - } -} - -class Play24TestUtils {} diff --git a/dd-java-agent/instrumentation/play-2.6/.gitignore b/dd-java-agent/instrumentation/play-2.6/.gitignore new file mode 100644 index 0000000000..5292519a25 --- /dev/null +++ b/dd-java-agent/instrumentation/play-2.6/.gitignore @@ -0,0 +1 @@ +logs/ \ No newline at end of file diff --git a/dd-java-agent/instrumentation/play-2.6/play-2.6.gradle b/dd-java-agent/instrumentation/play-2.6/play-2.6.gradle new file mode 100644 index 0000000000..35d0977232 --- /dev/null +++ b/dd-java-agent/instrumentation/play-2.6/play-2.6.gradle @@ -0,0 +1,59 @@ +ext { + minJavaVersionForTests = JavaVersion.VERSION_1_8 + // Play doesn't work with Java 9+ until 2.6.12 + maxJavaVersionForTests = JavaVersion.VERSION_1_8 +} + +muzzle { + pass { + group = 'com.typesafe.play' + module = 'play_$scalaVersion' + versions = '[2.6.0,)' + assertInverse = true + } + pass { + group = 'com.typesafe.play' + module = 'play_2.12' + versions = '[2.6.0,)' + assertInverse = true + } + pass { + group = 'com.typesafe.play' + module = 'play_2.13' + versions = '[2.6.0,)' + assertInverse = true + } +} + +apply from: "${rootDir}/gradle/java.gradle" + +apply plugin: 'org.unbroken-dome.test-sets' + +testSets { + latestDepTest { + dirName = 'test' + } +} + +def scalaVersion = '2.11' +def playVersion = '2.6.0' + +dependencies { + main_java8Compile group: 'com.typesafe.play', name: "play_$scalaVersion", version: playVersion + + testCompile project(':dd-java-agent:instrumentation:java-concurrent') + testCompile project(':dd-java-agent:instrumentation:netty-4.0') + testCompile project(':dd-java-agent:instrumentation:netty-4.1') + testCompile project(':dd-java-agent:instrumentation:akka-http-10.0') + + testCompile group: 'com.typesafe.play', name: "play-java_$scalaVersion", version: playVersion + // TODO: Play WS is a separately versioned library starting with 2.6 and needs separate instrumentation. + testCompile (group: 'com.typesafe.play', name: "play-test_$scalaVersion", version: playVersion) { + exclude group: 'org.eclipse.jetty.websocket', module: 'websocket-client' + } + + latestDepTestCompile group: 'com.typesafe.play', name: "play-java_$scalaVersion", version: '2.+' + latestDepTestCompile (group: 'com.typesafe.play', name: "play-test_$scalaVersion", version: '2.+') { + exclude group: 'org.eclipse.jetty.websocket', module: 'websocket-client' + } +} diff --git a/dd-java-agent/instrumentation/play-2.6/src/main/java/datadog/trace/instrumentation/play26/PlayInstrumentation.java b/dd-java-agent/instrumentation/play-2.6/src/main/java/datadog/trace/instrumentation/play26/PlayInstrumentation.java new file mode 100644 index 0000000000..6b0dee65ad --- /dev/null +++ b/dd-java-agent/instrumentation/play-2.6/src/main/java/datadog/trace/instrumentation/play26/PlayInstrumentation.java @@ -0,0 +1,48 @@ +package datadog.trace.instrumentation.play26; + +import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; +import static java.util.Collections.singletonMap; +import static net.bytebuddy.matcher.ElementMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.returns; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import java.util.Map; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +@AutoService(Instrumenter.class) +public final class PlayInstrumentation extends Instrumenter.Default { + + public PlayInstrumentation() { + super("play"); + } + + @Override + public ElementMatcher typeMatcher() { + return safeHasSuperType(named("play.api.mvc.Action")); + } + + @Override + public String[] helperClassNames() { + return new String[] { + "datadog.trace.agent.decorator.BaseDecorator", + "datadog.trace.agent.decorator.ServerDecorator", + "datadog.trace.agent.decorator.HttpServerDecorator", + packageName + ".PlayHttpServerDecorator", + packageName + ".RequestCompleteCallback", + packageName + ".PlayHeaders", + }; + } + + @Override + public Map, String> transformers() { + return singletonMap( + named("apply") + .and(takesArgument(0, named("play.api.mvc.Request"))) + .and(returns(named("scala.concurrent.Future"))), + packageName + ".PlayAdvice"); + } +} diff --git a/dd-java-agent/instrumentation/play-2.6/src/main/java8/datadog/trace/instrumentation/play26/PlayAdvice.java b/dd-java-agent/instrumentation/play-2.6/src/main/java8/datadog/trace/instrumentation/play26/PlayAdvice.java new file mode 100644 index 0000000000..15e11a7a62 --- /dev/null +++ b/dd-java-agent/instrumentation/play-2.6/src/main/java8/datadog/trace/instrumentation/play26/PlayAdvice.java @@ -0,0 +1,77 @@ +package datadog.trace.instrumentation.play26; + +import static datadog.trace.instrumentation.play26.PlayHttpServerDecorator.DECORATE; + +import datadog.trace.context.TraceScope; +import io.opentracing.Scope; +import io.opentracing.Span; +import io.opentracing.SpanContext; +import io.opentracing.propagation.Format; +import io.opentracing.tag.Tags; +import io.opentracing.util.GlobalTracer; +import net.bytebuddy.asm.Advice; +import play.api.mvc.Action; +import play.api.mvc.Request; +import play.api.mvc.Result; +import scala.concurrent.Future; + +public class PlayAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static Scope startSpan(@Advice.Argument(0) final Request req) { + final Scope scope; + if (GlobalTracer.get().activeSpan() == null) { + final SpanContext extractedContext; + if (GlobalTracer.get().scopeManager().active() == null) { + extractedContext = + GlobalTracer.get().extract(Format.Builtin.HTTP_HEADERS, new PlayHeaders(req)); + } else { + extractedContext = null; + } + scope = + GlobalTracer.get() + .buildSpan("play.request") + .asChildOf(extractedContext) + .startActive(false); + } else { + // An upstream framework (e.g. akka-http, netty) has already started the span. + // Do not extract the context. + scope = GlobalTracer.get().buildSpan("play.request").startActive(false); + } + DECORATE.afterStart(scope); + DECORATE.onConnection(scope.span(), req); + + if (GlobalTracer.get().scopeManager().active() instanceof TraceScope) { + ((TraceScope) GlobalTracer.get().scopeManager().active()).setAsyncPropagation(true); + } + return scope; + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void stopTraceOnResponse( + @Advice.Enter final Scope playControllerScope, + @Advice.This final Object thisAction, + @Advice.Thrown final Throwable throwable, + @Advice.Argument(0) final Request req, + @Advice.Return(readOnly = false) final Future responseFuture) { + final Span playControllerSpan = playControllerScope.span(); + + // Call onRequest on return after tags are populated. + DECORATE.onRequest(playControllerSpan, req); + + if (throwable == null) { + responseFuture.onComplete( + new RequestCompleteCallback(playControllerSpan), + ((Action) thisAction).executionContext()); + } else { + DECORATE.onError(playControllerSpan, throwable); + Tags.HTTP_STATUS.set(playControllerSpan, 500); + DECORATE.beforeFinish(playControllerSpan); + playControllerSpan.finish(); + } + playControllerScope.close(); + + final Span rootSpan = GlobalTracer.get().activeSpan(); + // set the resource name on the upstream akka/netty span + DECORATE.onRequest(rootSpan, req); + } +} diff --git a/dd-java-agent/instrumentation/play-2.6/src/main/java8/datadog/trace/instrumentation/play26/PlayHeaders.java b/dd-java-agent/instrumentation/play-2.6/src/main/java8/datadog/trace/instrumentation/play26/PlayHeaders.java new file mode 100644 index 0000000000..c888581d7b --- /dev/null +++ b/dd-java-agent/instrumentation/play-2.6/src/main/java8/datadog/trace/instrumentation/play26/PlayHeaders.java @@ -0,0 +1,33 @@ +package datadog.trace.instrumentation.play26; + +import io.opentracing.propagation.TextMap; +import java.util.HashMap; +import java.util.Iterator; +import java.util.Map; +import play.api.mvc.Request; +import scala.Tuple2; + +public class PlayHeaders implements TextMap { + private final Request request; + + public PlayHeaders(final Request request) { + this.request = request; + } + + @Override + public Iterator> iterator() { + final scala.collection.Map scalaMap = request.headers().toSimpleMap(); + final Map javaMap = new HashMap<>(scalaMap.size()); + final scala.collection.Iterator> scalaIterator = scalaMap.iterator(); + while (scalaIterator.hasNext()) { + final Tuple2 tuple = scalaIterator.next(); + javaMap.put(tuple._1(), tuple._2()); + } + return javaMap.entrySet().iterator(); + } + + @Override + public void put(final String s, final String s1) { + throw new IllegalStateException("play headers can only be extracted"); + } +} diff --git a/dd-java-agent/instrumentation/play-2.6/src/main/java8/datadog/trace/instrumentation/play26/PlayHttpServerDecorator.java b/dd-java-agent/instrumentation/play-2.6/src/main/java8/datadog/trace/instrumentation/play26/PlayHttpServerDecorator.java new file mode 100644 index 0000000000..72338bdf32 --- /dev/null +++ b/dd-java-agent/instrumentation/play-2.6/src/main/java8/datadog/trace/instrumentation/play26/PlayHttpServerDecorator.java @@ -0,0 +1,94 @@ +package datadog.trace.instrumentation.play26; + +import datadog.trace.agent.decorator.HttpServerDecorator; +import datadog.trace.api.DDTags; +import io.opentracing.Span; +import io.opentracing.tag.Tags; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.UndeclaredThrowableException; +import java.net.URI; +import java.net.URISyntaxException; +import lombok.extern.slf4j.Slf4j; +import play.api.mvc.Request; +import play.api.mvc.Result; +import play.api.routing.HandlerDef; +import play.routing.Router; +import scala.Option; + +@Slf4j +public class PlayHttpServerDecorator extends HttpServerDecorator { + public static final PlayHttpServerDecorator DECORATE = new PlayHttpServerDecorator(); + + @Override + protected String[] instrumentationNames() { + return new String[] {"play"}; + } + + @Override + protected String component() { + return "play-action"; + } + + @Override + protected String method(final Request httpRequest) { + return httpRequest.method(); + } + + @Override + protected URI url(final Request request) throws URISyntaxException { + return new URI(request.secure() ? "https://" : "http://" + request.host() + request.uri()); + } + + @Override + protected String peerHostname(final Request request) { + return null; + } + + @Override + protected String peerHostIP(final Request request) { + return request.remoteAddress(); + } + + @Override + protected Integer peerPort(final Request request) { + return null; + } + + @Override + protected Integer status(final Result httpResponse) { + return httpResponse.header().status(); + } + + @Override + public Span onRequest(final Span span, final Request request) { + super.onRequest(span, request); + if (request != null) { + // more about routes here: + // https://github.com/playframework/playframework/blob/master/documentation/manual/releases/release26/migration26/Migration26.md + final Option defOption = + request.attrs().get(Router.Attrs.HANDLER_DEF.underlying()); + if (!defOption.isEmpty()) { + final String path = defOption.get().path(); + span.setTag(DDTags.RESOURCE_NAME, request.method() + " " + path); + } + } + return span; + } + + @Override + public Span onError(final Span span, Throwable throwable) { + Tags.HTTP_STATUS.set(span, 500); + if (throwable != null + // This can be moved to instanceof check when using Java 8. + && throwable.getClass().getName().equals("java.util.concurrent.CompletionException") + && throwable.getCause() != null) { + throwable = throwable.getCause(); + } + while ((throwable instanceof InvocationTargetException + || throwable instanceof UndeclaredThrowableException) + && throwable.getCause() != null) { + throwable = throwable.getCause(); + } + return super.onError(span, throwable); + } +} diff --git a/dd-java-agent/instrumentation/play-2.6/src/main/java8/datadog/trace/instrumentation/play26/RequestCompleteCallback.java b/dd-java-agent/instrumentation/play-2.6/src/main/java8/datadog/trace/instrumentation/play26/RequestCompleteCallback.java new file mode 100644 index 0000000000..d79339fba8 --- /dev/null +++ b/dd-java-agent/instrumentation/play-2.6/src/main/java8/datadog/trace/instrumentation/play26/RequestCompleteCallback.java @@ -0,0 +1,39 @@ +package datadog.trace.instrumentation.play26; + +import static datadog.trace.instrumentation.play26.PlayHttpServerDecorator.DECORATE; + +import datadog.trace.context.TraceScope; +import io.opentracing.Span; +import io.opentracing.util.GlobalTracer; +import lombok.extern.slf4j.Slf4j; +import play.api.mvc.Result; +import scala.util.Try; + +@Slf4j +public class RequestCompleteCallback extends scala.runtime.AbstractFunction1, Object> { + private final Span span; + + public RequestCompleteCallback(final Span span) { + this.span = span; + } + + @Override + public Object apply(final Try result) { + try { + if (result.isFailure()) { + DECORATE.onError(span, result.failed().get()); + } else { + DECORATE.onResponse(span, result.get()); + } + DECORATE.beforeFinish(span); + if (GlobalTracer.get().scopeManager().active() instanceof TraceScope) { + ((TraceScope) GlobalTracer.get().scopeManager().active()).setAsyncPropagation(false); + } + } catch (final Throwable t) { + log.debug("error in play instrumentation", t); + } finally { + span.finish(); + } + return null; + } +} diff --git a/dd-java-agent/instrumentation/play-2.6/src/test/groovy/server/AkkaHttpTestInstrumentation.java b/dd-java-agent/instrumentation/play-2.6/src/test/groovy/server/AkkaHttpTestInstrumentation.java new file mode 100644 index 0000000000..cd10a5c4ca --- /dev/null +++ b/dd-java-agent/instrumentation/play-2.6/src/test/groovy/server/AkkaHttpTestInstrumentation.java @@ -0,0 +1,19 @@ +import static net.bytebuddy.matcher.ElementMatchers.named; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.test.base.HttpServerTestAdvice; +import datadog.trace.agent.tooling.Instrumenter; +import net.bytebuddy.agent.builder.AgentBuilder; + +@AutoService(Instrumenter.class) +public class AkkaHttpTestInstrumentation implements Instrumenter { + + @Override + public AgentBuilder instrument(final AgentBuilder agentBuilder) { + return agentBuilder + .type(named("akka.http.impl.engine.server.HttpServerBluePrint$PrepareRequests$$anon$1")) + .transform( + new AgentBuilder.Transformer.ForAdvice() + .advice(named("onPush"), HttpServerTestAdvice.ServerEntryAdvice.class.getName())); + } +} diff --git a/dd-java-agent/instrumentation/play-2.6/src/test/groovy/server/PlayAsyncServerTest.groovy b/dd-java-agent/instrumentation/play-2.6/src/test/groovy/server/PlayAsyncServerTest.groovy new file mode 100644 index 0000000000..1695438450 --- /dev/null +++ b/dd-java-agent/instrumentation/play-2.6/src/test/groovy/server/PlayAsyncServerTest.groovy @@ -0,0 +1,64 @@ +package server + +import play.BuiltInComponents +import play.Mode +import play.libs.concurrent.HttpExecution +import play.mvc.Results +import play.routing.RoutingDsl +import play.server.Server +import spock.lang.Shared + +import java.util.concurrent.CompletableFuture +import java.util.concurrent.Executors +import java.util.function.Supplier + +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS + +class PlayAsyncServerTest extends PlayServerTest { + @Shared + def executor = Executors.newCachedThreadPool() + + def cleanupSpec() { + executor.shutdown() + } + + @Override + Server startServer(int port) { + def execContext = HttpExecution.fromThread(executor) + return Server.forRouter(Mode.TEST, port) { BuiltInComponents components -> + RoutingDsl.fromComponents(components) + .GET(SUCCESS.getPath()).routeAsync({ + CompletableFuture.supplyAsync({ + controller(SUCCESS) { + Results.status(SUCCESS.getStatus(), SUCCESS.getBody()) + } + }, execContext) + } as Supplier) + .GET(REDIRECT.getPath()).routeAsync({ + CompletableFuture.supplyAsync({ + controller(REDIRECT) { + Results.found(REDIRECT.getBody()) + } + }, execContext) + } as Supplier) + .GET(ERROR.getPath()).routeAsync({ + CompletableFuture.supplyAsync({ + controller(ERROR) { + Results.status(ERROR.getStatus(), ERROR.getBody()) + } + }, execContext) + } as Supplier) + .GET(EXCEPTION.getPath()).routeAsync({ + CompletableFuture.supplyAsync({ + controller(EXCEPTION) { + throw new Exception(EXCEPTION.getBody()) + } + }, execContext) + } as Supplier) + .build() + } + } +} diff --git a/dd-java-agent/instrumentation/play-2.6/src/test/groovy/server/PlayServerTest.groovy b/dd-java-agent/instrumentation/play-2.6/src/test/groovy/server/PlayServerTest.groovy new file mode 100644 index 0000000000..f38e407118 --- /dev/null +++ b/dd-java-agent/instrumentation/play-2.6/src/test/groovy/server/PlayServerTest.groovy @@ -0,0 +1,137 @@ +package server + +import datadog.opentracing.DDSpan +import datadog.trace.agent.test.asserts.TraceAssert +import datadog.trace.agent.test.base.HttpServerTest +import datadog.trace.api.DDSpanTypes +import datadog.trace.instrumentation.akkahttp.AkkaHttpServerDecorator +import datadog.trace.instrumentation.play26.PlayHttpServerDecorator +import io.opentracing.tag.Tags +import play.BuiltInComponents +import play.Mode +import play.mvc.Results +import play.routing.RoutingDsl +import play.server.Server + +import java.util.function.Supplier + +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.ERROR +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.EXCEPTION +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.REDIRECT +import static datadog.trace.agent.test.base.HttpServerTest.ServerEndpoint.SUCCESS + +class PlayServerTest extends HttpServerTest { + @Override + Server startServer(int port) { + return Server.forRouter(Mode.TEST, port) { BuiltInComponents components -> + RoutingDsl.fromComponents(components) + .GET(SUCCESS.getPath()).routeTo({ + controller(SUCCESS) { + Results.status(SUCCESS.getStatus(), SUCCESS.getBody()) + } + } as Supplier) + .GET(REDIRECT.getPath()).routeTo({ + controller(REDIRECT) { + Results.found(REDIRECT.getBody()) + } + } as Supplier) + .GET(ERROR.getPath()).routeTo({ + controller(ERROR) { + Results.status(ERROR.getStatus(), ERROR.getBody()) + } + } as Supplier) + .GET(EXCEPTION.getPath()).routeTo({ + controller(EXCEPTION) { + throw new Exception(EXCEPTION.getBody()) + } + } as Supplier) + .build() + } + } + + @Override + void stopServer(Server server) { + server.stop() + } + + @Override + AkkaHttpServerDecorator decorator() { + return AkkaHttpServerDecorator.DECORATE + } + + @Override + String expectedOperationName() { + return "akka-http.request" + } + + @Override + boolean hasHandlerSpan() { + true + } + + @Override + // Return the handler span's name + String reorderHandlerSpan() { + "play.request" + } + + boolean testExceptionBody() { + // I can't figure out how to set a proper exception handler to customize the response body. + false + } + + @Override + void handlerSpan(TraceAssert trace, int index, Object parent, ServerEndpoint endpoint = SUCCESS) { + trace.span(index) { + serviceName expectedServiceName() + operationName "play.request" + spanType DDSpanTypes.HTTP_SERVER + errored endpoint == ERROR || endpoint == EXCEPTION + childOf(parent as DDSpan) + tags { + "$Tags.COMPONENT.key" PlayHttpServerDecorator.DECORATE.component() + "$Tags.HTTP_STATUS.key" Integer + "$Tags.HTTP_URL.key" String + "$Tags.PEER_HOST_IPV4.key" { it == null || it == "127.0.0.1" } // Optional + "$Tags.HTTP_METHOD.key" String + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + defaultTags() + if (endpoint == ERROR) { + "$Tags.ERROR.key" true + } else if (endpoint == EXCEPTION) { + errorTags(Exception, EXCEPTION.body) + } + } + } + } + + void serverSpan(TraceAssert trace, int index, String traceID = null, String parentID = null, String method = "GET", ServerEndpoint endpoint = SUCCESS) { + trace.span(index) { + serviceName expectedServiceName() + operationName expectedOperationName() + resourceName endpoint.status == 404 ? "404" : "$method ${endpoint.resolve(address).path}" + spanType DDSpanTypes.HTTP_SERVER + errored endpoint.errored + if (parentID != null) { + traceId traceID + parentId parentID + } else { + parent() + } + tags { + defaultTags(true) + "$Tags.COMPONENT.key" serverDecorator.component() + if (endpoint.errored) { + "$Tags.ERROR.key" endpoint.errored + "error.msg" { it == null || it == EXCEPTION.body } + "error.type" { it == null || it == Exception.name } + "error.stack" { it == null || it instanceof String } + } + "$Tags.HTTP_STATUS.key" endpoint.status + "$Tags.HTTP_URL.key" "${endpoint.resolve(address)}" + "$Tags.HTTP_METHOD.key" method + "$Tags.SPAN_KIND.key" Tags.SPAN_KIND_SERVER + } + } + } +} diff --git a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy index 004c06921c..984b8f43bc 100644 --- a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy +++ b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy @@ -57,6 +57,10 @@ abstract class HttpServerTest ext abstract SERVER startServer(int port) def cleanupSpec() { + if (server == null) { + println getClass().name + " can't stop null server" + return + } stopServer(server) server = null println getClass().name + " http server stopped at: http://localhost:$port/" @@ -76,8 +80,13 @@ abstract class HttpServerTest ext false } + // Return the handler span's name + String reorderHandlerSpan() { + null + } + boolean reorderControllerSpan() { - true + false } boolean testNotFound() { @@ -356,7 +365,18 @@ abstract class HttpServerTest ext assert toRemove.size() == size TEST_WRITER.removeAll(toRemove) - if(reorderControllerSpan()) { + if (reorderHandlerSpan()) { + TEST_WRITER.each { + def controllerSpan = it.find { + it.operationName == reorderHandlerSpan() + } + if (controllerSpan) { + it.remove(controllerSpan) + it.add(controllerSpan) + } + } + } + if (reorderControllerSpan() || reorderHandlerSpan()) { // Some frameworks close the handler span before the controller returns, so we need to manually reorder it. TEST_WRITER.each { def controllerSpan = it.find { diff --git a/settings.gradle b/settings.gradle index e5f4eb2adc..69802bcc9c 100644 --- a/settings.gradle +++ b/settings.gradle @@ -80,6 +80,7 @@ include ':dd-java-agent:instrumentation:netty-4.1' include ':dd-java-agent:instrumentation:okhttp-3' include ':dd-java-agent:instrumentation:osgi-classloading' include ':dd-java-agent:instrumentation:play-2.4' +include ':dd-java-agent:instrumentation:play-2.6' include ':dd-java-agent:instrumentation:rabbitmq-amqp-2.7' include ':dd-java-agent:instrumentation:ratpack-1.4' include ':dd-java-agent:instrumentation:reactor-core-3.1' From 263c442bdb8983f89016a9e815b4ac3426551d4a Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Tue, 27 Aug 2019 12:06:21 -0400 Subject: [PATCH 3/3] Fix order of operations --- .../trace/instrumentation/play24/PlayHttpServerDecorator.java | 2 +- .../trace/instrumentation/play26/PlayHttpServerDecorator.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dd-java-agent/instrumentation/play-2.4/src/main/java8/datadog/trace/instrumentation/play24/PlayHttpServerDecorator.java b/dd-java-agent/instrumentation/play-2.4/src/main/java8/datadog/trace/instrumentation/play24/PlayHttpServerDecorator.java index e6e0468490..a13eeafb07 100644 --- a/dd-java-agent/instrumentation/play-2.4/src/main/java8/datadog/trace/instrumentation/play24/PlayHttpServerDecorator.java +++ b/dd-java-agent/instrumentation/play-2.4/src/main/java8/datadog/trace/instrumentation/play24/PlayHttpServerDecorator.java @@ -34,7 +34,7 @@ public class PlayHttpServerDecorator extends HttpServerDecorator