Repackage options (#7688)

Updates MetricsOptions to implement CLIOptions, and to contain its unstable options. --------- Signed-off-by: Justin Florentine <justin+github@florentine.us>
hyperledger · Oct 4, 2024 · 911f12e · 911f12e
1 parent 49c6cbd
commit 911f12e
Show file tree

Hide file tree

Showing 17 changed files with 172 additions and 180 deletions.
diff --git a/...ance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/BesuNode.java b/...ance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/BesuNode.java
@@ -18,7 +18,7 @@
 import static org.apache.tuweni.io.file.Files.copyResource;
 
 import org.hyperledger.besu.cli.config.NetworkName;
-import org.hyperledger.besu.config.MergeConfigOptions;
+import org.hyperledger.besu.config.MergeConfiguration;
 import org.hyperledger.besu.crypto.KeyPair;
 import org.hyperledger.besu.crypto.KeyPairUtil;
 import org.hyperledger.besu.datatypes.Address;
@@ -228,7 +228,7 @@ public BesuNode(
         });
     this.requestedPlugins = requestedPlugins;
     engineRpcConfiguration.ifPresent(
-        config -> MergeConfigOptions.setMergeEnabled(config.isEnabled()));
+        config -> MergeConfiguration.setMergeEnabled(config.isEnabled()));
     this.extraCLIOptions = extraCLIOptions;
     this.staticNodes = staticNodes;
     this.isDnsEnabled = isDnsEnabled;

diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java
@@ -50,7 +50,7 @@
 import org.hyperledger.besu.cli.options.stable.GraphQlOptions;
 import org.hyperledger.besu.cli.options.stable.JsonRpcHttpOptions;
 import org.hyperledger.besu.cli.options.stable.LoggingLevelOption;
-import org.hyperledger.besu.cli.options.stable.MetricsOptionGroup;
+import org.hyperledger.besu.cli.options.stable.MetricsOptions;
 import org.hyperledger.besu.cli.options.stable.NodePrivateKeyFileOption;
 import org.hyperledger.besu.cli.options.stable.P2PDiscoveryOptions;
 import org.hyperledger.besu.cli.options.stable.PermissionsOptions;
@@ -62,7 +62,6 @@
 import org.hyperledger.besu.cli.options.unstable.EvmOptions;
 import org.hyperledger.besu.cli.options.unstable.InProcessRpcOptions;
 import org.hyperledger.besu.cli.options.unstable.IpcOptions;
-import org.hyperledger.besu.cli.options.unstable.MetricsCLIOptions;
 import org.hyperledger.besu.cli.options.unstable.NatOptions;
 import org.hyperledger.besu.cli.options.unstable.NativeLibraryOptions;
 import org.hyperledger.besu.cli.options.unstable.NetworkingOptions;
@@ -89,7 +88,7 @@
 import org.hyperledger.besu.config.CheckpointConfigOptions;
 import org.hyperledger.besu.config.GenesisConfigFile;
 import org.hyperledger.besu.config.GenesisConfigOptions;
-import org.hyperledger.besu.config.MergeConfigOptions;
+import org.hyperledger.besu.config.MergeConfiguration;
 import org.hyperledger.besu.controller.BesuController;
 import org.hyperledger.besu.controller.BesuControllerBuilder;
 import org.hyperledger.besu.crypto.Blake2bfMessageDigest;
@@ -246,7 +245,6 @@
 import io.vertx.core.Vertx;
 import io.vertx.core.VertxOptions;
 import io.vertx.core.json.DecodeException;
-import io.vertx.core.metrics.MetricsOptions;
 import org.apache.tuweni.bytes.Bytes;
 import org.apache.tuweni.units.bigints.UInt256;
 import org.slf4j.Logger;
@@ -295,7 +293,6 @@ public class BesuCommand implements DefaultCommandValues, Runnable {
   final NetworkingOptions unstableNetworkingOptions = NetworkingOptions.create();
   final SynchronizerOptions unstableSynchronizerOptions = SynchronizerOptions.create();
   final EthProtocolOptions unstableEthProtocolOptions = EthProtocolOptions.create();
-  final MetricsCLIOptions unstableMetricsCLIOptions = MetricsCLIOptions.create();
   private final DnsOptions unstableDnsOptions = DnsOptions.create();
   private final NatOptions unstableNatOptions = NatOptions.create();
   private final NativeLibraryOptions unstableNativeLibraryOptions = NativeLibraryOptions.create();
@@ -584,7 +581,7 @@ static class PrivacyOptionGroup {
 
   // Metrics Option Group
   @CommandLine.ArgGroup(validate = false, heading = "@|bold Metrics Options|@%n")
-  MetricsOptionGroup metricsOptionGroup = new MetricsOptionGroup();
+  MetricsOptions metricsOptions = MetricsOptions.create();
 
   @Option(
       names = {"--host-allowlist"},
@@ -1153,7 +1150,6 @@ private void handleUnstableOptions() {
     final ImmutableMap<String, Object> unstableOptions =
         unstableOptionsBuild
             .put("Ethereum Wire Protocol", unstableEthProtocolOptions)
-            .put("Metrics", unstableMetricsCLIOptions)
             .put("P2P Network", unstableNetworkingOptions)
             .put("RPC", unstableRPCOptions)
             .put("DNS Configuration", unstableDnsOptions)
@@ -1864,7 +1860,7 @@ private void checkPrivacyTlsOptionsDependencies() {
    * @return instance of MetricsConfiguration.
    */
   public MetricsConfiguration metricsConfiguration() {
-    if (metricsOptionGroup.getMetricsEnabled() && metricsOptionGroup.getMetricsPushEnabled()) {
+    if (metricsOptions.getMetricsEnabled() && metricsOptions.getMetricsPushEnabled()) {
       throw new ParameterException(
           this.commandLine,
           "--metrics-enabled option and --metrics-push-enabled option can't be used at the same "
@@ -1875,40 +1871,33 @@ public MetricsConfiguration metricsConfiguration() {
         logger,
         commandLine,
         "--metrics-enabled",
-        !metricsOptionGroup.getMetricsEnabled(),
+        !metricsOptions.getMetricsEnabled(),
         asList("--metrics-host", "--metrics-port"));
 
     CommandLineUtils.checkOptionDependencies(
         logger,
         commandLine,
         "--metrics-push-enabled",
-        !metricsOptionGroup.getMetricsPushEnabled(),
+        !metricsOptions.getMetricsPushEnabled(),
         asList(
             "--metrics-push-host",
             "--metrics-push-port",
             "--metrics-push-interval",
             "--metrics-push-prometheus-job"));
 
-    return unstableMetricsCLIOptions
-        .toDomainObject()
-        .enabled(metricsOptionGroup.getMetricsEnabled())
+    final MetricsConfiguration.Builder metricsConfigurationBuilder =
+        metricsOptions.toDomainObject();
+    metricsConfigurationBuilder
         .host(
-            Strings.isNullOrEmpty(metricsOptionGroup.getMetricsHost())
+            Strings.isNullOrEmpty(metricsOptions.getMetricsHost())
                 ? p2PDiscoveryOptions.p2pHost
-                : metricsOptionGroup.getMetricsHost())
-        .port(metricsOptionGroup.getMetricsPort())
-        .protocol(metricsOptionGroup.getMetricsProtocol())
-        .metricCategories(metricsOptionGroup.getMetricCategories())
-        .pushEnabled(metricsOptionGroup.getMetricsPushEnabled())
+                : metricsOptions.getMetricsHost())
         .pushHost(
-            Strings.isNullOrEmpty(metricsOptionGroup.getMetricsPushHost())
+            Strings.isNullOrEmpty(metricsOptions.getMetricsPushHost())
                 ? p2PDiscoveryOptions.autoDiscoverDefaultIP().getHostAddress()
-                : metricsOptionGroup.getMetricsPushHost())
-        .pushPort(metricsOptionGroup.getMetricsPushPort())
-        .pushInterval(metricsOptionGroup.getMetricsPushInterval())
-        .hostsAllowlist(hostsAllowlist)
-        .prometheusJob(metricsOptionGroup.getMetricsPrometheusJob())
-        .build();
+                : metricsOptions.getMetricsPushHost())
+        .hostsAllowlist(hostsAllowlist);
+    return metricsConfigurationBuilder.build();
   }
 
   private PrivacyParameters privacyParameters() {
@@ -2281,7 +2270,7 @@ private VertxOptions createVertxOptions(final MetricsSystem metricsSystem) {
     return new VertxOptions()
         .setPreferNativeTransport(true)
         .setMetricsOptions(
-            new MetricsOptions()
+            new io.vertx.core.metrics.MetricsOptions()
                 .setEnabled(true)
                 .setFactory(new VertxMetricsAdapterFactory(metricsSystem)));
   }
@@ -2537,9 +2526,7 @@ private List<Integer> getEffectivePorts() {
         effectivePorts, rpcWebsocketOptions.getRpcWsPort(), rpcWebsocketOptions.isRpcWsEnabled());
     addPortIfEnabled(effectivePorts, engineRPCConfig.engineRpcPort(), isEngineApiEnabled());
     addPortIfEnabled(
-        effectivePorts,
-        metricsOptionGroup.getMetricsPort(),
-        metricsOptionGroup.getMetricsEnabled());
+        effectivePorts, metricsOptions.getMetricsPort(), metricsOptions.getMetricsEnabled());
     addPortIfEnabled(
         effectivePorts,
         miningParametersSupplier.get().getStratumPort(),
@@ -2613,7 +2600,7 @@ protected GenesisConfigOptions getGenesisConfigOptions() {
   }
 
   private void setMergeConfigOptions() {
-    MergeConfigOptions.setMergeEnabled(
+    MergeConfiguration.setMergeEnabled(
         genesisConfigOptionsSupplier.get().getTerminalTotalDifficulty().isPresent());
   }
 
@@ -2658,7 +2645,7 @@ private void validatePostMergeCheckpointBlockRequirements() {
   }
 
   private boolean isMergeEnabled() {
-    return MergeConfigOptions.isMergeEnabled();
+    return MergeConfiguration.isMergeEnabled();
   }
 
   private boolean isEngineApiEnabled() {
@@ -2752,11 +2739,20 @@ && getDataStorageConfiguration().getBonsaiLimitTrieLogsEnabled()) {
   }
 
   /**
-   * Returns the plugin context.
+   * 2 Returns the plugin context.
    *
    * @return the plugin context.
    */
   public BesuPluginContextImpl getBesuPluginContext() {
     return besuPluginContext;
   }
+
+  /**
+   * Returns the metrics options
+   *
+   * @return the metrics options
+   */
+  public MetricsOptions getMetricsOptions() {
+    return metricsOptions;
+  }
 }
diff --git a/...li/options/stable/MetricsOptionGroup.java → ...su/cli/options/stable/MetricsOptions.java b/...li/options/stable/MetricsOptionGroup.java → ...su/cli/options/stable/MetricsOptions.java
@@ -22,20 +22,80 @@
 import static org.hyperledger.besu.metrics.prometheus.MetricsConfiguration.DEFAULT_METRICS_PORT;
 import static org.hyperledger.besu.metrics.prometheus.MetricsConfiguration.DEFAULT_METRICS_PUSH_PORT;
 
+import org.hyperledger.besu.cli.options.CLIOptions;
+import org.hyperledger.besu.cli.util.CommandLineUtils;
 import org.hyperledger.besu.metrics.MetricsProtocol;
+import org.hyperledger.besu.metrics.prometheus.MetricsConfiguration;
 import org.hyperledger.besu.plugin.services.metrics.MetricCategory;
 
+import java.util.List;
 import java.util.Set;
 
 import picocli.CommandLine;
 
 /** Command line options for configuring metrics. */
-// TODO: implement CLIOption<MetricsConfiguration> and rename to drop the Group
-public class MetricsOptionGroup {
+// TODO: implement CLIOption<MetricsConfiguration>
+public class MetricsOptions implements CLIOptions<MetricsConfiguration.Builder> {
+
+  /**
+   * Returns a MetricsConfiguration.Builder because fields are often overridden from other domains,
+   * like using P2P settings as defaults.
+   *
+   * @return a newly created {@link MetricsOptions} with default values
+   */
+  @Override
+  public MetricsConfiguration.Builder toDomainObject() {
+    MetricsConfiguration.Builder builder = MetricsConfiguration.builder();
+    builder
+        .timersEnabled(unstableOptions.timersEnabled)
+        .idleTimeout(unstableOptions.idleTimeout)
+        .enabled(getMetricsEnabled())
+        .host(getMetricsHost())
+        .port(getMetricsPort())
+        .protocol(getMetricsProtocol())
+        .metricCategories(getMetricCategories())
+        .pushEnabled(getMetricsPushEnabled())
+        .pushHost(getMetricsPushHost())
+        .pushPort(getMetricsPushPort())
+        .pushInterval(getMetricsPushInterval())
+        .prometheusJob(getMetricsPrometheusJob());
+    return builder;
+  }
+
+  // TODO: why do we need to be able to reverse this?
+  /**
+   * Returns a newly created {@link MetricsOptions} reversed from the supplied MetricsConfiguration
+   *
+   * @param config the metrics configuration
+   * @return a newly created {@link MetricsOptions} reversed from the supplied MetricsConfiguration
+   */
+  public static MetricsOptions fromConfiguration(final MetricsConfiguration config) {
+    final MetricsOptions metricsOptions = create();
+    metricsOptions.unstableOptions.timersEnabled = config.isTimersEnabled();
+    metricsOptions.unstableOptions.idleTimeout = config.getIdleTimeout();
+    metricsOptions.isMetricsEnabled = config.isEnabled();
+    metricsOptions.metricsHost = config.getHost();
+    metricsOptions.metricsPort = config.getPort();
+    metricsOptions.metricsProtocol = config.getProtocol();
+    metricsOptions.metricCategories = config.getMetricCategories();
+    metricsOptions.metricsPrometheusJob = config.getPrometheusJob();
+    metricsOptions.isMetricsPushEnabled = config.isPushEnabled();
+    metricsOptions.metricsPushHost = config.getPushHost();
+    metricsOptions.metricsPushPort = config.getPushPort();
+    metricsOptions.metricsPushInterval = config.getPushInterval();
+
+    return metricsOptions;
+  }
+
+  @Override
+  public List<String> getCLIOptions() {
+    return CommandLineUtils.getCLIOptions(this, new MetricsOptions());
+  }
+
   @CommandLine.Option(
       names = {"--metrics-enabled"},
       description = "Set to start the metrics exporter (default: ${DEFAULT-VALUE})")
-  private final Boolean isMetricsEnabled = false;
+  private Boolean isMetricsEnabled = false;
 
   @SuppressWarnings({"FieldCanBeFinal", "FieldMayBeFinal"}) // PicoCLI requires non-final Strings.
   @CommandLine.Option(
@@ -50,14 +110,14 @@ public class MetricsOptionGroup {
       paramLabel = MANDATORY_HOST_FORMAT_HELP,
       description = "Host for the metrics exporter to listen on (default: ${DEFAULT-VALUE})",
       arity = "1")
-  private String metricsHost;
+  private String metricsHost = MetricsConfiguration.DEFAULT_METRICS_HOST;
 
   @CommandLine.Option(
       names = {"--metrics-port"},
       paramLabel = MANDATORY_PORT_FORMAT_HELP,
       description = "Port for the metrics exporter to listen on (default: ${DEFAULT-VALUE})",
       arity = "1")
-  private final Integer metricsPort = DEFAULT_METRICS_PORT;
+  private Integer metricsPort = DEFAULT_METRICS_PORT;
 
   @CommandLine.Option(
       names = {"--metrics-category", "--metrics-categories"},
@@ -66,35 +126,35 @@ public class MetricsOptionGroup {
       arity = "1..*",
       description =
           "Comma separated list of categories to track metrics for (default: ${DEFAULT-VALUE})")
-  private final Set<MetricCategory> metricCategories = DEFAULT_METRIC_CATEGORIES;
+  private Set<MetricCategory> metricCategories = DEFAULT_METRIC_CATEGORIES;
 
   @CommandLine.Option(
       names = {"--metrics-push-enabled"},
       description = "Enable the metrics push gateway integration (default: ${DEFAULT-VALUE})")
-  private final Boolean isMetricsPushEnabled = false;
+  private Boolean isMetricsPushEnabled = false;
 
   @SuppressWarnings({"FieldCanBeFinal", "FieldMayBeFinal"}) // PicoCLI requires non-final Strings.
   @CommandLine.Option(
       names = {"--metrics-push-host"},
       paramLabel = MANDATORY_HOST_FORMAT_HELP,
       description = "Host of the Prometheus Push Gateway for push mode (default: ${DEFAULT-VALUE})",
       arity = "1")
-  private String metricsPushHost;
+  private String metricsPushHost = MetricsConfiguration.DEFAULT_METRICS_PUSH_HOST;
 
   @CommandLine.Option(
       names = {"--metrics-push-port"},
       paramLabel = MANDATORY_PORT_FORMAT_HELP,
       description = "Port of the Prometheus Push Gateway for push mode (default: ${DEFAULT-VALUE})",
       arity = "1")
-  private final Integer metricsPushPort = DEFAULT_METRICS_PUSH_PORT;
+  private Integer metricsPushPort = DEFAULT_METRICS_PUSH_PORT;
 
   @CommandLine.Option(
       names = {"--metrics-push-interval"},
       paramLabel = MANDATORY_INTEGER_FORMAT_HELP,
       description =
           "Interval in seconds to push metrics when in push mode (default: ${DEFAULT-VALUE})",
       arity = "1")
-  private final Integer metricsPushInterval = 15;
+  private Integer metricsPushInterval = 15;
 
   @SuppressWarnings({"FieldCanBeFinal", "FieldMayBeFinal"}) // PicoCLI requires non-final Strings.
   @CommandLine.Option(
@@ -103,8 +163,16 @@ public class MetricsOptionGroup {
       arity = "1")
   private String metricsPrometheusJob = "besu-client";
 
-  /** Returns a newly created {@link MetricsOptionGroup} with default values. */
-  public MetricsOptionGroup() {}
+  /**
+   * Returns a newly created {@link MetricsOptions} with default values.
+   *
+   * @return new instance
+   */
+  public static MetricsOptions create() {
+    return new MetricsOptions();
+  }
+
+  private MetricsOptions() {}
 
   /**
    * Returns whether metrics are enabled.
@@ -195,4 +263,26 @@ public Integer getMetricsPushInterval() {
   public String getMetricsPrometheusJob() {
     return metricsPrometheusJob;
   }
+
+  @CommandLine.ArgGroup(validate = false)
+  private final MetricsOptions.Unstable unstableOptions = new MetricsOptions.Unstable();
+
+  static class Unstable {
+    private static final String TIMERS_ENABLED_FLAG = "--Xmetrics-timers-enabled";
+    private static final String IDLE_TIMEOUT_FLAG = "--Xmetrics-idle-timeout";
+
+    @CommandLine.Option(
+        names = TIMERS_ENABLED_FLAG,
+        hidden = true,
+        description = "Whether to enable timer metrics (default: ${DEFAULT-VALUE}).")
+    private Boolean timersEnabled = MetricsConfiguration.DEFAULT_METRICS_TIMERS_ENABLED;
+
+    @CommandLine.Option(
+        hidden = true,
+        names = {IDLE_TIMEOUT_FLAG},
+        paramLabel = "<INTEGER>",
+        description = "Timeout for metrics TCP connections, in seconds (default: ${DEFAULT-VALUE})",
+        arity = "1")
+    private int idleTimeout = MetricsConfiguration.DEFAULT_METRICS_IDLE_TIMEOUT_SECONDS;
+  }
 }