[GitHub] [geode] demery-pivotal commented on issue #3153: GEODE-6295: Add Micrometer-based metrics collector

["demery-pivotal (GitHub)" <gi...@apache.org>]

Hi Dan,

This PR does not add meters. We have identified a small number of meters add to geode-core very soon. The goal of this PR is to get the system in place to do that.

The API is temporarily internal in case we want to tweak it before moving it to a non-internal place. Specifically, we intend to make the MeterCollector interface and the method to obtain one externally available. We considered putting getMetricsCollector() in DistributedSystem and marking it experimental, but one of the questions yet to be decided is whether DistributedSystem is the right place to expose the metrics system.

We intend to expose the CompositeMeterRegistry only as a MeterRegistry, to allow us to substitute other implementations later if we choose. The MeterRegistry (abstract class) is all that’s needed to create meters.

The CompositeMetricsCollector allows us to move the configuration of the CompositeMeterRegistry out of InternalDistributedSystem, which doesn’t need yet more responsibilities. (Though, uh, in the code I submitted, the configuration is done in InternalDistributedSystem. That should be moved into CompositeMetricsCollector.)

MetricsCollector  (the primary hub for accessing the metrics system) is an interface in order to allow us to substitute other implementations later, and make it easier to test things that don’t need the concrete implementation.

Thank you for your review. Have I left you more puzzled? Less puzzled? Differently puzzled?

[ Full content available at: https://github.com/apache/geode/pull/3153 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org