From 37b10dff9a7701f51230fc155d38a61e41bd32a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Herv=C3=A9?= Date: Mon, 2 Mar 2020 14:08:28 +0100 Subject: [PATCH 1/4] 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/jmxfetch/reporter/JsonReporter.java | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/datadog/jmxfetch/reporter/JsonReporter.java b/src/main/java/org/datadog/jmxfetch/reporter/JsonReporter.java index 0188c46dd..e72b6ba29 100644 --- a/src/main/java/org/datadog/jmxfetch/reporter/JsonReporter.java +++ b/src/main/java/org/datadog/jmxfetch/reporter/JsonReporter.java @@ -2,10 +2,12 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.SerializationFeature; +import com.timgroup.statsd.ServiceCheck; import lombok.extern.slf4j.Slf4j; import org.datadog.jmxfetch.Instance; import org.datadog.jmxfetch.JmxAttribute; +import org.datadog.jmxfetch.Status; import java.io.IOException; import java.io.StringWriter; @@ -38,14 +40,25 @@ protected void sendMetricPoint( metrics.add(metric); } + 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; + } + /** Use the service check callback to display the JSON. */ public void doSendServiceCheck(String checkName, String status, String message, String[] tags) { log.debug("Displaying JSON output"); Map sc = new HashMap(); - sc.put("check", checkName); + sc.put("check", String.format("%s.can_connect", checkName)); 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); From 6ffb925e6b35891f509f9d9211434036ba60c821 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Herv=C3=A9?= Date: Wed, 11 Mar 2020 11:42:23 +0100 Subject: [PATCH 2/4] Build service check name in the reporter --- .../org/datadog/jmxfetch/reporter/ConsoleReporter.java | 6 +++--- .../java/org/datadog/jmxfetch/reporter/JsonReporter.java | 4 ++-- src/main/java/org/datadog/jmxfetch/reporter/Reporter.java | 4 ++-- .../org/datadog/jmxfetch/reporter/StatsdReporter.java | 4 ++-- src/test/java/org/datadog/jmxfetch/TestServiceChecks.java | 8 ++++---- 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/datadog/jmxfetch/reporter/ConsoleReporter.java b/src/main/java/org/datadog/jmxfetch/reporter/ConsoleReporter.java index d520f4996..65aa2f836 100644 --- a/src/main/java/org/datadog/jmxfetch/reporter/ConsoleReporter.java +++ b/src/main/java/org/datadog/jmxfetch/reporter/ConsoleReporter.java @@ -45,16 +45,16 @@ public List> 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); + serviceCheckName + tagString + " - " + System.currentTimeMillis() / 1000 + " = " + status); Map sc = new HashMap(); - 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 index e72b6ba29..2384e7ee4 100644 --- a/src/main/java/org/datadog/jmxfetch/reporter/JsonReporter.java +++ b/src/main/java/org/datadog/jmxfetch/reporter/JsonReporter.java @@ -52,10 +52,10 @@ private ServiceCheck.Status statusToServiceCheckStatus(String status) { } /** 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 sc = new HashMap(); - sc.put("check", String.format("%s.can_connect", checkName)); + sc.put("check", serviceCheckName); sc.put("host_name", "default"); sc.put("timestamp", System.currentTimeMillis() / 1000); sc.put("status", this.statusToServiceCheckStatus(status)); diff --git a/src/main/java/org/datadog/jmxfetch/reporter/Reporter.java b/src/main/java/org/datadog/jmxfetch/reporter/Reporter.java index 82df1f544..09f6019e8 100644 --- a/src/main/java/org/datadog/jmxfetch/reporter/Reporter.java +++ b/src/main/java/org/datadog/jmxfetch/reporter/Reporter.java @@ -157,9 +157,9 @@ public void sendMetrics(List 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. */ diff --git a/src/main/java/org/datadog/jmxfetch/reporter/StatsdReporter.java b/src/main/java/org/datadog/jmxfetch/reporter/StatsdReporter.java index e5d5594bc..f19931f1c 100644 --- a/src/main/java/org/datadog/jmxfetch/reporter/StatsdReporter.java +++ b/src/main/java/org/datadog/jmxfetch/reporter/StatsdReporter.java @@ -80,14 +80,14 @@ private ServiceCheck.Status statusToServiceCheckStatus(String status) { } /** 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 index 059491bfe..8f8a49f24 100644 --- 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.*`. " From 2a13cc660f526e22e645586a231b902ab752997f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Herv=C3=A9?= Date: Wed, 11 Mar 2020 11:48:02 +0100 Subject: [PATCH 3/4] Move statusToServiceCheckStatus to the reporter --- .../org/datadog/jmxfetch/reporter/JsonReporter.java | 13 ------------- .../org/datadog/jmxfetch/reporter/Reporter.java | 13 +++++++++++++ .../datadog/jmxfetch/reporter/StatsdReporter.java | 12 ------------ 3 files changed, 13 insertions(+), 25 deletions(-) diff --git a/src/main/java/org/datadog/jmxfetch/reporter/JsonReporter.java b/src/main/java/org/datadog/jmxfetch/reporter/JsonReporter.java index 2384e7ee4..58f3b9a20 100644 --- a/src/main/java/org/datadog/jmxfetch/reporter/JsonReporter.java +++ b/src/main/java/org/datadog/jmxfetch/reporter/JsonReporter.java @@ -2,12 +2,10 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.SerializationFeature; -import com.timgroup.statsd.ServiceCheck; import lombok.extern.slf4j.Slf4j; import org.datadog.jmxfetch.Instance; import org.datadog.jmxfetch.JmxAttribute; -import org.datadog.jmxfetch.Status; import java.io.IOException; import java.io.StringWriter; @@ -40,17 +38,6 @@ protected void sendMetricPoint( metrics.add(metric); } - 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; - } - /** Use the service check callback to display the JSON. */ public void doSendServiceCheck(String serviceCheckName, String status, String message, String[] tags) { log.debug("Displaying JSON output"); diff --git a/src/main/java/org/datadog/jmxfetch/reporter/Reporter.java b/src/main/java/org/datadog/jmxfetch/reporter/Reporter.java index 09f6019e8..faddaa997 100644 --- a/src/main/java/org/datadog/jmxfetch/reporter/Reporter.java +++ b/src/main/java/org/datadog/jmxfetch/reporter/Reporter.java @@ -1,5 +1,6 @@ package org.datadog.jmxfetch.reporter; +import com.timgroup.statsd.ServiceCheck; import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang.StringUtils; @@ -7,6 +8,7 @@ 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; @@ -188,6 +190,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 index f19931f1c..155ab4867 100644 --- 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,17 +67,6 @@ 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 serviceCheckName, String status, String message, String[] tags) { if (System.currentTimeMillis() - this.initializationTime > 300 * 1000) { From 3223db1741e14ece6de515ed5e71dd332cf18e39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20Herv=C3=A9?= Date: Wed, 11 Mar 2020 13:03:32 +0100 Subject: [PATCH 4/4] Fix style --- .../org/datadog/jmxfetch/reporter/ConsoleReporter.java | 7 ++++--- .../java/org/datadog/jmxfetch/reporter/JsonReporter.java | 3 ++- src/main/java/org/datadog/jmxfetch/reporter/Reporter.java | 3 ++- .../java/org/datadog/jmxfetch/reporter/StatsdReporter.java | 3 ++- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/datadog/jmxfetch/reporter/ConsoleReporter.java b/src/main/java/org/datadog/jmxfetch/reporter/ConsoleReporter.java index 65aa2f836..b15ceda73 100644 --- a/src/main/java/org/datadog/jmxfetch/reporter/ConsoleReporter.java +++ b/src/main/java/org/datadog/jmxfetch/reporter/ConsoleReporter.java @@ -45,13 +45,14 @@ public List> getMetrics() { } /** Adds service check to report on. */ - public void doSendServiceCheck(String serviceCheckName, 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( - serviceCheckName + tagString + " - " + System.currentTimeMillis() / 1000 + " = " + status); + log.info(serviceCheckName + tagString + " - " + System.currentTimeMillis() / 1000 + + " = " + status); Map sc = new HashMap(); sc.put("name", serviceCheckName); diff --git a/src/main/java/org/datadog/jmxfetch/reporter/JsonReporter.java b/src/main/java/org/datadog/jmxfetch/reporter/JsonReporter.java index 58f3b9a20..7426342ea 100644 --- a/src/main/java/org/datadog/jmxfetch/reporter/JsonReporter.java +++ b/src/main/java/org/datadog/jmxfetch/reporter/JsonReporter.java @@ -39,7 +39,8 @@ protected void sendMetricPoint( } /** Use the service check callback to display the JSON. */ - public void doSendServiceCheck(String serviceCheckName, String status, String message, String[] tags) { + public void doSendServiceCheck( + String serviceCheckName, String status, String message, String[] tags) { log.debug("Displaying JSON output"); Map sc = new HashMap(); sc.put("check", serviceCheckName); diff --git a/src/main/java/org/datadog/jmxfetch/reporter/Reporter.java b/src/main/java/org/datadog/jmxfetch/reporter/Reporter.java index faddaa997..f536c38c0 100644 --- a/src/main/java/org/datadog/jmxfetch/reporter/Reporter.java +++ b/src/main/java/org/datadog/jmxfetch/reporter/Reporter.java @@ -159,7 +159,8 @@ public void sendMetrics(List metrics, String instanceName, boolean canon /** Submits service check. */ public void sendServiceCheck(String checkName, String status, String message, String[] tags) { this.incrementServiceCheckCount(checkName); - String serviceCheckName = String.format("%s.can_connect", Reporter.formatServiceCheckPrefix(checkName)); + String serviceCheckName = String.format( + "%s.can_connect", Reporter.formatServiceCheckPrefix(checkName)); this.doSendServiceCheck(serviceCheckName, status, message, tags); } diff --git a/src/main/java/org/datadog/jmxfetch/reporter/StatsdReporter.java b/src/main/java/org/datadog/jmxfetch/reporter/StatsdReporter.java index 155ab4867..76e45a5ac 100644 --- a/src/main/java/org/datadog/jmxfetch/reporter/StatsdReporter.java +++ b/src/main/java/org/datadog/jmxfetch/reporter/StatsdReporter.java @@ -68,7 +68,8 @@ protected void sendMetricPoint( } /** Submits service check. */ - public void doSendServiceCheck(String serviceCheckName, 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();