From ecdf6664ef06e2d9c0258b4671e43f95e2ab0f63 Mon Sep 17 00:00:00 2001 From: Luca Abbati Date: Thu, 20 Jun 2019 17:36:10 -0400 Subject: [PATCH] Refactor log4jX instrumentations --- .../tooling/log/LogContextScopeListener.java | 2 +- .../instrumentation/log4j1/log4j1.gradle | 2 +- .../log4j1/Log4j1MDCInstrumentation.java | 7 ++----- .../log4j1/something/SomeClass.java | 18 ------------------ .../src/test/groovy/Log4j1MDCTest.groovy | 2 ++ .../instrumentation/log4j2/log4j2.gradle | 2 +- .../log4j2/ThreadContextInstrumentation.java | 7 ++----- .../test/groovy/Log4jThreadContextTest.groovy | 2 ++ 8 files changed, 11 insertions(+), 31 deletions(-) delete mode 100644 dd-java-agent/instrumentation/log4j1/src/main/java/datadoggggg/trace/instrumentation/log4j1/something/SomeClass.java diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/log/LogContextScopeListener.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/log/LogContextScopeListener.java index 3a4128a50a..7e3ddff23b 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/log/LogContextScopeListener.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/log/LogContextScopeListener.java @@ -7,7 +7,7 @@ import lombok.extern.slf4j.Slf4j; /** * A scope listener that receives the MDC/ThreadContext put and receive methods and update the trace - * and span reference anytime a new scope is activated. + * and span reference anytime a new scope is activated or closed. */ @Slf4j public class LogContextScopeListener implements ScopeListener { diff --git a/dd-java-agent/instrumentation/log4j1/log4j1.gradle b/dd-java-agent/instrumentation/log4j1/log4j1.gradle index 42ec36690c..33ff242c80 100644 --- a/dd-java-agent/instrumentation/log4j1/log4j1.gradle +++ b/dd-java-agent/instrumentation/log4j1/log4j1.gradle @@ -15,7 +15,7 @@ muzzle { configurations { // In order to test the real log4j library we need to remove the log4j transitive dependency // dependency brought in by :dd-java-agent:testing over 'log4j-over-slf4j' which would shadow - // the log4j module under test. + // the log4j module under test using a proxy to slf4j instead. testCompile.exclude group: 'org.slf4j', module: 'log4j-over-slf4j' } diff --git a/dd-java-agent/instrumentation/log4j1/src/main/java/datadog/trace/instrumentation/log4j1/Log4j1MDCInstrumentation.java b/dd-java-agent/instrumentation/log4j1/src/main/java/datadog/trace/instrumentation/log4j1/Log4j1MDCInstrumentation.java index 05a563f42a..16c71a1b8b 100644 --- a/dd-java-agent/instrumentation/log4j1/src/main/java/datadog/trace/instrumentation/log4j1/Log4j1MDCInstrumentation.java +++ b/dd-java-agent/instrumentation/log4j1/src/main/java/datadog/trace/instrumentation/log4j1/Log4j1MDCInstrumentation.java @@ -20,21 +20,18 @@ import net.bytebuddy.matcher.ElementMatcher; public class Log4j1MDCInstrumentation extends Instrumenter.Default { public static final String MDC_INSTRUMENTATION_NAME = "log4j-mdc"; - private static final String mdcClassName = "org.apache.log4j.MDC"; - public Log4j1MDCInstrumentation() { super(MDC_INSTRUMENTATION_NAME); } @Override protected boolean defaultEnabled() { - return Config.getBooleanSettingFromEnvironment( - Config.LOGS_INJECTION_ENABLED, Config.DEFAULT_LOGS_INJECTION_ENABLED); + return Config.get().isLogsInjectionEnabled(); } @Override public ElementMatcher typeMatcher() { - return named(mdcClassName); + return named("org.apache.log4j.MDC"); } @Override diff --git a/dd-java-agent/instrumentation/log4j1/src/main/java/datadoggggg/trace/instrumentation/log4j1/something/SomeClass.java b/dd-java-agent/instrumentation/log4j1/src/main/java/datadoggggg/trace/instrumentation/log4j1/something/SomeClass.java deleted file mode 100644 index b72c5f4a14..0000000000 --- a/dd-java-agent/instrumentation/log4j1/src/main/java/datadoggggg/trace/instrumentation/log4j1/something/SomeClass.java +++ /dev/null @@ -1,18 +0,0 @@ -package datadoggggg.trace.instrumentation.log4j1.something; - -public class SomeClass { - - private static SomeClass instance = new SomeClass(); - - public SomeClass() { - System.out.println("SomeClass Constructor.......!!!!!!!"); - } - - public static void put() { - instance.doSomething(); - } - - public void doSomething() { - System.out.println("SomeClass Doing something............"); - } -} diff --git a/dd-java-agent/instrumentation/log4j1/src/test/groovy/Log4j1MDCTest.groovy b/dd-java-agent/instrumentation/log4j1/src/test/groovy/Log4j1MDCTest.groovy index b54617e10d..2f0c8e4887 100644 --- a/dd-java-agent/instrumentation/log4j1/src/test/groovy/Log4j1MDCTest.groovy +++ b/dd-java-agent/instrumentation/log4j1/src/test/groovy/Log4j1MDCTest.groovy @@ -9,7 +9,9 @@ import java.util.concurrent.atomic.AtomicReference class Log4j1MDCTest extends AgentTestRunner { static { + ConfigUtils.updateConfig { System.setProperty("dd.logs.injection", "true") + } } def "MDC shows trace and span ids for active scope"() { diff --git a/dd-java-agent/instrumentation/log4j2/log4j2.gradle b/dd-java-agent/instrumentation/log4j2/log4j2.gradle index b9fb992551..c92bad8cf6 100644 --- a/dd-java-agent/instrumentation/log4j2/log4j2.gradle +++ b/dd-java-agent/instrumentation/log4j2/log4j2.gradle @@ -7,7 +7,7 @@ ext { configurations { // In order to test the real log4j library we need to remove the log4j transitive dependency // dependency brought in by :dd-java-agent:testing over 'log4j-over-slf4j' which would shadow - // the log4j module under test. + // the log4j module under test using a proxy to slf4j instead. testCompile.exclude group: 'org.slf4j', module: 'log4j-over-slf4j' } diff --git a/dd-java-agent/instrumentation/log4j2/src/main/java/datadog/trace/instrumentation/log4j2/ThreadContextInstrumentation.java b/dd-java-agent/instrumentation/log4j2/src/main/java/datadog/trace/instrumentation/log4j2/ThreadContextInstrumentation.java index 03d16f21ff..9aebc903f4 100644 --- a/dd-java-agent/instrumentation/log4j2/src/main/java/datadog/trace/instrumentation/log4j2/ThreadContextInstrumentation.java +++ b/dd-java-agent/instrumentation/log4j2/src/main/java/datadog/trace/instrumentation/log4j2/ThreadContextInstrumentation.java @@ -22,21 +22,18 @@ import net.bytebuddy.utility.JavaModule; public class ThreadContextInstrumentation extends Instrumenter.Default { public static final String MDC_INSTRUMENTATION_NAME = "log4j-thread-context"; - private static final String mdcClassName = "org.apache.logging.log4j.ThreadContext"; - public ThreadContextInstrumentation() { super(MDC_INSTRUMENTATION_NAME); } @Override protected boolean defaultEnabled() { - return Config.getBooleanSettingFromEnvironment( - Config.LOGS_INJECTION_ENABLED, Config.DEFAULT_LOGS_INJECTION_ENABLED); + return Config.get().isLogsInjectionEnabled(); } @Override public ElementMatcher typeMatcher() { - return named(mdcClassName); + return named("org.apache.logging.log4j.ThreadContext"); } @Override diff --git a/dd-java-agent/instrumentation/log4j2/src/test/groovy/Log4jThreadContextTest.groovy b/dd-java-agent/instrumentation/log4j2/src/test/groovy/Log4jThreadContextTest.groovy index 1590e002de..19b4daa547 100644 --- a/dd-java-agent/instrumentation/log4j2/src/test/groovy/Log4jThreadContextTest.groovy +++ b/dd-java-agent/instrumentation/log4j2/src/test/groovy/Log4jThreadContextTest.groovy @@ -10,7 +10,9 @@ import java.util.concurrent.atomic.AtomicReference class Log4jThreadContextTest extends AgentTestRunner { static { + ConfigUtils.updateConfig { System.setProperty("dd.logs.injection", "true") + } } def "ThreadContext shows trace and span ids for active scope"() {