# This is a combination of 2 commits.

richardstartin · richardstartin · commit 2a5b4b750663 · 2020-10-19T15:46:58.000+01:00
# This is the 1st commit message: make excludefilterprovider interface easier to work with # The commit message #2 will be skipped: # fixup
diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/java/concurrent/ExcludeFilter.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/java/concurrent/ExcludeFilter.java
@@ -1,5 +1,6 @@
 package datadog.trace.bootstrap.instrumentation.java.concurrent;
 
+import java.util.Collection;
 import java.util.EnumMap;
 import java.util.EnumSet;
 import java.util.HashSet;
@@ -78,11 +79,11 @@ protected EnumSet<ExcludeType> computeValue(Class<?> clazz) {
    *
    * @param excludeTypes the types to exclude
    */
-  public static void add(Map<ExcludeType, Set<String>> excludeTypes) {
+  public static void add(Map<ExcludeType, ? extends Collection<String>> excludeTypes) {
     if (excludeTypes.isEmpty()) {
       return;
     }
-    for (Map.Entry<ExcludeType, Set<String>> entry : excludeTypes.entrySet()) {
+    for (Map.Entry<ExcludeType, ? extends Collection<String>> entry : excludeTypes.entrySet()) {
       Set<String> currentExcluded = excludedClassNames.get(entry.getKey());
       currentExcluded.addAll(entry.getValue());
     }
diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/ExcludeFilterProvider.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/ExcludeFilterProvider.java
@@ -1,6 +1,7 @@
 package datadog.trace.agent.tooling;
 
 import datadog.trace.bootstrap.instrumentation.java.concurrent.ExcludeFilter.ExcludeType;
+import java.util.Collection;
 import java.util.Map;
 import java.util.Set;
 
@@ -17,5 +18,5 @@ public interface ExcludeFilterProvider {
    * @return A mapping from {@link ExcludeType} -> {@link Set<String>} for the class names that
    *     should be excluded from broad instrumentations like {@link Runnable}
    */
-  Map<ExcludeType, Set<String>> excludedClasses();
+  Map<ExcludeType, ? extends Collection<String>> excludedClasses();
 }
diff --git a/dd-java-agent/instrumentation/akka-concurrent/src/main/java/datadog/trace/instrumentation/akka/concurrent/AkkaForkJoinTaskInstrumentation.java b/dd-java-agent/instrumentation/akka-concurrent/src/main/java/datadog/trace/instrumentation/akka/concurrent/AkkaForkJoinTaskInstrumentation.java
@@ -19,11 +19,9 @@
 import datadog.trace.bootstrap.instrumentation.java.concurrent.State;
 import datadog.trace.context.TraceScope;
 import java.util.Arrays;
-import java.util.Collections;
+import java.util.Collection;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.Map;
-import java.util.Set;
 import lombok.extern.slf4j.Slf4j;
 import net.bytebuddy.asm.Advice;
 import net.bytebuddy.description.method.MethodDescription;
@@ -57,14 +55,13 @@ public ElementMatcher<? super TypeDescription> typeMatcher() {
   }
 
   @Override
-  public Map<ExcludeFilter.ExcludeType, Set<String>> excludedClasses() {
-    return Collections.<ExcludeFilter.ExcludeType, Set<String>>singletonMap(
+  public Map<ExcludeFilter.ExcludeType, ? extends Collection<String>> excludedClasses() {
+    return singletonMap(
         RUNNABLE_FUTURE,
-        new HashSet<>(
-            Arrays.asList(
-                "akka.dispatch.forkjoin.ForkJoinTask$AdaptedCallable",
-                "akka.dispatch.forkjoin.ForkJoinTask$AdaptedRunnable",
-                "akka.dispatch.forkjoin.ForkJoinTask$AdaptedRunnableAction")));
+        Arrays.asList(
+            "akka.dispatch.forkjoin.ForkJoinTask$AdaptedCallable",
+            "akka.dispatch.forkjoin.ForkJoinTask$AdaptedRunnable",
+            "akka.dispatch.forkjoin.ForkJoinTask$AdaptedRunnableAction"));
   }
 
   @Override
diff --git a/dd-java-agent/instrumentation/java-concurrent/java-completablefuture/src/main/java/datadog/trace/instrumentation/java/completablefuture/CompletableFutureUniCompletionInstrumentation.java b/dd-java-agent/instrumentation/java-concurrent/java-completablefuture/src/main/java/datadog/trace/instrumentation/java/completablefuture/CompletableFutureUniCompletionInstrumentation.java
@@ -10,11 +10,11 @@
 import datadog.trace.bootstrap.instrumentation.java.concurrent.ConcurrentState;
 import datadog.trace.bootstrap.instrumentation.java.concurrent.ExcludeFilter.ExcludeType;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.EnumMap;
-import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
-import java.util.Set;
 import net.bytebuddy.description.method.MethodDescription;
 import net.bytebuddy.description.type.TypeDescription;
 import net.bytebuddy.matcher.ElementMatcher;
@@ -73,7 +73,7 @@ public Map<? extends ElementMatcher<? super MethodDescription>, String> transfor
   }
 
   @Override
-  public Map<ExcludeType, Set<String>> excludedClasses() {
+  public Map<ExcludeType, ? extends Collection<String>> excludedClasses() {
     if (!enabled) {
       return Collections.emptyMap();
     }
@@ -105,8 +105,8 @@ public Map<ExcludeType, Set<String>> excludedClasses() {
       // This is not a subclass of UniCompletion and doesn't have a dependent CompletableFuture
       // "java.util.concurrent.CompletableFuture$Signaller",
     };
-    Set<String> excludedClasses = new HashSet<>(Arrays.asList(classes));
-    EnumMap<ExcludeType, Set<String>> excludedTypes = new EnumMap(ExcludeType.class);
+    List<String> excludedClasses = Arrays.asList(classes);
+    EnumMap<ExcludeType, Collection<String>> excludedTypes = new EnumMap<>(ExcludeType.class);
     excludedTypes.put(ExcludeType.RUNNABLE, excludedClasses);
     excludedTypes.put(ExcludeType.FORK_JOIN_TASK, excludedClasses);
     excludedTypes.put(ExcludeType.FUTURE, excludedClasses);
diff --git a/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/JavaForkJoinTaskInstrumentation.java b/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/JavaForkJoinTaskInstrumentation.java
@@ -4,6 +4,7 @@
 import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.namedOneOf;
 import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeScope;
 import static datadog.trace.bootstrap.instrumentation.java.concurrent.ExcludeFilter.ExcludeType.RUNNABLE_FUTURE;
+import static java.util.Collections.singletonMap;
 import static net.bytebuddy.matcher.ElementMatchers.isMethod;
 import static net.bytebuddy.matcher.ElementMatchers.named;
 
@@ -17,11 +18,10 @@
 import datadog.trace.bootstrap.instrumentation.java.concurrent.State;
 import datadog.trace.context.TraceScope;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.Map;
-import java.util.Set;
 import java.util.concurrent.ForkJoinTask;
 import lombok.extern.slf4j.Slf4j;
 import net.bytebuddy.asm.Advice;
@@ -59,7 +59,7 @@ public boolean matches(TypeDescription target) {
 
   @Override
   public Map<String, String> contextStoreForAll() {
-    return Collections.singletonMap("java.util.concurrent.ForkJoinTask", State.class.getName());
+    return singletonMap("java.util.concurrent.ForkJoinTask", State.class.getName());
   }
 
   @Override
@@ -72,14 +72,13 @@ public Map<? extends ElementMatcher<? super MethodDescription>, String> transfor
   }
 
   @Override
-  public Map<ExcludeType, Set<String>> excludedClasses() {
-    return Collections.<ExcludeType, Set<String>>singletonMap(
+  public Map<ExcludeType, ? extends Collection<String>> excludedClasses() {
+    return singletonMap(
         RUNNABLE_FUTURE,
-        new HashSet<>(
-            Arrays.asList(
-                "java.util.concurrent.ForkJoinTask$AdaptedCallable",
-                "java.util.concurrent.ForkJoinTask$AdaptedRunnable",
-                "java.util.concurrent.ForkJoinTask$AdaptedRunnableAction")));
+        Arrays.asList(
+            "java.util.concurrent.ForkJoinTask$AdaptedCallable",
+            "java.util.concurrent.ForkJoinTask$AdaptedRunnable",
+            "java.util.concurrent.ForkJoinTask$AdaptedRunnableAction"));
   }
 
   public static final class Exec {
diff --git a/dd-java-agent/instrumentation/scala-promise-2.10/src/main/java/datadog/trace/instrumentation/scala/promise/CallbackRunnableInstrumentation.java b/dd-java-agent/instrumentation/scala-promise-2.10/src/main/java/datadog/trace/instrumentation/scala/promise/CallbackRunnableInstrumentation.java
@@ -16,10 +16,10 @@
 import datadog.trace.bootstrap.instrumentation.java.concurrent.ExcludeFilter;
 import datadog.trace.bootstrap.instrumentation.java.concurrent.State;
 import datadog.trace.context.TraceScope;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
-import java.util.Set;
 import net.bytebuddy.asm.Advice;
 import net.bytebuddy.description.method.MethodDescription;
 import net.bytebuddy.description.type.TypeDescription;
@@ -53,7 +53,7 @@ public Map<? extends ElementMatcher<? super MethodDescription>, String> transfor
   }
 
   @Override
-  public Map<ExcludeFilter.ExcludeType, Set<String>> excludedClasses() {
+  public Map<ExcludeFilter.ExcludeType, ? extends Collection<String>> excludedClasses() {
     // force other instrumentations (e.g. Runnable) not to deal with this type
     return singletonMap(RUNNABLE, Collections.singleton("scala.concurrent.impl.CallbackRunnable"));
   }
diff --git a/dd-java-agent/instrumentation/scala-promise-2.13/src/main/java/datadog/trace/instrumentation/scala/concurrent/PromiseTransformationInstrumentation.java b/dd-java-agent/instrumentation/scala-promise-2.13/src/main/java/datadog/trace/instrumentation/scala/concurrent/PromiseTransformationInstrumentation.java
@@ -16,10 +16,10 @@
 import datadog.trace.bootstrap.instrumentation.java.concurrent.ExcludeFilter;
 import datadog.trace.bootstrap.instrumentation.java.concurrent.State;
 import datadog.trace.context.TraceScope;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
-import java.util.Set;
 import net.bytebuddy.asm.Advice;
 import net.bytebuddy.description.method.MethodDescription;
 import net.bytebuddy.description.type.TypeDescription;
@@ -41,7 +41,7 @@ public ElementMatcher<TypeDescription> typeMatcher() {
 
   @Override
   public Map<String, String> contextStore() {
-    return singletonMap("scala.concurrent.impl.Promise$Transformation", State.class.getName());
+    return singletonMap("s ocala.concurrent.impl.Promise$Transformation", State.class.getName());
   }
 
   @Override
@@ -54,7 +54,7 @@ public Map<? extends ElementMatcher<? super MethodDescription>, String> transfor
   }
 
   @Override
-  public Map<ExcludeFilter.ExcludeType, Set<String>> excludedClasses() {
+  public Map<ExcludeFilter.ExcludeType, ? extends Collection<String>> excludedClasses() {
     // make sure nothing else instruments this
     return singletonMap(
         RUNNABLE, Collections.singleton("scala.concurrent.impl.Promise$Transformation"));
diff --git a/dd-java-agent/testing/src/test/java/excludefilter/ExcludeFilterProviderTestInstrumentation.java b/dd-java-agent/testing/src/test/java/excludefilter/ExcludeFilterProviderTestInstrumentation.java
@@ -8,12 +8,11 @@
 import datadog.trace.agent.tooling.Instrumenter;
 import datadog.trace.bootstrap.instrumentation.java.concurrent.ExcludeFilter;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.EnumMap;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.Map;
-import java.util.Set;
 import java.util.concurrent.Callable;
 import net.bytebuddy.description.method.MethodDescription;
 import net.bytebuddy.description.type.TypeDescription;
@@ -46,22 +45,19 @@ public Map<String, String> contextStoreForAll() {
   }
 
   @Override
-  public Map<ExcludeFilter.ExcludeType, Set<String>> excludedClasses() {
-    EnumMap<ExcludeFilter.ExcludeType, Set<String>> excludedTypes =
-        new EnumMap(ExcludeFilter.ExcludeType.class);
+  public Map<ExcludeFilter.ExcludeType, ? extends Collection<String>> excludedClasses() {
+    EnumMap<ExcludeFilter.ExcludeType, Collection<String>> excludedTypes =
+        new EnumMap<>(ExcludeFilter.ExcludeType.class);
     excludedTypes.put(
         RUNNABLE,
-        new HashSet(
-            Arrays.asList(
-                ExcludedRunnable.class.getName(), CallableExcludedRunnable.class.getName())));
+        Arrays.asList(ExcludedRunnable.class.getName(), CallableExcludedRunnable.class.getName()));
     excludedTypes.put(
         CALLABLE,
-        new HashSet(
-            Arrays.asList(
-                ExcludedCallable.class.getName(),
-                RunnableExcludedCallable.class.getName(),
-                "net.sf.cglib.core.internal.LoadingCache$2" // Excluded for global ignore matcher
-                )));
+        Arrays.asList(
+            ExcludedCallable.class.getName(),
+            RunnableExcludedCallable.class.getName(),
+            "net.sf.cglib.core.internal.LoadingCache$2" // Excluded for global ignore matcher
+            ));
     return excludedTypes;
   }
 

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,6 @@`
`1`	`1`	`package datadog.trace.bootstrap.instrumentation.java.concurrent;`
`2`	`2`
	`3`	`+import java.util.Collection;`
`3`	`4`	`import java.util.EnumMap;`
`4`	`5`	`import java.util.EnumSet;`
`5`	`6`	`import java.util.HashSet;`
`@@ -78,11 +79,11 @@ protected EnumSet<ExcludeType> computeValue(Class<?> clazz) {`
`78`	`79`	`*`
`79`	`80`	`* @param excludeTypes the types to exclude`
`80`	`81`	`*/`
`81`		`- public static void add(Map<ExcludeType, Set<String>> excludeTypes) {`
	`82`	`+ public static void add(Map<ExcludeType, ? extends Collection<String>> excludeTypes) {`
`82`	`83`	`if (excludeTypes.isEmpty()) {`
`83`	`84`	`return;`
`84`	`85`	`}`
`85`		`- for (Map.Entry<ExcludeType, Set<String>> entry : excludeTypes.entrySet()) {`
	`86`	`+ for (Map.Entry<ExcludeType, ? extends Collection<String>> entry : excludeTypes.entrySet()) {`
`86`	`87`	`Set<String> currentExcluded = excludedClassNames.get(entry.getKey());`
`87`	`88`	`currentExcluded.addAll(entry.getValue());`
`88`	`89`	`}`