dajac commented on code in PR #12121:
URL: https://github.com/apache/kafka/pull/12121#discussion_r875156714
##########
clients/src/main/java/org/apache/kafka/common/metrics/Metrics.java:
##########
@@ -524,6 +524,58 @@ public void addMetric(MetricName metricName,
MetricValueProvider<?> metricValueP
addMetric(metricName, null, metricValueProvider);
}
+ /**
+ * Create or get an existing metric to monitor an object that implements
Measurable.
+ * This metric won't be associated with any sensor. This is a way to
expose existing values as metrics.
+ *
+ * This method is kept for binary compatibility purposes, it has the same
behaviour as
+ * {@link #metricOrElseCreate(MetricName, MetricConfig,
MetricValueProvider)}.
+ *
+ * @param metricName The name of the metric
+ * @param config The configuration to use when measuring this measurable
+ * @param measurable The measurable that will be measured by this metric
+ * @return Existing KafkaMetric if already registered or else a newly
created one
+ */
+ public KafkaMetric metricOrElseCreate(MetricName metricName, MetricConfig
config, Measurable measurable) {
Review Comment:
I wonder if we really need all these overloads. Do we have use cases for all
of them? I have the impression that the last one would be sufficient.
##########
clients/src/main/java/org/apache/kafka/common/metrics/Metrics.java:
##########
@@ -563,10 +615,15 @@ public synchronized void removeReporter(MetricsReporter
reporter) {
}
}
- synchronized void registerMetric(KafkaMetric metric) {
+ synchronized void registerMetric(KafkaMetric metric, boolean
raiseIfMetricExists) {
Review Comment:
I am wonder if this change is really necessary. Is it?
##########
clients/src/main/java/org/apache/kafka/common/metrics/Metrics.java:
##########
@@ -524,6 +524,58 @@ public void addMetric(MetricName metricName,
MetricValueProvider<?> metricValueP
addMetric(metricName, null, metricValueProvider);
}
+ /**
+ * Create or get an existing metric to monitor an object that implements
Measurable.
+ * This metric won't be associated with any sensor. This is a way to
expose existing values as metrics.
+ *
+ * This method is kept for binary compatibility purposes, it has the same
behaviour as
+ * {@link #metricOrElseCreate(MetricName, MetricConfig,
MetricValueProvider)}.
+ *
+ * @param metricName The name of the metric
+ * @param config The configuration to use when measuring this measurable
+ * @param measurable The measurable that will be measured by this metric
+ * @return Existing KafkaMetric if already registered or else a newly
created one
+ */
+ public KafkaMetric metricOrElseCreate(MetricName metricName, MetricConfig
config, Measurable measurable) {
+ return metricOrElseCreate(metricName, config, (MetricValueProvider<?>)
measurable);
+ }
+
+ /**
+ * Create or get an existing metric to monitor an object that implements
MetricValueProvider.
+ * This metric won't be associated with any sensor. This is a way to
expose existing values as metrics.
+ * This method takes care of synchronisation while updating/accessing
metrics by concurrent threads.
+ *
+ * @param metricName The name of the metric
+ * @param metricValueProvider The metric value provider associated with
this metric
+ * @return Existing KafkaMetric if already registered or else a newly
created one
+ */
+ public KafkaMetric metricOrElseCreate(MetricName metricName,
MetricValueProvider<?> metricValueProvider) {
+ return metricOrElseCreate(metricName, null, metricValueProvider);
+ }
+
+ /**
+ * Create or get an existing metric to monitor an object that implements
MetricValueProvider.
+ * This metric won't be associated with any sensor. This is a way to
expose existing values as metrics.
+ * This method takes care of synchronisation while updating/accessing
metrics by concurrent threads.
+ *
+ * @param metricName The name of the metric
+ * @param metricValueProvider The metric value provider associated with
this metric
+ * @return Existing KafkaMetric if already registered or else a newly
created one
+ */
+ public KafkaMetric metricOrElseCreate(MetricName metricName, MetricConfig
config, MetricValueProvider<?> metricValueProvider) {
Review Comment:
Instead of updating ‘registerMetric’, did we consider using synchronized?
--
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]
