From 106b59f11ee0e554e4b5f66c20ceee40c8e1be6e Mon Sep 17 00:00:00 2001 From: Richard Startin Date: Tue, 30 Jun 2020 21:04:16 +0100 Subject: [PATCH 1/3] Exclude clojure standard library and anonymous functions from class transformation (DataDog/dd-trace-java#1642) --- .../auto/tooling/matcher/GlobalIgnoresMatcher.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/agent-tooling/src/main/java/io/opentelemetry/auto/tooling/matcher/GlobalIgnoresMatcher.java b/agent-tooling/src/main/java/io/opentelemetry/auto/tooling/matcher/GlobalIgnoresMatcher.java index 45604bf495..758ae5315b 100644 --- a/agent-tooling/src/main/java/io/opentelemetry/auto/tooling/matcher/GlobalIgnoresMatcher.java +++ b/agent-tooling/src/main/java/io/opentelemetry/auto/tooling/matcher/GlobalIgnoresMatcher.java @@ -89,6 +89,10 @@ public class GlobalIgnoresMatcher } return true; } + // clojure + if (name.startsWith("clojure.") || name.contains("$fn__")) { + return true; + } if (name.startsWith("io.opentelemetry.auto.")) { // FIXME: We should remove this once From ad1941528cbaf54603341dcf87ab635a19f1e658 Mon Sep 17 00:00:00 2001 From: Lev Priima <1118651+lpriima@users.noreply.github.com> Date: Wed, 1 Jul 2020 14:37:45 -0700 Subject: [PATCH 2/3] MDC ThreadLocal initValue should be modifiable for SLF4j's copy-on-write thread context map (DataDog/dd-trace-java#1645) --- .../auto/test/ClassLoaderMatcherTest.groovy | 6 +++++ .../log4j/v1_1/Log4jMDCInstrumentation.java | 3 +-- .../src/test/groovy/Log4jMDCTest.groovy | 11 +++++++++ .../test/groovy/Log4jThreadContextTest.groovy | 10 ++++++++ .../mdc/MDCInjectionInstrumentation.java | 2 +- .../src/test/groovy/Slf4jMDCTest.groovy | 10 ++++++++ .../LogContextInjectionTestBase.groovy | 24 +++++++++++++++++++ 7 files changed, 63 insertions(+), 3 deletions(-) diff --git a/agent-tooling/src/test/groovy/io/opentelemetry/auto/test/ClassLoaderMatcherTest.groovy b/agent-tooling/src/test/groovy/io/opentelemetry/auto/test/ClassLoaderMatcherTest.groovy index 54f2137a23..7b690d5c5e 100644 --- a/agent-tooling/src/test/groovy/io/opentelemetry/auto/test/ClassLoaderMatcherTest.groovy +++ b/agent-tooling/src/test/groovy/io/opentelemetry/auto/test/ClassLoaderMatcherTest.groovy @@ -19,6 +19,7 @@ package io.opentelemetry.auto.test import io.opentelemetry.auto.bootstrap.AgentClassLoader import io.opentelemetry.auto.tooling.ClassLoaderMatcher import io.opentelemetry.auto.tooling.ExporterClassLoader +import io.opentelemetry.auto.tooling.log.LogContextScopeListener import io.opentelemetry.auto.util.test.AgentSpecification class ClassLoaderMatcherTest extends AgentSpecification { @@ -60,4 +61,9 @@ class ClassLoaderMatcherTest extends AgentSpecification { expect: ExporterClassLoader.name == "io.opentelemetry.auto.tooling.ExporterClassLoader" } + + def "helper class names are hardcoded in Log Instrumentations"() { + expect: + LogContextScopeListener.name == "io.opentelemetry.auto.tooling.log.LogContextScopeListener" + } } diff --git a/instrumentation/log4j/log4j-1.1/src/main/java/io/opentelemetry/auto/instrumentation/log4j/v1_1/Log4jMDCInstrumentation.java b/instrumentation/log4j/log4j-1.1/src/main/java/io/opentelemetry/auto/instrumentation/log4j/v1_1/Log4jMDCInstrumentation.java index 32dc2f9b05..26c3ae7d35 100644 --- a/instrumentation/log4j/log4j-1.1/src/main/java/io/opentelemetry/auto/instrumentation/log4j/v1_1/Log4jMDCInstrumentation.java +++ b/instrumentation/log4j/log4j-1.1/src/main/java/io/opentelemetry/auto/instrumentation/log4j/v1_1/Log4jMDCInstrumentation.java @@ -22,7 +22,6 @@ import static net.bytebuddy.matcher.ElementMatchers.named; import io.opentelemetry.auto.config.Config; import io.opentelemetry.auto.tooling.Instrumenter; -import io.opentelemetry.auto.tooling.log.LogContextScopeListener; import java.lang.reflect.Method; import java.util.Map; import net.bytebuddy.asm.Advice; @@ -55,7 +54,7 @@ public class Log4jMDCInstrumentation extends Instrumenter.Default { @Override public String[] helperClassNames() { - return new String[] {LogContextScopeListener.class.getName()}; + return new String[] {"io.opentelemetry.auto.tooling.log.LogContextScopeListener"}; } public static class MDCContextAdvice { diff --git a/instrumentation/log4j/log4j-1.1/src/test/groovy/Log4jMDCTest.groovy b/instrumentation/log4j/log4j-1.1/src/test/groovy/Log4jMDCTest.groovy index a0822c3445..50922358f4 100644 --- a/instrumentation/log4j/log4j-1.1/src/test/groovy/Log4jMDCTest.groovy +++ b/instrumentation/log4j/log4j-1.1/src/test/groovy/Log4jMDCTest.groovy @@ -38,4 +38,15 @@ class Log4jMDCTest extends LogContextInjectionTestBase { def get(String key) { return MDC.get(key) } + + @Override + def remove(String key) { + MDC.context + return MDC.remove(key) + } + + @Override + def clear() { + return MDC.clear() + } } diff --git a/instrumentation/log4j/log4j-2.0/src/test/groovy/Log4jThreadContextTest.groovy b/instrumentation/log4j/log4j-2.0/src/test/groovy/Log4jThreadContextTest.groovy index bbd6553315..6d981532e8 100644 --- a/instrumentation/log4j/log4j-2.0/src/test/groovy/Log4jThreadContextTest.groovy +++ b/instrumentation/log4j/log4j-2.0/src/test/groovy/Log4jThreadContextTest.groovy @@ -31,4 +31,14 @@ class Log4jThreadContextTest extends LogContextInjectionTestBase { def get(String key) { return ThreadContext.get(key) } + + @Override + def remove(String key) { + return ThreadContext.remove(key) + } + + @Override + def clear() { + return ThreadContext.clearAll() + } } diff --git a/instrumentation/slf4j-mdc/src/main/java/io/opentelemetry/auto/instrumentation/slf4j/mdc/MDCInjectionInstrumentation.java b/instrumentation/slf4j-mdc/src/main/java/io/opentelemetry/auto/instrumentation/slf4j/mdc/MDCInjectionInstrumentation.java index 8808145e5f..029d5f6a9d 100644 --- a/instrumentation/slf4j-mdc/src/main/java/io/opentelemetry/auto/instrumentation/slf4j/mdc/MDCInjectionInstrumentation.java +++ b/instrumentation/slf4j-mdc/src/main/java/io/opentelemetry/auto/instrumentation/slf4j/mdc/MDCInjectionInstrumentation.java @@ -70,7 +70,7 @@ public class MDCInjectionInstrumentation extends Instrumenter.Default { @Override public String[] helperClassNames() { - return new String[] {LogContextScopeListener.class.getName()}; + return new String[] {"io.opentelemetry.auto.tooling.log.LogContextScopeListener"}; } public static class MDCAdvice { diff --git a/instrumentation/slf4j-mdc/src/test/groovy/Slf4jMDCTest.groovy b/instrumentation/slf4j-mdc/src/test/groovy/Slf4jMDCTest.groovy index eeabccac08..4591ba2850 100644 --- a/instrumentation/slf4j-mdc/src/test/groovy/Slf4jMDCTest.groovy +++ b/instrumentation/slf4j-mdc/src/test/groovy/Slf4jMDCTest.groovy @@ -12,4 +12,14 @@ class Slf4jMDCTest extends LogContextInjectionTestBase { def get(String key) { return MDC.get(key) } + + @Override + def remove(String key) { + return MDC.remove(key) + } + + @Override + def clear() { + return MDC.clear() + } } diff --git a/testing-common/src/main/groovy/io/opentelemetry/auto/test/log/injection/LogContextInjectionTestBase.groovy b/testing-common/src/main/groovy/io/opentelemetry/auto/test/log/injection/LogContextInjectionTestBase.groovy index 68fb947e86..8bce673945 100644 --- a/testing-common/src/main/groovy/io/opentelemetry/auto/test/log/injection/LogContextInjectionTestBase.groovy +++ b/testing-common/src/main/groovy/io/opentelemetry/auto/test/log/injection/LogContextInjectionTestBase.groovy @@ -44,6 +44,13 @@ abstract class LogContextInjectionTestBase extends AgentTestRunner { */ abstract get(String key) + /** + * Remove from the framework-specific context the value at the given key + */ + abstract remove(String key) + + abstract clear() + static { ConfigUtils.updateConfig { System.setProperty("ota.logs.injection", "true") @@ -139,4 +146,21 @@ abstract class LogContextInjectionTestBase extends AgentTestRunner { mainSpan?.end() mainScope?.close() } + + def "modify thread context after clear of context map at the beginning of new thread"() { + def t1A + final Thread thread1 = new Thread() { + @Override + void run() { + clear() + put("a", "a thread1") + t1A = get("a") + } + } + thread1.start() + thread1.join() + + expect: + t1A == "a thread1" + } } From 95492b739257e6be0da09c3d6fb53d32901f0e10 Mon Sep 17 00:00:00 2001 From: Richard Startin Date: Thu, 2 Jul 2020 10:14:39 +0100 Subject: [PATCH 3/3] Handle null header values (DataDog/dd-trace-java#1650) --- .../instrumentation/kafkaclients/TextMapExtractAdapter.java | 6 +++++- .../instrumentation/kafkastreams/TextMapExtractAdapter.java | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/instrumentation/kafka-clients-0.11/src/main/java/io/opentelemetry/auto/instrumentation/kafkaclients/TextMapExtractAdapter.java b/instrumentation/kafka-clients-0.11/src/main/java/io/opentelemetry/auto/instrumentation/kafkaclients/TextMapExtractAdapter.java index fdaaaf0dbe..278b82e622 100644 --- a/instrumentation/kafka-clients-0.11/src/main/java/io/opentelemetry/auto/instrumentation/kafkaclients/TextMapExtractAdapter.java +++ b/instrumentation/kafka-clients-0.11/src/main/java/io/opentelemetry/auto/instrumentation/kafkaclients/TextMapExtractAdapter.java @@ -31,6 +31,10 @@ public class TextMapExtractAdapter implements HttpTextFormat.Getter { if (header == null) { return null; } - return new String(header.value(), StandardCharsets.UTF_8); + byte[] value = header.value(); + if (value == null) { + return null; + } + return new String(value, StandardCharsets.UTF_8); } } diff --git a/instrumentation/kafka-streams-0.11/src/main/java/io/opentelemetry/auto/instrumentation/kafkastreams/TextMapExtractAdapter.java b/instrumentation/kafka-streams-0.11/src/main/java/io/opentelemetry/auto/instrumentation/kafkastreams/TextMapExtractAdapter.java index 8449938c8a..e7765dc794 100644 --- a/instrumentation/kafka-streams-0.11/src/main/java/io/opentelemetry/auto/instrumentation/kafkastreams/TextMapExtractAdapter.java +++ b/instrumentation/kafka-streams-0.11/src/main/java/io/opentelemetry/auto/instrumentation/kafkastreams/TextMapExtractAdapter.java @@ -31,6 +31,10 @@ public class TextMapExtractAdapter implements HttpTextFormat.Getter { if (header == null) { return null; } - return new String(header.value(), StandardCharsets.UTF_8); + byte[] value = header.value(); + if (value == null) { + return null; + } + return new String(value, StandardCharsets.UTF_8); } }