Chetan Mehrotra wrote >> - I don't see a reason to have NoopMetric and MetricsService.NOOP in the API. > > In Oak I have found it useful to have NOOP in API for code to evolve. > Some examples > > 1. Allow use of Metrics in code Sling bundle - If we add a direct > dependency on Metrics then any bundle if to be deployed on older setup > would also pull in metrics bundle and Metrics library. Having NOOP > would allow such bundles to have a optional dependency on > MetricsService and default to NOOP (which can be easily inlined and > reimported) and let them be usable in older setups
Hmm...ok, that allows for nicer code than doing a null check all over the place. > > 2. It also enables unit testing - One can stub out the MetricsService > using NOOP implementation to test the logic without pulling in > complete implementation Testing should not drive api design :) > Also note that NoopMetric is package protected. So not exposed But it appears in the api as a main class. Can we maybe hide it and inline it in MetricsService? > >> what happens if a creation method on the MetricsService for the same name is >> called twice? > > The service is thread safe and same object would be reused. So yes it > can hide duplicates. One idea I had was to use a ServiceFactory and > detect such duplicates there based on calling bundle Ok, so the javadocs need to be adjusted that you either get a new or an existing one :) > >> what about an api to get all metrics or a specific one? > > The focus for wrapper API was collection side as that would be used by > majority of classes. For getting all Metrics one can directly access > the underlying MetricRegistry by using dropwizard Metrics library API I see...ok, this api is purely for generating metrics - which makes totally sense. Regards Carsten -- Carsten Ziegeler Adobe Research Switzerland cziege...@apache.org