Fix service checks in the JSON reporter

This uses the wrong service check name and doesn't produce the proper value compared to the statsd reporter.
DataDog · Mar 12, 2020 · 37fc55f · 37fc55f
2 parents 3485cb3 + 3223db1
commit 37fc55f
Show file tree

Hide file tree

Showing 5 changed files with 32 additions and 27 deletions.
diff --git a/src/main/java/org/datadog/jmxfetch/reporter/ConsoleReporter.java b/src/main/java/org/datadog/jmxfetch/reporter/ConsoleReporter.java
@@ -45,16 +45,17 @@ public List<Map<String, Object>> getMetrics() {
     }
 
     /** Adds service check to report on. */
-    public void doSendServiceCheck(String checkName, String status, String message, String[] tags) {
+    public void doSendServiceCheck(
+            String serviceCheckName, String status, String message, String[] tags) {
         String tagString = "";
         if (tags != null && tags.length > 0) {
             tagString = "[" + Joiner.on(",").join(tags) + "]";
         }
-        log.info(
-                checkName + tagString + " - " + System.currentTimeMillis() / 1000 + " = " + status);
+        log.info(serviceCheckName + tagString + " - " + System.currentTimeMillis() / 1000
+                 + " = " + status);
 
         Map<String, Object> sc = new HashMap<String, Object>();
-        sc.put("name", checkName);
+        sc.put("name", serviceCheckName);
         sc.put("status", status);
         sc.put("message", message);
         sc.put("tags", tags);

diff --git a/src/main/java/org/datadog/jmxfetch/reporter/JsonReporter.java b/src/main/java/org/datadog/jmxfetch/reporter/JsonReporter.java
@@ -39,13 +39,14 @@ protected void sendMetricPoint(
     }
 
     /** Use the service check callback to display the JSON. */
-    public void doSendServiceCheck(String checkName, String status, String message, String[] tags) {
+    public void doSendServiceCheck(
+            String serviceCheckName, String status, String message, String[] tags) {
         log.debug("Displaying JSON output");
         Map<String, Object> sc = new HashMap<String, Object>();
-        sc.put("check", checkName);
+        sc.put("check", serviceCheckName);
         sc.put("host_name", "default");
         sc.put("timestamp", System.currentTimeMillis() / 1000);
-        sc.put("status", status);
+        sc.put("status", this.statusToServiceCheckStatus(status));
         sc.put("message", message);
         sc.put("tags", tags);
 

diff --git a/src/main/java/org/datadog/jmxfetch/reporter/Reporter.java b/src/main/java/org/datadog/jmxfetch/reporter/Reporter.java
@@ -1,12 +1,14 @@
 package org.datadog.jmxfetch.reporter;
 
+import com.timgroup.statsd.ServiceCheck;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.commons.lang.StringUtils;
 
 import org.datadog.jmxfetch.App;
 import org.datadog.jmxfetch.Instance;
 import org.datadog.jmxfetch.JmxAttribute;
 import org.datadog.jmxfetch.Metric;
+import org.datadog.jmxfetch.Status;
 
 import java.util.HashMap;
 import java.util.List;
@@ -157,9 +159,10 @@ public void sendMetrics(List<Metric> metrics, String instanceName, boolean canon
     /** Submits service check. */
     public void sendServiceCheck(String checkName, String status, String message, String[] tags) {
         this.incrementServiceCheckCount(checkName);
-        String dataName = Reporter.formatServiceCheckPrefix(checkName);
+        String serviceCheckName = String.format(
+            "%s.can_connect", Reporter.formatServiceCheckPrefix(checkName));
 
-        this.doSendServiceCheck(dataName, status, message, tags);
+        this.doSendServiceCheck(serviceCheckName, status, message, tags);
     }
 
     /** Increments the service check count - for book-keeping purposes. */
@@ -188,6 +191,17 @@ public static String formatServiceCheckPrefix(String fullname) {
         return StringUtils.join(chunks, ".");
     }
 
+    protected ServiceCheck.Status statusToServiceCheckStatus(String status) {
+        if (status == Status.STATUS_OK) {
+            return ServiceCheck.Status.OK;
+        } else if (status == Status.STATUS_WARNING) {
+            return ServiceCheck.Status.WARNING;
+        } else if (status == Status.STATUS_ERROR) {
+            return ServiceCheck.Status.CRITICAL;
+        }
+        return ServiceCheck.Status.UNKNOWN;
+    }
+
     protected abstract void sendMetricPoint(
             String metricType, String metricName, double value, String[] tags);
 

diff --git a/src/main/java/org/datadog/jmxfetch/reporter/StatsdReporter.java b/src/main/java/org/datadog/jmxfetch/reporter/StatsdReporter.java
@@ -7,7 +7,6 @@
 import lombok.extern.slf4j.Slf4j;
 import org.datadog.jmxfetch.Instance;
 import org.datadog.jmxfetch.JmxAttribute;
-import org.datadog.jmxfetch.Status;
 
 /** A reporter class to submit metrics via statsd. */
 @Slf4j
@@ -68,26 +67,16 @@ protected void sendMetricPoint(
         }
     }
 
-    private ServiceCheck.Status statusToServiceCheckStatus(String status) {
-        if (status == Status.STATUS_OK) {
-            return ServiceCheck.Status.OK;
-        } else if (status == Status.STATUS_WARNING) {
-            return ServiceCheck.Status.WARNING;
-        } else if (status == Status.STATUS_ERROR) {
-            return ServiceCheck.Status.CRITICAL;
-        }
-        return ServiceCheck.Status.UNKNOWN;
-    }
-
     /** Submits service check. */
-    public void doSendServiceCheck(String checkName, String status, String message, String[] tags) {
+    public void doSendServiceCheck(
+            String serviceCheckName, String status, String message, String[] tags) {
         if (System.currentTimeMillis() - this.initializationTime > 300 * 1000) {
             this.statsDClient.stop();
             init();
         }
 
         ServiceCheck sc = ServiceCheck.builder()
-                .withName(String.format("%s.can_connect", checkName))
+                .withName(serviceCheckName)
                 .withStatus(this.statusToServiceCheckStatus(status))
                 .withMessage(message)
                 .withTags(tags)

diff --git a/src/test/java/org/datadog/jmxfetch/TestServiceChecks.java b/src/test/java/org/datadog/jmxfetch/TestServiceChecks.java
@@ -42,7 +42,7 @@ public void testServiceCheckOK() throws Exception {
         String scStatus = (String) (sc.get("status"));
         String[] scTags = (String[]) (sc.get("tags"));
 
-        assertEquals(Reporter.formatServiceCheckPrefix("jmx"), scName);
+        assertEquals(Reporter.formatServiceCheckPrefix("jmx") + ".can_connect", scName);
         assertEquals(Status.STATUS_OK, scStatus);
         assertEquals(scTags.length, 3);
         assertTrue(Arrays.asList(scTags).contains("instance:jmx_test_instance"));
@@ -83,7 +83,7 @@ public void testServiceCheckWarning() throws Exception {
         String scStatus = (String) (sc.get("status"));
         String[] scTags = (String[]) (sc.get("tags"));
 
-        assertEquals(Reporter.formatServiceCheckPrefix("too_many_metrics"), scName);
+        assertEquals(Reporter.formatServiceCheckPrefix("too_many_metrics") + ".can_connect", scName);
         // We should have an OK service check status when too many metrics are getting sent
         assertEquals(Status.STATUS_OK, scStatus);
         assertEquals(scTags.length, 3);
@@ -115,7 +115,7 @@ public void testServiceCheckCRITICAL() throws Exception {
         String scMessage = (String) (sc.get("message"));
         String[] scTags = (String[]) (sc.get("tags"));
 
-        assertEquals(Reporter.formatServiceCheckPrefix("non_running_process"), scName);
+        assertEquals(Reporter.formatServiceCheckPrefix("non_running_process") + ".can_connect", scName);
         assertEquals(Status.STATUS_ERROR, scStatus);
         assertEquals(
                 "Unable to instantiate or initialize instance process_regex: `.*non_running_process_test.*`. "
@@ -142,7 +142,7 @@ public void testServiceCheckCRITICAL() throws Exception {
         scMessage = (String) (sc.get("message"));
         scTags = (String[]) (sc.get("tags"));
 
-        assertEquals(Reporter.formatServiceCheckPrefix("non_running_process"), scName);
+        assertEquals(Reporter.formatServiceCheckPrefix("non_running_process") + ".can_connect", scName);
         assertEquals(Status.STATUS_ERROR, scStatus);
         assertEquals(
                 "Unable to instantiate or initialize instance process_regex: `.*non_running_process_test.*`. "