From bcb08ba82b842acf3020cf56fc3adeb5ed81d694 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Fri, 28 Dec 2018 11:03:28 -0800 Subject: [PATCH 1/3] Remove unneeded dependency and use Charset instead of String Avoids the need to catch the exception for an unknown charset. --- pom.xml | 7 +--- src/main/java/org/datadog/jmxfetch/App.java | 45 +++++++++------------ 2 files changed, 19 insertions(+), 33 deletions(-) diff --git a/pom.xml b/pom.xml index 62b82bdef..72fb55420 100644 --- a/pom.xml +++ b/pom.xml @@ -32,6 +32,7 @@ + 1.6 1.6 2.4 @@ -92,12 +93,6 @@ commons-lang ${commons-lang.version} - - - org.apache.commons - commons-lang3 - ${apache-commons-lang3.version} - com.datadoghq java-dogstatsd-client diff --git a/src/main/java/org/datadog/jmxfetch/App.java b/src/main/java/org/datadog/jmxfetch/App.java index 60bc5d292..3d8ced344 100644 --- a/src/main/java/org/datadog/jmxfetch/App.java +++ b/src/main/java/org/datadog/jmxfetch/App.java @@ -6,7 +6,7 @@ import java.io.FileNotFoundException; import java.io.InputStream; import java.io.IOException; -import java.io.UnsupportedEncodingException; +import java.nio.charset.Charset; import java.util.Arrays; import java.util.ArrayList; import java.util.Enumeration; @@ -27,7 +27,6 @@ import org.apache.log4j.Appender; import org.apache.log4j.Level; import org.apache.log4j.Logger; -import org.apache.commons.lang3.CharEncoding; import org.apache.commons.io.IOUtils; import org.datadog.jmxfetch.reporter.Reporter; import org.datadog.jmxfetch.util.CustomLogger; @@ -51,6 +50,7 @@ public class App { private static final String AD_LEGACY_CONFIG_TERM = "#### SERVICE-DISCOVERY TERM ####"; private static final int AD_MAX_NAME_LEN = 80; private static final int AD_MAX_MAG_INSTANCES = 4; // 1000 instances ought to be enough for anyone + private static final Charset UTF_8 = Charset.forName("UTF-8"); private static int loopCounter; private int lastJSONConfigTS; @@ -219,38 +219,29 @@ public boolean processAutoDiscovery(byte[] buffer) { boolean reinit = false; String[] discovered; - try { - String configs = new String(buffer, CharEncoding.UTF_8); - String separator = App.AD_CONFIG_SEP; + String configs = new String(buffer, UTF_8); + String separator = App.AD_CONFIG_SEP; - if (configs.indexOf(App.AD_LEGACY_CONFIG_SEP) != -1) { - separator = App.AD_LEGACY_CONFIG_SEP; - } - discovered = configs.split(separator + System.getProperty("line.separator")); - } catch(UnsupportedEncodingException e) { - LOGGER.debug("Unable to parse byte buffer to UTF-8 String."); - return false; + if (configs.indexOf(App.AD_LEGACY_CONFIG_SEP) != -1) { + separator = App.AD_LEGACY_CONFIG_SEP; } + discovered = configs.split(separator + System.getProperty("line.separator")); for (String config : discovered) { if (config == null || config.isEmpty()) { continue; } - try{ - String name = getAutoDiscoveryName(config); - LOGGER.debug("Attempting to apply config. Name: " + name + "\nconfig: \n" + config); - InputStream stream = new ByteArrayInputStream(config.getBytes(CharEncoding.UTF_8)); - YamlParser yaml = new YamlParser(stream); - - if (this.addConfig(name, yaml)){ - reinit = true; - LOGGER.debug("Configuration added succesfully reinit in order"); - } else { - LOGGER.debug("Unable to apply configuration."); - } - } catch(UnsupportedEncodingException e) { - LOGGER.debug("Unable to parse byte buffer to UTF-8 String."); + String name = getAutoDiscoveryName(config); + LOGGER.debug("Attempting to apply config. Name: " + name + "\nconfig: \n" + config); + InputStream stream = new ByteArrayInputStream(config.getBytes(UTF_8)); + YamlParser yaml = new YamlParser(stream); + + if (this.addConfig(name, yaml)){ + reinit = true; + LOGGER.debug("Configuration added succesfully reinit in order"); + } else { + LOGGER.debug("Unable to apply configuration."); } } @@ -587,7 +578,7 @@ private boolean getJSONConfigs() { LOGGER.debug("Received the following JSON configs: " + response.getResponseBody()); - InputStream jsonInputStream = IOUtils.toInputStream(response.getResponseBody(), "UTF-8"); + InputStream jsonInputStream = IOUtils.toInputStream(response.getResponseBody(), UTF_8); JsonParser parser = new JsonParser(jsonInputStream); int timestamp = ((Integer) parser.getJsonTimestamp()).intValue(); if (timestamp > lastJSONConfigTS) { From 2125937b91f6d75bd980352eca31d641f3989b73 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Fri, 21 Dec 2018 16:38:41 -0800 Subject: [PATCH 2/3] Allow metrics configs to be loaded via resources MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Metrics configs embedded inside the agent aren’t directly accessible on the file system. Loading as a resource avoids having to copy to an external location. --- .../java/org/datadog/jmxfetch/AppConfig.java | 8 ++++ .../java/org/datadog/jmxfetch/Instance.java | 47 ++++++++++++++++++- .../jmxfetch/TestLogInitialization.java | 2 + 3 files changed, 55 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/datadog/jmxfetch/AppConfig.java b/src/main/java/org/datadog/jmxfetch/AppConfig.java index 3e3425040..fb2cc02b0 100644 --- a/src/main/java/org/datadog/jmxfetch/AppConfig.java +++ b/src/main/java/org/datadog/jmxfetch/AppConfig.java @@ -121,6 +121,8 @@ public class AppConfig { // 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 + private List metricConfigResources; // This is used by things like APM agent to provide metric configuration from files private List metricConfigFiles; // This is used by things like APM agent to provide global override for bean refresh period @@ -229,6 +231,10 @@ public List getInstanceConfigResources() { return instanceConfigResources; } + public List getMetricConfigResources() { + return metricConfigResources; + } + public List getMetricConfigFiles() { return metricConfigFiles; } @@ -246,6 +252,7 @@ public Map getGlobalTags() { */ public static AppConfig create( List instanceConfigResources, + List metricConfigResources, List metricConfigFiles, Integer checkPeriod, Integer refreshBeansPeriod, @@ -256,6 +263,7 @@ public static AppConfig create( AppConfig config = new AppConfig(); config.action = ImmutableList.of(ACTION_COLLECT); config.instanceConfigResources = ImmutableList.copyOf(instanceConfigResources); + config.metricConfigResources = ImmutableList.copyOf(metricConfigResources); config.metricConfigFiles = ImmutableList.copyOf(metricConfigFiles); if (checkPeriod != null) { config.checkPeriod = checkPeriod; diff --git a/src/main/java/org/datadog/jmxfetch/Instance.java b/src/main/java/org/datadog/jmxfetch/Instance.java index ce12c67d6..6fb596a6a 100644 --- a/src/main/java/org/datadog/jmxfetch/Instance.java +++ b/src/main/java/org/datadog/jmxfetch/Instance.java @@ -4,6 +4,7 @@ import java.io.FileInputStream; import java.io.FileNotFoundException; import java.io.IOException; +import java.io.InputStream; import java.lang.ClassCastException; import java.util.ArrayList; import java.util.Arrays; @@ -38,6 +39,13 @@ public class Instance { public static final String PROCESS_NAME_REGEX = "process_name_regex"; public static final String ATTRIBUTE = "Attribute: "; + private static final ThreadLocal YAML = new ThreadLocal() { + @Override + public Yaml initialValue() { + return new Yaml(); + } + }; + private Set beans; private LinkedList beanScopes; private LinkedList configurationList = new LinkedList(); @@ -149,6 +157,7 @@ public Instance(LinkedHashMap instanceMap, LinkedHashMap instanceMap, LinkedHashMap> defaultConf = (ArrayList>) new Yaml().load(this.getClass().getResourceAsStream(configResourcePath)); + ArrayList> defaultConf = (ArrayList>) YAML.get().load(this.getClass().getResourceAsStream(configResourcePath)); for (LinkedHashMap conf : defaultConf) { configurationList.add(new Configuration(conf)); } @@ -178,7 +187,7 @@ private void loadMetricConfigFiles(AppConfig appConfig, LinkedList> confs = (ArrayList>) new Yaml().load(yamlInputStream); + ArrayList> confs = (ArrayList>) YAML.get().load(yamlInputStream); for (LinkedHashMap conf : confs) { configurationList.add(new Configuration(conf)); } @@ -199,6 +208,40 @@ private void loadMetricConfigFiles(AppConfig appConfig, LinkedList configurationList) { + List resourceConfigList = config.getMetricConfigResources(); + if (resourceConfigList != null) { + ClassLoader classLoader = Thread.currentThread().getContextClassLoader(); + for (String resourceName : resourceConfigList) { + LOGGER.info("Reading metric config resource " + resourceName); + InputStream inputStream = classLoader.getResourceAsStream(resourceName); + if (inputStream == null) { + LOGGER.warn("Cannot find metric config resource" + resourceName); + } else { + try { + LinkedHashMap>> topYaml = (LinkedHashMap>>) YAML.get().load(inputStream); + ArrayList> jmxConf = topYaml.get("jmx_metrics"); + if(jmxConf != null) { + for (LinkedHashMap conf : jmxConf) { + configurationList.add(new Configuration(conf)); + } + } else { + LOGGER.warn("jmx_metrics block not found in resource " + resourceName); + } + } catch (Exception e) { + LOGGER.warn("Cannot parse yaml resource " + resourceName, e); + } finally { + try { + inputStream.close(); + } catch (IOException e) { + // ignore + } + } + } + } + } + } + /** * Format the instance tags defined in the YAML configuration file to a `LinkedHashMap`. * Supported inputs: `List`, `Map`. diff --git a/src/test/java/org/datadog/jmxfetch/TestLogInitialization.java b/src/test/java/org/datadog/jmxfetch/TestLogInitialization.java index 3091187d9..f393dde51 100644 --- a/src/test/java/org/datadog/jmxfetch/TestLogInitialization.java +++ b/src/test/java/org/datadog/jmxfetch/TestLogInitialization.java @@ -37,6 +37,7 @@ public AppConfig call() throws Exception { return AppConfig.create( ImmutableList.of("org/datadog/jmxfetch/dd-java-agent-jmx.yaml"), Collections.emptyList(), + Collections.emptyList(), (int) TimeUnit.SECONDS.toMillis(30), (int) TimeUnit.SECONDS.toMillis(30), Collections.emptyMap(), @@ -51,6 +52,7 @@ public AppConfig call() throws Exception { return AppConfig.create( ImmutableList.of("org/datadog/jmxfetch/remote-jmx.yaml"), Collections.emptyList(), + Collections.emptyList(), (int) TimeUnit.SECONDS.toMillis(30), (int) TimeUnit.SECONDS.toMillis(30), Collections.emptyMap(), From 0f43dc7d91a9b91c2eece646a9b349a6046f05c7 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Thu, 3 Jan 2019 13:18:53 -0800 Subject: [PATCH 3/3] Add some tests for loading metric configs. --- .../java/org/datadog/jmxfetch/Instance.java | 7 ++- .../org/datadog/jmxfetch/TestInstance.java | 27 ++++++++++++ .../org/datadog/jmxfetch/sample-metrics.yaml | 43 +++++++++++++++++++ 3 files changed, 75 insertions(+), 2 deletions(-) create mode 100644 src/test/resources/org/datadog/jmxfetch/sample-metrics.yaml diff --git a/src/main/java/org/datadog/jmxfetch/Instance.java b/src/main/java/org/datadog/jmxfetch/Instance.java index 6fb596a6a..f292684de 100644 --- a/src/main/java/org/datadog/jmxfetch/Instance.java +++ b/src/main/java/org/datadog/jmxfetch/Instance.java @@ -1,5 +1,6 @@ package org.datadog.jmxfetch; +import com.google.common.annotations.VisibleForTesting; import java.io.File; import java.io.FileInputStream; import java.io.FileNotFoundException; @@ -179,7 +180,8 @@ private void loadDefaultConfig(String configResourcePath) { } } - private void loadMetricConfigFiles(AppConfig appConfig, LinkedList configurationList) { + @VisibleForTesting + static void loadMetricConfigFiles(AppConfig appConfig, LinkedList configurationList) { if (appConfig.getMetricConfigFiles() != null) { for (String fileName : appConfig.getMetricConfigFiles()) { String yamlPath = new File(fileName).getAbsolutePath(); @@ -208,7 +210,8 @@ private void loadMetricConfigFiles(AppConfig appConfig, LinkedList configurationList) { + @VisibleForTesting + static void loadMetricConfigResources(AppConfig config, LinkedList configurationList) { List resourceConfigList = config.getMetricConfigResources(); if (resourceConfigList != null) { ClassLoader classLoader = Thread.currentThread().getContextClassLoader(); diff --git a/src/test/java/org/datadog/jmxfetch/TestInstance.java b/src/test/java/org/datadog/jmxfetch/TestInstance.java index 0fe3dd244..fdffa2930 100644 --- a/src/test/java/org/datadog/jmxfetch/TestInstance.java +++ b/src/test/java/org/datadog/jmxfetch/TestInstance.java @@ -4,7 +4,11 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertFalse; import static org.junit.Assert.fail; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import com.google.common.collect.Lists; +import java.net.URL; import java.util.HashMap; import java.util.LinkedList; import java.util.Arrays; @@ -72,4 +76,27 @@ public void testEmptyDefaultHostname() throws Exception { this.assertHostnameTags(Arrays.asList(tags)); } } + + @Test + public void testLoadMetricConfigFiles() throws Exception { + URL defaultConfig = Instance.class.getResource("default-jmx-metrics.yaml"); + AppConfig config = mock(AppConfig.class); + when(config.getMetricConfigFiles()).thenReturn(Lists.newArrayList(defaultConfig.getPath())); + LinkedList configurationList = new LinkedList(); + Instance.loadMetricConfigFiles(config, configurationList); + + assertEquals(2, configurationList.size()); + } + + @Test + public void testLoadMetricConfigResources() throws Exception { + URL defaultConfig = Instance.class.getResource("sample-metrics.yaml"); + String configResource = defaultConfig.getPath().split("test-classes/")[1]; + AppConfig config = mock(AppConfig.class); + when(config.getMetricConfigResources()).thenReturn(Lists.newArrayList(configResource)); + LinkedList configurationList = new LinkedList(); + Instance.loadMetricConfigResources(config, configurationList); + + assertEquals(2, configurationList.size()); + } } diff --git a/src/test/resources/org/datadog/jmxfetch/sample-metrics.yaml b/src/test/resources/org/datadog/jmxfetch/sample-metrics.yaml new file mode 100644 index 000000000..f5905581f --- /dev/null +++ b/src/test/resources/org/datadog/jmxfetch/sample-metrics.yaml @@ -0,0 +1,43 @@ +# Same as default-jmx-metrics.yaml but with the top level jmx_metrics to match standard metrics.yaml file structure. + +jmx_metrics: + + # Memory + - include: + domain: java.lang + type: Memory + attribute: + HeapMemoryUsage.used: + alias: jvm.heap_memory + metric_type: gauge + HeapMemoryUsage.committed: + alias: jvm.heap_memory_committed + metric_type: gauge + HeapMemoryUsage.init: + alias: jvm.heap_memory_init + metric_type: gauge + HeapMemoryUsage.max: + alias: jvm.heap_memory_max + metric_type: gauge + NonHeapMemoryUsage.used: + alias: jvm.non_heap_memory + metric_type: gauge + NonHeapMemoryUsage.committed: + alias: jvm.non_heap_memory_committed + metric_type: gauge + NonHeapMemoryUsage.init: + alias: jvm.non_heap_memory_init + metric_type: gauge + NonHeapMemoryUsage.max: + alias: jvm.non_heap_memory_max + metric_type: gauge + + # Threads + - include: + domain: java.lang + type: Threading + attribute: + ThreadCount: + alias: jvm.thread_count + metric_type: gauge +