eolivelli commented on issue #840: ZOOKEEPER-3143 Pluggable metrics system for ZooKeeper - Data Collection on Server URL: https://github.com/apache/zookeeper/pull/840#issuecomment-471477973 @jhuan31 Thank you very much for paying attention to this patch, there is not stupid question. > Are we going to have a read API other than dump()? I see Counter has get() bug Summary doesn't. How are we supposed to read values of a summary? What is the purpose of such API, already Counter#get() should be dropped. I think that we you want to "test" that the code is tracking metrics "correctly" we should have some "TestableMetricsProvider" which tracks method calls or is able to report current values. We must not force a "MetricProvider" to be able to return current values by implementing the API, maybe it is using some external/remote service, so from an API/pluggability perspective we have to add to the API the minimum in order to let ZooKeeper report its own workfoad. Currently I had to put DefaultMetricsProvider (which is a wrapper around Facebook implementation) as default provider and to use it in tests in order to make the result as close as possible to the current behaviour. So this series of changes from my side is mostly a refactor, in order to preserve current behaviour and open the way a new way of integration with third party providers. I will do my best to be 100% compatible with you work, that is currently in production and it is working great. cc @lvfangmin @enixon
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
