Rename trace.methods.exclude config (#842)

2020-07-31 12:30:08 -07:00 · 2020-07-31 12:30:08 -07:00 · 2af43dbf26
parent d8ba6e3d51
commit 2af43dbf26
8 changed files with 57 additions and 26 deletions
--- a/README.md
+++ b/README.md
@ -233,6 +233,10 @@ instrumentation for Grizzly http server is disabled by default. If needed,
 you can enable it by add the following system property:
 `-Dota.integration.grizzly.enabled=true`

+### Suppressing specific auto-instrumentation
+
+See [Suppressing specific auto-instrumentation](docs/suppressing-instrumentation.md)
+
 ## Manually instrumenting

 > :warning: starting with 0.6.0, and prior to version 1.0.0, `opentelemetry-javaagent-all.jar`
@ -270,16 +274,14 @@ public class MyClass {
 Each time the application invokes the annotated method, it creates a span
 that denote its duration and provides any thrown exceptions.

-#### Configuration
+#### Suppressing `@WithSpan` instrumentation

-The `@WithSpan` annotation requires code changes to implement. You can
-disable the annotation at runtime via the exclude configuration or
-environment variables:
+This is useful in case you have code that is over-instrumented using `@WithSpan`,
+and you want to suppress some of them without modifying the code.

-| System property                  | Environment variable             | Purpose                                                              |
-|----------------------------------|----------------------------------|----------------------------------------------------------------------|
-| trace.classes.exclude            | TRACE_CLASSES_EXCLUDE            | Exclude classes with the `@WithSpan` annotation                      |
-| trace.methods.exclude            | TRACE_METHODS_EXCLUDE            | Exclude methods with the `@WithSpan` annotation                      |
+| System property                 | Environment variable            | Purpose                                                                                                                                  |
+|---------------------------------|---------------------------------|------------------------------------------------------------------------------------------------------------------------------------------|
+| trace.annotated.methods.exclude | TRACE_ANNOTATED_METHODS_EXCLUDE | Suppress `@WithSpan` instrumentation for specific methods, format is "my.package.MyClass1[method1,method2];my.package.MyClass2[method3]" |


 ## Troubleshooting
--- a/agent-bootstrap/src/main/java/io/opentelemetry/auto/config/Config.java
+++ b/agent-bootstrap/src/main/java/io/opentelemetry/auto/config/Config.java
@ -64,7 +64,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_ANNOTATED_METHODS_EXCLUDE = "trace.annotated.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";
@ -100,7 +100,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;
+  private static final String DEFAULT_TRACE_ANNOTATED_METHODS_EXCLUDE = null;

  public static final String SQL_NORMALIZER_ENABLED = "sql.normalizer.enabled";
  public static final boolean DEFAULT_SQL_NORMALIZER_ENABLED = true;
@ -137,7 +137,7 @@ public class Config {
  private final String traceAnnotations;

  private final String traceMethods;
-  private final String traceMethodsExclude;
+  private final String traceAnnotatedMethodsExclude;

  private final boolean traceExecutorsAll;
  private final List<String> traceExecutors;
@ -191,8 +191,9 @@ 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);
+    traceAnnotatedMethodsExclude =
+        getSettingFromEnvironment(
+            TRACE_ANNOTATED_METHODS_EXCLUDE, DEFAULT_TRACE_ANNOTATED_METHODS_EXCLUDE);

    traceExecutorsAll =
        getBooleanSettingFromEnvironment(TRACE_EXECUTORS_ALL, DEFAULT_TRACE_EXECUTORS_ALL);
@ -251,7 +252,9 @@ 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);
+    traceAnnotatedMethodsExclude =
+        properties.getProperty(
+            TRACE_ANNOTATED_METHODS_EXCLUDE, parent.traceAnnotatedMethodsExclude);

    traceExecutorsAll =
        getPropertyBooleanValue(properties, TRACE_EXECUTORS_ALL, parent.traceExecutorsAll);
@ -586,8 +589,8 @@ public class Config {
    return traceMethods;
  }

-  public String getTraceMethodsExclude() {
-    return traceMethodsExclude;
+  public String getTraceAnnotatedMethodsExclude() {
+    return traceAnnotatedMethodsExclude;
  }

  public boolean isTraceExecutorsAll() {
@ -646,8 +649,8 @@ public class Config {
        + ", traceMethods='"
        + traceMethods
        + '\''
-        + ", traceMethodsExclude='"
-        + traceMethodsExclude
+        + ", traceAnnotatedMethodsExclude='"
+        + traceAnnotatedMethodsExclude
        + '\''
        + ", traceExecutorsAll="
        + traceExecutorsAll
--- a/docs/suppressing-instrumentation.md
+++ b/docs/suppressing-instrumentation.md
@ -0,0 +1,26 @@
+## Suppressing specific auto-instrumentation
+
+You can suppress auto-instrumentation of specific libraries by using
+`-Dota.integration.[id].enabled=true`.
+
+where `id` is the instrumentation `id`:
+
+[TODO add table here with all instrumentation ids]
+
+### Even more fine-grained control
+
+You can also exclude specific classes from being instrumented.
+
+This can be useful to completely silence spans from a given class/package.
+
+Or as a quick workaround for an instrumentation bug, when byte code in one specific class is problematic.
+
+This option should not be used lightly, as it can leave some instrumentation partially applied,
+which could have unknown side-effects.
+
+If you find yourself needing to use this, it would be great if you could drop us an issue explaining why,
+so that we can try to come up with a better solution to address your need.
+
+| System property       | Environment variable  | Purpose                                                                                           |
+|-----------------------|-----------------------|---------------------------------------------------------------------------------------------------|
+| trace.classes.exclude | TRACE_CLASSES_EXCLUDE | Suppresses all instrumentation for specific classes, format is "my.package.MyClass,my.package2.*" |
--- a/instrumentation/annotations/src/main/java/io/opentelemetry/auto/instrumentation/traceannotation/AbstractTraceAnnotationInstrumentation.java
+++ b/instrumentation/annotations/src/main/java/io/opentelemetry/auto/instrumentation/traceannotation/AbstractTraceAnnotationInstrumentation.java
@ -43,7 +43,7 @@ public abstract class AbstractTraceAnnotationInstrumentation extends Instrumente
    ElementMatcher.Junction<MethodDescription> result = none();

    Map<String, Set<String>> excludedMethods =
-        MethodsConfigurationParser.parse(Config.get().getTraceMethodsExclude());
+        MethodsConfigurationParser.parse(Config.get().getTraceAnnotatedMethodsExclude());
    for (Map.Entry<String, Set<String>> entry : excludedMethods.entrySet()) {
      String className = entry.getKey();
      ElementMatcher.Junction<ByteCodeElement> classMather =
--- a/instrumentation/annotations/src/main/java/io/opentelemetry/auto/instrumentation/traceannotation/MethodsConfigurationParser.java
+++ b/instrumentation/annotations/src/main/java/io/opentelemetry/auto/instrumentation/traceannotation/MethodsConfigurationParser.java
@ -48,7 +48,7 @@ public class MethodsConfigurationParser {
   * 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#getTraceMethodsExclude()}
+   * {@link Config#getTraceAnnotatedMethodsExclude()}
   */
  public static Map<String, Set<String>> parse(String configString) {
    if (configString == null || configString.trim().isEmpty()) {
--- a/instrumentation/annotations/src/main/java/io/opentelemetry/auto/instrumentation/traceannotation/TraceConfigInstrumentation.java
+++ b/instrumentation/annotations/src/main/java/io/opentelemetry/auto/instrumentation/traceannotation/TraceConfigInstrumentation.java
@ -53,7 +53,7 @@ public class TraceConfigInstrumentation implements Instrumenter {
    classMethodsToTrace = MethodsConfigurationParser.parse(Config.get().getTraceMethods());

    Map<String, Set<String>> excludedMethods =
-        MethodsConfigurationParser.parse(Config.get().getTraceMethodsExclude());
+        MethodsConfigurationParser.parse(Config.get().getTraceAnnotatedMethodsExclude());
    for (Map.Entry<String, Set<String>> entry : excludedMethods.entrySet()) {
      Set<String> tracedMethods = classMethodsToTrace.get(entry.getKey());
      if (tracedMethods != null) {
--- a/instrumentation/annotations/src/test/groovy/TracedMethodsExclusionTest.groovy
+++ b/instrumentation/annotations/src/test/groovy/TracedMethodsExclusionTest.groovy
@ -23,14 +23,14 @@ class TracedMethodsExclusionTest extends AgentTestRunner {
  static {
    ConfigUtils.updateConfig {
      System.setProperty("otel.trace.methods", "${TestClass.name}[included,excluded]")
-      System.setProperty("otel.trace.methods.exclude", "${TestClass.name}[excluded,annotatedButExcluded]")
+      System.setProperty("otel.trace.annotated.methods.exclude", "${TestClass.name}[excluded,annotatedButExcluded]")
    }
  }

  def specCleanup() {
    ConfigUtils.updateConfig {
      System.clearProperty("otel.trace.methods")
-      System.clearProperty("otel.trace.methods.exclude")
+      System.clearProperty("otel.trace.annotated.methods.exclude")
    }
  }

--- a/instrumentation/annotations/src/test/groovy/WithSpanInstrumentationTest.groovy
+++ b/instrumentation/annotations/src/test/groovy/WithSpanInstrumentationTest.groovy
@ -27,14 +27,14 @@ class WithSpanInstrumentationTest extends AgentTestRunner {
  static {
    ConfigUtils.updateConfig {
      System.setProperty("otel.trace.classes.exclude", WithSpanInstrumentationTest.name + "*")
-      System.setProperty("otel.trace.methods.exclude", "${TracedWithSpan.name}[ignored]")
+      System.setProperty("otel.trace.annotated.methods.exclude", "${TracedWithSpan.name}[ignored]")
    }
  }

  def specCleanup() {
    ConfigUtils.updateConfig {
      System.clearProperty("otel.trace.classes.exclude")
-      System.clearProperty("otel.trace.methods.exclude")
+      System.clearProperty("otel.trace.annotated.methods.exclude")
    }
  }

@ -98,7 +98,7 @@ class WithSpanInstrumentationTest extends AgentTestRunner {
  }


-  def "should ignore method excluded by trace.methods.exclude configuration"() {
+  def "should ignore method excluded by trace.annotated.methods.exclude configuration"() {
    setup:
    new TracedWithSpan().ignored()