dlmarion commented on code in PR #3288:
URL: https://github.com/apache/accumulo/pull/3288#discussion_r1169907576
##########
server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java:
##########
@@ -602,7 +602,7 @@ public void run() {
LOG.error("Error initializing metrics, metrics will not be emitted.",
e1);
}
pausedMetrics = new PausedCompactionMetrics();
- MetricsUtil.initializeProducers(this, pausedMetrics);
+ initServerMetrics(pausedMetrics);
Review Comment:
I think `this` was dropped accidentally. `Compactor` implements
`MetricsProducer` too, so it's `registerMetrics` method is not getting called.
##########
server/base/src/main/java/org/apache/accumulo/server/AbstractServer.java:
##########
@@ -87,6 +94,16 @@ public void runServer() throws Exception {
}
}
+ @Override
+ public void registerMetrics(MeterRegistry registry) {
+ processMetrics.registerMetrics(registry);
+ }
+
+ public void initServerMetrics(MetricsProducer... producers) {
Review Comment:
I don't think this method is necessary, and it's initializing the metrics
for AbstractServer differently than everything else. Since you have modified
`AbstractServer` to implement `MetricsProducer`, then:
1. You should have any subclasses of `AbstractServer` call
`super.registerMetrics` in their respective `registerMetrics` method. For
example,
[Compactor.registerMetrics](https://github.com/apache/accumulo/blob/15c9f55280f4ce28a80b800f1e05128f33c85db3/server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java#L170).
2. You should pass `this` in subclasses of `AbstractServer` in the call to
`MetricsUtil.initializeProducers`.
--
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]