ZOOKEEPER-3366: Pluggable metrics system for ZooKeeper - move remaining metrics to MetricsProvider

Migrate all remaining metrics to MetricsProvider.
We are introducing now *Gauges* which are callbacks to be called when the Provider needs to publish current values, a Gauge is a numeric value that can go up and down.

As during the lifecycle of a ZK server process we can have several ZooKeeperServer instances (we have several subclasses), depending on the role of the local peer, sometimes we have to clean up unused Gauges.
The old approach  in 4lw and on http admin  API was to hard code metrics, with multiple 'instanceof' conditions.
So we introduce ZooKeeperServer#registerMetrics and ZooKeeperServer#unregisterMetrics: these overridable functions enable each  ZooKeeperServer subclass to declare specific Gauges.
We are also introducing ZooKeeperServer#collectMonitorValues in order to push non-metrics to monitor commands (admin and 4lw), like "server state" or "version".

Author: Enrico Olivelli <eolivelli@apache.org>

Reviewers: Norbert Kalmar <nkalmar@apache.org>

Closes #918 from eolivelli/fix/other-metrics

3 files changed, 19 insertions, 14 deletions

diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/metrics/BaseTestMetricsProvider.java b/zookeeper-server/src/test/java/org/apache/zookeeper/metrics/BaseTestMetricsProvider.java
index 7c6e1ae6f..3e2301997 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/metrics/BaseTestMetricsProvider.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/metrics/BaseTestMetricsProvider.java
@@ -85,10 +85,8 @@ public abstract class BaseTestMetricsProvider implements MetricsProvider {
 
         @Override
         public MetricsContext getRootContext() {
-            if (!getRootContextCalled.compareAndSet(false, true)) {
-                // called twice
-                throw new IllegalStateException();
-            }
+            getRootContextCalled.set(true);
+
             return NullMetricsProvider.NullMetricsContext.INSTANCE;
         }
 
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/admin/CommandsTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/admin/CommandsTest.java
index f8813d006..be803c0e0 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/admin/CommandsTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/admin/CommandsTest.java
@@ -202,6 +202,15 @@ public class CommandsTest extends ClientBase {
         Map<String, Object> metrics = MetricsUtils.currentServerMetrics();
         
         for (String metric : metrics.keySet()) {
+            boolean alreadyDefined = fields
+                    .stream()
+                    .anyMatch(f -> {
+                        return f.key.equals(metric);
+                    });
+            if (alreadyDefined) {
+                // known metrics are defined statically in the block above
+                continue;
+            }
             if (metric.startsWith("avg_")) {
                 fields.add(new Field(metric, Double.class));  
             } else {
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/test/ObserverMasterTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/test/ObserverMasterTest.java
index 4e07d4875..f72063418 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/test/ObserverMasterTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/test/ObserverMasterTest.java
@@ -516,34 +516,32 @@ public class ObserverMasterTest extends QuorumPeerTestBase implements Watcher{
         Assert.assertTrue("observer not emitting observer_master_id", stats.containsKey("observer_master_id"));
 
         // check the stats for the first peer
-        stats = Commands.runCommand("mntr", q1.getQuorumPeer().getActiveServer(), emptyMap).toMap();
         if (testObserverMaster) {
             if (q1.getQuorumPeer().leader == null) {
-                Assert.assertEquals(1, stats.get("synced_observers"));
+                Assert.assertEquals(Integer.valueOf(1), q1.getQuorumPeer().getSynced_observers_metric());
             } else {
-                Assert.assertEquals(0, stats.get("synced_observers"));
+                Assert.assertEquals(Integer.valueOf(0), q1.getQuorumPeer().getSynced_observers_metric());
             }
         } else {
             if (q1.getQuorumPeer().leader == null) {
-                Assert.assertNull(stats.get("synced_observers"));
+                Assert.assertNull(q1.getQuorumPeer().getSynced_observers_metric());
             } else {
-                Assert.assertEquals(1, stats.get("synced_observers"));
+                Assert.assertEquals(Integer.valueOf(1), q1.getQuorumPeer().getSynced_observers_metric());
             }
         }
 
         // check the stats for the second peer
-        stats = Commands.runCommand("mntr", q2.getQuorumPeer().getActiveServer(), emptyMap).toMap();
         if (testObserverMaster) {
             if (q2.getQuorumPeer().leader == null) {
-                Assert.assertEquals(1, stats.get("synced_observers"));
+                Assert.assertEquals(Integer.valueOf(1), q2.getQuorumPeer().getSynced_observers_metric());
             } else {
-                Assert.assertEquals(0, stats.get("synced_observers"));
+                Assert.assertEquals(Integer.valueOf(0), q2.getQuorumPeer().getSynced_observers_metric());
             }
         } else {
             if (q2.getQuorumPeer().leader == null) {
-                Assert.assertNull(stats.get("synced_observers"));
+                Assert.assertNull(q2.getQuorumPeer().getSynced_observers_metric());
             } else {
-                Assert.assertEquals(1, stats.get("synced_observers"));
+                Assert.assertEquals(Integer.valueOf(1), q2.getQuorumPeer().getSynced_observers_metric());
             }
         }