From b33e1d30bb863623ce14333b045a7b182244d0e6 Mon Sep 17 00:00:00 2001 From: Carlos Date: Thu, 10 Aug 2023 14:02:40 +0100 Subject: [PATCH] Reverted custom jmxfetch bean to track number of ingested beans, attributes, and metrics (#468) * Revert "custom jmxfetch bean to track number of ingested beans, attributes, and metrics (#453)" This reverts commit 3c030643b369c32824b220fb0c1312d0bebfe541. * Revert "remove quotes from bean parameter based tags (#456)" This reverts commit 02c071f0d5a7a601144e2a31b9f817aac46523b1. --- .gitignore | 1 + src/main/java/org/datadog/jmxfetch/App.java | 6 +- .../java/org/datadog/jmxfetch/Instance.java | 77 ++----------------- .../org/datadog/jmxfetch/JmxAttribute.java | 26 +------ .../datadog/jmxfetch/JmxComplexAttribute.java | 6 +- .../datadog/jmxfetch/JmxSimpleAttribute.java | 6 +- .../org/datadog/jmxfetch/JmxSubAttribute.java | 6 +- .../datadog/jmxfetch/JmxTabularAttribute.java | 6 +- .../jmxfetch/util/InstanceTelemetry.java | 44 ----------- .../jmxfetch/util/InstanceTelemetryMBean.java | 11 --- .../java/org/datadog/jmxfetch/TestApp.java | 62 --------------- .../jmx_bean_tags_dont_normalize_params.yaml | 18 ----- .../jmx_bean_tags_normalize_params.yaml | 19 ----- 13 files changed, 18 insertions(+), 270 deletions(-) delete mode 100644 src/main/java/org/datadog/jmxfetch/util/InstanceTelemetry.java delete mode 100644 src/main/java/org/datadog/jmxfetch/util/InstanceTelemetryMBean.java delete mode 100644 src/test/resources/jmx_bean_tags_dont_normalize_params.yaml delete mode 100644 src/test/resources/jmx_bean_tags_normalize_params.yaml diff --git a/.gitignore b/.gitignore index ac7e58ab9..a1a8ed0fb 100644 --- a/.gitignore +++ b/.gitignore @@ -12,6 +12,7 @@ target/* .idea .vscode *.iml + *.ucls *.png diff --git a/src/main/java/org/datadog/jmxfetch/App.java b/src/main/java/org/datadog/jmxfetch/App.java index d80e9fe0f..4f3e3aee5 100644 --- a/src/main/java/org/datadog/jmxfetch/App.java +++ b/src/main/java/org/datadog/jmxfetch/App.java @@ -388,13 +388,11 @@ void start() { System.arraycopy(minibuff, 0, buffer, oldLen, len); } } - boolean result = processAutoDiscovery(buffer); - this.setReinit(result); + this.setReinit(processAutoDiscovery(buffer)); } if (this.appConfig.remoteEnabled()) { - boolean result = getJsonConfigs(); - this.setReinit(result); + this.setReinit(getJsonConfigs()); } } catch (IOException e) { log.warn( diff --git a/src/main/java/org/datadog/jmxfetch/Instance.java b/src/main/java/org/datadog/jmxfetch/Instance.java index 1542869d2..e924284b3 100644 --- a/src/main/java/org/datadog/jmxfetch/Instance.java +++ b/src/main/java/org/datadog/jmxfetch/Instance.java @@ -4,17 +4,13 @@ import org.datadog.jmxfetch.reporter.Reporter; import org.datadog.jmxfetch.service.ConfigServiceNameProvider; import org.datadog.jmxfetch.service.ServiceNameProvider; -import org.datadog.jmxfetch.util.InstanceTelemetry; import org.yaml.snakeyaml.Yaml; - - import java.io.File; import java.io.FileInputStream; import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; -import java.lang.management.ManagementFactory; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; @@ -24,15 +20,8 @@ import java.util.Map; import java.util.Map.Entry; import java.util.Set; - -import javax.management.InstanceAlreadyExistsException; -import javax.management.InstanceNotFoundException; import javax.management.MBeanAttributeInfo; import javax.management.MBeanInfo; -import javax.management.MBeanRegistrationException; -import javax.management.MBeanServer; -import javax.management.MalformedObjectNameException; -import javax.management.NotCompliantMBeanException; import javax.management.ObjectName; import javax.security.auth.login.FailedLoginException; @@ -114,10 +103,6 @@ public Yaml initialValue() { private AppConfig appConfig; private Boolean cassandraAliasing; private boolean emptyDefaultHostname; - private InstanceTelemetry instanceTelemetryBean; - private ObjectName instanceTelemetryBeanName; - private MBeanServer mbs; - private Boolean normalizeBeanParamTags; /** Constructor, instantiates Instance based of a previous instance and appConfig. */ public Instance(Instance instance, AppConfig appConfig) { @@ -226,12 +211,6 @@ public Instance( this.serviceCheckPrefix = (String) initConfig.get("service_check_prefix"); } - this.normalizeBeanParamTags = (Boolean) instanceMap.get("normalize_bean_param_tags"); - if (this.normalizeBeanParamTags == null) { - this.normalizeBeanParamTags = false; - } - - // Alternative aliasing for CASSANDRA-4009 metrics // More information: https://issues.apache.org/jira/browse/CASSANDRA-4009 this.cassandraAliasing = (Boolean) instanceMap.get("cassandra_aliasing"); @@ -278,34 +257,8 @@ public Instance( } else { log.info("collect_default_jvm_metrics is false - not collecting default JVM metrics"); } - - instanceTelemetryBean = createJmxBean(); - } - - private ObjectName getObjName(String domain,String instance) - throws MalformedObjectNameException { - return new ObjectName(domain + ":target_instance=" + ObjectName.quote(instance)); - } - - private InstanceTelemetry createJmxBean() { - mbs = ManagementFactory.getPlatformMBeanServer(); - InstanceTelemetry bean = new InstanceTelemetry(); - log.debug("Created jmx bean for instance: " + this.getCheckName()); - - try { - instanceTelemetryBeanName = getObjName("JMXFetch" , this.getName()); - mbs.registerMBean(bean,instanceTelemetryBeanName); - log.debug("Succesfully registered jmx bean for instance: " + this.getCheckName()); - - } catch (MalformedObjectNameException | InstanceAlreadyExistsException - | MBeanRegistrationException | NotCompliantMBeanException e) { - log.warn("Could not register bean for instance: " + this.getCheckName(),e); - } - - return bean; } - public static boolean isDirectInstance(Map configInstance) { Object directInstance = configInstance.get(JVM_DIRECT); return directInstance instanceof Boolean && (Boolean) directInstance; @@ -500,7 +453,7 @@ public List getMetrics() throws IOException { ? this.initialRefreshBeansPeriod : this.refreshBeansPeriod; if (isPeriodDue(this.lastRefreshTime, period)) { - log.info("Refreshing bean list for " + this.getCheckName()); + log.info("Refreshing bean list"); this.refreshBeansList(); this.getMatchingAttributes(); } @@ -532,13 +485,6 @@ public List getMetrics() throws IOException { } } } - instanceTelemetryBean.setBeanCount(beans.size()); - instanceTelemetryBean.setAttributeCount(matchingAttributes.size()); - instanceTelemetryBean.setMetricCount(metrics.size()); - log.debug("Updated jmx bean for instance: " + this.getCheckName() - + " With number of beans = " + instanceTelemetryBean.getBeanCount() - + " attributes = " + instanceTelemetryBean.getAttributeCount() - + " metrics = " + instanceTelemetryBean.getMetricCount()); return metrics; } @@ -632,8 +578,7 @@ private void getMatchingAttributes() throws IOException { serviceNameProvider, tags, cassandraAliasing, - emptyDefaultHostname, - normalizeBeanParamTags); + emptyDefaultHostname); } else if (COMPOSED_TYPES.contains(attributeType)) { log.debug( ATTRIBUTE @@ -651,8 +596,7 @@ private void getMatchingAttributes() throws IOException { connection, serviceNameProvider, tags, - emptyDefaultHostname, - normalizeBeanParamTags); + emptyDefaultHostname); } else if (MULTI_TYPES.contains(attributeType)) { log.debug( ATTRIBUTE @@ -670,8 +614,7 @@ private void getMatchingAttributes() throws IOException { connection, serviceNameProvider, tags, - emptyDefaultHostname, - normalizeBeanParamTags); + emptyDefaultHostname); } else { try { log.debug( @@ -830,18 +773,8 @@ public boolean isLimitReached() { return this.limitReached; } - private void cleanupTelemetryBean() { - try { - mbs.unregisterMBean(instanceTelemetryBeanName); - log.debug("Successfully unregistered bean for instance: " + this.getCheckName()); - } catch (MBeanRegistrationException | InstanceNotFoundException e) { - 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(); @@ -854,7 +787,7 @@ public void cleanUp() { * */ public synchronized void cleanUpAsync() { appConfig = null; - cleanupTelemetryBean(); + class AsyncCleaner implements Runnable { Connection conn; diff --git a/src/main/java/org/datadog/jmxfetch/JmxAttribute.java b/src/main/java/org/datadog/jmxfetch/JmxAttribute.java index 38e926790..68661d82c 100644 --- a/src/main/java/org/datadog/jmxfetch/JmxAttribute.java +++ b/src/main/java/org/datadog/jmxfetch/JmxAttribute.java @@ -62,7 +62,6 @@ public abstract class JmxAttribute { private List defaultTagsList; private boolean cassandraAliasing; protected String checkName; - private boolean normalizeBeanParamTags; JmxAttribute( MBeanAttributeInfo attribute, @@ -74,8 +73,7 @@ public abstract class JmxAttribute { ServiceNameProvider serviceNameProvider, Map instanceTags, boolean cassandraAliasing, - boolean emptyDefaultHostname, - boolean normalizeBeanParamTags) { + boolean emptyDefaultHostname) { this.attribute = attribute; this.beanName = beanName; this.className = className; @@ -86,7 +84,6 @@ public abstract class JmxAttribute { this.cassandraAliasing = cassandraAliasing; this.checkName = checkName; this.serviceNameProvider = serviceNameProvider; - this.normalizeBeanParamTags = normalizeBeanParamTags; // A bean name is formatted like that: // org.apache.cassandra.db:type=Caches,keyspace=system,cache=HintsColumnFamilyKeyCache @@ -194,35 +191,16 @@ private List getBeanParametersList( return beanTags; } - /** - * Wrapper for javax.management.ObjectName.unqoute that removes quotes from the bean parameter - * value if possible. If not, it hits the catch block and returns the original parameter. - */ - private String unquote(String beanParameter) { - int valueIndex = beanParameter.indexOf(':') + 1; - try { - return beanParameter.substring(0,valueIndex) - + ObjectName.unquote(beanParameter.substring(valueIndex)); - } catch (IllegalArgumentException e) { - return beanParameter; - } - } - /** * Sanitize MBean parameter names and values, i.e. - Rename parameter names conflicting with * existing tags - Remove illegal characters */ - private List sanitizeParameters(List beanParametersList) { + private static List sanitizeParameters(List beanParametersList) { List defaultTagsList = new ArrayList(beanParametersList.size()); for (String rawBeanParameter : beanParametersList) { // Remove `|` characters String beanParameter = rawBeanParameter.replace("|", ""); - // Unquote bean parameters that have been quoted and escaped by ObjectName.quote() - if (normalizeBeanParamTags == true) { - beanParameter = unquote(beanParameter); - } - // 'host' parameter is renamed to 'bean_host' if (beanParameter.startsWith("host:")) { defaultTagsList.add("bean_host:" + beanParameter.substring("host:".length())); diff --git a/src/main/java/org/datadog/jmxfetch/JmxComplexAttribute.java b/src/main/java/org/datadog/jmxfetch/JmxComplexAttribute.java index 3ea2db51b..70ca4cb47 100644 --- a/src/main/java/org/datadog/jmxfetch/JmxComplexAttribute.java +++ b/src/main/java/org/datadog/jmxfetch/JmxComplexAttribute.java @@ -30,8 +30,7 @@ public JmxComplexAttribute( Connection connection, ServiceNameProvider serviceNameProvider, Map instanceTags, - boolean emptyDefaultHostname, - boolean normalizeBeanParamTags) { + boolean emptyDefaultHostname) { super( attribute, beanName, @@ -42,8 +41,7 @@ public JmxComplexAttribute( serviceNameProvider, instanceTags, false, - emptyDefaultHostname, - normalizeBeanParamTags); + emptyDefaultHostname); } private void populateSubAttributeList(Object attributeValue) { diff --git a/src/main/java/org/datadog/jmxfetch/JmxSimpleAttribute.java b/src/main/java/org/datadog/jmxfetch/JmxSimpleAttribute.java index b890613ea..c57f16e67 100644 --- a/src/main/java/org/datadog/jmxfetch/JmxSimpleAttribute.java +++ b/src/main/java/org/datadog/jmxfetch/JmxSimpleAttribute.java @@ -28,8 +28,7 @@ public JmxSimpleAttribute( ServiceNameProvider serviceNameProvider, Map instanceTags, boolean cassandraAliasing, - Boolean emptyDefaultHostname, - Boolean normalizeBeanParamTags) { + Boolean emptyDefaultHostname) { super( attribute, beanName, @@ -40,8 +39,7 @@ public JmxSimpleAttribute( serviceNameProvider, instanceTags, cassandraAliasing, - emptyDefaultHostname, - normalizeBeanParamTags); + emptyDefaultHostname); } @Override diff --git a/src/main/java/org/datadog/jmxfetch/JmxSubAttribute.java b/src/main/java/org/datadog/jmxfetch/JmxSubAttribute.java index 98a6e1666..0c8899ed4 100644 --- a/src/main/java/org/datadog/jmxfetch/JmxSubAttribute.java +++ b/src/main/java/org/datadog/jmxfetch/JmxSubAttribute.java @@ -21,8 +21,7 @@ public JmxSubAttribute( ServiceNameProvider serviceNameProvider, Map instanceTags, boolean cassandraAliasing, - boolean emptyDefaultHostname, - boolean normalizeBeanParamTags) { + boolean emptyDefaultHostname) { super( attribute, beanName, @@ -33,8 +32,7 @@ public JmxSubAttribute( serviceNameProvider, instanceTags, cassandraAliasing, - emptyDefaultHostname, - normalizeBeanParamTags); + emptyDefaultHostname); } public Metric getCachedMetric(String name) { diff --git a/src/main/java/org/datadog/jmxfetch/JmxTabularAttribute.java b/src/main/java/org/datadog/jmxfetch/JmxTabularAttribute.java index efc5a1f35..d36369f6c 100644 --- a/src/main/java/org/datadog/jmxfetch/JmxTabularAttribute.java +++ b/src/main/java/org/datadog/jmxfetch/JmxTabularAttribute.java @@ -38,8 +38,7 @@ public JmxTabularAttribute( Connection connection, ServiceNameProvider serviceNameProvider, Map instanceTags, - boolean emptyDefaultHostname, - boolean normalizeBeanParamTags) { + boolean emptyDefaultHostname) { super( attribute, beanName, @@ -50,8 +49,7 @@ public JmxTabularAttribute( serviceNameProvider, instanceTags, false, - emptyDefaultHostname, - normalizeBeanParamTags); + emptyDefaultHostname); subAttributeList = new HashMap>(); } diff --git a/src/main/java/org/datadog/jmxfetch/util/InstanceTelemetry.java b/src/main/java/org/datadog/jmxfetch/util/InstanceTelemetry.java deleted file mode 100644 index f8e3b5224..000000000 --- a/src/main/java/org/datadog/jmxfetch/util/InstanceTelemetry.java +++ /dev/null @@ -1,44 +0,0 @@ -package org.datadog.jmxfetch.util; - - -/** Jmxfetch telemetry JMX MBean. */ -public class InstanceTelemetry implements InstanceTelemetryMBean { - - private int beanCount; - private int attributeCount; - private int metricCount; - - /** Jmxfetch telemetry bean constructor. */ - public InstanceTelemetry() { - beanCount = 0; - attributeCount = 0; - metricCount = 0; - } - - public int getBeanCount() { - return beanCount; - } - - public int getAttributeCount() { - return attributeCount; - } - - public int getMetricCount() { - return metricCount; - } - - - public void setBeanCount(int count) { - beanCount = count; - } - - public void setAttributeCount(int count) { - attributeCount = count; - } - - public void setMetricCount(int count) { - metricCount = count; - } - - -} diff --git a/src/main/java/org/datadog/jmxfetch/util/InstanceTelemetryMBean.java b/src/main/java/org/datadog/jmxfetch/util/InstanceTelemetryMBean.java deleted file mode 100644 index 9402679ec..000000000 --- a/src/main/java/org/datadog/jmxfetch/util/InstanceTelemetryMBean.java +++ /dev/null @@ -1,11 +0,0 @@ -package org.datadog.jmxfetch.util; - -public interface InstanceTelemetryMBean { - - int getBeanCount(); - - int getAttributeCount(); - - int getMetricCount(); - -} diff --git a/src/test/java/org/datadog/jmxfetch/TestApp.java b/src/test/java/org/datadog/jmxfetch/TestApp.java index e45a5de3d..a2563ef50 100644 --- a/src/test/java/org/datadog/jmxfetch/TestApp.java +++ b/src/test/java/org/datadog/jmxfetch/TestApp.java @@ -11,8 +11,6 @@ import java.util.List; import java.util.Map; -import javax.management.ObjectName; - import org.junit.Test; public class TestApp extends TestCommon { @@ -72,66 +70,6 @@ public void testBeanTags() throws Exception { assertMetric("this.is.100", tags, 6); } - /** Tag metrics with MBeans parameters with normalize_bean_param_tags option enabled. */ - @Test - public void testBeanTagsNormalizeParams() throws Exception { - // We expose a few metrics through JMX - registerMBean( - new SimpleTestJavaApp(), - "org.datadog.jmxfetch.test:type=\"SimpleTestJavaApp\",scope=\"Co|olScope\",host=\"localhost\",component=,target_instance=" - + ObjectName.quote(".*example.process.regex.*")); - initApplication("jmx_bean_tags_normalize_params.yaml"); - - // Collecting metrics - run(); - List> metrics = getMetrics(); - - // 14 = 13 metrics from java.lang + 1 metric explicitly defined in the yaml config file - assertEquals(14, metrics.size()); - - List tags = - Arrays.asList( - "type:SimpleTestJavaApp", - "scope:CoolScope", - "instance:jmx_test_instance", - "jmx_domain:org.datadog.jmxfetch.test", - "bean_host:localhost", - "component", - "target_instance:.*example.process.regex.*"); - - assertMetric("this.is.100", tags, 7); - } - - /** Tag metrics with MBeans parameters with normalize_bean_param_tags option disabled. */ - @Test - public void testBeanTagsDontNormalizeParams() throws Exception { - // We expose a few metrics through JMX - registerMBean( - new SimpleTestJavaApp(), - "org.datadog.jmxfetch.test:type=\"SimpleTestJavaApp\",scope=\"Co|olScope\",host=\"localhost\",component=,target_instance=" - + ObjectName.quote(".*example.process.regex.*")); - initApplication("jmx_bean_tags_dont_normalize_params.yaml"); - - // Collecting metrics - run(); - List> metrics = getMetrics(); - - // 14 = 13 metrics from java.lang + 1 metric explicitly defined in the yaml config file - assertEquals(14, metrics.size()); - - List tags = - Arrays.asList( - "type:\"SimpleTestJavaApp\"", - "scope:\"CoolScope\"", - "instance:jmx_test_instance", - "jmx_domain:org.datadog.jmxfetch.test", - "bean_host:\"localhost\"", - "component", - "target_instance:\".\\*example.process.regex.\\*\""); - - assertMetric("this.is.100", tags, 7); - } - /** Generate metric aliases from a `alias_match` regular expression. */ @Test public void testRegexpAliasing() throws Exception { diff --git a/src/test/resources/jmx_bean_tags_dont_normalize_params.yaml b/src/test/resources/jmx_bean_tags_dont_normalize_params.yaml deleted file mode 100644 index b9ddb5267..000000000 --- a/src/test/resources/jmx_bean_tags_dont_normalize_params.yaml +++ /dev/null @@ -1,18 +0,0 @@ -init_config: - -instances: - - process_name_regex: .*surefire.* - name: jmx_test_instance - conf: - - include: - bean: org.datadog.jmxfetch.test:type="SimpleTestJavaApp",scope="Co|olScope",host="localhost",component=,target_instance=".\*example.process.regex.\*" - attribute: - ShouldBe100: - metric_type: gauge - alias: this.is.100 - - include: - bean: org.datadog.jmxfetch.test:type=WrongType,scope=WrongScope,host=localhost,component= - attribute: - Hashmap.thisis0: - metric_type: gauge - alias: bean.parameters.should.not.match diff --git a/src/test/resources/jmx_bean_tags_normalize_params.yaml b/src/test/resources/jmx_bean_tags_normalize_params.yaml deleted file mode 100644 index b2aee6578..000000000 --- a/src/test/resources/jmx_bean_tags_normalize_params.yaml +++ /dev/null @@ -1,19 +0,0 @@ -init_config: - -instances: - - process_name_regex: .*surefire.* - name: jmx_test_instance - conf: - - include: - bean: org.datadog.jmxfetch.test:type="SimpleTestJavaApp",scope="Co|olScope",host="localhost",component=,target_instance=".\*example.process.regex.\*" - attribute: - ShouldBe100: - metric_type: gauge - alias: this.is.100 - - include: - bean: org.datadog.jmxfetch.test:type=WrongType,scope=WrongScope,host=localhost,component= - attribute: - Hashmap.thisis0: - metric_type: gauge - alias: bean.parameters.should.not.match - normalize_bean_param_tags: true