diff --git a/agent-bootstrap/src/main/java/io/opentelemetry/auto/config/Config.java b/agent-bootstrap/src/main/java/io/opentelemetry/auto/config/Config.java index d7386ae2ca..f9ec93ef2b 100644 --- a/agent-bootstrap/src/main/java/io/opentelemetry/auto/config/Config.java +++ b/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 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); diff --git a/instrumentation/trace-annotation/src/main/java/io/opentelemetry/auto/instrumentation/traceannotation/MethodsConfigurationParser.java b/instrumentation/trace-annotation/src/main/java/io/opentelemetry/auto/instrumentation/traceannotation/MethodsConfigurationParser.java new file mode 100644 index 0000000000..9d64bec03b --- /dev/null +++ b/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. + * + *

Strings of such format are used e.g. to configure {@link Config#getTraceMethods()} and + * {@link Config#getExcludedMethods()} + */ + public static Map> 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> 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 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() {} +} diff --git a/instrumentation/trace-annotation/src/main/java/io/opentelemetry/auto/instrumentation/traceannotation/TraceAnnotationsInstrumentation.java b/instrumentation/trace-annotation/src/main/java/io/opentelemetry/auto/instrumentation/traceannotation/TraceAnnotationsInstrumentation.java index 18d91afed1..7b86e61e6e 100644 --- a/instrumentation/trace-annotation/src/main/java/io/opentelemetry/auto/instrumentation/traceannotation/TraceAnnotationsInstrumentation.java +++ b/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 additionalTraceAnnotations; - private final ElementMatcher.Junction methodTraceMatcher; + private final ElementMatcher.Junction annotationMatcher; + /* + This matcher matches all methods that should be excluded from transformation + */ + private final ElementMatcher.Junction 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 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 configureExcludedMethods() { + ElementMatcher.Junction result = none(); + + Map> excludedMethods = + MethodsConfigurationParser.parse(Config.get().getTraceMethodsExclude()); + for (Map.Entry> entry : excludedMethods.entrySet()) { + String className = entry.getKey(); + ElementMatcher.Junction classMather = + isDeclaredBy(ElementMatchers.named(className)); + + ElementMatcher.Junction 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 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, String> transformers() { - return singletonMap(isAnnotatedWith(methodTraceMatcher), packageName + ".TraceAdvice"); + return singletonMap( + isAnnotatedWith(annotationMatcher).and(not(excludedMethodsMatcher)), + packageName + ".TraceAdvice"); } } diff --git a/instrumentation/trace-annotation/src/main/java/io/opentelemetry/auto/instrumentation/traceannotation/TraceConfigInstrumentation.java b/instrumentation/trace-annotation/src/main/java/io/opentelemetry/auto/instrumentation/traceannotation/TraceConfigInstrumentation.java index e2680b9133..b82cdd039b 100644 --- a/instrumentation/trace-annotation/src/main/java/io/opentelemetry/auto/instrumentation/traceannotation/TraceConfigInstrumentation.java +++ b/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> 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(); - if (configString == null || configString.trim().isEmpty()) { - classMethodsToTrace = Collections.emptyMap(); + classMethodsToTrace = MethodsConfigurationParser.parse(Config.get().getTraceMethods()); - } else if (!validateConfigString(configString)) { - log.warn( - "Invalid trace method config '{}'. Must match 'package.Class$Name[method1,method2];*'.", - configString); - classMethodsToTrace = Collections.emptyMap(); - - } else { - final Map> 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 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); - } + Map> excludedMethods = + MethodsConfigurationParser.parse(Config.get().getTraceMethodsExclude()); + for (Map.Entry> entry : excludedMethods.entrySet()) { + Set tracedMethods = classMethodsToTrace.get(entry.getKey()); + if (tracedMethods != null) { + tracedMethods.removeAll(entry.getValue()); } - classMethodsToTrace = Collections.unmodifiableMap(toTrace); } } diff --git a/instrumentation/trace-annotation/src/test/groovy/TraceConfigTest.groovy b/instrumentation/trace-annotation/src/test/groovy/TraceConfigTest.groovy index 6ad5f498df..e5ad8ba5f2 100644 --- a/instrumentation/trace-annotation/src/test/groovy/TraceConfigTest.groovy +++ b/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()] - } } diff --git a/instrumentation/trace-annotation/src/test/groovy/TraceProvidersTest.groovy b/instrumentation/trace-annotation/src/test/groovy/TraceProvidersTest.groovy index d63b3b5415..0fccc16aca 100644 --- a/instrumentation/trace-annotation/src/test/groovy/TraceProvidersTest.groovy +++ b/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 + "*") } } diff --git a/instrumentation/trace-annotation/src/test/groovy/TracedMethodsExclusionTest.groovy b/instrumentation/trace-annotation/src/test/groovy/TracedMethodsExclusionTest.groovy new file mode 100644 index 0000000000..e8aeac597a --- /dev/null +++ b/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) {} + } +} diff --git a/instrumentation/trace-annotation/src/test/groovy/io/opentelemetry/auto/instrumentation/traceannotation/MethodsConfigurationParserTest.groovy b/instrumentation/trace-annotation/src/test/groovy/io/opentelemetry/auto/instrumentation/traceannotation/MethodsConfigurationParserTest.groovy new file mode 100644 index 0000000000..53421960db --- /dev/null +++ b/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()] + } + +}