[GitHub] zookeeper issue #582: ZOOKEEPER-3103 Pluggable metrics system for ZooKeeper ...

2018-08-14 Thread eolivelli
Github user eolivelli commented on the issue: https://github.com/apache/zookeeper/pull/582 Cool. Will follow up with the next part. Thank you ---

[GitHub] zookeeper issue #582: ZOOKEEPER-3103 Pluggable metrics system for ZooKeeper ...

2018-08-14 Thread anmolnar
Github user anmolnar commented on the issue: https://github.com/apache/zookeeper/pull/582 Committed to master with 2 approvals. Thanks @eolivelli ---

[GitHub] zookeeper issue #582: ZOOKEEPER-3103 Pluggable metrics system for ZooKeeper ...

2018-08-14 Thread eolivelli
Github user eolivelli commented on the issue: https://github.com/apache/zookeeper/pull/582 Btw I think this is good to go now ---

[GitHub] zookeeper issue #582: ZOOKEEPER-3103 Pluggable metrics system for ZooKeeper ...

2018-08-14 Thread eolivelli
Github user eolivelli commented on the issue: https://github.com/apache/zookeeper/pull/582 @anmolnar I have updated the docs. Using ant javadoc will not produce an usefull javadoc site, most package will be empty. Which is the command to use to build the full docs ? ---

[GitHub] zookeeper issue #582: ZOOKEEPER-3103 Pluggable metrics system for ZooKeeper ...

2018-08-14 Thread anmolnar
Github user anmolnar commented on the issue: https://github.com/apache/zookeeper/pull/582 Thanks @eolivelli . It makes perfect sense to me now. Adding more docs (especially javadoc in this case) is always a good idea. Copying the above description to javadoc is sufficient I

[GitHub] zookeeper issue #582: ZOOKEEPER-3103 Pluggable metrics system for ZooKeeper ...

2018-08-14 Thread eolivelli
Github user eolivelli commented on the issue: https://github.com/apache/zookeeper/pull/582 I will add more docs. Seems that we are agreeing on this way. I will send a new patch based on this branch (or on master if we merge this patch) New step is about booting a

[GitHub] zookeeper issue #582: ZOOKEEPER-3103 Pluggable metrics system for ZooKeeper ...

2018-08-13 Thread eolivelli
Github user eolivelli commented on the issue: https://github.com/apache/zookeeper/pull/582 @anmolnar you are right. MetricsContext will be like a namespace, I expect that each component/submodule will have its own MetricsContext. This is useful for two reasons/usecases.

[GitHub] zookeeper issue #582: ZOOKEEPER-3103 Pluggable metrics system for ZooKeeper ...

2018-08-13 Thread eolivelli
Github user eolivelli commented on the issue: https://github.com/apache/zookeeper/pull/582 @anmolnar you are right. MetricsContext will be like a namespace, I expect that each component/submodule will have its own MetricsContext. This is useful for two reasons/usecases.

[GitHub] zookeeper issue #582: ZOOKEEPER-3103 Pluggable metrics system for ZooKeeper ...

2018-08-13 Thread anmolnar
Github user anmolnar commented on the issue: https://github.com/apache/zookeeper/pull/582 Thanks @eolivelli, I got that. However, what I'm interested in is slightly different. :) Let me try to rephrase it: I'd like to have an idea of what the different interfaces that

[GitHub] zookeeper issue #582: ZOOKEEPER-3103 Pluggable metrics system for ZooKeeper ...

2018-08-10 Thread eolivelli
Github user eolivelli commented on the issue: https://github.com/apache/zookeeper/pull/582 @anmolnar thank you for following up on this. Background of this issue is on JIRA https://issues.apache.org/jira/browse/ZOOKEEPER-3092 the plan is the following: - send

[GitHub] zookeeper issue #582: ZOOKEEPER-3103 Pluggable metrics system for ZooKeeper ...

2018-08-09 Thread anmolnar
Github user anmolnar commented on the issue: https://github.com/apache/zookeeper/pull/582 Sorry @eolivelli I really don't want to block this patch, but I'd like to get a better understanding on the roles of the classes/interfaces here. - What's the original concept that you're

[GitHub] zookeeper issue #582: ZOOKEEPER-3103 Pluggable metrics system for ZooKeeper ...

2018-08-09 Thread eolivelli
Github user eolivelli commented on the issue: https://github.com/apache/zookeeper/pull/582 Shall we commit this patch ? So we can make little steps forward. This change does not impact how zk works, it is safe ---

[GitHub] zookeeper issue #582: ZOOKEEPER-3103 Pluggable metrics system for ZooKeeper ...

2018-08-08 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/582 +1 Thanks @eolivelli. The new change looks good to me. ---

[GitHub] zookeeper issue #582: ZOOKEEPER-3103 Pluggable metrics system for ZooKeeper ...

2018-08-06 Thread eolivelli
Github user eolivelli commented on the issue: https://github.com/apache/zookeeper/pull/582 @anmolnar that you for catching those file, I did a 'git add .' too aggressively. @lvfangmin I have addressed you comments ---

[GitHub] zookeeper issue #582: ZOOKEEPER-3103 Pluggable metrics system for ZooKeeper ...

2018-07-31 Thread anmolnar
Github user anmolnar commented on the issue: https://github.com/apache/zookeeper/pull/582 @eolivelli I'll take a look today. ---

[GitHub] zookeeper issue #582: ZOOKEEPER-3103 Pluggable metrics system for ZooKeeper ...

2018-07-31 Thread eolivelli
Github user eolivelli commented on the issue: https://github.com/apache/zookeeper/pull/582 @hanm @anmolnar do you have cycles for review? If this approach is good I can send the second part, about booting the provider in the server and providing some sample instrumentation ---

[GitHub] zookeeper issue #582: ZOOKEEPER-3103 Pluggable metrics system for ZooKeeper ...

2018-07-28 Thread eolivelli
Github user eolivelli commented on the issue: https://github.com/apache/zookeeper/pull/582 @pravsingh thank you for taking a look. We are going to design a zookeeper proprierary internal API mostly because we want the project to be owner of its own internal instrumentation system.

[GitHub] zookeeper issue #582: ZOOKEEPER-3103 Pluggable metrics system for ZooKeeper ...

2018-07-25 Thread eolivelli
Github user eolivelli commented on the issue: https://github.com/apache/zookeeper/pull/582 @lvfangmin > One question is are we just publishing the raw data to the external metric report system, or we need to do our aggregation to find out values like min/avg/max? >

[GitHub] zookeeper issue #582: ZOOKEEPER-3103 Pluggable metrics system for ZooKeeper ...

2018-07-25 Thread lvfangmin
Github user lvfangmin commented on the issue: https://github.com/apache/zookeeper/pull/582 One question is are we just publishing the raw data to the external metric report system, or we need to do our aggregation to find out values like min/avg/max? If we need to aggregate

[GitHub] zookeeper issue #582: ZOOKEEPER-3103 Pluggable metrics system for ZooKeeper ...

2018-07-24 Thread hanm
Github user hanm commented on the issue: https://github.com/apache/zookeeper/pull/582 thanks for quick work @eolivelli . I'll have a more close look on this tomorrow. Just quick comment - I am not sure if you can kick off jenkins by just add a comment. You can kick jenkins

[GitHub] zookeeper issue #582: ZOOKEEPER-3103 Pluggable metrics system for ZooKeeper ...

2018-07-24 Thread eolivelli
Github user eolivelli commented on the issue: https://github.com/apache/zookeeper/pull/582 retest this please ---