Skip to content

Commit

Permalink
Fix OpenMetrics formatting error (3.x) (#4900)
Browse files Browse the repository at this point in the history
  • Loading branch information
@tjquinno
tjquinno authored Sep 20, 2022
1 parent 84123e8 commit 005679c
Show file tree
Hide file tree
Showing 4 changed files with 138 additions and 19 deletions.
8 changes: 8 additions & 0 deletions common/http/src/main/java/io/helidon/common/http/MediaType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,11 @@ public final class MediaType implements AcceptPredicate<MediaType> {
*/
public static final MediaType APPLICATION_X_NDJSON;

/**
* A {@link MediaType} constant representing the {@code application/openmetrics-text} media type.
*/
public static final MediaType APPLICATION_OPENMETRICS;

static {
Map<String, MediaType> knownTypes = new HashMap<>();

Expand Down Expand Up @@ -221,6 +226,9 @@ public final class MediaType implements AcceptPredicate<MediaType> {
APPLICATION_X_NDJSON = new MediaType("application", "x-ndjson");
knownTypes.put("application/x-ndjson", APPLICATION_X_NDJSON);

APPLICATION_OPENMETRICS = new MediaType("application", "openmetrics-text");
knownTypes.put("application/openmetrics-text", APPLICATION_OPENMETRICS);

KNOWN_TYPES = Collections.unmodifiableMap(knownTypes);
}

Expand Down
43 changes: 34 additions & 9 deletions metrics/metrics/src/main/java/io/helidon/metrics/HelidonSimpleTimer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ public void prometheusData(StringBuilder sb, MetricID metricID, boolean withHelp
SimpleTimerImpl simpleTimerImpl = (delegate instanceof SimpleTimerImpl) ? ((SimpleTimerImpl) delegate) : null;
Sample.Labeled sample = simpleTimerImpl != null ? simpleTimerImpl.sample : null;
if (sample != null) {
sb.append(prometheusExemplar(elapsedTimeInSeconds(sample.value()), simpleTimerImpl.sample));
sb.append(prometheusExemplar(1, sample)); // exemplar always contributes 1 to the count
}
sb.append("\n");

Expand All @@ -124,6 +124,7 @@ public void prometheusData(StringBuilder sb, MetricID metricID, boolean withHelp
.append(tags)
.append(" ")
.append(elapsedTimeInSeconds())
.append(exemplarForElapsedTime(sample))
.append("\n");

promName = prometheusNameWithUnits(name + "_maxTimeDuration", Optional.of(MetricUnits.SECONDS));
Expand All @@ -134,6 +135,8 @@ public void prometheusData(StringBuilder sb, MetricID metricID, boolean withHelp
.append(tags)
.append(" ")
.append(durationPrometheusOutput(getMaxTimeDuration()))
.append(exemplarForElapsedTime(getMaxTimeDuration(),
simpleTimerImpl == null ? null : simpleTimerImpl.lastMaxSample))
.append("\n");

promName = prometheusNameWithUnits(name + "_minTimeDuration", Optional.of(MetricUnits.SECONDS));
Expand All @@ -144,13 +147,9 @@ public void prometheusData(StringBuilder sb, MetricID metricID, boolean withHelp
.append(tags)
.append(" ")
.append(durationPrometheusOutput(getMinTimeDuration()))
.append(exemplarForElapsedTime(getMinTimeDuration(),
simpleTimerImpl == null ? null : simpleTimerImpl.lastMinSample))
.append("\n");


if (sample != null) {
sb.append(prometheusExemplar(elapsedTimeInSeconds(sample.value()), sample));
}
sb.append("\n");
}

@Override
Expand All @@ -168,6 +167,15 @@ public void jsonData(JsonObjectBuilder builder, MetricID metricID) {
builder.add(metricID.getName(), myBuilder);
}

private String exemplarForElapsedTime(Duration duration, Sample.Labeled sample) {
// If the duration is null, then there is no value to which a sample might have contributed. So there's no exemplar.
return duration == null ? "" : exemplarForElapsedTime(sample);
}

private String exemplarForElapsedTime(Sample.Labeled sample) {
return sample == null ? "" : prometheusExemplar(sample.value(), sample);
}

private static String durationPrometheusOutput(Duration duration) {
return duration == null ? "NaN" : Double.toString(((double) duration.toNanos()) / 1000.0 / 1000.0 / 1000.0);
}
Expand Down Expand Up @@ -221,6 +229,10 @@ private static class SimpleTimerImpl implements SimpleTimer {
private long lastMinute;

private Sample.Labeled sample = null;
private Sample.Labeled currentMaxSample = null;
private Sample.Labeled currentMinSample = null;
private Sample.Labeled lastMaxSample = null;
private Sample.Labeled lastMinSample = null;

private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock();

Expand Down Expand Up @@ -300,9 +312,11 @@ private void update(long nanos) {
updateStateLocked();
if (currentMin == null || currentMin.toNanos() > nanos) {
currentMin = Duration.ofNanos(nanos);
currentMinSample = sample;
}
if (currentMax == null || currentMax.toNanos() < nanos) {
currentMax = Duration.ofNanos(nanos);
currentMaxSample = sample;
}
}
return null;
Expand All @@ -324,6 +338,10 @@ private void updateStateLocked() {
lastMin = currentMin;
currentMax = null;
currentMin = null;
lastMaxSample = currentMaxSample;
lastMinSample = currentMinSample;
currentMaxSample = null;
currentMinSample = null;
lastMinute = currentMinute;
}
}
Expand All @@ -341,12 +359,19 @@ public boolean equals(Object o) {
&& elapsed.equals(that.elapsed)
&& Objects.equals(lastMin, that.lastMin)
&& Objects.equals(lastMax, that.lastMax)
&& Objects.equals(sample, that.sample);
&& Objects.equals(sample, that.sample)
&& Objects.equals(currentMin, that.currentMin)
&& Objects.equals(currentMax, that.currentMax)
&& Objects.equals(currentMinSample, that.currentMinSample)
&& Objects.equals(currentMaxSample, that.currentMaxSample)
&& Objects.equals(lastMinSample, that.lastMinSample)
&& Objects.equals(lastMaxSample, that.lastMaxSample);
}

@Override
public int hashCode() {
return Objects.hash(counter, elapsed, lastMin, lastMax, sample);
return Objects.hash(counter, elapsed, lastMin, lastMax, sample, currentMin, currentMax,
currentMinSample, currentMaxSample, lastMinSample, lastMaxSample);
}

private <T> T writeAccess(Callable<T> action) {
Expand Down
46 changes: 36 additions & 10 deletions metrics/metrics/src/main/java/io/helidon/metrics/MetricsSupport.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,9 @@ public static Builder builder() {
}

private static MediaType findBestAccepted(RequestHeaders headers) {
Optional<MediaType> mediaType = headers.bestAccepted(MediaType.TEXT_PLAIN, MediaType.APPLICATION_JSON);
Optional<MediaType> mediaType = headers.bestAccepted(MediaType.TEXT_PLAIN,
MediaType.APPLICATION_JSON,
MediaType.APPLICATION_OPENMETRICS);
return mediaType.orElse(null);
}

Expand All @@ -219,10 +221,10 @@ private static void getAll(ServerRequest req, ServerResponse res, Registry regis
}

MediaType mediaType = findBestAccepted(req.headers());
if (mediaType == MediaType.APPLICATION_JSON) {
if (matches(mediaType, MediaType.APPLICATION_JSON)) {
sendJson(res, toJsonData(registry));
} else if (mediaType == MediaType.TEXT_PLAIN) {
res.send(toPrometheusData(registry));
} else if (matches(mediaType, MediaType.TEXT_PLAIN, MediaType.APPLICATION_OPENMETRICS)) {
sendPrometheus(res, toPrometheusData(registry), mediaType);
} else {
res.status(Http.Status.NOT_ACCEPTABLE_406);
res.send();
Expand Down Expand Up @@ -524,10 +526,10 @@ private void getByName(ServerRequest req, ServerResponse res, Registry registry)
registry.getOptionalMetricEntry(metricName)
.ifPresentOrElse(entry -> {
MediaType mediaType = findBestAccepted(req.headers());
if (mediaType == MediaType.APPLICATION_JSON) {
if (matches(mediaType, MediaType.APPLICATION_JSON)) {
sendJson(res, jsonDataByName(registry, metricName));
} else if (mediaType == MediaType.TEXT_PLAIN) {
res.send(prometheusDataByName(registry, metricName));
} else if (matches(mediaType, MediaType.TEXT_PLAIN, MediaType.APPLICATION_OPENMETRICS)) {
sendPrometheus(res, prometheusDataByName(registry, metricName), mediaType);
} else {
res.status(Http.Status.NOT_ACCEPTABLE_406);
res.send();
Expand Down Expand Up @@ -569,10 +571,10 @@ private static void sendJson(ServerResponse res, JsonObject object) {
private void getMultiple(ServerRequest req, ServerResponse res, Registry... registries) {
MediaType mediaType = findBestAccepted(req.headers());
res.cachingStrategy(ServerResponse.CachingStrategy.NO_CACHING);
if (mediaType == MediaType.APPLICATION_JSON) {
if (matches(mediaType, MediaType.APPLICATION_JSON)) {
sendJson(res, toJsonData(registries));
} else if (mediaType == MediaType.TEXT_PLAIN) {
res.send(toPrometheusData(registries));
} else if (matches(mediaType, MediaType.TEXT_PLAIN, MediaType.APPLICATION_OPENMETRICS)) {
sendPrometheus(res, toPrometheusData(registries), mediaType);
} else {
res.status(Http.Status.NOT_ACCEPTABLE_406);
res.send();
Expand All @@ -589,6 +591,15 @@ private void optionsMultiple(ServerRequest req, ServerResponse res, Registry...
}
}

private static boolean matches(MediaType candidateMediaType, MediaType... standardTypes) {
for (MediaType mt : standardTypes) {
if (mt.test(candidateMediaType)) {
return true;
}
}
return false;
}

private void optionsOne(ServerRequest req, ServerResponse res, Registry registry) {
String metricName = req.path().param("metric");

Expand All @@ -611,6 +622,21 @@ private void optionsOne(ServerRequest req, ServerResponse res, Registry registry
});
}

private static void sendPrometheus(ServerResponse res, String formattedOutput, MediaType requestedMediaType) {
MediaType.Builder responseMediaTypeBuilder = MediaType.builder()
.type(requestedMediaType.type())
.subtype(requestedMediaType.subtype())
.charset("UTF-8");

if (matches(requestedMediaType, MediaType.APPLICATION_OPENMETRICS)) {
responseMediaTypeBuilder.addParameter("version", "1.0.0");
} else if (matches(requestedMediaType, MediaType.TEXT_PLAIN)) {
responseMediaTypeBuilder.addParameter("version", "0.0.4");
}
res.addHeader("Content-Type", responseMediaTypeBuilder.build().toString());
res.send(formattedOutput + "# EOF\n");
}

/**
* A fluent API builder to build instances of {@link MetricsSupport}.
*/
Expand Down
60 changes: 60 additions & 0 deletions metrics/metrics/src/test/java/io/helidon/metrics/TestServer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
*/
package io.helidon.metrics;

import java.io.IOException;
import java.io.LineNumberReader;
import java.io.StringReader;
import java.time.Duration;
import java.util.Optional;
import java.util.concurrent.ExecutionException;
Expand All @@ -37,6 +40,7 @@
import org.eclipse.microprofile.metrics.ConcurrentGauge;
import org.eclipse.microprofile.metrics.MetricRegistry;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
Expand All @@ -45,6 +49,7 @@

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.endsWith;
import static org.hamcrest.Matchers.greaterThan;
import static org.hamcrest.Matchers.hasKey;
import static org.hamcrest.Matchers.is;
Expand All @@ -65,6 +70,20 @@ public class TestServer {

private static final Duration CLIENT_TIMEOUT = Duration.ofSeconds(5);

private static final MediaType EXPECTED_OPENMETRICS_CONTENT_TYPE = MediaType.builder()
.type(MediaType.APPLICATION_OPENMETRICS.type())
.subtype(MediaType.APPLICATION_OPENMETRICS.subtype())
.addParameter("version", "1.0.0")
.charset("UTF-8")
.build();

private static final MediaType EXPECTED_PROMETHEUS_CONTENT_TYPE = MediaType.builder()
.type(MediaType.TEXT_PLAIN.type())
.subtype(MediaType.TEXT_PLAIN.subtype())
.addParameter("version", "0.0.4")
.charset("UTF-8")
.build();

private WebClient.Builder webClientBuilder;

@BeforeAll
Expand Down Expand Up @@ -252,4 +271,45 @@ void testCacheSuppression(String pathSuffix) {
not(containsInAnyOrder(EXPECTED_NO_CACHE_HEADER_SETTINGS)));
}

@Test
void testOpenMetricsFormatting() throws IOException {
WebClientResponse response = webClientBuilder
.build()
.get()
.accept(MediaType.APPLICATION_OPENMETRICS)
.path("/metrics")
.submit()
.await();

assertThat("Content-Type",
response.headers().values(Http.Header.CONTENT_TYPE),
containsInAnyOrder(EXPECTED_OPENMETRICS_CONTENT_TYPE.toString()));

String content = response.content().as(String.class).await(10, TimeUnit.SECONDS);
assertThat("Terminated content", content, endsWith("EOF\n"));

LineNumberReader reader = new LineNumberReader(new StringReader(content));
String line;
while ((line = reader.readLine()) != null) {
if (line.isBlank()) {
Assertions.fail("Found blank line where none is allowed in response: \n" + content);
}
}
}

@Test
void testPrometheusFormatting() {
WebClientResponse response = webClientBuilder
.build()
.get()
.accept(MediaType.TEXT_PLAIN)
.path("/metrics")
.submit()
.await();

assertThat("Content-Type",
response.headers().values(Http.Header.CONTENT_TYPE),
containsInAnyOrder(EXPECTED_PROMETHEUS_CONTENT_TYPE.toString()));
}

}

0 comments on commit 005679c

Please sign in to comment.