Merge pull request #25684 from vespa-engine/bjorncs/jdisc-response-metrics

Implement response metric aggregator as a HttpChannel.Listener

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) {
-        }
-    }
 }