Skip to content

Commit

Permalink
Starts recording Jetty metrics through network listeners
Browse files Browse the repository at this point in the history
before this change we were dumping in / out bytes from the connection upon the connection close which could result in big traffic whenever the connection got closed
after this change we're measuring the bytes through NetworkTrafficListener that hooks in whenever actual in / out bytes are being processed

fixes gh-3873
  • Loading branch information
marcingrzejszczak committed Dec 29, 2023
1 parent 15755a1 commit 9d5a65b
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,12 @@
import io.micrometer.core.instrument.binder.BaseUnits;
import io.micrometer.core.instrument.distribution.DistributionStatisticConfig;
import io.micrometer.core.instrument.distribution.TimeWindowMax;

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.HttpConnection;
import org.eclipse.jetty.server.Server;
Expand Down Expand Up @@ -48,7 +53,7 @@
* @author Jon Schneider
* @since 1.4.0
*/
public class JettyConnectionMetrics extends AbstractLifeCycle implements Connection.Listener {
public class JettyConnectionMetrics extends AbstractLifeCycle implements Connection.Listener, NetworkTrafficListener {

private final MeterRegistry registry;

Expand Down Expand Up @@ -169,12 +174,18 @@ public void onClosed(Connection connection) {
.tags(tags)
.register(registry));
}

messagesIn.increment(connection.getMessagesIn());
messagesOut.increment(connection.getMessagesOut());
}

bytesIn.record(connection.getBytesIn());
bytesOut.record(connection.getBytesOut());
@Override
public void incoming(Socket socket, ByteBuffer bytes) {
bytesIn.record(bytes.limit());
}

@Override
public void outgoing(Socket socket, ByteBuffer bytes) {
bytesOut.record(bytes.limit());
}

public static void addToAllConnectors(Server server, MeterRegistry registry, Iterable<Tag> tags) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.eclipse.jetty.client.api.Request;
import org.eclipse.jetty.client.util.StringContentProvider;
import org.eclipse.jetty.server.Connector;
import org.eclipse.jetty.server.NetworkTrafficServerConnector;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.util.component.LifeCycle;
Expand All @@ -44,12 +45,14 @@ class JettyConnectionMetricsTest {

private Server server = new Server(0);

private ServerConnector connector = new ServerConnector(server);
private NetworkTrafficServerConnector connector = new NetworkTrafficServerConnector(server);

private CloseableHttpClient client = HttpClients.createDefault();

void setup() throws Exception {
connector.addBean(new JettyConnectionMetrics(registry));
JettyConnectionMetrics metrics = new JettyConnectionMetrics(registry);
connector.addBean(metrics);
connector.addNetworkTrafficListener(metrics);
server.setConnectors(new Connector[] { connector });
server.start();
}
Expand Down Expand Up @@ -88,7 +91,7 @@ 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()).isGreaterThan(1);
assertThat(registry.get("jetty.connections.bytes.in").summary().totalAmount()).isPositive();
}

@Test
Expand Down Expand Up @@ -116,7 +119,12 @@ 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);
// 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

0 comments on commit 9d5a65b

Please sign in to comment.