diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2023-01-23 16:13:01 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-01-23 16:13:01 +0100 |
commit | 2ac26e289ee124ae909efd820bb37e67796ed948 (patch) | |
tree | f6a7f9b61cb6ece57d7becc3eef35c8c3afc28d7 /container-core | |
parent | a1e2776f0c23553b1ade02b10d3ee8b11020f13e (diff) | |
parent | 8847d9b4eef2cd159febd04cad3ebc31cc005da6 (diff) |
Merge pull request #25684 from vespa-engine/bjorncs/jdisc-response-metrics
Implement response metric aggregator as a HttpChannel.Listener
Diffstat (limited to 'container-core')
-rw-r--r-- | container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java | 13 | ||||
-rw-r--r-- | container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ResponseMetricAggregator.java (renamed from container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollector.java) | 132 | ||||
-rw-r--r-- | container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ServerMetricReporter.java | 11 | ||||
-rw-r--r-- | container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletResponseController.java | 2 | ||||
-rw-r--r-- | container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java | 33 | ||||
-rw-r--r-- | container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/ResponseMetricAggregatorTest.java (renamed from container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollectorTest.java) | 117 |
6 files changed, 92 insertions, 216 deletions
diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java index 46fbc158f0a..3ebb65e7979 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java @@ -79,6 +79,7 @@ public class JettyHttpServer extends AbstractServerProvider { server.addConnector(connectorFactory.createConnector(metric, server, connectionLogger, connectionMetricAggregator)); listenedPorts.add(connectorConfig.listenPort()); } + server.addBeanToAllConnectors(new ResponseMetricAggregator(serverConfig.metric())); JDiscContext jDiscContext = new JDiscContext(filterBindings, container, janitor, metric, serverConfig); @@ -86,7 +87,7 @@ public class JettyHttpServer extends AbstractServerProvider { List<JDiscServerConnector> connectors = Arrays.stream(server.getConnectors()) .map(JDiscServerConnector.class::cast) .toList(); - server.setHandler(createRootHandler(serverConfig, connectors, jdiscServlet)); + server.setHandler(createRootHandler(connectors, jdiscServlet)); this.metricsReporter = new ServerMetricReporter(metric, server); } @@ -117,8 +118,7 @@ public class JettyHttpServer extends AbstractServerProvider { } } - private Handler createRootHandler( - ServerConfig serverCfg, List<JDiscServerConnector> connectors, ServletHolder jdiscServlet) { + private Handler createRootHandler(List<JDiscServerConnector> connectors, ServletHolder jdiscServlet) { HandlerCollection perConnectorHandlers = new ContextHandlerCollection(); for (JDiscServerConnector connector : connectors) { ConnectorConfig connectorCfg = connector.connectorConfig(); @@ -136,8 +136,7 @@ public class JettyHttpServer extends AbstractServerProvider { perConnectorHandlers.addHandler(connectorRoot); } StatisticsHandler root = newGenericStatisticsHandler(); - addChainToRoot(root, List.of( - newResponseStatisticsHandler(serverCfg), newGzipHandler(), perConnectorHandlers)); + addChainToRoot(root, List.of(newGzipHandler(), perConnectorHandlers)); return root; } @@ -228,10 +227,6 @@ public class JettyHttpServer extends AbstractServerProvider { return new TlsClientAuthenticationEnforcer(cfg.tlsClientAuthEnforcer()); } - private static HttpResponseStatisticsCollector newResponseStatisticsHandler(ServerConfig cfg) { - return new HttpResponseStatisticsCollector(cfg.metric()); - } - private static StatisticsHandler newGenericStatisticsHandler() { StatisticsHandler statisticsHandler = new StatisticsHandler(); statisticsHandler.statsReset(); 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/ResponseMetricAggregator.java index 81789881b68..5569919c39a 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/ResponseMetricAggregator.java @@ -4,135 +4,65 @@ package com.yahoo.jdisc.http.server.jetty; import com.yahoo.jdisc.Metric; import com.yahoo.jdisc.http.HttpRequest; import com.yahoo.jdisc.http.ServerConfig; -import jakarta.servlet.AsyncEvent; -import jakarta.servlet.AsyncListener; -import jakarta.servlet.ServletException; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; import org.eclipse.jetty.http.HttpHeader; -import org.eclipse.jetty.http.HttpStatus; -import org.eclipse.jetty.server.AsyncContextEvent; -import org.eclipse.jetty.server.Handler; -import org.eclipse.jetty.server.HttpChannelState; +import org.eclipse.jetty.server.HttpChannel; import org.eclipse.jetty.server.Request; -import org.eclipse.jetty.server.handler.HandlerWrapper; -import org.eclipse.jetty.util.component.Graceful; +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.util.component.AbstractLifeCycle; -import java.io.IOException; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; -import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; -import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.LongAdder; import java.util.function.ObjLongConsumer; import java.util.stream.Collectors; /** - * HttpResponseStatisticsCollector collects statistics about HTTP response types aggregated by category - * (1xx, 2xx, etc). It is similar to {@link org.eclipse.jetty.server.handler.StatisticsHandler} - * with the distinction that this class collects response type statistics grouped - * by HTTP method and only collects the numbers that are reported as metrics from Vespa. + * Collects statistics about HTTP response types aggregated by category. * * @author ollivir + * @author bjorncs */ -class HttpResponseStatisticsCollector extends HandlerWrapper implements Graceful { +class ResponseMetricAggregator extends AbstractLifeCycle implements HttpChannel.Listener { static final String requestTypeAttribute = "requestType"; - private final Shutdown shutdown; private final List<String> monitoringHandlerPaths; private final List<String> searchHandlerPaths; private final Set<String> ignoredUserAgents; - private final AtomicLong inFlight = new AtomicLong(); private final ConcurrentMap<StatusCodeMetric, LongAdder> statistics = new ConcurrentHashMap<>(); - HttpResponseStatisticsCollector(ServerConfig.Metric cfg) { + ResponseMetricAggregator(ServerConfig.Metric cfg) { this(cfg.monitoringHandlerPaths(), cfg.searchHandlerPaths(), cfg.ignoredUserAgents()); } - HttpResponseStatisticsCollector(List<String> monitoringHandlerPaths, List<String> searchHandlerPaths, - Collection<String> ignoredUserAgents) { + ResponseMetricAggregator(List<String> monitoringHandlerPaths, List<String> searchHandlerPaths, + Collection<String> ignoredUserAgents) { this.monitoringHandlerPaths = monitoringHandlerPaths; this.searchHandlerPaths = searchHandlerPaths; this.ignoredUserAgents = Set.copyOf(ignoredUserAgents); - this.shutdown = new Shutdown(this) { - @Override public boolean isShutdownDone() { return inFlight.get() == 0; } - }; } - private final AsyncListener completionWatcher = new AsyncListener() { - - @Override - public void onTimeout(AsyncEvent event) { } - - @Override - public void onStartAsync(AsyncEvent event) { - event.getAsyncContext().addListener(this); - } - - @Override - public void onError(AsyncEvent event) { } - - @Override - public void onComplete(AsyncEvent event) throws IOException { - HttpChannelState state = ((AsyncContextEvent) event).getHttpChannelState(); - Request request = state.getBaseRequest(); - - observeEndOfRequest(request, null); - } - }; - - @Override - public void handle(String path, Request baseRequest, HttpServletRequest request, HttpServletResponse response) - throws IOException, ServletException { - inFlight.incrementAndGet(); - - try { - Handler handler = getHandler(); - if (handler != null && !shutdown.isShutdown() && isStarted()) { - handler.handle(path, baseRequest, request, response); - } else if ( ! baseRequest.isHandled()) { - baseRequest.setHandled(true); - response.sendError(HttpStatus.SERVICE_UNAVAILABLE_503); - } - } finally { - HttpChannelState state = baseRequest.getHttpChannelState(); - if (state.isSuspended()) { - if (state.isInitial()) { - state.addListener(completionWatcher); - } - } else if (state.isInitial()) { - observeEndOfRequest(baseRequest, response); - } - } - } - - private boolean shouldLogMetricsFor(Request request) { - String agent = request.getHeader(HttpHeader.USER_AGENT.toString()); - if (agent == null) return true; - return ! ignoredUserAgents.contains(agent); + static ResponseMetricAggregator getBean(JettyHttpServer server) { return getBean(server.server()); } + static ResponseMetricAggregator getBean(Server server) { + return Arrays.stream(server.getConnectors()) + .map(c -> c.getBean(ResponseMetricAggregator.class)).filter(Objects::nonNull).findAny().orElseThrow(); } - private void observeEndOfRequest(Request request, HttpServletResponse flushableResponse) throws IOException { + @Override + public void onResponseCommit(Request request) { if (shouldLogMetricsFor(request)) { var metrics = StatusCodeMetric.of(request, monitoringHandlerPaths, searchHandlerPaths); - metrics.forEach(metric -> - statistics.computeIfAbsent(metric, __ -> new LongAdder()) - .increment()); - } - long live = inFlight.decrementAndGet(); - if (shutdown.isShutdown()) { - if (flushableResponse != null) flushableResponse.flushBuffer(); - if (live == 0) shutdown.check(); + metrics.forEach(metric -> statistics.computeIfAbsent(metric, __ -> new LongAdder()).increment()); } } @@ -149,6 +79,12 @@ class HttpResponseStatisticsCollector extends HandlerWrapper implements Graceful }); } + private boolean shouldLogMetricsFor(Request request) { + String agent = request.getHeader(HttpHeader.USER_AGENT.toString()); + if (agent == null) return true; + return ! ignoredUserAgents.contains(agent); + } + private void consume(ObjLongConsumer<StatusCodeMetric> consumer) { statistics.forEach((metric, adder) -> { long value = adder.sumThenReset(); @@ -156,21 +92,8 @@ class HttpResponseStatisticsCollector extends HandlerWrapper implements Graceful }); } - @Override - protected void doStart() throws Exception { - shutdown.cancel(); - super.doStart(); - } - - @Override - protected void doStop() throws Exception { - shutdown.cancel(); - super.doStop(); - } - - @Override public CompletableFuture<Void> shutdown() { return shutdown.shutdown(); } - @Override public boolean isShutdown() { return shutdown.isShutdown(); } - + // Note: Request.getResponse().getStatus() may return invalid response code + private static int statusCode(Request r) { return r.getResponse().getCommittedMetaData().getStatus(); } static class Dimensions { final String protocol; @@ -242,8 +165,6 @@ 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); @@ -295,9 +216,8 @@ class HttpResponseStatisticsCollector extends HandlerWrapper implements Graceful .collect(Collectors.toSet()); } - @SuppressWarnings("removal") private static Collection<String> metricNames(Request req) { - int code = req.getResponse().getStatus(); + int code = statusCode(req); if (code < 200) return Set.of(MetricDefinitions.RESPONSES_1XX); else if (code < 300) return Set.of(MetricDefinitions.RESPONSES_2XX); else if (code < 400) return Set.of(MetricDefinitions.RESPONSES_3XX); diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ServerMetricReporter.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ServerMetricReporter.java index 9340dda2652..d2f9de5ae28 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ServerMetricReporter.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ServerMetricReporter.java @@ -51,11 +51,8 @@ class ServerMetricReporter { @Override public void run() { - HttpResponseStatisticsCollector statisticsCollector = ((AbstractHandlerContainer) jetty.getHandler()) - .getChildHandlerByClass(HttpResponseStatisticsCollector.class); - if (statisticsCollector != null) { - setServerMetrics(statisticsCollector); - } + var collector = ResponseMetricAggregator.getBean(jetty); + if (collector != null) setServerMetrics(collector); // reset statisticsHandler to preserve earlier behavior StatisticsHandler statisticsHandler = ((AbstractHandlerContainer) jetty.getHandler()) @@ -71,14 +68,14 @@ class ServerMetricReporter { setJettyThreadpoolMetrics(); } - private void setServerMetrics(HttpResponseStatisticsCollector statisticsCollector) { + private void setServerMetrics(ResponseMetricAggregator statisticsCollector) { long timeSinceStarted = System.currentTimeMillis() - timeStarted.toEpochMilli(); metric.set(MetricDefinitions.STARTED_MILLIS, timeSinceStarted, null); addResponseMetrics(statisticsCollector); } - private void addResponseMetrics(HttpResponseStatisticsCollector statisticsCollector) { + private void addResponseMetrics(ResponseMetricAggregator statisticsCollector) { statisticsCollector.reportSnapshot(metric); } diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletResponseController.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletResponseController.java index 6afb55f5b13..c423ceab2b9 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletResponseController.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletResponseController.java @@ -175,7 +175,7 @@ class ServletResponseController { handlerResponse = response; state = State.ACCEPTED_RESPONSE_FROM_HANDLER; servletRequest.setAttribute( - HttpResponseStatisticsCollector.requestTypeAttribute, handlerResponse.getRequestType()); + ResponseMetricAggregator.requestTypeAttribute, handlerResponse.getRequestType()); return; case COMMITTED_RESPONSE_FROM_HANDLER: case COMPLETED_WITH_RESPONSE_FROM_HANDLER: diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java index 39b6dcdc6d5..8639680e335 100644 --- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java +++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java @@ -11,6 +11,7 @@ import com.yahoo.jdisc.References; import com.yahoo.jdisc.Request; import com.yahoo.jdisc.Response; import com.yahoo.jdisc.application.BindingSetSelector; +import com.yahoo.jdisc.application.MetricConsumer; import com.yahoo.jdisc.handler.AbstractRequestHandler; import com.yahoo.jdisc.handler.CompletionHandler; import com.yahoo.jdisc.handler.ContentChannel; @@ -38,7 +39,6 @@ import org.apache.hc.client5.http.impl.async.CloseableHttpAsyncClient; import org.apache.hc.core5.http.ConnectionClosedException; import org.apache.hc.core5.http.ContentType; import org.assertj.core.api.Assertions; -import org.eclipse.jetty.server.handler.AbstractHandlerContainer; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -65,6 +65,7 @@ import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import java.util.regex.Pattern; +import static com.yahoo.jdisc.Response.Status.BAD_REQUEST; import static com.yahoo.jdisc.Response.Status.GATEWAY_TIMEOUT; import static com.yahoo.jdisc.Response.Status.INTERNAL_SERVER_ERROR; import static com.yahoo.jdisc.Response.Status.NOT_FOUND; @@ -161,6 +162,26 @@ public class HttpServerTest { } @Test + void requireThatMultipleHostHeadersReturns400() throws Exception { + var metricConsumer = new MetricConsumerMock(); + JettyTestDriver driver = JettyTestDriver.newConfiguredInstance( + mockRequestHandler(), + new ServerConfig.Builder(), + new ConnectorConfig.Builder(), + binder -> binder.bind(MetricConsumer.class).toInstance(metricConsumer.mockitoMock())); + driver.client() + .newGet("/status.html").addHeader("Host", "localhost").addHeader("Host", "vespa.ai").execute() + .expectStatusCode(is(BAD_REQUEST)).expectContent(containsString("Bad Host: multiple headers")); + assertTrue(driver.close()); + var aggregator = ResponseMetricAggregator.getBean(driver.server()); + var metrics = aggregator.takeStatistics(); + long badRequestResponses = metrics.stream() + .filter(m -> m.dimensions.statusCode == 400 && m.dimensions.method.equals("GET")) + .count(); + assertEquals(1, badRequestResponses, metrics::toString); + } + + @Test void requireThatAccessLogIsCalledForRequestRejectedByJetty() throws Exception { BlockingQueueRequestLog requestLogMock = new BlockingQueueRequestLog(); final JettyTestDriver driver = JettyTestDriver.newConfiguredInstance( @@ -584,11 +605,9 @@ public class HttpServerTest { void requireThatResponseStatsAreCollected() throws Exception { RequestTypeHandler handler = new RequestTypeHandler(); JettyTestDriver driver = JettyTestDriver.newInstance(handler); - HttpResponseStatisticsCollector statisticsCollector = ((AbstractHandlerContainer) driver.server().server().getHandler()) - .getChildHandlerByClass(HttpResponseStatisticsCollector.class); - + var statisticsCollector = ResponseMetricAggregator.getBean(driver.server());; { - List<HttpResponseStatisticsCollector.StatisticsEntry> stats = statisticsCollector.takeStatistics(); + List<ResponseMetricAggregator.StatisticsEntry> stats = statisticsCollector.takeStatistics(); assertEquals(0, stats.size()); } @@ -622,9 +641,9 @@ public class HttpServerTest { assertTrue(driver.close()); } - private HttpResponseStatisticsCollector.StatisticsEntry waitForStatistics(HttpResponseStatisticsCollector + private ResponseMetricAggregator.StatisticsEntry waitForStatistics(ResponseMetricAggregator statisticsCollector) { - List<HttpResponseStatisticsCollector.StatisticsEntry> entries = Collections.emptyList(); + List<ResponseMetricAggregator.StatisticsEntry> entries = Collections.emptyList(); int tries = 0; while (entries.isEmpty() && tries < 10000) { entries = statisticsCollector.takeStatistics(); 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/ResponseMetricAggregatorTest.java index 502702ccf35..e80b6b777db 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/ResponseMetricAggregatorTest.java @@ -1,47 +1,37 @@ // Copyright Yahoo. 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.http.server.jetty.HttpResponseStatisticsCollector.StatisticsEntry; -import jakarta.servlet.ServletException; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; +import com.yahoo.jdisc.http.server.jetty.ResponseMetricAggregator.StatisticsEntry; import org.eclipse.jetty.http.HttpFields; -import org.eclipse.jetty.http.HttpURI; import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.http.MetaData; -import org.eclipse.jetty.http.MetaData.Response; -import org.eclipse.jetty.server.AbstractConnector; -import org.eclipse.jetty.server.Connector; -import org.eclipse.jetty.server.HttpChannel; -import org.eclipse.jetty.server.HttpChannelOverHttp; -import org.eclipse.jetty.server.HttpConfiguration; -import org.eclipse.jetty.server.HttpTransport; import org.eclipse.jetty.server.Request; -import org.eclipse.jetty.server.Server; -import org.eclipse.jetty.server.handler.AbstractHandler; -import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.server.Response; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import java.io.IOException; -import java.nio.ByteBuffer; import java.util.List; import java.util.Set; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; /** * @author ollivir * @author bjorncs */ -public class HttpResponseStatisticsCollectorTest { +public class ResponseMetricAggregatorTest { - private Connector connector; - private List<String> monitoringPaths = List.of("/status.html"); - private List<String> searchPaths = List.of("/search"); - private HttpResponseStatisticsCollector collector = new HttpResponseStatisticsCollector(monitoringPaths, searchPaths, Set.of()); - private int httpResponseCode = 500; + private final List<String> monitoringPaths = List.of("/status.html"); + private final List<String> searchPaths = List.of("/search"); + private final ResponseMetricAggregator collector = new ResponseMetricAggregator(monitoringPaths, searchPaths, Set.of()); + + @BeforeEach + public void initializeCollector() { + collector.takeStatistics(); + } @Test void statistics_are_aggregated_by_category() { @@ -78,7 +68,6 @@ public class HttpResponseStatisticsCollectorTest { } @Test - @SuppressWarnings("removal") void statistics_include_grouped_and_single_statuscodes() { testRequest("http", 401, "GET"); testRequest("http", 404, "GET"); @@ -132,49 +121,28 @@ public class HttpResponseStatisticsCollectorTest { assertStatisticsEntry(stats, "http", "GET", MetricDefinitions.RESPONSES_2XX, "write", 200, 1L); } - @BeforeEach - public void initializeCollector() throws Exception { - Server server = new Server(); - connector = new AbstractConnector(server, null, null, null, 0) { - @Override - protected void accept(int acceptorID) throws IOException, InterruptedException { - } - - @Override - public Object getTransport() { - return null; - } - }; - collector.setHandler(new AbstractHandler() { - @Override - public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) - throws IOException, ServletException { - baseRequest.setHandled(true); - baseRequest.getResponse().setStatus(httpResponseCode); - } - }); - server.setHandler(collector); - server.start(); - } - private Request testRequest(String scheme, int responseCode, String httpMethod) { - return testRequest(scheme, responseCode, httpMethod, "foo/bar"); + private void testRequest(String scheme, int responseCode, String httpMethod) { + testRequest(scheme, responseCode, httpMethod, "foo/bar"); } - private Request testRequest(String scheme, int responseCode, String httpMethod, String path) { - return testRequest(scheme, responseCode, httpMethod, path, null); + private void testRequest(String scheme, int responseCode, String httpMethod, String path) { + testRequest(scheme, responseCode, httpMethod, path, null); } - private Request testRequest(String scheme, int responseCode, String httpMethod, String path, + private void testRequest(String scheme, int responseCode, String httpMethod, String path, com.yahoo.jdisc.Request.RequestType explicitRequestType) { - HttpChannel channel = new HttpChannelOverHttp(null, connector, new HttpConfiguration(), null, new DummyTransport()); - MetaData.Request metaData = new MetaData.Request(httpMethod, HttpURI.build(scheme + "://" + path), HttpVersion.HTTP_1_1, HttpFields.build()); - Request req = channel.getRequest(); - if (explicitRequestType != null) - req.setAttribute("requestType", explicitRequestType); - req.setMetaData(metaData); - - this.httpResponseCode = responseCode; - channel.handle(); - return req; + + Response resp = mock(Response.class); + when(resp.getCommittedMetaData()) + .thenReturn(new MetaData.Response(HttpVersion.HTTP_1_1, responseCode, HttpFields.EMPTY)); + Request req = mock(Request.class); + when(req.getResponse()).thenReturn(resp); + when(req.getMethod()).thenReturn(httpMethod); + when(req.getScheme()).thenReturn(scheme); + when(req.getRequestURI()).thenReturn(path); + when(req.getAttribute(ResponseMetricAggregator.requestTypeAttribute)).thenReturn(explicitRequestType); + when(req.getProtocol()).thenReturn(HttpVersion.HTTP_1_1.asString()); + + collector.onResponseCommit(req); } private static void assertStatisticsEntry(List<StatisticsEntry> result, String scheme, String method, String name, @@ -191,27 +159,4 @@ public class HttpResponseStatisticsCollectorTest { assertThat(value, equalTo(expectedValue)); } - private final class DummyTransport implements HttpTransport { - @Override - public void send(MetaData.Request request, Response response, ByteBuffer byteBuffer, boolean b, Callback callback) { - callback.succeeded(); - } - - @Override - public boolean isPushSupported() { - return false; - } - - @Override - public void push(MetaData.Request request) { - } - - @Override - public void onCompleted() { - } - - @Override - public void abort(Throwable failure) { - } - } } |