From 9b509d11634025b2b157e2ba8c348ac7089d1867 Mon Sep 17 00:00:00 2001 From: Adrian Sutton Date: Wed, 19 Jun 2019 14:24:57 +1000 Subject: [PATCH] Support arbitrary MetricCategories (#1550) * Make MetricCategory an interface so it isn't locked into the pantheon specific categories. * Split categories into StandardMetricCategory and PanteonMetricCategory to separate the common and Pantheon specific categories. Signed-off-by: Adrian Sutton --- .../JsonRpcHttpServiceRpcApisTest.java | 2 +- metrics/core/build.gradle | 3 +- .../pantheon/metrics/MetricCategory.java | 22 +++++ .../pantheon/metrics/MetricsSystem.java | 21 ++--- .../pegasys/pantheon/metrics/Observation.java | 6 +- .../metrics/PantheonMetricCategory.java | 33 ++++--- .../metrics/StandardMetricCategory.java | 36 ++++++++ .../metrics/noop/NoOpMetricsSystem.java | 10 +-- .../prometheus/MetricsConfiguration.java | 12 +-- .../prometheus/MetricsHttpService.java | 15 +--- .../prometheus/MetricsPushGatewayService.java | 4 +- .../prometheus/PrometheusMetricsSystem.java | 85 +++++++++---------- .../pantheon/metrics/StubMetricsSystem.java | 13 ++- .../metrics/noop/NoOpMetricsSystemTest.java | 10 +-- .../prometheus/MetricsHttpServiceTest.java | 6 +- .../PrometheusMetricsSystemTest.java | 12 +-- .../metrics/rocksdb/RocksDBStats.java | 3 +- .../pegasys/pantheon/cli/PantheonCommand.java | 10 ++- .../converter/MetricCategoryConverter.java | 40 +++++++++ .../pantheon/cli/PantheonCommandTest.java | 6 +- 20 files changed, 227 insertions(+), 122 deletions(-) create mode 100644 metrics/core/src/main/java/tech/pegasys/pantheon/metrics/MetricCategory.java create mode 100644 metrics/core/src/main/java/tech/pegasys/pantheon/metrics/StandardMetricCategory.java create mode 100644 pantheon/src/main/java/tech/pegasys/pantheon/cli/converter/MetricCategoryConverter.java diff --git a/ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/JsonRpcHttpServiceRpcApisTest.java b/ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/JsonRpcHttpServiceRpcApisTest.java index 09a12a27a9..89ecce8218 100644 --- a/ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/JsonRpcHttpServiceRpcApisTest.java +++ b/ethereum/jsonrpc/src/test/java/tech/pegasys/pantheon/ethereum/jsonrpc/JsonRpcHttpServiceRpcApisTest.java @@ -257,7 +257,7 @@ public class JsonRpcHttpServiceRpcApisTest { } private MetricsConfiguration createMetricsConfiguration() { - return MetricsConfiguration.builder().enabled(true).build(); + return MetricsConfiguration.builder().enabled(true).port(0).build(); } private JsonRpcHttpService createJsonRpcHttpService( diff --git a/metrics/core/build.gradle b/metrics/core/build.gradle index 8770fea7b1..d2709abc7d 100644 --- a/metrics/core/build.gradle +++ b/metrics/core/build.gradle @@ -26,8 +26,6 @@ jar { } dependencies { - implementation project(':util') - implementation 'com.google.guava:guava' implementation 'io.prometheus:simpleclient' implementation 'io.prometheus:simpleclient_common' @@ -40,6 +38,7 @@ dependencies { runtime 'org.apache.logging.log4j:log4j-core' // test dependencies. + testImplementation project(':util') testImplementation 'junit:junit' testImplementation 'org.assertj:assertj-core' testImplementation 'org.mockito:mockito-core' diff --git a/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/MetricCategory.java b/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/MetricCategory.java new file mode 100644 index 0000000000..6f6de531f8 --- /dev/null +++ b/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/MetricCategory.java @@ -0,0 +1,22 @@ +/* + * 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.metrics; + +import java.util.Optional; + +public interface MetricCategory { + + String getName(); + + Optional getAppliationPrefix(); +} diff --git a/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/MetricsSystem.java b/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/MetricsSystem.java index 0efe0cf3d3..d4364daa3c 100644 --- a/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/MetricsSystem.java +++ b/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/MetricsSystem.java @@ -20,26 +20,25 @@ import java.util.stream.Stream; public interface MetricsSystem { default Counter createCounter( - final PantheonMetricCategory category, final String name, final String help) { + final MetricCategory category, final String name, final String help) { return createLabelledCounter(category, name, help, new String[0]).labels(); } LabelledMetric createLabelledCounter( - PantheonMetricCategory category, String name, String help, String... labelNames); + MetricCategory category, String name, String help, String... labelNames); default OperationTimer createTimer( - final PantheonMetricCategory category, final String name, final String help) { + final MetricCategory category, final String name, final String help) { return createLabelledTimer(category, name, help, new String[0]).labels(); } LabelledMetric createLabelledTimer( - PantheonMetricCategory category, String name, String help, String... labelNames); + MetricCategory category, String name, String help, String... labelNames); - void createGauge( - PantheonMetricCategory category, String name, String help, DoubleSupplier valueSupplier); + void createGauge(MetricCategory category, String name, String help, DoubleSupplier valueSupplier); default void createIntegerGauge( - final PantheonMetricCategory category, + final MetricCategory category, final String name, final String help, final IntSupplier valueSupplier) { @@ -47,16 +46,14 @@ public interface MetricsSystem { } default void createLongGauge( - final PantheonMetricCategory category, + final MetricCategory category, final String name, final String help, final LongSupplier valueSupplier) { createGauge(category, name, help, () -> (double) valueSupplier.getAsLong()); } - Stream streamObservations(PantheonMetricCategory category); + Stream streamObservations(MetricCategory category); - default Stream streamObservations() { - return Stream.of(PantheonMetricCategory.values()).flatMap(this::streamObservations); - } + Stream streamObservations(); } diff --git a/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/Observation.java b/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/Observation.java index 2385b18d96..6fe1efd9c1 100644 --- a/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/Observation.java +++ b/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/Observation.java @@ -18,13 +18,13 @@ import java.util.Objects; import com.google.common.base.MoreObjects; public class Observation { - private final PantheonMetricCategory category; + private final MetricCategory category; private final String metricName; private final List labels; private final Object value; public Observation( - final PantheonMetricCategory category, + final MetricCategory category, final String metricName, final Object value, final List labels) { @@ -34,7 +34,7 @@ public class Observation { this.labels = labels; } - public PantheonMetricCategory getCategory() { + public MetricCategory getCategory() { return category; } diff --git a/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/PantheonMetricCategory.java b/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/PantheonMetricCategory.java index fb5e6fa686..797fcec003 100644 --- a/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/PantheonMetricCategory.java +++ b/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/PantheonMetricCategory.java @@ -1,5 +1,5 @@ /* - * Copyright 2018 ConsenSys AG. + * 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 @@ -13,16 +13,16 @@ package tech.pegasys.pantheon.metrics; import java.util.EnumSet; +import java.util.Optional; import java.util.Set; -public enum PantheonMetricCategory { - BIG_QUEUE("big_queue"), +import com.google.common.collect.ImmutableSet; + +public enum PantheonMetricCategory implements MetricCategory { BLOCKCHAIN("blockchain"), EXECUTORS("executors"), - JVM("jvm", false), NETWORK("network"), PEERS("peers"), - PROCESS("process", false), PERMISSIONING("permissioning"), KVSTORE_ROCKSDB("rocksdb"), KVSTORE_ROCKSDB_STATS("rocksdb", false), @@ -30,9 +30,20 @@ public enum PantheonMetricCategory { SYNCHRONIZER("synchronizer"), TRANSACTION_POOL("transaction_pool"); - // Why not BIG_QUEUE and ROCKSDB? They hurt performance under load. - public static final Set DEFAULT_METRIC_CATEGORIES = - EnumSet.complementOf(EnumSet.of(BIG_QUEUE, KVSTORE_ROCKSDB, KVSTORE_ROCKSDB_STATS)); + private static final Optional PANTHEON_PREFIX = Optional.of("pantheon_"); + public static final Set DEFAULT_METRIC_CATEGORIES; + + static { + // Why not ROCKSDB and KVSTORE_ROCKSDB_STATS? They hurt performance under load. + final EnumSet pantheonCategories = + EnumSet.complementOf(EnumSet.of(KVSTORE_ROCKSDB, KVSTORE_ROCKSDB_STATS)); + + DEFAULT_METRIC_CATEGORIES = + ImmutableSet.builder() + .addAll(pantheonCategories) + .addAll(EnumSet.allOf(StandardMetricCategory.class)) + .build(); + } private final String name; private final boolean pantheonSpecific; @@ -46,11 +57,13 @@ public enum PantheonMetricCategory { this.pantheonSpecific = pantheonSpecific; } + @Override public String getName() { return name; } - public boolean isPantheonSpecific() { - return pantheonSpecific; + @Override + public Optional getAppliationPrefix() { + return pantheonSpecific ? PANTHEON_PREFIX : Optional.empty(); } } diff --git a/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/StandardMetricCategory.java b/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/StandardMetricCategory.java new file mode 100644 index 0000000000..76e8fcdc47 --- /dev/null +++ b/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/StandardMetricCategory.java @@ -0,0 +1,36 @@ +/* + * 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.metrics; + +import java.util.Optional; + +public enum StandardMetricCategory implements MetricCategory { + JVM("jvm"), + PROCESS("process"); + + private final String name; + + StandardMetricCategory(final String name) { + this.name = name; + } + + @Override + public String getName() { + return name; + } + + @Override + public Optional getAppliationPrefix() { + return Optional.empty(); + } +} diff --git a/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/noop/NoOpMetricsSystem.java b/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/noop/NoOpMetricsSystem.java index 19dbc361e1..e478e093f6 100644 --- a/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/noop/NoOpMetricsSystem.java +++ b/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/noop/NoOpMetricsSystem.java @@ -14,11 +14,11 @@ package tech.pegasys.pantheon.metrics.noop; import tech.pegasys.pantheon.metrics.Counter; import tech.pegasys.pantheon.metrics.LabelledMetric; +import tech.pegasys.pantheon.metrics.MetricCategory; import tech.pegasys.pantheon.metrics.MetricsSystem; import tech.pegasys.pantheon.metrics.Observation; import tech.pegasys.pantheon.metrics.OperationTimer; import tech.pegasys.pantheon.metrics.OperationTimer.TimingContext; -import tech.pegasys.pantheon.metrics.PantheonMetricCategory; import java.util.function.DoubleSupplier; import java.util.stream.Stream; @@ -42,7 +42,7 @@ public class NoOpMetricsSystem implements MetricsSystem { @Override public LabelledMetric createLabelledCounter( - final PantheonMetricCategory category, + final MetricCategory category, final String name, final String help, final String... labelNames) { @@ -64,7 +64,7 @@ public class NoOpMetricsSystem implements MetricsSystem { @Override public LabelledMetric createLabelledTimer( - final PantheonMetricCategory category, + final MetricCategory category, final String name, final String help, final String... labelNames) { @@ -82,13 +82,13 @@ public class NoOpMetricsSystem implements MetricsSystem { @Override public void createGauge( - final PantheonMetricCategory category, + final MetricCategory category, final String name, final String help, final DoubleSupplier valueSupplier) {} @Override - public Stream streamObservations(final PantheonMetricCategory category) { + public Stream streamObservations(final MetricCategory category) { return Stream.empty(); } 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 b9de976fc9..b859ff916c 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 @@ -14,7 +14,7 @@ package tech.pegasys.pantheon.metrics.prometheus; import static tech.pegasys.pantheon.metrics.PantheonMetricCategory.DEFAULT_METRIC_CATEGORIES; -import tech.pegasys.pantheon.metrics.PantheonMetricCategory; +import tech.pegasys.pantheon.metrics.MetricCategory; import java.util.Arrays; import java.util.Collection; @@ -35,7 +35,7 @@ public class MetricsConfiguration { private final boolean enabled; private final int port; private final String host; - private final Set metricCategories; + private final Set metricCategories; private final boolean pushEnabled; private final int pushPort; private final String pushHost; @@ -51,7 +51,7 @@ public class MetricsConfiguration { final boolean enabled, final int port, final String host, - final Set metricCategories, + final Set metricCategories, final boolean pushEnabled, final int pushPort, final String pushHost, @@ -82,7 +82,7 @@ public class MetricsConfiguration { return host; } - public Set getMetricCategories() { + public Set getMetricCategories() { return metricCategories; } @@ -166,7 +166,7 @@ public class MetricsConfiguration { private boolean enabled = false; private int port = DEFAULT_METRICS_PORT; private String host = DEFAULT_METRICS_HOST; - private Set metricCategories = DEFAULT_METRIC_CATEGORIES; + private Set metricCategories = DEFAULT_METRIC_CATEGORIES; private boolean pushEnabled = false; private int pushPort = DEFAULT_METRICS_PUSH_PORT; private String pushHost = DEFAULT_METRICS_PUSH_HOST; @@ -191,7 +191,7 @@ public class MetricsConfiguration { return this; } - public Builder metricCategories(final Set metricCategories) { + public Builder metricCategories(final Set metricCategories) { this.metricCategories = metricCategories; return this; } diff --git a/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/prometheus/MetricsHttpService.java b/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/prometheus/MetricsHttpService.java index 8b77cceed9..6edc76c4d1 100644 --- a/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/prometheus/MetricsHttpService.java +++ b/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/prometheus/MetricsHttpService.java @@ -14,10 +14,8 @@ package tech.pegasys.pantheon.metrics.prometheus; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.collect.Streams.stream; -import static tech.pegasys.pantheon.util.NetworkUtility.urlForSocketAddress; import tech.pegasys.pantheon.metrics.MetricsSystem; -import tech.pegasys.pantheon.util.NetworkUtility; import java.io.ByteArrayOutputStream; import java.io.IOException; @@ -30,7 +28,6 @@ import java.util.Set; import java.util.TreeSet; import java.util.concurrent.CompletableFuture; -import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Splitter; import com.google.common.collect.Iterables; import io.netty.handler.codec.http.HttpResponseStatus; @@ -68,9 +65,7 @@ class MetricsHttpService implements MetricsService { } private void validateConfig(final MetricsConfiguration config) { - checkArgument( - config.getPort() == 0 || NetworkUtility.isValidPort(config.getPort()), - "Invalid port configuration."); + checkArgument(config.getPort() >= 0 && config.getPort() < 65535, "Invalid port configuration."); checkArgument(config.getHost() != null, "Required host is not configured."); checkArgument( !(config.isEnabled() && config.isPushEnabled()), @@ -232,14 +227,6 @@ class MetricsHttpService implements MetricsService { return Optional.of(httpServer.actualPort()); } - @VisibleForTesting - public String url() { - if (httpServer == null) { - return ""; - } - return urlForSocketAddress("http", socketAddress()); - } - // Facilitate remote health-checks in AWS, inter alia. private void handleEmptyRequest(final RoutingContext routingContext) { routingContext.response().setStatusCode(201).end(); diff --git a/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/prometheus/MetricsPushGatewayService.java b/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/prometheus/MetricsPushGatewayService.java index 78d5eb5482..329ab9a1d4 100644 --- a/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/prometheus/MetricsPushGatewayService.java +++ b/metrics/core/src/main/java/tech/pegasys/pantheon/metrics/prometheus/MetricsPushGatewayService.java @@ -15,7 +15,6 @@ package tech.pegasys.pantheon.metrics.prometheus; import static com.google.common.base.Preconditions.checkArgument; import tech.pegasys.pantheon.metrics.MetricsSystem; -import tech.pegasys.pantheon.util.NetworkUtility; import java.io.IOException; import java.util.Optional; @@ -45,8 +44,7 @@ class MetricsPushGatewayService implements MetricsService { private void validateConfig(final MetricsConfiguration config) { checkArgument( - config.getPushPort() == 0 || NetworkUtility.isValidPort(config.getPushPort()), - "Invalid port configuration."); + config.getPushPort() >= 0 && config.getPushPort() < 65536, "Invalid port configuration."); checkArgument(config.getPushHost() != null, "Required host is not configured."); checkArgument( !(config.isEnabled() && config.isPushEnabled()), 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 4f2a6101fa..c8d300fb7e 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 @@ -16,22 +16,24 @@ import static java.util.Arrays.asList; import static java.util.Collections.singleton; import tech.pegasys.pantheon.metrics.LabelledMetric; +import tech.pegasys.pantheon.metrics.MetricCategory; import tech.pegasys.pantheon.metrics.MetricsSystem; import tech.pegasys.pantheon.metrics.Observation; import tech.pegasys.pantheon.metrics.OperationTimer; -import tech.pegasys.pantheon.metrics.PantheonMetricCategory; +import tech.pegasys.pantheon.metrics.StandardMetricCategory; import tech.pegasys.pantheon.metrics.noop.NoOpMetricsSystem; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; -import java.util.EnumSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.function.DoubleSupplier; import java.util.stream.Stream; +import com.google.common.collect.ImmutableSet; import io.prometheus.client.Collector; import io.prometheus.client.Collector.MetricFamilySamples; import io.prometheus.client.Collector.MetricFamilySamples.Sample; @@ -48,34 +50,33 @@ import io.prometheus.client.hotspot.ThreadExports; public class PrometheusMetricsSystem implements MetricsSystem { - private static final String PANTHEON_PREFIX = "pantheon_"; - private final Map> collectors = - new ConcurrentHashMap<>(); + private final Map> collectors = new ConcurrentHashMap<>(); private final CollectorRegistry registry = new CollectorRegistry(true); private final Map> cachedCounters = new ConcurrentHashMap<>(); private final Map> cachedTimers = new ConcurrentHashMap<>(); - private final EnumSet enabledCategories = - EnumSet.allOf(PantheonMetricCategory.class); + private final Set enabledCategories; - PrometheusMetricsSystem() {} + PrometheusMetricsSystem(final Set enabledCategories) { + this.enabledCategories = ImmutableSet.copyOf(enabledCategories); + } public static MetricsSystem init(final MetricsConfiguration metricsConfiguration) { if (!metricsConfiguration.isEnabled() && !metricsConfiguration.isPushEnabled()) { return new NoOpMetricsSystem(); } - final PrometheusMetricsSystem metricsSystem = new PrometheusMetricsSystem(); - metricsSystem.enabledCategories.retainAll(metricsConfiguration.getMetricCategories()); - if (metricsSystem.enabledCategories.contains(PantheonMetricCategory.PROCESS)) { + final PrometheusMetricsSystem metricsSystem = + new PrometheusMetricsSystem(metricsConfiguration.getMetricCategories()); + if (metricsSystem.isCategoryEnabled(StandardMetricCategory.PROCESS)) { metricsSystem.collectors.put( - PantheonMetricCategory.PROCESS, + StandardMetricCategory.PROCESS, singleton(new StandardExports().register(metricsSystem.registry))); } - if (metricsSystem.enabledCategories.contains(PantheonMetricCategory.JVM)) { + if (metricsSystem.isCategoryEnabled(StandardMetricCategory.JVM)) { metricsSystem.collectors.put( - PantheonMetricCategory.JVM, + StandardMetricCategory.JVM, asList( new MemoryPoolsExports().register(metricsSystem.registry), new BufferPoolsExports().register(metricsSystem.registry), @@ -88,7 +89,7 @@ public class PrometheusMetricsSystem implements MetricsSystem { @Override public LabelledMetric createLabelledCounter( - final PantheonMetricCategory category, + final MetricCategory category, final String name, final String help, final String... labelNames) { @@ -96,7 +97,7 @@ public class PrometheusMetricsSystem implements MetricsSystem { return cachedCounters.computeIfAbsent( metricName, (k) -> { - if (enabledCategories.contains(category)) { + if (isCategoryEnabled(category)) { final Counter counter = Counter.build(metricName, help).labelNames(labelNames).create(); addCollectorUnchecked(category, counter); return new PrometheusCounter(counter); @@ -108,7 +109,7 @@ public class PrometheusMetricsSystem implements MetricsSystem { @Override public LabelledMetric createLabelledTimer( - final PantheonMetricCategory category, + final MetricCategory category, final String name, final String help, final String... labelNames) { @@ -116,7 +117,7 @@ public class PrometheusMetricsSystem implements MetricsSystem { return cachedTimers.computeIfAbsent( metricName, (k) -> { - if (enabledCategories.contains(category)) { + if (isCategoryEnabled(category)) { final Summary summary = Summary.build(metricName, help) .quantile(0.2, 0.02) @@ -137,25 +138,28 @@ public class PrometheusMetricsSystem implements MetricsSystem { @Override public void createGauge( - final PantheonMetricCategory category, + final MetricCategory category, final String name, final String help, final DoubleSupplier valueSupplier) { final String metricName = convertToPrometheusName(category, name); - if (enabledCategories.contains(category)) { + if (isCategoryEnabled(category)) { final Collector collector = new CurrentValueCollector(metricName, help, valueSupplier); addCollectorUnchecked(category, collector); } } - public void addCollector(final PantheonMetricCategory category, final Collector metric) { - if (enabledCategories.contains(category)) { + private boolean isCategoryEnabled(final MetricCategory category) { + return enabledCategories.contains(category); + } + + public void addCollector(final MetricCategory category, final Collector metric) { + if (isCategoryEnabled(category)) { addCollectorUnchecked(category, metric); } } - private void addCollectorUnchecked( - final PantheonMetricCategory category, final Collector metric) { + private void addCollectorUnchecked(final MetricCategory category, final Collector metric) { metric.register(registry); collectors .computeIfAbsent(category, key -> Collections.newSetFromMap(new ConcurrentHashMap<>())) @@ -163,22 +167,25 @@ public class PrometheusMetricsSystem implements MetricsSystem { } @Override - public Stream streamObservations(final PantheonMetricCategory category) { + public Stream streamObservations(final MetricCategory category) { return collectors.getOrDefault(category, Collections.emptySet()).stream() .flatMap(collector -> collector.collect().stream()) .flatMap(familySamples -> convertSamplesToObservations(category, familySamples)); } + @Override + public Stream streamObservations() { + return collectors.keySet().stream().flatMap(this::streamObservations); + } + private Stream convertSamplesToObservations( - final PantheonMetricCategory category, final MetricFamilySamples familySamples) { + final MetricCategory category, final MetricFamilySamples familySamples) { return familySamples.samples.stream() .map(sample -> createObservationFromSample(category, sample, familySamples)); } private Observation createObservationFromSample( - final PantheonMetricCategory category, - final Sample sample, - final MetricFamilySamples familySamples) { + final MetricCategory category, final Sample sample, final MetricFamilySamples familySamples) { if (familySamples.type == Type.HISTOGRAM) { return convertHistogramSampleNamesToLabels(category, sample, familySamples); } @@ -193,9 +200,7 @@ public class PrometheusMetricsSystem implements MetricsSystem { } private Observation convertHistogramSampleNamesToLabels( - final PantheonMetricCategory category, - final Sample sample, - final MetricFamilySamples familySamples) { + final MetricCategory category, final Sample sample, final MetricFamilySamples familySamples) { final List labelValues = new ArrayList<>(sample.labelValues); if (sample.name.endsWith("_bucket")) { labelValues.add(labelValues.size() - 1, "bucket"); @@ -210,9 +215,7 @@ public class PrometheusMetricsSystem implements MetricsSystem { } private Observation convertSummarySampleNamesToLabels( - final PantheonMetricCategory category, - final Sample sample, - final MetricFamilySamples familySamples) { + final MetricCategory category, final Sample sample, final MetricFamilySamples familySamples) { final List labelValues = new ArrayList<>(sample.labelValues); if (sample.name.endsWith("_sum")) { labelValues.add("sum"); @@ -228,21 +231,17 @@ public class PrometheusMetricsSystem implements MetricsSystem { labelValues); } - public static String convertToPrometheusName( - final PantheonMetricCategory category, final String name) { + public String convertToPrometheusName(final MetricCategory category, final String name) { return prometheusPrefix(category) + name; } - private String convertFromPrometheusName( - final PantheonMetricCategory category, final String metricName) { + private String convertFromPrometheusName(final MetricCategory category, final String metricName) { final String prefix = prometheusPrefix(category); return metricName.startsWith(prefix) ? metricName.substring(prefix.length()) : metricName; } - private static String prometheusPrefix(final PantheonMetricCategory category) { - return category.isPantheonSpecific() - ? PANTHEON_PREFIX + category.getName() + "_" - : category.getName() + "_"; + private String prometheusPrefix(final MetricCategory category) { + return category.getAppliationPrefix().orElse("") + category.getName() + "_"; } CollectorRegistry getRegistry() { diff --git a/metrics/core/src/test-support/java/tech/pegasys/pantheon/metrics/StubMetricsSystem.java b/metrics/core/src/test-support/java/tech/pegasys/pantheon/metrics/StubMetricsSystem.java index 468af9d951..cd6c08ab5d 100644 --- a/metrics/core/src/test-support/java/tech/pegasys/pantheon/metrics/StubMetricsSystem.java +++ b/metrics/core/src/test-support/java/tech/pegasys/pantheon/metrics/StubMetricsSystem.java @@ -29,7 +29,7 @@ public class StubMetricsSystem implements MetricsSystem { @Override public LabelledMetric createLabelledCounter( - final PantheonMetricCategory category, + final MetricCategory category, final String name, final String help, final String... labelNames) { @@ -50,7 +50,7 @@ public class StubMetricsSystem implements MetricsSystem { @Override public LabelledMetric createLabelledTimer( - final PantheonMetricCategory category, + final MetricCategory category, final String name, final String help, final String... labelNames) { @@ -59,7 +59,7 @@ public class StubMetricsSystem implements MetricsSystem { @Override public void createGauge( - final PantheonMetricCategory category, + final MetricCategory category, final String name, final String help, final DoubleSupplier valueSupplier) { @@ -75,7 +75,12 @@ public class StubMetricsSystem implements MetricsSystem { } @Override - public Stream streamObservations(final PantheonMetricCategory category) { + public Stream streamObservations(final MetricCategory category) { + throw new UnsupportedOperationException("Observations aren't actually recorded"); + } + + @Override + public Stream streamObservations() { throw new UnsupportedOperationException("Observations aren't actually recorded"); } diff --git a/metrics/core/src/test/java/tech/pegasys/pantheon/metrics/noop/NoOpMetricsSystemTest.java b/metrics/core/src/test/java/tech/pegasys/pantheon/metrics/noop/NoOpMetricsSystemTest.java index 098fb67147..88251bb8a3 100644 --- a/metrics/core/src/test/java/tech/pegasys/pantheon/metrics/noop/NoOpMetricsSystemTest.java +++ b/metrics/core/src/test/java/tech/pegasys/pantheon/metrics/noop/NoOpMetricsSystemTest.java @@ -19,7 +19,7 @@ import tech.pegasys.pantheon.metrics.Counter; import tech.pegasys.pantheon.metrics.LabelledMetric; import tech.pegasys.pantheon.metrics.MetricsSystem; import tech.pegasys.pantheon.metrics.OperationTimer; -import tech.pegasys.pantheon.metrics.PantheonMetricCategory; +import tech.pegasys.pantheon.metrics.StandardMetricCategory; import org.junit.Test; @@ -31,7 +31,7 @@ public class NoOpMetricsSystemTest { public void labelCountsMatchOnCounter() { final LabelledMetric labeledCounter = metricsSystem.createLabelledCounter( - PantheonMetricCategory.PROCESS, "name", "help", "label1"); + StandardMetricCategory.PROCESS, "name", "help", "label1"); assertThat(labeledCounter.labels("one")).isSameAs(NoOpMetricsSystem.NO_OP_COUNTER); } @@ -39,7 +39,7 @@ public class NoOpMetricsSystemTest { public void failsWheLabelCountsDoNotMatchOnCounter() { final LabelledMetric labeledCounter = metricsSystem.createLabelledCounter( - PantheonMetricCategory.PROCESS, "name", "help", "label1", "label2"); + StandardMetricCategory.PROCESS, "name", "help", "label1", "label2"); assertThatExceptionOfType(IllegalArgumentException.class) .isThrownBy(() -> labeledCounter.labels("one")) @@ -52,7 +52,7 @@ public class NoOpMetricsSystemTest { @Test public void labelCountsMatchOnTimer() { final LabelledMetric labeledTimer = - metricsSystem.createLabelledTimer(PantheonMetricCategory.PROCESS, "name", "help", "label1"); + metricsSystem.createLabelledTimer(StandardMetricCategory.PROCESS, "name", "help", "label1"); assertThat(labeledTimer.labels("one")).isSameAs(NoOpMetricsSystem.NO_OP_OPERATION_TIMER); } @@ -60,7 +60,7 @@ public class NoOpMetricsSystemTest { public void failsWheLabelCountsDoNotMatchOnTimer() { final LabelledMetric labeledTimer = metricsSystem.createLabelledTimer( - PantheonMetricCategory.PROCESS, "name", "help", "label1", "label2"); + StandardMetricCategory.PROCESS, "name", "help", "label1", "label2"); assertThatExceptionOfType(IllegalArgumentException.class) .isThrownBy(() -> labeledTimer.labels("one")) diff --git a/metrics/core/src/test/java/tech/pegasys/pantheon/metrics/prometheus/MetricsHttpServiceTest.java b/metrics/core/src/test/java/tech/pegasys/pantheon/metrics/prometheus/MetricsHttpServiceTest.java index baf0b48185..3d0218fe0c 100644 --- a/metrics/core/src/test/java/tech/pegasys/pantheon/metrics/prometheus/MetricsHttpServiceTest.java +++ b/metrics/core/src/test/java/tech/pegasys/pantheon/metrics/prometheus/MetricsHttpServiceTest.java @@ -14,6 +14,7 @@ package tech.pegasys.pantheon.metrics.prometheus; import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; +import static tech.pegasys.pantheon.util.NetworkUtility.urlForSocketAddress; import java.net.InetSocketAddress; import java.util.Properties; @@ -45,7 +46,7 @@ public class MetricsHttpServiceTest { // Build an OkHttp client. client = new OkHttpClient(); - baseUrl = service.url(); + baseUrl = urlForSocketAddress("http", service.socketAddress()); } private static MetricsHttpService createMetricsHttpService(final MetricsConfiguration config) { @@ -108,7 +109,7 @@ public class MetricsHttpServiceTest { final InetSocketAddress socketAddress = service.socketAddress(); assertThat("0.0.0.0").isEqualTo(socketAddress.getAddress().getHostAddress()); assertThat(0).isEqualTo(socketAddress.getPort()); - assertThat("").isEqualTo(service.url()); + assertThat(new InetSocketAddress("0.0.0.0", 0)).isEqualTo(service.socketAddress()); } @Test @@ -121,7 +122,6 @@ public class MetricsHttpServiceTest { final InetSocketAddress socketAddress = service.socketAddress(); assertThat("0.0.0.0").isEqualTo(socketAddress.getAddress().getHostAddress()); assertThat(socketAddress.getPort() > 0).isTrue(); - assertThat(!service.url().contains("0.0.0.0")).isTrue(); } finally { service.stop().join(); } 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 b40e9a78d2..749d919caf 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 @@ -17,10 +17,11 @@ import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; -import static tech.pegasys.pantheon.metrics.PantheonMetricCategory.JVM; +import static tech.pegasys.pantheon.metrics.PantheonMetricCategory.DEFAULT_METRIC_CATEGORIES; import static tech.pegasys.pantheon.metrics.PantheonMetricCategory.NETWORK; import static tech.pegasys.pantheon.metrics.PantheonMetricCategory.PEERS; import static tech.pegasys.pantheon.metrics.PantheonMetricCategory.RPC; +import static tech.pegasys.pantheon.metrics.StandardMetricCategory.JVM; import tech.pegasys.pantheon.metrics.Counter; import tech.pegasys.pantheon.metrics.LabelledMetric; @@ -32,18 +33,19 @@ import tech.pegasys.pantheon.metrics.PantheonMetricCategory; import tech.pegasys.pantheon.metrics.noop.NoOpMetricsSystem; import java.util.Comparator; -import java.util.EnumSet; +import com.google.common.collect.ImmutableSet; import org.junit.Test; public class PrometheusMetricsSystemTest { private static final Comparator IGNORE_VALUES = - Comparator.comparing(Observation::getCategory) + Comparator.comparing(observation -> observation.getCategory().getName()) .thenComparing(Observation::getMetricName) .thenComparing((o1, o2) -> o1.getLabels().equals(o2.getLabels()) ? 0 : 1); - private final MetricsSystem metricsSystem = new PrometheusMetricsSystem(); + private final MetricsSystem metricsSystem = + new PrometheusMetricsSystem(DEFAULT_METRIC_CATEGORIES); @Test public void shouldCreateObservationFromCounter() { @@ -175,7 +177,7 @@ public class PrometheusMetricsSystemTest { public void shouldOnlyObserveEnabledMetrics() { final MetricsConfiguration metricsConfiguration = MetricsConfiguration.builder() - .metricCategories(EnumSet.of(PantheonMetricCategory.RPC)) + .metricCategories(ImmutableSet.of(PantheonMetricCategory.RPC)) .enabled(true) .build(); final MetricsSystem localMetricSystem = PrometheusMetricsSystem.init(metricsConfiguration); diff --git a/metrics/rocksdb/src/main/java/tech/pegasys/pantheon/metrics/rocksdb/RocksDBStats.java b/metrics/rocksdb/src/main/java/tech/pegasys/pantheon/metrics/rocksdb/RocksDBStats.java index 58316d87bd..cd363ebb5a 100644 --- a/metrics/rocksdb/src/main/java/tech/pegasys/pantheon/metrics/rocksdb/RocksDBStats.java +++ b/metrics/rocksdb/src/main/java/tech/pegasys/pantheon/metrics/rocksdb/RocksDBStats.java @@ -187,8 +187,7 @@ public class RocksDBStats { final Statistics stats, final HistogramType histogram) { return new Collector() { final String metricName = - PrometheusMetricsSystem.convertToPrometheusName( - KVSTORE_ROCKSDB_STATS, histogram.name().toLowerCase()); + KVSTORE_ROCKSDB_STATS.getName() + "_" + histogram.name().toLowerCase(); @Override public List collect() { 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 7d002e65c3..7d29a5bf36 100644 --- a/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java +++ b/pantheon/src/main/java/tech/pegasys/pantheon/cli/PantheonCommand.java @@ -31,6 +31,7 @@ import static tech.pegasys.pantheon.metrics.prometheus.MetricsConfiguration.DEFA import tech.pegasys.pantheon.Runner; import tech.pegasys.pantheon.RunnerBuilder; import tech.pegasys.pantheon.cli.PublicKeySubCommand.KeyLoader; +import tech.pegasys.pantheon.cli.converter.MetricCategoryConverter; import tech.pegasys.pantheon.cli.converter.RpcApisConverter; import tech.pegasys.pantheon.cli.custom.CorsAllowedOriginsProperty; import tech.pegasys.pantheon.cli.custom.JsonRPCWhitelistHostsProperty; @@ -61,8 +62,10 @@ import tech.pegasys.pantheon.ethereum.permissioning.LocalPermissioningConfigurat import tech.pegasys.pantheon.ethereum.permissioning.PermissioningConfiguration; import tech.pegasys.pantheon.ethereum.permissioning.PermissioningConfigurationBuilder; import tech.pegasys.pantheon.ethereum.permissioning.SmartContractPermissioningConfiguration; +import tech.pegasys.pantheon.metrics.MetricCategory; import tech.pegasys.pantheon.metrics.MetricsSystem; import tech.pegasys.pantheon.metrics.PantheonMetricCategory; +import tech.pegasys.pantheon.metrics.StandardMetricCategory; import tech.pegasys.pantheon.metrics.prometheus.MetricsConfiguration; import tech.pegasys.pantheon.metrics.prometheus.PrometheusMetricsSystem; import tech.pegasys.pantheon.metrics.vertx.VertxMetricsAdapterFactory; @@ -412,7 +415,7 @@ public class PantheonCommand implements DefaultCommandValues, Runnable { arity = "1..*", description = "Comma separated list of categories to track metrics for (default: ${DEFAULT-VALUE})") - private final Set metricCategories = DEFAULT_METRIC_CATEGORIES; + private final Set metricCategories = DEFAULT_METRIC_CATEGORIES; @Option( names = {"--metrics-push-enabled"}, @@ -650,6 +653,11 @@ public class PantheonCommand implements DefaultCommandValues, Runnable { commandLine.registerConverter(Wei.class, (arg) -> Wei.of(Long.parseUnsignedLong(arg))); commandLine.registerConverter(PositiveNumber.class, PositiveNumber::fromString); + final MetricCategoryConverter metricCategoryConverter = new MetricCategoryConverter(); + metricCategoryConverter.addCategories(PantheonMetricCategory.class); + metricCategoryConverter.addCategories(StandardMetricCategory.class); + commandLine.registerConverter(MetricCategory.class, metricCategoryConverter); + // Add performance options UnstableOptionsSubCommand.createUnstableOptions( commandLine, diff --git a/pantheon/src/main/java/tech/pegasys/pantheon/cli/converter/MetricCategoryConverter.java b/pantheon/src/main/java/tech/pegasys/pantheon/cli/converter/MetricCategoryConverter.java new file mode 100644 index 0000000000..24f6f5a949 --- /dev/null +++ b/pantheon/src/main/java/tech/pegasys/pantheon/cli/converter/MetricCategoryConverter.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.converter; + +import tech.pegasys.pantheon.metrics.MetricCategory; + +import java.util.EnumSet; +import java.util.HashMap; +import java.util.Map; + +import picocli.CommandLine; + +public class MetricCategoryConverter implements CommandLine.ITypeConverter { + + private final Map metricCategories = new HashMap<>(); + + @Override + public MetricCategory convert(final String value) { + final MetricCategory category = metricCategories.get(value); + if (category == null) { + throw new IllegalArgumentException("Unknown category: " + value); + } + return category; + } + + public & MetricCategory> void addCategories(final Class categoryEnum) { + EnumSet.allOf(categoryEnum) + .forEach(category -> metricCategories.put(category.name(), category)); + } +} diff --git a/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java b/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java index c9bbcc9242..7873d67da6 100644 --- a/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java +++ b/pantheon/src/test/java/tech/pegasys/pantheon/cli/PantheonCommandTest.java @@ -49,7 +49,7 @@ import tech.pegasys.pantheon.ethereum.p2p.peers.EnodeURL; import tech.pegasys.pantheon.ethereum.permissioning.LocalPermissioningConfiguration; import tech.pegasys.pantheon.ethereum.permissioning.PermissioningConfiguration; import tech.pegasys.pantheon.ethereum.permissioning.SmartContractPermissioningConfiguration; -import tech.pegasys.pantheon.metrics.PantheonMetricCategory; +import tech.pegasys.pantheon.metrics.StandardMetricCategory; import tech.pegasys.pantheon.metrics.prometheus.MetricsConfiguration; import tech.pegasys.pantheon.util.bytes.BytesValue; @@ -2054,13 +2054,13 @@ public class PantheonCommandTest extends CommandTestAbstract { @Test public void metricsCategoryPropertyMustBeUsed() { - parseCommand("--metrics-enabled", "--metrics-category", PantheonMetricCategory.JVM.toString()); + parseCommand("--metrics-enabled", "--metrics-category", StandardMetricCategory.JVM.toString()); verify(mockRunnerBuilder).metricsConfiguration(metricsConfigArgumentCaptor.capture()); verify(mockRunnerBuilder).build(); assertThat(metricsConfigArgumentCaptor.getValue().getMetricCategories()) - .containsExactly(PantheonMetricCategory.JVM); + .containsExactly(StandardMetricCategory.JVM); assertThat(commandOutput.toString()).isEmpty(); assertThat(commandErrorOutput.toString()).isEmpty();