[AMLII-1815] Telemetry is no longer initialized when telemetry disabled

CHANGELOG.md

@@ -1,6 +1,7 @@
 Changelog
 =========
 # 0.49.2-SNAPSHOT / TBC
+* [BUGFIX] Telemetry is no longer initialized when telemetry is disabled [#522][]
 
 # 0.49.1 / 2024-04-09
 * [FEATURE] Add ZGC Major and Minor Cycles and ZGC Major and Minor Pauses beans support out of the box (Generational ZGC support) [#509][]
@@ -767,6 +768,7 @@ Changelog
 [#477]: https://github.com/DataDog/jmxfetch/issues/477
 [#509]: https://github.com/DataDog/jmxfetch/issues/509
 [#512]: https://github.com/DataDog/jmxfetch/pull/512
+[#522]: https://github.com/DataDog/jmxfetch/pull/522
 [@alz]: https://github.com/alz
 [@aoking]: https://github.com/aoking
 [@arrawatia]: https://github.com/arrawatia

pom.xml

@@ -50,7 +50,7 @@
         <checkstyle.config.location>style.xml</checkstyle.config.location>
 
 
-        <junit.version>4.13.1</junit.version>
+        <junit.version>4.13.2</junit.version>
         <mockito.version>2.28.2</mockito.version>
 
         <maven-surefire-plugin.version>2.9</maven-surefire-plugin.version>

src/main/java/org/datadog/jmxfetch/App.java

@@ -133,7 +133,30 @@ public App(final AppConfig appConfig) {
         }
         this.configs = getConfigs(this.appConfig);
 
-        this.initTelemetryBean();
+        this.appTelemetry = new AppTelemetry();
+
+        if (this.appConfig.getJmxfetchTelemetry()) {
+            log.info("Enabling JMX Fetch Telemetry");
+            this.registerTelemetryBean(this.appTelemetry);
+        }
+    }
+
+    private void registerTelemetryBean(AppTelemetry bean) {
+        MBeanServer mbs =  ManagementFactory.getPlatformMBeanServer();
+        ObjectName appTelemetryBeanName = getAppTelemetryBeanName();
+        if (appTelemetryBeanName == null) {
+            return;
+        }
+
+        try {
+            mbs.registerMBean(bean, appTelemetryBeanName);
+            log.debug("Succesfully registered app telemetry bean");
+        } catch (InstanceAlreadyExistsException
+                 | MBeanRegistrationException
+                 | NotCompliantMBeanException e) {
+            log.warn("Could not register bean named '{}' for instance: ",
+                appTelemetryBeanName.toString(), e);
+        }
     }
 
     private ObjectName getAppTelemetryBeanName() {
@@ -520,7 +543,9 @@ void start() {
     }
 
     void stop() {
-        this.teardownTelemetry();
+        if (this.appConfig.getJmxfetchTelemetry()) {
+            this.teardownTelemetry();
+        }
         this.collectionProcessor.stop();
         this.recoveryProcessor.stop();
     }

src/main/java/org/datadog/jmxfetch/AppConfig.java

@@ -125,6 +125,8 @@ public class AppConfig {
             required = false)
     private boolean statsdTelemetry;
 
+    /* Do not default to true as this affects embedded mode where
+            customers can have custom MBean managers */
     @Parameter(
             names = {"--jmxfetch_telemetry", "-jt"},
             description = "Enable additional jmxfetch telemetry reporting",

src/main/java/org/datadog/jmxfetch/Instance.java

@@ -240,18 +240,20 @@ public Instance(
             log.info("collect_default_jvm_metrics is false - not collecting default JVM metrics");
         }
 
-        instanceTelemetryBean = createInstanceTelemetryBean();
+        instanceTelemetryBean = new InstanceTelemetry();
+        if (appConfig.getJmxfetchTelemetry()) {
+            registerTelemetryBean(instanceTelemetryBean);
+        }
     }
 
     private ObjectName getObjName(String domain,String instance)
             throws MalformedObjectNameException {
         return new ObjectName(domain + ":target_instance=" + ObjectName.quote(instance));
     }
 
-    private InstanceTelemetry createInstanceTelemetryBean() {
-        mbs =  ManagementFactory.getPlatformMBeanServer();
-        InstanceTelemetry bean = new InstanceTelemetry();
-        log.debug("Created jmx bean for instance: " + this.getCheckName());
+    private InstanceTelemetry registerTelemetryBean(InstanceTelemetry bean) {
+        mbs = ManagementFactory.getPlatformMBeanServer();
+        log.debug("Created jmx bean for instance: {}", this.getCheckName());
 
         try {
             instanceTelemetryBeanName = getObjName(appConfig.getJmxfetchTelemetryDomain(),
@@ -516,8 +518,8 @@ public List<Metric> getMetrics() throws IOException {
                     + " With beans fetched = " + instanceTelemetryBean.getBeansFetched()
                     + " top attributes = " + instanceTelemetryBean.getTopLevelAttributeCount()
                     + " metrics = " + instanceTelemetryBean.getMetricCount()
-                    + " wildcard domain query count = " 
-                    + instanceTelemetryBean.getWildcardDomainQueryCount() 
+                    + " wildcard domain query count = "
+                    + instanceTelemetryBean.getWildcardDomainQueryCount()
                     + " bean match ratio = " + instanceTelemetryBean.getBeanMatchRatio());
         }
         return metrics;
@@ -710,15 +712,15 @@ private void getMatchingAttributes() throws IOException {
                     reporter.displayNonMatchingAttributeName(jmxAttribute);
                 }
                 if (jmxAttribute.getMatchingConf() != null) {
-                    attributeMatched = true; 
+                    attributeMatched = true;
                 }
             }
             if (attributeMatched) {
                 beansWithAttributeMatch += 1;
             }
         }
         if (instanceTelemetryBean != null) {
-            instanceTelemetryBean.setBeanMatchRatio((double) 
+            instanceTelemetryBean.setBeanMatchRatio((double)
                                   beansWithAttributeMatch / beans.size());
         }
         log.info("Found {} matching attributes", matchingAttributes.size());
@@ -837,18 +839,21 @@ public boolean isLimitReached() {
     }
 
     private void cleanupTelemetryBean() {
+        if (!appConfig.getJmxfetchTelemetry()) {
+            // If telemetry is not enabled, no need to unregister the bean
+            return;
+        }
         try {
             mbs.unregisterMBean(instanceTelemetryBeanName);
-            log.debug("Successfully unregistered bean for instance: " + this.getCheckName());
+            log.debug("Successfully unregistered bean for instance: {}", this.getCheckName());
         } catch (MBeanRegistrationException | InstanceNotFoundException e) {
-            log.debug("Unable to unregister bean for instance: " + this.getCheckName());
+            log.debug("Unable to unregister bean for instance: {}", this.getCheckName());
         }
     }
 
     /** Clean up config and close connection. */
     public void cleanUp() {
         cleanupTelemetryBean();
-        this.appConfig = null;
         if (connection != null) {
             connection.closeConnector();
             connection = null;
@@ -859,7 +864,6 @@ public void cleanUp() {
      * Asynchronoush cleanup of instance, including connection.
      * */
     public synchronized void cleanUpAsync() {
-        appConfig = null;
         cleanupTelemetryBean();
         class AsyncCleaner implements Runnable {
             Connection conn;

src/test/java/org/datadog/jmxfetch/TestApp.java

@@ -61,10 +61,9 @@ public void testBeanTags() throws Exception {
 
         // Collecting metrics
         run();
-        List<Map<String, Object>> metrics = getMetrics();
 
         // 14 = 13 metrics from java.lang + 1 metric explicitly defined in the yaml config file
-        assertEquals(14, metrics.size());
+        assertEquals(14, getMetrics().size());
 
         List<String> tags =
                 Arrays.asList(
@@ -777,12 +776,10 @@ public void testAppCanonicalRate() throws Exception {
         initApplication("jmx_canonical.yaml");
 
         run();
-        List<Map<String, Object>> metrics = getMetrics();
-
         // 29 = 13 metrics from java.lang + the 5 gauges we are explicitly collecting + 9 gauges
         // implicitly collected
         // + 2 multi-value, see jmx.yaml in the test/resources folder
-        assertEquals(29, metrics.size());
+        assertEquals(29, getMetrics().size());
 
         // We test for the presence and the value of the metrics we want to collect
         List<String> commonTags =
@@ -810,11 +807,10 @@ public void testAppCanonicalRate() throws Exception {
 
         // We run a second collection. The counter should now be present
         run();
-        metrics = getMetrics();
         // 30 = 13 metrics from java.lang + the 5 gauges we are explicitly collecting + 9 gauges
         // implicitly collected
         // + 2 multi-value + 2 counter, see jmx.yaml in the test/resources folder
-        assertEquals(31, metrics.size());
+        assertEquals(31, getMetrics().size());
 
         // We test for the same metrics but this time, the counter should be here
         // Previous metrics
@@ -846,13 +842,11 @@ public void testAppCanonicalRate() throws Exception {
         testApp.decrementCounter(5);
 
         run();
-        metrics = getMetrics();
-        assertEquals(30, metrics.size());
+        assertEquals(30, getMetrics().size());
 
         // The metric should be back in the next cycle.
         run();
-        metrics = getMetrics();
-        assertEquals(31, metrics.size());
+        assertEquals(31, getMetrics().size());
         assertMetric("test.counter", 0.0, commonTags, 6);
 
         // Check that they are working again
@@ -862,8 +856,7 @@ public void testAppCanonicalRate() throws Exception {
         testApp.populateTabularData(2);
 
         run();
-        metrics = getMetrics();
-        assertEquals(31, metrics.size());
+        assertEquals(31, getMetrics().size());
 
         // Previous metrics
         assertMetric("this.is.100", 100.0, commonTags, 9);
@@ -903,20 +896,17 @@ public void testAppCount() throws Exception {
 
         // First collection should not contain our count
         run();
-        metrics = getMetrics();
-        assertEquals(13, metrics.size());
+        assertEquals(13, getMetrics().size());
 
         // Since our count is still equal to 0, we should report a delta equal to 0
         run();
-        metrics = getMetrics();
-        assertEquals(14, metrics.size());
+        assertEquals(14, getMetrics().size());
         assertMetric("test.counter", 0, Collections.<String>emptyList(), 4);
 
         // For the 3rd collection we increment the count to 5 so we should get a +5 delta
         testApp.incrementCounter(5);
         run();
-        metrics = getMetrics();
-        assertEquals(14, metrics.size());
+        assertEquals(14, getMetrics().size());
         assertMetric("test.counter", 5, Collections.<String>emptyList(), 4);
 
         assertCoverage();
@@ -935,22 +925,19 @@ public void testAppCounterRate() throws Exception {
 
         // First collection should not contain our count
         run();
-        metrics = getMetrics();
-        assertEquals(26, metrics.size());
+        assertEquals(26, getMetrics().size());
 
         // Since our count is still equal to 0, we should report a delta equal to 0
         run();
-        metrics = getMetrics();
-        assertEquals(28, metrics.size());
+        assertEquals(28, getMetrics().size());
         assertMetric("test.counter", 0, Collections.<String>emptyList(), 4);
         assertMetric("test.rate", 0, Collections.<String>emptyList(), 4);
 
         Thread.sleep(5000);
         // For the 3rd collection we increment the count to 5 so we should get a +5 delta
         testApp.incrementCounter(5);
         run();
-        metrics = getMetrics();
-        assertEquals(28, metrics.size());
+        assertEquals(28, getMetrics().size());
         assertMetric("test.counter", 0.95, 1, Collections.<String>emptyList(), 4);
         assertMetric("test.rate", 0.95, 1, Collections.<String>emptyList(), 4);
 

src/test/java/org/datadog/jmxfetch/TestCommon.java

@@ -62,8 +62,8 @@ public class TestCommon {
     AppConfig appConfig = spy(AppConfig.builder().build());
     App app;
     MBeanServer mbs;
-    List<ObjectName> objectNames = new ArrayList<ObjectName>();
-    List<Map<String, Object>> metrics;
+    List<ObjectName> objectNames = new ArrayList<>();
+    List<Map<String, Object>> metrics = new ArrayList<>();
     List<Map<String, Object>> serviceChecks;
 
     /** Setup logger. */
@@ -73,7 +73,7 @@ public static void init() throws Exception {
         if (level == null) {
             level = "ALL";
         }
-        CustomLogger.setup(LogLevel.ALL, "/tmp/jmxfetch_test.log", false);
+        CustomLogger.setup(LogLevel.fromString(level), "/tmp/jmxfetch_test.log", false);
     }
 
     /**
@@ -104,6 +104,7 @@ public void unregisterMBeans() throws MBeanRegistrationException, InstanceNotFou
             for (ObjectName objectName : objectNames) {
                 mbs.unregisterMBean(objectName);
             }
+            this.objectNames.clear();
         }
     }
 
@@ -112,6 +113,7 @@ public void unregisterMBeans() throws MBeanRegistrationException, InstanceNotFou
      */
     @After
     public void teardown() {
+        this.metrics.clear();
         if (app != null) {
             app.clearAllInstances();
             app.stop();
@@ -214,8 +216,9 @@ protected void initApplicationWithYamlLines(String... yamlLines)
     /** Run a JMXFetch iteration. */
     protected void run() {
         if (app != null) {
+            this.metrics.clear();
             app.doIteration();
-            metrics = ((ConsoleReporter) appConfig.getReporter()).getMetrics();
+            this.metrics.addAll(((ConsoleReporter) appConfig.getReporter()).getMetrics());
         }
     }
 
@@ -382,7 +385,7 @@ public void assertCoverage() {
             }
         }
 
-        if (untestedMetrics.size() > 0) {
+        if (!untestedMetrics.isEmpty()) {
             String message = generateReport(untestedMetrics, totalMetrics);
             fail(message);
         }

src/test/java/org/datadog/jmxfetch/util/MetricsAssert.java

@@ -48,14 +48,14 @@ public static void assertMetric(
                 }
 
                 if (countTags != -1) {
-                    assertEquals(countTags, mTags.size());
+                    assertEquals("Tag count didn't match", countTags, mTags.size());
                 }
                 for (String t : tags) {
-                    assertThat(mTags, hasItem(t));
+                    assertThat("Did not contain tag", mTags, hasItem(t));
                 }
 
                 if (metricType != null) {
-                    assertEquals(metricType, m.get("type"));
+                    assertEquals("Metric types not equal", metricType, m.get("type"));
                 }
                 // Brand the metric
                 m.put("tested", true);