From 3c2aa4f2c4f7a75e68e4fd72076443598ec65e7f Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Thu, 16 May 2019 14:00:24 -0700 Subject: [PATCH] Better support for running JMXFetch in the JVM. More consistent file config with existing format. --- src/main/java/org/datadog/jmxfetch/App.java | 12 +++++++++++- .../java/org/datadog/jmxfetch/AppConfig.java | 11 +++++++++++ .../org/datadog/jmxfetch/ConnectionFactory.java | 17 +++++++++-------- .../java/org/datadog/jmxfetch/Instance.java | 8 ++++++++ ...Connection.java => JvmDirectConnection.java} | 4 ++-- src/test/java/org/datadog/jmxfetch/TestApp.java | 2 ++ .../org/datadog/jmxfetch/TestServiceChecks.java | 3 +++ src/test/resources/jmx.yaml | 2 +- 8 files changed, 47 insertions(+), 12 deletions(-) rename src/main/java/org/datadog/jmxfetch/{LocalConnection.java => JvmDirectConnection.java} (79%) diff --git a/src/main/java/org/datadog/jmxfetch/App.java b/src/main/java/org/datadog/jmxfetch/App.java index 359541f03..e4a0f35d7 100644 --- a/src/main/java/org/datadog/jmxfetch/App.java +++ b/src/main/java/org/datadog/jmxfetch/App.java @@ -1,5 +1,7 @@ package org.datadog.jmxfetch; +import static org.datadog.jmxfetch.Instance.isDirectInstance; + import com.google.common.primitives.Bytes; import com.beust.jcommander.JCommander; @@ -827,8 +829,16 @@ public void init(boolean forceNewConnection) { } for (LinkedHashMap configInstance : configInstances) { + if (appConfig.isAllowDirectInstances() != isDirectInstance(configInstance)) { + log.debug("Skipping instance '{}'. allowDirectInstances={} jvm_direct={}", + name, + appConfig.isAllowDirectInstances(), + isDirectInstance(configInstance)); + continue; + } + // Create a new Instance object - log.info("Instantiating instance for: " + name); + log.info("Instantiating instance for: {}", name); Instance instance = instantiate( configInstance, diff --git a/src/main/java/org/datadog/jmxfetch/AppConfig.java b/src/main/java/org/datadog/jmxfetch/AppConfig.java index 14727b0c0..f69f3e6b8 100644 --- a/src/main/java/org/datadog/jmxfetch/AppConfig.java +++ b/src/main/java/org/datadog/jmxfetch/AppConfig.java @@ -204,6 +204,13 @@ public class AppConfig { @Builder.Default private int ipcPort = 0; + /** + * Boolean setting to determine whether to ignore jvm_direct instances. + * If set to true, other instances will be ignored. + */ + @Builder.Default + private boolean allowDirectInstances = false; + // This is used by things like APM agent to provide configuration from resources private List instanceConfigResources; // This is used by things like APM agent to provide metric configuration from resources @@ -339,6 +346,10 @@ public String getJmxLaunchFile() { return getTmpDirectory() + "/" + AD_LAUNCH_FILE; } + public boolean isAllowDirectInstances() { + return allowDirectInstances; + } + public List getInstanceConfigResources() { return instanceConfigResources; } diff --git a/src/main/java/org/datadog/jmxfetch/ConnectionFactory.java b/src/main/java/org/datadog/jmxfetch/ConnectionFactory.java index 00bd3545f..705cec6b4 100644 --- a/src/main/java/org/datadog/jmxfetch/ConnectionFactory.java +++ b/src/main/java/org/datadog/jmxfetch/ConnectionFactory.java @@ -1,5 +1,7 @@ package org.datadog.jmxfetch; +import static org.datadog.jmxfetch.Instance.isDirectInstance; + import lombok.extern.slf4j.Slf4j; import java.io.IOException; @@ -9,11 +11,17 @@ @Slf4j public class ConnectionFactory { public static final String PROCESS_NAME_REGEX = "process_name_regex"; - private static ConnectionFactory connectionFactory = null; /** Factory method to create connections, both remote and local to the target JVM. */ public static Connection createConnection(LinkedHashMap connectionParams) throws IOException { + // This is used by dd-java-agent to enable directly connecting to the mbean server. + // This works since jmxfetch is being run as a library inside the process being monitored. + if (isDirectInstance(connectionParams)) { + log.info("Connecting to JMX directly on the JVM"); + return new JvmDirectConnection(); + } + if (connectionParams.get(PROCESS_NAME_REGEX) != null) { try { Class.forName("com.sun.tools.attach.AttachNotSupportedException"); @@ -26,13 +34,6 @@ public static Connection createConnection(LinkedHashMap connecti return new AttachApiConnection(connectionParams); } - // This is used by dd-java-agent to enable directly connecting to the mbean server. - // This works because jmxfetch is being run as a library inside the process. - if ("service:jmx:local:///".equals(connectionParams.get("jmx_url"))) { - log.info("Connecting using JMX Local"); - return new LocalConnection(); - } - log.info("Connecting using JMX Remote"); return new RemoteConnection(connectionParams); } diff --git a/src/main/java/org/datadog/jmxfetch/Instance.java b/src/main/java/org/datadog/jmxfetch/Instance.java index 3cb76943f..3ee9a2a2b 100644 --- a/src/main/java/org/datadog/jmxfetch/Instance.java +++ b/src/main/java/org/datadog/jmxfetch/Instance.java @@ -56,6 +56,7 @@ public class Instance { private static final int MAX_RETURNED_METRICS = 350; private static final int DEFAULT_REFRESH_BEANS_PERIOD = 600; public static final String PROCESS_NAME_REGEX = "process_name_regex"; + public static final String JVM_DIRECT = "jvm_direct"; public static final String ATTRIBUTE = "Attribute: "; private static final ThreadLocal YAML = @@ -209,6 +210,11 @@ public Instance( loadDefaultConfig(gcMetricConfig); } + public static boolean isDirectInstance(LinkedHashMap configInstance) { + Object directInstance = configInstance.get(JVM_DIRECT); + return directInstance instanceof Boolean && (Boolean) directInstance; + } + private void loadDefaultConfig(String configResourcePath) { ArrayList> defaultConf = (ArrayList>) @@ -222,6 +228,8 @@ private void loadDefaultConfig(String configResourcePath) { static void loadMetricConfigFiles( AppConfig appConfig, LinkedList configurationList) { if (appConfig.getMetricConfigFiles() != null) { + log.warn("Loading files via metricConfigFiles setting is deprecated. Please " + + "migrate to using standard agent config files in the conf.d directory."); for (String fileName : appConfig.getMetricConfigFiles()) { String yamlPath = new File(fileName).getAbsolutePath(); FileInputStream yamlInputStream = null; diff --git a/src/main/java/org/datadog/jmxfetch/LocalConnection.java b/src/main/java/org/datadog/jmxfetch/JvmDirectConnection.java similarity index 79% rename from src/main/java/org/datadog/jmxfetch/LocalConnection.java rename to src/main/java/org/datadog/jmxfetch/JvmDirectConnection.java index 841f042ce..08c85c770 100644 --- a/src/main/java/org/datadog/jmxfetch/LocalConnection.java +++ b/src/main/java/org/datadog/jmxfetch/JvmDirectConnection.java @@ -6,9 +6,9 @@ import java.lang.management.ManagementFactory; @Slf4j -public class LocalConnection extends Connection { +public class JvmDirectConnection extends Connection { - public LocalConnection() throws IOException { + public JvmDirectConnection() throws IOException { createConnection(); } diff --git a/src/test/java/org/datadog/jmxfetch/TestApp.java b/src/test/java/org/datadog/jmxfetch/TestApp.java index 19df09c4f..438b94f7e 100644 --- a/src/test/java/org/datadog/jmxfetch/TestApp.java +++ b/src/test/java/org/datadog/jmxfetch/TestApp.java @@ -3,6 +3,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.when; import java.io.File; import java.util.ArrayList; @@ -490,6 +491,7 @@ public void testApp() throws Exception { registerMBean(testApp, "org.datadog.jmxfetch.test:type=SimpleTestJavaApp"); // We do a first collection + when(appConfig.isAllowDirectInstances()).thenReturn(true); initApplication("jmx.yaml"); run(); diff --git a/src/test/java/org/datadog/jmxfetch/TestServiceChecks.java b/src/test/java/org/datadog/jmxfetch/TestServiceChecks.java index c2e2cfc8b..4838c022d 100644 --- a/src/test/java/org/datadog/jmxfetch/TestServiceChecks.java +++ b/src/test/java/org/datadog/jmxfetch/TestServiceChecks.java @@ -4,6 +4,7 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.when; import java.util.Arrays; import java.util.HashMap; @@ -19,6 +20,7 @@ public void testServiceCheckOK() throws Exception { registerMBean(new SimpleTestJavaApp(), "org.datadog.jmxfetch.test:type=ServiceCheckTest"); // We do a first collection + when(appConfig.isAllowDirectInstances()).thenReturn(true); initApplication("jmx.yaml"); run(); @@ -153,6 +155,7 @@ public void testServiceCheckCRITICAL() throws Exception { @Test public void testServiceCheckCounter() throws Exception { + when(appConfig.isAllowDirectInstances()).thenReturn(true); initApplication("jmx.yaml"); Reporter repo = getReporter(); diff --git a/src/test/resources/jmx.yaml b/src/test/resources/jmx.yaml index b83be4896..60c1db9f9 100644 --- a/src/test/resources/jmx.yaml +++ b/src/test/resources/jmx.yaml @@ -1,7 +1,7 @@ init_config: instances: - - process_name_regex: .*surefire.* + - jvm_direct: true refresh_beans: 4 name: jmx_test_instance tags: