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 026a8f73e..e101624ae 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,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 * * prometheusFilter - * net.cccnext.ssp.portal.spring.filter.PrometheusMetricsFilter + * io.prometheus.client.filter.MetricsFilter * * metric-name * webapp_metrics_filter @@ -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); } } diff --git a/simpleclient_servlet/src/main/java/io/prometheus/client/filter/StatusAwareMetricsFilter.java b/simpleclient_servlet/src/main/java/io/prometheus/client/filter/StatusAwareMetricsFilter.java new file mode 100644 index 000000000..06e79be24 --- /dev/null +++ b/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 + * + * statusAwarePrometheusFilter + * io.prometheus.client.filter.StatusAwareMetricsFilter + * + * metric-name + * webapp_metrics_filter + * + * + * help + * The time taken fulfilling servlet requests + * + * + * buckets + * 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 + * + * + * path-components + * 0 + * + * + * } + */ +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); + } + +} diff --git a/simpleclient_servlet/src/test/java/io/prometheus/client/filter/MetricsFilterCommonTest.java b/simpleclient_servlet/src/test/java/io/prometheus/client/filter/MetricsFilterCommonTest.java new file mode 100644 index 000000000..497a6704e --- /dev/null +++ b/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(); + } + + +} 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 4c69f0b1f..3e5ac8ef1 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 @@ -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); } + + } diff --git a/simpleclient_servlet/src/test/java/io/prometheus/client/filter/StatusAwareMetricsFilterTest.java b/simpleclient_servlet/src/test/java/io/prometheus/client/filter/StatusAwareMetricsFilterTest.java new file mode 100644 index 000000000..556e52316 --- /dev/null +++ b/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 sampleStatusCodes = new HashMap(); + 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); + } + } + +}