Hi, On 19 January 2017 at 09:26, Chetan Mehrotra <chetan.mehro...@gmail.com> wrote:
> On Thu, Jan 19, 2017 at 2:48 PM, Ian Boston <ianbos...@gmail.com> wrote: > > > > Is the intention that [1] should use the same approach as [2] so that > there > > is 1 MetricsRegistry per JVM ? > > No. Sling Metrics creates and manages its own MetricRegistry and > registers it in OSGi with a "name" service property set to "sling". > The one in Oak also dose registers its managed MetricRegistry and > registers it with "name=oak" service property. > The webconsole plugin accounts for multiple registry instance. > > > ie Check for a MetricsRegistry as a service, and if not present create > one, so that there is only 1 which reporters can report on. > > The reporter implemented so far [1] and [2] account for that and can > work with multiple MetricRegistry present in OSGi service registry > I think this design decision should be revisited, but is not my decision, so what follows is simply opinion and although a patch might be welcome, it would be too large and disruptive to apply. <imvho> it adds unecessary complexity and code making it harder to use and consume metrics. I also fear that there will be multiple APIs, all nearly the same, providing a "registry", "meter", "counter", "guage", "histogram" etc. (both Sling and Oak have their own APIs already) The same happened in Sling/Oaks support of JMX. There is no consistency between bundles, and the vast majority of beans that could provide statistics for monitoring are unusable. Fortunately if every bundle uses the MetricsRegistry API underneath, at least the exposed metrics will have consistency, even if the APIs are many and unrecognisable. Metrics is like Logging. Over the years Logging, and which Logging library to use has sparked pasionate argument. Fortunately, we dont have that problem in Sling where 1 API (slf4j) makes logging simple. The evidence is plain to see in error.log. IMHO, there should be 1 bundle providing 1 service and API that all bundles can use. That means Oak and Sling will have to agree on ownership of that bundle, and resolve the "Oak depends deeply on Sling vs Sling depends deeply on Oak" conflict. I dont really have a strong objection to 100s of MetricsRegistry services other than it adds to bloat (a very small increment to the existing footprint admittedly), but I think it is an unecessary complexity that hinders creating and using metrics. Without metrics, we are all blind in production. </imvho> It could be so simple to have 1 bundle exposing a service, adaptable to the required MetricsRegistry singleton for reporting. BTW. [2] doesnt work with the version of Oak 1.6 I am testing on. It didnt export MetricsRegistry. Backports or dirty hacks will probably be required to make [2] work with versions of Oak in production. I will work on a dirty hack to get what I need to, to work. Best Regards Ian > > Chetan Mehrotra > [1] https://github.com/apache/sling/blob/trunk/bundles/ > commons/metrics/src/main/java/org/apache/sling/commons/metrics/internal/ > MetricWebConsolePlugin.java#L431 > [2] https://github.com/bdelacretaz/sling-adaptto- > 2016/blob/master/sling-images/bundles/metrics-es-reporter/ > src/main/java/ch/x42/sling/at16/internal/MetricCopyingListener.java#L36 >
