You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by ma...@apache.org on 2021/10/13 00:16:50 UTC
[pinot] branch master updated: Adding config for metrics library
(#7551)
This is an automated email from the ASF dual-hosted git repository.
mayanks pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 8150322 Adding config for metrics library (#7551)
8150322 is described below
commit 8150322d8033b91bb8775f7772a158ff7b781f58
Author: Tim Santos <ti...@gmail.com>
AuthorDate: Tue Oct 12 17:16:35 2021 -0700
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 <xi...@gmail.com>
---
.../pinot/common/metrics/PinotMetricUtils.java | 40 +++++++++++++++-------
.../pinot/common/metrics/PinotMetricUtilsTest.java | 13 +++++++
.../org/apache/pinot/minion/BaseMinionStarter.java | 2 +-
.../java/org/apache/pinot/minion/MinionConf.java | 4 +++
.../apache/pinot/spi/utils/CommonConstants.java | 5 +++
5 files changed, 50 insertions(+), 14 deletions(-)
diff --git a/pinot-common/src/main/java/org/apache/pinot/common/metrics/PinotMetricUtils.java b/pinot-common/src/main/java/org/apache/pinot/common/metrics/PinotMetricUtils.java
index 7c3cac0..81f0096 100644
--- a/pinot-common/src/main/java/org/apache/pinot/common/metrics/PinotMetricUtils.java
+++ b/pinot-common/src/main/java/org/apache/pinot/common/metrics/PinotMetricUtils.java
@@ -24,6 +24,7 @@ import java.util.Arrays;
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.apache.pinot.spi.utils.PinotReflectionUtils;
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 class PinotMetricUtils {
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.");
diff --git a/pinot-common/src/test/java/org/apache/pinot/common/metrics/PinotMetricUtilsTest.java b/pinot-common/src/test/java/org/apache/pinot/common/metrics/PinotMetricUtilsTest.java
index 9683491..d8477aa 100644
--- a/pinot-common/src/test/java/org/apache/pinot/common/metrics/PinotMetricUtilsTest.java
+++ b/pinot-common/src/test/java/org/apache/pinot/common/metrics/PinotMetricUtilsTest.java
@@ -106,4 +106,17 @@ public class PinotMetricUtilsTest {
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
+ }
+ }
}
diff --git a/pinot-minion/src/main/java/org/apache/pinot/minion/BaseMinionStarter.java b/pinot-minion/src/main/java/org/apache/pinot/minion/BaseMinionStarter.java
index e08033c..4a04bc9 100644
--- a/pinot-minion/src/main/java/org/apache/pinot/minion/BaseMinionStarter.java
+++ b/pinot-minion/src/main/java/org/apache/pinot/minion/BaseMinionStarter.java
@@ -168,7 +168,7 @@ public abstract class BaseMinionStarter implements ServiceStartable {
// 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();
diff --git a/pinot-minion/src/main/java/org/apache/pinot/minion/MinionConf.java b/pinot-minion/src/main/java/org/apache/pinot/minion/MinionConf.java
index 49d04ea..0d365b8 100644
--- a/pinot-minion/src/main/java/org/apache/pinot/minion/MinionConf.java
+++ b/pinot-minion/src/main/java/org/apache/pinot/minion/MinionConf.java
@@ -63,4 +63,8 @@ public class MinionConf extends PinotConfiguration {
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);
+ }
}
diff --git a/pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java b/pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java
index 787e414..0411264 100644
--- a/pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java
+++ b/pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java
@@ -37,6 +37,10 @@ public class 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 class CommonConstants {
// 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;
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org