EdColeman commented on code in PR #3288:
URL: https://github.com/apache/accumulo/pull/3288#discussion_r1169988835
##########
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:
@dlmarion What do you think is easier to understand? Classes that extended
abstract server must call `super.registerMetrics` or call a method
'initServerMetrics' ? Either way requires something that may not be obvious.
Because of the tagging, the registration / initialization needs to occur in
the run method - and after certain things have been initialized (host and port)
I'm not thrilled with either approach, but went with a specific
`initServerMetrics` method - if relying on calling `super.registerMetrics' and
the needing to call the static `MetricsUtil.initializeProducers` which results
in an indirect call to `registerMetrics`?
I was thinking about moving the static `initializeMetrics` out of
MetricsUtil and put the functionality into AbstractServer - I think that
functionality only applies to services extending AbstractServer.
The `initializeProducers` functionality needs to exist to register metrics
producers for things that are created outside of the AbstarctServer context
(thrift metrics, cache,....) - but those are created / exist within a service
context that has already been expected to initialize the registry.
--
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]
