So.. in a nutshell, what @galen-pivotal and I spoke about:

- The notion of a `MetricsCollector` as it stands and is currently used, makes 
no sense. @upthewaterspout has already pointed out that it abstracts the usage 
of  the micrometer.io `CompositeMeterRegistry` and at the same time it does 
not. To me, the simplest approach is to pass in the `MeterRegistry` into the 
`InternalDistributedSystem` from "outside" (maybe as @upthewaterspout has 
indicated the `CacheFactory`.
- IMO, some thought has to be put into the current management layer, to enable 
the creation, amendment of a constructed `CompositeMeterRegistry`. If there 
would be a `MetricsConfigurationService`, it could potentially hold onto all 
configurations for downstream registries. With the correct amount of 
design/abstraction and implementation, this `*ConfigurationService` could 
"discovered" using the extensions framework. With a concrete 
`MetricsConfigurationService` all configurations can be distributed/replicated 
to other servers with the cluster. (also include persistence)
- The adding and removing of `MeterRegistry` from the `CompositeMeterRegistry` 
is a great feature! e.g Adding or removing a `JMXRegistry` at runtime is nice. 
BUT in lieu of a properly defined lifecycle and management service, I would not 
make that the only reason to have a `MetricController` interface.

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

Reply via email to