diff options
author | Bjørn Christian Seime <bjorncs@verizonmedia.com> | 2020-11-17 14:14:20 +0100 |
---|---|---|
committer | Bjørn Christian Seime <bjorncs@verizonmedia.com> | 2020-11-17 14:16:47 +0100 |
commit | 5e7454179d37b2d639fd3374bb9d916e15f6885c (patch) | |
tree | f7a77f12729534f8e667d9709d96a4200d188c97 | |
parent | eeb4a602f9b50cf7e58a547bf48c06502d46b5b9 (diff) |
Add metrics for request filtering
5 files changed, 75 insertions, 26 deletions
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/VespaMetricSet.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/VespaMetricSet.java index 5c3fe59b3fd..21f323fc0f3 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/VespaMetricSet.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/monitoring/VespaMetricSet.java @@ -215,6 +215,11 @@ public class VespaMetricSet { addMetric(metrics, "jdisc.http.jetty.threadpool.thread.total", List.of("sum", "count", "min", "max")); addMetric(metrics, "jdisc.http.jetty.threadpool.queue.size", List.of("sum", "count", "min", "max")); + addMetric(metrics, "jdisc.http.filtering.request.handled", List.of("rate")); + addMetric(metrics, "jdisc.http.filtering.request.unhandled", List.of("rate")); + addMetric(metrics, "jdisc.http.filtering.response.handled", List.of("rate")); + addMetric(metrics, "jdisc.http.filtering.response.unhandled", List.of("rate")); + return metrics; } diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/FilterResolver.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/FilterResolver.java index badb0e736ae..6590b9791d4 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/FilterResolver.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/FilterResolver.java @@ -1,11 +1,13 @@ // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.jdisc.http.server.jetty; +import com.yahoo.jdisc.Metric; import com.yahoo.jdisc.http.filter.RequestFilter; import com.yahoo.jdisc.http.filter.ResponseFilter; import javax.servlet.http.HttpServletRequest; import java.net.URI; +import java.util.Map; import java.util.Optional; import static com.yahoo.jdisc.http.server.jetty.JDiscHttpServlet.getConnector; @@ -18,18 +20,37 @@ import static com.yahoo.jdisc.http.server.jetty.JDiscHttpServlet.getConnector; class FilterResolver { private final FilterBindings bindings; + private final Metric metric; - FilterResolver(FilterBindings bindings) { + FilterResolver(FilterBindings bindings, Metric metric) { this.bindings = bindings; + this.metric = metric; } Optional<RequestFilter> resolveRequestFilter(HttpServletRequest servletRequest, URI jdiscUri) { - return bindings.resolveRequestFilter(jdiscUri, getConnector(servletRequest).listenPort()) - .map(bindings::getRequestFilter); + Optional<String> maybeFilterId = bindings.resolveRequestFilter(jdiscUri, getConnector(servletRequest).listenPort()); + if (maybeFilterId.isPresent()) { + metric.add(MetricDefinitions.FILTERING_REQUEST_HANDLED, 1L, createMetricContext(servletRequest, maybeFilterId.get())); + } else { + metric.add(MetricDefinitions.FILTERING_REQUEST_UNHANDLED, 1L, createMetricContext(servletRequest, null)); + } + return maybeFilterId.map(bindings::getRequestFilter); } Optional<ResponseFilter> resolveResponseFilter(HttpServletRequest servletRequest, URI jdiscUri) { - return bindings.resolveResponseFilter(jdiscUri, getConnector(servletRequest).listenPort()) - .map(bindings::getResponseFilter); + Optional<String> maybeFilterId = bindings.resolveResponseFilter(jdiscUri, getConnector(servletRequest).listenPort()); + if (maybeFilterId.isPresent()) { + metric.add(MetricDefinitions.FILTERING_RESPONSE_HANDLED, 1L, createMetricContext(servletRequest, maybeFilterId.get())); + } else { + metric.add(MetricDefinitions.FILTERING_RESPONSE_UNHANDLED, 1L, createMetricContext(servletRequest, null)); + } + return maybeFilterId.map(bindings::getResponseFilter); + } + + private Metric.Context createMetricContext(HttpServletRequest request, String filterId) { + Map<String, String> extraDimensions = filterId != null + ? Map.of(MetricDefinitions.FILTER_CHAIN_ID_DIMENSION, filterId) + : Map.of(); + return JDiscHttpServlet.getConnector(request).createRequestMetricContext(request, extraDimensions); } } diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscContext.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscContext.java index a308149beb5..66471587bd5 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscContext.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscContext.java @@ -20,7 +20,7 @@ public class JDiscContext { Metric metric, ServerConfig serverConfig) { - this.filterResolver = new FilterResolver(filterBindings); + this.filterResolver = new FilterResolver(filterBindings, metric); this.container = container; this.janitor = janitor; this.metric = metric; diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/MetricDefinitions.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/MetricDefinitions.java index 1821f17bc49..5e953179b53 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/MetricDefinitions.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/MetricDefinitions.java @@ -15,6 +15,7 @@ class MetricDefinitions { static final String CLIENT_IP_DIMENSION = "clientIp"; static final String CLIENT_AUTHENTICATED_DIMENSION = "clientAuthenticated"; static final String REQUEST_SERVER_NAME_DIMENSION = "requestServerName"; + static final String FILTER_CHAIN_ID_DIMENSION = "chainId"; static final String NUM_OPEN_CONNECTIONS = "serverNumOpenConnections"; static final String NUM_CONNECTIONS_OPEN_MAX = "serverConnectionsOpenMax"; @@ -69,5 +70,10 @@ class MetricDefinitions { static final String JETTY_THREADPOOL_TOTAL_THREADS = "jdisc.http.jetty.threadpool.thread.total"; static final String JETTY_THREADPOOL_QUEUE_SIZE = "jdisc.http.jetty.threadpool.queue.size"; + static final String FILTERING_REQUEST_HANDLED = "jdisc.http.filtering.request.handled"; + static final String FILTERING_REQUEST_UNHANDLED = "jdisc.http.filtering.request.unhandled"; + static final String FILTERING_RESPONSE_HANDLED = "jdisc.http.filtering.response.handled"; + static final String FILTERING_RESPONSE_UNHANDLED = "jdisc.http.filtering.response.unhandled"; + private MetricDefinitions() {} } diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/FilterTestCase.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/FilterTestCase.java index b2e28c6af67..fd929b3e037 100644 --- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/FilterTestCase.java +++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/FilterTestCase.java @@ -407,10 +407,7 @@ public class FilterTestCase { .setRequestFilterDefaultForPort(defaultFilterId, 0) .build(); MyRequestHandler requestHandler = new MyRequestHandler(); - TestDriver testDriver = TestDriver.newInstance( - JettyHttpServer.class, - requestHandler, - newFilterModule(filterBindings)); + TestDriver testDriver = newDriver(requestHandler, filterBindings); testDriver.client().get("/status.html"); @@ -433,10 +430,7 @@ public class FilterTestCase { .setResponseFilterDefaultForPort(defaultFilterId, 0) .build(); MyRequestHandler requestHandler = new MyRequestHandler(); - TestDriver testDriver = TestDriver.newInstance( - JettyHttpServer.class, - requestHandler, - newFilterModule(filterBindings)); + TestDriver testDriver = newDriver(requestHandler, filterBindings); testDriver.client().get("/status.html"); @@ -459,10 +453,7 @@ public class FilterTestCase { .setRequestFilterDefaultForPort(defaultFilterId, 0) .build(); MyRequestHandler requestHandler = new MyRequestHandler(); - TestDriver testDriver = TestDriver.newInstance( - JettyHttpServer.class, - requestHandler, - newFilterModule(filterBindings)); + TestDriver testDriver = newDriver(requestHandler, filterBindings); testDriver.client().get("/filtered/status.html"); @@ -474,7 +465,7 @@ public class FilterTestCase { } @Test - public void requireThatResponsFilterWithBindingMatchHasPrecedenceOverDefaultFilter() throws IOException, InterruptedException { + public void requireThatResponseFilterWithBindingMatchHasPrecedenceOverDefaultFilter() throws IOException, InterruptedException { ResponseFilter filterWithBinding = mock(ResponseFilter.class); ResponseFilter defaultFilter = mock(ResponseFilter.class); String defaultFilterId = "default-response-filter"; @@ -485,10 +476,7 @@ public class FilterTestCase { .setResponseFilterDefaultForPort(defaultFilterId, 0) .build(); MyRequestHandler requestHandler = new MyRequestHandler(); - TestDriver testDriver = TestDriver.newInstance( - JettyHttpServer.class, - requestHandler, - newFilterModule(filterBindings)); + TestDriver testDriver = newDriver(requestHandler, filterBindings); testDriver.client().get("/filtered/status.html"); @@ -499,25 +487,54 @@ public class FilterTestCase { assertThat(testDriver.close(), is(true)); } + @Test + public void requireThatMetricAreReported() throws IOException, InterruptedException { + FilterBindings filterBindings = new FilterBindings.Builder() + .addRequestFilter("my-request-filter", mock(RequestFilter.class)) + .addRequestFilterBinding("my-request-filter", "http://*/*") + .build(); + MetricConsumerMock metricConsumerMock = new MetricConsumerMock(); + MyRequestHandler requestHandler = new MyRequestHandler(); + TestDriver testDriver = newDriver(requestHandler, filterBindings, metricConsumerMock); + + testDriver.client().get("/status.html"); + assertThat(requestHandler.awaitInvocation(), is(true)); + verify(metricConsumerMock.mockitoMock()) + .add(MetricDefinitions.FILTERING_REQUEST_HANDLED, 1L, MetricConsumerMock.STATIC_CONTEXT); + verify(metricConsumerMock.mockitoMock(), never()) + .add(MetricDefinitions.FILTERING_REQUEST_UNHANDLED, 1L, MetricConsumerMock.STATIC_CONTEXT); + verify(metricConsumerMock.mockitoMock(), never()) + .add(MetricDefinitions.FILTERING_RESPONSE_HANDLED, 1L, MetricConsumerMock.STATIC_CONTEXT); + verify(metricConsumerMock.mockitoMock()) + .add(MetricDefinitions.FILTERING_RESPONSE_UNHANDLED, 1L, MetricConsumerMock.STATIC_CONTEXT); + assertThat(testDriver.close(), is(true)); + } + private static TestDriver newDriver(MyRequestHandler requestHandler, FilterBindings filterBindings) { + return newDriver(requestHandler, filterBindings, new MetricConsumerMock()); + } + + private static TestDriver newDriver(MyRequestHandler requestHandler, FilterBindings filterBindings, MetricConsumerMock metricConsumer) { return TestDriver.newInstance( JettyHttpServer.class, requestHandler, - newFilterModule(filterBindings)); + newFilterModule(filterBindings, metricConsumer)); } - private static com.google.inject.Module newFilterModule(FilterBindings filterBindings) { + private static com.google.inject.Module newFilterModule(FilterBindings filterBindings, MetricConsumerMock metricConsumer) { return Modules.combine( new AbstractModule() { @Override protected void configure() { + bind(FilterBindings.class).toInstance(filterBindings); bind(ServerConfig.class).toInstance(new ServerConfig(new ServerConfig.Builder())); bind(ConnectorConfig.class).toInstance(new ConnectorConfig(new ConnectorConfig.Builder())); bind(ServletPathsConfig.class).toInstance(new ServletPathsConfig(new ServletPathsConfig.Builder())); } }, - new ConnectorFactoryRegistryModule()); + new ConnectorFactoryRegistryModule(), + metricConsumer.asGuiceModule()); } private static abstract class RequestFilterMockBase extends AbstractResource implements RequestFilter {} |