Add request type dimension to http.nxx metrics

5 files changed, 80 insertions, 14 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/http/JettyHttpServer.java b/config-model/src/main/java/com/yahoo/vespa/model/container/http/JettyHttpServer.java
index ab491367510..7374242e94b 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/http/JettyHttpServer.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/http/JettyHttpServer.java
@@ -52,6 +52,9 @@ public class JettyHttpServer extends SimpleComponent implements ServerConfig.Pro
 
     @Override
     public void getConfig(ServerConfig.Builder builder) {
+        builder.metric(new ServerConfig.Metric.Builder().monitoringHandlerPaths(
+                List.of("/state/v1", "/status.html")
+        ));
     }
 
     static ComponentModel providerComponentModel(final ComponentId parentId, String className) {
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollector.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollector.java
index 4239d2120cf..731e737d683 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollector.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollector.java
@@ -34,6 +34,7 @@ import java.util.concurrent.atomic.LongAdder;
  */
 public class HttpResponseStatisticsCollector extends HandlerWrapper implements Graceful {
     private final AtomicReference<FutureCallback> shutdown = new AtomicReference<>();
+    private final List<String> monitoringHandlerPaths;
 
     public static enum HttpMethod {
         GET, PATCH, POST, PUT, DELETE, OPTIONS, HEAD, OTHER
@@ -43,20 +44,28 @@ public class HttpResponseStatisticsCollector extends HandlerWrapper implements G
         HTTP, HTTPS, OTHER
     }
 
+    public enum RequestType {
+        READ, WRITE, MONITORING
+    }
+
     private static final String[] HTTP_RESPONSE_GROUPS = { Metrics.RESPONSES_1XX, Metrics.RESPONSES_2XX, Metrics.RESPONSES_3XX,
             Metrics.RESPONSES_4XX, Metrics.RESPONSES_5XX, Metrics.RESPONSES_401, Metrics.RESPONSES_403};
 
     private final AtomicLong inFlight = new AtomicLong();
-    private final LongAdder statistics[][][];
+    private final LongAdder statistics[][][][];
 
-    public HttpResponseStatisticsCollector() {
+    public HttpResponseStatisticsCollector(List<String> monitoringHandlerPaths) {
         super();
-        statistics = new LongAdder[HttpScheme.values().length][HttpMethod.values().length][];
+        this.monitoringHandlerPaths = monitoringHandlerPaths;
+        statistics = new LongAdder[HttpScheme.values().length][HttpMethod.values().length][][];
         for (int scheme = 0; scheme < HttpScheme.values().length; ++scheme) {
             for (int method = 0; method < HttpMethod.values().length; method++) {
-                statistics[scheme][method] = new LongAdder[HTTP_RESPONSE_GROUPS.length];
+                statistics[scheme][method] = new LongAdder[HTTP_RESPONSE_GROUPS.length][];
                 for (int group = 0; group < HTTP_RESPONSE_GROUPS.length; group++) {
-                    statistics[scheme][method][group] = new LongAdder();
+                    statistics[scheme][method][group] = new LongAdder[RequestType.values().length];
+                    for (int requestType = 0 ; requestType < RequestType.values().length; requestType++) {
+                        statistics[scheme][method][group][requestType] = new LongAdder();
+                    }
                 }
             }
         }
@@ -117,9 +126,11 @@ public class HttpResponseStatisticsCollector extends HandlerWrapper implements G
         if (group >= 0) {
             HttpScheme scheme = getScheme(request);
             HttpMethod method = getMethod(request);
-            statistics[scheme.ordinal()][method.ordinal()][group].increment();
+            RequestType requestType = getRequestType(request);
+
+            statistics[scheme.ordinal()][method.ordinal()][group][requestType.ordinal()].increment();
             if (group == 5 || group == 6) { // if 401/403, also increment 4xx
-                statistics[scheme.ordinal()][method.ordinal()][3].increment();
+                statistics[scheme.ordinal()][method.ordinal()][3][requestType.ordinal()].increment();
             }
         }
 
@@ -184,6 +195,18 @@ public class HttpResponseStatisticsCollector extends HandlerWrapper implements G
         }
     }
 
+    private RequestType getRequestType(Request request) {
+        String path = request.getRequestURI();
+        for (String monitoringHandlerPath : monitoringHandlerPaths) {
+            if (path.startsWith(monitoringHandlerPath)) return RequestType.MONITORING;
+        }
+        if ("GET".equals(request.getMethod())) {
+            return RequestType.READ;
+        } else {
+            return RequestType.WRITE;
+        }
+    }
+
     public List<StatisticsEntry> takeStatistics() {
         var ret = new ArrayList<StatisticsEntry>();
         for (HttpScheme scheme : HttpScheme.values()) {
@@ -191,9 +214,11 @@ public class HttpResponseStatisticsCollector extends HandlerWrapper implements G
             for (HttpMethod method : HttpMethod.values()) {
                 int methodIndex = method.ordinal();
                 for (int group = 0; group < HTTP_RESPONSE_GROUPS.length; group++) {
-                    long value = statistics[schemeIndex][methodIndex][group].sumThenReset();
-                    if (value > 0) {
-                        ret.add(new StatisticsEntry(scheme.name().toLowerCase(), method.name(), HTTP_RESPONSE_GROUPS[group], value));
+                    for (RequestType type : RequestType.values()) {
+                        long value = statistics[schemeIndex][methodIndex][group][type.ordinal()].sumThenReset();
+                        if (value > 0) {
+                            ret.add(new StatisticsEntry(scheme.name().toLowerCase(), method.name(), HTTP_RESPONSE_GROUPS[group], type.name().toLowerCase(), value));
+                        }
                     }
                 }
             }
@@ -239,13 +264,15 @@ public class HttpResponseStatisticsCollector extends HandlerWrapper implements G
         public final String scheme;
         public final String method;
         public final String name;
+        public final String requestType;
         public final long value;
 
 
-        public StatisticsEntry(String scheme, String method, String name, long value) {
+        public StatisticsEntry(String scheme, String method, String name, String requestType, long value) {
             this.scheme = scheme;
             this.method = method;
             this.name = name;
+            this.requestType = requestType;
             this.value = value;
         }
     }
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java
index 04db58f6d07..a33278c7e02 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java
@@ -70,6 +70,7 @@ public class JettyHttpServer extends AbstractServerProvider {
         String PORT_DIMENSION = "serverPort";
         String METHOD_DIMENSION = "httpMethod";
         String SCHEME_DIMENSION = "scheme";
+        String REQUEST_TYPE_DIMENSION = "requestType";
 
         String NUM_OPEN_CONNECTIONS = "serverNumOpenConnections";
         String NUM_CONNECTIONS_OPEN_MAX = "serverConnectionsOpenMax";
@@ -255,7 +256,7 @@ public class JettyHttpServer extends AbstractServerProvider {
         GzipHandler gzipHandler = newGzipHandler(serverConfig);
         gzipHandler.setHandler(authEnforcer);
 
-        HttpResponseStatisticsCollector statisticsCollector = new HttpResponseStatisticsCollector();
+        HttpResponseStatisticsCollector statisticsCollector = new HttpResponseStatisticsCollector(serverConfig.metric().monitoringHandlerPaths());
         statisticsCollector.setHandler(gzipHandler);
 
         StatisticsHandler statisticsHandler = newStatisticsHandler();
@@ -380,6 +381,7 @@ public class JettyHttpServer extends AbstractServerProvider {
             Map<String, Object> dimensions = new HashMap<>();
             dimensions.put(Metrics.METHOD_DIMENSION, metricEntry.method);
             dimensions.put(Metrics.SCHEME_DIMENSION, metricEntry.scheme);
+            dimensions.put(Metrics.REQUEST_TYPE_DIMENSION, metricEntry.requestType);
             metric.add(metricEntry.name, metricEntry.value, metric.createContext(dimensions));
         }
     }
diff --git a/jdisc_http_service/src/main/resources/configdefinitions/jdisc.http.server.def b/jdisc_http_service/src/main/resources/configdefinitions/jdisc.http.server.def
index 33f82963243..e968d17df85 100644
--- a/jdisc_http_service/src/main/resources/configdefinitions/jdisc.http.server.def
+++ b/jdisc_http_service/src/main/resources/configdefinitions/jdisc.http.server.def
@@ -38,3 +38,6 @@ jmx.enabled                           bool   default = false
 
 # Listen port for the JMX server.
 jmx.listenPort                        int    default = 1099
+
+# Paths that should be reported with monitoring dimensions where applicable
+metric.monitoringHandlerPaths[]       string
\ No newline at end of file
diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollectorTest.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollectorTest.java
index 7176a0a1ff6..e06905a0212 100644
--- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollectorTest.java
+++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollectorTest.java
@@ -35,7 +35,8 @@ import static org.hamcrest.Matchers.equalTo;
  */
 public class HttpResponseStatisticsCollectorTest {
     private Connector connector;
-    private HttpResponseStatisticsCollector collector = new HttpResponseStatisticsCollector();
+    private List<String> monitoringPaths = List.of("/status.html");
+    private HttpResponseStatisticsCollector collector = new HttpResponseStatisticsCollector(monitoringPaths);
     private int httpResponseCode = 500;
 
     @Test
@@ -98,6 +99,24 @@ public class HttpResponseStatisticsCollectorTest {
         assertStatisticsEntryPresent(stats, "http", "GET", Metrics.RESPONSES_2XX, 1L);
     }
 
+    @Test
+    public void statistics_include_request_type_dimension() throws Exception {
+        testRequest("http", 200, "GET", "/search");
+        testRequest("http", 200, "POST", "/feed");
+        testRequest("http", 200, "GET", "/status.html?foo=bar");
+
+        var stats = collector.takeStatistics();
+        assertStatisticsEntryWithRequestTypePresent(stats, "http", "GET", Metrics.RESPONSES_2XX, "monitoring", 1L);
+        assertStatisticsEntryWithRequestTypePresent(stats, "http", "GET", Metrics.RESPONSES_2XX, "read", 1L);
+        assertStatisticsEntryWithRequestTypePresent(stats, "http", "POST", Metrics.RESPONSES_2XX, "write", 1L);
+
+        testRequest("http", 200, "GET");
+
+        stats = collector.takeStatistics();
+        assertStatisticsEntryPresent(stats, "http", "GET", Metrics.RESPONSES_2XX, 1L);
+
+    }
+
     @Before
     public void initializeCollector() throws Exception {
         Server server = new Server();
@@ -124,8 +143,11 @@ public class HttpResponseStatisticsCollectorTest {
     }
 
     private Request testRequest(String scheme, int responseCode, String httpMethod) throws Exception {
+        return testRequest(scheme, responseCode, httpMethod, "foo/bar");
+    }
+    private Request testRequest(String scheme, int responseCode, String httpMethod, String path) throws Exception {
         HttpChannel channel = new HttpChannel(connector, new HttpConfiguration(), null, new DummyTransport());
-        MetaData.Request metaData = new MetaData.Request(httpMethod, new HttpURI(scheme + "://foo/bar"), HttpVersion.HTTP_1_1, new HttpFields());
+        MetaData.Request metaData = new MetaData.Request(httpMethod, new HttpURI(scheme + "://" + path), HttpVersion.HTTP_1_1, new HttpFields());
         Request req = channel.getRequest();
         req.setMetaData(metaData);
 
@@ -143,6 +165,15 @@ public class HttpResponseStatisticsCollectorTest {
         assertThat(value, equalTo(expectedValue));
     }
 
+    private static void assertStatisticsEntryWithRequestTypePresent(List<StatisticsEntry> result, String scheme, String method, String name, String requestType, long expectedValue) {
+        long value = result.stream()
+                .filter(entry -> entry.method.equals(method) && entry.scheme.equals(scheme) && entry.name.equals(name) && entry.requestType.equals(requestType))
+                .mapToLong(entry -> entry.value)
+                .reduce(Long::sum)
+                .orElseThrow(() -> new AssertionError(String.format("Not matching entry in result (scheme=%s, method=%s, name=%s, type=%s)", scheme, method, name, requestType)));
+        assertThat(value, equalTo(expectedValue));
+    }
+
     private final class DummyTransport implements HttpTransport {
         @Override
         public void send(Response info, boolean head, ByteBuffer content, boolean lastContent, Callback callback) {