From 1c2da80c2af6b84a59e25ac080a263d5fd8da75e Mon Sep 17 00:00:00 2001 From: Fabio Di Fabio Date: Mon, 25 Nov 2024 20:37:17 +0100 Subject: [PATCH] Fixes Signed-off-by: Fabio Di Fabio --- .../besu/metrics/noop/NoOpMetricsSystem.java | 3 --- .../opentelemetry/OpenTelemetrySystem.java | 3 ++- .../prometheus/AbstractPrometheusSummary.java | 20 ++----------------- .../prometheus/PrometheusSuppliedSummary.java | 20 ++++++++----------- .../metrics/prometheus/PrometheusTimer.java | 11 ++-------- .../OpenTelemetryMetricsSystemTest.java | 4 ++-- .../PrometheusMetricsSystemTest.java | 4 ++-- plugin-api/build.gradle | 2 +- .../besu/plugin/services/MetricsSystem.java | 1 - 9 files changed, 19 insertions(+), 49 deletions(-) diff --git a/metrics/core/src/main/java/org/hyperledger/besu/metrics/noop/NoOpMetricsSystem.java b/metrics/core/src/main/java/org/hyperledger/besu/metrics/noop/NoOpMetricsSystem.java index aa86e53b1cd..fd8a7c935f2 100644 --- a/metrics/core/src/main/java/org/hyperledger/besu/metrics/noop/NoOpMetricsSystem.java +++ b/metrics/core/src/main/java/org/hyperledger/besu/metrics/noop/NoOpMetricsSystem.java @@ -42,9 +42,6 @@ public class NoOpMetricsSystem implements ObservableMetricsSystem { /** The constant NO_OP_COUNTER. */ public static final Counter NO_OP_COUNTER = new NoOpCounter(); - /** The constant NO_OP_GAUGE. */ - public static final LabelledSuppliedMetric NO_OP_GAUGE = new NoOpValueCollector(); - private static final OperationTimer.TimingContext NO_OP_TIMING_CONTEXT = () -> 0; /** The constant NO_OP_OPERATION_TIMER. */ diff --git a/metrics/core/src/main/java/org/hyperledger/besu/metrics/opentelemetry/OpenTelemetrySystem.java b/metrics/core/src/main/java/org/hyperledger/besu/metrics/opentelemetry/OpenTelemetrySystem.java index a93f0383860..10da5f40e2e 100644 --- a/metrics/core/src/main/java/org/hyperledger/besu/metrics/opentelemetry/OpenTelemetrySystem.java +++ b/metrics/core/src/main/java/org/hyperledger/besu/metrics/opentelemetry/OpenTelemetrySystem.java @@ -250,7 +250,8 @@ public LabelledSuppliedSummary createLabelledSuppliedSummary( final String name, final String help, final String... labelNames) { - return null; + // not yet supported + return (LabelledSuppliedSummary) NoOpMetricsSystem.getLabelledSuppliedMetric(labelNames.length); } @Override diff --git a/metrics/core/src/main/java/org/hyperledger/besu/metrics/prometheus/AbstractPrometheusSummary.java b/metrics/core/src/main/java/org/hyperledger/besu/metrics/prometheus/AbstractPrometheusSummary.java index be13ccb9d90..6de25970694 100644 --- a/metrics/core/src/main/java/org/hyperledger/besu/metrics/prometheus/AbstractPrometheusSummary.java +++ b/metrics/core/src/main/java/org/hyperledger/besu/metrics/prometheus/AbstractPrometheusSummary.java @@ -34,34 +34,18 @@ */ abstract class AbstractPrometheusSummary extends CategorizedPrometheusCollector { /** The Prometheus collector */ - protected final Collector collector; + protected Collector collector; /** * Constructs a new AbstractPrometheusSummary. * * @param category The {@link MetricCategory} this collector is assigned to * @param name The name of this collector - * @param help The help description for this collector - * @param labelNames The label names for this collector */ - protected AbstractPrometheusSummary( - final MetricCategory category, - final String name, - final String help, - final String... labelNames) { + protected AbstractPrometheusSummary(final MetricCategory category, final String name) { super(category, name); - this.collector = createCollector(help, labelNames); } - /** - * Creates the actual Prometheus collector. - * - * @param help The help description for this collector - * @param labelNames The label names for this collector - * @return The created Prometheus collector - */ - protected abstract Collector createCollector(final String help, final String... labelNames); - /** * Gets the identifier for this collector. * diff --git a/metrics/core/src/main/java/org/hyperledger/besu/metrics/prometheus/PrometheusSuppliedSummary.java b/metrics/core/src/main/java/org/hyperledger/besu/metrics/prometheus/PrometheusSuppliedSummary.java index c42d06da812..9ea975040c2 100644 --- a/metrics/core/src/main/java/org/hyperledger/besu/metrics/prometheus/PrometheusSuppliedSummary.java +++ b/metrics/core/src/main/java/org/hyperledger/besu/metrics/prometheus/PrometheusSuppliedSummary.java @@ -24,7 +24,6 @@ import java.util.function.Supplier; import io.prometheus.metrics.core.metrics.SummaryWithCallback; -import io.prometheus.metrics.model.registry.Collector; import io.prometheus.metrics.model.snapshots.Quantile; import io.prometheus.metrics.model.snapshots.Quantiles; @@ -43,17 +42,14 @@ public PrometheusSuppliedSummary( final String name, final String help, final String... labelNames) { - super(category, name, help, labelNames); - } - - @Override - protected Collector createCollector(final String help, final String... labelNames) { - return SummaryWithCallback.builder() - .name(name) - .help(help) - .labelNames(labelNames) - .callback(this::callback) - .build(); + super(category, name); + this.collector = + SummaryWithCallback.builder() + .name(name) + .help(help) + .labelNames(labelNames) + .callback(this::callback) + .build(); } private void callback(final SummaryWithCallback.Callback callback) { diff --git a/metrics/core/src/main/java/org/hyperledger/besu/metrics/prometheus/PrometheusTimer.java b/metrics/core/src/main/java/org/hyperledger/besu/metrics/prometheus/PrometheusTimer.java index 72bb266bf07..ccbe9c1b699 100644 --- a/metrics/core/src/main/java/org/hyperledger/besu/metrics/prometheus/PrometheusTimer.java +++ b/metrics/core/src/main/java/org/hyperledger/besu/metrics/prometheus/PrometheusTimer.java @@ -22,7 +22,6 @@ import io.prometheus.metrics.core.datapoints.DistributionDataPoint; import io.prometheus.metrics.core.metrics.Summary; -import io.prometheus.metrics.model.registry.Collector; /** * An implementation of Besu timer backed by a Prometheus summary. The summary provides a total @@ -30,7 +29,6 @@ * a sliding time window. */ class PrometheusTimer extends AbstractPrometheusSummary implements LabelledMetric { - private final Map quantiles; public PrometheusTimer( final MetricCategory category, @@ -38,16 +36,11 @@ public PrometheusTimer( final String help, final Map quantiles, final String... labelNames) { - super(category, name, help, labelNames); - this.quantiles = quantiles; - } - - @Override - protected Collector createCollector(final String help, final String... labelNames) { + super(category, name); final var summaryBuilder = Summary.builder().name(this.prefixedName).help(help).labelNames(labelNames); quantiles.forEach(summaryBuilder::quantile); - return summaryBuilder.build(); + this.collector = summaryBuilder.build(); } @Override diff --git a/metrics/core/src/test/java/org/hyperledger/besu/metrics/opentelemetry/OpenTelemetryMetricsSystemTest.java b/metrics/core/src/test/java/org/hyperledger/besu/metrics/opentelemetry/OpenTelemetryMetricsSystemTest.java index da34cda1cf1..5324d8cf3f9 100644 --- a/metrics/core/src/test/java/org/hyperledger/besu/metrics/opentelemetry/OpenTelemetryMetricsSystemTest.java +++ b/metrics/core/src/test/java/org/hyperledger/besu/metrics/opentelemetry/OpenTelemetryMetricsSystemTest.java @@ -266,7 +266,7 @@ public void shouldOnlyObserveEnabledMetrics() throws InterruptedException { final LabelledMetric counterN = localMetricSystem.createLabelledCounter( NETWORK, "ABC", "Not that kind of network", "show"); - assertThat(counterN).isSameAs(NoOpMetricsSystem.NO_OP_LABELLED_1_COUNTER); + assertThat(counterN).isInstanceOf(NoOpMetricsSystem.LabelCountingNoOpMetric.class); counterN.labels("show").inc(); assertThat(localMetricSystem.streamObservations()).isEmpty(); @@ -274,7 +274,7 @@ public void shouldOnlyObserveEnabledMetrics() throws InterruptedException { // do a category we are watching final LabelledMetric counterR = localMetricSystem.createLabelledCounter(RPC, "name", "Not useful", "method"); - assertThat(counterR).isNotSameAs(NoOpMetricsSystem.NO_OP_LABELLED_1_COUNTER); + assertThat(counterR).isNotInstanceOf(NoOpMetricsSystem.LabelCountingNoOpMetric.class); counterR.labels("op").inc(); assertThat(getObservation(localMetricSystem)) diff --git a/metrics/core/src/test/java/org/hyperledger/besu/metrics/prometheus/PrometheusMetricsSystemTest.java b/metrics/core/src/test/java/org/hyperledger/besu/metrics/prometheus/PrometheusMetricsSystemTest.java index 59f769f8fa3..9f1878f5998 100644 --- a/metrics/core/src/test/java/org/hyperledger/besu/metrics/prometheus/PrometheusMetricsSystemTest.java +++ b/metrics/core/src/test/java/org/hyperledger/besu/metrics/prometheus/PrometheusMetricsSystemTest.java @@ -244,7 +244,7 @@ public void shouldOnlyObserveEnabledMetrics() { // do a category we are not watching final LabelledMetric counterN = localMetricSystem.createLabelledCounter(NETWORK, "ABC", "Not that kind of network", "show"); - assertThat(counterN).isSameAs(NoOpMetricsSystem.NO_OP_LABELLED_1_COUNTER); + assertThat(counterN).isInstanceOf(NoOpMetricsSystem.LabelCountingNoOpMetric.class); counterN.labels("show").inc(); assertThat(localMetricSystem.streamObservations()).isEmpty(); @@ -252,7 +252,7 @@ public void shouldOnlyObserveEnabledMetrics() { // do a category we are watching final LabelledMetric counterR = localMetricSystem.createLabelledCounter(RPC, "name", "Not useful", "method"); - assertThat(counterR).isNotSameAs(NoOpMetricsSystem.NO_OP_LABELLED_1_COUNTER); + assertThat(counterR).isNotInstanceOf(NoOpMetricsSystem.LabelCountingNoOpMetric.class); counterR.labels("op").inc(); assertThat(localMetricSystem.streamObservations()) diff --git a/plugin-api/build.gradle b/plugin-api/build.gradle index 4e8435acdd5..58ce7b7c5e8 100644 --- a/plugin-api/build.gradle +++ b/plugin-api/build.gradle @@ -71,7 +71,7 @@ Calculated : ${currentHash} tasks.register('checkAPIChanges', FileStateChecker) { description = "Checks that the API for the Plugin-API project does not change without deliberate thought" files = sourceSets.main.allJava.files - knownHash = '5YqXajl5nrSGiQcU9PXzAvmSE8q65pMtIg4ejSZ3meI=' + knownHash = 'sLvLKRbqxri7hovTEcpiyF9eV4WnF8YZ1eGo5GF7mYw=' } check.dependsOn('checkAPIChanges') diff --git a/plugin-api/src/main/java/org/hyperledger/besu/plugin/services/MetricsSystem.java b/plugin-api/src/main/java/org/hyperledger/besu/plugin/services/MetricsSystem.java index d1652701743..7b466f85663 100644 --- a/plugin-api/src/main/java/org/hyperledger/besu/plugin/services/MetricsSystem.java +++ b/plugin-api/src/main/java/org/hyperledger/besu/plugin/services/MetricsSystem.java @@ -256,7 +256,6 @@ default void createSummary( final Supplier summarySupplier) { createLabelledSuppliedSummary(category, name, help).labels(summarySupplier); } - ; /** * Collect metrics from Guava cache.