From d3a2a16b497fc473d1e6a9fd36389e8db79bb46b Mon Sep 17 00:00:00 2001 From: Olivier Vielpeau Date: Fri, 31 Jul 2015 18:05:22 -0400 Subject: [PATCH] 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`. --- src/main/java/org/datadog/jmxfetch/App.java | 3 +-- src/main/java/org/datadog/jmxfetch/Instance.java | 13 ++++--------- .../datadog/jmxfetch/reporter/ConsoleReporter.java | 3 +-- .../org/datadog/jmxfetch/reporter/Reporter.java | 6 +++--- .../datadog/jmxfetch/reporter/StatsdReporter.java | 5 ++--- src/test/java/org/datadog/jmxfetch/TestApp.java | 13 +++++-------- 6 files changed, 16 insertions(+), 27 deletions(-) diff --git a/src/main/java/org/datadog/jmxfetch/App.java b/src/main/java/org/datadog/jmxfetch/App.java index 203fc3fca..e8cce7ab7 100644 --- a/src/main/java/org/datadog/jmxfetch/App.java +++ b/src/main/java/org/datadog/jmxfetch/App.java @@ -290,8 +290,7 @@ private HashMap 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 index 63cacc190..2e0436d52 100644 --- a/src/main/java/org/datadog/jmxfetch/Instance.java +++ b/src/main/java/org/datadog/jmxfetch/Instance.java @@ -290,7 +290,11 @@ private void refreshBeansList() throws IOException { public String[] getServiceCheckTags() { List tags = new ArrayList(); + 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()]); } @@ -298,15 +302,6 @@ 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 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 index 19638eb41..122a2cc93 100644 --- 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> 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 index a0b4b91dc..f220611ac 100644 --- 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 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 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 index f7f14e98a..ae1b7709b 100644 --- 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 index 1c2e85e9d..eeff37f20 100644 --- 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,7 +473,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.*. 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); @@ -481,9 +481,6 @@ public void testServiceCheckCRITICAL() throws Exception { @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"));