[PIE-1859] - remove metrics from plugin registration (#1918)

Remove metrics from plugin registration.  Doing it this way here always results in a
NoOpsMetrics being inited because CLI options have not been parsed.

Add an acceptance test for metrics.  This is what should have been in place and
would have detected metrics not working earlier.

Signed-off-by: Adrian Sutton <adrian.sutton@consensys.net>

acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/MetricsAcceptanceTest.java

@@ -0,0 +1,54 @@
+/*
+ * Copyright 2019 ConsenSys AG.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ */
+package tech.pegasys.pantheon.tests.acceptance;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import tech.pegasys.pantheon.tests.acceptance.dsl.AcceptanceTestBase;
+import tech.pegasys.pantheon.tests.acceptance.dsl.node.PantheonNode;
+import tech.pegasys.pantheon.tests.acceptance.dsl.node.configuration.PantheonNodeConfigurationBuilder;
+
+import java.io.IOException;
+
+import okhttp3.Call;
+import okhttp3.OkHttpClient;
+import okhttp3.Request;
+import okhttp3.Response;
+import org.junit.Before;
+import org.junit.Test;
+
+public class MetricsAcceptanceTest extends AcceptanceTestBase {
+
+  private PantheonNode metricsNode;
+  private OkHttpClient client;
+
+  @Before
+  public void setUp() throws Exception {
+    metricsNode =
+        pantheon.create(
+            new PantheonNodeConfigurationBuilder().name("metrics-node").metricsEnabled().build());
+    cluster.start(metricsNode);
+    client = new OkHttpClient();
+  }
+
+  @Test
+  public void metricsReporting() throws IOException {
+    assertThat(metricsNode.metricsHttpUrl()).isPresent();
+
+    final Call metricsRequest =
+        client.newCall(new Request.Builder().url(metricsNode.metricsHttpUrl().get()).build());
+    final Response response = metricsRequest.execute();
+
+    assertThat(response.body().string()).contains("# TYPE pantheon_peers_connected_total counter");
+  }
+}

acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/PantheonNode.java

@@ -171,6 +171,10 @@ public class PantheonNode implements NodeConfiguration, RunnableNode, AutoClosea
     return webSocketConfiguration().isEnabled();
   }
 
+  boolean isMetricsEnabled() {
+    return metricsConfiguration.isEnabled();
+  }
+
   @Override
   public String getName() {
     return name;
@@ -237,6 +241,19 @@ public class PantheonNode implements NodeConfiguration, RunnableNode, AutoClosea
     }
   }
 
+  public Optional<String> metricsHttpUrl() {
+    if (isMetricsEnabled()) {
+      return Optional.of(
+          "http://"
+              + metricsConfiguration.getHost()
+              + ":"
+              + portsProperties.getProperty("metrics")
+              + "/metrics");
+    } else {
+      return Optional.empty();
+    }
+  }
+
   @Override
   public Optional<Integer> getJsonRpcWebSocketPort() {
     if (isWebSocketsRpcEnabled()) {
@@ -470,7 +487,7 @@ public class PantheonNode implements NodeConfiguration, RunnableNode, AutoClosea
     return Optional.of(webSocketConfiguration().getPort());
   }
 
-  MetricsConfiguration metricsConfiguration() {
+  MetricsConfiguration getMetricsConfiguration() {
     return metricsConfiguration;
   }
 

acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/ProcessPantheonNodeRunner.java

@@ -18,6 +18,8 @@ import tech.pegasys.pantheon.cli.options.NetworkingOptions;
 import tech.pegasys.pantheon.ethereum.jsonrpc.RpcApi;
 import tech.pegasys.pantheon.ethereum.jsonrpc.RpcApis;
 import tech.pegasys.pantheon.ethereum.permissioning.PermissioningConfiguration;
+import tech.pegasys.pantheon.metrics.prometheus.MetricsConfiguration;
+import tech.pegasys.pantheon.plugin.services.metrics.MetricCategory;
 import tech.pegasys.pantheon.tests.acceptance.dsl.StaticNodesUtils;
 
 import java.io.BufferedReader;
@@ -140,6 +142,30 @@ public class ProcessPantheonNodeRunner implements PantheonNodeRunner {
       }
     }
 
+    if (node.isMetricsEnabled()) {
+      final MetricsConfiguration metricsConfiguration = node.getMetricsConfiguration();
+      params.add("--metrics-enabled");
+      params.add("--metrics-host");
+      params.add(metricsConfiguration.getHost());
+      params.add("--metrics-port");
+      params.add(Integer.toString(metricsConfiguration.getPort()));
+      for (final MetricCategory category : metricsConfiguration.getMetricCategories()) {
+        params.add("--metrics-category");
+        params.add(((Enum<?>) category).name());
+      }
+      if (metricsConfiguration.isPushEnabled()) {
+        params.add("--metrics-push-enabled");
+        params.add("--metrics-push-host");
+        params.add(metricsConfiguration.getPushHost());
+        params.add("--metrics-push-port");
+        params.add(Integer.toString(metricsConfiguration.getPushPort()));
+        params.add("--metrics-push-interval");
+        params.add(Integer.toString(metricsConfiguration.getPushInterval()));
+        params.add("--metrics-push-prometheus-job");
+        params.add(metricsConfiguration.getPrometheusJob());
+      }
+    }
+
     node.getGenesisConfig()
         .ifPresent(
             genesis -> {

acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/ThreadPantheonNodeRunner.java

@@ -27,7 +27,7 @@ import tech.pegasys.pantheon.ethereum.graphql.GraphQLConfiguration;
 import tech.pegasys.pantheon.ethereum.p2p.peers.EnodeURL;
 import tech.pegasys.pantheon.ethereum.permissioning.PermissioningConfiguration;
 import tech.pegasys.pantheon.metrics.ObservableMetricsSystem;
-import tech.pegasys.pantheon.metrics.noop.NoOpMetricsSystem;
+import tech.pegasys.pantheon.metrics.prometheus.PrometheusMetricsSystem;
 import tech.pegasys.pantheon.plugin.services.PantheonEvents;
 import tech.pegasys.pantheon.plugin.services.PicoCLIOptions;
 import tech.pegasys.pantheon.services.PantheonEventsImpl;
@@ -90,7 +90,8 @@ public class ThreadPantheonNodeRunner implements PantheonNodeRunner {
 
     commandLine.parseArgs(node.getConfiguration().getExtraCLIOptions().toArray(new String[0]));
 
-    final ObservableMetricsSystem noOpMetricsSystem = new NoOpMetricsSystem();
+    final ObservableMetricsSystem metricsSystem =
+        PrometheusMetricsSystem.init(node.getMetricsConfiguration());
     final List<EnodeURL> bootnodes =
         node.getConfiguration().getBootnodes().stream()
             .map(EnodeURL::fromURI)
@@ -113,7 +114,7 @@ public class ThreadPantheonNodeRunner implements PantheonNodeRunner {
               .miningParameters(node.getMiningParameters())
               .privacyParameters(node.getPrivacyParameters())
               .nodePrivateKeyFile(KeyPairUtil.getDefaultKeyFile(node.homeDirectory()))
-              .metricsSystem(noOpMetricsSystem)
+              .metricsSystem(metricsSystem)
               .transactionPoolConfiguration(TransactionPoolConfiguration.builder().build())
               .rocksDbConfiguration(RocksDbConfiguration.builder().databaseDir(tempDir).build())
               .ethProtocolConfiguration(EthProtocolConfiguration.defaultConfig())
@@ -152,8 +153,8 @@ public class ThreadPantheonNodeRunner implements PantheonNodeRunner {
             .jsonRpcConfiguration(node.jsonRpcConfiguration())
             .webSocketConfiguration(node.webSocketConfiguration())
             .dataDir(node.homeDirectory())
-            .metricsSystem(noOpMetricsSystem)
-            .metricsConfiguration(node.metricsConfiguration())
+            .metricsSystem(metricsSystem)
+            .metricsConfiguration(node.getMetricsConfiguration())
             .p2pEnabled(node.isP2pEnabled())
             .graphQLConfiguration(GraphQLConfiguration.createDefault())
             .staticNodes(

acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/configuration/PantheonNodeConfigurationBuilder.java

@@ -83,6 +83,17 @@ public class PantheonNodeConfigurationBuilder {
     return this;
   }
 
+  public PantheonNodeConfigurationBuilder metricsEnabled() {
+    this.metricsConfiguration =
+        MetricsConfiguration.builder()
+            .enabled(true)
+            .port(0)
+            .hostsWhitelist(singletonList("*"))
+            .build();
+
+    return this;
+  }
+
   public PantheonNodeConfigurationBuilder enablePrivateTransactions() {
     this.jsonRpcConfiguration.addRpcApi(RpcApis.EEA);
     this.jsonRpcConfiguration.addRpcApi(RpcApis.PRIV);

ethereum/jsonrpc/src/main/java/tech/pegasys/pantheon/ethereum/jsonrpc/internal/methods/NetServices.java

@@ -19,6 +19,7 @@ import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcResponse;
 import tech.pegasys.pantheon.ethereum.jsonrpc.internal.response.JsonRpcSuccessResponse;
 import tech.pegasys.pantheon.ethereum.jsonrpc.websocket.WebSocketConfiguration;
 import tech.pegasys.pantheon.ethereum.p2p.network.P2PNetwork;
+import tech.pegasys.pantheon.ethereum.p2p.peers.EnodeURL;
 import tech.pegasys.pantheon.metrics.prometheus.MetricsConfiguration;
 
 import com.google.common.collect.ImmutableMap;
@@ -65,7 +66,7 @@ public class NetServices implements JsonRpcMethod {
     if (p2pNetwork.isP2pEnabled()) {
       p2pNetwork
           .getLocalEnode()
-          .filter(e -> e.isListening())
+          .filter(EnodeURL::isListening)
           .ifPresent(
               enode ->
                   servicesMapBuilder.put(
@@ -76,7 +77,8 @@ public class NetServices implements JsonRpcMethod {
     if (metricsConfiguration.isEnabled()) {
       servicesMapBuilder.put(
           "metrics",
-          createServiceDetailsMap(metricsConfiguration.getHost(), metricsConfiguration.getPort()));
+          createServiceDetailsMap(
+              metricsConfiguration.getHost(), metricsConfiguration.getActualPort()));
     }
 
     return new JsonRpcSuccessResponse(req.getId(), servicesMapBuilder.build());

metrics/core/src/main/java/tech/pegasys/pantheon/metrics/prometheus/MetricsConfiguration.java

@@ -35,6 +35,7 @@ public class MetricsConfiguration {
 
   private final boolean enabled;
   private final int port;
+  private int actualPort;
   private final String host;
   private final Set<MetricCategory> metricCategories;
   private final boolean pushEnabled;
@@ -78,12 +79,20 @@ public class MetricsConfiguration {
     return enabled;
   }
 
+  public String getHost() {
+    return host;
+  }
+
   public int getPort() {
     return port;
   }
 
-  public String getHost() {
-    return host;
+  public int getActualPort() {
+    return actualPort;
+  }
+
+  void setActualPort(final int actualPort) {
+    this.actualPort = actualPort;
   }
 
   public Set<MetricCategory> getMetricCategories() {

metrics/core/src/main/java/tech/pegasys/pantheon/metrics/prometheus/MetricsHttpService.java

@@ -102,9 +102,11 @@ class MetricsHttpService implements MetricsService {
             res -> {
               if (!res.failed()) {
                 resultFuture.complete(null);
+                final int actualPort = httpServer.actualPort();
+                config.setActualPort(actualPort);
                 LOG.info(
                     "Metrics service started and listening on {}:{}",
-                    config.getHost(),
+                    actualPort,
                     httpServer.actualPort());
                 return;
               }
@@ -114,8 +116,11 @@ class MetricsHttpService implements MetricsService {
                 resultFuture.completeExceptionally(
                     new RuntimeException(
                         String.format(
-                            "Failed to bind metrics listener to %s:%s: %s",
-                            config.getHost(), config.getPort(), cause.getMessage())));
+                            "Failed to bind metrics listener to %s:%s (actual port %s): %s",
+                            config.getHost(),
+                            config.getPort(),
+                            config.getActualPort(),
+                            cause.getMessage())));
                 return;
               }
               resultFuture.completeExceptionally(cause);

pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java

@@ -784,7 +784,6 @@ public class PantheonCommand implements DefaultCommandValues, Runnable {
 
   private PantheonCommand preparePlugins() {
     pantheonPluginContext.addService(PicoCLIOptions.class, new PicoCLIOptionsImpl(commandLine));
-    pantheonPluginContext.addService(MetricsSystem.class, getMetricsSystem());
     pantheonPluginContext.registerPlugins(pluginsDir());
     return this;
   }