Maximum length for instance configs should be longer - improve tests. (…

…#147) * [auto-discovery] accept names up to 80 characters for instances. * [auto-discovery] configuraiton name could be longer. * [test] test long instance name. * [auto-discovery] remove underscore from template + PR fixes.
DataDog · Aug 7, 2017 · c2b1a84 · c2b1a84
1 parent 3922953
commit c2b1a84
Show file tree

Hide file tree

Showing 5 changed files with 103 additions and 2 deletions.
diff --git a/src/main/java/org/datadog/jmxfetch/App.java b/src/main/java/org/datadog/jmxfetch/App.java
@@ -7,6 +7,7 @@
 import java.io.InputStream;
 import java.io.IOException;
 import java.io.UnsupportedEncodingException;
+import java.lang.Math;
 import java.util.Arrays;
 import java.util.ArrayList;
 import java.util.Enumeration;
@@ -47,6 +48,9 @@ public class App {
     private static final String AD_LEGACY_CONFIG_SEP = "#### SERVICE-DISCOVERY ####";
     private static final String AD_CONFIG_TERM = "#### AUTO-DISCOVERY TERM ####";
     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 int loopCounter;
     private AtomicBoolean reinit = new AtomicBoolean(false);
     private ConcurrentHashMap<String, YamlParser> configs;
@@ -218,6 +222,8 @@ public boolean processAutoDiscovery(byte[] buffer) {
           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.");
@@ -227,6 +233,10 @@ public boolean processAutoDiscovery(byte[] buffer) {
       return reinit;
     }
 
+    protected ArrayList<Instance> getInstances() {
+        return this.instances;
+    }
+
     void start() {
         // Main Loop that will periodically collect metrics from the JMX Server
         long start_ms = System.currentTimeMillis();
@@ -422,19 +432,31 @@ public void doIteration() {
     }
 
     public boolean addConfig(String name, YamlParser config) {
-        // named groups not supported with Java6: "(?<check>.{1,30})_(?<version>\\d{0,30})"
-        Pattern pattern = Pattern.compile(AUTO_DISCOVERY_PREFIX+"(.{1,30})_(\\d{0,30})");
+        // named groups not supported with Java6:
+        //              "AUTO_DISCOVERY_PREFIX(?<check>.{1,80})_(?<version>\\d{0,AD_MAX_MAG_INSTANCES})"
+        // + 2 cause of underscores.
+        if (name.length() > AUTO_DISCOVERY_PREFIX.length() +
+                AD_MAX_NAME_LEN + AD_MAX_MAG_INSTANCES + 2) {
+            LOGGER.debug("Name too long - skipping: " + name);
+            return false;
+        }
+        String patternText = AUTO_DISCOVERY_PREFIX+"(.{1," + AD_MAX_NAME_LEN +
+            "})_(\\d{0,"+ AD_MAX_MAG_INSTANCES +"})";
+
+        Pattern pattern = Pattern.compile(patternText);
 
         Matcher matcher = pattern.matcher(name);
         if (!matcher.find()) {
             // bad name.
+            LOGGER.debug("Cannot match instance name: " + name);
             return false;
         }
 
         // Java 6 doesn't allow name matching - group 1 is "check"
         String check = matcher.group(1);
         if (this.configs.containsKey(check)) {
             // there was already a file config for the check.
+            LOGGER.debug("Key already present - skipping: " + name);
             return false;
         }
 

diff --git a/src/test/java/org/datadog/jmxfetch/TestApp.java b/src/test/java/org/datadog/jmxfetch/TestApp.java
@@ -627,4 +627,29 @@ public void testServiceDiscovery() throws Exception {
         assertMetric("this.is.100.bar.baz", tags, 4);
         assertMetric("org.datadog.jmxfetch.test.baz.hashmap.thisis0", tags, 4);
     }
+
+    /**
+     * Test JMX Service Discovery.
+     *
+     */
+    @Test
+    public void testServiceDiscoveryLong() throws Exception {
+        // We expose a few metrics through JMX
+        SimpleTestJavaApp test = new SimpleTestJavaApp();
+        registerMBean(test, "org.datadog.jmxfetch.test:foo=Bar,qux=Baz");
+        registerMBean(test, "org.datadog.jmxfetch.test:type=SimpleTestJavaApp,scope=Co|olScope,host=localhost,component=");
+        registerMBean(test, "org.apache.cassandra.metrics:keyspace=MyKeySpace,type=ColumnFamily,scope=MyColumnFamily,name=PendingTasks");
+        initApplication("jmx_alias_match.yaml", "jmx_sd_pipe_longname.txt");
+
+        // Collecting metrics
+        run();
+        LinkedList<HashMap<String, Object>> metrics = getMetrics();
+        ArrayList<Instance> instances = getInstances();
+
+        assertEquals(31, metrics.size());
+
+        // 2(jmx_alias_match)  + 1 (jmx_sd_pipe_longname discards one)
+        assertEquals(2, instances.size());
+
+    }
 }
diff --git a/src/test/java/org/datadog/jmxfetch/TestCommon.java b/src/test/java/org/datadog/jmxfetch/TestCommon.java
@@ -139,6 +139,13 @@ protected void run(){
         }
     }
 
+    /**
+     * Return configured instances
+     */
+    protected ArrayList<Instance> getInstances() {
+        return app.getInstances();
+    }
+
     /**
      * Return JMXFetch reporter.
      */

diff --git a/src/test/resources/jmx_sd_pipe.txt b/src/test/resources/jmx_sd_pipe.txt
@@ -39,3 +39,6 @@ instances:
                     Hashmap.thisis0:
                         metric_type: gauge
                         alias: bean.parameters.should.not.match
+
+
+#### AUTO-DISCOVERY TERM ####
diff --git a/src/test/resources/jmx_sd_pipe_longname.txt b/src/test/resources/jmx_sd_pipe_longname.txt
@@ -0,0 +1,44 @@
+#### SERVICE-DISCOVERY ####
+# jmx_0
+init_config:
+
+instances:
+    -   process_name_regex: .*surefire.*
+        name: jmx_test_instance
+        conf:
+            - include:
+               bean: org.datadog.jmxfetch.test:type=SimpleTestJavaApp,scope=Co|olScope,host=localhost,component=
+               attribute:
+                    ShouldBe100:
+                        metric_type: gauge
+                        alias: this.is.100
+            - include:
+               bean: org.datadog.jmxfetch.test:type=WrongType,scope=WrongScope,host=localhost,component=
+               attribute:
+                    Hashmap.thisis0:
+                        metric_type: gauge
+                        alias: bean.parameters.should.not.match
+
+#### SERVICE-DISCOVERY ####
+# jmxbutthisisincrediblylongandshouldneverbeavalidnamebecausewhoneedsanamelikethishonestly_0
+init_config:
+
+instances:
+    -   process_name_regex: .*surefire.*
+        name: jmx_test_instance
+        conf:
+            - include:
+               bean: org.datadog.jmxfetch.test:type=SimpleTestJavaApp,scope=Co|olScope,host=localhost,component=
+               attribute:
+                    ShouldBe100:
+                        metric_type: gauge
+                        alias: this.is.100
+            - include:
+               bean: org.datadog.jmxfetch.test:type=WrongType,scope=WrongScope,host=localhost,component=
+               attribute:
+                    Hashmap.thisis0:
+                        metric_type: gauge
+                        alias: bean.parameters.should.not.match
+
+
+#### AUTO-DISCOVERY TERM ####