Skip to content

Commit

Permalink
Prometheus format problems (helidon-io#1427) Do not repeat LINE and H…
Browse files Browse the repository at this point in the history
…ELP lines in Prometheus format as this is a violation that can be flagged by tools with validation enabled. This can happen for metrics with same name and different tags. We now keep track of TYPE and HELP lines added to format to avoid repetition.
  • Loading branch information
spericas committed Feb 24, 2020
1 parent a446a13 commit d1d41ba
Show file tree
Hide file tree
Showing 14 changed files with 132 additions and 51 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,13 @@ public void jsonData(JsonObjectBuilder builder, MetricID metricID) {
}

@Override
public void prometheusData(StringBuilder sb, MetricID metricID) {
public void prometheusData(StringBuilder sb, MetricID metricID, boolean withHelpType) {
String name = prometheusNameWithUnits(metricID);
final String nameCurrent = name + "_current";
prometheusType(sb, nameCurrent, metadata().getType());
prometheusHelp(sb, nameCurrent);
if (withHelpType) {
prometheusType(sb, nameCurrent, metadata().getType());
prometheusHelp(sb, nameCurrent);
}
sb.append(nameCurrent).append(prometheusTags(metricID.getTags()))
.append(" ").append(prometheusValue()).append('\n');
final String nameMin = name + "_min";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public Snapshot getSnapshot() {
}

@Override
public void prometheusData(StringBuilder sb, MetricID metricID) {
public void prometheusData(StringBuilder sb, MetricID metricID, boolean withHelpType) {
Units units = getUnits();
String tags = prometheusTags(metricID.getTags());
String name = metricID.getName();
Expand All @@ -84,7 +84,9 @@ public void prometheusData(StringBuilder sb, MetricID metricID) {
// # TYPE application:file_sizes_mean_bytes gauge
// application:file_sizes_mean_bytes 4738.231
nameUnits = prometheusNameWithUnits(name + "_mean", unit);
prometheusType(sb, nameUnits, "gauge");
if (withHelpType) {
prometheusType(sb, nameUnits, "gauge");
}
sb.append(nameUnits)
.append(tags)
.append(" ")
Expand All @@ -94,7 +96,9 @@ public void prometheusData(StringBuilder sb, MetricID metricID) {
// # TYPE application:file_sizes_max_bytes gauge
// application:file_sizes_max_bytes 31716
nameUnits = prometheusNameWithUnits(name + "_max", unit);
prometheusType(sb, nameUnits, "gauge");
if (withHelpType) {
prometheusType(sb, nameUnits, "gauge");
}
sb.append(nameUnits)
.append(tags)
.append(" ")
Expand All @@ -104,7 +108,9 @@ public void prometheusData(StringBuilder sb, MetricID metricID) {
// # TYPE application:file_sizes_min_bytes gauge
// application:file_sizes_min_bytes 180
nameUnits = prometheusNameWithUnits(name + "_min", unit);
prometheusType(sb, nameUnits, "gauge");
if (withHelpType) {
prometheusType(sb, nameUnits, "gauge");
}
sb.append(nameUnits)
.append(tags)
.append(" ")
Expand All @@ -114,7 +120,9 @@ public void prometheusData(StringBuilder sb, MetricID metricID) {
// # TYPE application:file_sizes_stddev_bytes gauge
// application:file_sizes_stddev_bytes 1054.7343037063602
nameUnits = prometheusNameWithUnits(name + "_stddev", unit);
prometheusType(sb, nameUnits, "gauge");
if (withHelpType) {
prometheusType(sb, nameUnits, "gauge");
}
sb.append(nameUnits)
.append(tags)
.append(" ")
Expand All @@ -125,8 +133,10 @@ public void prometheusData(StringBuilder sb, MetricID metricID) {
// # HELP application:file_sizes_bytes Users file size
// application:file_sizes_bytes_count 2037
nameUnits = prometheusNameWithUnits(name, unit);
prometheusType(sb, nameUnits, "summary");
prometheusHelp(sb, nameUnits);
if (withHelpType) {
prometheusType(sb, nameUnits, "summary");
prometheusHelp(sb, nameUnits);
}
nameUnits = prometheusNameWithUnits(name, unit) + "_count";
sb.append(nameUnits)
.append(tags)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,15 @@ static HelidonMeter create(String type, Metadata metadata, Meter delegate) {
application:requests_fifteen_min_rate_per_second 12.126
*/
@Override
public void prometheusData(StringBuilder sb, MetricID metricID) {
public void prometheusData(StringBuilder sb, MetricID metricID, boolean withHelpType) {
String name = metricID.getName();
String nameUnits = prometheusNameWithUnits(name, Optional.empty()) + "_total";
String tags = prometheusTags(metricID.getTags());
prometheusType(sb, nameUnits, "counter");
prometheusHelp(sb, nameUnits);

if (withHelpType) {
prometheusType(sb, nameUnits, "counter");
prometheusHelp(sb, nameUnits);
}
sb.append(nameUnits)
.append(tags)
.append(" ")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,9 @@ interface HelidonMetric extends Metric {
*
* @param sb the {@code StringBuilder} used to accumulate the output
* @param metricID the {@code MetricID} for the metric to be formatted
* @param withHelpType flag to control if TYPE and HELP are to be included
*/
void prometheusData(StringBuilder sb, MetricID metricID);
void prometheusData(StringBuilder sb, MetricID metricID, boolean withHelpType);

/**
* Return a name for this metric, possibly including a unit suffix.
Expand Down
40 changes: 29 additions & 11 deletions metrics/metrics/src/main/java/io/helidon/metrics/HelidonTimer.java
Original file line number Diff line number Diff line change
Expand Up @@ -106,36 +106,44 @@ public Snapshot getSnapshot() {
}

@Override
public void prometheusData(StringBuilder sb, MetricID metricID) {
public void prometheusData(StringBuilder sb, MetricID metricID, boolean withHelpType) {
String nameUnits;
String name = metricID.getName();
String tags = prometheusTags(metricID.getTags());
nameUnits = prometheusNameWithUnits(name, Optional.empty()) + "_rate_per_second";
prometheusType(sb, nameUnits, "gauge");
if (withHelpType) {
prometheusType(sb, nameUnits, "gauge");
}
sb.append(nameUnits)
.append(tags)
.append(" ")
.append(getMeanRate())
.append("\n");

nameUnits = prometheusNameWithUnits(name, Optional.empty()) + "_one_min_rate_per_second";
prometheusType(sb, nameUnits, "gauge");
if (withHelpType) {
prometheusType(sb, nameUnits, "gauge");
}
sb.append(nameUnits)
.append(tags)
.append(" ")
.append(getOneMinuteRate())
.append("\n");

nameUnits = prometheusNameWithUnits(name, Optional.empty()) + "_five_min_rate_per_second";
prometheusType(sb, nameUnits, "gauge");
if (withHelpType) {
prometheusType(sb, nameUnits, "gauge");
}
sb.append(nameUnits)
.append(tags)
.append(" ")
.append(getFiveMinuteRate())
.append("\n");

nameUnits = prometheusNameWithUnits(name, Optional.empty()) + "_fifteen_min_rate_per_second";
prometheusType(sb, nameUnits, "gauge");
if (withHelpType) {
prometheusType(sb, nameUnits, "gauge");
}
sb.append(nameUnits)
.append(tags)
.append(" ")
Expand All @@ -147,40 +155,50 @@ public void prometheusData(StringBuilder sb, MetricID metricID) {
Snapshot snap = getSnapshot();

nameUnits = prometheusNameWithUnits(name + "_mean", unit);
prometheusType(sb, nameUnits, "gauge");
if (withHelpType) {
prometheusType(sb, nameUnits, "gauge");
}
sb.append(nameUnits)
.append(tags)
.append(" ")
.append(units.convert(snap.getMean()))
.append("\n");

nameUnits = prometheusNameWithUnits(name + "_max", unit);
prometheusType(sb, nameUnits, "gauge");
if (withHelpType) {
prometheusType(sb, nameUnits, "gauge");
}
sb.append(nameUnits)
.append(tags)
.append(" ")
.append(units.convert(snap.getMax()))
.append("\n");

nameUnits = prometheusNameWithUnits(name + "_min", unit);
prometheusType(sb, nameUnits, "gauge");
if (withHelpType) {
prometheusType(sb, nameUnits, "gauge");
}
sb.append(nameUnits)
.append(tags)
.append(" ")
.append(units.convert(snap.getMin()))
.append("\n");

nameUnits = prometheusNameWithUnits(name + "_stddev", unit);
prometheusType(sb, nameUnits, "gauge");
if (withHelpType) {
prometheusType(sb, nameUnits, "gauge");
}
sb.append(nameUnits)
.append(tags)
.append(" ")
.append(units.convert(snap.getStdDev()))
.append("\n");

nameUnits = prometheusNameWithUnits(name, unit);
prometheusType(sb, nameUnits, "summary");
prometheusHelp(sb, nameUnits);
if (withHelpType) {
prometheusType(sb, nameUnits, "summary");
prometheusHelp(sb, nameUnits);
}
nameUnits = prometheusNameWithUnits(name, unit) + "_count";
sb.append(nameUnits)
.append(tags)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,10 +214,12 @@ void prometheusHelp(StringBuilder sb, String nameWithUnits) {
}

@Override
public void prometheusData(StringBuilder sb, MetricID metricID) {
public void prometheusData(StringBuilder sb, MetricID metricID, boolean withHelpType) {
String nameWithUnits = prometheusNameWithUnits(metricID);
prometheusType(sb, nameWithUnits, metadata.getType());
prometheusHelp(sb, nameWithUnits);
if (withHelpType) {
prometheusType(sb, nameWithUnits, metadata.getType());
prometheusHelp(sb, nameWithUnits);
}
sb.append(nameWithUnits).append(prometheusTags(metricID.getTags())).append(" ").append(prometheusValue()).append('\n');
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,12 @@
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.function.BiConsumer;
import java.util.function.Function;
import java.util.function.Supplier;
Expand Down Expand Up @@ -187,29 +189,39 @@ private void optionsAll(ServerRequest req, ServerResponse res, Registry registry

static String toPrometheusData(Registry... registries) {
return Arrays.stream(registries)
.filter(r -> !r.empty())
.map(MetricsSupport::toPrometheusData)
.collect(Collectors.joining());
}

static String toPrometheusData(Registry registry) {
return registry.stream()
.sorted(Comparator.comparing(Map.Entry::getKey))
.collect(StringBuilder::new,
(sb, entry) -> toPrometheusData(sb, entry.getKey(), entry.getValue()),
StringBuilder::append)
.toString();
StringBuilder builder = new StringBuilder();
Set<String> serialized = new HashSet<>();
registry.stream()
.sorted(Map.Entry.comparingByKey())
.forEach(entry -> {
String name = entry.getKey().getName();
if (!serialized.contains(name)) {
toPrometheusData(builder, entry.getKey(), entry.getValue(), true);
serialized.add(name);
} else {
toPrometheusData(builder, entry.getKey(), entry.getValue(), false);
}
});
return builder.toString();
}

/**
* Formats a metric in Prometheus format.
*
* @param metricID the {@code MetricID} for the metric to be formatted
* @param metric the {@code Metric} containing the data to be formatted
* @param withHelpType flag controlling serialization of HELP and TYPE
* @return metric info in Prometheus format
*/
public static String toPrometheusData(MetricID metricID, Metric metric) {
public static String toPrometheusData(MetricID metricID, Metric metric, boolean withHelpType) {
final StringBuilder sb = new StringBuilder();
checkMetricTypeThenRun(sb, metricID, metric);
checkMetricTypeThenRun(sb, metricID, metric, withHelpType);
return sb.toString();
}

Expand All @@ -218,10 +230,11 @@ public static String toPrometheusData(MetricID metricID, Metric metric) {
*
* @param name the name of the metric
* @param metric the {@code Metric} containing the data to be formatted
* @param withHelpType flag controlling serialization of HELP and TYPE
* @return metric info in Prometheus format
*/
public static String toPrometheusData(String name, Metric metric) {
return toPrometheusData(new MetricID(name), metric);
public static String toPrometheusData(String name, Metric metric, boolean withHelpType) {
return toPrometheusData(new MetricID(name), metric, withHelpType);
}

/**
Expand All @@ -233,13 +246,15 @@ public static String toPrometheusData(String name, Metric metric) {
*
* @param metricID the {@code MetricID} for the metric to convert
* @param metric the {@code Metric} to convert to Prometheus format
* @param withHelpType flag controlling serialization of HELP and TYPE
* @return {@code String} containing the Prometheus data
*/
static void toPrometheusData(StringBuilder sb, MetricID metricID, Metric metric) {
checkMetricTypeThenRun(sb, metricID, metric);
static void toPrometheusData(StringBuilder sb, MetricID metricID, Metric metric, boolean withHelpType) {
checkMetricTypeThenRun(sb, metricID, metric, withHelpType);
}

private static void checkMetricTypeThenRun(StringBuilder sb, MetricID metricID, Metric metric) {
private static void checkMetricTypeThenRun(StringBuilder sb, MetricID metricID, Metric metric,
boolean withHelpType) {
Objects.requireNonNull(metric);

if (!(metric instanceof HelidonMetric)) {
Expand All @@ -249,7 +264,7 @@ private static void checkMetricTypeThenRun(StringBuilder sb, MetricID metricID,
HelidonMetric.class.getName()));
}

((HelidonMetric) metric).prometheusData(sb, metricID);
((HelidonMetric) metric).prometheusData(sb, metricID, withHelpType);
}

// unit testable
Expand Down Expand Up @@ -418,7 +433,7 @@ private void getOne(ServerRequest req, ServerResponse res, Registry registry) {
res.send(builder.build());
} else if (mediaType == MediaType.TEXT_PLAIN) {
final StringBuilder sb = new StringBuilder();
entry.getValue().prometheusData(sb, entry.getKey());
entry.getValue().prometheusData(sb, entry.getKey(), true);
res.send(sb.toString());
} else {
res.status(Http.Status.NOT_ACCEPTABLE_406);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,14 +109,14 @@ void testPrometheusData() {
+ "# HELP base_theName_total theDescription\n"
+ "base_theName_total{a=\"b\",c=\"d\"} 17\n";

counter.prometheusData(sb, counterID);
counter.prometheusData(sb, counterID, true);
assertThat(sb.toString(), is(expected));

expected = "# TYPE base_theName_total counter\n"
+ "# HELP base_theName_total theDescription\n"
+ "base_theName_total 49\n";
sb = new StringBuilder();
wrappingCounter.prometheusData(sb, wrappingCounterID);
wrappingCounter.prometheusData(sb, wrappingCounterID, true);
assertThat(sb.toString(), is(expected));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ void testJson() {
@Test
void testPrometheus() throws IOException, ParseException {
final StringBuilder sb = new StringBuilder();
histoInt.prometheusData(sb, histoIntID);
histoInt.prometheusData(sb, histoIntID, true);
parsePrometheusText(new LineNumberReader(new StringReader(sb.toString())).lines())
.forEach(entry -> assertThat("Unexpected value checking " + entry.getKey(),
EXPECTED_PROMETHEUS_RESULTS.get(entry.getKey()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ void testJson() {
@Test
void testPrometheus() {
final StringBuilder sb = new StringBuilder();
meter.prometheusData(sb, meterID);
meter.prometheusData(sb, meterID, true);
String data = sb.toString();

assertThat(data, startsWith(EXPECTED_PROMETHEUS_START));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ void testJson() {
@Test
void testPrometheus() {
final StringBuilder sb = new StringBuilder();
dataSetTimer.prometheusData(sb, dataSetTimerID);
dataSetTimer.prometheusData(sb, dataSetTimerID, true);
final String prometheusData = sb.toString();
assertThat(prometheusData, startsWith("# TYPE application_response_time_rate_per_second gauge\n"
+ "application_response_time_rate_per_second 200.0\n"
Expand Down
Loading

0 comments on commit d1d41ba

Please sign in to comment.