Skip to content

Commit c329d08

Browse files
Allow agent to be automatically injected when running aside Log4J patch agent (#8648)
# What Does This Do * add an exception in double injection check for log4j patch agent # Motivation This fix is meant for DJM running in AWS EMR. AWS adds a javaagent on their spark processes to patch the log4j vulnerability. This wouldn't pass the current double injection check so the injector is currently configured to pass `DD_INJECT_FORCE` on EMR to force the check. The issue is that this is configuration is currently passed to the injector in a legacy way we want to remove, so we wouldn't be able to skip checks. This exception is also, in general, a bit scary because it applies everywhere we will inject everything on EMR regardless of deny lists. So this PR adds a more restricted exception in the AgentBootstrap, and only works if there 2 agents one of them being for log4j.
1 parent 745b2f1 commit c329d08

File tree

2 files changed

+78
-23
lines changed

2 files changed

+78
-23
lines changed

dd-java-agent/src/main/java/datadog/trace/bootstrap/AgentBootstrap.java

+34-21
Original file line numberDiff line numberDiff line change
@@ -291,32 +291,45 @@ static int parseJavaMajorVersion(String version) {
291291

292292
@SuppressForbidden
293293
static boolean shouldAbortDueToOtherJavaAgents() {
294-
// Simply considering having multiple agents
294+
// We don't abort if either
295+
// * We are not using SSI
296+
// * Injection is forced
297+
// * There is only one agent
298+
if (!getConfig(LIB_INJECTION_ENABLED_ENV_VAR)
299+
|| getConfig(LIB_INJECTION_FORCE_SYS_PROP)
300+
|| getAgentFilesFromVMArguments().size() <= 1) {
301+
return false;
302+
}
295303

296-
if (getConfig(LIB_INJECTION_ENABLED_ENV_VAR)
297-
&& !getConfig(LIB_INJECTION_FORCE_SYS_PROP)
298-
&& getAgentFilesFromVMArguments().size() > 1) {
299-
// Formatting agent file list, Java 7 style
300-
StringBuilder agentFiles = new StringBuilder();
301-
boolean first = true;
304+
// If there are 2 agents and one of them is for patching log4j, it's fine
305+
if (getAgentFilesFromVMArguments().size() == 2) {
302306
for (File agentFile : getAgentFilesFromVMArguments()) {
303-
if (first) {
304-
first = false;
305-
} else {
306-
agentFiles.append(", ");
307+
if (agentFile.getName().toLowerCase().contains("log4j")) {
308+
return false;
307309
}
308-
agentFiles.append('"');
309-
agentFiles.append(agentFile.getAbsolutePath());
310-
agentFiles.append('"');
311310
}
312-
System.err.println(
313-
"Info: multiple JVM agents detected, found "
314-
+ agentFiles
315-
+ ". Loading multiple APM/Tracing agent is not a recommended or supported configuration."
316-
+ "Please set the environment variable DD_INJECT_FORCE or the system property dd.inject.force to TRUE to load Datadog APM/Tracing agent.");
317-
return true;
318311
}
319-
return false;
312+
313+
// Simply considering having multiple agents
314+
// Formatting agent file list, Java 7 style
315+
StringBuilder agentFiles = new StringBuilder();
316+
boolean first = true;
317+
for (File agentFile : getAgentFilesFromVMArguments()) {
318+
if (first) {
319+
first = false;
320+
} else {
321+
agentFiles.append(", ");
322+
}
323+
agentFiles.append('"');
324+
agentFiles.append(agentFile.getAbsolutePath());
325+
agentFiles.append('"');
326+
}
327+
System.err.println(
328+
"Info: multiple JVM agents detected, found "
329+
+ agentFiles
330+
+ ". Loading multiple APM/Tracing agent is not a recommended or supported configuration."
331+
+ "Please set the environment variable DD_INJECT_FORCE or the system property dd.inject.force to TRUE to load Datadog APM/Tracing agent.");
332+
return true;
320333
}
321334

322335
public static void main(final String[] args) {

dd-smoke-tests/lib-injection/src/test/groovy/datadog/smoketest/MultipleAgentGuardrailsTest.groovy

+44-2
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,21 @@
11
package datadog.smoketest
22

3+
import java.nio.file.Files
4+
import java.nio.file.Paths
5+
6+
import static java.nio.file.StandardCopyOption.REPLACE_EXISTING
7+
38
abstract class MultipleAgentGuardrailsTest extends AbstractSmokeTest {
49
static final String LIB_INJECTION_ENABLED_FLAG = 'DD_INJECTION_ENABLED'
510
static final String LIB_INJECTION_FORCE_FLAG = 'DD_INJECT_FORCE'
611

712
@Override
813
ProcessBuilder createProcessBuilder() {
9-
def jarPath = System.getProperty("datadog.smoketest.shadowJar.path")
14+
def jarPath = System.getProperty('datadog.smoketest.shadowJar.path')
1015
def command = []
1116
command+= javaPath()
1217
command.addAll(defaultJavaProperties)
13-
command+= "-javaagent:${jarPath}" as String // Happen the fake agent too
18+
command+= "-javaagent:${getFakeAgentPath(jarPath)}" as String // Happen the fake agent too
1419
command+= '-Ddd.trace.otel.enabled=true'
1520
command+= '-jar'
1621
command+= jarPath
@@ -25,9 +30,23 @@ abstract class MultipleAgentGuardrailsTest extends AbstractSmokeTest {
2530
processBuilder.directory(new File(buildDirectory))
2631
}
2732

33+
String getFakeAgentPath(String jarPath) {
34+
if (overriddenFakeAgentFilename() != null) {
35+
def buildFakeAgentPath = Paths.get(jarPath)
36+
def overriddenFakeAgentPath = buildFakeAgentPath.getParent().resolve(overriddenFakeAgentFilename())
37+
Files.copy(buildFakeAgentPath, overriddenFakeAgentPath, REPLACE_EXISTING)
38+
jarPath = overriddenFakeAgentPath.toString()
39+
}
40+
return jarPath
41+
}
42+
2843
abstract boolean isLibInjectionEnabled()
2944
abstract boolean isLibInjectionForced()
3045

46+
String overriddenFakeAgentFilename() {
47+
return null
48+
}
49+
3150
boolean isExpectingTrace() {
3251
return !isLibInjectionEnabled() || isLibInjectionForced()
3352
}
@@ -78,3 +97,26 @@ class LibInjectionForcedTest extends MultipleAgentGuardrailsTest {
7897
return true
7998
}
8099
}
100+
101+
// Test that injection still works if we have to agent if one of them is the aws emr log4j patcher
102+
class LibsInjectionLog4jExclusionTest extends MultipleAgentGuardrailsTest {
103+
@Override
104+
boolean isLibInjectionEnabled() {
105+
return true
106+
}
107+
108+
@Override
109+
boolean isLibInjectionForced() {
110+
return false
111+
}
112+
113+
@Override
114+
String overriddenFakeAgentFilename() {
115+
return 'Log4jHotPatchFat.jar'
116+
}
117+
118+
@Override
119+
boolean isExpectingTrace() {
120+
return true
121+
}
122+
}

0 commit comments

Comments
 (0)