diff options
Diffstat (limited to 'container-core/src')
6 files changed, 39 insertions, 32 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 1747541bef5..618c58e31c5 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 @@ -9,7 +9,6 @@ import com.yahoo.jdisc.AbstractResource; import com.yahoo.jdisc.Metric; import com.yahoo.jdisc.http.ConnectorConfig; import com.yahoo.jdisc.http.ServerConfig; -import com.yahoo.jdisc.service.AbstractServerProvider; import com.yahoo.jdisc.service.CurrentContainer; import com.yahoo.jdisc.service.ServerProvider; import org.eclipse.jetty.http.HttpField; @@ -41,7 +40,6 @@ import java.util.Arrays; import java.util.Deque; import java.util.List; import java.util.concurrent.ConcurrentLinkedDeque; -import java.util.function.Supplier; import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Collectors; @@ -174,7 +172,7 @@ public class JettyHttpServer extends AbstractResource implements ServerProvider public void start() { try { server.start(); - metricsReporter.start(); + if (config.metric().reporterEnabled()) metricsReporter.start(); logEffectiveSslConfiguration(); } catch (final Exception e) { if (e instanceof IOException && e.getCause() instanceof BindException) { diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ResponseMetricAggregator.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ResponseMetricAggregator.java index 36ca1a63753..0880e33d41a 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ResponseMetricAggregator.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ResponseMetricAggregator.java @@ -37,19 +37,20 @@ class ResponseMetricAggregator extends AbstractLifeCycle implements HttpChannel. private final List<String> monitoringHandlerPaths; private final List<String> searchHandlerPaths; private final Set<String> ignoredUserAgents; + private final boolean reporterEnabled; private final ConcurrentMap<StatusCodeMetric, LongAdder> statistics = new ConcurrentHashMap<>(); ResponseMetricAggregator(ServerConfig.Metric cfg) { - this(cfg.monitoringHandlerPaths(), cfg.searchHandlerPaths(), cfg.ignoredUserAgents()); + this(cfg.monitoringHandlerPaths(), cfg.searchHandlerPaths(), cfg.ignoredUserAgents(), cfg.reporterEnabled()); } ResponseMetricAggregator(List<String> monitoringHandlerPaths, List<String> searchHandlerPaths, - Collection<String> ignoredUserAgents) { + Collection<String> ignoredUserAgents, boolean reporterEnabled) { this.monitoringHandlerPaths = monitoringHandlerPaths; this.searchHandlerPaths = searchHandlerPaths; this.ignoredUserAgents = Set.copyOf(ignoredUserAgents); - + this.reporterEnabled = reporterEnabled; } static ResponseMetricAggregator getBean(JettyHttpServer server) { return getBean(server.server()); } @@ -67,12 +68,15 @@ class ResponseMetricAggregator extends AbstractLifeCycle implements HttpChannel. } List<StatisticsEntry> takeStatistics() { + if (reporterEnabled) + throw new IllegalStateException("Cannot take consistent snapshot while reporter is enabled"); var ret = new ArrayList<StatisticsEntry>(); consume((metric, value) -> ret.add(new StatisticsEntry(metric, value))); return ret; } void reportSnapshot(Metric metricAggregator) { + if (!reporterEnabled) throw new IllegalStateException("Reporter is not enabled"); consume((metric, value) -> { Metric.Context ctx = metricAggregator.createContext(metric.dimensions.asMap()); metricAggregator.add(metric.name, value, ctx); diff --git a/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.server.def b/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.server.def index b43db0c5287..dd4d138b8c1 100644 --- a/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.server.def +++ b/container-core/src/main/resources/configdefinitions/jdisc.http.jdisc.http.server.def @@ -52,5 +52,8 @@ metric.searchHandlerPaths[] string # User-agent names to ignore wrt statistics (crawlers etc) metric.ignoredUserAgents[] string +# Whether to enable scheduled metric reporter. Disabled for unit testing to stop metric counters from being reset. +metric.reporterEnabled bool default = true + # Whether to enable jdisc connection log connectionLog.enabled bool default=false 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 adb35db8ebf..32375be2a68 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 @@ -178,7 +178,7 @@ public class HttpServerTest { var metricConsumer = new MetricConsumerMock(); JettyTestDriver driver = JettyTestDriver.newConfiguredInstance( mockRequestHandler(), - new ServerConfig.Builder(), + new ServerConfig.Builder().metric(new ServerConfig.Metric.Builder().reporterEnabled(false)), new ConnectorConfig.Builder(), binder -> binder.bind(MetricConsumer.class).toInstance(metricConsumer.mockitoMock())); driver.client() @@ -614,7 +614,8 @@ public class HttpServerTest { @Test void requireThatResponseStatsAreCollected() throws Exception { RequestTypeHandler handler = new RequestTypeHandler(); - JettyTestDriver driver = JettyTestDriver.newInstance(handler); + var cfg = new ServerConfig.Builder().metric(new ServerConfig.Metric.Builder().reporterEnabled(false)); + JettyTestDriver driver = JettyTestDriver.newConfiguredInstance(handler, cfg, new ConnectorConfig.Builder()); var statisticsCollector = ResponseMetricAggregator.getBean(driver.server());; { List<ResponseMetricAggregator.StatisticsEntry> stats = statisticsCollector.takeStatistics(); @@ -655,7 +656,8 @@ public class HttpServerTest { statisticsCollector) { List<ResponseMetricAggregator.StatisticsEntry> entries = Collections.emptyList(); int tries = 0; - while (entries.isEmpty() && tries < 10000) { + // Wait up to 30 seconds before giving up + while (entries.isEmpty() && tries < 300) { entries = statisticsCollector.takeStatistics(); if (entries.isEmpty()) try {Thread.sleep(100); } catch (InterruptedException e) {} diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/ProxyProtocolTest.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/ProxyProtocolTest.java index eac7b7c5df7..8df20239875 100644 --- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/ProxyProtocolTest.java +++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/ProxyProtocolTest.java @@ -17,6 +17,7 @@ import org.eclipse.jetty.util.ssl.SslContextFactory; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.Timeout; import org.junit.jupiter.api.io.TempDir; import java.io.IOException; @@ -40,6 +41,7 @@ import static org.junit.jupiter.api.Assertions.fail; /** * @author bjorncs */ +@Timeout(value = 60) class ProxyProtocolTest { private static final Logger log = Logger.getLogger(ProxyProtocolTest.class.getName()); @@ -155,30 +157,28 @@ class ProxyProtocolTest { private ContentResponse sendJettyClientRequest(JettyTestDriver testDriver, Path certificateFile, Object tag) throws Exception { - HttpClient client = createJettyHttpClient(certificateFile); ExecutionException cause = null; - try { - int maxAttempts = 10; - for (int attempt = 0; attempt < maxAttempts; attempt++) { - try { - ContentResponse response = client.newRequest(URI.create("https://localhost:" + testDriver.server().getListenPort() + "/")) - .tag(tag) - .send(); - assertEquals(200, response.getStatus()); - return response; - } catch (ExecutionException e) { - // Retry when the server closes the connection before the TLS handshake is completed. This has been observed in CI. - // We have been unable to reproduce this locally. The cause is therefor currently unknown. - log.log(Level.WARNING, String.format("Attempt %d failed: %s", attempt, e.getMessage()), e); - Thread.sleep(10); - cause = e; - } + int maxAttempts = 10; + for (int attempt = 0; attempt < maxAttempts; attempt++) { + HttpClient client = createJettyHttpClient(certificateFile); + try { + ContentResponse response = client.newRequest(URI.create("https://localhost:" + testDriver.server().getListenPort() + "/")) + .tag(tag) + .send(); + assertEquals(200, response.getStatus()); + return response; + } catch (ExecutionException e) { + // Retry when the server closes the connection before the TLS handshake is completed. This has been observed in CI. + // We have been unable to reproduce this locally. The cause is therefor currently unknown. + log.log(Level.WARNING, String.format("Attempt %d failed: %s", attempt, e.getMessage()), e); + Thread.sleep(10); + cause = e; + } finally { + client.stop(); + client.destroy(); } - throw cause; - } finally { - client.stop(); - client.destroy(); } + throw cause; } // Using Jetty's http client as Apache httpclient does not support the proxy-protocol v1/v2. @@ -189,7 +189,7 @@ class ProxyProtocolTest { var connector = new ClientConnector(); connector.setSslContextFactory(ssl); HttpClient client = new HttpClient(new HttpClientTransportOverHTTP(connector)); - int timeout = 60 * 1000; + int timeout = 20 * 1000; client.setConnectTimeout(timeout); client.setIdleTimeout(timeout); client.start(); diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/ResponseMetricAggregatorTest.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/ResponseMetricAggregatorTest.java index 25a93849acb..7f355922b35 100644 --- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/ResponseMetricAggregatorTest.java +++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/ResponseMetricAggregatorTest.java @@ -26,7 +26,7 @@ public class ResponseMetricAggregatorTest { 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()); + private final ResponseMetricAggregator collector = new ResponseMetricAggregator(monitoringPaths, searchPaths, Set.of(), false); @BeforeEach public void initializeCollector() { |