From 3c30e559b8b6af92843364a8e2e757ef5b28dfe6 Mon Sep 17 00:00:00 2001 From: paullegranddc Date: Mon, 31 Mar 2025 16:47:41 +0200 Subject: [PATCH 1/3] fix(AgentBootstrap): add an exception in double injection check for log4j patch agent --- .../trace/bootstrap/AgentBootstrap.java | 56 ++++++++++++------- 1 file changed, 35 insertions(+), 21 deletions(-) diff --git a/dd-java-agent/src/main/java/datadog/trace/bootstrap/AgentBootstrap.java b/dd-java-agent/src/main/java/datadog/trace/bootstrap/AgentBootstrap.java index f70038f26ba..8c1b7f56a71 100644 --- a/dd-java-agent/src/main/java/datadog/trace/bootstrap/AgentBootstrap.java +++ b/dd-java-agent/src/main/java/datadog/trace/bootstrap/AgentBootstrap.java @@ -284,31 +284,45 @@ static int parseJavaMajorVersion(String version) { } static boolean shouldAbortDueToOtherJavaAgents() { - // Simply considering having multiple agents - if (getConfig(LIB_INJECTION_ENABLED_FLAG) - && !getConfig(LIB_INJECTION_FORCE_FLAG) - && getAgentFilesFromVMArguments().size() > 1) { - // Formatting agent file list, Java 7 style - StringBuilder agentFiles = new StringBuilder(); - boolean first = true; + // We don't abort if either + // * We are not using SSI + // * Injection is forced + // * There is only one agent + if (!getConfig(LIB_INJECTION_ENABLED_FLAG) + || getConfig(LIB_INJECTION_FORCE_FLAG) + || getAgentFilesFromVMArguments().size() <= 1) { + return false; + } + + // If there are 2 agents and one of them is for patching log4j, it's fine + if (getAgentFilesFromVMArguments().size() == 2) { for (File agentFile : getAgentFilesFromVMArguments()) { - if (first) { - first = false; - } else { - agentFiles.append(", "); + if (agentFile.getName().toLowerCase().contains("log4j")) { + return false; } - agentFiles.append('"'); - agentFiles.append(agentFile.getAbsolutePath()); - agentFiles.append('"'); } - System.err.println( - "Info: multiple JVM agents detected, found " - + agentFiles - + ". Loading multiple APM/Tracing agent is not a recommended or supported configuration." - + "Please set the DD_INJECT_FORCE configuration to TRUE to load Datadog APM/Tracing agent."); - return true; } - return false; + + // Simply considering having multiple agents + // Formatting agent file list, Java 7 style + StringBuilder agentFiles = new StringBuilder(); + boolean first = true; + for (File agentFile : getAgentFilesFromVMArguments()) { + if (first) { + first = false; + } else { + agentFiles.append(", "); + } + agentFiles.append('"'); + agentFiles.append(agentFile.getAbsolutePath()); + agentFiles.append('"'); + } + System.err.println( + "Info: multiple JVM agents detected, found " + + agentFiles + + ". Loading multiple APM/Tracing agent is not a recommended or supported configuration." + + "Please set the DD_INJECT_FORCE configuration to TRUE to load Datadog APM/Tracing agent."); + return true; } public static void main(final String[] args) { From c94652ca0fb83f46d7e2b3eb64b760fc9f7e3c9a Mon Sep 17 00:00:00 2001 From: paullegranddc Date: Thu, 3 Apr 2025 16:35:23 +0200 Subject: [PATCH 2/3] Add test case for log4j agent --- .../MultipleAgentGuardrailsTest.groovy | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/dd-smoke-tests/lib-injection/src/test/groovy/datadog/smoketest/MultipleAgentGuardrailsTest.groovy b/dd-smoke-tests/lib-injection/src/test/groovy/datadog/smoketest/MultipleAgentGuardrailsTest.groovy index b644a87296a..52fef9a4ce0 100644 --- a/dd-smoke-tests/lib-injection/src/test/groovy/datadog/smoketest/MultipleAgentGuardrailsTest.groovy +++ b/dd-smoke-tests/lib-injection/src/test/groovy/datadog/smoketest/MultipleAgentGuardrailsTest.groovy @@ -1,12 +1,23 @@ package datadog.smoketest +import java.nio.file.Files +import java.nio.file.StandardCopyOption +import java.nio.file.Paths + + abstract class MultipleAgentGuardrailsTest extends AbstractSmokeTest { static final String LIB_INJECTION_ENABLED_FLAG = 'DD_INJECTION_ENABLED' static final String LIB_INJECTION_FORCE_FLAG = 'DD_INJECT_FORCE' @Override ProcessBuilder createProcessBuilder() { + def jarPath = System.getProperty("datadog.smoketest.shadowJar.path") + if (extraAgentFilename() != null) { + def renamedJar = Paths.get(jarPath).getParent().resolve(extraAgentFilename()) + Files.copy(Paths.get(jarPath), renamedJar, StandardCopyOption.REPLACE_EXISTING) + jarPath = renamedJar.toString() + } def command = [] command+= javaPath() command.addAll(defaultJavaProperties) @@ -28,6 +39,11 @@ abstract class MultipleAgentGuardrailsTest extends AbstractSmokeTest { abstract boolean isLibInjectionEnabled() abstract boolean isLibInjectionForced() + String extraAgentFilename() { + return null + } + + boolean isExpectingTrace() { return !isLibInjectionEnabled() || isLibInjectionForced() } @@ -78,3 +94,26 @@ class LibInjectionForcedTest extends MultipleAgentGuardrailsTest { return true } } + +// Test that injection still works if we have to agent if one of them is the aws emr log4j patcher +class LibsInjectionWorksEmr extends MultipleAgentGuardrailsTest { + @Override + boolean isLibInjectionEnabled() { + return true + } + + @Override + boolean isLibInjectionForced() { + return false + } + + @Override + String extraAgentFilename() { + return "Log4jHotPatchFat.jar" + } + + @Override + boolean isExpectingTrace() { + return true + } +} From d9eeea6b71e6efec92436f19c3ad506254d00dd8 Mon Sep 17 00:00:00 2001 From: Bruce Bujon Date: Fri, 2 May 2025 17:17:35 +0200 Subject: [PATCH 3/3] chore: Clean up smoke tests --- .../MultipleAgentGuardrailsTest.groovy | 31 ++++++++++--------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/dd-smoke-tests/lib-injection/src/test/groovy/datadog/smoketest/MultipleAgentGuardrailsTest.groovy b/dd-smoke-tests/lib-injection/src/test/groovy/datadog/smoketest/MultipleAgentGuardrailsTest.groovy index 52fef9a4ce0..c0e9350e468 100644 --- a/dd-smoke-tests/lib-injection/src/test/groovy/datadog/smoketest/MultipleAgentGuardrailsTest.groovy +++ b/dd-smoke-tests/lib-injection/src/test/groovy/datadog/smoketest/MultipleAgentGuardrailsTest.groovy @@ -1,9 +1,9 @@ package datadog.smoketest import java.nio.file.Files -import java.nio.file.StandardCopyOption import java.nio.file.Paths +import static java.nio.file.StandardCopyOption.REPLACE_EXISTING abstract class MultipleAgentGuardrailsTest extends AbstractSmokeTest { static final String LIB_INJECTION_ENABLED_FLAG = 'DD_INJECTION_ENABLED' @@ -11,17 +11,11 @@ abstract class MultipleAgentGuardrailsTest extends AbstractSmokeTest { @Override ProcessBuilder createProcessBuilder() { - - def jarPath = System.getProperty("datadog.smoketest.shadowJar.path") - if (extraAgentFilename() != null) { - def renamedJar = Paths.get(jarPath).getParent().resolve(extraAgentFilename()) - Files.copy(Paths.get(jarPath), renamedJar, StandardCopyOption.REPLACE_EXISTING) - jarPath = renamedJar.toString() - } + def jarPath = System.getProperty('datadog.smoketest.shadowJar.path') def command = [] command+= javaPath() command.addAll(defaultJavaProperties) - command+= "-javaagent:${jarPath}" as String // Happen the fake agent too + command+= "-javaagent:${getFakeAgentPath(jarPath)}" as String // Happen the fake agent too command+= '-Ddd.trace.otel.enabled=true' command+= '-jar' command+= jarPath @@ -36,14 +30,23 @@ abstract class MultipleAgentGuardrailsTest extends AbstractSmokeTest { processBuilder.directory(new File(buildDirectory)) } + String getFakeAgentPath(String jarPath) { + if (overriddenFakeAgentFilename() != null) { + def buildFakeAgentPath = Paths.get(jarPath) + def overriddenFakeAgentPath = buildFakeAgentPath.getParent().resolve(overriddenFakeAgentFilename()) + Files.copy(buildFakeAgentPath, overriddenFakeAgentPath, REPLACE_EXISTING) + jarPath = overriddenFakeAgentPath.toString() + } + return jarPath + } + abstract boolean isLibInjectionEnabled() abstract boolean isLibInjectionForced() - String extraAgentFilename() { + String overriddenFakeAgentFilename() { return null } - boolean isExpectingTrace() { return !isLibInjectionEnabled() || isLibInjectionForced() } @@ -96,7 +99,7 @@ class LibInjectionForcedTest extends MultipleAgentGuardrailsTest { } // Test that injection still works if we have to agent if one of them is the aws emr log4j patcher -class LibsInjectionWorksEmr extends MultipleAgentGuardrailsTest { +class LibsInjectionLog4jExclusionTest extends MultipleAgentGuardrailsTest { @Override boolean isLibInjectionEnabled() { return true @@ -108,8 +111,8 @@ class LibsInjectionWorksEmr extends MultipleAgentGuardrailsTest { } @Override - String extraAgentFilename() { - return "Log4jHotPatchFat.jar" + String overriddenFakeAgentFilename() { + return 'Log4jHotPatchFat.jar' } @Override