Skip to content

Commit

Permalink
Do not use separate agent directories when agent is invoked on main
Browse files Browse the repository at this point in the history
This is a workaround for goal execution ordering problems with Maven.
For tests, we can add a synthetic goal which is executed in the same
phase and will merge the agent files generated by the test execution.

Unfortunately, this approach doesn't work if we want to instrument
the main execution, which is sometimes the case. This is not doable
because we won't have a phase to hook into and a goal to execute in
order to merge the files. Even if we synthetize a goal and try to
add it to the session execution, this would cause ordering issues
because we don't know in which order the plugins are defined in the
build file. In addition, this would simply not work for multi-module
builds.

As a workaround, we simply disable the split agent output directories
in case we instrument the main execution. This will hopefully work
for most invocations.
  • Loading branch information
melix committed Feb 9, 2022
1 parent 932f93a commit fc820ac
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,14 @@ public class MergeAgentFilesMojo extends AbstractMojo {
public void execute() throws MojoExecutionException, MojoFailureException {
String agentOutputDirectory = agentOutputDirectoryFor(target, NativeExtension.Context.valueOf(context));
File baseDir = new File(agentOutputDirectory);
Path nativeImageExecutable = Utils.getNativeImage();
File mergerExecutable = tryInstall(nativeImageExecutable);
List<File> sessionDirectories = sessionDirectoriesFrom(baseDir.listFiles()).collect(Collectors.toList());
invokeMerge(mergerExecutable, sessionDirectories, baseDir);
if (baseDir.exists()) {
Path nativeImageExecutable = Utils.getNativeImage();
File mergerExecutable = tryInstall(nativeImageExecutable);
List<File> sessionDirectories = sessionDirectoriesFrom(baseDir.listFiles()).collect(Collectors.toList());
invokeMerge(mergerExecutable, sessionDirectories, baseDir);
} else {
getLog().debug("Agent output directory " + baseDir + " doesn't exist. Skipping merge.");
}
}

private File tryInstall(Path nativeImageExecutablePath) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,26 @@ public class NativeExtension extends AbstractMavenLifecycleParticipant {
* and within the Maven POM as values of the {@code name} attribute in
* {@code <options name="...">}.
*/
enum Context { main, test };
enum Context {main, test}

static String testIdsDirectory(String baseDir) {
return baseDir + File.separator + "test-ids";
}

static String buildAgentArgument(String baseDir, Context context, List<String> agentOptions) {
List<String> options = new ArrayList<>(agentOptions);
options.add("config-output-dir=" + agentOutputDirectoryFor(baseDir, context) + File.separator + SharedConstants.AGENT_SESSION_SUBDIR);
String effectiveOutputDir = agentOutputDirectoryFor(baseDir, context);
if (context == Context.test) {
// We need to patch the config dir IF, and only IF, we are running tests, because
// test execution can be forked into a separate process and there's a race condition.
// We have to special case testing here instead of using a generic strategy THEN
// invoke the merging tool, because there's no way in Maven to do something as easy
// as finalizing a goal (that is, let me do the merge AFTER you're done executing tests,
// or invoking exec, or whatever, because what I need to do actually participates into
// the same unit of work !).
effectiveOutputDir = effectiveOutputDir + File.separator + SharedConstants.AGENT_SESSION_SUBDIR;
}
options.add("config-output-dir=" + effectiveOutputDir);
return "-agentlib:native-image-agent=" + String.join(",", options);
}

Expand Down Expand Up @@ -153,18 +164,24 @@ public void afterProjectsRead(MavenSession session) throws MavenExecutionExcepti
Context context = exec.getGoals().stream().anyMatch("test"::equals) ? Context.test : Context.main;
Xpp3Dom agentResourceDirectory = findOrAppend(configuration, "agentResourceDirectory");
agentResourceDirectory.setValue(agentOutputDirectoryFor(target, context));
List<String> goals = new ArrayList<>();
goals.add("merge-agent-files");
goals.addAll(exec.getGoals());
exec.setGoals(goals);
Xpp3Dom agentContext = findOrAppend(configuration, "context");
agentContext.setValue(context.name());
if (context == Context.test) {
setupMergeAgentFiles(exec, configuration, context);
}
});
}
});
}
}

private static void setupMergeAgentFiles(PluginExecution exec, Xpp3Dom configuration, Context context) {
List<String> goals = new ArrayList<>();
goals.add("merge-agent-files");
goals.addAll(exec.getGoals());
exec.setGoals(goals);
Xpp3Dom agentContext = findOrAppend(configuration, "context");
agentContext.setValue(context.name());
}

private static void withPlugin(Build build, String artifactId, Consumer<? super Plugin> consumer) {
build.getPlugins()
.stream()
Expand Down Expand Up @@ -204,6 +221,7 @@ private static String getSelectedOptionsName(MavenSession session) {
* {@code <options>} elements whose names match the name of the supplied {@code context}
* or {@code selectedOptionsName} and for unnamed, shared {@code <options>} elements,
* and returns a list of the collected agent options.
*
* @param nativePlugin the {@code native-maven-plugin}; never null
* @param context the current execution context; never null
* @param selectedOptionsName the name of the set of custom agent options activated
Expand Down Expand Up @@ -260,9 +278,12 @@ private static void processOptionNodes(Xpp3Dom options, List<String> optionsList
private static boolean parseBoolean(String description, String value) {
value = assertNotEmptyAndTrim(value, description + " must have a value").toLowerCase();
switch (value) {
case "true": return true;
case "false": return false;
default: throw new IllegalStateException(description + " must have a value of 'true' or 'false'");
case "true":
return true;
case "false":
return false;
default:
throw new IllegalStateException(description + " must have a value of 'true' or 'false'");
}
}

Expand Down Expand Up @@ -304,15 +325,20 @@ private static void configureJunitListener(Plugin surefirePlugin, String testIds

private static void updatePluginConfiguration(Plugin plugin, BiConsumer<PluginExecution, ? super Xpp3Dom> consumer) {
plugin.getExecutions().forEach(exec -> {
Xpp3Dom configuration = (Xpp3Dom) exec.getConfiguration();
if (configuration == null) {
configuration = new Xpp3Dom("configuration");
exec.setConfiguration(configuration);
}
Xpp3Dom configuration = configurationBlockOf(exec);
consumer.accept(exec, configuration);
});
}

private static Xpp3Dom configurationBlockOf(PluginExecution exec) {
Xpp3Dom configuration = (Xpp3Dom) exec.getConfiguration();
if (configuration == null) {
configuration = new Xpp3Dom("configuration");
exec.setConfiguration(configuration);
}
return configuration;
}

private static Xpp3Dom findOrAppend(Xpp3Dom parent, String childName) {
Xpp3Dom child = parent.getChild(childName);
if (child == null) {
Expand Down

0 comments on commit fc820ac

Please sign in to comment.