Merge pull request #23807 Enable agent-based instrumentation by default

Fixes #23297

Co-authored-by: Mikhail Lopatkin <mlopatkin@gradle.com>

build-logic/integration-testing/src/main/kotlin/gradlebuild/integrationtests/shared-configuration.kt

@@ -179,16 +179,17 @@ fun Project.createTestTask(name: String, executer: String, sourceSet: SourceSet,
 
 private
 fun IntegrationTest.setUpAgentIfNeeded(testType: TestType, executer: String) {
+    if (executer == "embedded") {
+        // Apply the instrumentation agent to the test process when running integration tests with embedded Gradle executer.
+        jvmArgumentProviders.add(project.objects.newInstance<AgentsClasspathProvider>().apply {
+            agentsClasspath.from(project.configurations["${testType.prefix}TestAgentsClasspath"])
+        })
+    }
+
     val integTestUseAgentSysPropName = "org.gradle.integtest.agent.allowed"
     if (project.hasProperty(integTestUseAgentSysPropName)) {
         val shouldUseAgent = (project.property(integTestUseAgentSysPropName) as? String).toBoolean()
         systemProperties[integTestUseAgentSysPropName] = shouldUseAgent.toString()
-
-        if (shouldUseAgent && executer == "embedded") {
-            jvmArgumentProviders.add(project.objects.newInstance<AgentsClasspathProvider>().apply {
-                agentsClasspath.from(project.configurations["${testType.prefix}TestAgentsClasspath"])
-            })
-        }
     }
 }
 

subprojects/integ-test/src/crossVersionTest/groovy/org/gradle/integtests/TapiAgentInstrumentationCrossVersionSpec.groovy

@@ -28,14 +28,14 @@ class TapiAgentInstrumentationCrossVersionSpec extends ToolingApiSpecification {
         toolingApi.requireDaemons()
     }
 
-    def "agent is disabled in TAPI by default"() {
+    def "agent is enabled in TAPI by default"() {
         withDumpAgentStatusTask()
 
         when:
         runDumpTaskWithTapi()
 
         then:
-        agentWasNotApplied()
+        agentWasApplied()
     }
 
     def "agent is applied if enabled in settings"() {

subprojects/integ-test/src/integTest/groovy/org/gradle/instrumentation/agent/AgentApplicationTest.groovy

@@ -19,13 +19,11 @@ package org.gradle.instrumentation.agent
 import org.gradle.integtests.fixtures.AbstractIntegrationSpec
 import org.gradle.integtests.fixtures.configurationcache.ConfigurationCacheFixture
 import org.gradle.integtests.fixtures.daemon.DaemonLogsAnalyzer
-import org.gradle.integtests.fixtures.executer.AbstractGradleExecuter
 import org.gradle.integtests.fixtures.executer.GradleContextualExecuter
 import org.gradle.integtests.fixtures.executer.GradleHandle
 import org.gradle.internal.agents.AgentStatus
 import org.gradle.launcher.daemon.configuration.DaemonBuildOptions
 import org.gradle.util.TestPrecondition
-import spock.lang.IgnoreIf
 import spock.lang.Requires
 
 import static org.gradle.test.fixtures.ConcurrentTestUtil.poll
@@ -34,16 +32,15 @@ import static org.gradle.test.fixtures.ConcurrentTestUtil.poll
 // the test runtime classpath as part of the main source set's output.
 // It is important to have the agent appended to the classpath of all integration tests.
 class AgentApplicationTest extends AbstractIntegrationSpec {
-    @IgnoreIf({ AbstractGradleExecuter.agentInstrumentationEnabled }) // TODO: agent is enabled by default in tests
-    def "agent is disabled by default"() {
+    def "agent is enabled by default"() {
         given:
         withDumpAgentStatusTask()
 
         when:
         succeeds()
 
         then:
-        agentWasNotApplied()
+        agentWasApplied()
     }
 
     def "agent is not applied if disabled in the command-line"() {

subprojects/internal-integ-testing/src/main/groovy/org/gradle/integtests/fixtures/executer/AbstractGradleExecuter.java

@@ -837,7 +837,7 @@ public boolean isUseDaemon() {
     }
 
     public static boolean isAgentInstrumentationEnabled() {
-        return Boolean.getBoolean(ALLOW_INSTRUMENTATION_AGENT_SYSPROP);
+        return Boolean.parseBoolean(System.getProperty(ALLOW_INSTRUMENTATION_AGENT_SYSPROP, "true"));
     }
 
     @Override

subprojects/launcher/src/main/java/org/gradle/launcher/daemon/configuration/DaemonParameters.java

@@ -48,7 +48,7 @@ public class DaemonParameters {
 
     private int periodicCheckInterval = DEFAULT_PERIODIC_CHECK_INTERVAL_MILLIS;
     private final DaemonJvmOptions jvmOptions;
-    private boolean applyInstrumentationAgent;
+    private boolean applyInstrumentationAgent = true;
     private Map<String, String> envVariables;
     private boolean enabled = true;
     private boolean hasJvmArgs;

subprojects/launcher/src/test/groovy/org/gradle/launcher/daemon/configuration/BuildProcessTest.groovy

@@ -37,7 +37,9 @@ class BuildProcessTest extends Specification {
 
     private def fileCollectionFactory = TestFiles.fileCollectionFactory(tmpDir.testDirectory)
     private def currentJvm = Stub(JavaInfo)
-    private def agentStatus = Stub(AgentStatus)
+    private def agentStatus = Stub(AgentStatus) {
+        isAgentInstrumentationEnabled() >> true
+    }
 
     def "current and requested build vm match if vm arguments match"() {
         given:

subprojects/tooling-api/build.gradle.kts

@@ -87,7 +87,3 @@ integTest.usesJavadocCodeSnippets = true
 testFilesCleanup.reportOnly = true
 
 apply(from = "buildship.gradle")
-
-// TODO(mlopatkin) Remove this when agent is on by default
-// TAPI doesn't know about the feature flag to enable the instrumentation agent, so all daemons started in this build have to be agentless too to meet test expectations.
-extra["org.gradle.integtest.agent.allowed"] = "false"