summaryrefslogtreecommitdiffstats
path: root/jdisc_http_service
diff options
context:
space:
mode:
authorJon Bratseth <bratseth@gmail.com>2020-09-02 16:42:10 +0200
committerJon Bratseth <bratseth@gmail.com>2020-09-02 16:42:10 +0200
commit3c9de78274d5f7e5d7c7c19105e4925a91103e9e (patch)
tree1e0027dc08663b890ecdc9b68b6543fadb394691 /jdisc_http_service
parent5a10a3cc2bbad52d783e75a92c1527e33e976fe9 (diff)
Allow setting a request type explicitly
This lets handler authors control the requestType explicitly by setting it on the HttpResponse, which is useful to avoid misclassification of POST requests to reading handlers as writes.
Diffstat (limited to 'jdisc_http_service')
-rw-r--r--jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java38
-rw-r--r--jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollector.java72
-rw-r--r--jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServlet.java36
-rw-r--r--jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscServerConnector.java1
-rw-r--r--jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java101
-rw-r--r--jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletRequestReader.java7
-rw-r--r--jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollectorTest.java28
7 files changed, 142 insertions, 141 deletions
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java
index b9d686c1d6b..81577561c5b 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java
@@ -67,12 +67,11 @@ class HttpRequestDispatch {
this.jettyRequest = (Request) servletRequest;
this.metricReporter = new MetricReporter(jDiscContext.metric, metricContext, jettyRequest.getTimeStamp());
- this.servletResponseController = new ServletResponseController(
- servletRequest,
- servletResponse,
- jDiscContext.janitor,
- metricReporter,
- jDiscContext.developerMode());
+ this.servletResponseController = new ServletResponseController(servletRequest,
+ servletResponse,
+ jDiscContext.janitor,
+ metricReporter,
+ jDiscContext.developerMode());
markConnectionAsNonPersistentIfThresholdReached(servletRequest);
this.async = servletRequest.startAsync();
async.setTimeout(0);
@@ -86,17 +85,13 @@ class HttpRequestDispatch {
} catch (Throwable throwable) {
servletResponseController.trySendError(throwable);
servletResponseController.finishedFuture().whenComplete((result, exception) ->
- completeRequestCallback.accept(null, throwable));
+ completeRequestCallback.accept(null, throwable));
return;
}
try {
- onError(servletRequestReader.finishedFuture,
- servletResponseController::trySendError);
-
- onError(servletResponseController.finishedFuture(),
- servletRequestReader::onError);
-
+ onError(servletRequestReader.finishedFuture, servletResponseController::trySendError);
+ onError(servletResponseController.finishedFuture(), servletRequestReader::onError);
CompletableFuture.allOf(servletRequestReader.finishedFuture, servletResponseController.finishedFuture())
.whenComplete(completeRequestCallback);
} catch (Throwable throwable) {
@@ -104,7 +99,7 @@ class HttpRequestDispatch {
}
}
- private BiConsumer<Void, Throwable> completeRequestCallback;
+ private final BiConsumer<Void, Throwable> completeRequestCallback;
{
AtomicBoolean completeRequestCalled = new AtomicBoolean(false);
HttpRequestDispatch parent = this; //used to avoid binding uninitialized variables
@@ -139,7 +134,7 @@ class HttpRequestDispatch {
log.finest(() -> "Request completed successfully: " + parent.jettyRequest.getRequestURI());
} catch (Throwable throwable) {
Level level = reportedError ? Level.FINE: Level.WARNING;
- log.log(level, "async.complete failed", throwable);
+ log.log(level, "Async.complete failed", throwable);
}
};
}
@@ -180,16 +175,17 @@ class HttpRequestDispatch {
try (ResourceReference ref = References.fromResource(jdiscRequest)) {
HttpRequestFactory.copyHeaders(jettyRequest, jdiscRequest);
requestContentChannel = requestHandler.handleRequest(jdiscRequest, servletResponseController.responseHandler);
+ if (jdiscRequest.getRequestType() != null)
+ jettyRequest.setAttribute(HttpResponseStatisticsCollector.requestTypeAttribute,
+ jdiscRequest.getRequestType());
}
ServletInputStream servletInputStream = jettyRequest.getInputStream();
- ServletRequestReader servletRequestReader =
- new ServletRequestReader(
- servletInputStream,
- requestContentChannel,
- jDiscContext.janitor,
- metricReporter);
+ ServletRequestReader servletRequestReader = new ServletRequestReader(servletInputStream,
+ requestContentChannel,
+ jDiscContext.janitor,
+ metricReporter);
servletInputStream.setReadListener(servletRequestReader);
return servletRequestReader;
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollector.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollector.java
index 13abb8ddd4d..d2ac0cb7f6a 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollector.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollector.java
@@ -1,6 +1,7 @@
// Copyright 2018 Yahoo Holdings. 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.HttpRequest;
import com.yahoo.jdisc.http.server.jetty.JettyHttpServer.Metrics;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.server.AsyncContextEvent;
@@ -26,18 +27,22 @@ import java.util.concurrent.atomic.AtomicReference;
import java.util.concurrent.atomic.LongAdder;
/**
- * 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
+ * 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.
*
* @author ollivir
*/
public class HttpResponseStatisticsCollector extends HandlerWrapper implements Graceful {
+
+ static final String requestTypeAttribute = "requestType";
+
private final AtomicReference<FutureCallback> shutdown = new AtomicReference<>();
private final List<String> monitoringHandlerPaths;
private final List<String> searchHandlerPaths;
- public static enum HttpMethod {
+ public enum HttpMethod {
GET, PATCH, POST, PUT, DELETE, OPTIONS, HEAD, OTHER
}
@@ -45,18 +50,20 @@ public class HttpResponseStatisticsCollector extends HandlerWrapper implements G
HTTP, HTTPS, OTHER
}
- public enum RequestType {
- READ, WRITE, MONITORING
- }
-
- private static final String[] HTTP_RESPONSE_GROUPS = { Metrics.RESPONSES_1XX, Metrics.RESPONSES_2XX, Metrics.RESPONSES_3XX,
- Metrics.RESPONSES_4XX, Metrics.RESPONSES_5XX, Metrics.RESPONSES_401, Metrics.RESPONSES_403};
+ private static final String[] HTTP_RESPONSE_GROUPS = {
+ Metrics.RESPONSES_1XX,
+ Metrics.RESPONSES_2XX,
+ Metrics.RESPONSES_3XX,
+ Metrics.RESPONSES_4XX,
+ Metrics.RESPONSES_5XX,
+ Metrics.RESPONSES_401,
+ Metrics.RESPONSES_403
+ };
private final AtomicLong inFlight = new AtomicLong();
- private final LongAdder statistics[][][][];
+ private final LongAdder[][][][] statistics;
public HttpResponseStatisticsCollector(List<String> monitoringHandlerPaths, List<String> searchHandlerPaths) {
- super();
this.monitoringHandlerPaths = monitoringHandlerPaths;
this.searchHandlerPaths = searchHandlerPaths;
statistics = new LongAdder[HttpScheme.values().length][HttpMethod.values().length][][];
@@ -64,8 +71,8 @@ public class HttpResponseStatisticsCollector extends HandlerWrapper implements G
for (int method = 0; method < HttpMethod.values().length; method++) {
statistics[scheme][method] = new LongAdder[HTTP_RESPONSE_GROUPS.length][];
for (int group = 0; group < HTTP_RESPONSE_GROUPS.length; group++) {
- statistics[scheme][method][group] = new LongAdder[RequestType.values().length];
- for (int requestType = 0 ; requestType < RequestType.values().length; requestType++) {
+ statistics[scheme][method][group] = new LongAdder[HttpRequest.RequestType.values().length];
+ for (int requestType = 0; requestType < HttpRequest.RequestType.values().length; requestType++) {
statistics[scheme][method][group][requestType] = new LongAdder();
}
}
@@ -74,18 +81,17 @@ public class HttpResponseStatisticsCollector extends HandlerWrapper implements G
}
private final AsyncListener completionWatcher = new AsyncListener() {
+
@Override
- public void onTimeout(AsyncEvent event) throws IOException {
- }
+ public void onTimeout(AsyncEvent event) { }
@Override
- public void onStartAsync(AsyncEvent event) throws IOException {
+ public void onStartAsync(AsyncEvent event) {
event.getAsyncContext().addListener(this);
}
@Override
- public void onError(AsyncEvent event) throws IOException {
- }
+ public void onError(AsyncEvent event) { }
@Override
public void onComplete(AsyncEvent event) throws IOException {
@@ -101,18 +107,16 @@ public class HttpResponseStatisticsCollector extends HandlerWrapper implements G
throws IOException, ServletException {
inFlight.incrementAndGet();
- /* The control flow logic here is mostly a copy from org.eclipse.jetty.server.handler.StatisticsHandler.handle(..) */
try {
Handler handler = getHandler();
if (handler != null && shutdown.get() == null && isStarted()) {
handler.handle(path, baseRequest, request, response);
- } else if (!baseRequest.isHandled()) {
+ } 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);
@@ -128,7 +132,7 @@ public class HttpResponseStatisticsCollector extends HandlerWrapper implements G
if (group >= 0) {
HttpScheme scheme = getScheme(request);
HttpMethod method = getMethod(request);
- RequestType requestType = getRequestType(request);
+ HttpRequest.RequestType requestType = getRequestType(request);
statistics[scheme.ordinal()][method.ordinal()][group][requestType.ordinal()].increment();
if (group == 5 || group == 6) { // if 401/403, also increment 4xx
@@ -197,18 +201,22 @@ public class HttpResponseStatisticsCollector extends HandlerWrapper implements G
}
}
- private RequestType getRequestType(Request request) {
+ private HttpRequest.RequestType getRequestType(Request request) {
+ HttpRequest.RequestType requestType = (HttpRequest.RequestType)request.getAttribute(requestTypeAttribute);
+ if (requestType != null) return requestType;
+
+ // Deduce from path and method:
String path = request.getRequestURI();
for (String monitoringHandlerPath : monitoringHandlerPaths) {
- if (path.startsWith(monitoringHandlerPath)) return RequestType.MONITORING;
+ if (path.startsWith(monitoringHandlerPath)) return HttpRequest.RequestType.MONITORING;
}
for (String searchHandlerPath : searchHandlerPaths) {
- if (path.startsWith(searchHandlerPath)) return RequestType.READ;
+ if (path.startsWith(searchHandlerPath)) return HttpRequest.RequestType.READ;
}
if ("GET".equals(request.getMethod())) {
- return RequestType.READ;
+ return HttpRequest.RequestType.READ;
} else {
- return RequestType.WRITE;
+ return HttpRequest.RequestType.WRITE;
}
}
@@ -219,7 +227,7 @@ public class HttpResponseStatisticsCollector extends HandlerWrapper implements G
for (HttpMethod method : HttpMethod.values()) {
int methodIndex = method.ordinal();
for (int group = 0; group < HTTP_RESPONSE_GROUPS.length; group++) {
- for (RequestType type : RequestType.values()) {
+ for (HttpRequest.RequestType type : HttpRequest.RequestType.values()) {
long value = statistics[schemeIndex][methodIndex][group][type.ordinal()].sumThenReset();
if (value > 0) {
ret.add(new StatisticsEntry(scheme.name().toLowerCase(), method.name(), HTTP_RESPONSE_GROUPS[group], type.name().toLowerCase(), value));
@@ -241,15 +249,13 @@ public class HttpResponseStatisticsCollector extends HandlerWrapper implements G
protected void doStop() throws Exception {
super.doStop();
FutureCallback shutdownCb = shutdown.get();
- if (shutdown != null && !shutdownCb.isDone()) {
+ if ( ! shutdownCb.isDone()) {
shutdownCb.failed(new TimeoutException());
}
}
@Override
public Future<Void> shutdown() {
- /* This shutdown callback logic is a copy from org.eclipse.jetty.server.handler.StatisticsHandler */
-
FutureCallback shutdownCb = new FutureCallback(false);
shutdown.compareAndSet(null, shutdownCb);
shutdownCb = shutdown.get();
@@ -266,13 +272,13 @@ public class HttpResponseStatisticsCollector extends HandlerWrapper implements G
}
public static class StatisticsEntry {
+
public final String scheme;
public final String method;
public final String name;
public final String requestType;
public final long value;
-
public StatisticsEntry(String scheme, String method, String name, String requestType, long value) {
this.scheme = scheme;
this.method = method;
@@ -280,5 +286,7 @@ public class HttpResponseStatisticsCollector extends HandlerWrapper implements G
this.requestType = requestType;
this.value = value;
}
+
}
+
}
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServlet.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServlet.java
index a6b2deb4681..dfbcfb741f5 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServlet.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscHttpServlet.java
@@ -33,49 +33,47 @@ class JDiscHttpServlet extends HttpServlet {
private final static Logger log = Logger.getLogger(JDiscHttpServlet.class.getName());
private final JDiscContext context;
+ private static final Set<String> servletSupportedMethods =
+ Stream.of(Method.OPTIONS, Method.GET, Method.HEAD, Method.POST, Method.PUT, Method.DELETE, Method.TRACE)
+ .map(Method::name)
+ .collect(Collectors.toSet());
+
public JDiscHttpServlet(JDiscContext context) {
this.context = context;
}
@Override
- protected void doGet(HttpServletRequest request, HttpServletResponse response)
- throws ServletException, IOException {
+ protected void doGet(HttpServletRequest request, HttpServletResponse response) throws IOException {
dispatchHttpRequest(request, response);
}
@Override
- protected void doPost(HttpServletRequest request, HttpServletResponse response)
- throws ServletException, IOException {
+ protected void doPost(HttpServletRequest request, HttpServletResponse response) throws IOException {
dispatchHttpRequest(request, response);
}
@Override
- protected void doHead(HttpServletRequest request, HttpServletResponse response)
- throws ServletException, IOException {
+ protected void doHead(HttpServletRequest request, HttpServletResponse response) throws IOException {
dispatchHttpRequest(request, response);
}
@Override
- protected void doPut(HttpServletRequest request, HttpServletResponse response)
- throws ServletException, IOException {
+ protected void doPut(HttpServletRequest request, HttpServletResponse response) throws IOException {
dispatchHttpRequest(request, response);
}
@Override
- protected void doDelete(HttpServletRequest request, HttpServletResponse response)
- throws ServletException, IOException {
+ protected void doDelete(HttpServletRequest request, HttpServletResponse response) throws IOException {
dispatchHttpRequest(request, response);
}
@Override
- protected void doOptions(HttpServletRequest request, HttpServletResponse response)
- throws ServletException, IOException {
+ protected void doOptions(HttpServletRequest request, HttpServletResponse response) throws IOException {
dispatchHttpRequest(request, response);
}
@Override
- protected void doTrace(HttpServletRequest request, HttpServletResponse response)
- throws ServletException, IOException {
+ protected void doTrace(HttpServletRequest request, HttpServletResponse response) throws IOException {
dispatchHttpRequest(request, response);
}
@@ -92,11 +90,6 @@ class JDiscHttpServlet extends HttpServlet {
context.metric.add(JettyHttpServer.Metrics.NUM_REQUESTS, 1, metricContext);
context.metric.add(JettyHttpServer.Metrics.JDISC_HTTP_REQUESTS, 1, metricContext);
-
- Set<String> servletSupportedMethods =
- Stream.of(Method.OPTIONS, Method.GET, Method.HEAD, Method.POST, Method.PUT, Method.DELETE, Method.TRACE)
- .map(Method::name)
- .collect(Collectors.toSet());
String method = request.getMethod().toUpperCase();
if (servletSupportedMethods.contains(method)) {
super.service(request, response);
@@ -109,8 +102,6 @@ class JDiscHttpServlet extends HttpServlet {
}
}
-
-
static JDiscServerConnector getConnector(HttpServletRequest request) {
return (JDiscServerConnector)getConnection(request).getConnector();
}
@@ -121,8 +112,7 @@ class JDiscHttpServlet extends HttpServlet {
try {
switch (request.getDispatcherType()) {
case REQUEST:
- new HttpRequestDispatch(context, accessLogEntry, getMetricContext(request), request, response)
- .dispatch();
+ new HttpRequestDispatch(context, accessLogEntry, getMetricContext(request), request, response).dispatch();
break;
default:
if (log.isLoggable(Level.INFO)) {
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscServerConnector.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscServerConnector.java
index 51bcb892591..f480c659578 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscServerConnector.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscServerConnector.java
@@ -21,6 +21,7 @@ import java.util.concurrent.ConcurrentHashMap;
* @author bjorncs
*/
class JDiscServerConnector extends ServerConnector {
+
public static final String REQUEST_ATTRIBUTE = JDiscServerConnector.class.getName();
private final Metric.Context metricCtx;
private final Map<RequestDimensions, Metric.Context> requestMetricContextCache = new ConcurrentHashMap<>();
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java
index 386704a5cc2..ba477f9d32f 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java
@@ -128,17 +128,16 @@ public class JettyHttpServer extends AbstractServerProvider {
private final List<Integer> listenedPorts = new ArrayList<>();
@Inject
- public JettyHttpServer(
- final CurrentContainer container,
- final Metric metric,
- final ServerConfig serverConfig,
- final ServletPathsConfig servletPathsConfig,
- final ThreadFactory threadFactory,
- final FilterBindings filterBindings,
- final ComponentRegistry<ConnectorFactory> connectorFactories,
- final ComponentRegistry<ServletHolder> servletHolders,
- final FilterInvoker filterInvoker,
- final AccessLog accessLog) {
+ public JettyHttpServer(CurrentContainer container,
+ Metric metric,
+ ServerConfig serverConfig,
+ ServletPathsConfig servletPathsConfig,
+ ThreadFactory threadFactory,
+ FilterBindings filterBindings,
+ ComponentRegistry<ConnectorFactory> connectorFactories,
+ ComponentRegistry<ServletHolder> servletHolders,
+ FilterInvoker filterInvoker,
+ AccessLog accessLog) {
super(container);
if (connectorFactories.allComponents().isEmpty())
throw new IllegalArgumentException("No connectors configured.");
@@ -160,44 +159,40 @@ public class JettyHttpServer extends AbstractServerProvider {
janitor = newJanitor(threadFactory);
- JDiscContext jDiscContext = new JDiscContext(
- filterBindings.getRequestFilters().activate(),
- filterBindings.getResponseFilters().activate(),
- container,
- janitor,
- metric,
- serverConfig);
+ JDiscContext jDiscContext = new JDiscContext(filterBindings.getRequestFilters().activate(),
+ filterBindings.getResponseFilters().activate(),
+ container,
+ janitor,
+ metric,
+ serverConfig);
ServletHolder jdiscServlet = new ServletHolder(new JDiscHttpServlet(jDiscContext));
FilterHolder jDiscFilterInvokerFilter = new FilterHolder(new JDiscFilterInvokerFilter(jDiscContext, filterInvoker));
List<JDiscServerConnector> connectors = Arrays.stream(server.getConnectors())
- .map(JDiscServerConnector.class::cast)
- .collect(toList());
-
- server.setHandler(
- getHandlerCollection(
- serverConfig,
- servletPathsConfig,
- connectors,
- jdiscServlet,
- servletHolders,
- jDiscFilterInvokerFilter));
+ .map(JDiscServerConnector.class::cast)
+ .collect(toList());
+
+ server.setHandler(getHandlerCollection(serverConfig,
+ servletPathsConfig,
+ connectors,
+ jdiscServlet,
+ servletHolders,
+ jDiscFilterInvokerFilter));
int numMetricReporterThreads = 1;
- metricReporterExecutor = Executors.newScheduledThreadPool(
- numMetricReporterThreads,
- new ThreadFactoryBuilder()
- .setDaemon(true)
- .setNameFormat(JettyHttpServer.class.getName() + "-MetricReporter-%d")
- .setThreadFactory(threadFactory)
- .build()
- );
+ metricReporterExecutor =
+ Executors.newScheduledThreadPool(numMetricReporterThreads,
+ new ThreadFactoryBuilder()
+ .setDaemon(true)
+ .setNameFormat(JettyHttpServer.class.getName() + "-MetricReporter-%d")
+ .setThreadFactory(threadFactory)
+ .build());
metricReporterExecutor.scheduleAtFixedRate(new MetricTask(), 0, 2, TimeUnit.SECONDS);
}
private static void initializeJettyLogging() {
- // Note: Jetty is logging stderr if no logger is explicitly configured.
+ // Note: Jetty is logging stderr if no logger is explicitly configured
try {
Log.setLog(new JavaUtilLog());
} catch (Exception e) {
@@ -208,32 +203,26 @@ public class JettyHttpServer extends AbstractServerProvider {
private static void setupJmx(Server server, ServerConfig serverConfig) {
if (serverConfig.jmx().enabled()) {
System.setProperty("java.rmi.server.hostname", "localhost");
- server.addBean(
- new MBeanContainer(ManagementFactory.getPlatformMBeanServer()));
- server.addBean(
- new ConnectorServer(
- createJmxLoopbackOnlyServiceUrl(serverConfig.jmx().listenPort()),
- "org.eclipse.jetty.jmx:name=rmiconnectorserver"));
+ server.addBean(new MBeanContainer(ManagementFactory.getPlatformMBeanServer()));
+ server.addBean(new ConnectorServer(createJmxLoopbackOnlyServiceUrl(serverConfig.jmx().listenPort()),
+ "org.eclipse.jetty.jmx:name=rmiconnectorserver"));
}
}
private static JMXServiceURL createJmxLoopbackOnlyServiceUrl(int port) {
try {
- return new JMXServiceURL(
- "rmi", "localhost", port, "/jndi/rmi://localhost:" + port + "/jmxrmi");
+ return new JMXServiceURL("rmi", "localhost", port, "/jndi/rmi://localhost:" + port + "/jmxrmi");
} catch (MalformedURLException e) {
throw new RuntimeException(e);
}
}
- private HandlerCollection getHandlerCollection(
- ServerConfig serverConfig,
- ServletPathsConfig servletPathsConfig,
- List<JDiscServerConnector> connectors,
- ServletHolder jdiscServlet,
- ComponentRegistry<ServletHolder> servletHolders,
- FilterHolder jDiscFilterInvokerFilter) {
-
+ private HandlerCollection getHandlerCollection(ServerConfig serverConfig,
+ ServletPathsConfig servletPathsConfig,
+ List<JDiscServerConnector> connectors,
+ ServletHolder jdiscServlet,
+ ComponentRegistry<ServletHolder> servletHolders,
+ FilterHolder jDiscFilterInvokerFilter) {
ServletContextHandler servletContextHandler = createServletContextHandler();
servletHolders.allComponentsById().forEach((id, servlet) -> {
@@ -257,7 +246,9 @@ public class JettyHttpServer extends AbstractServerProvider {
GzipHandler gzipHandler = newGzipHandler(serverConfig);
gzipHandler.setHandler(authEnforcer);
- HttpResponseStatisticsCollector statisticsCollector = new HttpResponseStatisticsCollector(serverConfig.metric().monitoringHandlerPaths(), serverConfig.metric().searchHandlerPaths());
+ HttpResponseStatisticsCollector statisticsCollector =
+ new HttpResponseStatisticsCollector(serverConfig.metric().monitoringHandlerPaths(),
+ serverConfig.metric().searchHandlerPaths());
statisticsCollector.setHandler(gzipHandler);
StatisticsHandler statisticsHandler = newStatisticsHandler();
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletRequestReader.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletRequestReader.java
index 9314247b83b..fd1f84f7d49 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletRequestReader.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletRequestReader.java
@@ -28,6 +28,7 @@ import java.util.logging.Logger;
* it's important that errors are delivered synchronously.
*/
class ServletRequestReader implements ReadListener {
+
private enum State {
READING, ALL_DATA_READ, REQUEST_CONTENT_CLOSED
}
@@ -136,7 +137,7 @@ class ServletRequestReader implements ReadListener {
requestContentChannel.write(buf, writeCompletionHandler);
metricReporter.successfulRead(bytesReceived);
bytesRead += bytesReceived;
- } catch (final Throwable t) {
+ } catch (Throwable t) {
finishedFuture.completeExceptionally(t);
} finally {
//decrease due to this method completing.
@@ -145,7 +146,7 @@ class ServletRequestReader implements ReadListener {
}
private void decreaseOutstandingUserCallsAndCloseRequestContentChannelConditionally() {
- final boolean shouldCloseRequestContentChannel;
+ boolean shouldCloseRequestContentChannel;
synchronized (monitor) {
assertStateNotEquals(state, State.REQUEST_CONTENT_CLOSED);
@@ -154,7 +155,7 @@ class ServletRequestReader implements ReadListener {
numberOfOutstandingUserCalls -= 1;
shouldCloseRequestContentChannel = numberOfOutstandingUserCalls == 0 &&
- (finishedFuture.isDone() || state == State.ALL_DATA_READ);
+ (finishedFuture.isDone() || state == State.ALL_DATA_READ);
if (shouldCloseRequestContentChannel) {
state = State.REQUEST_CONTENT_CLOSED;
diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollectorTest.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollectorTest.java
index 1a8aa23668b..4ae824e2b7a 100644
--- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollectorTest.java
+++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpResponseStatisticsCollectorTest.java
@@ -34,6 +34,7 @@ import static org.hamcrest.Matchers.equalTo;
* @author ollivir
*/
public class HttpResponseStatisticsCollectorTest {
+
private Connector connector;
private List<String> monitoringPaths = List.of("/status.html");
private List<String> searchPaths = List.of("/search");
@@ -41,7 +42,7 @@ public class HttpResponseStatisticsCollectorTest {
private int httpResponseCode = 500;
@Test
- public void statistics_are_aggregated_by_category() throws Exception {
+ public void statistics_are_aggregated_by_category() {
testRequest("http", 300, "GET");
testRequest("http", 301, "GET");
testRequest("http", 200, "GET");
@@ -52,7 +53,7 @@ public class HttpResponseStatisticsCollectorTest {
}
@Test
- public void statistics_are_grouped_by_http_method_and_scheme() throws Exception {
+ public void statistics_are_grouped_by_http_method_and_scheme() {
testRequest("http", 200, "GET");
testRequest("http", 200, "PUT");
testRequest("http", 200, "POST");
@@ -74,7 +75,7 @@ public class HttpResponseStatisticsCollectorTest {
}
@Test
- public void statistics_include_grouped_and_single_statuscodes() throws Exception {
+ public void statistics_include_grouped_and_single_statuscodes() {
testRequest("http", 401, "GET");
testRequest("http", 404, "GET");
testRequest("http", 403, "GET");
@@ -87,7 +88,7 @@ public class HttpResponseStatisticsCollectorTest {
}
@Test
- public void retrieving_statistics_resets_the_counters() throws Exception {
+ public void retrieving_statistics_resets_the_counters() {
testRequest("http", 200, "GET");
testRequest("http", 200, "GET");
@@ -101,7 +102,7 @@ public class HttpResponseStatisticsCollectorTest {
}
@Test
- public void statistics_include_request_type_dimension() throws Exception {
+ public void statistics_include_request_type_dimension() {
testRequest("http", 200, "GET", "/search");
testRequest("http", 200, "POST", "/search");
testRequest("http", 200, "POST", "/feed");
@@ -117,7 +118,14 @@ public class HttpResponseStatisticsCollectorTest {
stats = collector.takeStatistics();
assertStatisticsEntryPresent(stats, "http", "GET", Metrics.RESPONSES_2XX, 1L);
+ }
+
+ @Test
+ public void request_type_can_be_set_explicitly() {
+ testRequest("http", 200, "GET", "/search", com.yahoo.jdisc.Request.RequestType.WRITE);
+ var stats = collector.takeStatistics();
+ assertStatisticsEntryWithRequestTypePresent(stats, "http", "GET", Metrics.RESPONSES_2XX, "write", 1L);
}
@Before
@@ -145,13 +153,19 @@ public class HttpResponseStatisticsCollectorTest {
server.start();
}
- private Request testRequest(String scheme, int responseCode, String httpMethod) throws Exception {
+ private Request testRequest(String scheme, int responseCode, String httpMethod) {
return testRequest(scheme, responseCode, httpMethod, "foo/bar");
}
- private Request testRequest(String scheme, int responseCode, String httpMethod, String path) throws Exception {
+ private Request testRequest(String scheme, int responseCode, String httpMethod, String path) {
+ return testRequest(scheme, responseCode, httpMethod, path, null);
+ }
+ private Request testRequest(String scheme, int responseCode, String httpMethod, String path,
+ com.yahoo.jdisc.Request.RequestType explicitRequestType) {
HttpChannel channel = new HttpChannel(connector, new HttpConfiguration(), null, new DummyTransport());
MetaData.Request metaData = new MetaData.Request(httpMethod, new HttpURI(scheme + "://" + path), HttpVersion.HTTP_1_1, new HttpFields());
Request req = channel.getRequest();
+ if (explicitRequestType != null)
+ req.setAttribute("requestType", explicitRequestType);
req.setMetaData(metaData);
this.httpResponseCode = responseCode;