timsants commented on a change in pull request #7551:
URL: https://github.com/apache/pinot/pull/7551#discussion_r726773832
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/metrics/PinotMetricUtils.java
##########
@@ -54,35 +57,49 @@ private PinotMetricUtils() {
private static PinotMetricsFactory _pinotMetricsFactory = null;
- public static void init(PinotConfiguration metricsConfiguration) {
+ public static void init(PinotConfiguration metricsConfiguration, @Nullable
String metricsFactoryClassName) {
// Initializes PinotMetricsFactory.
- initializePinotMetricsFactory(metricsConfiguration);
+ initializePinotMetricsFactory(metricsConfiguration,
metricsFactoryClassName);
// Initializes metrics using the metrics configuration.
initializeMetrics(metricsConfiguration);
registerMetricsRegistry(getPinotMetricsRegistry());
}
+ public static void init(PinotConfiguration metricsConfiguration) {
+ init(metricsConfiguration, null);
+ }
+
/**
* Initializes PinotMetricsFactory with metrics configurations.
* @param metricsConfiguration The subset of the configuration containing
the metrics-related keys
+ * @param configuredMetricsFactoryClassName The configuration value for the
desired metrics factory class. Will
+ * default to Yammer if not
provided.
*/
- private static void initializePinotMetricsFactory(PinotConfiguration
metricsConfiguration) {
+ private static void initializePinotMetricsFactory(PinotConfiguration
metricsConfiguration,
Review comment:
I was also thinking I could just use the existing metricsConfiguration.
But the reason why I decided not to was because the Broker, Controller and
Server instance starters each call `.subset` on the `PinotConfiguration` for
their particular metrics config prefix (e.g. `pinot.broker.metrics`). But the
issue is we want to create 1 global config value
`pinot.metrics.factory.className` that is not contained within a metrics
configuration prefix.
We could construct a new PinotConfiguration combine the exiting configs with
the metrics factory config but seemed like a bit of overhead for just 1
property to be immediately retrieved again. It would look something like this:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
