From 53439215a52d0a7d1035b4ef85028af0f0971df0 Mon Sep 17 00:00:00 2001 From: Ken Dombeck Date: Tue, 14 Jul 2020 21:59:50 -0500 Subject: [PATCH 1/3] Count HTTP statuses returned along with the HTTP response times Signed-off-by: Ken Dombeck --- .../client/filter/MetricsFilter.java | 27 ++++++++-- .../client/filter/MetricsFilterTest.java | 51 +++++++++++++++++++ 2 files changed, 75 insertions(+), 3 deletions(-) diff --git a/simpleclient_servlet/src/main/java/io/prometheus/client/filter/MetricsFilter.java b/simpleclient_servlet/src/main/java/io/prometheus/client/filter/MetricsFilter.java index 742dcf5a1..0a8b36916 100644 --- a/simpleclient_servlet/src/main/java/io/prometheus/client/filter/MetricsFilter.java +++ b/simpleclient_servlet/src/main/java/io/prometheus/client/filter/MetricsFilter.java @@ -1,5 +1,6 @@ package io.prometheus.client.filter; +import io.prometheus.client.Counter; import io.prometheus.client.Histogram; import javax.servlet.Filter; @@ -9,13 +10,14 @@ import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; import java.io.IOException; /** * The MetricsFilter class exists to provide a high-level filter that enables tunable collection of metrics for Servlet * performance. * - *

The Histogram name itself is required, and configured with a {@code metric-name} init parameter. + *

The metric name itself is required, and configured with a {@code metric-name} init parameter. * *

The help parameter, configured with the {@code help} init parameter, is not required but strongly recommended. * @@ -26,6 +28,8 @@ *

The Histogram buckets can be configured with a {@code buckets} init parameter whose value is a comma-separated list * of valid {@code double} values. * + *

HTTP statuses will be aggregated via Counter. The name for this counter will be derived from the {@code metric-name} init parameter. + * * 

{@code
  * 
  *   prometheusFilter
@@ -34,7 +38,7 @@
  *      metric-name
  *      webapp_metrics_filter
  *   
- *    
+ *   
  *      help
  *      The time taken fulfilling servlet requests
  *   
@@ -56,8 +60,10 @@ public class MetricsFilter implements Filter {
     static final String HELP_PARAM = "help";
     static final String METRIC_NAME_PARAM = "metric-name";
     static final String BUCKET_CONFIG_PARAM = "buckets";
+    static final String UNKNOWN_HTTP_STATUS_CODE = "";
 
     private Histogram histogram = null;
+    private Counter statusCounter = null;
 
     // Package-level for testing purposes.
     int pathComponents = 1;
@@ -149,6 +155,10 @@ public void init(FilterConfig filterConfig) throws ServletException {
                 .help(help)
                 .name(metricName)
                 .register();
+
+        statusCounter = Counter.build(metricName + "_status", "HTTP status codes of " + help)
+                .labelNames("path", "method", "status")
+                .register();
     }
 
     @Override
@@ -162,17 +172,28 @@ public void doFilter(ServletRequest servletRequest, ServletResponse servletRespo
 
         String path = request.getRequestURI();
 
+        String components = getComponents(path);
+        String method = request.getMethod();
         Histogram.Timer timer = histogram
-            .labels(getComponents(path), request.getMethod())
+            .labels(components, method)
             .startTimer();
 
         try {
             filterChain.doFilter(servletRequest, servletResponse);
         } finally {
             timer.observeDuration();
+            statusCounter.labels(components, method, getStatusCode(servletResponse)).inc();
         }
     }
 
+    private String getStatusCode(ServletResponse servletResponse) {
+        if (!(servletResponse instanceof HttpServletResponse)) {
+            return UNKNOWN_HTTP_STATUS_CODE;
+        }
+
+        return Integer.toString(((HttpServletResponse) servletResponse).getStatus());
+    }
+
     @Override
     public void destroy() {
     }
diff --git a/simpleclient_servlet/src/test/java/io/prometheus/client/filter/MetricsFilterTest.java b/simpleclient_servlet/src/test/java/io/prometheus/client/filter/MetricsFilterTest.java
index 627b2e88c..d04d3e905 100644
--- a/simpleclient_servlet/src/test/java/io/prometheus/client/filter/MetricsFilterTest.java
+++ b/simpleclient_servlet/src/test/java/io/prometheus/client/filter/MetricsFilterTest.java
@@ -10,6 +10,7 @@
 
 import javax.servlet.FilterChain;
 import javax.servlet.FilterConfig;
+import javax.servlet.ServletResponse;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import java.util.Enumeration;
@@ -180,4 +181,54 @@ public Void answer(InvocationOnMock invocationOnMock) throws Throwable {
         assertEquals(buckets.split(",").length+1, count);
     }
 
+    @Test
+    public void testStatusCode() throws Exception {
+        HttpServletRequest req = mock(HttpServletRequest.class);
+        when(req.getRequestURI()).thenReturn("/foo/bar/baz/bang");
+        when(req.getMethod()).thenReturn(HttpMethods.GET);
+
+        HttpServletResponse res = mock(HttpServletResponse.class);
+        when(res.getStatus()).thenReturn(200);
+
+        FilterChain c = mock(FilterChain.class);
+
+        MetricsFilter constructed = new MetricsFilter(
+                "foobar_filter",
+                "Help for my filter",
+                2,
+                null
+        );
+        constructed.init(mock(FilterConfig.class));
+
+        constructed.doFilter(req, res, c);
+
+        final Double sampleValue = CollectorRegistry.defaultRegistry.getSampleValue("foobar_filter_status", new String[]{"path", "method", "status"}, new String[]{"/foo/bar", HttpMethods.GET, "200"});
+        assertNotNull(sampleValue);
+        assertEquals(1, sampleValue, 0.0001);
+    }
+
+    @Test
+    public void testStatusCodeWithNonHttpServletResponse() throws Exception {
+        HttpServletRequest req = mock(HttpServletRequest.class);
+        when(req.getRequestURI()).thenReturn("/foo/bar/baz/bang");
+        when(req.getMethod()).thenReturn(HttpMethods.GET);
+
+        ServletResponse res = mock(ServletResponse.class);
+
+        FilterChain c = mock(FilterChain.class);
+
+        MetricsFilter constructed = new MetricsFilter(
+                "foobar_filter",
+                "Help for my filter",
+                2,
+                null
+        );
+        constructed.init(mock(FilterConfig.class));
+
+        constructed.doFilter(req, res, c);
+
+        final Double sampleValue = CollectorRegistry.defaultRegistry.getSampleValue("foobar_filter_status", new String[]{"path", "method", "status"}, new String[]{"/foo/bar", HttpMethods.GET, MetricsFilter.UNKNOWN_HTTP_STATUS_CODE});
+        assertNotNull(sampleValue);
+        assertEquals(1, sampleValue, 0.0001);
+    }
 }

From 704348980b360fdafd1ecabf5efb03d9a4daf18b Mon Sep 17 00:00:00 2001
From: Ken Dombeck 
Date: Wed, 15 Jul 2020 16:41:20 -0500
Subject: [PATCH 2/3] Change count metric to end in _total to match standards

Signed-off-by: Ken Dombeck 
---
 .../main/java/io/prometheus/client/filter/MetricsFilter.java  | 2 +-
 .../java/io/prometheus/client/filter/MetricsFilterTest.java   | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/simpleclient_servlet/src/main/java/io/prometheus/client/filter/MetricsFilter.java b/simpleclient_servlet/src/main/java/io/prometheus/client/filter/MetricsFilter.java
index 0a8b36916..8ec221bb9 100644
--- a/simpleclient_servlet/src/main/java/io/prometheus/client/filter/MetricsFilter.java
+++ b/simpleclient_servlet/src/main/java/io/prometheus/client/filter/MetricsFilter.java
@@ -156,7 +156,7 @@ public void init(FilterConfig filterConfig) throws ServletException {
                 .name(metricName)
                 .register();
 
-        statusCounter = Counter.build(metricName + "_status", "HTTP status codes of " + help)
+        statusCounter = Counter.build(metricName + "_total", "HTTP status codes of " + help)
                 .labelNames("path", "method", "status")
                 .register();
     }
diff --git a/simpleclient_servlet/src/test/java/io/prometheus/client/filter/MetricsFilterTest.java b/simpleclient_servlet/src/test/java/io/prometheus/client/filter/MetricsFilterTest.java
index d04d3e905..a42634891 100644
--- a/simpleclient_servlet/src/test/java/io/prometheus/client/filter/MetricsFilterTest.java
+++ b/simpleclient_servlet/src/test/java/io/prometheus/client/filter/MetricsFilterTest.java
@@ -202,7 +202,7 @@ public void testStatusCode() throws Exception {
 
         constructed.doFilter(req, res, c);
 
-        final Double sampleValue = CollectorRegistry.defaultRegistry.getSampleValue("foobar_filter_status", new String[]{"path", "method", "status"}, new String[]{"/foo/bar", HttpMethods.GET, "200"});
+        final Double sampleValue = CollectorRegistry.defaultRegistry.getSampleValue("foobar_filter_total", new String[]{"path", "method", "status"}, new String[]{"/foo/bar", HttpMethods.GET, "200"});
         assertNotNull(sampleValue);
         assertEquals(1, sampleValue, 0.0001);
     }
@@ -227,7 +227,7 @@ public void testStatusCodeWithNonHttpServletResponse() throws Exception {
 
         constructed.doFilter(req, res, c);
 
-        final Double sampleValue = CollectorRegistry.defaultRegistry.getSampleValue("foobar_filter_status", new String[]{"path", "method", "status"}, new String[]{"/foo/bar", HttpMethods.GET, MetricsFilter.UNKNOWN_HTTP_STATUS_CODE});
+        final Double sampleValue = CollectorRegistry.defaultRegistry.getSampleValue("foobar_filter_total", new String[]{"path", "method", "status"}, new String[]{"/foo/bar", HttpMethods.GET, MetricsFilter.UNKNOWN_HTTP_STATUS_CODE});
         assertNotNull(sampleValue);
         assertEquals(1, sampleValue, 0.0001);
     }

From cdef5ee7ad6eab76739763b8db7854cf897554a7 Mon Sep 17 00:00:00 2001
From: Ken Dombeck 
Date: Wed, 15 Jul 2020 17:29:29 -0500
Subject: [PATCH 3/3] Add status prefix to prevent collision with histogram

Signed-off-by: Ken Dombeck 
---
 .../main/java/io/prometheus/client/filter/MetricsFilter.java  | 2 +-
 .../java/io/prometheus/client/filter/MetricsFilterTest.java   | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/simpleclient_servlet/src/main/java/io/prometheus/client/filter/MetricsFilter.java b/simpleclient_servlet/src/main/java/io/prometheus/client/filter/MetricsFilter.java
index 8ec221bb9..3b61a0ed3 100644
--- a/simpleclient_servlet/src/main/java/io/prometheus/client/filter/MetricsFilter.java
+++ b/simpleclient_servlet/src/main/java/io/prometheus/client/filter/MetricsFilter.java
@@ -156,7 +156,7 @@ public void init(FilterConfig filterConfig) throws ServletException {
                 .name(metricName)
                 .register();
 
-        statusCounter = Counter.build(metricName + "_total", "HTTP status codes of " + help)
+        statusCounter = Counter.build(metricName + "_status_total", "HTTP status codes of " + help)
                 .labelNames("path", "method", "status")
                 .register();
     }
diff --git a/simpleclient_servlet/src/test/java/io/prometheus/client/filter/MetricsFilterTest.java b/simpleclient_servlet/src/test/java/io/prometheus/client/filter/MetricsFilterTest.java
index a42634891..622756b6b 100644
--- a/simpleclient_servlet/src/test/java/io/prometheus/client/filter/MetricsFilterTest.java
+++ b/simpleclient_servlet/src/test/java/io/prometheus/client/filter/MetricsFilterTest.java
@@ -202,7 +202,7 @@ public void testStatusCode() throws Exception {
 
         constructed.doFilter(req, res, c);
 
-        final Double sampleValue = CollectorRegistry.defaultRegistry.getSampleValue("foobar_filter_total", new String[]{"path", "method", "status"}, new String[]{"/foo/bar", HttpMethods.GET, "200"});
+        final Double sampleValue = CollectorRegistry.defaultRegistry.getSampleValue("foobar_filter_status_total", new String[]{"path", "method", "status"}, new String[]{"/foo/bar", HttpMethods.GET, "200"});
         assertNotNull(sampleValue);
         assertEquals(1, sampleValue, 0.0001);
     }
@@ -227,7 +227,7 @@ public void testStatusCodeWithNonHttpServletResponse() throws Exception {
 
         constructed.doFilter(req, res, c);
 
-        final Double sampleValue = CollectorRegistry.defaultRegistry.getSampleValue("foobar_filter_total", new String[]{"path", "method", "status"}, new String[]{"/foo/bar", HttpMethods.GET, MetricsFilter.UNKNOWN_HTTP_STATUS_CODE});
+        final Double sampleValue = CollectorRegistry.defaultRegistry.getSampleValue("foobar_filter_status_total", new String[]{"path", "method", "status"}, new String[]{"/foo/bar", HttpMethods.GET, MetricsFilter.UNKNOWN_HTTP_STATUS_CODE});
         assertNotNull(sampleValue);
         assertEquals(1, sampleValue, 0.0001);
     }