This is an automated email from the ASF dual-hosted git repository. dlmarion pushed a commit to branch 2.1 in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/2.1 by this push: new 41fb8ef8f4 Disabled metric collection when GENERAL_MICROMETER_ENABLED is false (#4993) 41fb8ef8f4 is described below commit 41fb8ef8f4c0d4afc979ac52a87f190659f029df Author: Dave Marion <dlmar...@apache.org> AuthorDate: Mon Oct 28 09:58:41 2024 -0400 Disabled metric collection when GENERAL_MICROMETER_ENABLED is false (#4993) Prior to this change user specified MeterRegistry's were not added when GENERAL_MICROMETER_ENABLED was false. JVM metrics could still be collected, but never reported to a destination. Closes #4989 --- .../org/apache/accumulo/core/conf/Property.java | 7 ++- .../accumulo/server/metrics/MetricsInfoImpl.java | 57 +++++++++++++--------- .../tserver/TabletServerResourceManager.java | 2 +- 3 files changed, 40 insertions(+), 26 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/conf/Property.java b/core/src/main/java/org/apache/accumulo/core/conf/Property.java index e6d853e40e..2cae14b189 100644 --- a/core/src/main/java/org/apache/accumulo/core/conf/Property.java +++ b/core/src/main/java/org/apache/accumulo/core/conf/Property.java @@ -308,9 +308,12 @@ public enum Property { "The maximum amount of time that a Scanner should wait before retrying a failed RPC.", "1.7.3"), GENERAL_MICROMETER_ENABLED("general.micrometer.enabled", "false", PropertyType.BOOLEAN, - "Enables metrics functionality using Micrometer.", "2.1.0"), + "Enables metrics collection and reporting functionality using Micrometer.", "2.1.0"), GENERAL_MICROMETER_JVM_METRICS_ENABLED("general.micrometer.jvm.metrics.enabled", "false", - PropertyType.BOOLEAN, "Enables JVM metrics functionality using Micrometer.", "2.1.0"), + PropertyType.BOOLEAN, + "Enables additional JVM metrics collection and reporting using Micrometer. Requires " + + "property 'general.micrometer.enabled' to be set to 'true' to take effect.", + "2.1.0"), GENERAL_MICROMETER_FACTORY("general.micrometer.factory", "org.apache.accumulo.core.spi.metrics.LoggingMeterRegistryFactory", PropertyType.CLASSNAMELIST, diff --git a/server/base/src/main/java/org/apache/accumulo/server/metrics/MetricsInfoImpl.java b/server/base/src/main/java/org/apache/accumulo/server/metrics/MetricsInfoImpl.java index 19adacc554..06ef34b566 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/metrics/MetricsInfoImpl.java +++ b/server/base/src/main/java/org/apache/accumulo/server/metrics/MetricsInfoImpl.java @@ -89,17 +89,14 @@ public class MetricsInfoImpl implements MetricsInfo { final boolean jvmMetricsEnabled = context.getConfiguration().getBoolean(Property.GENERAL_MICROMETER_JVM_METRICS_ENABLED); LOG.info("micrometer metrics enabled: {}", metricsEnabled); - if (jvmMetricsEnabled) { - if (metricsEnabled) { - LOG.info("detailed jvm metrics enabled: {}", jvmMetricsEnabled); - } else { - LOG.info("requested jvm metrics, but micrometer metrics are disabled."); - } + if (!metricsEnabled) { + return; } - if (metricsEnabled) { - LOG.info("metrics registry factories: {}", - context.getConfiguration().get(Property.GENERAL_MICROMETER_FACTORY)); + if (jvmMetricsEnabled) { + LOG.info("detailed jvm metrics enabled: {}", jvmMetricsEnabled); } + LOG.info("metrics registry factories: {}", + context.getConfiguration().get(Property.GENERAL_MICROMETER_FACTORY)); } @Override @@ -113,6 +110,9 @@ public class MetricsInfoImpl implements MetricsInfo { @Override public void addServiceTags(final String applicationName, final HostAndPort hostAndPort, final String resourceGroupName) { + if (!metricsEnabled) { + return; + } List<Tag> tags = new ArrayList<>(); tags.add(MetricsInfo.processTag(applicationName)); @@ -124,6 +124,9 @@ public class MetricsInfoImpl implements MetricsInfo { @Override public void addCommonTags(List<Tag> updates) { + if (!metricsEnabled) { + return; + } lock.lock(); try { if (composite != null) { @@ -150,6 +153,9 @@ public class MetricsInfoImpl implements MetricsInfo { @Override public void addRegistry(MeterRegistry registry) { + if (!metricsEnabled) { + return; + } lock.lock(); try { if (composite != null) { @@ -166,6 +172,9 @@ public class MetricsInfoImpl implements MetricsInfo { @Override public void addMetricsProducers(MetricsProducer... producer) { + if (!metricsEnabled) { + return; + } if (producer.length == 0) { LOG.debug( "called addMetricsProducers() without providing at least one producer - this has no effect"); @@ -198,6 +207,10 @@ public class MetricsInfoImpl implements MetricsInfo { @Override public void init() { + if (!metricsEnabled) { + LOG.info("Metrics not initialized, metrics are disabled."); + return; + } lock.lock(); try { if (composite != null) { @@ -234,20 +247,18 @@ public class MetricsInfoImpl implements MetricsInfo { } }; - if (isMetricsEnabled()) { - // user specified registries - String userRegistryFactories = - context.getConfiguration().get(Property.GENERAL_MICROMETER_FACTORY); - - for (String factoryName : getTrimmedStrings(userRegistryFactories)) { - try { - MeterRegistry registry = getRegistryFromFactory(factoryName, context); - registry.config().commonTags(commonTags.values()); - registry.config().meterFilter(replicationFilter); - addRegistry(registry); - } catch (ReflectiveOperationException ex) { - LOG.warn("Could not load registry {}", factoryName, ex); - } + // user specified registries + String userRegistryFactories = + context.getConfiguration().get(Property.GENERAL_MICROMETER_FACTORY); + + for (String factoryName : getTrimmedStrings(userRegistryFactories)) { + try { + MeterRegistry registry = getRegistryFromFactory(factoryName, context); + registry.config().commonTags(commonTags.values()); + registry.config().meterFilter(replicationFilter); + addRegistry(registry); + } catch (ReflectiveOperationException ex) { + LOG.warn("Could not load registry {}", factoryName, ex); } } diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java index a171d7d33c..0421b9d19f 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java @@ -264,7 +264,7 @@ public class TabletServerResourceManager { public TabletServerResourceManager(ServerContext context, TabletHostingServer tserver) { this.context = context; final AccumuloConfiguration acuConf = context.getConfiguration(); - final boolean enableMetrics = context.getMetricsInfo().isMetricsEnabled(); // acuConf.getBoolean(Property.GENERAL_MICROMETER_ENABLED); + final boolean enableMetrics = context.getMetricsInfo().isMetricsEnabled(); long maxMemory = acuConf.getAsBytes(Property.TSERV_MAXMEM); boolean usingNativeMap = acuConf.getBoolean(Property.TSERV_NATIVEMAP_ENABLED); if (usingNativeMap) {