From c6841c9d0642044279687654abba69de3adaab45 Mon Sep 17 00:00:00 2001 From: Andrew Kent Date: Mon, 20 Aug 2018 18:39:52 -0700 Subject: [PATCH 1/5] Add maven version scanning to muzzle --- .../src/main/groovy/MuzzleExtension.groovy | 4 - buildSrc/src/main/groovy/MuzzlePlugin.groovy | 255 ++++++++++++++++-- .../muzzle/MuzzleVersionScanPlugin.java | 78 ++++-- 3 files changed, 287 insertions(+), 50 deletions(-) delete mode 100644 buildSrc/src/main/groovy/MuzzleExtension.groovy diff --git a/buildSrc/src/main/groovy/MuzzleExtension.groovy b/buildSrc/src/main/groovy/MuzzleExtension.groovy deleted file mode 100644 index d8af853c9e..0000000000 --- a/buildSrc/src/main/groovy/MuzzleExtension.groovy +++ /dev/null @@ -1,4 +0,0 @@ -class MuzzleExtension { - String group - String module -} diff --git a/buildSrc/src/main/groovy/MuzzlePlugin.groovy b/buildSrc/src/main/groovy/MuzzlePlugin.groovy index ef92b5ff7a..4e6a6c4da5 100644 --- a/buildSrc/src/main/groovy/MuzzlePlugin.groovy +++ b/buildSrc/src/main/groovy/MuzzlePlugin.groovy @@ -1,32 +1,61 @@ +import org.apache.maven.repository.internal.MavenRepositorySystemUtils +import org.eclipse.aether.DefaultRepositorySystemSession +import org.eclipse.aether.RepositorySystem +import org.eclipse.aether.RepositorySystemSession +import org.eclipse.aether.artifact.Artifact +import org.eclipse.aether.artifact.DefaultArtifact +import org.eclipse.aether.connector.basic.BasicRepositoryConnectorFactory +import org.eclipse.aether.impl.DefaultServiceLocator +import org.eclipse.aether.repository.LocalRepository +import org.eclipse.aether.repository.RemoteRepository +import org.eclipse.aether.resolution.VersionRangeRequest +import org.eclipse.aether.resolution.VersionRangeResult +import org.eclipse.aether.spi.connector.RepositoryConnectorFactory +import org.eclipse.aether.spi.connector.transport.TransporterFactory +import org.eclipse.aether.transport.http.HttpTransporterFactory +import org.eclipse.aether.version.Version +import org.gradle.api.Action import org.gradle.api.Plugin import org.gradle.api.Project +import org.gradle.api.Task +import org.gradle.api.model.ObjectFactory import java.lang.reflect.Method /** - * muzzle task plugin which runs muzzle validation against an instrumentation's compile-time dependencies. - * - *

TODO: merge this with version scan + * muzzle task plugin which runs muzzle validation against a range of dependencies. */ class MuzzlePlugin implements Plugin { + /** + * Remote repositories used to query version ranges and fetch dependencies + */ + private static final List MUZZLE_REPOS + static { + RemoteRepository central = new RemoteRepository.Builder("central", "default", "http://central.maven.org/maven2/").build() + MUZZLE_REPOS = new ArrayList(Arrays.asList(central)) + } + @Override void apply(Project project) { def bootstrapProject = project.rootProject.getChildProjects().get('dd-java-agent').getChildProjects().get('agent-bootstrap') def toolingProject = project.rootProject.getChildProjects().get('dd-java-agent').getChildProjects().get('agent-tooling') - project.extensions.create("muzzle", MuzzleExtension) - def compileMuzzle = project.task('compileMuzzle') { - // not adding user and group to hide this from `gradle tasks` - } + project.extensions.create("muzzle", MuzzleExtension, project.objects) + + // compileMuzzle compiles all projects required to run muzzle validation. + // Not adding group and description to keep this task from showing in `gradle tasks`. + def compileMuzzle = project.task('compileMuzzle') def muzzle = project.task('muzzle') { group = 'Muzzle' description = "Run instrumentation muzzle on compile time dependencies" doLast { - final ClassLoader userCL = createUserClassLoader(project, bootstrapProject) - final ClassLoader agentCL = createDDClassloader(project, toolingProject) - // find all instrumenters, get muzzle, and assert - Method assertionMethod = agentCL.loadClass('datadog.trace.agent.tooling.muzzle.MuzzleVersionScanPlugin') - .getMethod('assertInstrumentationNotMuzzled', ClassLoader.class) - assertionMethod.invoke(null, userCL) + if (project.muzzle.passDirectives.size() == 0) { + project.getLogger().info('No muzzle pass directives configured. Asserting pass against instrumentation compile-time dependencies') + final ClassLoader userCL = createCompileDepsClassLoader(project, bootstrapProject) + final ClassLoader agentCL = createDDClassloader(project, toolingProject) + Method assertionMethod = agentCL.loadClass('datadog.trace.agent.tooling.muzzle.MuzzleVersionScanPlugin') + .getMethod('assertInstrumentationMuzzled', ClassLoader.class, boolean.class) + assertionMethod.invoke(null, userCL, true) + } } } def printReferences = project.task('printReferences') { @@ -47,15 +76,52 @@ class MuzzlePlugin implements Plugin { project.tasks.compileMuzzle.dependsOn(project.tasks.compileScala) } } - + // TODO: consider: + // project.tasks.withType(ScalaCompile) { Task scalaTask -> + // project.tasks.compileMuzzle.dependsOn(scalaTask) + // } project.tasks.muzzle.dependsOn(project.tasks.compileMuzzle) project.tasks.printReferences.dependsOn(project.tasks.compileMuzzle) + + def hasRelevantTask = project.gradle.startParameter.taskNames.any { taskName -> + // removing leading ':' if present + taskName = taskName.replaceFirst('^:', '') + String muzzleTaskPath = project.path.replaceFirst('^:', '') + return 'muzzle' == taskName || "${muzzleTaskPath}:muzzle" == taskName + } + if (!hasRelevantTask) { + // Adding muzzle dependencies has a large config overhead. Stop unless muzzle is explicitly run. + return + } + + final RepositorySystem system = newRepositorySystem() + final RepositorySystemSession session = newRepositorySystemSession(system) + + project.afterEvaluate { + // use runAfter to set up task finalizers in version order + Task runAfter = project.tasks.muzzle + + for (MuzzleDirective pass : project.muzzle.passDirectives) { + project.getLogger().info("configured pass directive: ${pass.group}:${pass.module}:${pass.versions}") + + muzzleDirectiveToArtifacts(pass, system, session).collect() { Artifact singleVersion -> + runAfter = addMuzzleTask(true, singleVersion, project, runAfter, bootstrapProject, toolingProject) + } + } + for (MuzzleDirective fail : project.muzzle.failDirectives) { + project.getLogger().info("configured fail directive: ${fail.group}:${fail.module}:${fail.versions}") + + muzzleDirectiveToArtifacts(fail, system, session).collect() { Artifact singleVersion -> + runAfter = addMuzzleTask(false, singleVersion, project, runAfter, bootstrapProject, toolingProject) + } + } + } } /** * Create a classloader with core agent classes and project instrumentation on the classpath. */ - private ClassLoader createDDClassloader(Project project, Project toolingProject) { + private static ClassLoader createDDClassloader(Project project, Project toolingProject) { project.getLogger().info("Creating dd classpath for: " + project.getName()) Set ddUrls = new HashSet<>() for (File f : toolingProject.sourceSets.main.runtimeClasspath.getFiles()) { @@ -71,11 +137,11 @@ class MuzzlePlugin implements Plugin { } /** - * Create a classloader with user/library classes on the classpath. + * Create a classloader with all compile-time dependencies on the classpath */ - private ClassLoader createUserClassLoader(Project project, Project bootstrapProject) { + private static ClassLoader createCompileDepsClassLoader(Project project, Project bootstrapProject) { List userUrls = new ArrayList<>() - project.getLogger().info("Creating user classpath for: " + project.getName()) + project.getLogger().info("Creating compile-time classpath for: " + project.getName()) for (File f : project.configurations.compileOnly.getFiles()) { project.getLogger().info('--' + f) userUrls.add(f.toURI().toURL()) @@ -86,4 +152,157 @@ class MuzzlePlugin implements Plugin { } return new URLClassLoader(userUrls.toArray(new URL[0]), (ClassLoader) null) } + + /** + * Create a classloader with dependencies for a single muzzle task. + */ + private static ClassLoader createClassLoaderForTask(Project project, Project bootstrapProject, String muzzleTaskName) { + final List userUrls = new ArrayList<>() + + project.getLogger().info("Creating task classpath") + project.configurations.getByName(muzzleTaskName).resolvedConfiguration.files.each { File jarFile -> + project.getLogger().info("-- Added to instrumentation classpath: $jarFile") + userUrls.add(jarFile.toURI().toURL()) + } + + for (File f : bootstrapProject.sourceSets.main.runtimeClasspath.getFiles()) { + project.getLogger().info("-- Added to instrumentation bootstrap classpath: $f") + userUrls.add(f.toURI().toURL()) + } + return new URLClassLoader(userUrls.toArray(new URL[0]), (ClassLoader) null) + } + + /** + * Convert a muzzle directive to a list of artifacts + */ + private static List muzzleDirectiveToArtifacts(MuzzleDirective muzzleDirective, RepositorySystem system, RepositorySystemSession session) { + final Artifact directiveArtifact = new DefaultArtifact(muzzleDirective.group, muzzleDirective.module, "jar", muzzleDirective.versions) + + final VersionRangeRequest rangeRequest = new VersionRangeRequest() + rangeRequest.setRepositories(MUZZLE_REPOS) + rangeRequest.setArtifact(directiveArtifact) + final VersionRangeResult rangeResult = system.resolveVersionRange(session, rangeRequest) + + final List allVersionArtifacts = filterVersion(rangeResult.versions).collect { version -> + new DefaultArtifact(muzzleDirective.group, muzzleDirective.module, "jar", version.toString()) + } + + return allVersionArtifacts + } + + /** + * Configure a muzzle task to pass or fail a given version. + * + * @param assertPass If true, assert that muzzle validation passes + * @param versionArtifact version to assert against. + * @param instrumentationProject instrumentation being asserted against. + * @param runAfter Task which runs before the new muzzle task. + * @param bootstrapProject Agent bootstrap project. + * @param toolingProject Agent tooling project. + * + * @return The created muzzle task. + */ + private static Task addMuzzleTask(boolean assertPass, Artifact versionArtifact, Project instrumentationProject, Task runAfter, Project bootstrapProject, Project toolingProject) { + def taskName = "muzzle-Assert${assertPass ? "Pass" : "Fail"}-$versionArtifact.groupId-$versionArtifact.artifactId-$versionArtifact.version" + def config = instrumentationProject.configurations.create(taskName) + config.dependencies.add(instrumentationProject.dependencies.create("$versionArtifact.groupId:$versionArtifact.artifactId:$versionArtifact.version") { + transitive = true + }) + + def muzzleTask = instrumentationProject.task(taskName) { + doLast { + final ClassLoader userCL = createClassLoaderForTask(instrumentationProject, bootstrapProject, taskName) + final ClassLoader agentCL = createDDClassloader(instrumentationProject, toolingProject) + // find all instrumenters, get muzzle, and assert + Method assertionMethod = agentCL.loadClass('datadog.trace.agent.tooling.muzzle.MuzzleVersionScanPlugin') + .getMethod('assertInstrumentationMuzzled', ClassLoader.class, boolean.class) + assertionMethod.invoke(null, userCL, assertPass) + } + } + runAfter.finalizedBy(muzzleTask) + return muzzleTask + } + + /** + * Create muzzle's repository system + */ + private static RepositorySystem newRepositorySystem() { + DefaultServiceLocator locator = MavenRepositorySystemUtils.newServiceLocator() + locator.addService(RepositoryConnectorFactory.class, BasicRepositoryConnectorFactory.class) + locator.addService(TransporterFactory.class, HttpTransporterFactory.class) + + return locator.getService(RepositorySystem.class) + } + + /** + * Create muzzle's repository system session + */ + private static RepositorySystemSession newRepositorySystemSession(RepositorySystem system) { + DefaultRepositorySystemSession session = MavenRepositorySystemUtils.newSession() + + def tempDir = File.createTempDir() + tempDir.deleteOnExit() + LocalRepository localRepo = new LocalRepository(tempDir) + session.setLocalRepositoryManager(system.newLocalRepositoryManager(session, localRepo)) + + return session + } + + /** + * Filter out snapshot-type builds from versions list. + */ + private static filterVersion(List list) { + list.removeIf { + def version = it.toString().toLowerCase() + return version.contains("rc") || + version.contains(".cr") || + version.contains("alpha") || + version.contains("beta") || + version.contains("-b") || + version.contains(".m") || + version.contains("-dev") || + version.contains("public_draft") + } + return list + } +} + +// plugin extension classes + +/** + * A pass or fail directive for a single dependency. + */ +class MuzzleDirective { + String group + String module + String versions +} + +/** + * Muzzle extension containing all pass and fail directives. + */ +class MuzzleExtension { + // TODO: merge pass and fail directives into single collection + final List passDirectives + final List failDirectives + private final ObjectFactory objectFactory + + @javax.inject.Inject + MuzzleExtension(final ObjectFactory objectFactory) { + this.objectFactory = objectFactory + passDirectives = new ArrayList<>() + failDirectives = new ArrayList<>() + } + + void pass(Action action) { + final MuzzleDirective pass = objectFactory.newInstance(MuzzleDirective) + action.execute(pass) + passDirectives.add(pass) + } + + void fail(Action action) { + final MuzzleDirective fail = objectFactory.newInstance(MuzzleDirective) + action.execute(fail) + failDirectives.add(fail) + } } diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/MuzzleVersionScanPlugin.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/MuzzleVersionScanPlugin.java index 5490e6cdcd..f7a00b78db 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/MuzzleVersionScanPlugin.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/MuzzleVersionScanPlugin.java @@ -2,9 +2,12 @@ package datadog.trace.agent.tooling.muzzle; import datadog.trace.agent.tooling.HelperInjector; import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.bootstrap.WeakMap; import java.lang.reflect.Method; +import java.util.Collections; import java.util.List; import java.util.ServiceLoader; +import java.util.WeakHashMap; /** * Entry point for muzzle version scan gradle plugin. @@ -15,7 +18,19 @@ import java.util.ServiceLoader; *

Additionally, after a successful muzzle validation run each instrumenter's helper injector. */ public class MuzzleVersionScanPlugin { - public static void assertInstrumentationNotMuzzled(ClassLoader cl) throws Exception { + static { + // prevent WeakMap from logging warning while plugin is running + WeakMap.Provider.registerIfAbsent( + new WeakMap.Supplier() { + @Override + public WeakMap get() { + return new WeakMap.MapAdapter<>(Collections.synchronizedMap(new WeakHashMap())); + } + }); + } + + public static void assertInstrumentationMuzzled(ClassLoader cl, boolean assertPass) + throws Exception { // muzzle validate all instrumenters for (Instrumenter instrumenter : ServiceLoader.load(Instrumenter.class, MuzzleGradlePlugin.class.getClassLoader())) { @@ -36,7 +51,12 @@ public class MuzzleVersionScanPlugin { m.setAccessible(true); ReferenceMatcher muzzle = (ReferenceMatcher) m.invoke(instrumenter); List mismatches = muzzle.getMismatchedReferenceSources(cl); - if (mismatches.size() > 0) { + boolean passed = mismatches.size() == 0; + if (mismatches.size() > 0) {} + if (passed && !assertPass) { + System.err.println("MUZZLE PASSED BUT FAILURE WAS EXPECTED"); + throw new RuntimeException("Instrumentation unexpectedly passed Muzzle validation"); + } else if (!passed && assertPass) { System.err.println( "FAILED MUZZLE VALIDATION: " + instrumenter.getClass().getName() + " mismatches:"); for (Reference.Mismatch mismatch : mismatches) { @@ -51,33 +71,35 @@ public class MuzzleVersionScanPlugin { } } // run helper injector on all instrumenters - for (Instrumenter instrumenter : - ServiceLoader.load(Instrumenter.class, MuzzleGradlePlugin.class.getClassLoader())) { - if (instrumenter.getClass().getName().endsWith("TraceConfigInstrumentation")) { - // TraceConfigInstrumentation doesn't do muzzle checks - // check on TracerClassInstrumentation instead - instrumenter = - (Instrumenter) - MuzzleGradlePlugin.class - .getClassLoader() - .loadClass(instrumenter.getClass().getName() + "$TracerClassInstrumentation") - .getDeclaredConstructor() - .newInstance(); - } - try { - // Ratpack injects the scope manager as a helper. - // This is likely a bug, but we'll grandfather it out of the helper checks for now. - if (!instrumenter.getClass().getName().contains("Ratpack")) { - // verify helper injector works - final String[] helperClassNames = instrumenter.helperClassNames(); - if (helperClassNames.length > 0) { - new HelperInjector(helperClassNames).transform(null, null, cl, null); - } + if (assertPass) { + for (Instrumenter instrumenter : + ServiceLoader.load(Instrumenter.class, MuzzleGradlePlugin.class.getClassLoader())) { + if (instrumenter.getClass().getName().endsWith("TraceConfigInstrumentation")) { + // TraceConfigInstrumentation doesn't do muzzle checks + // check on TracerClassInstrumentation instead + instrumenter = + (Instrumenter) + MuzzleGradlePlugin.class + .getClassLoader() + .loadClass(instrumenter.getClass().getName() + "$TracerClassInstrumentation") + .getDeclaredConstructor() + .newInstance(); + } + try { + // Ratpack injects the scope manager as a helper. + // This is likely a bug, but we'll grandfather it out of the helper checks for now. + if (!instrumenter.getClass().getName().contains("Ratpack")) { + // verify helper injector works + final String[] helperClassNames = instrumenter.helperClassNames(); + if (helperClassNames.length > 0) { + new HelperInjector(helperClassNames).transform(null, null, cl, null); + } + } + } catch (Exception e) { + System.err.println( + "FAILED HELPER INJECTION. Are Helpers being injected in the correct order?"); + throw e; } - } catch (Exception e) { - System.err.println( - "FAILED HELPER INJECTION. Are Helpers being injected in the correct order?"); - throw e; } } } From f06eb5744931bb3f7a688b0ea1bdd05d3a1b05e7 Mon Sep 17 00:00:00 2001 From: Andrew Kent Date: Mon, 20 Aug 2018 18:41:17 -0700 Subject: [PATCH 2/5] Convert apache-http, akka, and play to muzzle plugin --- .../akka-http-10.0/akka-http-10.0.gradle | 14 +++++++ .../apache-httpclient-4.3.gradle | 42 ++++++++----------- .../ApacheHttpClientInstrumentation.java | 17 -------- .../instrumentation/play-2.4/play-2.4.gradle | 13 ++++++ .../play/PlayInstrumentation.java | 14 ------- 5 files changed, 45 insertions(+), 55 deletions(-) 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 e10a43321c..e92ed94488 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 @@ -16,6 +16,20 @@ compileLagomTestGroovy { targetCompatibility = 1.8 } + +muzzle { + pass { + group = 'com.typesafe.akka' + module = 'akka-http_2.11' + versions = "[10.0.0,10.0.12)" + } + pass { + group = 'com.typesafe.akka' + module = 'akka-http_2.12' + versions = "[10.0.0,10.0.12)" + } +} + dependencies { compileOnly group: 'com.typesafe.akka', name: 'akka-http_2.11', version: '10.0.0' diff --git a/dd-java-agent/instrumentation/apache-httpclient-4.3/apache-httpclient-4.3.gradle b/dd-java-agent/instrumentation/apache-httpclient-4.3/apache-httpclient-4.3.gradle index d4ed32568a..57014820f5 100644 --- a/dd-java-agent/instrumentation/apache-httpclient-4.3/apache-httpclient-4.3.gradle +++ b/dd-java-agent/instrumentation/apache-httpclient-4.3/apache-httpclient-4.3.gradle @@ -1,29 +1,23 @@ -apply plugin: 'version-scan' - -versionScan { - group = "org.apache.httpcomponents" - module = "httpclient" - versions = "[4.3,)" - legacyGroup = "commons-httpclient" - legacyModule = "commons-httpclient" - verifyPresent = [ - // The commented out classes are required for the instrumentation (this is checked by our bytebuddy rules). - // Once the verifier can report on the failed versions these classes can be commented out and the pass range can be widened. - - // "org.apache.http.HttpException" : null, - // "org.apache.http.HttpRequest" : null, - // "org.apache.http.client.RedirectStrategy" : null, - "org.apache.http.client.methods.CloseableHttpResponse": null, - "org.apache.http.client.methods.HttpExecutionAware" : null, - "org.apache.http.client.methods.HttpRequestWrapper" : null, - "org.apache.http.client.protocol.HttpClientContext" : null, - // "org.apache.http.conn.routing.HttpRoute" : null, - "org.apache.http.impl.execchain.ClientExecChain" : null - ] -} - apply from: "${rootDir}/gradle/java.gradle" +muzzle { + pass { + group = "org.apache.httpcomponents" + module = "httpclient" + versions = "[4.3,)" + } + pass { + group = "commons-httpclient" + module = "commons-httpclient" + versions = "[4.3,)" + } + fail { + group = "org.apache.httpcomponents" + module = "httpclient" + versions = "[,4.3)" + } +} + apply plugin: 'org.unbroken-dome.test-sets' testSets { diff --git a/dd-java-agent/instrumentation/apache-httpclient-4.3/src/main/java/datadog/trace/instrumentation/apachehttpclient/ApacheHttpClientInstrumentation.java b/dd-java-agent/instrumentation/apache-httpclient-4.3/src/main/java/datadog/trace/instrumentation/apachehttpclient/ApacheHttpClientInstrumentation.java index 1973afa5b0..2e4fde528c 100644 --- a/dd-java-agent/instrumentation/apache-httpclient-4.3/src/main/java/datadog/trace/instrumentation/apachehttpclient/ApacheHttpClientInstrumentation.java +++ b/dd-java-agent/instrumentation/apache-httpclient-4.3/src/main/java/datadog/trace/instrumentation/apachehttpclient/ApacheHttpClientInstrumentation.java @@ -1,7 +1,6 @@ package datadog.trace.instrumentation.apachehttpclient; import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; -import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; import static io.opentracing.log.Fields.ERROR_OBJECT; import static net.bytebuddy.matcher.ElementMatchers.isAbstract; import static net.bytebuddy.matcher.ElementMatchers.isMethod; @@ -36,22 +35,6 @@ public class ApacheHttpClientInstrumentation extends Instrumenter.Default { .or(safeHasSuperType(named("org.apache.http.impl.client.CloseableHttpClient"))); } - @Override - public ElementMatcher classLoaderMatcher() { - return classLoaderHasClasses( - "org.apache.http.HttpException", - "org.apache.http.HttpRequest", - "org.apache.http.client.RedirectStrategy", - "org.apache.http.client.methods.CloseableHttpResponse", - "org.apache.http.client.methods.HttpExecutionAware", - "org.apache.http.client.methods.HttpRequestWrapper", - "org.apache.http.client.protocol.HttpClientContext", - "org.apache.http.conn.routing.HttpRoute", - "org.apache.http.impl.execchain.ClientExecChain", - "org.apache.http.impl.client.CloseableHttpClient", - "org.apache.http.impl.client.InternalHttpClient"); - } - @Override public String[] helperClassNames() { return new String[] { 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 7173c28208..1014bab501 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 @@ -7,6 +7,19 @@ testSets { latestDepTest } +muzzle { + pass { + group = 'com.typesafe.play' + module = 'play_2.11' + versions = '[2.4.0,2.7.0-M1)' + } + pass { + group = 'com.typesafe.play' + module = 'play_2.12' + versions = '[2.4.0,2.7.0-M1)' + } +} + dependencies { compileOnly group: 'com.typesafe.play', name: 'play_2.11', version: '2.4.0' 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 index 15f41e3c92..0310342e70 100644 --- 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 @@ -1,8 +1,6 @@ package datadog.trace.instrumentation.play; import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.safeHasSuperType; -import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClassWithMethod; -import static datadog.trace.agent.tooling.ClassLoaderMatcher.classLoaderHasClasses; import static io.opentracing.log.Fields.ERROR_OBJECT; import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.returns; @@ -50,18 +48,6 @@ public final class PlayInstrumentation extends Instrumenter.Default { return safeHasSuperType(named("play.api.mvc.Action")); } - @Override - public ElementMatcher classLoaderMatcher() { - return classLoaderHasClasses( - "akka.japi.JavaPartialFunction", - "play.api.mvc.Action", - "play.api.mvc.Result", - "scala.Option", - "scala.Tuple2", - "scala.concurrent.Future") - .and(classLoaderHasClassWithMethod("play.api.mvc.Request", "tags")); - } - @Override public String[] helperClassNames() { return new String[] { From b6d1c18af99f61bba2f49f9a383cbbdf0cabe190 Mon Sep 17 00:00:00 2001 From: Andrew Kent Date: Mon, 20 Aug 2018 19:29:11 -0700 Subject: [PATCH 3/5] Allow muzzle directives to have extra dependencies --- buildSrc/src/main/groovy/MuzzlePlugin.groovy | 15 ++++++++++++--- .../akka-http-10.0/akka-http-10.0.gradle | 8 ++++++-- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/buildSrc/src/main/groovy/MuzzlePlugin.groovy b/buildSrc/src/main/groovy/MuzzlePlugin.groovy index 4e6a6c4da5..11d1a0ec03 100644 --- a/buildSrc/src/main/groovy/MuzzlePlugin.groovy +++ b/buildSrc/src/main/groovy/MuzzlePlugin.groovy @@ -105,14 +105,14 @@ class MuzzlePlugin implements Plugin { project.getLogger().info("configured pass directive: ${pass.group}:${pass.module}:${pass.versions}") muzzleDirectiveToArtifacts(pass, system, session).collect() { Artifact singleVersion -> - runAfter = addMuzzleTask(true, singleVersion, project, runAfter, bootstrapProject, toolingProject) + runAfter = addMuzzleTask(pass, true, singleVersion, project, runAfter, bootstrapProject, toolingProject) } } for (MuzzleDirective fail : project.muzzle.failDirectives) { project.getLogger().info("configured fail directive: ${fail.group}:${fail.module}:${fail.versions}") muzzleDirectiveToArtifacts(fail, system, session).collect() { Artifact singleVersion -> - runAfter = addMuzzleTask(false, singleVersion, project, runAfter, bootstrapProject, toolingProject) + runAfter = addMuzzleTask(fail, false, singleVersion, project, runAfter, bootstrapProject, toolingProject) } } } @@ -202,12 +202,17 @@ class MuzzlePlugin implements Plugin { * * @return The created muzzle task. */ - private static Task addMuzzleTask(boolean assertPass, Artifact versionArtifact, Project instrumentationProject, Task runAfter, Project bootstrapProject, Project toolingProject) { + private static Task addMuzzleTask(MuzzleDirective directive, boolean assertPass, Artifact versionArtifact, Project instrumentationProject, Task runAfter, Project bootstrapProject, Project toolingProject) { def taskName = "muzzle-Assert${assertPass ? "Pass" : "Fail"}-$versionArtifact.groupId-$versionArtifact.artifactId-$versionArtifact.version" def config = instrumentationProject.configurations.create(taskName) config.dependencies.add(instrumentationProject.dependencies.create("$versionArtifact.groupId:$versionArtifact.artifactId:$versionArtifact.version") { transitive = true }) + for (String additionalDependency : directive.additionalDependencies) { + config.dependencies.add(instrumentationProject.dependencies.create(additionalDependency) { + transitive = true + }) + } def muzzleTask = instrumentationProject.task(taskName) { doLast { @@ -276,6 +281,10 @@ class MuzzleDirective { String group String module String versions + List additionalDependencies = new ArrayList<>() + void extraDependency(String compileString) { + additionalDependencies.add(compileString) + } } /** 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 e92ed94488..34829de3c3 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 @@ -21,12 +21,16 @@ muzzle { pass { group = 'com.typesafe.akka' module = 'akka-http_2.11' - versions = "[10.0.0,10.0.12)" + versions = "[10.0.0,)" + // later versions of akka-http expect streams to be provided + extraDependency 'com.typesafe.akka:akka-stream_2.11:2.4.14' } pass { group = 'com.typesafe.akka' module = 'akka-http_2.12' - versions = "[10.0.0,10.0.12)" + versions = "[10.0.0,)" + // later versions of akka-http expect streams to be provided + extraDependency 'com.typesafe.akka:akka-stream_2.12:2.4.14' } } From 26705142bb2510104a9b2cf76837af417b8a291c Mon Sep 17 00:00:00 2001 From: Andrew Kent Date: Mon, 20 Aug 2018 20:03:23 -0700 Subject: [PATCH 4/5] Muzzle pass and fail directives in the same collection --- buildSrc/src/main/groovy/MuzzlePlugin.groovy | 44 +++++++------------ .../apache-httpclient-4.3.gradle | 10 ++--- 2 files changed, 21 insertions(+), 33 deletions(-) diff --git a/buildSrc/src/main/groovy/MuzzlePlugin.groovy b/buildSrc/src/main/groovy/MuzzlePlugin.groovy index 11d1a0ec03..3c8507f1ac 100644 --- a/buildSrc/src/main/groovy/MuzzlePlugin.groovy +++ b/buildSrc/src/main/groovy/MuzzlePlugin.groovy @@ -48,7 +48,7 @@ class MuzzlePlugin implements Plugin { group = 'Muzzle' description = "Run instrumentation muzzle on compile time dependencies" doLast { - if (project.muzzle.passDirectives.size() == 0) { + if (!project.muzzle.directives.any { it.assertPass }) { project.getLogger().info('No muzzle pass directives configured. Asserting pass against instrumentation compile-time dependencies') final ClassLoader userCL = createCompileDepsClassLoader(project, bootstrapProject) final ClassLoader agentCL = createDDClassloader(project, toolingProject) @@ -72,14 +72,10 @@ class MuzzlePlugin implements Plugin { project.tasks.compileMuzzle.dependsOn(toolingProject.tasks.compileJava) project.afterEvaluate { project.tasks.compileMuzzle.dependsOn(project.tasks.compileJava) - if (project.tasks.getNames().contains("compileScala")) { + if (project.tasks.getNames().contains('compileScala')) { project.tasks.compileMuzzle.dependsOn(project.tasks.compileScala) } } - // TODO: consider: - // project.tasks.withType(ScalaCompile) { Task scalaTask -> - // project.tasks.compileMuzzle.dependsOn(scalaTask) - // } project.tasks.muzzle.dependsOn(project.tasks.compileMuzzle) project.tasks.printReferences.dependsOn(project.tasks.compileMuzzle) @@ -101,18 +97,11 @@ class MuzzlePlugin implements Plugin { // use runAfter to set up task finalizers in version order Task runAfter = project.tasks.muzzle - for (MuzzleDirective pass : project.muzzle.passDirectives) { - project.getLogger().info("configured pass directive: ${pass.group}:${pass.module}:${pass.versions}") + for (MuzzleDirective muzzleDirective : project.muzzle.directives) { + project.getLogger().info("configured ${muzzleDirective.assertPass ? 'pass' : 'fail'} directive: ${muzzleDirective.group}:${muzzleDirective.module}:${muzzleDirective.versions}") - muzzleDirectiveToArtifacts(pass, system, session).collect() { Artifact singleVersion -> - runAfter = addMuzzleTask(pass, true, singleVersion, project, runAfter, bootstrapProject, toolingProject) - } - } - for (MuzzleDirective fail : project.muzzle.failDirectives) { - project.getLogger().info("configured fail directive: ${fail.group}:${fail.module}:${fail.versions}") - - muzzleDirectiveToArtifacts(fail, system, session).collect() { Artifact singleVersion -> - runAfter = addMuzzleTask(fail, false, singleVersion, project, runAfter, bootstrapProject, toolingProject) + muzzleDirectiveToArtifacts(muzzleDirective, system, session).collect() { Artifact singleVersion -> + runAfter = addMuzzleTask(muzzleDirective, singleVersion, project, runAfter, bootstrapProject, toolingProject) } } } @@ -202,13 +191,13 @@ class MuzzlePlugin implements Plugin { * * @return The created muzzle task. */ - private static Task addMuzzleTask(MuzzleDirective directive, boolean assertPass, Artifact versionArtifact, Project instrumentationProject, Task runAfter, Project bootstrapProject, Project toolingProject) { - def taskName = "muzzle-Assert${assertPass ? "Pass" : "Fail"}-$versionArtifact.groupId-$versionArtifact.artifactId-$versionArtifact.version" + private static Task addMuzzleTask(MuzzleDirective muzzleDirective, Artifact versionArtifact, Project instrumentationProject, Task runAfter, Project bootstrapProject, Project toolingProject) { + def taskName = "muzzle-Assert${muzzleDirective.assertPass ? "Pass" : "Fail"}-$versionArtifact.groupId-$versionArtifact.artifactId-$versionArtifact.version" def config = instrumentationProject.configurations.create(taskName) config.dependencies.add(instrumentationProject.dependencies.create("$versionArtifact.groupId:$versionArtifact.artifactId:$versionArtifact.version") { transitive = true }) - for (String additionalDependency : directive.additionalDependencies) { + for (String additionalDependency : muzzleDirective.additionalDependencies) { config.dependencies.add(instrumentationProject.dependencies.create(additionalDependency) { transitive = true }) @@ -221,7 +210,7 @@ class MuzzlePlugin implements Plugin { // find all instrumenters, get muzzle, and assert Method assertionMethod = agentCL.loadClass('datadog.trace.agent.tooling.muzzle.MuzzleVersionScanPlugin') .getMethod('assertInstrumentationMuzzled', ClassLoader.class, boolean.class) - assertionMethod.invoke(null, userCL, assertPass) + assertionMethod.invoke(null, userCL, muzzleDirective.assertPass) } } runAfter.finalizedBy(muzzleTask) @@ -282,6 +271,7 @@ class MuzzleDirective { String module String versions List additionalDependencies = new ArrayList<>() + boolean assertPass void extraDependency(String compileString) { additionalDependencies.add(compileString) } @@ -291,27 +281,25 @@ class MuzzleDirective { * Muzzle extension containing all pass and fail directives. */ class MuzzleExtension { - // TODO: merge pass and fail directives into single collection - final List passDirectives - final List failDirectives + final List directives = new ArrayList<>() private final ObjectFactory objectFactory @javax.inject.Inject MuzzleExtension(final ObjectFactory objectFactory) { this.objectFactory = objectFactory - passDirectives = new ArrayList<>() - failDirectives = new ArrayList<>() } void pass(Action action) { final MuzzleDirective pass = objectFactory.newInstance(MuzzleDirective) action.execute(pass) - passDirectives.add(pass) + pass.assertPass = true + directives.add(pass) } void fail(Action action) { final MuzzleDirective fail = objectFactory.newInstance(MuzzleDirective) action.execute(fail) - failDirectives.add(fail) + fail.assertPass = false + directives.add(fail) } } diff --git a/dd-java-agent/instrumentation/apache-httpclient-4.3/apache-httpclient-4.3.gradle b/dd-java-agent/instrumentation/apache-httpclient-4.3/apache-httpclient-4.3.gradle index 57014820f5..bed8c30d1f 100644 --- a/dd-java-agent/instrumentation/apache-httpclient-4.3/apache-httpclient-4.3.gradle +++ b/dd-java-agent/instrumentation/apache-httpclient-4.3/apache-httpclient-4.3.gradle @@ -6,16 +6,16 @@ muzzle { module = "httpclient" versions = "[4.3,)" } - pass { - group = "commons-httpclient" - module = "commons-httpclient" - versions = "[4.3,)" - } fail { group = "org.apache.httpcomponents" module = "httpclient" versions = "[,4.3)" } + fail { + group = "commons-httpclient" + module = "commons-httpclient" + versions = "[,4.3)" + } } apply plugin: 'org.unbroken-dome.test-sets' From 9338faa013fe45664f657c07193f4b9948ce8e6b Mon Sep 17 00:00:00 2001 From: Andrew Kent Date: Tue, 21 Aug 2018 10:32:56 -0700 Subject: [PATCH 5/5] Muzzle inverse assertions --- buildSrc/src/main/groovy/MuzzlePlugin.groovy | 43 +++++++++++++++++++ .../apache-httpclient-4.3.gradle | 16 +++---- 2 files changed, 49 insertions(+), 10 deletions(-) diff --git a/buildSrc/src/main/groovy/MuzzlePlugin.groovy b/buildSrc/src/main/groovy/MuzzlePlugin.groovy index 3c8507f1ac..dfd7ea8e4e 100644 --- a/buildSrc/src/main/groovy/MuzzlePlugin.groovy +++ b/buildSrc/src/main/groovy/MuzzlePlugin.groovy @@ -103,6 +103,13 @@ class MuzzlePlugin implements Plugin { muzzleDirectiveToArtifacts(muzzleDirective, system, session).collect() { Artifact singleVersion -> runAfter = addMuzzleTask(muzzleDirective, singleVersion, project, runAfter, bootstrapProject, toolingProject) } + if (muzzleDirective.assertInverse) { + inverseOf(muzzleDirective, system, session).collect() { MuzzleDirective inverseDirective -> + muzzleDirectiveToArtifacts(inverseDirective, system, session).collect() { Artifact singleVersion -> + runAfter = addMuzzleTask(inverseDirective, singleVersion, project, runAfter, bootstrapProject, toolingProject) + } + } + } } } } @@ -179,6 +186,41 @@ class MuzzlePlugin implements Plugin { return allVersionArtifacts } + /** + * Create a list of muzzle directives which assert the opposite of the given MuzzleDirective. + */ + private static List inverseOf(MuzzleDirective muzzleDirective, RepositorySystem system, RepositorySystemSession session) { + List inverseDirectives = new ArrayList<>() + + final Artifact allVerisonsArtifact = new DefaultArtifact(muzzleDirective.group, muzzleDirective.module, "jar", "[,)") + final Artifact directiveArtifact = new DefaultArtifact(muzzleDirective.group, muzzleDirective.module, "jar", muzzleDirective.versions) + + + final VersionRangeRequest allRangeRequest = new VersionRangeRequest() + allRangeRequest.setRepositories(MUZZLE_REPOS) + allRangeRequest.setArtifact(allVerisonsArtifact) + final VersionRangeResult allRangeResult = system.resolveVersionRange(session, allRangeRequest) + + final VersionRangeRequest rangeRequest = new VersionRangeRequest() + rangeRequest.setRepositories(MUZZLE_REPOS) + rangeRequest.setArtifact(directiveArtifact) + final VersionRangeResult rangeResult = system.resolveVersionRange(session, rangeRequest) + + filterVersion(allRangeResult.versions).collect { version -> + if (!rangeResult.versions.contains(version)) { + final MuzzleDirective inverseDirective = new MuzzleDirective() + inverseDirective.group = muzzleDirective.group + inverseDirective.module = muzzleDirective.module + inverseDirective.versions = "$version" + inverseDirective.assertPass = !muzzleDirective.assertPass + inverseDirectives.add(inverseDirective) + } + } + + return inverseDirectives + } + + /** * Configure a muzzle task to pass or fail a given version. * @@ -272,6 +314,7 @@ class MuzzleDirective { String versions List additionalDependencies = new ArrayList<>() boolean assertPass + boolean assertInverse = false void extraDependency(String compileString) { additionalDependencies.add(compileString) } diff --git a/dd-java-agent/instrumentation/apache-httpclient-4.3/apache-httpclient-4.3.gradle b/dd-java-agent/instrumentation/apache-httpclient-4.3/apache-httpclient-4.3.gradle index bed8c30d1f..75abaff407 100644 --- a/dd-java-agent/instrumentation/apache-httpclient-4.3/apache-httpclient-4.3.gradle +++ b/dd-java-agent/instrumentation/apache-httpclient-4.3/apache-httpclient-4.3.gradle @@ -1,21 +1,17 @@ apply from: "${rootDir}/gradle/java.gradle" muzzle { - pass { - group = "org.apache.httpcomponents" - module = "httpclient" - versions = "[4.3,)" - } - fail { - group = "org.apache.httpcomponents" - module = "httpclient" - versions = "[,4.3)" - } fail { group = "commons-httpclient" module = "commons-httpclient" versions = "[,4.3)" } + pass { + group = "org.apache.httpcomponents" + module = "httpclient" + versions = "[4.3,)" + assertInverse = true + } } apply plugin: 'org.unbroken-dome.test-sets'