Skip to content

Commit

Permalink
Configure NetworkTrafficListener in addToAllConnectors
Browse files Browse the repository at this point in the history 
Without this the metrics on bytes in/out were missing when instrumenting with addToAllConnectors. Reflection is used to support Jetty 9 through 12 where the method for setting the listener has changed.

Fixes gh-5092
  • Loading branch information
@shakuzen
shakuzen committed May 13, 2024
1 parent 4348f37 commit 4cd3490
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 22 deletions.
59 changes: 57 additions & 2 deletions ...core/src/main/java/io/micrometer/core/instrument/binder/jetty/JettyConnectionMetrics.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -46,14 +52,16 @@
* server.setConnectors(new Connector[] { connector });
* }</pre>
*
* Alternatively, configure on all connectors with
* Alternatively, configure on all server connectors with
* {@link JettyConnectionMetrics#addToAllConnectors(Server, MeterRegistry, Iterable)}.
*
* @author Jon Schneider
* @since 1.4.0
*/
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<Tag> tags;
Expand Down Expand Up @@ -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<Tag> 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;
}

}
38 changes: 25 additions & 13 deletions .../src/test/java/io/micrometer/core/instrument/binder/jetty/JettyConnectionMetricsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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();
}
Expand All @@ -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"));

Expand All @@ -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));
Expand All @@ -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
Expand Down
31 changes: 24 additions & 7 deletions micrometer-jetty12/src/test/java/io/micrometer/jetty12/JettyConnectionMetricsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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();
}
Expand All @@ -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"));

Expand Down Expand Up @@ -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));
Expand All @@ -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
Expand Down

0 comments on commit 4cd3490

Please sign in to comment.