From 41ece1de64515e1875622c1667ece4ccf6ca9f6d Mon Sep 17 00:00:00 2001 From: mbaxter Date: Fri, 9 Aug 2019 16:41:57 -0400 Subject: [PATCH] [PIE-1846] Add an experimental flag for disabling timers (#1837) Signed-off-by: Adrian Sutton --- .../prometheus/MetricsConfiguration.java | 19 ++++++- .../prometheus/PrometheusMetricsSystem.java | 10 ++-- .../PrometheusMetricsSystemTest.java | 15 +++++- .../pegasys/pantheon/cli/PantheonCommand.java | 30 ++++++----- .../cli/options/MetricsCLIOptions.java | 53 +++++++++++++++++++ .../pantheon/cli/CommandTestAbstract.java | 5 ++ .../cli/options/MetricsCLIOptionsTest.java | 40 ++++++++++++++ 7 files changed, 152 insertions(+), 20 deletions(-) create mode 100644 pantheon/src/main/java/tech/pegasys/pantheon/cli/options/MetricsCLIOptions.java create mode 100644 pantheon/src/test/java/tech/pegasys/pantheon/cli/options/MetricsCLIOptionsTest.java diff --git a/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/prometheus/MetricsConfiguration.java b/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/prometheus/MetricsConfiguration.java index b859ff916c..f5a2b7b8f8 100644 --- a/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/prometheus/MetricsConfiguration.java +++ b/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/prometheus/MetricsConfiguration.java @@ -31,6 +31,7 @@ public class MetricsConfiguration { private static final String DEFAULT_METRICS_PUSH_HOST = "127.0.0.1"; public static final int DEFAULT_METRICS_PUSH_PORT = 9001; + public static final Boolean DEFAULT_TIMERS_ENABLED = true; private final boolean enabled; private final int port; @@ -42,6 +43,7 @@ public class MetricsConfiguration { private final int pushInterval; private final String prometheusJob; private final List hostsWhitelist; + private final boolean timersEnabled; public static Builder builder() { return new Builder(); @@ -57,7 +59,8 @@ public class MetricsConfiguration { final String pushHost, final int pushInterval, final String prometheusJob, - final List hostsWhitelist) { + final List hostsWhitelist, + final boolean timersEnabled) { this.enabled = enabled; this.port = port; this.host = host; @@ -68,6 +71,7 @@ public class MetricsConfiguration { this.pushInterval = pushInterval; this.prometheusJob = prometheusJob; this.hostsWhitelist = hostsWhitelist; + this.timersEnabled = timersEnabled; } public boolean isEnabled() { @@ -110,6 +114,10 @@ public class MetricsConfiguration { return Collections.unmodifiableCollection(this.hostsWhitelist); } + public boolean isTimersEnabled() { + return timersEnabled; + } + @Override public String toString() { return MoreObjects.toStringHelper(this) @@ -173,6 +181,7 @@ public class MetricsConfiguration { private int pushInterval = 15; private String prometheusJob = "pantheon-client"; private List hostsWhitelist = Arrays.asList("localhost", "127.0.0.1"); + private boolean timersEnabled = DEFAULT_TIMERS_ENABLED; private Builder() {} @@ -226,6 +235,11 @@ public class MetricsConfiguration { return this; } + public Builder timersEnabled(final boolean timersEnabled) { + this.timersEnabled = timersEnabled; + return this; + } + public MetricsConfiguration build() { return new MetricsConfiguration( enabled, @@ -237,7 +251,8 @@ public class MetricsConfiguration { pushHost, pushInterval, prometheusJob, - hostsWhitelist); + hostsWhitelist, + timersEnabled); } } } diff --git a/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/prometheus/PrometheusMetricsSystem.java b/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/prometheus/PrometheusMetricsSystem.java index cc40a1e6dc..79cd8db56a 100644 --- a/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/prometheus/PrometheusMetricsSystem.java +++ b/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/prometheus/PrometheusMetricsSystem.java @@ -58,9 +58,12 @@ public class PrometheusMetricsSystem implements MetricsSystem { cachedTimers = new ConcurrentHashMap<>(); private final Set enabledCategories; + private final boolean timersEnabled; - PrometheusMetricsSystem(final Set enabledCategories) { + PrometheusMetricsSystem( + final Set enabledCategories, final boolean timersEnabled) { this.enabledCategories = ImmutableSet.copyOf(enabledCategories); + this.timersEnabled = timersEnabled; } public static MetricsSystem init(final MetricsConfiguration metricsConfiguration) { @@ -68,7 +71,8 @@ public class PrometheusMetricsSystem implements MetricsSystem { return new NoOpMetricsSystem(); } final PrometheusMetricsSystem metricsSystem = - new PrometheusMetricsSystem(metricsConfiguration.getMetricCategories()); + new PrometheusMetricsSystem( + metricsConfiguration.getMetricCategories(), metricsConfiguration.isTimersEnabled()); if (metricsSystem.isCategoryEnabled(StandardMetricCategory.PROCESS)) { metricsSystem.collectors.put( StandardMetricCategory.PROCESS, @@ -117,7 +121,7 @@ public class PrometheusMetricsSystem implements MetricsSystem { return cachedTimers.computeIfAbsent( metricName, (k) -> { - if (isCategoryEnabled(category)) { + if (timersEnabled && isCategoryEnabled(category)) { final Summary summary = Summary.build(metricName, help) .quantile(0.2, 0.02) diff --git a/metrics/core/src/test/java/tech/pegasys/pantheon/metrics/prometheus/PrometheusMetricsSystemTest.java b/metrics/core/src/test/java/tech/pegasys/pantheon/metrics/prometheus/PrometheusMetricsSystemTest.java index 749d919caf..d102dea9fe 100644 --- a/metrics/core/src/test/java/tech/pegasys/pantheon/metrics/prometheus/PrometheusMetricsSystemTest.java +++ b/metrics/core/src/test/java/tech/pegasys/pantheon/metrics/prometheus/PrometheusMetricsSystemTest.java @@ -45,7 +45,7 @@ public class PrometheusMetricsSystemTest { .thenComparing((o1, o2) -> o1.getLabels().equals(o2.getLabels()) ? 0 : 1); private final MetricsSystem metricsSystem = - new PrometheusMetricsSystem(DEFAULT_METRIC_CATEGORIES); + new PrometheusMetricsSystem(DEFAULT_METRIC_CATEGORIES, true); @Test public void shouldCreateObservationFromCounter() { @@ -155,6 +155,19 @@ public class PrometheusMetricsSystemTest { new Observation(RPC, "request", null, asList("method", "count"))); } + @Test + public void shouldNotCreateObservationsFromTimerWhenTimersDisabled() { + final MetricsSystem metricsSystem = + new PrometheusMetricsSystem(DEFAULT_METRIC_CATEGORIES, false); + final LabelledMetric timer = + metricsSystem.createLabelledTimer(RPC, "request", "Some help", "methodName"); + + //noinspection EmptyTryBlock + try (final TimingContext ignored = timer.labels("method").startTimer()) {} + + assertThat(metricsSystem.streamObservations()).isEmpty(); + } + @Test public void shouldCreateObservationFromGauge() { metricsSystem.createGauge(JVM, "myValue", "Help", () -> 7d); diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java b/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java index 832a3de809..285f56a0c6 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java @@ -41,6 +41,7 @@ import tech.pegasys.pantheon.cli.custom.JsonRPCWhitelistHostsProperty; import tech.pegasys.pantheon.cli.custom.RpcAuthFileValidator; import tech.pegasys.pantheon.cli.error.PantheonExceptionHandler; import tech.pegasys.pantheon.cli.options.EthProtocolOptions; +import tech.pegasys.pantheon.cli.options.MetricsCLIOptions; import tech.pegasys.pantheon.cli.options.NetworkingOptions; import tech.pegasys.pantheon.cli.options.RocksDBOptions; import tech.pegasys.pantheon.cli.options.SynchronizerOptions; @@ -160,6 +161,7 @@ public class PantheonCommand implements DefaultCommandValues, Runnable { final NetworkingOptions networkingOptions = NetworkingOptions.create(); final SynchronizerOptions synchronizerOptions = SynchronizerOptions.create(); final EthProtocolOptions ethProtocolOptions = EthProtocolOptions.create(); + final MetricsCLIOptions metricsCLIOptions = MetricsCLIOptions.create(); final RocksDBOptions rocksDBOptions = RocksDBOptions.create(); final TransactionPoolOptions transactionPoolOptions = TransactionPoolOptions.create(); private final RunnerBuilder runnerBuilder; @@ -715,19 +717,18 @@ public class PantheonCommand implements DefaultCommandValues, Runnable { private PantheonCommand handleUnstableOptions() { // Add unstable options - UnstableOptionsSubCommand.createUnstableOptions( - commandLine, - ImmutableMap.of( - "P2P Network", - networkingOptions, - "Synchronizer", - synchronizerOptions, - "RocksDB", - rocksDBOptions, - "Ethereum Wire Protocol", - ethProtocolOptions, - "TransactionPool", - transactionPoolOptions)); + final ImmutableMap.Builder unstableOptionsBuild = ImmutableMap.builder(); + final ImmutableMap unstableOptions = + unstableOptionsBuild + .put("Ethereum Wire Protocol", ethProtocolOptions) + .put("Metrics", metricsCLIOptions) + .put("P2P Network", networkingOptions) + .put("RocksDB", rocksDBOptions) + .put("Synchronizer", synchronizerOptions) + .put("TransactionPool", transactionPoolOptions) + .build(); + + UnstableOptionsSubCommand.createUnstableOptions(commandLine, unstableOptions); return this; } @@ -1006,7 +1007,8 @@ public class PantheonCommand implements DefaultCommandValues, Runnable { "--metrics-push-interval", "--metrics-push-prometheus-job")); - return MetricsConfiguration.builder() + return metricsCLIOptions + .toDomainObject() .enabled(isMetricsEnabled) .host(metricsHost) .port(metricsPort) diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/cli/options/MetricsCLIOptions.java b/pantheon/src/main/java/tech/pegasys/pantheon/cli/options/MetricsCLIOptions.java new file mode 100644 index 0000000000..cf3faf94d3 --- /dev/null +++ b/pantheon/src/main/java/tech/pegasys/pantheon/cli/options/MetricsCLIOptions.java @@ -0,0 +1,53 @@ +/* + * 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.cli.options; + +import tech.pegasys.pantheon.metrics.prometheus.MetricsConfiguration; + +import java.util.Arrays; +import java.util.List; + +import picocli.CommandLine; + +public class MetricsCLIOptions implements CLIOptions { + private static final String TIMERS_ENABLED_FLAG = "--Xmetrics-timers-enabled"; + + @CommandLine.Option( + names = TIMERS_ENABLED_FLAG, + hidden = true, + defaultValue = "true", + description = "Whether to enable timer metrics (default: ${DEFAULT-VALUE}).") + private Boolean timersEnabled = MetricsConfiguration.DEFAULT_TIMERS_ENABLED; + + private MetricsCLIOptions() {} + + public static MetricsCLIOptions create() { + return new MetricsCLIOptions(); + } + + public static MetricsCLIOptions fromConfiguration(final MetricsConfiguration config) { + final MetricsCLIOptions metricsOptions = create(); + metricsOptions.timersEnabled = config.isTimersEnabled(); + return metricsOptions; + } + + @Override + public MetricsConfiguration.Builder toDomainObject() { + return MetricsConfiguration.builder().timersEnabled(timersEnabled); + } + + @Override + public List getCLIOptions() { + return Arrays.asList(TIMERS_ENABLED_FLAG + "=" + timersEnabled.toString()); + } +} diff --git a/pantheon/src/test/java/tech/pegasys/pantheon/cli/CommandTestAbstract.java b/pantheon/src/test/java/tech/pegasys/pantheon/cli/CommandTestAbstract.java index a823c88a47..b320adf355 100644 --- a/pantheon/src/test/java/tech/pegasys/pantheon/cli/CommandTestAbstract.java +++ b/pantheon/src/test/java/tech/pegasys/pantheon/cli/CommandTestAbstract.java @@ -25,6 +25,7 @@ import tech.pegasys.pantheon.Runner; import tech.pegasys.pantheon.RunnerBuilder; import tech.pegasys.pantheon.cli.config.EthNetworkConfig; import tech.pegasys.pantheon.cli.options.EthProtocolOptions; +import tech.pegasys.pantheon.cli.options.MetricsCLIOptions; import tech.pegasys.pantheon.cli.options.NetworkingOptions; import tech.pegasys.pantheon.cli.options.RocksDBOptions; import tech.pegasys.pantheon.cli.options.SynchronizerOptions; @@ -288,5 +289,9 @@ public abstract class CommandTestAbstract { public TransactionPoolOptions getTransactionPoolOptions() { return transactionPoolOptions; } + + public MetricsCLIOptions getMetricsCLIOptions() { + return metricsCLIOptions; + } } } diff --git a/pantheon/src/test/java/tech/pegasys/pantheon/cli/options/MetricsCLIOptionsTest.java b/pantheon/src/test/java/tech/pegasys/pantheon/cli/options/MetricsCLIOptionsTest.java new file mode 100644 index 0000000000..4ac4f30442 --- /dev/null +++ b/pantheon/src/test/java/tech/pegasys/pantheon/cli/options/MetricsCLIOptionsTest.java @@ -0,0 +1,40 @@ +/* + * 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.cli.options; + +import tech.pegasys.pantheon.metrics.prometheus.MetricsConfiguration; + +public class MetricsCLIOptionsTest + extends AbstractCLIOptionsTest { + + @Override + MetricsConfiguration.Builder createDefaultDomainObject() { + return MetricsConfiguration.builder(); + } + + @Override + MetricsConfiguration.Builder createCustomizedDomainObject() { + return MetricsConfiguration.builder() + .timersEnabled(!MetricsConfiguration.DEFAULT_TIMERS_ENABLED); + } + + @Override + MetricsCLIOptions optionsFromDomainObject(final MetricsConfiguration.Builder domainObject) { + return MetricsCLIOptions.fromConfiguration(domainObject.build()); + } + + @Override + MetricsCLIOptions getOptionsFromPantheonCommand(final TestPantheonCommand pantheonCommand) { + return pantheonCommand.getMetricsCLIOptions(); + } +}