On Thu, Oct 26, 2017 at 1:52 PM Josh Elser <els...@apache.org> wrote:
> That's the million-dollar question, now isn't it! :) > > Something to hbase-metrics-api/README.md? A chapter in the book? > In the Javadoc makes the most sense to me. I don’t think people would check the book. Apply, where did you try and fail to find the info? > Happy to try to translate some of this info into something more "permanent" > > On 10/26/17 12:59 AM, Misty Stanley-Jones wrote: > > If we keep it as is, how and where can we capture this knowledge to save > > future contributors from having to ask again? > > > > On Wed, Oct 25, 2017 at 9:22 AM Josh Elser <els...@apache.org> wrote: > > > >> :) no worries, Appy. These are good questions. > >> > >> It really comes down to the question: would HBase ever provide multiple > >> metrics implementations for users? > >> > >> As we know metrics in HBase now, there isn't any reason to support > >> multiple implementations of metrics. We have one implementation which is > >> optimal for people to use. > >> > >> One plausible use-case I can see being built is a metrics implementation > >> that natively uses something _other than_ Hadoop metrics2. Something you > >> may not have seen yet is the translation-layer from Dropwizard Metrics > >> to Metrics2 via HBaseMetrics2HadoopMetricsAdapter.java. > >> > >> I could see a world where folks would want to build an integration with > >> metrics-library1 to support the metrics platform for their $business1. > >> $business2 might want to use metrics-library2 that can automatically > >> push to something. As we know all too well, there may be conflicting > >> dependencies between metrics-library1 and metrics-library2 to prevent > >> them from successfully functioning on the classpath at the same time. > >> Additionally, $business1 may have a hard requirement to not deploy > >> metrics-library2 (and vice versa). One final concern, bundling our > >> "default" DropwizardMetrics 3-based implementation could cause headaches > >> for users who want to build their own metrics-api implementation. > >> > >> Granted, all of the above is hypothetical, but I think leaving this > >> flexibility in place is worth the separation of logic. > >> > >> On 10/25/17 3:35 AM, Apekshit Sharma wrote: > >>> Hi Josh > >>> > >>> You're right that naming doesn't make it clear, but Enis added a really > >>> great description in README of hbase-metrics-api to explain what's in > >> each > >>> module. However it's not obvious why are we splitting things into > >> separate > >>> module, as opposed to say, separating implementation in a separate > "sub" > >>> package. > >>> > >>> I don't know Avatica, but as you clearly state, one of the requirements > >> was > >>> being able to plug in different implementations. Using service provider > >>> clearly makes sense in that case, so does keeping the implementation > in a > >>> separate jar to avoid loading it if not needed. > >>> But in case of HBase, even if we have multiple implementations > >>> (hypothetically, because it doesn't seem probable), since the only user > >>> will be HBase and all modules will always be in classpath, it doesn't > >>> really get us anything. Right? > >>> I dug into this code first time today and just learnt about service > >>> providers, so i may be off, in which case please correct me. Thank you! > >>> > >>> On Tue, Oct 24, 2017 at 8:25 PM, Josh Elser <els...@apache.org> wrote: > >>> > >>>> Hey Appy, > >>>> > >>>> I think Enis essentially copied what I was originally trying to do. Up > >> in > >>>> Avatica[1], I made a similar API Maven module which just had > interfaces. > >>>> The implementation was them split up into a different module to avoid > >> the > >>>> implementation details of the API (specifically, using Dropwizard3) > from > >>>> "infecting" anyone who just wanted the API. Implementations of the > >> metrics > >>>> API could be picked up via ServiceLoader. > >>>> > >>>> So, here in HBase, our "hbase-metrics" module is really an > >> implementation > >>>> of hbase-metrics-api for dropwizard-metrics 3.2.1 (the naming just > loses > >>>> that detail). > >>>> > >>>> In Avatica, I wanted to make sure that people could use a metrics > >> library > >>>> implementation of their choosing (e.g. tied to whatever kind of > >> container > >>>> or app-server you're deploying Avatica). In HBase, that isn't such a > >>>> concern -- we publish via Metrics2 and that's it. > >>>> > >>>> That said, I could see a place where this separation enables some > extra > >>>> functionality with less headache, but I, admittedly, don't have a > >> concrete > >>>> use-case behind it. > >>>> > >>>> - Josh > >>>> > >>>> [1] https://github.com/apache/calcite-avatica > >>>> > >>>> On 10/24/17 6:09 PM, Apekshit Sharma wrote: > >>>> > >>>>> Hi > >>>>> > >>>>> Am confused why do we have this weird circular dependency between the > >> two > >>>>> modules: hbase-metrics-api & hbase-metrics. And since maven doesn't > >> allow > >>>>> it explicitly, this was tucked and hidden by using reflection? > >>>>> > >>>>> MetricRegistriesImpl[hbase-metrics] implements > >>>>> MetricRegistries[hbase-metrics-api]. > >>>>> MetricRegistriesLoader[hbase-metrics-api] depends on this > >> implementation, > >>>>> MetricRegistriesImpl, because it instantiates it via reflection > >>>>> <https://github.com/apache/hbase/blob/eee3b0180ead73c09b33f9 > >>>>> 583bfee9c01bc3aed2/hbase-metrics-api/src/main/java/org/ > >>>>> apache/hadoop/hbase/metrics/MetricRegistriesLoader.java#L39> > >>>>> [1]. > >>>>> > >>>>> Can we just move all interfaces and default implementation together > >> into > >>>>> single module. But i like the idea of keeping implementations in > >> separate > >>>>> package with "impl" explicitly in package name. > >>>>> > >>>>> [1] > >>>>> https://github.com/apache/hbase/blob/eee3b0180ead73c09b33f95 > >>>>> 83bfee9c01bc3aed2/hbase-metrics-api/src/main/java/org/ > >>>>> apache/hadoop/hbase/metrics/MetricRegistriesLoader.java#L39 > >>>>> > >>>>> -- Appy > >>>>> > >>>>> > >>> > >>> > >> > > >
