Allow to disable auto-instrumentation for specified methods (#369)

Add new configuration property, `trace.methods.exclude`, which marks methods which should be excluded from instrumentation. This does not affect any arbitrary auto-instrumentation. Only auto-instrumentation guided by supported annotations (see `trace.annotations` config option and its default value in TraceAnnotationsInstrumentation#DEFAULT_ANNOTATIONS) and by `trace.methods` configuration property can be disabled using this new configuration.

agent-bootstrap/src/main/java/io/opentelemetry/auto/config/Config.java

@@ -62,6 +62,7 @@ public class Config {
   public static final String TRACE_EXECUTORS_ALL = "trace.executors.all";
   public static final String TRACE_EXECUTORS = "trace.executors";
   public static final String TRACE_METHODS = "trace.methods";
+  public static final String TRACE_METHODS_EXCLUDE = "trace.methods.exclude";
   public static final String TRACE_CLASSES_EXCLUDE = "trace.classes.exclude";
   public static final String HTTP_SERVER_ERROR_STATUSES = "http.server.error.statuses";
   public static final String HTTP_CLIENT_ERROR_STATUSES = "http.client.error.statuses";
@@ -95,6 +96,7 @@ public class Config {
   private static final boolean DEFAULT_TRACE_EXECUTORS_ALL = false;
   private static final String DEFAULT_TRACE_EXECUTORS = "";
   private static final String DEFAULT_TRACE_METHODS = null;
+  private static final String DEFAULT_TRACE_METHODS_EXCLUDE = null;
 
   public static final String SQL_NORMALIZER_ENABLED = "sql.normalizer.enabled";
   public static final boolean DEFAULT_SQL_NORMALIZER_ENABLED = true;
@@ -132,6 +134,7 @@ public class Config {
   @Getter private final String traceAnnotations;
 
   @Getter private final String traceMethods;
+  @Getter private final String traceMethodsExclude;
 
   @Getter private final boolean traceExecutorsAll;
   @Getter private final List<String> traceExecutors;
@@ -188,6 +191,8 @@ public class Config {
     traceAnnotations = getSettingFromEnvironment(TRACE_ANNOTATIONS, DEFAULT_TRACE_ANNOTATIONS);
 
     traceMethods = getSettingFromEnvironment(TRACE_METHODS, DEFAULT_TRACE_METHODS);
+    traceMethodsExclude =
+        getSettingFromEnvironment(TRACE_METHODS_EXCLUDE, DEFAULT_TRACE_METHODS_EXCLUDE);
 
     traceExecutorsAll =
         getBooleanSettingFromEnvironment(TRACE_EXECUTORS_ALL, DEFAULT_TRACE_EXECUTORS_ALL);
@@ -246,6 +251,7 @@ public class Config {
     traceAnnotations = properties.getProperty(TRACE_ANNOTATIONS, parent.traceAnnotations);
 
     traceMethods = properties.getProperty(TRACE_METHODS, parent.traceMethods);
+    traceMethodsExclude = properties.getProperty(TRACE_METHODS_EXCLUDE, parent.traceMethodsExclude);
 
     traceExecutorsAll =
         getPropertyBooleanValue(properties, TRACE_EXECUTORS_ALL, parent.traceExecutorsAll);

instrumentation/trace-annotation/src/main/java/io/opentelemetry/auto/instrumentation/traceannotation/MethodsConfigurationParser.java

@@ -0,0 +1,95 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.opentelemetry.auto.instrumentation.traceannotation;
+
+import com.google.common.collect.Maps;
+import com.google.common.collect.Sets;
+import io.opentelemetry.auto.config.Config;
+import java.util.Collections;
+import java.util.Map;
+import java.util.Set;
+import lombok.extern.slf4j.Slf4j;
+
+@Slf4j
+public class MethodsConfigurationParser {
+  static final String PACKAGE_CLASS_NAME_REGEX = "[\\w.$]+";
+  private static final String METHOD_LIST_REGEX = "\\s*(?:\\w+\\s*,)*\\s*(?:\\w+\\s*,?)\\s*";
+  private static final String CONFIG_FORMAT =
+      "(?:\\s*"
+          + PACKAGE_CLASS_NAME_REGEX
+          + "\\["
+          + METHOD_LIST_REGEX
+          + "]\\s*;)*\\s*"
+          + PACKAGE_CLASS_NAME_REGEX
+          + "\\["
+          + METHOD_LIST_REGEX
+          + "]";
+
+  /**
+   * This method takes a string in a form of {@code
+   * "io.package.ClassName[method1,method2];my.example[someMethodName];"} and returns a map where
+   * keys are class names and corresponding value is a set of methods for that class.
+   *
+   * <p>Strings of such format are used e.g. to configure {@link Config#getTraceMethods()} and
+   * {@link Config#getExcludedMethods()}
+   */
+  public static Map<String, Set<String>> parse(String configString) {
+    if (configString == null || configString.trim().isEmpty()) {
+      return Collections.emptyMap();
+    } else if (!validateConfigString(configString)) {
+      log.warn(
+          "Invalid trace method config '{}'. Must match 'package.Class$Name[method1,method2];*'.",
+          configString);
+      return Collections.emptyMap();
+    } else {
+      final Map<String, Set<String>> toTrace = Maps.newHashMap();
+      final String[] classMethods = configString.split(";", -1);
+      for (final String classMethod : classMethods) {
+        if (classMethod.trim().isEmpty()) {
+          continue;
+        }
+        final String[] splitClassMethod = classMethod.split("\\[", -1);
+        final String className = splitClassMethod[0];
+        final String method = splitClassMethod[1].trim();
+        final String methodNames = method.substring(0, method.length() - 1);
+        final String[] splitMethodNames = methodNames.split(",", -1);
+        final Set<String> trimmedMethodNames =
+            Sets.newHashSetWithExpectedSize(splitMethodNames.length);
+        for (final String methodName : splitMethodNames) {
+          final String trimmedMethodName = methodName.trim();
+          if (!trimmedMethodName.isEmpty()) {
+            trimmedMethodNames.add(trimmedMethodName);
+          }
+        }
+        if (!trimmedMethodNames.isEmpty()) {
+          toTrace.put(className.trim(), trimmedMethodNames);
+        }
+      }
+      return toTrace;
+    }
+  }
+
+  private static boolean validateConfigString(final String configString) {
+    for (final String segment : configString.split(";")) {
+      if (!segment.trim().matches(CONFIG_FORMAT)) {
+        return false;
+      }
+    }
+    return true;
+  }
+
+  private MethodsConfigurationParser() {}
+}

instrumentation/trace-annotation/src/main/java/io/opentelemetry/auto/instrumentation/traceannotation/TraceAnnotationsInstrumentation.java

@@ -15,14 +15,15 @@
  */
 package io.opentelemetry.auto.instrumentation.traceannotation;
 
-import static io.opentelemetry.auto.instrumentation.traceannotation.TraceConfigInstrumentation.PACKAGE_CLASS_NAME_REGEX;
 import static io.opentelemetry.auto.tooling.ClassLoaderMatcher.hasClassesNamed;
 import static io.opentelemetry.auto.tooling.bytebuddy.matcher.AgentElementMatchers.safeHasSuperType;
 import static java.util.Collections.singletonMap;
 import static net.bytebuddy.matcher.ElementMatchers.declaresMethod;
 import static net.bytebuddy.matcher.ElementMatchers.isAnnotatedWith;
+import static net.bytebuddy.matcher.ElementMatchers.isDeclaredBy;
 import static net.bytebuddy.matcher.ElementMatchers.named;
 import static net.bytebuddy.matcher.ElementMatchers.none;
+import static net.bytebuddy.matcher.ElementMatchers.not;
 
 import com.google.auto.service.AutoService;
 import com.google.common.collect.Sets;
@@ -32,15 +33,19 @@ import java.util.Collections;
 import java.util.Map;
 import java.util.Set;
 import lombok.extern.slf4j.Slf4j;
+import net.bytebuddy.description.ByteCodeElement;
 import net.bytebuddy.description.NamedElement;
 import net.bytebuddy.description.method.MethodDescription;
 import net.bytebuddy.description.type.TypeDescription;
 import net.bytebuddy.matcher.ElementMatcher;
+import net.bytebuddy.matcher.ElementMatchers;
 
 @Slf4j
 @AutoService(Instrumenter.class)
 public final class TraceAnnotationsInstrumentation extends Instrumenter.Default {
 
+  private static final String PACKAGE_CLASS_NAME_REGEX = "[\\w.$]+";
+
   static final String CONFIG_FORMAT =
       "(?:\\s*"
           + PACKAGE_CLASS_NAME_REGEX
@@ -62,7 +67,11 @@ public final class TraceAnnotationsInstrumentation extends Instrumenter.Default
       };
 
   private final Set<String> additionalTraceAnnotations;
-  private final ElementMatcher.Junction<NamedElement> methodTraceMatcher;
+  private final ElementMatcher.Junction<NamedElement> annotationMatcher;
+  /*
+  This matcher matches all methods that should be excluded from transformation
+   */
+  private final ElementMatcher.Junction<MethodDescription> excludedMethodsMatcher;
 
   public TraceAnnotationsInstrumentation() {
     super("trace", "trace-annotation");
@@ -90,7 +99,7 @@ public final class TraceAnnotationsInstrumentation extends Instrumenter.Default
     }
 
     if (additionalTraceAnnotations.isEmpty()) {
-      methodTraceMatcher = none();
+      annotationMatcher = none();
     } else {
       ElementMatcher.Junction<NamedElement> methodTraceMatcher = null;
       for (final String annotationName : additionalTraceAnnotations) {
@@ -100,8 +109,31 @@ public final class TraceAnnotationsInstrumentation extends Instrumenter.Default
           methodTraceMatcher = methodTraceMatcher.or(named(annotationName));
         }
       }
-      this.methodTraceMatcher = methodTraceMatcher;
+      this.annotationMatcher = methodTraceMatcher;
     }
+
+    excludedMethodsMatcher = configureExcludedMethods();
+  }
+
+  private ElementMatcher.Junction<MethodDescription> configureExcludedMethods() {
+    ElementMatcher.Junction<MethodDescription> result = none();
+
+    Map<String, Set<String>> excludedMethods =
+        MethodsConfigurationParser.parse(Config.get().getTraceMethodsExclude());
+    for (Map.Entry<String, Set<String>> entry : excludedMethods.entrySet()) {
+      String className = entry.getKey();
+      ElementMatcher.Junction<ByteCodeElement> classMather =
+          isDeclaredBy(ElementMatchers.<TypeDescription>named(className));
+
+      ElementMatcher.Junction<MethodDescription> excludedMethodsMatcher = none();
+      for (String methodName : entry.getValue()) {
+        excludedMethodsMatcher = excludedMethodsMatcher.or(ElementMatchers.named(methodName));
+      }
+
+      result = result.or(classMather.and(excludedMethodsMatcher));
+    }
+
+    return result;
   }
 
   @Override
@@ -123,7 +155,7 @@ public final class TraceAnnotationsInstrumentation extends Instrumenter.Default
 
   @Override
   public ElementMatcher<TypeDescription> typeMatcher() {
-    return safeHasSuperType(declaresMethod(isAnnotatedWith(methodTraceMatcher)));
+    return safeHasSuperType(declaresMethod(isAnnotatedWith(annotationMatcher)));
   }
 
   @Override
@@ -135,6 +167,8 @@ public final class TraceAnnotationsInstrumentation extends Instrumenter.Default
 
   @Override
   public Map<? extends ElementMatcher<? super MethodDescription>, String> transformers() {
-    return singletonMap(isAnnotatedWith(methodTraceMatcher), packageName + ".TraceAdvice");
+    return singletonMap(
+        isAnnotatedWith(annotationMatcher).and(not(excludedMethodsMatcher)),
+        packageName + ".TraceAdvice");
   }
 }

instrumentation/trace-annotation/src/main/java/io/opentelemetry/auto/instrumentation/traceannotation/TraceConfigInstrumentation.java

@@ -20,8 +20,6 @@ import static io.opentelemetry.auto.tooling.bytebuddy.matcher.AgentElementMatche
 import static net.bytebuddy.matcher.ElementMatchers.named;
 
 import com.google.auto.service.AutoService;
-import com.google.common.collect.Maps;
-import com.google.common.collect.Sets;
 import io.opentelemetry.auto.config.Config;
 import io.opentelemetry.auto.tooling.Instrumenter;
 import java.util.Collections;
@@ -46,66 +44,18 @@ import net.bytebuddy.matcher.ElementMatcher;
 @AutoService(Instrumenter.class)
 public class TraceConfigInstrumentation implements Instrumenter {
 
-  static final String PACKAGE_CLASS_NAME_REGEX = "[\\w.\\$]+";
-  private static final String METHOD_LIST_REGEX = "\\s*(?:\\w+\\s*,)*\\s*(?:\\w+\\s*,?)\\s*";
-  private static final String CONFIG_FORMAT =
-      "(?:\\s*"
-          + PACKAGE_CLASS_NAME_REGEX
-          + "\\["
-          + METHOD_LIST_REGEX
-          + "\\]\\s*;)*\\s*"
-          + PACKAGE_CLASS_NAME_REGEX
-          + "\\["
-          + METHOD_LIST_REGEX
-          + "\\]";
-
   private final Map<String, Set<String>> classMethodsToTrace;
 
-  private boolean validateConfigString(final String configString) {
-    for (final String segment : configString.split(";")) {
-      if (!segment.trim().matches(CONFIG_FORMAT)) {
-        return false;
-      }
-    }
-    return true;
-  }
-
   public TraceConfigInstrumentation() {
-    final String configString = Config.get().getTraceMethods();
+    classMethodsToTrace = MethodsConfigurationParser.parse(Config.get().getTraceMethods());
-    if (configString == null || configString.trim().isEmpty()) {
-      classMethodsToTrace = Collections.emptyMap();
 
-    } else if (!validateConfigString(configString)) {
+    Map<String, Set<String>> excludedMethods =
-      log.warn(
+        MethodsConfigurationParser.parse(Config.get().getTraceMethodsExclude());
-          "Invalid trace method config '{}'. Must match 'package.Class$Name[method1,method2];*'.",
+    for (Map.Entry<String, Set<String>> entry : excludedMethods.entrySet()) {
-          configString);
+      Set<String> tracedMethods = classMethodsToTrace.get(entry.getKey());
-      classMethodsToTrace = Collections.emptyMap();
+      if (tracedMethods != null) {
-
+        tracedMethods.removeAll(entry.getValue());
-    } else {
-      final Map<String, Set<String>> toTrace = Maps.newHashMap();
-      final String[] classMethods = configString.split(";", -1);
-      for (final String classMethod : classMethods) {
-        if (classMethod.trim().isEmpty()) {
-          continue;
-        }
-        final String[] splitClassMethod = classMethod.split("\\[", -1);
-        final String className = splitClassMethod[0];
-        final String method = splitClassMethod[1].trim();
-        final String methodNames = method.substring(0, method.length() - 1);
-        final String[] splitMethodNames = methodNames.split(",", -1);
-        final Set<String> trimmedMethodNames =
-            Sets.newHashSetWithExpectedSize(splitMethodNames.length);
-        for (final String methodName : splitMethodNames) {
-          final String trimmedMethodName = methodName.trim();
-          if (!trimmedMethodName.isEmpty()) {
-            trimmedMethodNames.add(trimmedMethodName);
-          }
-        }
-        if (!trimmedMethodNames.isEmpty()) {
-          toTrace.put(className.trim(), trimmedMethodNames);
-        }
       }
-      classMethodsToTrace = Collections.unmodifiableMap(toTrace);
     }
   }
 

instrumentation/trace-annotation/src/test/groovy/TraceConfigTest.groovy

@@ -13,7 +13,6 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import io.opentelemetry.auto.instrumentation.traceannotation.TraceConfigInstrumentation
 import io.opentelemetry.auto.test.AgentTestRunner
 import io.opentelemetry.auto.test.utils.ConfigUtils
 
@@ -55,34 +54,4 @@ class TraceConfigTest extends AgentTestRunner {
       }
     }
   }
-
-  def "test configuration #value"() {
-    setup:
-    ConfigUtils.updateConfig {
-      if (value) {
-        System.properties.setProperty("ota.trace.methods", value)
-      } else {
-        System.clearProperty("ota.trace.methods")
-      }
-    }
-
-    expect:
-    new TraceConfigInstrumentation().classMethodsToTrace == expected
-
-    cleanup:
-    System.clearProperty("ota.trace.methods")
-
-    where:
-    value                                                           | expected
-    null                                                            | [:]
-    " "                                                             | [:]
-    "some.package.ClassName"                                        | [:]
-    "some.package.ClassName[ , ]"                                   | [:]
-    "some.package.ClassName[ , method]"                             | [:]
-    "some.package.Class\$Name[ method , ]"                          | ["some.package.Class\$Name": ["method"].toSet()]
-    "ClassName[ method1,]"                                          | ["ClassName": ["method1"].toSet()]
-    "ClassName[method1 , method2]"                                  | ["ClassName": ["method1", "method2"].toSet()]
-    "Class\$1[method1 ] ; Class\$2[ method2];"                      | ["Class\$1": ["method1"].toSet(), "Class\$2": ["method2"].toSet()]
-    "Duplicate[method1] ; Duplicate[method2]  ;Duplicate[method3];" | ["Duplicate": ["method3"].toSet()]
-  }
 }

instrumentation/trace-annotation/src/test/groovy/TraceProvidersTest.groovy

@@ -25,6 +25,7 @@ class TraceProvidersTest extends AgentTestRunner {
   static {
     ConfigUtils.updateConfig {
       System.clearProperty("ota.trace.annotations")
+      //Don't bother to instrument inner closures of this test class
       System.setProperty("ota.trace.classes.exclude", TraceProvidersTest.name + "*")
     }
   }

instrumentation/trace-annotation/src/test/groovy/TracedMethodsExclusionTest.groovy

@@ -0,0 +1,118 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+import io.opentracing.contrib.dropwizard.Trace
+import io.opentelemetry.auto.test.AgentTestRunner
+import io.opentelemetry.auto.test.utils.ConfigUtils
+
+class TracedMethodsExclusionTest extends AgentTestRunner {
+
+  static {
+    ConfigUtils.updateConfig {
+      System.setProperty("ota.trace.methods", "${TestClass.name}[included,excluded]")
+      System.setProperty("ota.trace.methods.exclude", "${TestClass.name}[excluded,annotatedButExcluded]")
+    }
+  }
+
+  def specCleanup() {
+    ConfigUtils.updateConfig {
+      System.clearProperty("ota.trace.methods")
+      System.clearProperty("ota.trace.methods.exclude")
+    }
+  }
+
+  static class TestClass {
+    //This method is configured to be traced
+    String included() {
+      return "Hello!"
+    }
+
+    //This method is not mentioned in any configuration
+    String notMentioned() {
+      return "Hello!"
+    }
+
+    //This method is both configured to be traced and to be excluded. Should NOT be traced.
+    String excluded() {
+      return "Hello!"
+    }
+
+    //This method is annotated with annotation which usually results in a captured span
+    @Trace
+    String annotated() {
+      return "Hello!"
+    }
+
+    //This method is annotated with annotation which usually results in a captured span, but is configured to be
+    //excluded.
+    @Trace
+    String annotatedButExcluded() {
+      return "Hello!"
+    }
+  }
+
+
+  //Baseline and assumption validation
+  def "Calling these methods should be traced"() {
+    expect:
+    new TestClass().included() == "Hello!"
+    new TestClass().annotated() == "Hello!"
+
+    and:
+    assertTraces(2) {
+      trace(0, 1) {
+        span(0) {
+          operationName "TestClass.included"
+          tags {
+          }
+        }
+      }
+      trace(1, 1) {
+        span(0) {
+          operationName "TestClass.annotated"
+          tags {
+          }
+        }
+      }
+    }
+  }
+
+  def "Not explicitly configured method should not be traced"() {
+    expect:
+    new TestClass().notMentioned() == "Hello!"
+
+    and:
+    Thread.sleep(500) // sleep a bit just to make sure no span is captured
+    assertTraces(0) {}
+  }
+
+  def "Method which is both included and excluded for tracing should NOT be traced"() {
+    expect:
+    new TestClass().excluded() == "Hello!"
+
+    and:
+    Thread.sleep(500) // sleep a bit just to make sure no span is captured
+    assertTraces(0) {}
+  }
+
+  def "Method exclusion should override tracing annotations"() {
+    expect:
+    new TestClass().annotatedButExcluded() == "Hello!"
+
+    and:
+    Thread.sleep(500) // sleep a bit just to make sure no span is captured
+    assertTraces(0) {}
+  }
+}

instrumentation/trace-annotation/src/test/groovy/io/opentelemetry/auto/instrumentation/traceannotation/MethodsConfigurationParserTest.groovy

@@ -0,0 +1,41 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.opentelemetry.auto.instrumentation.traceannotation
+
+
+import spock.lang.Specification
+
+class MethodsConfigurationParserTest extends Specification {
+
+  def "test configuration #value"() {
+    expect:
+    MethodsConfigurationParser.parse(value) == expected
+
+    where:
+    value                                                           | expected
+    null                                                            | [:]
+    " "                                                             | [:]
+    "some.package.ClassName"                                        | [:]
+    "some.package.ClassName[ , ]"                                   | [:]
+    "some.package.ClassName[ , method]"                             | [:]
+    "some.package.Class\$Name[ method , ]"                          | ["some.package.Class\$Name": ["method"].toSet()]
+    "ClassName[ method1,]"                                          | ["ClassName": ["method1"].toSet()]
+    "ClassName[method1 , method2]"                                  | ["ClassName": ["method1", "method2"].toSet()]
+    "Class\$1[method1 ] ; Class\$2[ method2];"                      | ["Class\$1": ["method1"].toSet(), "Class\$2": ["method2"].toSet()]
+    "Duplicate[method1] ; Duplicate[method2]  ;Duplicate[method3];" | ["Duplicate": ["method3"].toSet()]
+  }
+
+}