diff options
8 files changed, 97 insertions, 5 deletions
diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectionMetricAggregator.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectionMetricAggregator.java new file mode 100644 index 00000000000..a92cbf264a4 --- /dev/null +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectionMetricAggregator.java @@ -0,0 +1,65 @@ +// 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.ServerConfig; +import org.eclipse.jetty.io.Connection; +import org.eclipse.jetty.server.HttpChannel; +import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.util.component.AbstractLifeCycle; + +import java.util.Collection; +import java.util.concurrent.atomic.AtomicLong; + +import static com.yahoo.jdisc.http.server.jetty.RequestUtils.getConnector; +import static com.yahoo.jdisc.http.server.jetty.RequestUtils.isHttpServerConnection; + +/** + * @author bjorncs + */ +class ConnectionMetricAggregator extends AbstractLifeCycle implements Connection.Listener, HttpChannel.Listener{ + + private final SimpleConcurrentIdentityHashMap<Connection, ConnectionMetrics> connectionsMetrics = new SimpleConcurrentIdentityHashMap<>(); + + private final Metric metricAggregator; + private final Collection<String> monitoringHandlerPaths; + + ConnectionMetricAggregator(ServerConfig serverConfig, Metric metricAggregator) { + this.monitoringHandlerPaths = serverConfig.metric().monitoringHandlerPaths(); + this.metricAggregator = metricAggregator; + } + + @Override public void onOpened(Connection connection) {} + + @Override + public void onClosed(Connection connection) { + if (isHttpServerConnection(connection)) { + connectionsMetrics.remove(connection).ifPresent(metrics -> + metricAggregator.set(MetricDefinitions.REQUESTS_PER_CONNECTION, metrics.requests.get(), metrics.metricContext)); + } + } + + @Override + public void onRequestBegin(Request request) { + if (monitoringHandlerPaths.stream() + .anyMatch(pathPrefix -> request.getRequestURI().startsWith(pathPrefix))){ + return; + } + Connection connection = request.getHttpChannel().getConnection(); + if (isHttpServerConnection(connection)) { + ConnectionMetrics metrics = this.connectionsMetrics.computeIfAbsent( + connection, + () -> new ConnectionMetrics(getConnector(request).getConnectorMetricContext())); + metrics.requests.incrementAndGet(); + } + } + + private static class ConnectionMetrics { + final AtomicLong requests = new AtomicLong(); + final Metric.Context metricContext; + + ConnectionMetrics(Metric.Context metricContext) { + this.metricContext = metricContext; + } + } +} diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactory.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactory.java index b9b6bcf57f0..9d14771d242 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactory.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactory.java @@ -66,9 +66,11 @@ public class ConnectorFactory { return connectorConfig; } - public ServerConnector createConnector(final Metric metric, final Server server, JettyConnectionLogger connectionLogger) { + public ServerConnector createConnector(final Metric metric, final Server server, JettyConnectionLogger connectionLogger, + ConnectionMetricAggregator connectionMetricAggregator) { ServerConnector connector = new JDiscServerConnector( - connectorConfig, metric, server, connectionLogger, createConnectionFactories(metric).toArray(ConnectionFactory[]::new)); + connectorConfig, metric, server, connectionLogger, connectionMetricAggregator, + createConnectionFactories(metric).toArray(ConnectionFactory[]::new)); connector.setPort(connectorConfig.listenPort()); connector.setName(connectorConfig.name()); connector.setAcceptQueueSize(connectorConfig.acceptQueueSize()); diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscServerConnector.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscServerConnector.java index a355ca886c5..ae475ca4517 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscServerConnector.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscServerConnector.java @@ -31,7 +31,8 @@ class JDiscServerConnector extends ServerConnector { private final String connectorName; private final int listenPort; - JDiscServerConnector(ConnectorConfig config, Metric metric, Server server, JettyConnectionLogger connectionLogger, ConnectionFactory... factories) { + JDiscServerConnector(ConnectorConfig config, Metric metric, Server server, JettyConnectionLogger connectionLogger, + ConnectionMetricAggregator connectionMetricAggregator, ConnectionFactory... factories) { super(server, factories); this.config = config; this.tcpKeepAlive = config.tcpKeepAliveEnabled(); @@ -48,6 +49,7 @@ class JDiscServerConnector extends ServerConnector { new ConnectionThrottler(this, throttlingConfig).registerWithConnector(); } addBean(connectionLogger); + addBean(connectionMetricAggregator); } @Override 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 cf9945cc65b..70f173b74e5 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 @@ -83,10 +83,11 @@ public class JettyHttpServer extends AbstractServerProvider { setupJmx(server, serverConfig); configureJettyThreadpool(server, serverConfig); JettyConnectionLogger connectionLogger = new JettyConnectionLogger(serverConfig.connectionLog(), connectionLog); + ConnectionMetricAggregator connectionMetricAggregator = new ConnectionMetricAggregator(serverConfig, metric); for (ConnectorFactory connectorFactory : connectorFactories.allComponents()) { ConnectorConfig connectorConfig = connectorFactory.getConnectorConfig(); - server.addConnector(connectorFactory.createConnector(metric, server, connectionLogger)); + server.addConnector(connectorFactory.createConnector(metric, server, connectionLogger, connectionMetricAggregator)); listenedPorts.add(connectorConfig.listenPort()); } diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/MetricDefinitions.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/MetricDefinitions.java index b0b7ae14739..172e6483de2 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/MetricDefinitions.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/MetricDefinitions.java @@ -24,6 +24,7 @@ class MetricDefinitions { static final String CONNECTION_DURATION_MEAN = "serverConnectionDurationMean"; static final String CONNECTION_DURATION_STD_DEV = "serverConnectionDurationStdDev"; static final String NUM_PREMATURELY_CLOSED_CONNECTIONS = "jdisc.http.request.prematurely_closed"; + static final String REQUESTS_PER_CONNECTION = "jdisc.http.request.requests_per_connection"; static final String NUM_BYTES_RECEIVED = "serverBytesReceived"; static final String NUM_BYTES_SENT = "serverBytesSent"; diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/RequestUtils.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/RequestUtils.java index 5fca7a8d778..b248f55a3df 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/RequestUtils.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/RequestUtils.java @@ -1,6 +1,7 @@ // Copyright 2017 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 org.eclipse.jetty.http2.server.HTTP2ServerConnection; import org.eclipse.jetty.io.Connection; import org.eclipse.jetty.server.HttpConnection; import org.eclipse.jetty.server.Request; @@ -28,6 +29,10 @@ public class RequestUtils { return (JDiscServerConnector) request.getHttpChannel().getConnector(); } + static boolean isHttpServerConnection(Connection connection) { + return connection instanceof HttpConnection || connection instanceof HTTP2ServerConnection; + } + /** * Note: {@link HttpServletRequest#getLocalPort()} may return the local port of the load balancer / reverse proxy if proxy-protocol is enabled. * @return the actual local port of the underlying Jetty connector diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactoryTest.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactoryTest.java index df794c7ecb8..93261a2401f 100644 --- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactoryTest.java +++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactoryTest.java @@ -31,8 +31,10 @@ public class ConnectorFactoryTest { JettyConnectionLogger connectionLogger = new JettyConnectionLogger( new ServerConfig.ConnectionLog.Builder().enabled(false).build(), new VoidConnectionLog()); + DummyMetric metric = new DummyMetric(); + var connectionMetricAggregator = new ConnectionMetricAggregator(new ServerConfig(new ServerConfig.Builder()), metric); JDiscServerConnector connector = - (JDiscServerConnector)factory.createConnector(new DummyMetric(), server, connectionLogger); + (JDiscServerConnector)factory.createConnector(metric, server, connectionLogger, connectionMetricAggregator); server.addConnector(connector); server.setHandler(new HelloWorldHandler()); server.start(); 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 a5804dc9b86..5056cf91d79 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 @@ -917,6 +917,20 @@ public class HttpServerTest { assertThat(driver.close(), is(true)); } + @Test + public void requireThatRequestsPerConnectionMetricIsAggregated() throws IOException { + Path privateKeyFile = tmpFolder.newFile().toPath(); + Path certificateFile = tmpFolder.newFile().toPath(); + generatePrivateKeyAndCertificate(privateKeyFile, certificateFile); + var metricConsumer = new MetricConsumerMock(); + InMemoryConnectionLog connectionLog = new InMemoryConnectionLog(); + JettyTestDriver driver = createSslTestDriver(certificateFile, privateKeyFile, metricConsumer, connectionLog); + driver.client().get("/").expectStatusCode(is(OK)); + assertThat(driver.close(), is(true)); + verify(metricConsumer.mockitoMock(), atLeast(1)) + .set(MetricDefinitions.REQUESTS_PER_CONNECTION, 1L, MetricConsumerMock.STATIC_CONTEXT); + } + private ContentResponse sendJettyClientRequest(JettyTestDriver testDriver, Path certificateFile, Object tag) throws Exception { HttpClient client = createJettyHttpClient(certificateFile); |