Replace host metadata in dogstasd payload with jmx_server tag

The host name that's sent is the host that's defined in the yaml config for each instance, which is preventing the backend from correctly assigning host tags to the service checks. We fix that by sending a `jmx_server` tag in the dogstatsd payload instead of a host_name metadata field. The agent's dogstatsd will then add the agent's hostname. Also added an `is_jmx` tag to JMXFetch's service checks and removed unused variables in `TestApp`.
DataDog · Aug 4, 2015 · d3a2a16 · d3a2a16
1 parent 49f925f
commit d3a2a16
Show file tree

Hide file tree

Showing 6 changed files with 16 additions and 27 deletions.
diff --git a/src/main/java/org/datadog/jmxfetch/App.java b/src/main/java/org/datadog/jmxfetch/App.java
@@ -290,8 +290,7 @@ private HashMap<String, YamlParser> getConfigs(AppConfig config) {
     private void reportStatus(AppConfig appConfig, Reporter reporter, Instance instance,
                               int metricCount, String message, String status) {
         String checkName = instance.getCheckName();
-        reporter.sendServiceCheck(checkName, status, message, instance.getHostname(),
-                                  instance.getServiceCheckTags());
+        reporter.sendServiceCheck(checkName, status, message, instance.getServiceCheckTags());
 
         appConfig.getStatus().addInstanceStats(checkName, instance.getName(),
                                                metricCount, reporter.getServiceCheckCount(checkName),

diff --git a/src/main/java/org/datadog/jmxfetch/Instance.java b/src/main/java/org/datadog/jmxfetch/Instance.java
@@ -290,23 +290,18 @@ private void refreshBeansList() throws IOException {
     public String[] getServiceCheckTags() {
 
         List<String> tags = new ArrayList<String>();
+        if (this.yaml.get("host") != null) {
+            tags.add("jmx_server:" + this.yaml.get("host"));
+        }
         tags.add("instance:" + this.instanceName);
+        tags.add("is_jmx");
         return tags.toArray(new String[tags.size()]);
     }
 
     public String getName() {
         return this.instanceName;
     }
 
-    public String getHostname(){
-        Object host = this.yaml.get("host");
-        if (host != null) {
-            return host.toString();
-        } else {
-            return null;
-        }
-    }
-
     LinkedHashMap<String, Object> getYaml() {
         return this.yaml;
     }

diff --git a/src/main/java/org/datadog/jmxfetch/reporter/ConsoleReporter.java b/src/main/java/org/datadog/jmxfetch/reporter/ConsoleReporter.java
@@ -34,7 +34,7 @@ public LinkedList<HashMap<String, Object>> getMetrics() {
         return returnedMetrics;
     }
 
-    public void doSendServiceCheck(String checkName, String status, String message, String hostname, String[] tags) {
+    public void doSendServiceCheck(String checkName, String status, String message, String[] tags) {
         String tagString = "";
         if (tags != null && tags.length > 0) {
             tagString = "[" + Joiner.on(",").join(tags) + "]";
@@ -45,7 +45,6 @@ public void doSendServiceCheck(String checkName, String status, String message,
         sc.put("name", checkName);
         sc.put("status", status);
         sc.put("message", message);
-        sc.put("hostname", hostname);
         sc.put("tags", tags);
         serviceChecks.add(sc);
     }

diff --git a/src/main/java/org/datadog/jmxfetch/reporter/Reporter.java b/src/main/java/org/datadog/jmxfetch/reporter/Reporter.java
@@ -109,11 +109,11 @@ void postProcess(HashMap<String, Object> metric) {
         }
     }
 
-    public void sendServiceCheck(String checkName, String status, String message, String hostname, String[] tags){
+    public void sendServiceCheck(String checkName, String status, String message, String[] tags){
         this.incrementServiceCheckCount(checkName);
         String dataName = Reporter.formatServiceCheckPrefix(checkName);
 
-        this.doSendServiceCheck(dataName, status, message, hostname, tags);
+        this.doSendServiceCheck(dataName, status, message, tags);
     }
 
     private void postProcessCassandra(HashMap<String, Object> metric) {
@@ -146,7 +146,7 @@ public static String formatServiceCheckPrefix(String fullname){
 
     protected abstract void sendMetricPoint(String metricName, double value, String[] tags);
 
-    protected abstract void doSendServiceCheck(String checkName, String status, String message, String hostname, String[] tags);
+    protected abstract void doSendServiceCheck(String checkName, String status, String message, String[] tags);
 
     public abstract void displayMetricReached();
 

diff --git a/src/main/java/org/datadog/jmxfetch/reporter/StatsdReporter.java b/src/main/java/org/datadog/jmxfetch/reporter/StatsdReporter.java
@@ -43,15 +43,14 @@ private int statusToInt(String status) {
         return 3;
     }
 
-    public void doSendServiceCheck(String checkName, String status, String message,
-                                 String hostname, String[] tags) {
+    public void doSendServiceCheck(String checkName, String status, String message, String[] tags) {
         if (System.currentTimeMillis() - this.initializationTime > 300 * 1000) {
             this.statsDClient.stop();
             init();
         }
 
         ServiceCheck sc = new ServiceCheck(String.format("%s.can_connect", checkName),
-            this.statusToInt(status), message, hostname, tags);
+            this.statusToInt(status), message, tags);
         statsDClient.serviceCheck(sc);
     }
 

diff --git a/src/test/java/org/datadog/jmxfetch/TestApp.java b/src/test/java/org/datadog/jmxfetch/TestApp.java
@@ -367,7 +367,7 @@ public void testServiceCheckOK() throws Exception {
 
         assertEquals(Reporter.formatServiceCheckPrefix("jmx"), scName);
         assertEquals(Status.STATUS_OK, scStatus);
-        assertEquals(scTags.length, 1);
+        assertEquals(scTags.length, 2);
         assertTrue(Arrays.asList(scTags).contains("instance:jmx_test_instance"));
         mbs.unregisterMBean(objectName);
     }
@@ -413,7 +413,7 @@ public void testServiceCheckWarning() throws Exception {
         assertEquals(Reporter.formatServiceCheckPrefix("too_many_metrics"), scName);
         // We should have a warning status
         assertEquals(Status.STATUS_WARNING, scStatus);
-        assertEquals(scTags.length, 1);
+        assertEquals(scTags.length, 2);
         assertTrue(Arrays.asList(scTags).contains("instance:jmx_test_instance"));
         mbs.unregisterMBean(objectName);
     }
@@ -449,7 +449,7 @@ public void testServiceCheckCRITICAL() throws Exception {
         assertEquals(Reporter.formatServiceCheckPrefix("non_running_process"), scName);
         assertEquals(Status.STATUS_ERROR, scStatus);
         assertEquals("Cannot connect to instance process_regex: .*non_running_process_test.* Cannot find JVM matching regex: .*non_running_process_test.*", scMessage);
-        assertEquals(scTags.length, 1);
+        assertEquals(scTags.length, 2);
         assertTrue(Arrays.asList(scTags).contains("instance:jmx_test_instance"));
 
 
@@ -473,17 +473,14 @@ public void testServiceCheckCRITICAL() throws Exception {
         assertEquals(Reporter.formatServiceCheckPrefix("non_running_process"), scName);
         assertEquals(Status.STATUS_ERROR, scStatus);
         assertEquals("Cannot connect to instance process_regex: .*non_running_process_test.*. Is a JMX Server running at this address?", scMessage);
-        assertEquals(scTags.length, 1);
+        assertEquals(scTags.length, 2);
         assertTrue(Arrays.asList(scTags).contains("instance:jmx_test_instance"));
 
         mbs.unregisterMBean(objectName);
     }
 
     @Test
     public void testServiceCheckCounter() throws Exception {
-        MBeanServer mbs = ManagementFactory.getPlatformMBeanServer();
-        SimpleTestJavaApp testApp = new SimpleTestJavaApp();
-
         AppConfig appConfig = new AppConfig();
         App app = initApp("jmx.yaml", appConfig);
         Reporter repo = appConfig.getReporter();
@@ -494,7 +491,7 @@ public void testServiceCheckCounter() throws Exception {
         // Let's put a service check in the pipeline (we cannot call doIteration()
         // here unfortunately because it would call reportStatus which will flush
         // the count to the jmx_status.yaml file and reset the counter.
-        repo.sendServiceCheck("jmx", Status.STATUS_OK, "This is a test", "jmx_test_instance", null);
+        repo.sendServiceCheck("jmx", Status.STATUS_OK, "This is a test", null);
 
         // Let's check that the counter has been updated
         assertEquals(1, repo.getServiceCheckCount("jmx"));