Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix -DargLine propagation for Maven builds #7269

Merged
merged 1 commit into from
Jul 3, 2024
Merged

Fix -DargLine propagation for Maven builds #7269

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions ...n-3.2.1/src/main/java/datadog/trace/instrumentation/maven3/MavenLifecycleParticipant.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,9 +298,7 @@ private Void configureTestExecutions(

for (MavenTestExecution testExecution : testExecutions) {
MavenProjectConfigurator.INSTANCE.configureTracer(
testExecution.getProject(),
testExecution.getExecution(),
moduleExecutionSettings.getSystemProperties());
testExecution, moduleExecutionSettings.getSystemProperties());
MavenProjectConfigurator.INSTANCE.configureJacoco(testExecution, moduleExecutionSettings);
}
return null;
Expand Down
50 changes: 18 additions & 32 deletions ...en-3.2.1/src/main/java/datadog/trace/instrumentation/maven3/MavenProjectConfigurator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import org.apache.maven.model.PluginExecution;
import org.apache.maven.plugin.MojoExecution;
import org.apache.maven.project.MavenProject;
import org.codehaus.plexus.configuration.PlexusConfiguration;
import org.codehaus.plexus.util.xml.Xpp3Dom;

class MavenProjectConfigurator {
Expand All @@ -33,44 +34,42 @@ class MavenProjectConfigurator {
private static final String JAVAC_COMPILER_ID = "javac";
private static final String DATADOG_COMPILER_PLUGIN_ID = "DatadogCompilerPlugin";

private static final ComparableVersion LATE_SUBSTITUTION_SUPPORTED_VERSION =
new ComparableVersion("2.17");
private static final String JACOCO_EXCL_CLASS_LOADERS_PROPERTY = "jacoco.exclClassLoaders";

public void configureTracer(
MavenProject project,
MojoExecution mojoExecution,
Map<String, String> propagatedSystemProperties) {
Xpp3Dom configuration = mojoExecution.getConfiguration();
MavenTestExecution mavenTestExecution, Map<String, String> propagatedSystemProperties) {
MojoExecution mojoExecution = mavenTestExecution.getExecution();
PlexusConfiguration pomConfiguration = MavenUtils.getPomConfiguration(mojoExecution);

Xpp3Dom forkCount = configuration.getChild("forkCount");
PlexusConfiguration forkCount = pomConfiguration.getChild("forkCount");
if (forkCount != null && "0".equals(forkCount.getValue())) {
// tests will be executed inside this JVM, no need for additional configuration
return;
}

StringBuilder modifiedArgLine = new StringBuilder();
StringBuilder addedArgLine = new StringBuilder();
// propagate to child process all "dd." system properties available in current process
for (Map.Entry<String, String> e : propagatedSystemProperties.entrySet()) {
modifiedArgLine.append("-D").append(e.getKey()).append('=').append(e.getValue()).append(" ");
addedArgLine.append("-D").append(e.getKey()).append('=').append(e.getValue()).append(" ");
}

Config config = Config.get();
Integer ciVisibilityDebugPort = config.getCiVisibilityDebugPort();
if (ciVisibilityDebugPort != null) {
modifiedArgLine
addedArgLine
.append("-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=")
.append(ciVisibilityDebugPort)
.append(" ");
}

String additionalArgs = config.getCiVisibilityAdditionalChildProcessJvmArgs();
if (additionalArgs != null) {
modifiedArgLine.append(additionalArgs).append(" ");
addedArgLine.append(additionalArgs).append(" ");
}

MavenProject project = mavenTestExecution.getProject();
String moduleName = MavenUtils.getUniqueModuleName(project, mojoExecution);
modifiedArgLine
addedArgLine
.append("-D")
.append(
Strings.propertyNameToSystemPropertyName(CiVisibilityConfig.CIVISIBILITY_MODULE_NAME))
Expand All @@ -79,43 +78,30 @@ public void configureTracer(
.append("' ");

File agentJar = config.getCiVisibilityAgentJarFile();
modifiedArgLine.append("-javaagent:").append(agentJar.toPath());
addedArgLine.append("-javaagent:").append(agentJar.toPath());

Plugin plugin = mojoExecution.getPlugin();
Map<String, PluginExecution> pluginExecutions = plugin.getExecutionsAsMap();
PluginExecution pluginExecution = pluginExecutions.get(mojoExecution.getExecutionId());
Xpp3Dom executionConfiguration = (Xpp3Dom) pluginExecution.getConfiguration();

String argLine = MavenUtils.getConfigurationValue(executionConfiguration, "argLine");
boolean projectWideArgLineNeeded = argLine == null || !argLine.contains("{argLine}");

String finalArgLine =
(projectWideArgLineNeeded ? getReferenceToProjectWideArgLine(plugin) + " " : "")
+ (argLine != null ? argLine + " " : "")
PlexusConfiguration argLineConfiguration = pomConfiguration.getChild("argLine");
String existingArgLine = argLineConfiguration.getValue();
String updatedArgLine =
(existingArgLine != null ? existingArgLine + " " : "")
+
// -javaagent that injects the tracer
// has to be the last one,
// since if there are other agents
// we want to be able to instrument their code
// (namely Jacoco's)
modifiedArgLine;
addedArgLine;

Xpp3Dom updatedExecutionConfiguration =
MavenUtils.setConfigurationValue(finalArgLine, executionConfiguration, "argLine");
MavenUtils.setConfigurationValue(updatedArgLine, executionConfiguration, "argLine");
pluginExecution.setConfiguration(updatedExecutionConfiguration);
}

private static String getReferenceToProjectWideArgLine(Plugin plugin) {
String pluginVersion = plugin.getVersion();
ComparableVersion pluginVersionParsed =
new ComparableVersion(pluginVersion != null ? pluginVersion : "");
if (pluginVersionParsed.compareTo(LATE_SUBSTITUTION_SUPPORTED_VERSION) >= 0) {
return "@{argLine} ";
} else {
return "${argLine} ";
}
}

void configureCompilerPlugin(MavenProject project, String compilerPluginVersion) {
Plugin compilerPlugin = project.getPlugin(MAVEN_COMPILER_PLUGIN_KEY);
if (compilerPlugin == null || compilerPluginVersion == null) {
Expand Down
11 changes: 11 additions & 0 deletions ...umentation/maven-3.2.1/src/main/java/datadog/trace/instrumentation/maven3/MavenUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
import org.apache.maven.plugin.MojoExecution;
import org.apache.maven.project.MavenProject;
import org.codehaus.plexus.PlexusContainer;
import org.codehaus.plexus.configuration.PlexusConfiguration;
import org.codehaus.plexus.configuration.xml.XmlPlexusConfiguration;
import org.codehaus.plexus.logging.Logger;
import org.codehaus.plexus.util.xml.Xpp3Dom;

Expand Down Expand Up @@ -298,4 +300,13 @@ public static PlexusContainer getContainer(MavenSession mavenSession) {
}
return METHOD_HANDLES.invoke(LOOKUP_METHOD, lookup, PlexusContainer.class);
}

public static PlexusConfiguration getPomConfiguration(MojoExecution mojoExecution) {
Xpp3Dom configuration = mojoExecution.getConfiguration();
if (configuration == null) {
return new XmlPlexusConfiguration("configuration");
} else {
return new XmlPlexusConfiguration(configuration);
}
}
}
35 changes: 18 additions & 17 deletions dd-smoke-tests/maven/src/test/groovy/datadog/smoketest/MavenSmokeTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ class MavenSmokeTest extends CiVisibilitySmokeTest {
mockBackend.givenFlakyTest("Maven Smoke Tests Project maven-surefire-plugin default-test", "datadog.smoke.TestFailed", "test_failed")
mockBackend.givenSkippableTest("Maven Smoke Tests Project maven-surefire-plugin default-test", "datadog.smoke.TestSucceed", "test_to_skip_with_itr")

def exitCode = whenRunningMavenBuild(jacocoCoverage)
def exitCode = whenRunningMavenBuild(jacocoCoverage, commandLineParams)

if (expectSuccess) {
assert exitCode == 0
Expand All @@ -80,20 +80,21 @@ class MavenSmokeTest extends CiVisibilitySmokeTest {
verifyTelemetryMetrics(mockBackend.getAllReceivedTelemetryMetrics(), mockBackend.getAllReceivedTelemetryDistributions(), expectedEvents)

where:
projectName | mavenVersion | expectedEvents | expectedCoverages | expectSuccess | flakyRetries | jacocoCoverage | minSupportedJavaVersion
"test_successful_maven_run" | "3.2.1" | 5 | 1 | true | false | true | 8
"test_successful_maven_run" | "3.5.4" | 5 | 1 | true | false | true | 8
"test_successful_maven_run" | "3.6.3" | 5 | 1 | true | false | true | 8
"test_successful_maven_run" | "3.8.8" | 5 | 1 | true | false | true | 8
"test_successful_maven_run" | "3.9.7" | 5 | 1 | true | false | true | 8
"test_successful_maven_run_surefire_3_0_0" | "3.9.7" | 5 | 1 | true | false | true | 8
"test_successful_maven_run_surefire_3_0_0" | LATEST_MAVEN_VERSION | 5 | 1 | true | false | true | 17
"test_successful_maven_run_builtin_coverage" | "3.9.7" | 5 | 1 | true | false | false | 8
"test_successful_maven_run_with_jacoco_and_argline" | "3.9.7" | 5 | 1 | true | false | true | 8
projectName | mavenVersion | expectedEvents | expectedCoverages | expectSuccess | flakyRetries | jacocoCoverage | commandLineParams | minSupportedJavaVersion
"test_successful_maven_run" | "3.2.1" | 5 | 1 | true | false | true | [] | 8
"test_successful_maven_run" | "3.5.4" | 5 | 1 | true | false | true | [] | 8
"test_successful_maven_run" | "3.6.3" | 5 | 1 | true | false | true | [] | 8
"test_successful_maven_run" | "3.8.8" | 5 | 1 | true | false | true | [] | 8
"test_successful_maven_run" | "3.9.8" | 5 | 1 | true | false | true | [] | 8
"test_successful_maven_run_surefire_3_0_0" | "3.9.8" | 5 | 1 | true | false | true | [] | 8
"test_successful_maven_run_surefire_3_0_0" | LATEST_MAVEN_VERSION | 5 | 1 | true | false | true | [] | 17
"test_successful_maven_run_builtin_coverage" | "3.9.8" | 5 | 1 | true | false | false | [] | 8
"test_successful_maven_run_with_jacoco_and_argline" | "3.9.8" | 5 | 1 | true | false | true | [] | 8
// "expectedEvents" count for this test case does not include the spans that correspond to Cucumber steps
"test_successful_maven_run_with_cucumber" | "3.9.7" | 4 | 1 | true | false | true | 8
"test_failed_maven_run_flaky_retries" | "3.9.7" | 8 | 5 | false | true | true | 8
"test_successful_maven_run_junit_platform_runner" | "3.9.7" | 4 | 0 | true | false | false | 8
"test_successful_maven_run_with_cucumber" | "3.9.8" | 4 | 1 | true | false | true | [] | 8
"test_failed_maven_run_flaky_retries" | "3.9.8" | 8 | 5 | false | true | true | [] | 8
"test_successful_maven_run_junit_platform_runner" | "3.9.8" | 4 | 0 | true | false | false | [] | 8
"test_successful_maven_run_with_arg_line_property" | "3.9.8" | 4 | 0 | true | false | false | ["-DargLine='-Dmy-custom-property=provided-via-command-line'"] | 8
}

private void givenWrapperPropertiesFile(String mavenVersion) {
Expand Down Expand Up @@ -169,15 +170,15 @@ class MavenSmokeTest extends CiVisibilitySmokeTest {
throw new AssertionError((Object) "Tried $DEPENDENCIES_DOWNLOAD_RETRIES times to execute $mvnCommand and failed")
}

private int whenRunningMavenBuild(boolean injectJacoco) {
def processBuilder = createProcessBuilder(["-B", "test"], true, injectJacoco)
private int whenRunningMavenBuild(boolean injectJacoco, List<String> additionalCommandLineParams) {
def processBuilder = createProcessBuilder(["-B", "test"] + additionalCommandLineParams, true, injectJacoco)

processBuilder.environment().put("DD_API_KEY", "01234567890abcdef123456789ABCDEF")

return runProcess(processBuilder.start())
}

private runProcess(Process p) {
private static runProcess(Process p) {
StreamConsumer errorGobbler = new StreamConsumer(p.getErrorStream(), "ERROR")
StreamConsumer outputGobbler = new StreamConsumer(p.getInputStream(), "OUTPUT")
outputGobbler.start()
Expand Down
9 changes: 9 additions & 0 deletions ...s/maven/src/test/resources/test_successful_maven_run_with_arg_line_property/coverages.ftl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[ {
"test_session_id" : ${content_test_session_id},
"test_suite_id" : ${content_test_suite_id},
"span_id" : ${content_span_id},
"files" : [ {
"filename" : "src/test/java/datadog/smoke/TestSucceedPropertyAssertion.java",
"segments" : [ ]
} ]
} ]
Loading
Loading