Add jdisc.http.latency metric

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<Tuple> collapseHealthMetrics(MetricSnapshot snapshot) {
         Tuple latencySeconds = new Tuple(NULL_DIMENSIONS, "latencySeconds", null);
         for (Map.Entry<MetricDimensions, MetricSet> 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,

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);

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<String> searchHandlerPaths;
     private final Set<String> ignoredUserAgents;
     private final boolean reporterEnabled;
+    private final Metric metric;
 
     private final ConcurrentMap<StatusCodeMetric, LongAdder> 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<String> monitoringHandlerPaths, List<String> searchHandlerPaths,
-                                List<String> ignoredUserAgents, boolean reporterEnabled) {
+                                List<String> 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<StatisticsEntry> takeStatistics() {
         if (reporterEnabled)
             throw new IllegalStateException("Cannot take consistent snapshot while reporter is enabled");
-        var ret = new ArrayList<StatisticsEntry>();
-        consume((metric, value) -> ret.add(new StatisticsEntry(metric, value)));
-        return ret;
+        var statisticsList = new ArrayList<StatisticsEntry>();
+        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<StatusCodeMetric> 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<StatusCodeMetric> of(Request req, int statusCode, List<String> monitoringHandlerPaths,
-                                   List<String> searchHandlerPaths) {
-            Dimensions dimensions = Dimensions.of(req, statusCode, monitoringHandlerPaths, searchHandlerPaths);
-            return metricNames(statusCode).stream()
+        static Set<StatusCodeMetric> 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); }
     }
+
 }

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() {}
+
 }

container-disc/src/main/java/com/yahoo/jdisc/http/server/jetty/RequestMetricReporter.java

@@ -8,21 +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;
@@ -50,14 +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);
     }
 

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);
         }
 

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