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 <[email protected]> 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 <[email protected]> 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 > >>> > >>> > > > > >
