From e68a97a50612c68fcecb05cfe83978d614745637 Mon Sep 17 00:00:00 2001 From: Yann Mahe Date: Thu, 5 Feb 2015 16:13:46 -0500 Subject: [PATCH 1/3] Fix failingAttribute, instanceStats variables leaks + adjustments --- pom.xml | 2 +- src/main/java/org/datadog/jmxfetch/Instance.java | 11 +++++++---- src/main/java/org/datadog/jmxfetch/Status.java | 3 ++- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/pom.xml b/pom.xml index 0c67de7cd..2aa284f2f 100644 --- a/pom.xml +++ b/pom.xml @@ -3,7 +3,7 @@ datadog jmxfetch - 0.4.0 + 0.4.1 jar jmxfetch diff --git a/src/main/java/org/datadog/jmxfetch/Instance.java b/src/main/java/org/datadog/jmxfetch/Instance.java index 1c96c00d0..0ac5bb700 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.util.ArrayList; import java.util.Arrays; import java.util.HashMap; +import java.util.HashSet; import java.util.Iterator; import java.util.LinkedHashMap; import java.util.LinkedList; @@ -31,7 +32,7 @@ public class Instance { private Set beans; private LinkedList configurationList = new LinkedList(); private LinkedList matchingAttributes; - private LinkedList failingAttributes; + private HashSet failingAttributes; private Integer refreshBeansPeriod; private long lastRefreshTime; private LinkedHashMap yaml; @@ -65,11 +66,12 @@ public Instance(LinkedHashMap yamlInstance, LinkedHashMap) yaml.get("tags"); this.checkName = checkName; - this.failingAttributes = new LinkedList(); + this.matchingAttributes = new LinkedList(); + this.failingAttributes = new HashSet(); this.refreshBeansPeriod = (Integer) yaml.get("refresh_beans"); if (this.refreshBeansPeriod == null) { this.refreshBeansPeriod = DEFAULT_REFRESH_BEANS_PERIOD; // Make sure to refresh the beans list every 10 minutes - // Useful because sometimes if the application restarts, jmxfetch might read + // Useful because sometimes if the application restarts, jmxfetch might read // a jmxtree that is not completely initialized and would be missing some attributes } this.lastRefreshTime = 0; @@ -170,7 +172,8 @@ private void getMatchingAttributes() { String action = appConfig.getAction(); boolean metricReachedDisplayed = false; - this.matchingAttributes = new LinkedList(); + this.matchingAttributes.clear(); + this.failingAttributes.clear(); int metricsCount = 0; if (!action.equals(AppConfig.ACTION_COLLECT)) { diff --git a/src/main/java/org/datadog/jmxfetch/Status.java b/src/main/java/org/datadog/jmxfetch/Status.java index e6b296f67..2fb2dde6e 100644 --- a/src/main/java/org/datadog/jmxfetch/Status.java +++ b/src/main/java/org/datadog/jmxfetch/Status.java @@ -37,6 +37,7 @@ void configure(String statusFileLocation) { private void clearStats() { instanceStats.put(INITIALIZED_CHECKS, new HashMap()); + instanceStats.put(FAILED_CHECKS, new HashMap()); } public void addInstanceStats(String checkName, String instance, int metricCount, String message, String status) { @@ -92,8 +93,8 @@ public void flush() { } catch (Exception e) { LOGGER.warn("Cannot write status to temp file: " + e.getMessage()); } - this.clearStats(); } + this.clearStats(); } public String getStatusFileLocation() { From bd8915c2f1eec794f406414b678ce6110407dce1 Mon Sep 17 00:00:00 2001 From: Yann Mahe Date: Wed, 11 Feb 2015 14:05:24 -0500 Subject: [PATCH 2/3] Fetch more JVM (Non)Heap variables by default --- src/main/resources/jmx-2.yaml | 18 ++++++++++++++++++ .../java/org/datadog/jmxfetch/TestApp.java | 15 +++++++++++---- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/src/main/resources/jmx-2.yaml b/src/main/resources/jmx-2.yaml index d21b6b245..746144b28 100644 --- a/src/main/resources/jmx-2.yaml +++ b/src/main/resources/jmx-2.yaml @@ -3,9 +3,27 @@ include: 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 ThreadCount: alias: jvm.thread_count metric_type: gauge diff --git a/src/test/java/org/datadog/jmxfetch/TestApp.java b/src/test/java/org/datadog/jmxfetch/TestApp.java index ec40228a8..9d678a411 100644 --- a/src/test/java/org/datadog/jmxfetch/TestApp.java +++ b/src/test/java/org/datadog/jmxfetch/TestApp.java @@ -44,7 +44,7 @@ public void testApp() throws Exception { app.doIteration(); LinkedList> metrics = ((ConsoleReporter) appConfig.getReporter()).getMetrics(); - assertEquals(19, metrics.size()); // 19 = 7 metrics from java.lang + the 5 gauges we are explicitly collecting + the 7 gauges that is implicitly collected, see jmx.yaml in the test/resources folder + assertEquals(25, metrics.size()); // 25 = 13 metrics from java.lang + the 5 gauges we are explicitly collecting + the 7 gauges that is implicitly collected, see jmx.yaml in the test/resources folder // We test for the presence and the value of the metrics we want to collect boolean metric100Present = false; @@ -82,7 +82,7 @@ public void testApp() throws Exception { assertEquals(8, tags.length); assertEquals(new Double(100.0), value); metric100Present = true; - + assertTrue(Arrays.asList(tags).contains("foo")); assertTrue(Arrays.asList(tags).contains("gorch")); assertTrue(Arrays.asList(tags).contains("bar:baz")); @@ -159,7 +159,7 @@ public void testApp() throws Exception { // We run a second collection. The counter should now be present app.doIteration(); metrics = ((ConsoleReporter) appConfig.getReporter()).getMetrics(); - assertEquals(21, metrics.size()); // 21 = 7 metrics from java.lang + the 5 gauges we are explicitly collecting + 7 gauges implicitly collected + 2 counter, see jmx.yaml in the test/resources folder + assertEquals(27, metrics.size()); // 27 = 13 metrics from java.lang + the 5 gauges we are explicitly collecting + 7 gauges implicitly collected + 2 counter, see jmx.yaml in the test/resources folder // We test for the same metrics but this time, the counter should be here metric100Present = false; @@ -280,7 +280,7 @@ public void testApp() throws Exception { app.doIteration(); metrics = ((ConsoleReporter) appConfig.getReporter()).getMetrics(); - assertEquals(metrics.size(), 21); // 21 = 7 metrics from java.lang + the 5 gauges we are explicitly collecting + 7 gauges implicitly collected + 2 counter, see jmx.yaml in the test/resources folder + assertEquals(metrics.size(), 27); // 27 = 13 metrics from java.lang + the 5 gauges we are explicitly collecting + 7 gauges implicitly collected + 2 counter, see jmx.yaml in the test/resources folder metric100Present = false; metric1000Present = false; @@ -300,7 +300,14 @@ public void testApp() throws Exception { jvm_metrics.put("jvm.gc.cms.count", 2); jvm_metrics.put("jvm.gc.parnew.time", 2); jvm_metrics.put("jvm.heap_memory", 1); + jvm_metrics.put("jvm.heap_memory_committed", 1); + jvm_metrics.put("jvm.heap_memory_init", 1); + jvm_metrics.put("jvm.heap_memory_max", 1); jvm_metrics.put("jvm.non_heap_memory", 1); + jvm_metrics.put("jvm.non_heap_memory_committed", 1); + jvm_metrics.put("jvm.non_heap_memory_init", 1); + jvm_metrics.put("jvm.non_heap_memory_max", 1); + jvm_metrics.put("jvm.thread_count", 1); for (HashMap m : metrics) { From a15a77d4e330293ad1c65e4d8993f4a505fcc8f9 Mon Sep 17 00:00:00 2001 From: Yann Mahe Date: Wed, 11 Feb 2015 14:27:56 -0500 Subject: [PATCH 3/3] Update CHANGELOG --- CHANGELOG.md | 9 +++++++-- src/test/java/org/datadog/jmxfetch/TestApp.java | 4 ++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f84762bc..b11e86b6f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,10 @@ Changes ======= +# 0.4.1 / 02-11-2014 +* [FEATURE/BUGFIX] Fetch more JVM (Non)Heap variables by default. See[bd8915c2f1eec794f406414b678ce6110407dce1](https://github.com/DataDog/jmxfetch/commit/bd8915c2f1eec794f406414b678ce6110407dce1) +* [BUGFIX] Memory leak fix when fetching attributes value is failing. See [#30][] + # 0.3.0 / 03-25-2014 #### Changes @@ -8,7 +12,7 @@ Changes + [FEATURE] Support custom instance tags: See [#28][] (Thanks [@coupacooke][]) * [FEATURE] Support new types: Boolean, String, java.lang.Number, AtomicInteger, AtomicLong: See [#25][] [#26][] (Thanks [@coupacooke][]) * [BUGFIX] Reset statsd connection: See [#19][] -* [BUGFIX] Refresh JMX tree: See [4374b92cbf1b93d88fa5bd9d7339907e16a2da4a](https://github.com/DataDog/jmxfetch/commit/4374b92cbf1b93d88fa5bd9d7339907e16a2da4a) +* [BUGFIX] Refresh JMX tree: See [4374b92cbf1b93d88fa5bd9d7339907e16a2da4a](https://github.com/DataDog/jmxfetch/commit/4374b92cbf1b93d88fa5bd9d7339907e16a2da4a) [#14]: https://github.com/DataDog/jmxfetch/issues/14 @@ -16,4 +20,5 @@ Changes [#25]: https://github.com/DataDog/jmxfetch/issues/25 [#26]: https://github.com/DataDog/jmxfetch/issues/26 [#28]: https://github.com/DataDog/jmxfetch/issues/28 -[@coupacooke]: https://github.com/coupacooke +[#30]: https://github.com/DataDog/jmxfetch/issues/30 +[@coupacooke]: https://github.com/coupacooke \ No newline at end of file diff --git a/src/test/java/org/datadog/jmxfetch/TestApp.java b/src/test/java/org/datadog/jmxfetch/TestApp.java index 9d678a411..9cd7c4d5b 100644 --- a/src/test/java/org/datadog/jmxfetch/TestApp.java +++ b/src/test/java/org/datadog/jmxfetch/TestApp.java @@ -349,7 +349,7 @@ public void testApp() throws Exception { assertEquals(tags.length, 5); // The value should be a bit less than 1.0, as we incremented the counter by 5 and we slept for 5 seconds assertTrue(value < 1.00); - assertTrue(value > 0.99); + assertTrue(value > 0.98); counterAbsent = false; } else if (name.equals("subattr.this.is.0")) { assertEquals(tags.length, 5); @@ -375,7 +375,7 @@ public void testApp() throws Exception { assertEquals(tags.length, 5); // The value should be a bit less than 1.0, as we incremented the counter by 5 and we slept for 5 seconds assertTrue(value < 1.00); - assertTrue(value > 0.99); + assertTrue(value > 0.98); subattrCounterAbsent = false; } else if (name.equals("jmx.org.datadog.jmxfetch.test.atomic42")) { assertEquals(tags.length, 5);