Specify optional scheme (ws or wss) in Ethstats url (#5500)

* Allow scheme (ws or wss) to be specified in ethstats url
* Start ethstat service after main ethereum loop is up

Signed-off-by: Usman Saleem <usman@usmans.info>

CHANGELOG.md

@@ -5,7 +5,8 @@
 ### Breaking Changes
 
 ### Additions and Improvements
-
+- Allow Ethstats connection url to specify ws:// or wss:// scheme. [#5494](https://github.com/hyperledger/besu/issues/5494)
+- 
 ### Bug Fixes
 
 ### Download Links

besu/src/main/java/org/hyperledger/besu/Runner.java

@@ -154,6 +154,9 @@ public class Runner implements AutoCloseable {
             waitForServiceToStart(
                 "stratum", server.start().toCompletionStage().toCompletableFuture()));
     autoTransactionLogBloomCachingService.ifPresent(AutoTransactionLogBloomCachingService::start);
+  }
+
+  private void startExternalServicePostMainLoop() {
     ethStatsService.ifPresent(EthStatsService::start);
   }
 
@@ -174,6 +177,9 @@ public class Runner implements AutoCloseable {
       writeBesuPortsToFile();
       writeBesuNetworksToFile();
       writePidFile();
+
+      // start external service that depends on information from main loop
+      startExternalServicePostMainLoop();
     } catch (final Exception ex) {
       LOG.error("unable to start main loop", ex);
       throw new IllegalStateException("Startup failed", ex);

besu/src/main/java/org/hyperledger/besu/cli/options/stable/EthstatsOptions.java

@@ -33,8 +33,8 @@ public class EthstatsOptions implements CLIOptions<EthStatsConnectOptions> {
   @SuppressWarnings({"FieldCanBeFinal", "FieldMayBeFinal"})
   @CommandLine.Option(
       names = {ETHSTATS},
-      paramLabel = "<nodename:secret@host:port>",
-      description = "Reporting URL of a ethstats server",
+      paramLabel = "<[ws://|wss://]nodename:secret@host:[port]>",
+      description = "Reporting URL of a ethstats server. Scheme and port can be omitted.",
       arity = "1")
   private String ethstatsUrl = "";
 

ethereum/ethstats/src/main/java/org/hyperledger/besu/ethstats/EthStatsService.java

@@ -164,11 +164,16 @@ public class EthStatsService {
 
   private static WebSocketConnectOptions buildWebSocketConnectOptions(
       final EthStatsConnectOptions ethStatsConnectOptions) {
+    // if user specified scheme is null, default ssl to true, otherwise set ssl to true for wss
+    // scheme.
+    final boolean isSSL =
+        ethStatsConnectOptions.getScheme() == null
+            || ethStatsConnectOptions.getScheme().equalsIgnoreCase("wss");
     return new WebSocketConnectOptions()
         .setURI("/api")
-        .setSsl(true)
+        .setSsl(isSSL)
         .setHost(ethStatsConnectOptions.getHost())
-        .setPort(getWsPort(ethStatsConnectOptions, true));
+        .setPort(getWsPort(ethStatsConnectOptions, isSSL));
   }
 
   private static int getWsPort(
@@ -181,7 +186,7 @@ public class EthStatsService {
 
   /** Start. */
   public void start() {
-    LOG.debug("Connecting to EthStats: {}", getEthStatsHost());
+    LOG.debug("Connecting to EthStats: {}", getEthStatsURI());
     try {
       enodeURL = p2PNetwork.getLocalEnode().orElseThrow();
       vertx
@@ -229,7 +234,7 @@ public class EthStatsService {
     }
   }
 
-  private String getEthStatsHost() {
+  private String getEthStatsURI() {
     return String.format(
         "%s://%s:%s",
         webSocketConnectOptions.isSsl() ? "wss" : "ws",
@@ -237,11 +242,17 @@ public class EthStatsService {
         getWsPort(ethStatsConnectOptions, webSocketConnectOptions.isSsl()));
   }
 
-  /** Switch from ssl to non-ssl and vice-versa. Sets port to 443 or 80 if not specified. */
+  /**
+   * Switch from ssl to non-ssl and vice-versa if user specified scheme is null. Sets port to 443 or
+   * 80 if not specified.
+   */
   private void updateSSLProtocol() {
-    final boolean updatedSSL = !webSocketConnectOptions.isSsl();
-    webSocketConnectOptions.setSsl(updatedSSL);
-    webSocketConnectOptions.setPort(getWsPort(ethStatsConnectOptions, updatedSSL));
+    if (ethStatsConnectOptions.getScheme() == null) {
+      final boolean updatedSSL = !webSocketConnectOptions.isSsl();
+      webSocketConnectOptions.setSsl(updatedSSL);
+    }
+    webSocketConnectOptions.setPort(
+        getWsPort(ethStatsConnectOptions, webSocketConnectOptions.isSsl()));
   }
 
   /** Ends the current web socket connection, observers and schedulers */
@@ -259,6 +270,7 @@ public class EthStatsService {
     if (retryInProgress.getAndSet(true) == FALSE) {
       stop();
       updateSSLProtocol(); // switch from ssl:true to ssl:false and vice-versa
+      LOG.info("Attempting to reconnect to ethstats server in approximately 10 seconds.");
       protocolManager
           .ethContext()
           .getScheduler()

ethereum/ethstats/src/main/java/org/hyperledger/besu/ethstats/util/EthStatsConnectOptions.java

@@ -16,10 +16,8 @@ package org.hyperledger.besu.ethstats.util;
 
 import static com.google.common.base.Preconditions.checkArgument;
 
+import java.net.URI;
 import java.nio.file.Path;
-import java.util.Optional;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
 import javax.annotation.Nullable;
 
 import org.immutables.value.Value;
@@ -27,8 +25,8 @@ import org.slf4j.LoggerFactory;
 
 @Value.Immutable
 public interface EthStatsConnectOptions {
-
-  Pattern NETSTATS_URL_REGEX = Pattern.compile("([-\\w]+):([-\\w]+)?@([-.\\w]+)(:([\\d]+))?");
+  @Nullable
+  String getScheme();
 
   String getNodeName();
 
@@ -48,22 +46,48 @@ public interface EthStatsConnectOptions {
     try {
       checkArgument(url != null && !url.trim().isEmpty(), "Invalid empty value.");
 
-      final Matcher netStatsUrl = NETSTATS_URL_REGEX.matcher(url);
-      if (netStatsUrl.matches()) {
-        return ImmutableEthStatsConnectOptions.builder()
-            .nodeName(netStatsUrl.group(1))
-            .secret(netStatsUrl.group(2))
-            .host(netStatsUrl.group(3))
-            .port(Integer.parseInt(Optional.ofNullable(netStatsUrl.group(5)).orElse("-1")))
-            .contact(contact)
-            .caCert(caCert)
-            .build();
+      // if scheme is not specified in the URI, user info (nodename) gets converted to scheme.
+      final URI uri;
+      final String scheme;
+      if (url.matches("^.*://.*$")) {
+        // construct URI
+        uri = URI.create(url);
+        scheme = uri.getScheme();
+      } else {
+        // prepend ws:// to make a valid URI while keeping scheme as null
+        uri = URI.create("ws://" + url);
+        scheme = null;
+      }
+
+      if (scheme != null) {
+        // make sure that scheme is either ws or wss
+        if (!scheme.equalsIgnoreCase("ws") && !scheme.equalsIgnoreCase("wss")) {
+          throw new IllegalArgumentException("Ethstats URI only support ws:// or wss:// scheme.");
+        }
+      }
+
+      final String userInfo = uri.getUserInfo();
+
+      // make sure user info is specified
+      if (userInfo == null || !userInfo.contains(":")) {
+        throw new IllegalArgumentException("Ethstats URI missing user info.");
       }
+      final String nodeName = userInfo.substring(0, userInfo.indexOf(":"));
+      final String secret = userInfo.substring(userInfo.indexOf(":") + 1);
 
+      return ImmutableEthStatsConnectOptions.builder()
+          .scheme(scheme)
+          .nodeName(nodeName)
+          .secret(secret)
+          .host(uri.getHost())
+          .port(uri.getPort())
+          .contact(contact)
+          .caCert(caCert)
+          .build();
     } catch (IllegalArgumentException e) {
       LoggerFactory.getLogger(EthStatsConnectOptions.class).error(e.getMessage());
     }
     throw new IllegalArgumentException(
-        "Invalid netstats URL syntax. Netstats URL should have the following format 'nodename:secret@host:port' or 'nodename:secret@host'.");
+        "Invalid ethstats URL syntax. Ethstats URL should have the following format '[ws://|wss://]nodename:secret@host[:port]'.");
   }
 }

ethereum/ethstats/src/test/java/org/hyperledger/besu/ethstats/util/EthStatsConnectOptionsTest.java

@@ -30,13 +30,14 @@ public class EthStatsConnectOptionsTest {
   private final String CONTACT = "contact@mail.fr";
 
   private final String ERROR_MESSAGE =
-      "Invalid netstats URL syntax. Netstats URL should have the following format 'nodename:secret@host:port' or 'nodename:secret@host'.";
+      "Invalid ethstats URL syntax. Ethstats URL should have the following format '[ws://|wss://]nodename:secret@host[:port]'.";
 
   @Test
   public void buildWithValidParams() {
     final Path caCert = Path.of("./test.pem");
     final EthStatsConnectOptions ethStatsConnectOptions =
         EthStatsConnectOptions.fromParams(VALID_NETSTATS_URL, CONTACT, caCert);
+    assertThat(ethStatsConnectOptions.getScheme()).isNull();
     assertThat(ethStatsConnectOptions.getHost()).isEqualTo("127.0.0.1");
     assertThat(ethStatsConnectOptions.getNodeName()).isEqualTo("Dev-Node-1");
     assertThat(ethStatsConnectOptions.getPort()).isEqualTo(3001);
@@ -50,7 +51,9 @@ public class EthStatsConnectOptionsTest {
   public void buildWithValidHost(final String host) {
     final EthStatsConnectOptions ethStatsConnectOptions =
         EthStatsConnectOptions.fromParams("Dev-Node-1:secret@" + host + ":3001", CONTACT, null);
+    assertThat(ethStatsConnectOptions.getScheme()).isNull();
     assertThat(ethStatsConnectOptions.getHost()).isEqualTo(host);
+    assertThat(ethStatsConnectOptions.getPort()).isEqualTo(3001);
   }
 
   @ParameterizedTest(name = "#{index} - With Host {0}")
@@ -58,10 +61,34 @@ public class EthStatsConnectOptionsTest {
   public void buildWithValidHostWithoutPort(final String host) {
     final EthStatsConnectOptions ethStatsConnectOptions =
         EthStatsConnectOptions.fromParams("Dev-Node-1:secret@" + host, CONTACT, null);
+    assertThat(ethStatsConnectOptions.getScheme()).isNull();
     assertThat(ethStatsConnectOptions.getHost()).isEqualTo(host);
     assertThat(ethStatsConnectOptions.getPort()).isEqualTo(-1);
   }
 
+  @ParameterizedTest(name = "#{index} - With Scheme {0}")
+  @ValueSource(strings = {"ws", "wss", "WSS", "WS"})
+  public void buildWithValidScheme(final String scheme) {
+    final EthStatsConnectOptions ethStatsConnectOptions =
+        EthStatsConnectOptions.fromParams(
+            scheme + "://Dev-Node-1:secret@url-test.test.com:3001", CONTACT, null);
+    assertThat(ethStatsConnectOptions.getScheme()).isEqualTo(scheme);
+    assertThat(ethStatsConnectOptions.getHost()).isEqualTo("url-test.test.com");
+    assertThat(ethStatsConnectOptions.getPort()).isEqualTo(3001);
+  }
+
+  @ParameterizedTest(name = "#{index} - With Scheme {0}")
+  @ValueSource(strings = {"http", "https", "ftp"})
+  public void shouldRaiseErrorOnInvalidScheme(final String scheme) {
+    // missing node name
+    assertThatThrownBy(
+            () ->
+                EthStatsConnectOptions.fromParams(
+                    scheme + "://Dev-Node-1:secret@url-test.test.com:3001", CONTACT, null))
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessageEndingWith(ERROR_MESSAGE);
+  }
+
   @Test
   public void shouldDetectEmptyParams() {
     assertThatThrownBy(() -> EthStatsConnectOptions.fromParams("", CONTACT, null))
@@ -82,11 +109,10 @@ public class EthStatsConnectOptionsTest {
         .isInstanceOf(IllegalArgumentException.class)
         .hasMessageEndingWith(ERROR_MESSAGE);
 
-    // missing port
-    assertThatThrownBy(
-            () -> EthStatsConnectOptions.fromParams("Dev-Node-1:secret@127.0.0.1:", CONTACT, null))
-        .isInstanceOf(IllegalArgumentException.class)
-        .hasMessageEndingWith(ERROR_MESSAGE);
+    // missing port in URL should default to -1
+    EthStatsConnectOptions ethStatsConnectOptions =
+        EthStatsConnectOptions.fromParams("Dev-Node-1:secret@127.0.0.1:", CONTACT, null);
+    assertThat(ethStatsConnectOptions.getPort()).isEqualTo(-1);
   }
 
   @Test