Adding config for metrics library (#7551)

* Removing DropWizard metrics library from distribution

* Fixing conditional check

* Using global config value for setting metrics factory class

* Add back dropwizard in pinot-dist jar

* Adding back warning if multiple metrics factories found

* Update PinotMetricUtils.java

* Making separate metrics factory configs for each component. This simplifies the configs passed into the PinotMetricsUtil.init

* Cleanup whitespace

* Address minor comments

Co-authored-by: Xiang Fu <xiangfu.1024@gmail.com>

pinot-common/src/main/java/org/apache/pinot/common/metrics/PinotMetricUtils.java

@@ -24,6 +24,7 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.TimeUnit;
@@ -41,6 +42,9 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import static org.apache.pinot.spi.utils.CommonConstants.CONFIG_OF_METRICS_FACTORY_CLASS_NAME;
+import static org.apache.pinot.spi.utils.CommonConstants.DEFAULT_METRICS_FACTORY_CLASS_NAME;
+
 
 public class PinotMetricUtils {
   private PinotMetricUtils() {
@@ -70,21 +74,31 @@ public static void init(PinotConfiguration metricsConfiguration) {
   private static void initializePinotMetricsFactory(PinotConfiguration metricsConfiguration) {
     Set<Class<?>> classes = getPinotMetricsFactoryClasses();
     if (classes.size() > 1) {
-      LOGGER.warn("More than one PinotMetricsFactory is initialized: {}", classes);
+      LOGGER.warn("More than one PinotMetricsFactory was found: {}", classes);
     }
-    for (Class<?> clazz : classes) {
-      MetricsFactory annotation = clazz.getAnnotation(MetricsFactory.class);
-      LOGGER.info("Trying to init PinotMetricsFactory: {} and MetricsFactory: {}", clazz, annotation);
-      if (annotation.enabled()) {
-        try {
-          PinotMetricsFactory pinotMetricsFactory = (PinotMetricsFactory) clazz.newInstance();
-          pinotMetricsFactory.init(metricsConfiguration);
-          registerMetricsFactory(pinotMetricsFactory);
-        } catch (Exception e) {
-          LOGGER.error("Caught exception while initializing pinot metrics registry: {}, skipping it", clazz, e);
+
+    String metricsFactoryClassName = metricsConfiguration.getProperty(CONFIG_OF_METRICS_FACTORY_CLASS_NAME,
+        DEFAULT_METRICS_FACTORY_CLASS_NAME);
+    LOGGER.info("{} will be initialized as the PinotMetricsFactory", metricsFactoryClassName);
+
+    Optional<Class<?>> clazzFound = classes.stream().filter(c -> c.getName().equals(metricsFactoryClassName))
+        .findFirst();
+
+    clazzFound.ifPresent(clazz -> {
+          MetricsFactory annotation = clazz.getAnnotation(MetricsFactory.class);
+          LOGGER.info("Trying to init PinotMetricsFactory: {} and MetricsFactory: {}", clazz, annotation);
+          if (annotation.enabled()) {
+            try {
+              PinotMetricsFactory pinotMetricsFactory = (PinotMetricsFactory) clazz.newInstance();
+              pinotMetricsFactory.init(metricsConfiguration);
+              registerMetricsFactory(pinotMetricsFactory);
+            } catch (Exception e) {
+              LOGGER.error("Caught exception while initializing pinot metrics registry: {}, skipping it", clazz, e);
+            }
+          }
         }
-      }
-    }
+    );
+
     Preconditions.checkState(_pinotMetricsFactory != null,
         "Failed to initialize PinotMetricsFactory. Please check if any pinot-metrics related jar is actually added to"
             + " the classpath.");

pinot-common/src/test/java/org/apache/pinot/common/metrics/PinotMetricUtilsTest.java

@@ -106,4 +106,17 @@ public void testPinotMetricName() {
     Assert.assertNotNull(testMetricName2);
     Assert.assertEquals(testMetricName1, testMetricName2);
   }
+
+  @Test
+  public void testMetricRegistryFailure() {
+    try {
+      Map<String, Object> properties = new HashMap<>();
+      properties.put("factory.className", "NonExistentClass");
+      PinotConfiguration metricsConfiguration = new PinotConfiguration(properties);
+      PinotMetricUtils.init(metricsConfiguration);
+      Assert.fail("Illegal state exception should have been thrown since metrics factory class was not found");
+    } catch (IllegalStateException e) {
+      // Expected
+    }
+  }
 }

pinot-minion/src/main/java/org/apache/pinot/minion/BaseMinionStarter.java

@@ -168,7 +168,7 @@ public void start()
     // Initialize metrics
     LOGGER.info("Initializing metrics");
     // TODO: put all the metrics related configs down to "pinot.server.metrics"
-    PinotConfiguration metricsConfiguration = _config;
+    PinotConfiguration metricsConfiguration = _config.getMetricsConfig();
     PinotMetricUtils.init(metricsConfiguration);
     PinotMetricsRegistry metricsRegistry = PinotMetricUtils.getPinotMetricsRegistry();
 

pinot-minion/src/main/java/org/apache/pinot/minion/MinionConf.java

@@ -63,4 +63,8 @@ public String getInstanceId() {
   public int getEndReplaceSegmentsTimeoutMs() {
     return getProperty(END_REPLACE_SEGMENTS_TIMEOUT_MS_KEY, DEFAULT_END_REPLACE_SEGMENTS_SOCKET_TIMEOUT_MS);
   }
+
+  public PinotConfiguration getMetricsConfig() {
+    return subset(CommonConstants.Minion.METRICS_CONFIG_PREFIX);
+  }
 }

pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java

@@ -37,6 +37,10 @@ private CommonConstants() {
 
   public static final String TABLE_NAME = "tableName";
 
+  public static final String CONFIG_OF_METRICS_FACTORY_CLASS_NAME = "factory.className";
+  public static final String DEFAULT_METRICS_FACTORY_CLASS_NAME =
+      "org.apache.pinot.plugin.metrics.yammer.YammerMetricsFactory";
+
   /**
    * The state of the consumer for a given segment
    */
@@ -429,6 +433,7 @@ public static class Minion {
     // Config keys
     public static final String CONFIG_OF_METRICS_PREFIX_KEY = "metricsPrefix";
     public static final String METRICS_REGISTRY_REGISTRATION_LISTENERS_KEY = "metricsRegistryRegistrationListeners";
+    public static final String METRICS_CONFIG_PREFIX = "pinot.minion.metrics";
 
     // Default settings
     public static final int DEFAULT_HELIX_PORT = 9514;