Skip to content

Commit 4cd3490

Browse files
shakuzenshakuzen
committed
Configure NetworkTrafficListener in addToAllConnectors
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
1 parent 4348f37 commit 4cd3490

File tree

3 files changed

+106
-22
lines changed

3 files changed

+106
-22
lines changed

‎micrometer-core/src/main/java/io/micrometer/core/instrument/binder/jetty/JettyConnectionMetrics.java

+57-2
Original file line numberDiff line numberDiff line change
@@ -15,17 +15,23 @@
1515
*/
1616
package io.micrometer.core.instrument.binder.jetty;
1717

18+
import io.micrometer.common.lang.Nullable;
19+
import io.micrometer.common.util.internal.logging.InternalLogger;
20+
import io.micrometer.common.util.internal.logging.InternalLoggerFactory;
1821
import io.micrometer.core.instrument.*;
1922
import io.micrometer.core.instrument.binder.BaseUnits;
2023
import io.micrometer.core.instrument.distribution.DistributionStatisticConfig;
2124
import io.micrometer.core.instrument.distribution.TimeWindowMax;
2225

26+
import java.lang.reflect.InvocationTargetException;
27+
import java.lang.reflect.Method;
2328
import java.net.Socket;
2429
import java.nio.ByteBuffer;
2530

2631
import org.eclipse.jetty.io.Connection;
2732
import org.eclipse.jetty.io.NetworkTrafficListener;
2833
import org.eclipse.jetty.server.Connector;
34+
import org.eclipse.jetty.server.NetworkTrafficServerConnector;
2935
import org.eclipse.jetty.server.Server;
3036
import org.eclipse.jetty.util.component.AbstractLifeCycle;
3137

@@ -46,14 +52,16 @@
4652
* server.setConnectors(new Connector[] { connector });
4753
* }</pre>
4854
*
49-
* Alternatively, configure on all connectors with
55+
* Alternatively, configure on all server connectors with
5056
* {@link JettyConnectionMetrics#addToAllConnectors(Server, MeterRegistry, Iterable)}.
5157
*
5258
* @author Jon Schneider
5359
* @since 1.4.0
5460
*/
5561
public class JettyConnectionMetrics extends AbstractLifeCycle implements Connection.Listener, NetworkTrafficListener {
5662

63+
private static final InternalLogger logger = InternalLoggerFactory.getInstance(JettyConnectionMetrics.class);
64+
5765
private final MeterRegistry registry;
5866

5967
private final Iterable<Tag> tags;
@@ -193,16 +201,63 @@ public void outgoing(Socket socket, ByteBuffer bytes) {
193201
bytesOut.record(bytes.limit());
194202
}
195203

204+
/**
205+
* Configures metrics instrumentation on all the {@link Server}'s {@link Connector}.
206+
* @param server apply to this server's connectors
207+
* @param registry register metrics to this registry
208+
* @param tags add these tags as additional tags on metrics registered via this
209+
*/
196210
public static void addToAllConnectors(Server server, MeterRegistry registry, Iterable<Tag> tags) {
197211
for (Connector connector : server.getConnectors()) {
198212
if (connector != null) {
199-
connector.addBean(new JettyConnectionMetrics(registry, connector, tags));
213+
JettyConnectionMetrics metrics = new JettyConnectionMetrics(registry, connector, tags);
214+
connector.addBean(metrics);
215+
if (connector instanceof NetworkTrafficServerConnector) {
216+
NetworkTrafficServerConnector networkTrafficServerConnector = (NetworkTrafficServerConnector) connector;
217+
Method setNetworkTrafficListenerMethod = getNetworkTrafficListenerMethod(
218+
networkTrafficServerConnector);
219+
if (setNetworkTrafficListenerMethod != null) {
220+
try {
221+
setNetworkTrafficListenerMethod.invoke(networkTrafficServerConnector, metrics);
222+
}
223+
catch (IllegalAccessException | InvocationTargetException e) {
224+
logger.debug("Unable to set network traffic listener on connector " + connector, e);
225+
}
226+
}
227+
}
200228
}
201229
}
202230
}
203231

232+
/**
233+
* Configures metrics instrumentation on all the {@link Server}'s {@link Connector}.
234+
* @param server apply to this server's connectors
235+
* @param registry register metrics to this registry
236+
*/
204237
public static void addToAllConnectors(Server server, MeterRegistry registry) {
205238
addToAllConnectors(server, registry, Tags.empty());
206239
}
207240

241+
@Nullable
242+
private static Method getNetworkTrafficListenerMethod(NetworkTrafficServerConnector networkTrafficServerConnector) {
243+
Method method = null;
244+
try {
245+
// Jetty 9 method
246+
method = networkTrafficServerConnector.getClass()
247+
.getMethod("addNetworkTrafficListener", NetworkTrafficListener.class);
248+
}
249+
catch (NoSuchMethodException ignore) {
250+
}
251+
if (method != null)
252+
return method;
253+
try {
254+
// Jetty 12 method
255+
method = networkTrafficServerConnector.getClass()
256+
.getMethod("setNetworkTrafficListener", NetworkTrafficListener.class);
257+
}
258+
catch (NoSuchMethodException ignore) {
259+
}
260+
return method;
261+
}
262+
208263
}

‎micrometer-core/src/test/java/io/micrometer/core/instrument/binder/jetty/JettyConnectionMetricsTest.java

+25-13
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@
3838
import static java.util.concurrent.TimeUnit.SECONDS;
3939
import static org.assertj.core.api.Assertions.assertThat;
4040

41+
/**
42+
* Tests for {@link JettyConnectionMetrics} with Jetty 9.
43+
*/
4144
class JettyConnectionMetricsTest {
4245

4346
private SimpleMeterRegistry registry = new SimpleMeterRegistry(SimpleConfig.DEFAULT, new MockClock());
@@ -48,10 +51,12 @@ class JettyConnectionMetricsTest {
4851

4952
private CloseableHttpClient client = HttpClients.createDefault();
5053

51-
void setup() throws Exception {
52-
JettyConnectionMetrics metrics = new JettyConnectionMetrics(registry);
53-
connector.addBean(metrics);
54-
connector.addNetworkTrafficListener(metrics);
54+
void setup(boolean instrumentServer) throws Exception {
55+
if (instrumentServer) {
56+
JettyConnectionMetrics metrics = new JettyConnectionMetrics(registry);
57+
connector.addBean(metrics);
58+
connector.addNetworkTrafficListener(metrics);
59+
}
5560
server.setConnectors(new Connector[] { connector });
5661
server.start();
5762
}
@@ -64,8 +69,21 @@ void teardown() throws Exception {
6469
}
6570

6671
@Test
72+
void directServerConnectorInstrumentation() throws Exception {
73+
setup(true);
74+
contributesServerConnectorMetrics();
75+
}
76+
77+
@Test
78+
void addToAllConnectorsInstrumentation() throws Exception {
79+
server.setConnectors(new Connector[] { connector });
80+
JettyConnectionMetrics.addToAllConnectors(server, registry);
81+
server.start();
82+
83+
contributesServerConnectorMetrics();
84+
}
85+
6786
void contributesServerConnectorMetrics() throws Exception {
68-
setup();
6987
HttpPost post = new HttpPost("http://localhost:" + connector.getLocalPort());
7088
post.setEntity(new StringEntity("123456"));
7189

@@ -90,12 +108,12 @@ public void lifeCycleStopped(LifeCycle event) {
90108
assertThat(latch.await(10, SECONDS)).isTrue();
91109
assertThat(registry.get("jetty.connections.max").gauge().value()).isEqualTo(2.0);
92110
assertThat(registry.get("jetty.connections.request").tag("type", "server").timer().count()).isEqualTo(2);
93-
assertThat(registry.get("jetty.connections.bytes.in").summary().totalAmount()).isPositive();
111+
assertThat(registry.get("jetty.connections.bytes.in").summary().totalAmount()).isGreaterThan(1);
94112
}
95113

96114
@Test
97115
void contributesClientConnectorMetrics() throws Exception {
98-
setup();
116+
setup(false);
99117
HttpClient httpClient = new HttpClient();
100118
httpClient.setFollowRedirects(false);
101119
httpClient.addBean(new JettyConnectionMetrics(registry));
@@ -118,12 +136,6 @@ public void lifeCycleStopped(LifeCycle event) {
118136
assertThat(latch.await(10, SECONDS)).isTrue();
119137
assertThat(registry.get("jetty.connections.max").gauge().value()).isEqualTo(1.0);
120138
assertThat(registry.get("jetty.connections.request").tag("type", "client").timer().count()).isEqualTo(1);
121-
// assertThat(registry.get("jetty.connections.bytes.out").summary().totalAmount()).isEqualTo(784);
122-
// TODO: explain why there is a difference between what we had before and after
123-
// the change
124-
// assertThat(registry.get("jetty.connections.bytes.out").summary().totalAmount()).isEqualTo(618);
125-
// after the changes
126-
assertThat(registry.get("jetty.connections.bytes.out").summary().totalAmount()).isPositive();
127139
}
128140

129141
@Test

‎micrometer-jetty12/src/test/java/io/micrometer/jetty12/JettyConnectionMetricsTest.java

+24-7
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,9 @@
3939
import static java.util.concurrent.TimeUnit.SECONDS;
4040
import static org.assertj.core.api.Assertions.assertThat;
4141

42+
/**
43+
* Test {@link JettyConnectionMetrics} with Jetty 12.
44+
*/
4245
class JettyConnectionMetricsTest {
4346

4447
private SimpleMeterRegistry registry = new SimpleMeterRegistry(SimpleConfig.DEFAULT, new MockClock());
@@ -49,10 +52,12 @@ class JettyConnectionMetricsTest {
4952

5053
private CloseableHttpClient client = HttpClients.createDefault();
5154

52-
void setup() throws Exception {
53-
JettyConnectionMetrics metrics = new JettyConnectionMetrics(registry);
54-
connector.addBean(metrics);
55-
connector.setNetworkTrafficListener(metrics);
55+
void setup(boolean instrumentServer) throws Exception {
56+
if (instrumentServer) {
57+
JettyConnectionMetrics metrics = new JettyConnectionMetrics(registry, connector);
58+
connector.addBean(metrics);
59+
connector.setNetworkTrafficListener(metrics);
60+
}
5661
server.setConnectors(new Connector[] { connector });
5762
server.start();
5863
}
@@ -65,8 +70,21 @@ void teardown() throws Exception {
6570
}
6671

6772
@Test
73+
void directServerConnectorInstrumentation() throws Exception {
74+
setup(true);
75+
contributesServerConnectorMetrics();
76+
}
77+
78+
@Test
79+
void addToAllConnectorsInstrumentation() throws Exception {
80+
server.setConnectors(new Connector[] { connector });
81+
JettyConnectionMetrics.addToAllConnectors(server, registry);
82+
server.start();
83+
84+
contributesServerConnectorMetrics();
85+
}
86+
6887
void contributesServerConnectorMetrics() throws Exception {
69-
setup();
7088
HttpPost post = new HttpPost("http://localhost:" + connector.getLocalPort());
7189
post.setEntity(new StringEntity("123456"));
7290

@@ -96,7 +114,7 @@ public void lifeCycleStopped(LifeCycle event) {
96114

97115
@Test
98116
void contributesClientConnectorMetrics() throws Exception {
99-
setup();
117+
setup(false);
100118
HttpClient httpClient = new HttpClient();
101119
httpClient.setFollowRedirects(false);
102120
httpClient.addBean(new JettyConnectionMetrics(registry));
@@ -119,7 +137,6 @@ public void lifeCycleStopped(LifeCycle event) {
119137
assertThat(latch.await(10, SECONDS)).isTrue();
120138
assertThat(registry.get("jetty.connections.max").gauge().value()).isEqualTo(1.0);
121139
assertThat(registry.get("jetty.connections.request").tag("type", "client").timer().count()).isEqualTo(1);
122-
assertThat(registry.get("jetty.connections.bytes.out").summary().totalAmount()).isGreaterThan(1);
123140
}
124141

125142
@Test

0 commit comments

Comments
 (0)