Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix OpenMetrics formatting error (3.x) #4900

Merged
merged 4 commits into from
Sep 20, 2022
Merged

Fix OpenMetrics formatting error (3.x) #4900

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e972b0a
Fix OpenMetrics formatting error; also fix some undiscovered issues w…
tjquinno Sep 15, 2022
f5a08f7
Tidy up the media type handling a bit
tjquinno Sep 16, 2022
f5d79fd
Straggler from last commit - revised test
tjquinno Sep 16, 2022
9aca5d6
Improve handling in match method if Accept indicates an unacceptible …
tjquinno Sep 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions common/http/src/main/java/io/helidon/common/http/MediaType.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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()));
}

}