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) {

Reply via email to