diff --git a/dd-java-agent/agent-jmxfetch/src/main/java/datadog/trace/agent/jmxfetch/JMXFetch.java b/dd-java-agent/agent-jmxfetch/src/main/java/datadog/trace/agent/jmxfetch/JMXFetch.java index 83d28550f3..95ca378a5f 100644 --- a/dd-java-agent/agent-jmxfetch/src/main/java/datadog/trace/agent/jmxfetch/JMXFetch.java +++ b/dd-java-agent/agent-jmxfetch/src/main/java/datadog/trace/agent/jmxfetch/JMXFetch.java @@ -117,8 +117,11 @@ public class JMXFetch { final String[] split = configs.split("\n"); final List result = new ArrayList<>(split.length); for (final String config : split) { - final URL resource = JMXFetch.class.getResource("metricconfigs/" + config); - result.add(resource.getPath().split("\\.jar!/")[1]); + if (Config.integrationEnabled( + Collections.singleton(config.replace(".yaml", "")), false)) { + final URL resource = JMXFetch.class.getResource("metricconfigs/" + config); + result.add(resource.getPath().split("\\.jar!/")[1]); + } } return result; } diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Instrumenter.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Instrumenter.java index 8fface4c76..a41a1eefd0 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Instrumenter.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Instrumenter.java @@ -1,13 +1,13 @@ package datadog.trace.agent.tooling; import static datadog.trace.agent.tooling.ByteBuddyElementMatchers.failSafe; -import static datadog.trace.agent.tooling.Utils.getConfigEnabled; import static net.bytebuddy.matcher.ElementMatchers.any; import datadog.trace.agent.tooling.context.FieldBackedProvider; import datadog.trace.agent.tooling.context.InstrumentationContextProvider; import datadog.trace.agent.tooling.muzzle.Reference; import datadog.trace.agent.tooling.muzzle.ReferenceMatcher; +import datadog.trace.api.Config; import java.security.ProtectionDomain; import java.util.Arrays; import java.util.Collections; @@ -52,20 +52,7 @@ public interface Instrumenter { instrumentationNames.add(instrumentationName); instrumentationPrimaryName = instrumentationName; - // If default is enabled, we want to enable individually, - // if default is disabled, we want to disable individually. - final boolean defaultEnabled = defaultEnabled(); - boolean anyEnabled = defaultEnabled; - for (final String name : instrumentationNames) { - final boolean configEnabled = - getConfigEnabled("dd.integration." + name + ".enabled", defaultEnabled); - if (defaultEnabled) { - anyEnabled &= configEnabled; - } else { - anyEnabled |= configEnabled; - } - } - enabled = anyEnabled; + enabled = Config.integrationEnabled(instrumentationNames, defaultEnabled()); contextProvider = new FieldBackedProvider(this); } @@ -225,7 +212,7 @@ public interface Instrumenter { } protected boolean defaultEnabled() { - return getConfigEnabled("dd.integrations.enabled", true); + return Config.getBooleanSettingFromEnvironment("integrations.enabled", true); } } } diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Utils.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Utils.java index 391dcde97e..ff90d35645 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Utils.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/Utils.java @@ -84,12 +84,5 @@ public class Utils { return type.getDeclaredMethods().filter(named(methodName)).getOnly(); } - static boolean getConfigEnabled(final String name, final boolean fallback) { - final String property = - System.getProperty( - name, System.getenv(name.toUpperCase().replaceAll("[^a-zA-Z0-9_]", "_"))); - return property == null ? fallback : Boolean.parseBoolean(property); - } - private Utils() {} } diff --git a/dd-java-agent/src/test/groovy/datadog/trace/agent/JMXFetchTest.groovy b/dd-java-agent/src/test/groovy/datadog/trace/agent/JMXFetchTest.groovy index feb4d219a3..e7ce2349fd 100644 --- a/dd-java-agent/src/test/groovy/datadog/trace/agent/JMXFetchTest.groovy +++ b/dd-java-agent/src/test/groovy/datadog/trace/agent/JMXFetchTest.groovy @@ -49,4 +49,36 @@ class JMXFetchTest extends Specification { Thread.currentThread().setContextClassLoader(currentContextLoader) } + def "test jmxfetch config"() { + setup: + names.each { + System.setProperty("dd.integration.${it}.enabled", "$enable") + } + def classLoader = IntegrationTestUtils.getJmxFetchClassLoader() + // Have to set this so JMXFetch knows where to find resources + Thread.currentThread().setContextClassLoader(classLoader) + final Class jmxFetchAgentClass = + classLoader.loadClass("datadog.trace.agent.jmxfetch.JMXFetch") + final Method jmxFetchInstallerMethod = jmxFetchAgentClass.getDeclaredMethod("getInternalMetricFiles") + jmxFetchInstallerMethod.setAccessible(true) + + expect: + jmxFetchInstallerMethod.invoke(null).sort() == result.sort() + + cleanup: + names.each { + System.clearProperty("dd.integration.${it}.enabled") + } + + where: + names | enable | result + [] | true | [] + ["tomcat"] | false | [] + ["tomcat"] | true | ["datadog/trace/agent/jmxfetch/metricconfigs/tomcat.yaml"] + ["kafka"] | true | ["datadog/trace/agent/jmxfetch/metricconfigs/kafka.yaml"] + ["tomcat", "kafka"] | true | ["datadog/trace/agent/jmxfetch/metricconfigs/tomcat.yaml", "datadog/trace/agent/jmxfetch/metricconfigs/kafka.yaml"] + ["tomcat", "kafka"] | false | [] + ["invalid"] | true | [] + } + } diff --git a/dd-trace-api/src/main/java/datadog/trace/api/Config.java b/dd-trace-api/src/main/java/datadog/trace/api/Config.java index 262df08d56..6da4536fb3 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/Config.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/Config.java @@ -6,7 +6,9 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Properties; +import java.util.Set; import java.util.UUID; +import java.util.regex.Pattern; import lombok.Getter; import lombok.ToString; import lombok.extern.slf4j.Slf4j; @@ -26,6 +28,8 @@ public class Config { /** Config keys below */ private static final String PREFIX = "dd."; + private static final Pattern ENV_REPLACEMENT = Pattern.compile("[^a-zA-Z0-9_]"); + private static final Config INSTANCE = new Config(); public static final String SERVICE_NAME = "service.name"; @@ -256,6 +260,23 @@ public class Config { return Collections.unmodifiableMap(result); } + public static boolean integrationEnabled( + final Set integrationNames, final boolean defaultEnabled) { + // If default is enabled, we want to enable individually, + // if default is disabled, we want to disable individually. + boolean anyEnabled = defaultEnabled; + for (final String name : integrationNames) { + final boolean configEnabled = + getBooleanSettingFromEnvironment("integration." + name + ".enabled", defaultEnabled); + if (defaultEnabled) { + anyEnabled &= configEnabled; + } else { + anyEnabled |= configEnabled; + } + } + return anyEnabled; + } + /** * Helper method that takes the name, adds a "dd." prefix then checks for System Properties of * that name. If none found, the name is converted to an Environment Variable and used to check @@ -303,7 +324,7 @@ public class Config { } private static String propertyToEnvironmentName(final String name) { - return name.toUpperCase().replace(".", "_").replace("-", "_"); + return ENV_REPLACEMENT.matcher(name.toUpperCase()).replaceAll("_"); } private static Map getPropertyMapValue( diff --git a/dd-trace-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy b/dd-trace-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy index 3b891833fa..dda81db435 100644 --- a/dd-trace-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy +++ b/dd-trace-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy @@ -281,6 +281,40 @@ class ConfigTest extends Specification { config.writerType == "DDAgentWriter" } + def "verify integration config"() { + setup: + environmentVariables.set("DD_INTEGRATION_ORDER_ENABLED", "false") + environmentVariables.set("DD_INTEGRATION_TEST_ENV_ENABLED", "true") + environmentVariables.set("DD_INTEGRATION_DISABLED_ENV_ENABLED", "false") + + System.setProperty("dd.integration.order.enabled", "true") + System.setProperty("dd.integration.test-prop.enabled", "true") + System.setProperty("dd.integration.disabled-prop.enabled", "false") + + expect: + Config.integrationEnabled(integrationNames, defaultEnabled) == expected + + where: + names | defaultEnabled | expected + [] | true | true + [] | false | false + ["invalid"] | true | true + ["invalid"] | false | false + ["test-prop"] | false | true + ["test-env"] | false | true + ["disabled-prop"] | true | false + ["disabled-env"] | true | false + ["other", "test-prop"] | false | true + ["other", "test-env"] | false | true + ["order"] | false | true + ["test-prop", "disabled-prop"] | false | true + ["disabled-env", "test-env"] | false | true + ["test-prop", "disabled-prop"] | true | false + ["disabled-env", "test-env"] | true | false + + integrationNames = names.toSet() + } + def "verify mapping configs on tracer"() { setup: System.setProperty(PREFIX + SERVICE_MAPPING, mapString)