Skip to content

Commit

Permalink
Update code for new Prometheus simpleclient version
Browse files Browse the repository at this point in the history
Signed-off-by: Fabio Di Fabio <[email protected]>
  • Loading branch information
fab-10 committed Jan 8, 2024
1 parent cd99332 commit 8ba7d43
Show file tree
Hide file tree
Showing 2 changed files with 106 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand All @@ -58,6 +58,7 @@ public class PrometheusMetricsSystem implements ObservableMetricsSystem {
cachedCounters = new ConcurrentHashMap<>();
private final Map<String, LabelledMetric<OperationTimer>> cachedTimers =
new ConcurrentHashMap<>();
private final Set<String> totalSuffixedCounters = new ConcurrentHashSet<>();

private final Set<MetricCategory> enabledCategories;
private final boolean timersEnabled;
Expand Down Expand Up @@ -95,7 +96,7 @@ public LabelledMetric<org.hyperledger.besu.plugin.services.metrics.Counter> 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) -> {
Expand Down Expand Up @@ -185,9 +186,7 @@ private void addCollectorUnchecked(final MetricCategory category, final Collecto
category, key -> Collections.newSetFromMap(new ConcurrentHashMap<>()));

final List<String> newSamples =
metric.collect().stream()
.map(metricFamilySamples -> metricFamilySamples.name)
.collect(Collectors.toList());
metric.collect().stream().map(metricFamilySamples -> metricFamilySamples.name).toList();

metrics.stream()
.filter(
Expand Down Expand Up @@ -230,13 +229,30 @@ 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),
sample.value,
sample.labelValues);
}

private Observation convertCounterNamesToLabels(
final MetricCategory category, final Sample sample, final MetricFamilySamples familySamples) {
final List<String> 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<String> labelValues = new ArrayList<>(sample.labelValues);
Expand All @@ -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");
}
Expand All @@ -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() + "_";
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -51,6 +52,11 @@ public class PrometheusMetricsSystemTest {
Comparator.<Observation, String>comparing(observation -> observation.getCategory().getName())
.thenComparing(Observation::getMetricName)
.thenComparing((o1, o2) -> o1.getLabels().equals(o2.getLabels()) ? 0 : 1);
private static final Comparator<Observation> WITH_VALUES =
Comparator.<Observation, String>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() {
Expand All @@ -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
Expand All @@ -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> 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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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")));
}

Expand Down Expand Up @@ -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);
}
}

0 comments on commit 8ba7d43

Please sign in to comment.