From 8bd17e6f8a6584d08bc96d1ab9ea5e4dd22a00c1 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Tue, 10 Mar 2026 14:45:53 +0100 Subject: [PATCH 1/2] Add jdisc.http.latency metric - Collect from MetricAggregatingRequestLog - Use one metric both for failure and success - Add a consistent set of dimensions including status code - Remove the non-standard named serverTotalSuccessfulResponseLatency and serverTotalFailedResponseLatency --- .../container/jdisc/state/StateHandler.java | 3 +- .../http/server/jetty/JettyHttpServer.java | 4 +- .../jetty/MetricAggregatingRequestLog.java | 45 +++-- .../http/server/jetty/MetricDefinitions.java | 7 +- .../server/jetty/RequestMetricReporter.java | 17 +- .../server/jetty/ServerMetricReporter.java | 5 - .../jdisc/state/StateHandlerTest.java | 15 +- .../jdisc/http/server/jetty/HttpServerIT.java | 173 +++++++++--------- .../MetricAggregatingRequestLogTest.java | 28 ++- .../ai/vespa/metrics/ContainerMetrics.java | 8 +- .../ai/vespa/metrics/set/DefaultMetrics.java | 2 +- .../metrics/set/InfrastructureMetricSet.java | 2 +- .../metrics/set/Vespa9DefaultMetricSet.java | 3 +- .../metrics/set/Vespa9VespaMetricSet.java | 2 +- .../ai/vespa/metrics/set/VespaMetricSet.java | 2 +- 15 files changed, 162 insertions(+), 154 deletions(-) diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/state/StateHandler.java b/container-disc/src/main/java/com/yahoo/container/jdisc/state/StateHandler.java index 9c340f86bd60..205c97c5e1b1 100644 --- a/container-disc/src/main/java/com/yahoo/container/jdisc/state/StateHandler.java +++ b/container-disc/src/main/java/com/yahoo/container/jdisc/state/StateHandler.java @@ -36,7 +36,6 @@ import java.util.Iterator; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.concurrent.TimeUnit; import java.nio.charset.StandardCharsets; @@ -347,7 +346,7 @@ private static List collapseHealthMetrics(MetricSnapshot snapshot) { Tuple latencySeconds = new Tuple(NULL_DIMENSIONS, "latencySeconds", null); for (Map.Entry entry : snapshot) { MetricSet metricSet = entry.getValue(); - MetricValue val = metricSet.get(ContainerMetrics.SERVER_TOTAL_SUCCESSFUL_RESPONSE_LATENCY.baseName()); + MetricValue val = metricSet.get(ContainerMetrics.JDISC_HTTP_LATENCY.baseName()); if (val instanceof GaugeMetric gauge) { latencySeconds.add(GaugeMetric.newInstance(gauge.getLast() / 1000, gauge.getMax() / 1000, diff --git a/container-disc/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java b/container-disc/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java index f552c001a3e1..c2983ac8dfb2 100644 --- a/container-disc/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java +++ b/container-disc/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java @@ -68,8 +68,6 @@ public JettyHttpServer(Metric metric, throw new IllegalArgumentException("No connectors configured."); var histogramSettings = new MetricSettings.Builder().histogram(true).build(); - metricReceiver.declareGauge(MetricDefinitions.TOTAL_SUCCESSFUL_LATENCY, Optional.empty(), histogramSettings); - metricReceiver.declareGauge(MetricDefinitions.TOTAL_FAILED_LATENCY, Optional.empty(), histogramSettings); metricReceiver.declareGauge(MetricDefinitions.TIME_TO_FIRST_BYTE, Optional.empty(), histogramSettings); this.config = serverConfig; @@ -85,7 +83,7 @@ public JettyHttpServer(Metric metric, server.setErrorHandler(errorHandler); server.setStopTimeout((long)(serverConfig.stopTimeout() * 1000.0)); - var metricAggregatingRequestLog = new MetricAggregatingRequestLog(config.metric()); + var metricAggregatingRequestLog = new MetricAggregatingRequestLog(config.metric(), metric, metricReceiver); server.addBean(metricAggregatingRequestLog); if (requestLog instanceof VoidRequestLog) { server.setRequestLog(metricAggregatingRequestLog); diff --git a/container-disc/src/main/java/com/yahoo/jdisc/http/server/jetty/MetricAggregatingRequestLog.java b/container-disc/src/main/java/com/yahoo/jdisc/http/server/jetty/MetricAggregatingRequestLog.java index 801fb7f8a552..81e954a9cdb4 100644 --- a/container-disc/src/main/java/com/yahoo/jdisc/http/server/jetty/MetricAggregatingRequestLog.java +++ b/container-disc/src/main/java/com/yahoo/jdisc/http/server/jetty/MetricAggregatingRequestLog.java @@ -4,6 +4,8 @@ import com.yahoo.jdisc.Metric; import com.yahoo.jdisc.http.HttpRequest; import com.yahoo.jdisc.http.ServerConfig; +import com.yahoo.metrics.simple.MetricReceiver; +import com.yahoo.metrics.simple.MetricSettings; import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.RequestLog; @@ -15,6 +17,7 @@ import java.util.HashMap; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; @@ -39,19 +42,25 @@ class MetricAggregatingRequestLog implements RequestLog { private final List searchHandlerPaths; private final Set ignoredUserAgents; private final boolean reporterEnabled; + private final Metric metric; private final ConcurrentMap statistics = new ConcurrentHashMap<>(); - MetricAggregatingRequestLog(ServerConfig.Metric cfg) { - this(cfg.monitoringHandlerPaths(), cfg.searchHandlerPaths(), cfg.ignoredUserAgents(), cfg.reporterEnabled()); + MetricAggregatingRequestLog(ServerConfig.Metric config, Metric metric, MetricReceiver metricReceiver) { + this(config.monitoringHandlerPaths(), config.searchHandlerPaths(), config.ignoredUserAgents(), config.reporterEnabled(), + metric, metricReceiver); } MetricAggregatingRequestLog(List monitoringHandlerPaths, List searchHandlerPaths, - List ignoredUserAgents, boolean reporterEnabled) { + List ignoredUserAgents, boolean reporterEnabled, + Metric metric, MetricReceiver metricReceiver) { this.monitoringHandlerPaths = monitoringHandlerPaths; this.searchHandlerPaths = searchHandlerPaths; this.ignoredUserAgents = Set.copyOf(ignoredUserAgents); this.reporterEnabled = reporterEnabled; + var histogramSettings = new MetricSettings.Builder().histogram(true).build(); + metricReceiver.declareGauge(MetricDefinitions.LATENCY, Optional.empty(), histogramSettings); + this.metric = metric; } static MetricAggregatingRequestLog getBean(JettyHttpServer server) { return getBean(server.server()); } @@ -60,25 +69,27 @@ class MetricAggregatingRequestLog implements RequestLog { @Override public void log(Request req, Response resp) { onResponse(req, resp.getStatus()); } void onResponse(Request request, int status) { - if (shouldLogMetricsFor(request)) { - var metrics = StatusCodeMetric.of(request, status, monitoringHandlerPaths, searchHandlerPaths); - metrics.forEach(metric -> statistics.computeIfAbsent(metric, __ -> new LongAdder()).increment()); - } + if (!shouldLogMetricsFor(request)) return; + + Dimensions dimensions = Dimensions.of(request, status, monitoringHandlerPaths, searchHandlerPaths); + StatusCodeMetric.of(status, dimensions).forEach(metric -> statistics.computeIfAbsent(metric, __ -> new LongAdder()).increment()); + metric.set(MetricDefinitions.LATENCY, System.currentTimeMillis() - Request.getTimeStamp(request), metric.createContext(dimensions.asMap())); } + /** For testing */ List takeStatistics() { if (reporterEnabled) throw new IllegalStateException("Cannot take consistent snapshot while reporter is enabled"); - var ret = new ArrayList(); - consume((metric, value) -> ret.add(new StatisticsEntry(metric, value))); - return ret; + var statisticsList = new ArrayList(); + consume((metric, value) -> statisticsList.add(new StatisticsEntry(metric, value))); + return statisticsList; } void reportSnapshot(Metric metricAggregator) { if (!reporterEnabled) throw new IllegalStateException("Reporter is not enabled"); consume((metric, value) -> { - Metric.Context ctx = metricAggregator.createContext(metric.dimensions.asMap()); - metricAggregator.add(metric.name, value, ctx); + Metric.Context context = metricAggregator.createContext(metric.dimensions.asMap()); + metricAggregator.add(metric.name, value, context); }); } @@ -96,6 +107,7 @@ private void consume(ObjLongConsumer consumer) { } static class Dimensions { + final String protocol; final String scheme; final String method; @@ -192,6 +204,7 @@ public int hashCode() { } static class StatusCodeMetric { + final Dimensions dimensions; final String name; @@ -200,10 +213,8 @@ private StatusCodeMetric(Dimensions dimensions, String name) { this.name = name; } - static Set of(Request req, int statusCode, List monitoringHandlerPaths, - List searchHandlerPaths) { - Dimensions dimensions = Dimensions.of(req, statusCode, monitoringHandlerPaths, searchHandlerPaths); - return metricNames(statusCode).stream() + static Set of(int status, Dimensions dimensions) { + return metricNames(status).stream() .map(name -> new StatusCodeMetric(dimensions, name)) .collect(Collectors.toSet()); } @@ -228,6 +239,7 @@ public boolean equals(Object o) { } static class StatisticsEntry { + final Dimensions dimensions; final String name; final long value; @@ -248,4 +260,5 @@ public boolean equals(Object o) { @Override public int hashCode() { return Objects.hash(dimensions, name, value); } } + } diff --git a/container-disc/src/main/java/com/yahoo/jdisc/http/server/jetty/MetricDefinitions.java b/container-disc/src/main/java/com/yahoo/jdisc/http/server/jetty/MetricDefinitions.java index cf6536698982..16869840fd83 100644 --- a/container-disc/src/main/java/com/yahoo/jdisc/http/server/jetty/MetricDefinitions.java +++ b/container-disc/src/main/java/com/yahoo/jdisc/http/server/jetty/MetricDefinitions.java @@ -9,6 +9,7 @@ * @author bjorncs */ class MetricDefinitions { + static final String NAME_DIMENSION = "serverName"; static final String PORT_DIMENSION = "serverPort"; static final String METHOD_DIMENSION = "httpMethod"; @@ -39,9 +40,8 @@ class MetricDefinitions { static final String NUM_SUCCESSFUL_WRITES = ContainerMetrics.SERVER_NUM_SUCCESSFUL_RESPONSE_WRITES.baseName(); static final String NUM_FAILED_WRITES = ContainerMetrics.SERVER_NUM_FAILED_RESPONSE_WRITES.baseName(); - static final String TOTAL_SUCCESSFUL_LATENCY = ContainerMetrics.SERVER_TOTAL_SUCCESSFUL_RESPONSE_LATENCY.baseName(); - static final String TOTAL_FAILED_LATENCY = ContainerMetrics.SERVER_TOTAL_FAILED_RESPONSE_LATENCY.baseName(); - static final String TIME_TO_FIRST_BYTE = ContainerMetrics.SERVER_TIME_TO_FIRST_BYTE.baseName(); + static final String LATENCY = ContainerMetrics.JDISC_HTTP_LATENCY.baseName(); + static final String TIME_TO_FIRST_BYTE = ContainerMetrics.JDISC_HTTP_TIME_TO_FIRST_BYTE.baseName(); static final String RESPONSES_1XX = ContainerMetrics.HTTP_STATUS_1XX.baseName(); static final String RESPONSES_2XX = ContainerMetrics.HTTP_STATUS_2XX.baseName(); @@ -76,4 +76,5 @@ class MetricDefinitions { static final String FILTERING_RESPONSE_UNHANDLED = ContainerMetrics.JDISC_HTTP_FILTERING_RESPONSE_UNHANDLED.baseName(); private MetricDefinitions() {} + } diff --git a/container-disc/src/main/java/com/yahoo/jdisc/http/server/jetty/RequestMetricReporter.java b/container-disc/src/main/java/com/yahoo/jdisc/http/server/jetty/RequestMetricReporter.java index b358800e03d1..3669bc362da9 100644 --- a/container-disc/src/main/java/com/yahoo/jdisc/http/server/jetty/RequestMetricReporter.java +++ b/container-disc/src/main/java/com/yahoo/jdisc/http/server/jetty/RequestMetricReporter.java @@ -8,19 +8,20 @@ /** - * Responsible for metric reporting for JDisc http request handler support. + * Responsible for (some) metric reporting for JDisc http request handlers. + * See also {@link MetricAggregatingRequestLog}. + * * @author Tony Vaagenes */ class RequestMetricReporter { + private final Metric metric; private final Context context; - private final long requestStartTime; - //TODO: rename + // TODO: rename private final AtomicBoolean firstSetOfTimeToFirstByte = new AtomicBoolean(true); - RequestMetricReporter(Metric metric, Context context, long requestStartTime) { this.metric = metric; this.context = context; @@ -48,18 +49,11 @@ void failedWrite() { void successfulResponse() { setTimeToFirstByteFirstTime(); - - long requestLatency = getRequestLatency(); - - metric.set(MetricDefinitions.TOTAL_SUCCESSFUL_LATENCY, requestLatency, context); - metric.add(MetricDefinitions.NUM_SUCCESSFUL_RESPONSES, 1, context); } void failedResponse() { setTimeToFirstByteFirstTime(); - - metric.set(MetricDefinitions.TOTAL_FAILED_LATENCY, getRequestLatency(), context); metric.add(MetricDefinitions.NUM_FAILED_RESPONSES, 1, context); } @@ -82,4 +76,5 @@ void uriLength(int length) { void contentSize(long size) { metric.set(MetricDefinitions.CONTENT_SIZE, size, context); } + } diff --git a/container-disc/src/main/java/com/yahoo/jdisc/http/server/jetty/ServerMetricReporter.java b/container-disc/src/main/java/com/yahoo/jdisc/http/server/jetty/ServerMetricReporter.java index dbf16474f2ad..924ad9270f0e 100644 --- a/container-disc/src/main/java/com/yahoo/jdisc/http/server/jetty/ServerMetricReporter.java +++ b/container-disc/src/main/java/com/yahoo/jdisc/http/server/jetty/ServerMetricReporter.java @@ -70,11 +70,6 @@ public void run() { private void setServerMetrics(MetricAggregatingRequestLog statisticsCollector) { long timeSinceStarted = System.currentTimeMillis() - timeStarted.toEpochMilli(); metric.set(MetricDefinitions.STARTED_MILLIS, timeSinceStarted, null); - - addResponseMetrics(statisticsCollector); - } - - private void addResponseMetrics(MetricAggregatingRequestLog statisticsCollector) { statisticsCollector.reportSnapshot(metric); } diff --git a/container-disc/src/test/java/com/yahoo/container/jdisc/state/StateHandlerTest.java b/container-disc/src/test/java/com/yahoo/container/jdisc/state/StateHandlerTest.java index 784e66b1d452..16a25bca9d49 100644 --- a/container-disc/src/test/java/com/yahoo/container/jdisc/state/StateHandlerTest.java +++ b/container-disc/src/test/java/com/yahoo/container/jdisc/state/StateHandlerTest.java @@ -182,17 +182,10 @@ void testHealthAggregation() throws Exception { JsonNode json = requestAsJson(V1_URI + "health"); assertEquals("up", json.get("status").get("code").asText(), json.toString()); - assertEquals(2, json.get("metrics").get("values").size(), json.toString()); - assertEquals("requestsPerSecond", - json.get("metrics").get("values").get(0).get("name").asText(), - json.toString()); - assertEquals(6, - json.get("metrics").get("values").get(0).get("values").get("count").asDouble(), 0.001, json.toString()); - assertEquals("latencySeconds", - json.get("metrics").get("values").get(1).get("name").asText(), - json.toString()); - assertEquals(0.03, - json.get("metrics").get("values").get(1).get("values").get("average").asDouble(), 0.001, json.toString()); + var values = json.get("metrics").get("values"); + assertEquals(1, values.size(), json.toString()); + assertEquals("requestsPerSecond", values.get(0).get("name").asText(), json.toString()); + assertEquals(6, values.get(0).get("values").get("count").asDouble(), 0.001, json.toString()); } @Test diff --git a/container-disc/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerIT.java b/container-disc/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerIT.java index 83797555dd0a..d50a498e272d 100644 --- a/container-disc/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerIT.java +++ b/container-disc/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerIT.java @@ -96,6 +96,7 @@ import static org.hamcrest.Matchers.matchesPattern; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; @@ -117,14 +118,14 @@ public class HttpServerIT { @Test void requireThatServerCanListenToRandomPort() { - final JettyTestDriver driver = JettyTestDriver.newInstance(mockRequestHandler()); + JettyTestDriver driver = JettyTestDriver.newInstance(mockRequestHandler()); assertNotEquals(0, driver.server().getListenPort()); assertTrue(driver.close()); } @Test void requireThatServerCanNotListenToBoundPort() { - final JettyTestDriver driver = JettyTestDriver.newInstance(mockRequestHandler()); + JettyTestDriver driver = JettyTestDriver.newInstance(mockRequestHandler()); try { JettyTestDriver.newConfiguredInstance( mockRequestHandler(), @@ -132,15 +133,15 @@ void requireThatServerCanNotListenToBoundPort() { new ConnectorConfig.Builder() .listenPort(driver.server().getListenPort()) ); - } catch (final Throwable t) { - assertTrue(t.getCause() instanceof BindException); + } catch (Throwable t) { + assertInstanceOf(BindException.class, t.getCause()); } assertTrue(driver.close()); } @Test void requireThatBindingSetNotFoundReturns404() throws Exception { - final JettyTestDriver driver = JettyTestDriver.newConfiguredInstance( + JettyTestDriver driver = JettyTestDriver.newConfiguredInstance( mockRequestHandler(), new ServerConfig.Builder() .developerMode(true), @@ -157,7 +158,7 @@ void requireThatBindingSetNotFoundReturns404() throws Exception { @Test void requireThatTooLongInitLineReturns431() throws Exception { - final JettyTestDriver driver = JettyTestDriver.newConfiguredInstance( + JettyTestDriver driver = JettyTestDriver.newConfiguredInstance( mockRequestHandler(), new ServerConfig.Builder(), new ConnectorConfig.Builder() @@ -169,7 +170,7 @@ void requireThatTooLongInitLineReturns431() throws Exception { @Test void requireThatTooLargePayloadFailsWith413() throws Exception { - final JettyTestDriver driver = JettyTestDriver.newConfiguredInstance( + JettyTestDriver driver = JettyTestDriver.newConfiguredInstance( new EchoRequestHandler(), new ServerConfig.Builder(), new ConnectorConfig.Builder() @@ -202,7 +203,7 @@ void requireThatMultipleHostHeadersReturns400() throws Exception { @Test void requireThatAccessLogIsCalledForRequestRejectedByJetty() throws Exception { BlockingQueueRequestLog requestLogMock = new BlockingQueueRequestLog(); - final JettyTestDriver driver = JettyTestDriver.newConfiguredInstance( + JettyTestDriver driver = JettyTestDriver.newConfiguredInstance( mockRequestHandler(), new ServerConfig.Builder(), new ConnectorConfig.Builder().requestHeaderSize(1), @@ -216,25 +217,23 @@ void requireThatAccessLogIsCalledForRequestRejectedByJetty() throws Exception { @Test void requireThatServerCanEcho() throws Exception { - final JettyTestDriver driver = JettyTestDriver.newInstance(new EchoRequestHandler()); - driver.client().get("/status.html") - .expectStatusCode(is(OK)); + JettyTestDriver driver = JettyTestDriver.newInstance(new EchoRequestHandler()); + driver.client().get("/status.html").expectStatusCode(is(OK)); assertTrue(driver.close()); } @Test void requireThatServerCanEchoCompressed() throws Exception { - final JettyTestDriver driver = JettyTestDriver.newInstance(new EchoRequestHandler()); + JettyTestDriver driver = JettyTestDriver.newInstance(new EchoRequestHandler()); try (SimpleHttpClient client = driver.newClient(true)) { - client.get("/status.html") - .expectStatusCode(is(OK)); + client.get("/status.html").expectStatusCode(is(OK)); } assertTrue(driver.close()); } @Test void requireThatServerCanHandleMultipleRequests() throws Exception { - final JettyTestDriver driver = JettyTestDriver.newInstance(new EchoRequestHandler()); + JettyTestDriver driver = JettyTestDriver.newInstance(new EchoRequestHandler()); driver.client().get("/status.html") .expectStatusCode(is(OK)); driver.client().get("/status.html") @@ -244,9 +243,9 @@ void requireThatServerCanHandleMultipleRequests() throws Exception { @Test void requireThatFormPostWorks() throws Exception { - final JettyTestDriver driver = JettyTestDriver.newInstance(new ParameterPrinterRequestHandler()); - final String requestContent = generateContent('a', 30); - final ResponseValidator response = + JettyTestDriver driver = JettyTestDriver.newInstance(new ParameterPrinterRequestHandler()); + String requestContent = generateContent('a', 30); + ResponseValidator response = driver.client().newPost("/status.html") .addHeader(CONTENT_TYPE, APPLICATION_X_WWW_FORM_URLENCODED) .setContent(requestContent) @@ -258,8 +257,8 @@ void requireThatFormPostWorks() throws Exception { @Test void requireThatFormPostDoesNotRemoveContentByDefault() throws Exception { - final JettyTestDriver driver = JettyTestDriver.newInstance(new ParameterPrinterRequestHandler()); - final ResponseValidator response = + JettyTestDriver driver = JettyTestDriver.newInstance(new ParameterPrinterRequestHandler()); + ResponseValidator response = driver.client().newPost("/status.html") .addHeader(CONTENT_TYPE, APPLICATION_X_WWW_FORM_URLENCODED) .setContent("foo=bar") @@ -271,8 +270,8 @@ void requireThatFormPostDoesNotRemoveContentByDefault() throws Exception { @Test void requireThatFormPostKeepsContentWhenConfiguredTo() throws Exception { - final JettyTestDriver driver = newDriverWithFormPostContentRemoved(new ParameterPrinterRequestHandler(), false); - final ResponseValidator response = + JettyTestDriver driver = newDriverWithFormPostContentRemoved(new ParameterPrinterRequestHandler(), false); + ResponseValidator response = driver.client().newPost("/status.html") .addHeader(CONTENT_TYPE, APPLICATION_X_WWW_FORM_URLENCODED) .setContent("foo=bar") @@ -284,8 +283,8 @@ void requireThatFormPostKeepsContentWhenConfiguredTo() throws Exception { @Test void requireThatFormPostRemovesContentWhenConfiguredTo() throws Exception { - final JettyTestDriver driver = newDriverWithFormPostContentRemoved(new ParameterPrinterRequestHandler(), true); - final ResponseValidator response = + JettyTestDriver driver = newDriverWithFormPostContentRemoved(new ParameterPrinterRequestHandler(), true); + ResponseValidator response = driver.client().newPost("/status.html") .addHeader(CONTENT_TYPE, APPLICATION_X_WWW_FORM_URLENCODED) .setContent("foo=bar") @@ -297,8 +296,8 @@ void requireThatFormPostRemovesContentWhenConfiguredTo() throws Exception { @Test void requireThatFormPostWithInvalidDataFailsWith400() throws Exception { - final JettyTestDriver driver = newDriverWithFormPostContentRemoved(new ParameterPrinterRequestHandler(), true); - final ResponseValidator response = + JettyTestDriver driver = newDriverWithFormPostContentRemoved(new ParameterPrinterRequestHandler(), true); + ResponseValidator response = driver.client().newPost("/status.html") .addHeader(CONTENT_TYPE, APPLICATION_X_WWW_FORM_URLENCODED) .setContent("%!Foo=bar") @@ -310,9 +309,9 @@ void requireThatFormPostWithInvalidDataFailsWith400() throws Exception { @Test void requireThatFormPostWithCharsetSpecifiedWorks() throws Exception { - final JettyTestDriver driver = JettyTestDriver.newInstance(new ParameterPrinterRequestHandler()); - final String requestContent = generateContent('a', 30); - final ResponseValidator response = + JettyTestDriver driver = JettyTestDriver.newInstance(new ParameterPrinterRequestHandler()); + String requestContent = generateContent('a', 30); + ResponseValidator response = driver.client().newPost("/status.html") .addHeader(X_DISABLE_CHUNKING, "true") .addHeader(CONTENT_TYPE, APPLICATION_X_WWW_FORM_URLENCODED + ";charset=UTF-8") @@ -325,8 +324,8 @@ void requireThatFormPostWithCharsetSpecifiedWorks() throws Exception { @Test void requireThatEmptyFormPostWorks() throws Exception { - final JettyTestDriver driver = JettyTestDriver.newInstance(new ParameterPrinterRequestHandler()); - final ResponseValidator response = + JettyTestDriver driver = JettyTestDriver.newInstance(new ParameterPrinterRequestHandler()); + ResponseValidator response = driver.client().newPost("/status.html") .addHeader(CONTENT_TYPE, APPLICATION_X_WWW_FORM_URLENCODED) .execute(); @@ -337,8 +336,8 @@ void requireThatEmptyFormPostWorks() throws Exception { @Test void requireThatFormParametersAreParsed() throws Exception { - final JettyTestDriver driver = JettyTestDriver.newInstance(new ParameterPrinterRequestHandler()); - final ResponseValidator response = + JettyTestDriver driver = JettyTestDriver.newInstance(new ParameterPrinterRequestHandler()); + ResponseValidator response = driver.client().newPost("/status.html") .addHeader(CONTENT_TYPE, APPLICATION_X_WWW_FORM_URLENCODED) .setContent("a=b&c=d") @@ -350,8 +349,8 @@ void requireThatFormParametersAreParsed() throws Exception { @Test void requireThatUriParametersAreParsed() throws Exception { - final JettyTestDriver driver = JettyTestDriver.newInstance(new ParameterPrinterRequestHandler()); - final ResponseValidator response = + JettyTestDriver driver = JettyTestDriver.newInstance(new ParameterPrinterRequestHandler()); + ResponseValidator response = driver.client().newPost("/status.html?a=b&c=d") .addHeader(CONTENT_TYPE, APPLICATION_X_WWW_FORM_URLENCODED) .execute(); @@ -362,8 +361,8 @@ void requireThatUriParametersAreParsed() throws Exception { @Test void requireThatFormAndUriParametersAreMerged() throws Exception { - final JettyTestDriver driver = JettyTestDriver.newInstance(new ParameterPrinterRequestHandler()); - final ResponseValidator response = + JettyTestDriver driver = JettyTestDriver.newInstance(new ParameterPrinterRequestHandler()); + ResponseValidator response = driver.client().newPost("/status.html?a=b&c=d1") .addHeader(CONTENT_TYPE, APPLICATION_X_WWW_FORM_URLENCODED) .setContent("c=d2&e=f") @@ -375,8 +374,8 @@ void requireThatFormAndUriParametersAreMerged() throws Exception { @Test void requireThatFormCharsetIsHonored() throws Exception { - final JettyTestDriver driver = newDriverWithFormPostContentRemoved(new ParameterPrinterRequestHandler(), true); - final ResponseValidator response = + JettyTestDriver driver = newDriverWithFormPostContentRemoved(new ParameterPrinterRequestHandler(), true); + ResponseValidator response = driver.client().newPost("/status.html") .addHeader(CONTENT_TYPE, APPLICATION_X_WWW_FORM_URLENCODED + ";charset=ISO-8859-1") .setBinaryContent(new byte[]{66, (byte) 230, 114, 61, 98, 108, (byte) 229}) @@ -388,8 +387,8 @@ void requireThatFormCharsetIsHonored() throws Exception { @Test void requireThatUnknownFormCharsetIsTreatedAsBadRequest() throws Exception { - final JettyTestDriver driver = JettyTestDriver.newInstance(new ParameterPrinterRequestHandler()); - final ResponseValidator response = + JettyTestDriver driver = JettyTestDriver.newInstance(new ParameterPrinterRequestHandler()); + ResponseValidator response = driver.client().newPost("/status.html") .addHeader(CONTENT_TYPE, APPLICATION_X_WWW_FORM_URLENCODED + ";charset=FLARBA-GARBA-7") .setContent("a=b") @@ -400,8 +399,8 @@ void requireThatUnknownFormCharsetIsTreatedAsBadRequest() throws Exception { @Test void requireThatFormPostWithPercentEncodedContentIsDecoded() throws Exception { - final JettyTestDriver driver = JettyTestDriver.newInstance(new ParameterPrinterRequestHandler()); - final ResponseValidator response = + JettyTestDriver driver = JettyTestDriver.newInstance(new ParameterPrinterRequestHandler()); + ResponseValidator response = driver.client().newPost("/status.html") .addHeader(CONTENT_TYPE, APPLICATION_X_WWW_FORM_URLENCODED) .setContent("%20%3D%C3%98=%22%25+") @@ -413,8 +412,8 @@ void requireThatFormPostWithPercentEncodedContentIsDecoded() throws Exception { @Test void requireThatFormPostWithThrowingHandlerIsExceptionSafe() throws Exception { - final JettyTestDriver driver = JettyTestDriver.newInstance(new ThrowingHandler()); - final ResponseValidator response = + JettyTestDriver driver = JettyTestDriver.newInstance(new ThrowingHandler()); + ResponseValidator response = driver.client().newPost("/status.html") .addHeader(CONTENT_TYPE, APPLICATION_X_WWW_FORM_URLENCODED) .setContent("a=b") @@ -426,12 +425,12 @@ void requireThatFormPostWithThrowingHandlerIsExceptionSafe() throws Exception { @Test void requireThatMultiPostWorks() throws Exception { // This is taken from tcpdump of bug 5433352 and reassembled here to see that httpserver passes things on. - final String startTxtContent = "this is a test for POST."; - final String updaterConfContent + String startTxtContent = "this is a test for POST."; + String updaterConfContent = "identifier = updater\n" + "server_type = gds\n"; - final JettyTestDriver driver = JettyTestDriver.newInstance(new EchoRequestHandler()); - final ResponseValidator response = + JettyTestDriver driver = JettyTestDriver.newInstance(new EchoRequestHandler()); + ResponseValidator response = driver.client().newPost("/status.html") .setMultipartContent( newFileBody("start.txt", startTxtContent), @@ -444,8 +443,8 @@ void requireThatMultiPostWorks() throws Exception { @Test void requireThatRequestCookiesAreReceived() throws Exception { - final JettyTestDriver driver = JettyTestDriver.newInstance(new CookiePrinterRequestHandler()); - final ResponseValidator response = + JettyTestDriver driver = JettyTestDriver.newInstance(new CookiePrinterRequestHandler()); + ResponseValidator response = driver.client().newPost("/status.html") .addHeader(COOKIE, "foo=bar") .execute(); @@ -456,7 +455,7 @@ void requireThatRequestCookiesAreReceived() throws Exception { @Test void requireThatSetCookieHeaderIsCorrect() throws Exception { - final JettyTestDriver driver = JettyTestDriver.newInstance(new CookieSetterRequestHandler( + JettyTestDriver driver = JettyTestDriver.newInstance(new CookieSetterRequestHandler( new Cookie("foo", "bar") .setDomain(".localhost") .setHttpOnly(true) @@ -471,8 +470,8 @@ void requireThatSetCookieHeaderIsCorrect() throws Exception { @Test void requireThatTimeoutWorks() throws Exception { - final UnresponsiveHandler requestHandler = new UnresponsiveHandler(); - final JettyTestDriver driver = JettyTestDriver.newInstance(requestHandler); + UnresponsiveHandler requestHandler = new UnresponsiveHandler(); + JettyTestDriver driver = JettyTestDriver.newInstance(requestHandler); driver.client().get("/status.html") .expectStatusCode(is(GATEWAY_TIMEOUT)); ResponseDispatch.newInstance(OK).dispatch(requestHandler.responseHandler); @@ -483,7 +482,7 @@ void requireThatTimeoutWorks() throws Exception { // Details in https://github.com/eclipse/jetty.project/issues/1116 @Test void requireThatHeaderWithNullValueIsOmitted() throws Exception { - final JettyTestDriver driver = JettyTestDriver.newInstance(new EchoWithHeaderRequestHandler("X-Foo", null)); + JettyTestDriver driver = JettyTestDriver.newInstance(new EchoWithHeaderRequestHandler("X-Foo", null)); driver.client().get("/status.html") .expectStatusCode(is(OK)) .expectNoHeader("X-Foo"); @@ -494,7 +493,7 @@ void requireThatHeaderWithNullValueIsOmitted() throws Exception { // Details in https://github.com/eclipse/jetty.project/issues/1116 @Test void requireThatHeaderWithEmptyValueIsAllowed() throws Exception { - final JettyTestDriver driver = JettyTestDriver.newInstance(new EchoWithHeaderRequestHandler("X-Foo", "")); + JettyTestDriver driver = JettyTestDriver.newInstance(new EchoWithHeaderRequestHandler("X-Foo", "")); driver.client().get("/status.html") .expectStatusCode(is(OK)) .expectHeader("X-Foo", is("")); @@ -503,7 +502,7 @@ void requireThatHeaderWithEmptyValueIsAllowed() throws Exception { @Test void requireThatNoConnectionHeaderMeansKeepAliveInHttp11KeepAliveDisabled() throws Exception { - final JettyTestDriver driver = JettyTestDriver.newInstance(new EchoWithHeaderRequestHandler(CONNECTION, CLOSE)); + JettyTestDriver driver = JettyTestDriver.newInstance(new EchoWithHeaderRequestHandler(CONNECTION, CLOSE)); driver.client().get("/status.html") .expectHeader(CONNECTION, is(CLOSE)); assertTrue(driver.close()); @@ -512,7 +511,7 @@ void requireThatNoConnectionHeaderMeansKeepAliveInHttp11KeepAliveDisabled() thro @Test @Disabled("Temporarily ignore until stabilized") void requireThatConnectionIsClosedAfterXRequests() throws Exception { - final int MAX_REQUESTS = 10; + int MAX_REQUESTS = 10; Path privateKeyFile = File.createTempFile("junit", null, tmpFolder).toPath(); Path certificateFile = File.createTempFile("junit", null, tmpFolder).toPath(); generatePrivateKeyAndCertificate(privateKeyFile, certificateFile); @@ -567,7 +566,7 @@ void requireThatServerCanRespondToSslRequest() throws Exception { Path certificateFile = File.createTempFile("junit", null, tmpFolder).toPath(); generatePrivateKeyAndCertificate(privateKeyFile, certificateFile); - final JettyTestDriver driver = JettyTestDriver.newInstanceWithSsl(new EchoRequestHandler(), certificateFile, privateKeyFile, TlsClientAuth.WANT); + JettyTestDriver driver = JettyTestDriver.newInstanceWithSsl(new EchoRequestHandler(), certificateFile, privateKeyFile, TlsClientAuth.WANT); driver.client().get("/status.html") .expectStatusCode(is(OK)); assertTrue(driver.close()); @@ -612,7 +611,7 @@ void requireThatTlsClientAuthenticationEnforcerAllowsRequestForWhitelistedPaths( @Test void requireThatConnectedAtReturnsNonZero() throws Exception { - final JettyTestDriver driver = JettyTestDriver.newInstance(new ConnectedAtRequestHandler()); + JettyTestDriver driver = JettyTestDriver.newInstance(new ConnectedAtRequestHandler()); driver.client().get("/status.html") .expectStatusCode(is(OK)) .expectContent(matchesPattern("\\d{13,}")); @@ -928,14 +927,14 @@ private static JettyTestDriver createSslWithTlsClientAuthenticationEnforcer(Path } private static RequestHandler mockRequestHandler() { - final RequestHandler mockRequestHandler = mock(RequestHandler.class); + RequestHandler mockRequestHandler = mock(RequestHandler.class); when(mockRequestHandler.refer()).thenReturn(References.NOOP_REFERENCE); when(mockRequestHandler.refer(any())).thenReturn(References.NOOP_REFERENCE); return mockRequestHandler; } - private static String generateContent(final char c, final int len) { - final StringBuilder ret = new StringBuilder(len); + private static String generateContent(char c, int len) { + StringBuilder ret = new StringBuilder(len); for (int i = 0; i < len; ++i) { ret.append(c); } @@ -951,7 +950,7 @@ private static JettyTestDriver newDriverWithFormPostContentRemoved(RequestHandle new ConnectorConfig.Builder()); } - private static FormBodyPart newFileBody(final String fileName, final String fileContent) { + private static FormBodyPart newFileBody(String fileName, String fileContent) { return FormBodyPartBuilder.create() .setBody( new StringBody(fileContent, ContentType.TEXT_PLAIN) { @@ -966,10 +965,10 @@ private static FormBodyPart newFileBody(final String fileName, final String file private static class ConnectedAtRequestHandler extends AbstractRequestHandler { @Override - public ContentChannel handleRequest(final Request request, final ResponseHandler handler) { - final HttpRequest httpRequest = (HttpRequest)request; - final String connectedAt = String.valueOf(httpRequest.getConnectedAt(TimeUnit.MILLISECONDS)); - final ContentChannel ch = handler.handleResponse(new Response(OK)); + public ContentChannel handleRequest(Request request, ResponseHandler handler) { + HttpRequest httpRequest = (HttpRequest)request; + String connectedAt = String.valueOf(httpRequest.getConnectedAt(TimeUnit.MILLISECONDS)); + ContentChannel ch = handler.handleResponse(new Response(OK)); ch.write(ByteBuffer.wrap(connectedAt.getBytes(UTF_8)), null); ch.close(null); return null; @@ -982,7 +981,7 @@ private static class ReadBeforeWriteRequestHandler extends AbstractRequestHandle private ResponseHandler responseHandler; @Override - public synchronized ContentChannel handleRequest(final Request request, final ResponseHandler handler) { + public synchronized ContentChannel handleRequest(Request request, ResponseHandler handler) { this.responseHandler = handler; return this; } @@ -998,15 +997,15 @@ public void close(CompletionHandler completionHandler) { private static class CookieSetterRequestHandler extends AbstractRequestHandler { - final Cookie cookie; + Cookie cookie; - CookieSetterRequestHandler(final Cookie cookie) { + CookieSetterRequestHandler(Cookie cookie) { this.cookie = cookie; } @Override - public ContentChannel handleRequest(final Request request, final ResponseHandler handler) { - final HttpResponse response = HttpResponse.newInstance(OK); + public ContentChannel handleRequest(Request request, ResponseHandler handler) { + HttpResponse response = HttpResponse.newInstance(OK); response.encodeSetCookieHeader(List.of(cookie)); ResponseDispatch.newInstance(response).dispatch(handler); return null; @@ -1016,10 +1015,10 @@ public ContentChannel handleRequest(final Request request, final ResponseHandler private static class CookiePrinterRequestHandler extends AbstractRequestHandler { @Override - public ContentChannel handleRequest(final Request request, final ResponseHandler handler) { + public ContentChannel handleRequest(Request request, ResponseHandler handler) { List cookies = new ArrayList<>(((HttpRequest)request).decodeCookieHeader()); cookies.sort(new CookieComparator()); - final ContentChannel out = ResponseDispatch.newInstance(Response.Status.OK).connect(handler); + ContentChannel out = ResponseDispatch.newInstance(Response.Status.OK).connect(handler); out.write(UTF_8.encode(cookies.toString()), null); out.close(null); return null; @@ -1060,7 +1059,7 @@ public ContentChannel handleRequest(Request request, ResponseHandler handler) { private static class ThrowingHandler extends AbstractRequestHandler { @Override - public ContentChannel handleRequest(final Request request, final ResponseHandler handler) { + public ContentChannel handleRequest(Request request, ResponseHandler handler) { throw new RuntimeException("Deliberately thrown exception"); } } @@ -1070,7 +1069,7 @@ private static class UnresponsiveHandler extends AbstractRequestHandler { ResponseHandler responseHandler; @Override - public ContentChannel handleRequest(final Request request, final ResponseHandler handler) { + public ContentChannel handleRequest(Request request, ResponseHandler handler) { request.setTimeout(100, TimeUnit.MILLISECONDS); responseHandler = handler; return null; @@ -1088,17 +1087,17 @@ public ContentChannel handleRequest(Request request, ResponseHandler handler) { private static class EchoWithHeaderRequestHandler extends AbstractRequestHandler { - final String headerName; - final String headerValue; + String headerName; + String headerValue; - EchoWithHeaderRequestHandler(final String headerName, final String headerValue) { + EchoWithHeaderRequestHandler(String headerName, String headerValue) { this.headerName = headerName; this.headerValue = headerValue; } @Override - public ContentChannel handleRequest(final Request request, final ResponseHandler handler) { - final Response response = new Response(OK); + public ContentChannel handleRequest(Request request, ResponseHandler handler) { + Response response = new Response(OK); response.headers().add(headerName, headerValue); return handler.handleResponse(response); } @@ -1107,7 +1106,7 @@ public ContentChannel handleRequest(final Request request, final ResponseHandler private static class UriRequestHandler extends AbstractRequestHandler { @Override public ContentChannel handleRequest(Request req, ResponseHandler handler) { - final ContentChannel ch = handler.handleResponse(new Response(OK)); + ContentChannel ch = handler.handleResponse(new Response(OK)); ch.write(ByteBuffer.wrap(req.getUri().toString().getBytes(UTF_8)), null); ch.close(null); return null; @@ -1115,7 +1114,7 @@ public ContentChannel handleRequest(Request req, ResponseHandler handler) { } private static class RequestHeaderEchoingHandler extends AbstractRequestHandler { - final String headerName; + String headerName; RequestHeaderEchoingHandler(String headerName) { this.headerName = headerName; } @Override @@ -1129,7 +1128,7 @@ public ContentChannel handleRequest(Request request, ResponseHandler handler) { } } - private static Module newBindingSetSelector(final String setName) { + private static Module newBindingSetSelector(String setName) { return new AbstractModule() { @Override @@ -1137,7 +1136,7 @@ protected void configure() { bind(BindingSetSelector.class).toInstance(new BindingSetSelector() { @Override - public String select(final URI uri) { + public String select(URI uri) { return setName; } }); @@ -1148,7 +1147,7 @@ public String select(final URI uri) { private static class CookieComparator implements Comparator { @Override - public int compare(final Cookie lhs, final Cookie rhs) { + public int compare(Cookie lhs, Cookie rhs) { return lhs.getName().compareTo(rhs.getName()); } } diff --git a/container-disc/src/test/java/com/yahoo/jdisc/http/server/jetty/MetricAggregatingRequestLogTest.java b/container-disc/src/test/java/com/yahoo/jdisc/http/server/jetty/MetricAggregatingRequestLogTest.java index 5dba24f91d55..af41318d4897 100644 --- a/container-disc/src/test/java/com/yahoo/jdisc/http/server/jetty/MetricAggregatingRequestLogTest.java +++ b/container-disc/src/test/java/com/yahoo/jdisc/http/server/jetty/MetricAggregatingRequestLogTest.java @@ -1,17 +1,20 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.jdisc.http.server.jetty; +import ai.vespa.metrics.ContainerMetrics; import com.yahoo.jdisc.http.server.jetty.MetricAggregatingRequestLog.StatisticsEntry; +import com.yahoo.jdisc.test.MockMetric; +import com.yahoo.metrics.simple.MetricReceiver; import com.yahoo.text.Text; import org.eclipse.jetty.http.HttpVersion; + +import static com.yahoo.test.JunitCompat.assertTrue; +import static org.junit.jupiter.api.Assertions.assertEquals; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import java.util.List; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.equalTo; - /** * @author ollivir * @author bjorncs @@ -20,13 +23,25 @@ public class MetricAggregatingRequestLogTest { private final List monitoringPaths = List.of("/status.html"); private final List searchPaths = List.of("/search"); - private final MetricAggregatingRequestLog collector = new MetricAggregatingRequestLog(monitoringPaths, searchPaths, List.of(), false); + private final MockMetric metric = new MockMetric(); + private final MetricAggregatingRequestLog collector = new MetricAggregatingRequestLog(monitoringPaths, searchPaths, List.of(), false, metric, new MetricReceiver.MockReceiver()); @BeforeEach public void initializeCollector() { collector.takeStatistics(); } + @Test + void latency_is_recorded() { + testRequest("http", 200, "GET"); + assertTrue("Latency metric is recorded", metric.metrics().containsKey(MetricDefinitions.LATENCY)); + var metricSnapshot = metric.metrics().get(MetricDefinitions.LATENCY); + assertEquals(1, metricSnapshot.size()); + var latencySample = metricSnapshot.entrySet().iterator().next(); + assertEquals(200L, latencySample.getKey().get(MetricDefinitions.STATUS_CODE_DIMENSION)); + assertEquals(0.0, latencySample.getValue()); + } + @Test void statistics_are_aggregated_by_category() { testRequest("http", 300, "GET"); @@ -115,13 +130,14 @@ void request_type_can_be_set_explicitly() { assertStatisticsEntry(stats, "http", "GET", MetricDefinitions.RESPONSES_2XX, "write", 200, 1L); } - private void testRequest(String scheme, int responseCode, String httpMethod) { testRequest(scheme, responseCode, httpMethod, "foo/bar"); } + private void testRequest(String scheme, int responseCode, String httpMethod, String path) { testRequest(scheme, responseCode, httpMethod, path, null); } + private void testRequest(String scheme, int responseCode, String httpMethod, String path, com.yahoo.jdisc.Request.RequestType explicitRequestType) { var builder = JettyMockRequestBuilder.newBuilder() @@ -144,7 +160,7 @@ private static void assertStatisticsEntry(List result, String s .mapToLong(entry -> entry.value) .reduce(Long::sum) .orElseThrow(() -> new AssertionError(Text.format("Not matching entry in result (scheme=%s, method=%s, name=%s, type=%s)", scheme, method, name, requestType))); - assertThat(value, equalTo(expectedValue)); + assertEquals(expectedValue, value); } } diff --git a/metrics/src/main/java/ai/vespa/metrics/ContainerMetrics.java b/metrics/src/main/java/ai/vespa/metrics/ContainerMetrics.java index add4ba88091a..a91c3314c6ed 100644 --- a/metrics/src/main/java/ai/vespa/metrics/ContainerMetrics.java +++ b/metrics/src/main/java/ai/vespa/metrics/ContainerMetrics.java @@ -1,6 +1,8 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package ai.vespa.metrics; +import java.util.EnumSet; + /** * @author gjoranv */ @@ -52,6 +54,8 @@ public enum ContainerMetrics implements VespaMetrics { JDISC_HTTP_SSL_HANDSHAKE_FAILURE_CONNECTION_CLOSED("jdisc.http.ssl.handshake.failure.connection_closed", Unit.OPERATION, "JDISC HTTP SSL Handshake failures due to connection closed"), JDISC_HTTP_SSL_HANDSHAKE_FAILURE_UNKNOWN("jdisc.http.ssl.handshake.failure.unknown", Unit.OPERATION, "JDISC HTTP SSL Handshake failures for unknown reason"), + JDISC_HTTP_LATENCY("jdisc.http.latency", Unit.MILLISECOND, "Request latency including the HTTP layer"), + JDISC_HTTP_TIME_TO_FIRST_BYTE("jdisc.http.time_to_first_byte", Unit.MILLISECOND, "Time from request has been received by the server until the first byte is returned to the client"), JDISC_HTTP_REQUEST_PREMATURELY_CLOSED("jdisc.http.request.prematurely_closed", Unit.REQUEST, "HTTP requests prematurely closed"), JDISC_HTTP_REQUEST_REQUESTS_PER_CONNECTION("jdisc.http.request.requests_per_connection", Unit.REQUEST, "HTTP requests per connection"), JDISC_HTTP_REQUEST_URI_LENGTH("jdisc.http.request.uri_length", Unit.BYTE, "HTTP URI length"), @@ -205,10 +209,6 @@ public enum ContainerMetrics implements VespaMetrics { SERVER_NUM_SUCCESSFUL_RESPONSE_WRITES("serverNumSuccessfulResponseWrites", Unit.REQUEST, "Number of successful response writes"), SERVER_NUM_FAILED_RESPONSE_WRITES("serverNumFailedResponseWrites", Unit.REQUEST, "Number of failed response writes"), - SERVER_TOTAL_SUCCESSFUL_RESPONSE_LATENCY("serverTotalSuccessfulResponseLatency", Unit.MILLISECOND, "Total duration for execution of successful responses"), - SERVER_TOTAL_FAILED_RESPONSE_LATENCY("serverTotalFailedResponseLatency", Unit.MILLISECOND, "Total duration for execution of failed responses"), - SERVER_TIME_TO_FIRST_BYTE("serverTimeToFirstByte", Unit.MILLISECOND, "Time from request has been received by the server until the first byte is returned to the client"), - SERVER_STARTED_MILLIS("serverStartedMillis", Unit.MILLISECOND, "Time since the service was started"), EMBEDDER_LATENCY("embedder.latency", Unit.MILLISECOND, "Time spent creating an embedding"), diff --git a/metrics/src/main/java/ai/vespa/metrics/set/DefaultMetrics.java b/metrics/src/main/java/ai/vespa/metrics/set/DefaultMetrics.java index 61f969341a7e..2cbbadc746ce 100644 --- a/metrics/src/main/java/ai/vespa/metrics/set/DefaultMetrics.java +++ b/metrics/src/main/java/ai/vespa/metrics/set/DefaultMetrics.java @@ -62,7 +62,7 @@ private static MetricSet getContainerMetrics() { .metric(ContainerMetrics.HTTP_STATUS_5XX.rate()) .metric(ContainerMetrics.JDISC_GC_MS, EnumSet.of(max, average)) .metric(ContainerMetrics.MEM_HEAP_FREE.average()) - .metric(ContainerMetrics.SERVER_TOTAL_SUCCESSFUL_RESPONSE_LATENCY, EnumSet.of(sum, count, max, ninety_five_percentile, ninety_nine_percentile)) + .metric(ContainerMetrics.JDISC_HTTP_LATENCY, EnumSet.of(sum, count, max, ninety_five_percentile, ninety_nine_percentile)) .metric(ContainerMetrics.FEED_LATENCY, EnumSet.of(sum, count)) // .metric(ContainerMetrics.CPU.baseName()) // TODO: Add to container metrics .metric(ContainerMetrics.JDISC_THREAD_POOL_SIZE.max()) diff --git a/metrics/src/main/java/ai/vespa/metrics/set/InfrastructureMetricSet.java b/metrics/src/main/java/ai/vespa/metrics/set/InfrastructureMetricSet.java index d2db5bda8dbd..3987f331a357 100644 --- a/metrics/src/main/java/ai/vespa/metrics/set/InfrastructureMetricSet.java +++ b/metrics/src/main/java/ai/vespa/metrics/set/InfrastructureMetricSet.java @@ -147,7 +147,7 @@ private static Set getConfigServerMetrics() { addMetric(metrics, ContainerMetrics.MEM_HEAP_USED.average()); addMetric(metrics, ContainerMetrics.SERVER_NUM_REQUESTS.count()); addMetric(metrics, ContainerMetrics.SERVER_STARTED_MILLIS.max()); - addMetric(metrics, ContainerMetrics.SERVER_TOTAL_SUCCESSFUL_RESPONSE_LATENCY.max()); + addMetric(metrics, ContainerMetrics.JDISC_HTTP_LATENCY.max()); return metrics; } diff --git a/metrics/src/main/java/ai/vespa/metrics/set/Vespa9DefaultMetricSet.java b/metrics/src/main/java/ai/vespa/metrics/set/Vespa9DefaultMetricSet.java index 806de58e6c6f..c8cee056d0ab 100644 --- a/metrics/src/main/java/ai/vespa/metrics/set/Vespa9DefaultMetricSet.java +++ b/metrics/src/main/java/ai/vespa/metrics/set/Vespa9DefaultMetricSet.java @@ -16,7 +16,6 @@ import java.util.EnumSet; import java.util.List; -import static ai.vespa.metrics.Suffix.average; import static ai.vespa.metrics.Suffix.count; import static ai.vespa.metrics.Suffix.max; import static ai.vespa.metrics.Suffix.min; @@ -59,7 +58,7 @@ private static MetricSet getContainerMetrics() { return new MetricSet.Builder("default-container") .metric(MicrometerMetrics.JVM_GC_OVERHEAD, EnumSet.of(sum, count, max)) .metric(ContainerMetrics.MEM_HEAP_FREE.average()) - .metric(ContainerMetrics.SERVER_TOTAL_SUCCESSFUL_RESPONSE_LATENCY, EnumSet.of(sum, count, max, ninety_five_percentile, ninety_nine_percentile)) + .metric(ContainerMetrics.JDISC_HTTP_LATENCY, EnumSet.of(sum, count, max, ninety_five_percentile, ninety_nine_percentile)) .metric(ContainerMetrics.FEED_LATENCY, EnumSet.of(sum, count)) .metric(ContainerMetrics.HANDLED_LATENCY, EnumSet.of(sum, count, max)) .metric(ContainerMetrics.CPU.baseName()) diff --git a/metrics/src/main/java/ai/vespa/metrics/set/Vespa9VespaMetricSet.java b/metrics/src/main/java/ai/vespa/metrics/set/Vespa9VespaMetricSet.java index 26556d051049..4bd92aa81860 100644 --- a/metrics/src/main/java/ai/vespa/metrics/set/Vespa9VespaMetricSet.java +++ b/metrics/src/main/java/ai/vespa/metrics/set/Vespa9VespaMetricSet.java @@ -108,7 +108,7 @@ private static Set getContainerMetrics() { addMetric(metrics, ContainerMetrics.HANDLED_REQUESTS.count()); addMetric(metrics, ContainerMetrics.HANDLED_LATENCY, EnumSet.of(sum, count, max)); - addMetric(metrics, ContainerMetrics.SERVER_TOTAL_SUCCESSFUL_RESPONSE_LATENCY, EnumSet.of(max, sum, count, ninety_five_percentile, ninety_nine_percentile)); + addMetric(metrics, ContainerMetrics.JDISC_HTTP_LATENCY, EnumSet.of(max, sum, count, ninety_five_percentile, ninety_nine_percentile)); addMetric(metrics, ContainerMetrics.SERVER_NUM_OPEN_CONNECTIONS, EnumSet.of(max, average)); addMetric(metrics, ContainerMetrics.SERVER_NUM_CONNECTIONS, EnumSet.of(max, average)); diff --git a/metrics/src/main/java/ai/vespa/metrics/set/VespaMetricSet.java b/metrics/src/main/java/ai/vespa/metrics/set/VespaMetricSet.java index 4777f203c96d..1a51bf4e0ad6 100644 --- a/metrics/src/main/java/ai/vespa/metrics/set/VespaMetricSet.java +++ b/metrics/src/main/java/ai/vespa/metrics/set/VespaMetricSet.java @@ -133,7 +133,7 @@ private static Set getContainerMetrics() { addMetric(metrics, ContainerMetrics.HANDLED_REQUESTS.count()); addMetric(metrics, ContainerMetrics.HANDLED_LATENCY, EnumSet.of(sum, count, max)); - addMetric(metrics, ContainerMetrics.SERVER_TOTAL_SUCCESSFUL_RESPONSE_LATENCY, EnumSet.of(max, sum, count, ninety_five_percentile, ninety_nine_percentile)); + addMetric(metrics, ContainerMetrics.JDISC_HTTP_LATENCY, EnumSet.of(max, sum, count, ninety_five_percentile, ninety_nine_percentile)); addMetric(metrics, ContainerMetrics.SERVER_NUM_OPEN_CONNECTIONS, EnumSet.of(max, last, average)); // TODO: Vespa 9: Remove last addMetric(metrics, ContainerMetrics.SERVER_NUM_CONNECTIONS, EnumSet.of(max, last, average)); // TODO: Vespa 9: Remove last From 640bf437062721c0fdb2fd30612b197a4e997d4f Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Tue, 10 Mar 2026 22:03:33 +0100 Subject: [PATCH 2/2] Use the right assertEquals --- .../http/server/jetty/MetricAggregatingRequestLogTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/container-disc/src/test/java/com/yahoo/jdisc/http/server/jetty/MetricAggregatingRequestLogTest.java b/container-disc/src/test/java/com/yahoo/jdisc/http/server/jetty/MetricAggregatingRequestLogTest.java index af41318d4897..4e7efc1be1f7 100644 --- a/container-disc/src/test/java/com/yahoo/jdisc/http/server/jetty/MetricAggregatingRequestLogTest.java +++ b/container-disc/src/test/java/com/yahoo/jdisc/http/server/jetty/MetricAggregatingRequestLogTest.java @@ -8,8 +8,8 @@ import com.yahoo.text.Text; import org.eclipse.jetty.http.HttpVersion; -import static com.yahoo.test.JunitCompat.assertTrue; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -34,7 +34,7 @@ public void initializeCollector() { @Test void latency_is_recorded() { testRequest("http", 200, "GET"); - assertTrue("Latency metric is recorded", metric.metrics().containsKey(MetricDefinitions.LATENCY)); + assertTrue(metric.metrics().containsKey(MetricDefinitions.LATENCY)); var metricSnapshot = metric.metrics().get(MetricDefinitions.LATENCY); assertEquals(1, metricSnapshot.size()); var latencySample = metricSnapshot.entrySet().iterator().next();