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.

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

Correct Prometheus output for timer and JSON output for SimpleTimer (3.x) #4243

Merged
merged 4 commits into from
May 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ public void jsonData(JsonObjectBuilder builder, MetricID metricID) {
}

private static String durationPrometheusOutput(Duration duration) {
return duration == null ? "NaN" : Long.toString(duration.toSeconds());
return duration == null ? "NaN" : Double.toString(((double) duration.toNanos()) / 1000.0 / 1000.0 / 1000.0);
}

private double elapsedTimeInSeconds() {
Expand Down
47 changes: 3 additions & 44 deletions metrics/metrics/src/main/java/io/helidon/metrics/HelidonTimer.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import org.eclipse.microprofile.metrics.Meter;
import org.eclipse.microprofile.metrics.MetricID;
import org.eclipse.microprofile.metrics.MetricType;
import org.eclipse.microprofile.metrics.MetricUnits;
import org.eclipse.microprofile.metrics.Snapshot;
import org.eclipse.microprofile.metrics.Timer;

Expand Down Expand Up @@ -117,7 +116,9 @@ DisplayableLabeledSnapshot snapshot(){
@Override
public void prometheusData(StringBuilder sb, MetricID metricID, boolean withHelpType) {

PrometheusName name = PrometheusName.create(this, metricID);
// In Prometheus, times are always expressed in seconds. So force the TimeUnits value accordingly, ignoring
// whatever units were specified in the timer's metadata.
PrometheusName name = PrometheusName.create(this, metricID, TimeUnits.PROMETHEUS_TIMER_CONVERSION_TIME_UNITS);

appendPrometheusTimerStatElement(sb, name, "rate_per_second", withHelpType, "gauge", getMeanRate());
appendPrometheusTimerStatElement(sb, name, "one_min_rate_per_second", withHelpType, "gauge", getOneMinuteRate());
Expand Down Expand Up @@ -159,48 +160,6 @@ public void jsonData(JsonObjectBuilder builder, MetricID metricID) {
builder.add(metricID.getName(), myBuilder);
}

private long conversionFactor() {
Units units = getUnits();
String metricUnit = units.getMetricUnit();
if (metricUnit == null) {
return 1;
}
long divisor = 1;
switch (metricUnit) {
case MetricUnits.NANOSECONDS:
divisor = 1;
break;

case MetricUnits.MICROSECONDS:
divisor = 1000;
break;

case MetricUnits.MILLISECONDS:
divisor = 1000 * 1000;
break;

case MetricUnits.SECONDS:
divisor = 1000 * 1000 * 1000;
break;

case MetricUnits.MINUTES:
divisor = 1000 * 1000 * 1000 * 60;
break;

case MetricUnits.HOURS:
divisor = 1000 * 1000 * 1000 * 60 * 60;
break;

case MetricUnits.DAYS:
divisor = 1000 * 1000 * 1000 * 60 * 60 * 24;
break;

default:
divisor = 1;
}
return divisor;
}

void appendPrometheusTimerStatElement(StringBuilder sb,
PrometheusName name,
String statName,
Expand Down
81 changes: 60 additions & 21 deletions metrics/metrics/src/main/java/io/helidon/metrics/MetricImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ private static String bsls(String s) {
addTimeConverter(MetricUnits.HOURS, TimeUnit.HOURS);
addTimeConverter(MetricUnits.DAYS, TimeUnit.DAYS);

addConverter(new Units(MetricUnits.BITS, "bytes", o -> (double) o / 8));
addConverter(new Units(MetricUnits.BITS, "bytes", o -> ((Number) o).doubleValue() / 8));
addByteConverter(MetricUnits.KILOBITS, KILOBITS);
addByteConverter(MetricUnits.MEGABITS, MEGABITS);
addByteConverter(MetricUnits.GIGABITS, GIGABITS);
Expand Down Expand Up @@ -205,6 +205,47 @@ static String jsonFullKey(MetricID metricID) {
return jsonFullKey(metricID.getName(), metricID);
}

long conversionFactor() {
Units units = getUnits();
String metricUnit = units.getMetricUnit();
if (metricUnit == null) {
return 1;
}
long divisor = 1;
switch (metricUnit) {
case MetricUnits.NANOSECONDS:
divisor = 1;
break;

case MetricUnits.MICROSECONDS:
divisor = 1000;
break;

case MetricUnits.MILLISECONDS:
divisor = 1000 * 1000;
break;

case MetricUnits.SECONDS:
divisor = 1000 * 1000 * 1000;
break;

case MetricUnits.MINUTES:
divisor = 1000 * 1000 * 1000 * 60;
break;

case MetricUnits.HOURS:
divisor = 1000 * 1000 * 1000 * 60 * 60;
break;

case MetricUnits.DAYS:
divisor = 1000 * 1000 * 1000 * 60 * 60 * 24;
break;

default:
divisor = 1;
}
return divisor;
}

protected String toStringDetails() {
return "";
Expand Down Expand Up @@ -244,18 +285,7 @@ JsonValue jsonDuration(Duration duration) {
if (duration == null) {
return JsonObject.NULL;
}
long result = switch (metadata().getUnit()) {
case MetricUnits.DAYS -> duration.toDays();
case MetricUnits.HOURS -> duration.toHours();
case MetricUnits.MINUTES -> duration.toMinutes();
case MetricUnits.SECONDS -> duration.toSeconds();
case MetricUnits.MILLISECONDS -> duration.toMillis();
case MetricUnits.MICROSECONDS -> duration.toNanos() / 1000;

// includes explicit nanoseconds units and no units set (default)
default -> duration.toNanos();
};

double result = ((double) duration.toNanos()) / conversionFactor();
return Json.createValue(result);
}

Expand Down Expand Up @@ -340,7 +370,7 @@ private void appendPrometheusElement(StringBuilder sb,
void appendPrometheusHistogramElements(StringBuilder sb, MetricID metricID,
boolean withHelpType, long count, long sum, DisplayableLabeledSnapshot snap) {
PrometheusName name = PrometheusName.create(this, metricID);
appendPrometheusHistogramElements(sb, name, withHelpType, count, sum, snap);
appendPrometheusHistogramElements(sb, name, getUnits(), withHelpType, count, sum, snap);
}

void appendPrometheusHistogramElements(StringBuilder sb,
Expand All @@ -349,11 +379,18 @@ void appendPrometheusHistogramElements(StringBuilder sb,
long count,
Duration elapsedTime,
DisplayableLabeledSnapshot snap) {
appendPrometheusHistogramElements(sb, name, withHelpType, count, elapsedTime.toSeconds(), snap);
appendPrometheusHistogramElements(sb,
name,
TimeUnits.PROMETHEUS_TIMER_CONVERSION_TIME_UNITS,
withHelpType,
count,
elapsedTime.toSeconds(),
snap);
}

void appendPrometheusHistogramElements(StringBuilder sb,
PrometheusName name,
Units units,
boolean withHelpType,
long count,
long sum,
Expand Down Expand Up @@ -393,12 +430,12 @@ void appendPrometheusHistogramElements(StringBuilder sb,
.append('\n');
// application:file_sizes_bytes{quantile="0.5"} 4201
// for each supported quantile
prometheusQuantile(sb, name, getUnits(), "0.5", snap.median());
prometheusQuantile(sb, name, getUnits(), "0.75", snap.sample75thPercentile());
prometheusQuantile(sb, name, getUnits(), "0.95", snap.sample95thPercentile());
prometheusQuantile(sb, name, getUnits(), "0.98", snap.sample98thPercentile());
prometheusQuantile(sb, name, getUnits(), "0.99", snap.sample99thPercentile());
prometheusQuantile(sb, name, getUnits(), "0.999", snap.sample999thPercentile());
prometheusQuantile(sb, name, units, "0.5", snap.median());
prometheusQuantile(sb, name, units, "0.75", snap.sample75thPercentile());
prometheusQuantile(sb, name, units, "0.95", snap.sample95thPercentile());
prometheusQuantile(sb, name, units, "0.98", snap.sample98thPercentile());
prometheusQuantile(sb, name, units, "0.99", snap.sample99thPercentile());
prometheusQuantile(sb, name, units, "0.999", snap.sample999thPercentile());

}

Expand Down Expand Up @@ -521,6 +558,8 @@ private TimeUnits(String metricUnit, TimeUnit timeUnit) {
super(metricUnit, "seconds", timeConverter(timeUnit));
}

static final TimeUnits PROMETHEUS_TIMER_CONVERSION_TIME_UNITS = new TimeUnits("seconds", TimeUnit.NANOSECONDS);

static Function<Object, Object> timeConverter(TimeUnit from) {
switch (from) {
case NANOSECONDS:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019, 2021 Oracle and/or its affiliates.
* Copyright (c) 2019, 2022 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -23,26 +23,35 @@
* <p>
* Includes:
* <ul>
* <li>{@link #withinTolerance(java.lang.Number)} for checking within a range (tolerance) either side of an expected value</li>
* <li>{@link #withinTolerance(java.lang.Number)} for checking within a <em>configured</em> range (tolerance) either side of an expected
* value</li>
* <li>{@link #withinTolerance(java.lang.Number, double)} for checking within a <em>specified</em> range</li>
* </ul>
*/
class HelidonMetricsMatcher {

static final Double VARIANCE = Double.valueOf(System.getProperty("helidon.histogram.tolerance", "0.001"));

static TypeSafeMatcher<Number> withinTolerance(final Number expected) {
return new TypeSafeMatcher<Number>() {
return withinTolerance(expected, VARIANCE);
}

static TypeSafeMatcher<Number> withinTolerance(final Number expected, double variance) {
return new TypeSafeMatcher<>() {

private final double v = variance;

@Override
protected boolean matchesSafely(Number item) {
return Math.abs(expected.doubleValue() - item.doubleValue()) <= expected.doubleValue() * VARIANCE;
return Math.abs(expected.doubleValue() - item.doubleValue()) <= expected.doubleValue() * v;
}

@Override
public void describeTo(Description description) {
description.appendText("withinTolerance expected value in range [")
.appendValue(expected.doubleValue() * (1.0 - VARIANCE))
.appendValue(expected.doubleValue() * (1.0 - v))
.appendText(", ")
.appendValue(expected.doubleValue() * (1.0 + VARIANCE))
.appendValue(expected.doubleValue() * (1.0 + v))
.appendText("]");
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@

import java.time.Duration;
import java.time.temporal.ChronoUnit;
import java.util.Map;
import java.util.concurrent.TimeUnit;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import jakarta.json.Json;
import jakarta.json.JsonObject;
Expand All @@ -32,8 +35,9 @@
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.junit.platform.commons.JUnitException;

import static org.hamcrest.CoreMatchers.startsWith;
import static io.helidon.metrics.HelidonMetricsMatcher.withinTolerance;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.notNullValue;
Expand Down Expand Up @@ -228,22 +232,31 @@ void testJsonNonDefaultUnits(String metricUnits) {

@Test
void testPrometheus() {
Pattern responseTimePattern =
Pattern.compile("""
# TYPE application_response_time_total counter
# HELP application_response_time_total Server response time for /index.html
application_response_time_total (\\S*)
# TYPE application_response_time_elapsedTime_seconds gauge
application_response_time_elapsedTime_seconds (\\S*)
# TYPE application_response_time_maxTimeDuration_seconds gauge
application_response_time_maxTimeDuration_seconds (\\S*)
# TYPE application_response_time_minTimeDuration_seconds gauge
application_response_time_minTimeDuration_seconds (\\S*)
""", Pattern.MULTILINE);
StringBuilder sb = new StringBuilder();
ensureDataSetTimerClockAdvanced();
dataSetTimer.prometheusData(sb, dataSetTimerID, true);
String prometheusData = sb.toString();
assertThat(prometheusData,
startsWith("""
# TYPE application_response_time_total counter
# HELP application_response_time_total Server response time for /index.html
application_response_time_total 200
# TYPE application_response_time_elapsedTime_seconds gauge
application_response_time_elapsedTime_seconds 1.0127E-4
# TYPE application_response_time_maxTimeDuration_seconds gauge
application_response_time_maxTimeDuration_seconds 0
# TYPE application_response_time_minTimeDuration_seconds gauge
application_response_time_minTimeDuration_seconds 0
"""));
Matcher m = responseTimePattern.matcher(prometheusData);
if (!m.find()) {
throw new JUnitException("Could not match Prometheus output " + prometheusData
+ " to expected pattern " + responseTimePattern);
}
assertThat("total", Integer.parseInt(m.group(1)), is(200));
assertThat("elapsedTime", Double.parseDouble(m.group(2)), is(withinTolerance(1.0127E-4, 1.2E-6)));
assertThat("max", Double.parseDouble(m.group(3)), is(withinTolerance(9.9E-7, 1.2E-9)));
assertThat("min", Double.parseDouble(m.group(4)), is(withinTolerance(0.0, 0.012)));

// Because the batch of test data does not give non-zero min and max, do a separate test to check those.
TestClock clock = TestClock.create();
Expand All @@ -256,18 +269,16 @@ void testPrometheus() {
sb = new StringBuilder();
simpleTimer.prometheusData(sb, dataSetTimerID, true);
prometheusData = sb.toString();
assertThat(prometheusData,
startsWith("""
# TYPE application_response_time_total counter
# HELP application_response_time_total Server response time for /index.html
application_response_time_total 2
# TYPE application_response_time_elapsedTime_seconds gauge
application_response_time_elapsedTime_seconds 7.0
# TYPE application_response_time_maxTimeDuration_seconds gauge
application_response_time_maxTimeDuration_seconds 4
# TYPE application_response_time_minTimeDuration_seconds gauge
application_response_time_minTimeDuration_seconds 3
"""));

m = responseTimePattern.matcher(prometheusData);
if (!m.find()) {
throw new JUnitException("Could not match Prometheus output " + prometheusData
+ " to expected pattern" + responseTimePattern);
}
assertThat("total", Integer.parseInt(m.group(1)), is(2));
assertThat("elapsedTime", Double.parseDouble(m.group(2)), is(withinTolerance(7.0D, 0.012)));
assertThat("max", Double.parseDouble(m.group(3)), is(withinTolerance(4.0D, 0.012)));
assertThat("min", Double.parseDouble(m.group(4)), is(withinTolerance(3.0D, 0.012)));
}

@Test
Expand Down
Loading