[jira] [Comment Edited] (SLING-7043) Exporting com.codahale.metrics.MetricRegistry is breaking the abstraction
[ https://issues.apache.org/jira/browse/SLING-7043?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16128281#comment-16128281 ] Chetan Mehrotra edited comment on SLING-7043 at 8/16/17 4:21 AM: - I still do not think that current status is having a problem (but that can be just me!). As said earlier. Client code can bind to # Sling Commons Metrics API - Get some added benefits and stable package version. Majority of code needs would be fullfilled by this api. It also allows us to disable metrics collection anytime without affecting the application functionality just like logging # Dropwizzard Metrics API - Get access MetricRegistry and access to actual metrics value Note that Dropwizzard binds package export to release version and some of the exported interfaces are Consumer type. So if code in Sling uses that we need to update such bundles when upgrading Metrics bundle (SLING-7047). Even if we go for option #A above we would still need to export the MetricRegistry as services as quite a bit of existing reporters provided by Metrics itself work with that service. It does not make sense to wrap all such reporters. So access to MetricRegistry by reporter should be fine was (Author: chetanm): I still do not think that current status is having a problem (but that can be just me!). As said earlier. Client code can bind to # Sling Commons Metrics API - Get some added benefits and stable package version. Majority of code needs would be fullfilled by this api # Dropwizzard Metrics API - Get access MetricRegistry and access to actual metrics value Note that Dropwizzard binds package export to release version and some of the exported interfaces are Consumer type. So if code in Sling uses that we need to update such bundles when upgrading Metrics bundle (SLING-7047). Even if we go for option #A above we would still need to export the MetricRegistry as services as quite a bit of existing reporters provided by Metrics itself work with that service. It does not make sense to wrap all such reporters. So access to MetricRegistry by reporter should be fine > Exporting com.codahale.metrics.MetricRegistry is breaking the abstraction > -- > > Key: SLING-7043 > URL: https://issues.apache.org/jira/browse/SLING-7043 > Project: Sling > Issue Type: Bug >Affects Versions: Commons Metrics 1.0.0 >Reporter: Carsten Ziegeler >Priority: Blocker > Fix For: commons metrics 1.2.4 > > > commons metrics provides a nice abstraction over com.codahale.metrics - > however it is exporting com.codahale.metrics.MetricRegistry which seems to > be the only way to get at registered metrics objects. Which in turn is > completely breaking the purpose of this bundle. > So we should > a) drop exporting that service and avoid leaking internal implementation > details > b) create our own version of the registry service -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Comment Edited] (SLING-7043) Exporting com.codahale.metrics.MetricRegistry is breaking the abstraction
[ https://issues.apache.org/jira/browse/SLING-7043?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16125306#comment-16125306 ] Chetan Mehrotra edited comment on SLING-7043 at 8/14/17 6:52 AM: - bq. but again it is totally flawed as we expose a way to get to the underlying implementation as an API contract. This gives a false sense of safety. Well I disagree here. It depends on the client code what it wants to use. If the client only binds to Commons Metrics api then it gets the required support of the abstraction (and possibly some extra features like JMX Domain name, future support for metadata etc). If the code needs more features (which in general does not happen, at least based on usage we have in Oak side which follows similar pattern) then it binds to Metrics api. For me the usage is similar to Resource and its adaptation support for JCR Node. Most code can work with Resource api but some code may need lower level JCR features provided by Node api was (Author: chetanm): bq. but again it is totally flawed as we expose a way to get to the underlying implementation as an API contract. This gives a false sense of safety. Well I disagree here. It depends on the client code what it wants to use. If the client only binds to Commons Metrics api then it gets the required support of the abstraction (and possibly some extra features like JMX Domain name, future support for metadata etc). If the code needs more features (which in general does not happen, at least based on usage we have in Oak side which follows similar pattern) then it binds to Metrics api. > Exporting com.codahale.metrics.MetricRegistry is breaking the abstraction > -- > > Key: SLING-7043 > URL: https://issues.apache.org/jira/browse/SLING-7043 > Project: Sling > Issue Type: Bug >Affects Versions: Commons Metrics 1.0.0 >Reporter: Carsten Ziegeler >Priority: Blocker > Fix For: commons metrics 1.2.4 > > > commons metrics provides a nice abstraction over com.codahale.metrics - > however it is exporting com.codahale.metrics.MetricRegistry which seems to > be the only way to get at registered metrics objects. Which in turn is > completely breaking the purpose of this bundle. > So we should > a) drop exporting that service and avoid leaking internal implementation > details > b) create our own version of the registry service -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Comment Edited] (SLING-7043) Exporting com.codahale.metrics.MetricRegistry is breaking the abstraction
[ https://issues.apache.org/jira/browse/SLING-7043?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16123346#comment-16123346 ] Carsten Ziegeler edited comment on SLING-7043 at 8/11/17 1:37 PM: -- Why do we have sling commons metrics at all? was (Author: cziegeler): Why do we have the then sling commons metrics at all? > Exporting com.codahale.metrics.MetricRegistry is breaking the abstraction > -- > > Key: SLING-7043 > URL: https://issues.apache.org/jira/browse/SLING-7043 > Project: Sling > Issue Type: Bug >Affects Versions: Commons Metrics 1.0.0 >Reporter: Carsten Ziegeler >Priority: Blocker > Fix For: commons metrics 1.2.4 > > > commons metrics provides a nice abstraction over com.codahale.metrics - > however it is exporting com.codahale.metrics.MetricRegistry which seems to > be the only way to get at registered metrics objects. Which in turn is > completely breaking the purpose of this bundle. > So we should > a) drop exporting that service and avoid leaking internal implementation > details > b) create our own version of the registry service -- This message was sent by Atlassian JIRA (v6.4.14#64029)
