dlmarion commented on code in PR #3288:
URL: https://github.com/apache/accumulo/pull/3288#discussion_r1170196112
##########
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.
With Java, I think it's standard practice, or should be, for developers to
evaluate whether or not the superclass method needs to be called when
overriding a method.
>
> Because of the tagging, the registration / initialization needs to occur
in the run method - and after certain things have been initialized (host and
port)
>
agreed
> 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.
>
I think that might be fine iff only AbstractServer subclasses call that
method.
> 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.
agreed
--
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]
