diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/jetty/JettyConnectionMetrics.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/jetty/JettyConnectionMetrics.java index 611aeb4d33..7548de1e3f 100644 --- a/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/jetty/JettyConnectionMetrics.java +++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/binder/jetty/JettyConnectionMetrics.java @@ -15,17 +15,23 @@ */ package io.micrometer.core.instrument.binder.jetty; +import io.micrometer.common.lang.Nullable; +import io.micrometer.common.util.internal.logging.InternalLogger; +import io.micrometer.common.util.internal.logging.InternalLoggerFactory; import io.micrometer.core.instrument.*; import io.micrometer.core.instrument.binder.BaseUnits; import io.micrometer.core.instrument.distribution.DistributionStatisticConfig; import io.micrometer.core.instrument.distribution.TimeWindowMax; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; import java.net.Socket; import java.nio.ByteBuffer; import org.eclipse.jetty.io.Connection; import org.eclipse.jetty.io.NetworkTrafficListener; import org.eclipse.jetty.server.Connector; +import org.eclipse.jetty.server.NetworkTrafficServerConnector; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.util.component.AbstractLifeCycle; @@ -46,7 +52,7 @@ * server.setConnectors(new Connector[] { connector }); * } * - * Alternatively, configure on all connectors with + * Alternatively, configure on all server connectors with * {@link JettyConnectionMetrics#addToAllConnectors(Server, MeterRegistry, Iterable)}. * * @author Jon Schneider @@ -54,6 +60,8 @@ */ public class JettyConnectionMetrics extends AbstractLifeCycle implements Connection.Listener, NetworkTrafficListener { + private static final InternalLogger logger = InternalLoggerFactory.getInstance(JettyConnectionMetrics.class); + private final MeterRegistry registry; private final Iterable tags; @@ -193,16 +201,63 @@ public void outgoing(Socket socket, ByteBuffer bytes) { bytesOut.record(bytes.limit()); } + /** + * Configures metrics instrumentation on all the {@link Server}'s {@link Connector}. + * @param server apply to this server's connectors + * @param registry register metrics to this registry + * @param tags add these tags as additional tags on metrics registered via this + */ public static void addToAllConnectors(Server server, MeterRegistry registry, Iterable tags) { for (Connector connector : server.getConnectors()) { if (connector != null) { - connector.addBean(new JettyConnectionMetrics(registry, connector, tags)); + JettyConnectionMetrics metrics = new JettyConnectionMetrics(registry, connector, tags); + connector.addBean(metrics); + if (connector instanceof NetworkTrafficServerConnector) { + NetworkTrafficServerConnector networkTrafficServerConnector = (NetworkTrafficServerConnector) connector; + Method setNetworkTrafficListenerMethod = getNetworkTrafficListenerMethod( + networkTrafficServerConnector); + if (setNetworkTrafficListenerMethod != null) { + try { + setNetworkTrafficListenerMethod.invoke(networkTrafficServerConnector, metrics); + } + catch (IllegalAccessException | InvocationTargetException e) { + logger.debug("Unable to set network traffic listener on connector " + connector, e); + } + } + } } } } + /** + * Configures metrics instrumentation on all the {@link Server}'s {@link Connector}. + * @param server apply to this server's connectors + * @param registry register metrics to this registry + */ public static void addToAllConnectors(Server server, MeterRegistry registry) { addToAllConnectors(server, registry, Tags.empty()); } + @Nullable + private static Method getNetworkTrafficListenerMethod(NetworkTrafficServerConnector networkTrafficServerConnector) { + Method method = null; + try { + // Jetty 9 method + method = networkTrafficServerConnector.getClass() + .getMethod("addNetworkTrafficListener", NetworkTrafficListener.class); + } + catch (NoSuchMethodException ignore) { + } + if (method != null) + return method; + try { + // Jetty 12 method + method = networkTrafficServerConnector.getClass() + .getMethod("setNetworkTrafficListener", NetworkTrafficListener.class); + } + catch (NoSuchMethodException ignore) { + } + return method; + } + } diff --git a/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/jetty/JettyConnectionMetricsTest.java b/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/jetty/JettyConnectionMetricsTest.java index 272f8fb55a..7aed167a65 100644 --- a/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/jetty/JettyConnectionMetricsTest.java +++ b/micrometer-core/src/test/java/io/micrometer/core/instrument/binder/jetty/JettyConnectionMetricsTest.java @@ -38,6 +38,9 @@ import static java.util.concurrent.TimeUnit.SECONDS; import static org.assertj.core.api.Assertions.assertThat; +/** + * Tests for {@link JettyConnectionMetrics} with Jetty 9. + */ class JettyConnectionMetricsTest { private SimpleMeterRegistry registry = new SimpleMeterRegistry(SimpleConfig.DEFAULT, new MockClock()); @@ -48,10 +51,12 @@ class JettyConnectionMetricsTest { private CloseableHttpClient client = HttpClients.createDefault(); - void setup() throws Exception { - JettyConnectionMetrics metrics = new JettyConnectionMetrics(registry); - connector.addBean(metrics); - connector.addNetworkTrafficListener(metrics); + void setup(boolean instrumentServer) throws Exception { + if (instrumentServer) { + JettyConnectionMetrics metrics = new JettyConnectionMetrics(registry); + connector.addBean(metrics); + connector.addNetworkTrafficListener(metrics); + } server.setConnectors(new Connector[] { connector }); server.start(); } @@ -64,8 +69,21 @@ void teardown() throws Exception { } @Test + void directServerConnectorInstrumentation() throws Exception { + setup(true); + contributesServerConnectorMetrics(); + } + + @Test + void addToAllConnectorsInstrumentation() throws Exception { + server.setConnectors(new Connector[] { connector }); + JettyConnectionMetrics.addToAllConnectors(server, registry); + server.start(); + + contributesServerConnectorMetrics(); + } + void contributesServerConnectorMetrics() throws Exception { - setup(); HttpPost post = new HttpPost("http://localhost:" + connector.getLocalPort()); post.setEntity(new StringEntity("123456")); @@ -90,12 +108,12 @@ public void lifeCycleStopped(LifeCycle event) { assertThat(latch.await(10, SECONDS)).isTrue(); assertThat(registry.get("jetty.connections.max").gauge().value()).isEqualTo(2.0); assertThat(registry.get("jetty.connections.request").tag("type", "server").timer().count()).isEqualTo(2); - assertThat(registry.get("jetty.connections.bytes.in").summary().totalAmount()).isPositive(); + assertThat(registry.get("jetty.connections.bytes.in").summary().totalAmount()).isGreaterThan(1); } @Test void contributesClientConnectorMetrics() throws Exception { - setup(); + setup(false); HttpClient httpClient = new HttpClient(); httpClient.setFollowRedirects(false); httpClient.addBean(new JettyConnectionMetrics(registry)); @@ -118,12 +136,6 @@ public void lifeCycleStopped(LifeCycle event) { assertThat(latch.await(10, SECONDS)).isTrue(); assertThat(registry.get("jetty.connections.max").gauge().value()).isEqualTo(1.0); assertThat(registry.get("jetty.connections.request").tag("type", "client").timer().count()).isEqualTo(1); - // assertThat(registry.get("jetty.connections.bytes.out").summary().totalAmount()).isEqualTo(784); - // TODO: explain why there is a difference between what we had before and after - // the change - // assertThat(registry.get("jetty.connections.bytes.out").summary().totalAmount()).isEqualTo(618); - // after the changes - assertThat(registry.get("jetty.connections.bytes.out").summary().totalAmount()).isPositive(); } @Test diff --git a/micrometer-jetty12/src/test/java/io/micrometer/jetty12/JettyConnectionMetricsTest.java b/micrometer-jetty12/src/test/java/io/micrometer/jetty12/JettyConnectionMetricsTest.java index 2587b4315e..172366a2a4 100644 --- a/micrometer-jetty12/src/test/java/io/micrometer/jetty12/JettyConnectionMetricsTest.java +++ b/micrometer-jetty12/src/test/java/io/micrometer/jetty12/JettyConnectionMetricsTest.java @@ -39,6 +39,9 @@ import static java.util.concurrent.TimeUnit.SECONDS; import static org.assertj.core.api.Assertions.assertThat; +/** + * Test {@link JettyConnectionMetrics} with Jetty 12. + */ class JettyConnectionMetricsTest { private SimpleMeterRegistry registry = new SimpleMeterRegistry(SimpleConfig.DEFAULT, new MockClock()); @@ -49,10 +52,12 @@ class JettyConnectionMetricsTest { private CloseableHttpClient client = HttpClients.createDefault(); - void setup() throws Exception { - JettyConnectionMetrics metrics = new JettyConnectionMetrics(registry); - connector.addBean(metrics); - connector.setNetworkTrafficListener(metrics); + void setup(boolean instrumentServer) throws Exception { + if (instrumentServer) { + JettyConnectionMetrics metrics = new JettyConnectionMetrics(registry, connector); + connector.addBean(metrics); + connector.setNetworkTrafficListener(metrics); + } server.setConnectors(new Connector[] { connector }); server.start(); } @@ -65,8 +70,21 @@ void teardown() throws Exception { } @Test + void directServerConnectorInstrumentation() throws Exception { + setup(true); + contributesServerConnectorMetrics(); + } + + @Test + void addToAllConnectorsInstrumentation() throws Exception { + server.setConnectors(new Connector[] { connector }); + JettyConnectionMetrics.addToAllConnectors(server, registry); + server.start(); + + contributesServerConnectorMetrics(); + } + void contributesServerConnectorMetrics() throws Exception { - setup(); HttpPost post = new HttpPost("http://localhost:" + connector.getLocalPort()); post.setEntity(new StringEntity("123456")); @@ -96,7 +114,7 @@ public void lifeCycleStopped(LifeCycle event) { @Test void contributesClientConnectorMetrics() throws Exception { - setup(); + setup(false); HttpClient httpClient = new HttpClient(); httpClient.setFollowRedirects(false); httpClient.addBean(new JettyConnectionMetrics(registry)); @@ -119,7 +137,6 @@ public void lifeCycleStopped(LifeCycle event) { assertThat(latch.await(10, SECONDS)).isTrue(); assertThat(registry.get("jetty.connections.max").gauge().value()).isEqualTo(1.0); assertThat(registry.get("jetty.connections.request").tag("type", "client").timer().count()).isEqualTo(1); - assertThat(registry.get("jetty.connections.bytes.out").summary().totalAmount()).isGreaterThan(1); } @Test