MetricsFilter to report on HTTP status code

simpleclient_servlet/src/main/java/io/prometheus/client/filter/MetricsFilter.java

@@ -1,6 +1,7 @@
 package io.prometheus.client.filter;
 
 import io.prometheus.client.Histogram;
+import io.prometheus.client.SimpleTimer;
 
 import javax.servlet.Filter;
 import javax.servlet.FilterChain;
@@ -9,6 +10,7 @@
 import javax.servlet.ServletRequest;
 import javax.servlet.ServletResponse;
 import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
 import java.io.IOException;
 
 /**
@@ -29,7 +31,7 @@
  * {@code
  * <filter>
  *   <filter-name>prometheusFilter</filter-name>
- *   <filter-class>net.cccnext.ssp.portal.spring.filter.PrometheusMetricsFilter</filter-class>
+ *   <filter-class>io.prometheus.client.filter.MetricsFilter</filter-class>
  *   <init-param>
  *      <param-name>metric-name</param-name>
  *      <param-value>webapp_metrics_filter</param-value>
@@ -65,7 +67,8 @@ public class MetricsFilter implements Filter {
     private String help = "The time taken fulfilling servlet requests";
     private double[] buckets = null;
 
-    public MetricsFilter() {}
+    public MetricsFilter() {
+    }
 
     public MetricsFilter(
             String metricName,
@@ -86,12 +89,12 @@ private boolean isEmpty(String s) {
         return s == null || s.length() == 0;
     }
 
-    private String getComponents(String str) {
+    protected String getComponents(String str) {
         if (str == null || pathComponents < 1) {
             return str;
         }
         int count = 0;
-        int i =  -1;
+        int i = -1;
         do {
             i = str.indexOf("/", i + 1);
             if (i < 0) {
@@ -106,8 +109,7 @@ private String getComponents(String str) {
 
     @Override
     public void init(FilterConfig filterConfig) throws ServletException {
-        Histogram.Builder builder = Histogram.build()
-                .labelNames("path", "method");
+        Histogram.Builder builder = Histogram.build();
 
         if (filterConfig == null && isEmpty(metricName)) {
             throw new ServletException("No configuration object provided, and no metricName passed via constructor");
@@ -141,6 +143,8 @@ public void init(FilterConfig filterConfig) throws ServletException {
             }
         }
 
+        addLabelsNames(builder);
+
         if (buckets != null) {
             builder = builder.buckets(buckets);
         }
@@ -151,25 +155,33 @@ public void init(FilterConfig filterConfig) throws ServletException {
                 .register();
     }
 
+    Histogram.Builder addLabelsNames(Histogram.Builder builder) {
+        return builder.labelNames("path", "method");
+    }
+
+    Histogram.Child addLabelsValues(Histogram builder, HttpServletRequest request, HttpServletResponse response) {
+        String path = request.getRequestURI();
+
+        return builder.labels(getComponents(path), request.getMethod());
+    }
+
+
     @Override
     public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse, FilterChain filterChain) throws IOException, ServletException {
-        if (!(servletRequest instanceof HttpServletRequest)) {
+        if (!(servletRequest instanceof HttpServletRequest) || !(servletResponse instanceof HttpServletResponse)) {
             filterChain.doFilter(servletRequest, servletResponse);
             return;
         }
 
-        HttpServletRequest request = (HttpServletRequest) servletRequest;
-
-        String path = request.getRequestURI();
-
-        Histogram.Timer timer = histogram
-            .labels(getComponents(path), request.getMethod())
-            .startTimer();
+        SimpleTimer timer = new SimpleTimer();
 
         try {
             filterChain.doFilter(servletRequest, servletResponse);
         } finally {
-            timer.observeDuration();
+            double time = timer.elapsedSeconds();
+            Histogram.Child child = addLabelsValues(histogram, (HttpServletRequest) servletRequest, (HttpServletResponse) servletResponse);
+
+            child.observe(time);
         }
     }
 

simpleclient_servlet/src/main/java/io/prometheus/client/filter/StatusAwareMetricsFilter.java

@@ -0,0 +1,53 @@
+package io.prometheus.client.filter;
+
+import io.prometheus.client.Histogram;
+
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+/**
+ * The StatusAwareMetricsFilter class provides an histogram metrics with the following labels : path, method, and status.
+ *
+ * This filter behave similarly than MetricsFilter with an extra label that report the status.
+ *
+ * Do not use MetricsFilter and StatusAwareMetricsFilter with the same metric name. Due to their different labels doing
+ * so would raise cardinality issues.
+ *
+ * {@code
+ * <filter>
+ *   <filter-name>statusAwarePrometheusFilter</filter-name>
+ *   <filter-class>io.prometheus.client.filter.StatusAwareMetricsFilter</filter-class>
+ *   <init-param>
+ *      <param-name>metric-name</param-name>
+ *      <param-value>webapp_metrics_filter</param-value>
+ *   </init-param>
+ *    <init-param>
+ *      <param-name>help</param-name>
+ *      <param-value>The time taken fulfilling servlet requests</param-value>
+ *   </init-param>
+ *   <init-param>
+ *      <param-name>buckets</param-name>
+ *      <param-value>0.005,0.01,0.025,0.05,0.075,0.1,0.25,0.5,0.75,1,2.5,5,7.5,10</param-value>
+ *   </init-param>
+ *   <init-param>
+ *      <param-name>path-components</param-name>
+ *      <param-value>0</param-value>
+ *   </init-param>
+ * </filter>
+ * }
+ */
+public class StatusAwareMetricsFilter extends MetricsFilter {
+
+    @Override
+    Histogram.Builder addLabelsNames(Histogram.Builder builder) {
+        return builder.labelNames("path", "method", "status");
+    }
+
+    @Override
+    Histogram.Child addLabelsValues(Histogram builder, HttpServletRequest request, HttpServletResponse response) {
+        String path = request.getRequestURI();
+        String status = Integer.toString((response).getStatus());
+        return builder.labels(getComponents(path), request.getMethod(), status);
+    }
+
+}

simpleclient_servlet/src/test/java/io/prometheus/client/filter/MetricsFilterCommonTest.java

@@ -0,0 +1,36 @@
+package io.prometheus.client.filter;
+
+import io.prometheus.client.Collector;
+import io.prometheus.client.CollectorRegistry;
+import org.eclipse.jetty.http.HttpMethods;
+import org.junit.After;
+import org.junit.Test;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
+
+import javax.servlet.FilterChain;
+import javax.servlet.FilterConfig;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import java.util.Enumeration;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.anyString;
+import static org.mockito.Mockito.*;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+abstract public class MetricsFilterCommonTest {
+    MetricsFilter f =getMetricsFilter();
+
+    abstract public MetricsFilter getMetricsFilter();
+
+    @After
+    public void clear() {
+        CollectorRegistry.defaultRegistry.clear();
+    }
+
+
+}

simpleclient_servlet/src/test/java/io/prometheus/client/filter/MetricsFilterTest.java

@@ -13,6 +13,8 @@
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import java.util.Enumeration;
+import java.util.HashMap;
+import java.util.Map;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
@@ -23,8 +25,11 @@
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
-public class MetricsFilterTest {
-    MetricsFilter f = new MetricsFilter();
+public class MetricsFilterTest extends MetricsFilterCommonTest {
+    public MetricsFilter getMetricsFilter(){
+        return new MetricsFilter();
+    }
+
 
     @After
     public void clear() {
@@ -180,4 +185,6 @@ public Void answer(InvocationOnMock invocationOnMock) throws Throwable {
         assertEquals(buckets.split(",").length+1, count);
     }
 
+
+
 }

simpleclient_servlet/src/test/java/io/prometheus/client/filter/StatusAwareMetricsFilterTest.java

@@ -0,0 +1,75 @@
+package io.prometheus.client.filter;
+
+import io.prometheus.client.Collector;
+import io.prometheus.client.CollectorRegistry;
+import org.eclipse.jetty.http.HttpMethods;
+import org.junit.After;
+import org.junit.Test;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
+
+import javax.servlet.FilterChain;
+import javax.servlet.FilterConfig;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import java.util.Enumeration;
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.anyString;
+import static org.mockito.Mockito.*;
+
+public class StatusAwareMetricsFilterTest extends MetricsFilterCommonTest {
+    public MetricsFilter getMetricsFilter(){
+        return new StatusAwareMetricsFilter();
+    }
+
+    @Test
+    public void testStatusCode() throws Exception {
+        Map<String, Integer> sampleStatusCodes = new HashMap<String, Integer>();
+        sampleStatusCodes.put("/a/page/that/exists", HttpServletResponse.SC_OK);
+        sampleStatusCodes.put("/a/page/that/doesn-t-exist", HttpServletResponse.SC_NOT_FOUND);
+        sampleStatusCodes.put("/a/page/that/crashes", HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
+
+        FilterConfig cfg = mock(FilterConfig.class);
+        when(cfg.getInitParameter(anyString())).thenReturn(null);
+
+        String metricName = "foo";
+
+        when(cfg.getInitParameter(MetricsFilter.METRIC_NAME_PARAM)).thenReturn(metricName);
+        when(cfg.getInitParameter(MetricsFilter.PATH_COMPONENT_PARAM)).thenReturn("4");
+
+        f.init(cfg);
+
+        for (String uri : sampleStatusCodes.keySet()) {
+            HttpServletRequest req = mock(HttpServletRequest.class);
+
+            when(req.getRequestURI()).thenReturn(uri);
+            when(req.getMethod()).thenReturn(HttpMethods.GET);
+
+            HttpServletResponse res = mock(HttpServletResponse.class);
+            when(res.getStatus()).thenReturn(sampleStatusCodes.get(uri));
+
+            FilterChain c = mock(FilterChain.class);
+
+            f.doFilter(req, res, c);
+
+            verify(c).doFilter(req, res);
+        }
+
+        for (String uri : sampleStatusCodes.keySet()) {
+
+            final Double sampleValue = CollectorRegistry.defaultRegistry
+                    .getSampleValue(metricName + "_count",
+                            new String[]{"path", "method", "status"},
+                            new String[]{uri, HttpMethods.GET,
+                                    Integer.toString(sampleStatusCodes.get(uri))});
+            assertNotNull(sampleValue);
+            assertEquals(1, sampleValue, 0.0001);
+        }
+    }
+
+}