Merge pull request #42 from DataDog/yann/add-jvm-metrics-and-fix-leak

Fix variables leaks and fetch more JVM (Non)Heap variables by default

CHANGELOG.md

@@ -1,19 +1,24 @@
 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
 * [BUGFIX] Support WARN log level, See [#14][]
 + [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)
 
 <!--- The following link definition list is generated by PimpMyChangelog --->
 [#14]: https://github.com/DataDog/jmxfetch/issues/14
 [#19]: https://github.com/DataDog/jmxfetch/issues/19
 [#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

pom.xml

@@ -3,7 +3,7 @@
 
 	<groupId>datadog</groupId>
 	<artifactId>jmxfetch</artifactId>
-	<version>0.4.0</version>
+	<version>0.4.1</version>
 	<packaging>jar</packaging>
 
 	<name>jmxfetch</name>

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<ObjectInstance> beans;
     private LinkedList<Configuration> configurationList = new LinkedList<Configuration>();
     private LinkedList<JMXAttribute> matchingAttributes;
-    private LinkedList<JMXAttribute> failingAttributes;
+    private HashSet<JMXAttribute> failingAttributes;
     private Integer refreshBeansPeriod;
     private long lastRefreshTime;
     private LinkedHashMap<String, Object> yaml;
@@ -65,11 +66,12 @@ public Instance(LinkedHashMap<String, Object> yamlInstance, LinkedHashMap<String
         this.instanceName = (String) yaml.get("name");
         this.tags = (LinkedHashMap<String, String>) yaml.get("tags");
         this.checkName = checkName;
-        this.failingAttributes = new LinkedList<JMXAttribute>();
+        this.matchingAttributes = new LinkedList<JMXAttribute>();
+        this.failingAttributes = new HashSet<JMXAttribute>();
         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<JMXAttribute>();
+        this.matchingAttributes.clear();
+        this.failingAttributes.clear();
         int metricsCount = 0;
 
         if (!action.equals(AppConfig.ACTION_COLLECT)) {

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<String, Object>());
+        instanceStats.put(FAILED_CHECKS, new HashMap<String, Object>());
     }
 
     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() {

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

src/test/java/org/datadog/jmxfetch/TestApp.java

@@ -44,7 +44,7 @@ public void testApp() throws Exception {
         app.doIteration();
         LinkedList<HashMap<String, Object>> 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<String, Object> m : metrics) {
@@ -342,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);
@@ -368,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);