Skip to content

Fix Gradle Launcher instrumentation to not interfere with Gradle Test Kit #8465

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Feb 28, 2025
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,12 @@ abstract class CiVisibilityTestUtils {
try {
JSONAssert.assertEquals(expectedEvents, actualEvents, comparisonMode)
} catch (AssertionError e) {
if (ciRun) {
// When running in CI the assertion error message does not contain the actual diff,
// so we print the events to the console to help debug the issue
println "Expected events: $expectedEvents"
println "Actual events: $actualEvents"
}
throw new org.opentest4j.AssertionFailedError("Events mismatch", expectedEvents, actualEvents, e)
}

Expand All @@ -111,6 +117,12 @@ abstract class CiVisibilityTestUtils {
try {
JSONAssert.assertEquals(expectedCoverages, actualCoverages, comparisonMode)
} catch (AssertionError e) {
if (ciRun) {
// When running in CI the assertion error message does not contain the actual diff,
// so we print the events to the console to help debug the issue
println "Expected coverages: $expectedCoverages"
println "Actual coverages: $actualCoverages"
}
throw new org.opentest4j.AssertionFailedError("Coverages mismatch", expectedCoverages, actualCoverages, e)
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package datadog.trace.instrumentation.gradle;

import static datadog.trace.util.Strings.propertyNameToSystemPropertyName;

import datadog.trace.api.Config;
import datadog.trace.api.config.CiVisibilityConfig;
import java.io.File;
import java.nio.file.Path;
import java.util.HashMap;
Expand All @@ -11,17 +14,30 @@ public class GradleDaemonInjectionUtils {

public static Map<String, String> addJavaagentToGradleDaemonProperties(
Map<String, String> jvmOptions) {
Properties systemProperties = System.getProperties();
if (systemProperties.containsKey(
propertyNameToSystemPropertyName(
CiVisibilityConfig.CIVISIBILITY_INJECTED_TRACER_VERSION))) {
// This Gradle launcher is started by a process that is itself instrumented,
// most likely this is a Gradle build using Gradle Test Kit to fork another Gradle instance
// (e.g. to test a Gradle plugin).
// We don't want to instrument/trace such "nested" Gradle instances
return jvmOptions;
}

File agentJar = Config.get().getCiVisibilityAgentJarFile();
Path agentJarPath = agentJar.toPath();

StringBuilder agentArg = new StringBuilder("-javaagent:").append(agentJarPath).append('=');

Properties systemProperties = System.getProperties();
for (Map.Entry<Object, Object> e : systemProperties.entrySet()) {
String propertyName = (String) e.getKey();
Object propertyValue = e.getValue();
if (propertyName.startsWith(Config.PREFIX)) {
agentArg.append(propertyName).append('=').append(propertyValue).append(',');
agentArg
.append(propertyName)
.append("='")
.append(String.valueOf(propertyValue).replace("'", "'\\''"))
.append("',");
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
package datadog.trace.instrumentation.gradle;

import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
import static net.bytebuddy.matcher.ElementMatchers.takesArgument;

import com.google.auto.service.AutoService;
import datadog.trace.agent.tooling.Instrumenter;
import datadog.trace.agent.tooling.InstrumenterModule;
import datadog.trace.api.Config;
import java.util.Set;
import net.bytebuddy.asm.Advice;

/**
* We inject {@link CiVisibilityGradleListener} into Gradle and register it with {@code
* Scope.Build.class} scope.
*
* <p>A class named {@code org.gradle.internal.service.ServiceScopeValidator} checks that every
* service registered with a scope is annotated with {@code @ServiceScope(<ScopeClass>.class)}.
*
* <p>We cannot annotate our service, as {@code Scope.Build.class} is a recent addition: we need to
* support older Gradle versions, besides this class is absent in the Gradle API version that the
* tracer currently uses.
*
* <p>To suppress validation for our service we patch a "workaround" class that is used internally
* by Gradle.
*/
@AutoService(InstrumenterModule.class)
public class GradleServiceValidationInstrumentation extends InstrumenterModule.CiVisibility
implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice {

public GradleServiceValidationInstrumentation() {
super("gradle", "gradle-build-scope-services");
}

@Override
public String instrumentedType() {
return "org.gradle.internal.service.ServiceScopeValidatorWorkarounds";
}

@Override
public boolean isApplicable(Set<TargetSystem> enabledSystems) {
return super.isApplicable(enabledSystems)
&& Config.get().isCiVisibilityBuildInstrumentationEnabled();
}

@Override
public void methodAdvice(MethodTransformer transformer) {
transformer.applyAdvice(
named("shouldSuppressValidation").and(takesArgument(0, Class.class)),
getClass().getName() + "$SuppressValidation");
}

public static class SuppressValidation {
@Advice.OnMethodExit(suppress = Throwable.class)
public static void suppressValidationForCiVisService(
@Advice.Argument(0) final Class<?> validatedClass,
@Advice.Return(readOnly = false) boolean suppressValidation) {
if (validatedClass.getName().endsWith("CiVisibilityGradleListener")) {
suppressValidation = true;
}
}
}
}
Original file line number Diff line number Diff line change
@@ -1,20 +1,17 @@
package datadog.smoketest


import datadog.trace.api.Config
import datadog.trace.api.Platform
import datadog.trace.api.config.CiVisibilityConfig
import datadog.trace.api.config.GeneralConfig
import datadog.trace.api.config.TraceInstrumentationConfig
import datadog.trace.util.Strings
import org.gradle.testkit.runner.BuildResult
import org.gradle.testkit.runner.GradleRunner
import org.gradle.testkit.runner.TaskOutcome
import org.gradle.util.DistributionLocator
import org.gradle.util.GradleVersion
import org.gradle.wrapper.Download
import org.gradle.wrapper.Install
import org.gradle.wrapper.PathAssembler
import org.gradle.wrapper.WrapperConfiguration
import org.gradle.wrapper.*
import spock.lang.IgnoreIf
import spock.lang.Shared
import spock.lang.TempDir
Expand Down Expand Up @@ -49,34 +46,35 @@ class GradleDaemonSmokeTest extends AbstractGradleTest {
runGradleTest(gradleVersion, projectName, false, successExpected, false, expectedTraces, expectedCoverages)

where:
gradleVersion | projectName | successExpected | expectedTraces | expectedCoverages
"3.0" | "test-succeed-old-gradle" | true | 5 | 1
"7.6.4" | "test-succeed-legacy-instrumentation" | true | 5 | 1
"7.6.4" | "test-succeed-multi-module-legacy-instrumentation" | true | 7 | 2
"7.6.4" | "test-succeed-multi-forks-legacy-instrumentation" | true | 6 | 2
"7.6.4" | "test-skip-legacy-instrumentation" | true | 2 | 0
"7.6.4" | "test-failed-legacy-instrumentation" | false | 4 | 0
"7.6.4" | "test-corrupted-config-legacy-instrumentation" | false | 1 | 0
gradleVersion | projectName | successExpected | expectedTraces | expectedCoverages
"3.0" | "test-succeed-old-gradle" | true | 5 | 1
"7.6.4" | "test-succeed-legacy-instrumentation" | true | 5 | 1
"7.6.4" | "test-succeed-multi-module-legacy-instrumentation" | true | 7 | 2
"7.6.4" | "test-succeed-multi-forks-legacy-instrumentation" | true | 6 | 2
"7.6.4" | "test-skip-legacy-instrumentation" | true | 2 | 0
"7.6.4" | "test-failed-legacy-instrumentation" | false | 4 | 0
"7.6.4" | "test-corrupted-config-legacy-instrumentation" | false | 1 | 0
}

def "test #projectName, v#gradleVersion, configCache: #configurationCache"() {
runGradleTest(gradleVersion, projectName, configurationCache, successExpected, flakyRetries, expectedTraces, expectedCoverages)

where:
gradleVersion | projectName | configurationCache | successExpected | flakyRetries | expectedTraces | expectedCoverages
"8.3" | "test-succeed-new-instrumentation" | false | true | false | 5 | 1
"8.9" | "test-succeed-new-instrumentation" | false | true | false | 5 | 1
LATEST_GRADLE_VERSION | "test-succeed-new-instrumentation" | false | true | false | 5 | 1
"8.3" | "test-succeed-new-instrumentation" | true | true | false | 5 | 1
"8.9" | "test-succeed-new-instrumentation" | true | true | false | 5 | 1
LATEST_GRADLE_VERSION | "test-succeed-new-instrumentation" | true | true | false | 5 | 1
LATEST_GRADLE_VERSION | "test-succeed-multi-module-new-instrumentation" | false | true | false | 7 | 2
LATEST_GRADLE_VERSION | "test-succeed-multi-forks-new-instrumentation" | false | true | false | 6 | 2
LATEST_GRADLE_VERSION | "test-skip-new-instrumentation" | false | true | false | 2 | 0
LATEST_GRADLE_VERSION | "test-failed-new-instrumentation" | false | false | false | 4 | 0
LATEST_GRADLE_VERSION | "test-corrupted-config-new-instrumentation" | false | false | false | 1 | 0
LATEST_GRADLE_VERSION | "test-succeed-junit-5" | false | true | false | 5 | 1
LATEST_GRADLE_VERSION | "test-failed-flaky-retries" | false | false | true | 8 | 0
gradleVersion | projectName | configurationCache | successExpected | flakyRetries | expectedTraces | expectedCoverages
"8.3" | "test-succeed-new-instrumentation" | false | true | false | 5 | 1
"8.9" | "test-succeed-new-instrumentation" | false | true | false | 5 | 1
LATEST_GRADLE_VERSION | "test-succeed-new-instrumentation" | false | true | false | 5 | 1
"8.3" | "test-succeed-new-instrumentation" | true | true | false | 5 | 1
"8.9" | "test-succeed-new-instrumentation" | true | true | false | 5 | 1
LATEST_GRADLE_VERSION | "test-succeed-new-instrumentation" | true | true | false | 5 | 1
LATEST_GRADLE_VERSION | "test-succeed-multi-module-new-instrumentation" | false | true | false | 7 | 2
LATEST_GRADLE_VERSION | "test-succeed-multi-forks-new-instrumentation" | false | true | false | 6 | 2
LATEST_GRADLE_VERSION | "test-skip-new-instrumentation" | false | true | false | 2 | 0
LATEST_GRADLE_VERSION | "test-failed-new-instrumentation" | false | false | false | 4 | 0
LATEST_GRADLE_VERSION | "test-corrupted-config-new-instrumentation" | false | false | false | 1 | 0
LATEST_GRADLE_VERSION | "test-succeed-junit-5" | false | true | false | 5 | 1
LATEST_GRADLE_VERSION | "test-failed-flaky-retries" | false | false | true | 8 | 0
LATEST_GRADLE_VERSION | "test-succeed-gradle-plugin-test" | false | true | false | 5 | 0
}

private runGradleTest(String gradleVersion, String projectName, boolean configurationCache, boolean successExpected, boolean flakyRetries, int expectedTraces, int expectedCoverages) {
Expand Down Expand Up @@ -131,6 +129,16 @@ class GradleDaemonSmokeTest extends AbstractGradleTest {
"${Strings.propertyNameToSystemPropertyName(CiVisibilityConfig.CIVISIBILITY_AGENTLESS_ENABLED)}=true," +
"${Strings.propertyNameToSystemPropertyName(CiVisibilityConfig.CIVISIBILITY_GIT_UPLOAD_ENABLED)}=false," +
"${Strings.propertyNameToSystemPropertyName(CiVisibilityConfig.CIVISIBILITY_CIPROVIDER_INTEGRATION_ENABLED)}=false," +
/*
* Some of the smoke tests (in particular the one with the Gradle plugin), are using Gradle Test Kit for their tests.
* Gradle Test Kit needs to do a "chmod" when starting a Gradle Daemon.
* This "chmod" operation is traced by datadog.trace.instrumentation.java.lang.ProcessImplInstrumentation and is reported as a span.
* The problem is that the "chmod" only happens when running in CI (could be due to differences in OS or FS permissions),
* so when running the tests locally, the "chmod" span is not there.
* This causes the tests to fail because the number of reported traces is different.
* To avoid this discrepancy between local and CI runs, we disable tracing instrumentations.
*/
"${Strings.propertyNameToSystemPropertyName(TraceInstrumentationConfig.TRACE_ENABLED)}=false," +
"${Strings.propertyNameToSystemPropertyName(CiVisibilityConfig.CIVISIBILITY_JACOCO_PLUGIN_VERSION)}=$JACOCO_PLUGIN_VERSION," +
"${Strings.propertyNameToSystemPropertyName(CiVisibilityConfig.CIVISIBILITY_AGENTLESS_URL)}=${mockBackend.intakeUrl}"

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
plugins {
id 'java'
id 'java-gradle-plugin'
}

gradlePlugin {
plugins {
mySimplePlugin {
id = 'datadog.smoke.helloplugin'
implementationClass = 'datadog.smoke.HelloPlugin'
}
}
}

repositories {
mavenLocal()
mavenCentral()
}

dependencies {
testImplementation gradleTestKit()

testImplementation 'org.junit.jupiter:junit-jupiter-engine:5.10.2'
}

tasks.withType(Test).configureEach {
useJUnitPlatform()
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[ {
"files" : [ {
"bitmap" : "AAAAx98=",
"filename" : "src/test/java/datadog/smoke/HelloPluginFunctionalTest.java"
} ],
"span_id" : ${content_span_id_4},
"test_session_id" : ${content_test_session_id},
"test_suite_id" : ${content_test_suite_id}
} ]
Loading
Loading