Add status code as dimension to response metrics

3 files changed, 43 insertions, 26 deletions

diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollector.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollector.java
index d6c0699edbd..8fd4c064a2c 100644
--- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollector.java
+++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollector.java
@@ -181,18 +181,20 @@ class HttpResponseStatisticsCollector extends HandlerWrapper implements Graceful
         final String scheme;
         final String method;
         final String requestType;
+        final int statusCode;
 
-        private Dimensions(String protocol, String scheme, String method, String requestType) {
+        private Dimensions(String protocol, String scheme, String method, String requestType, int statusCode) {
             this.protocol = protocol;
             this.scheme = scheme;
             this.method = method;
             this.requestType = requestType;
+            this.statusCode = statusCode;
         }
 
         static Dimensions of(Request req, Collection<String> monitoringHandlerPaths,
                              Collection<String> searchHandlerPaths) {
             String requestType = requestType(req, monitoringHandlerPaths, searchHandlerPaths);
-            return new Dimensions(protocol(req), scheme(req), method(req), requestType);
+            return new Dimensions(protocol(req), scheme(req), method(req), requestType, statusCode(req));
         }
 
         Map<String, Object> asMap() {
@@ -201,6 +203,7 @@ class HttpResponseStatisticsCollector extends HandlerWrapper implements Graceful
             builder.put(MetricDefinitions.SCHEME_DIMENSION, scheme);
             builder.put(MetricDefinitions.METHOD_DIMENSION, method);
             builder.put(MetricDefinitions.REQUEST_TYPE_DIMENSION, requestType);
+            builder.put(MetricDefinitions.STATUS_CODE_DIMENSION, (long) statusCode);
             return Map.copyOf(builder);
         }
 
@@ -243,6 +246,8 @@ class HttpResponseStatisticsCollector extends HandlerWrapper implements Graceful
             }
         }
 
+        private static int statusCode(Request req) { return req.getResponse().getStatus(); }
+
         private static String requestType(Request req, Collection<String> monitoringHandlerPaths,
                                           Collection<String> searchHandlerPaths) {
             HttpRequest.RequestType requestType = (HttpRequest.RequestType)req.getAttribute(requestTypeAttribute);
@@ -259,16 +264,21 @@ class HttpResponseStatisticsCollector extends HandlerWrapper implements Graceful
             else return "write";
         }
 
+
         @Override
         public boolean equals(Object o) {
             if (this == o) return true;
             if (o == null || getClass() != o.getClass()) return false;
             Dimensions that = (Dimensions) o;
-            return Objects.equals(protocol, that.protocol) && Objects.equals(scheme, that.scheme)
-                    && Objects.equals(method, that.method) && Objects.equals(requestType, that.requestType);
+            return statusCode == that.statusCode && Objects.equals(protocol, that.protocol)
+                    && Objects.equals(scheme, that.scheme) && Objects.equals(method, that.method)
+                    && Objects.equals(requestType, that.requestType);
         }
 
-        @Override public int hashCode() { return Objects.hash(protocol, scheme, method, requestType); }
+        @Override
+        public int hashCode() {
+            return Objects.hash(protocol, scheme, method, requestType, statusCode);
+        }
 
     }
 
diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/MetricDefinitions.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/MetricDefinitions.java
index 71c60ad6737..4099511c2c2 100644
--- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/MetricDefinitions.java
+++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/MetricDefinitions.java
@@ -17,6 +17,7 @@ class MetricDefinitions {
     static final String REQUEST_SERVER_NAME_DIMENSION = "requestServerName";
     static final String FILTER_CHAIN_ID_DIMENSION = "chainId";
     static final String PROTOCOL_DIMENSION = "protocol";
+    static final String STATUS_CODE_DIMENSION = "statusCode";
 
     static final String NUM_OPEN_CONNECTIONS = "serverNumOpenConnections";
     static final String NUM_CONNECTIONS_OPEN_MAX = "serverConnectionsOpenMax";
diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollectorTest.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollectorTest.java
index eecab11d9fe..3187ea510d4 100644
--- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollectorTest.java
+++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollectorTest.java
@@ -31,6 +31,7 @@ import static org.hamcrest.Matchers.equalTo;
 
 /**
  * @author ollivir
+ * @author bjorncs
  */
 public class HttpResponseStatisticsCollectorTest {
 
@@ -47,8 +48,9 @@ public class HttpResponseStatisticsCollectorTest {
         testRequest("http", 200, "GET");
 
         var stats = collector.takeStatistics();
-        assertStatisticsEntry(stats, "http", "GET", MetricDefinitions.RESPONSES_2XX, "read", 1L);
-        assertStatisticsEntry(stats, "http", "GET", MetricDefinitions.RESPONSES_3XX, "read", 2L);
+        assertStatisticsEntry(stats, "http", "GET", MetricDefinitions.RESPONSES_2XX, "read", 200, 1L);
+        assertStatisticsEntry(stats, "http", "GET", MetricDefinitions.RESPONSES_3XX, "read", 301, 1L);
+        assertStatisticsEntry(stats, "http", "GET", MetricDefinitions.RESPONSES_3XX, "read", 300, 1L);
     }
 
     @Test
@@ -65,12 +67,12 @@ public class HttpResponseStatisticsCollectorTest {
         testRequest("https", 200, "POST");
 
         var stats = collector.takeStatistics();
-        assertStatisticsEntry(stats, "http", "GET", MetricDefinitions.RESPONSES_2XX, "read", 1L);
-        assertStatisticsEntry(stats, "http", "GET", MetricDefinitions.RESPONSES_4XX, "read", 1L);
-        assertStatisticsEntry(stats, "http", "PUT", MetricDefinitions.RESPONSES_2XX, "write", 1L);
-        assertStatisticsEntry(stats, "http", "POST", MetricDefinitions.RESPONSES_2XX, "write", 2L);
-        assertStatisticsEntry(stats, "https", "GET", MetricDefinitions.RESPONSES_4XX, "read", 1L);
-        assertStatisticsEntry(stats, "https", "POST", MetricDefinitions.RESPONSES_2XX, "write", 4L);
+        assertStatisticsEntry(stats, "http", "GET", MetricDefinitions.RESPONSES_2XX, "read", 200, 1L);
+        assertStatisticsEntry(stats, "http", "GET", MetricDefinitions.RESPONSES_4XX, "read", 404, 1L);
+        assertStatisticsEntry(stats, "http", "PUT", MetricDefinitions.RESPONSES_2XX, "write", 200, 1L);
+        assertStatisticsEntry(stats, "http", "POST", MetricDefinitions.RESPONSES_2XX, "write", 200, 2L);
+        assertStatisticsEntry(stats, "https", "GET", MetricDefinitions.RESPONSES_4XX, "read", 404, 1L);
+        assertStatisticsEntry(stats, "https", "POST", MetricDefinitions.RESPONSES_2XX, "write", 200, 4L);
     }
 
     @Test
@@ -80,9 +82,11 @@ public class HttpResponseStatisticsCollectorTest {
         testRequest("http", 403, "GET");
 
         var stats = collector.takeStatistics();
-        assertStatisticsEntry(stats, "http", "GET", MetricDefinitions.RESPONSES_4XX, "read", 3L);
-        assertStatisticsEntry(stats, "http", "GET", MetricDefinitions.RESPONSES_401, "read", 1L);
-        assertStatisticsEntry(stats, "http", "GET", MetricDefinitions.RESPONSES_403, "read", 1L);
+        assertStatisticsEntry(stats, "http", "GET", MetricDefinitions.RESPONSES_4XX, "read", 401, 1L);
+        assertStatisticsEntry(stats, "http", "GET", MetricDefinitions.RESPONSES_4XX, "read", 403, 1L);
+        assertStatisticsEntry(stats, "http", "GET", MetricDefinitions.RESPONSES_4XX, "read", 404, 1L);
+        assertStatisticsEntry(stats, "http", "GET", MetricDefinitions.RESPONSES_401, "read", 401, 1L);
+        assertStatisticsEntry(stats, "http", "GET", MetricDefinitions.RESPONSES_403, "read", 403, 1L);
 
     }
 
@@ -92,12 +96,12 @@ public class HttpResponseStatisticsCollectorTest {
         testRequest("http", 200, "GET");
 
         var stats = collector.takeStatistics();
-        assertStatisticsEntry(stats, "http", "GET", MetricDefinitions.RESPONSES_2XX, "read", 2L);
+        assertStatisticsEntry(stats, "http", "GET", MetricDefinitions.RESPONSES_2XX, "read", 200, 2L);
 
         testRequest("http", 200, "GET");
 
         stats = collector.takeStatistics();
-        assertStatisticsEntry(stats, "http", "GET", MetricDefinitions.RESPONSES_2XX, "read", 1L);
+        assertStatisticsEntry(stats, "http", "GET", MetricDefinitions.RESPONSES_2XX, "read", 200, 1L);
     }
 
     @Test
@@ -108,15 +112,15 @@ public class HttpResponseStatisticsCollectorTest {
         testRequest("http", 200, "GET", "/status.html?foo=bar");
 
         var stats = collector.takeStatistics();
-        assertStatisticsEntry(stats, "http", "GET", MetricDefinitions.RESPONSES_2XX, "monitoring", 1L);
-        assertStatisticsEntry(stats, "http", "GET", MetricDefinitions.RESPONSES_2XX, "read", 1L);
-        assertStatisticsEntry(stats, "http", "POST", MetricDefinitions.RESPONSES_2XX, "read", 1L);
-        assertStatisticsEntry(stats, "http", "POST", MetricDefinitions.RESPONSES_2XX, "write", 1L);
+        assertStatisticsEntry(stats, "http", "GET", MetricDefinitions.RESPONSES_2XX, "monitoring", 200, 1L);
+        assertStatisticsEntry(stats, "http", "GET", MetricDefinitions.RESPONSES_2XX, "read", 200, 1L);
+        assertStatisticsEntry(stats, "http", "POST", MetricDefinitions.RESPONSES_2XX, "read", 200, 1L);
+        assertStatisticsEntry(stats, "http", "POST", MetricDefinitions.RESPONSES_2XX, "write", 200, 1L);
 
         testRequest("http", 200, "GET");
 
         stats = collector.takeStatistics();
-        assertStatisticsEntry(stats, "http", "GET", MetricDefinitions.RESPONSES_2XX, "read", 1L);
+        assertStatisticsEntry(stats, "http", "GET", MetricDefinitions.RESPONSES_2XX, "read", 200, 1L);
     }
 
     @Test
@@ -124,7 +128,7 @@ public class HttpResponseStatisticsCollectorTest {
         testRequest("http", 200, "GET", "/search", com.yahoo.jdisc.Request.RequestType.WRITE);
 
         var stats = collector.takeStatistics();
-        assertStatisticsEntry(stats, "http", "GET", MetricDefinitions.RESPONSES_2XX, "write", 1L);
+        assertStatisticsEntry(stats, "http", "GET", MetricDefinitions.RESPONSES_2XX, "write", 200, 1L);
     }
 
     @Before
@@ -172,12 +176,14 @@ public class HttpResponseStatisticsCollectorTest {
         return req;
     }
 
-    private static void assertStatisticsEntry(List<StatisticsEntry> result, String scheme, String method, String name, String requestType, long expectedValue) {
+    private static void assertStatisticsEntry(List<StatisticsEntry> result, String scheme, String method, String name,
+                                              String requestType, int statusCode, long expectedValue) {
         long value = result.stream()
                 .filter(entry -> entry.dimensions.method.equals(method)
                         && entry.dimensions.scheme.equals(scheme)
                         && entry.name.equals(name)
-                        && entry.dimensions.requestType.equals(requestType))
+                        && entry.dimensions.requestType.equals(requestType)
+                        && entry.dimensions.statusCode == statusCode)
                 .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)));