diff --git a/metrics/core/src/main/java/org/hyperledger/besu/metrics/prometheus/PrometheusMetricsSystem.java b/metrics/core/src/main/java/org/hyperledger/besu/metrics/prometheus/PrometheusMetricsSystem.java index fa07670521a..bab2c5c7d3f 100644 --- a/metrics/core/src/main/java/org/hyperledger/besu/metrics/prometheus/PrometheusMetricsSystem.java +++ b/metrics/core/src/main/java/org/hyperledger/besu/metrics/prometheus/PrometheusMetricsSystem.java @@ -32,7 +32,6 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.function.DoubleSupplier; import java.util.function.Supplier; -import java.util.stream.Collectors; import java.util.stream.Stream; import com.google.common.collect.ImmutableSet; @@ -48,6 +47,7 @@ import io.prometheus.client.hotspot.MemoryPoolsExports; import io.prometheus.client.hotspot.StandardExports; import io.prometheus.client.hotspot.ThreadExports; +import io.vertx.core.impl.ConcurrentHashSet; /** The Prometheus metrics system. */ public class PrometheusMetricsSystem implements ObservableMetricsSystem { @@ -58,6 +58,7 @@ public class PrometheusMetricsSystem implements ObservableMetricsSystem { cachedCounters = new ConcurrentHashMap<>(); private final Map> cachedTimers = new ConcurrentHashMap<>(); + private final Set totalSuffixedCounters = new ConcurrentHashSet<>(); private final Set enabledCategories; private final boolean timersEnabled; @@ -95,7 +96,7 @@ public LabelledMetric crea final String name, final String help, final String... labelNames) { - final String metricName = convertToPrometheusName(category, name); + final String metricName = convertToPrometheusCounterName(category, name); return cachedCounters.computeIfAbsent( metricName, (k) -> { @@ -185,9 +186,7 @@ private void addCollectorUnchecked(final MetricCategory category, final Collecto category, key -> Collections.newSetFromMap(new ConcurrentHashMap<>())); final List newSamples = - metric.collect().stream() - .map(metricFamilySamples -> metricFamilySamples.name) - .collect(Collectors.toList()); + metric.collect().stream().map(metricFamilySamples -> metricFamilySamples.name).toList(); metrics.stream() .filter( @@ -230,6 +229,9 @@ private Observation createObservationFromSample( if (familySamples.type == Collector.Type.SUMMARY) { return convertSummarySampleNamesToLabels(category, sample, familySamples); } + if (familySamples.type == Collector.Type.COUNTER) { + return convertCounterNamesToLabels(category, sample, familySamples); + } return new Observation( category, convertFromPrometheusName(category, sample.name), @@ -237,6 +239,20 @@ private Observation createObservationFromSample( sample.labelValues); } + private Observation convertCounterNamesToLabels( + final MetricCategory category, final Sample sample, final MetricFamilySamples familySamples) { + final List labelValues = new ArrayList<>(sample.labelValues); + if (sample.name.endsWith("_created")) { + labelValues.add("created"); + } + + return new Observation( + category, + convertFromPrometheusCounterName(category, familySamples.name), + sample.value, + labelValues); + } + private Observation convertHistogramSampleNamesToLabels( final MetricCategory category, final Sample sample, final MetricFamilySamples familySamples) { final List labelValues = new ArrayList<>(sample.labelValues); @@ -259,6 +275,8 @@ private Observation convertSummarySampleNamesToLabels( labelValues.add("sum"); } else if (sample.name.endsWith("_count")) { labelValues.add("count"); + } else if (sample.name.endsWith("_created")) { + labelValues.add("created"); } else { labelValues.add(labelValues.size() - 1, "quantile"); } @@ -280,11 +298,36 @@ public String convertToPrometheusName(final MetricCategory category, final Strin return prometheusPrefix(category) + name; } + /** + * Convert to prometheus counter name. Prometheus adds a _total suffix to the name if not present, + * so we remember if the original name already has it, to be able to covert back correctly + * + * @param category the category + * @param name the name + * @return the name as string + */ + public String convertToPrometheusCounterName(final MetricCategory category, final String name) { + if (name.endsWith("_total")) { + totalSuffixedCounters.add(name); + } + return convertFromPrometheusName(category, name); + } + private String convertFromPrometheusName(final MetricCategory category, final String metricName) { final String prefix = prometheusPrefix(category); return metricName.startsWith(prefix) ? metricName.substring(prefix.length()) : metricName; } + private String convertFromPrometheusCounterName( + final MetricCategory category, final String metricName) { + final String prefix = prometheusPrefix(category); + final String unPrefixedName = + metricName.startsWith(prefix) ? metricName.substring(prefix.length()) : metricName; + return totalSuffixedCounters.contains(unPrefixedName + "_total") + ? unPrefixedName + "_total" + : unPrefixedName; + } + private String prometheusPrefix(final MetricCategory category) { return category.getApplicationPrefix().orElse("") + category.getName() + "_"; } 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 7c62f1f0b6c..2ddf86d347b 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 @@ -17,6 +17,7 @@ import static java.util.Arrays.asList; import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; +import static java.util.function.Predicate.not; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.hyperledger.besu.metrics.BesuMetricCategory.DEFAULT_METRIC_CATEGORIES; @@ -51,6 +52,11 @@ public class PrometheusMetricsSystemTest { Comparator.comparing(observation -> observation.getCategory().getName()) .thenComparing(Observation::getMetricName) .thenComparing((o1, o2) -> o1.getLabels().equals(o2.getLabels()) ? 0 : 1); + private static final Comparator WITH_VALUES = + Comparator.comparing(observation -> observation.getCategory().getName()) + .thenComparing(Observation::getMetricName) + .thenComparing((o1, o2) -> o1.getLabels().equals(o2.getLabels()) ? 0 : 1) + .thenComparing((o1, o2) -> o1.getValue().equals(o2.getValue()) ? 0 : 1); @BeforeEach public void resetGlobalOpenTelemetry() { @@ -66,11 +72,17 @@ public void shouldCreateObservationFromCounter() { counter.inc(); assertThat(metricsSystem.streamObservations()) - .containsExactly(new Observation(PEERS, "connected", 1.0, emptyList())); + .usingElementComparator(this::compareCounters) + .containsExactlyInAnyOrder( + new Observation(PEERS, "connected", 1.0, emptyList()), + new Observation(PEERS, "connected", null, List.of("created"))); counter.inc(); assertThat(metricsSystem.streamObservations()) - .containsExactly(new Observation(PEERS, "connected", 2.0, emptyList())); + .usingElementComparator(this::compareCounters) + .containsExactly( + new Observation(PEERS, "connected", 2.0, emptyList()), + new Observation(PEERS, "connected", null, List.of("created"))); } @Test @@ -83,26 +95,36 @@ public void shouldHandleDuplicateCounterCreation() { counter1.labels().inc(); assertThat(metricsSystem.streamObservations()) - .containsExactly(new Observation(PEERS, "connected", 1.0, emptyList())); + .usingElementComparator(this::compareCounters) + .containsExactly( + new Observation(PEERS, "connected", 1.0, emptyList()), + new Observation(PEERS, "connected", null, List.of("created"))); counter2.labels().inc(); assertThat(metricsSystem.streamObservations()) - .containsExactly(new Observation(PEERS, "connected", 2.0, emptyList())); + .usingElementComparator(this::compareCounters) + .containsExactly( + new Observation(PEERS, "connected", 2.0, emptyList()), + new Observation(PEERS, "connected", null, List.of("created"))); } @Test public void shouldCreateSeparateObservationsForEachCounterLabelValue() { final LabelledMetric counter = - metricsSystem.createLabelledCounter(PEERS, "connected", "Some help string", "labelName"); + metricsSystem.createLabelledCounter( + PEERS, "connected_total", "Some help string", "labelName"); counter.labels("value1").inc(); counter.labels("value2").inc(); counter.labels("value1").inc(); assertThat(metricsSystem.streamObservations()) + .usingElementComparator(this::compareCounters) .containsExactlyInAnyOrder( - new Observation(PEERS, "connected", 2.0, singletonList("value1")), - new Observation(PEERS, "connected", 1.0, singletonList("value2"))); + new Observation(PEERS, "connected_total", 2.0, singletonList("value1")), + new Observation(PEERS, "connected_total", 1.0, singletonList("value2")), + new Observation(PEERS, "connected_total", null, List.of("value1", "created")), + new Observation(PEERS, "connected_total", null, List.of("value2", "created"))); } @Test @@ -138,11 +160,18 @@ public void shouldIncrementCounterBySpecifiedAmount() { counter.inc(5); assertThat(metricsSystem.streamObservations()) - .containsExactly(new Observation(PEERS, "connected", 5.0, emptyList())); + .usingElementComparator(this::compareCounters) + .containsExactly( + new Observation(PEERS, "connected", 5.0, emptyList()), + new Observation(PEERS, "connected", null, List.of("created"))); counter.inc(6); assertThat(metricsSystem.streamObservations()) - .containsExactly(new Observation(PEERS, "connected", 11.0, emptyList())); + .usingDefaultElementComparator() + .usingElementComparator(this::compareCounters) + .containsExactly( + new Observation(PEERS, "connected", 11.0, emptyList()), + new Observation(PEERS, "connected", null, List.of("created"))); } @Test @@ -162,7 +191,8 @@ public void shouldCreateObservationsFromTimer() { new Observation(RPC, "request", null, asList("quantile", "0.99")), new Observation(RPC, "request", null, asList("quantile", "1.0")), new Observation(RPC, "request", null, singletonList("sum")), - new Observation(RPC, "request", null, singletonList("count"))); + new Observation(RPC, "request", null, singletonList("count")), + new Observation(RPC, "request", null, singletonList("created"))); } @Test @@ -192,7 +222,8 @@ public void shouldCreateObservationsFromTimerWithLabels() { new Observation(RPC, "request", null, asList("method", "quantile", "0.99")), new Observation(RPC, "request", null, asList("method", "quantile", "1.0")), new Observation(RPC, "request", null, asList("method", "sum")), - new Observation(RPC, "request", null, asList("method", "count"))); + new Observation(RPC, "request", null, asList("method", "count")), + new Observation(RPC, "request", null, asList("method", "created"))); } @Test @@ -251,6 +282,8 @@ public void shouldOnlyObserveEnabledMetrics() { counterR.labels("op").inc(); assertThat(localMetricSystem.streamObservations()) + .usingRecursiveFieldByFieldElementComparator() + .filteredOn(not(this::isCreatedSample)) .containsExactly(new Observation(RPC, "name", 1.0, singletonList("op"))); } @@ -280,4 +313,18 @@ public void returnsNoOpMetricsWhenPushEnabled() { assertThat(localMetricSystem).isInstanceOf(PrometheusMetricsSystem.class); } + + private boolean isCreatedSample(final Observation obs) { + // Simple client 0.10.0 add a _created sample to every counter, histogram and summary, that we + // may want to ignore + return obs.getLabels().contains("created"); + } + + private int compareCounters(final Observation obs1, final Observation obs2) { + // for created samples ignore values + if (obs1.getLabels().contains("created") && obs2.getLabels().contains("created")) { + return IGNORE_VALUES.compare(obs1, obs2); + } + return WITH_VALUES.compare(obs1, obs2); + } }