Skip to content

Commit

Permalink
Improve JettyConnectionMetrics connection type detection
Browse files Browse the repository at this point in the history
The `HttpConnection` class was relocated to a different package in Jetty 12 which was causing a `NoClassDefFoundError` when doing an instanceof check on it. The goal of it is to determine whether the connection instrumented is a server connection or a client connection. However, it specifically checks against `HttpConnection` and assumes that all server connections will be an instanceof `HttpConnection` and any not an instanceof it are client connections. This is brittle because Jetty supports more implementations of `Connection` on the server side than `HttpConnection` and there could in theory be an arbitrary implementation where we do not know whether it is a server or client connection (it could also be neither).

This instead checks whether the package name contains `server` or `client` or neither. This is admittedly also brittle, but given the known implementations of `Connection` provided by the Jetty project, this pattern generally seems to hold, and it is at least more honest using `UNKNOWN` when our heuristic fails. It also avoids the `NoClassDefFoundError` when using `JettyConnectionMetrics` with Jetty 12.

Resolves micrometer-metricsgh-4324
  • Loading branch information
shakuzen committed Nov 9, 2023
1 parent 214f038 commit d5620fa
Show file tree
Hide file tree
Showing 9 changed files with 178 additions and 21 deletions.
3 changes: 0 additions & 3 deletions dependencies.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,6 @@ def VERSIONS = [
libs.aspectjweaver,
libs.assertj,
libs.awaitility,
libs.jettyClient,
libs.jettyServer,
libs.jettyServlet,
libs.jersey2Server,
libs.jersey2Hk2,
libs.jersey2TestFrameworkInmemory,
Expand Down
12 changes: 8 additions & 4 deletions gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,9 @@ jackson-databind = "2.15.3"
javax-cache = "1.1.1"
javax-inject = "1"
jaxb = "2.3.1"
jetty = "9.4.53.v20231009"
jetty9 = "9.4.53.v20231009"
jetty11 = "11.0.16"
jetty12 = "12.0.3"
jersey2 = "2.41"
jersey3 = "3.0.11"
jmh = "1.37"
Expand Down Expand Up @@ -133,10 +134,13 @@ javax-cacheApi = { module = "javax.cache:cache-api", version.ref = "javax-cache"
javax-inject = { module = "javax.inject:javax.inject", version.ref = "javax-inject" }
javax-servletApi = { module = "javax.servlet:javax.servlet-api", version = "4.0.1" }
jaxbApi = { module = "javax.xml.bind:jaxb-api", version.ref = "jaxb" }
jettyClient = { module = "org.eclipse.jetty:jetty-client", version.ref = "jetty" }
jettyServer = { module = "org.eclipse.jetty:jetty-server", version.ref = "jetty" }
jettyServlet = { module = "org.eclipse.jetty:jetty-servlet", version.ref = "jetty" }
jetty9Client = { module = "org.eclipse.jetty:jetty-client", version.ref = "jetty9" }
jetty9Server = { module = "org.eclipse.jetty:jetty-server", version.ref = "jetty9" }
jetty9Servlet = { module = "org.eclipse.jetty:jetty-servlet", version.ref = "jetty9" }
jetty9http2Server = { module = "org.eclipse.jetty.http2:http2-server", version.ref = "jetty9" }
jetty11Server = { module = "org.eclipse.jetty:jetty-server", version.ref = "jetty11" }
jetty12Server = { module = "org.eclipse.jetty:jetty-server", version.ref = "jetty12" }
jetty12Client = { module = "org.eclipse.jetty:jetty-client", version.ref = "jetty12" }
jersey2Server = { module = "org.glassfish.jersey.core:jersey-server", version.ref = "jersey2" }
jersey2Hk2 = { module = "org.glassfish.jersey.inject:jersey-hk2", version.ref = "jersey2" }
jersey2TestFrameworkInmemory = { module = "org.glassfish.jersey.test-framework.providers:jersey-test-framework-provider-inmemory", version.ref = "jersey2" }
Expand Down
6 changes: 4 additions & 2 deletions micrometer-core/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,10 @@ dependencies {
optionalApi 'org.hibernate:hibernate-entitymanager'

// server runtime monitoring
optionalApi 'org.eclipse.jetty:jetty-server'
optionalApi libs.jetty9Server
// jakarta servlet
optionalApi 'jakarta.servlet:jakarta.servlet-api'
optionalApi 'org.eclipse.jetty:jetty-client'
optionalApi libs.jetty9Client
optionalApi 'org.apache.tomcat.embed:tomcat-embed-core'
optionalApi 'org.glassfish.jersey.core:jersey-server'
optionalApi 'io.grpc:grpc-api'
Expand Down Expand Up @@ -198,6 +198,8 @@ dependencies {
testImplementation 'io.grpc:grpc-inprocess'
testImplementation 'io.grpc:grpc-testing-proto'
testImplementation 'com.squareup.retrofit2:retrofit'

testImplementation libs.jetty9http2Server
}

task shenandoahTest(type: Test) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import io.micrometer.core.instrument.distribution.TimeWindowMax;
import org.eclipse.jetty.io.Connection;
import org.eclipse.jetty.server.Connector;
import org.eclipse.jetty.server.HttpConnection;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.util.component.AbstractLifeCycle;

Expand Down Expand Up @@ -162,10 +161,16 @@ public void onClosed(Connection connection) {
}

if (sample != null) {
String serverOrClient = connection instanceof HttpConnection ? "server" : "client";
String type = "UNKNOWN";
if (connection.getClass().getName().contains("server")) {
type = "server";
}
else if (connection.getClass().getName().contains("client")) {
type = "client";
}
sample.stop(Timer.builder("jetty.connections.request")
.description("Jetty client or server requests")
.tag("type", serverOrClient)
.tag("type", type)
.tags(tags)
.register(registry));
}
Expand Down
6 changes: 1 addition & 5 deletions micrometer-jetty11/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,7 @@ description 'Micrometer instrumentation for Jetty 11'

dependencies {
api project(":micrometer-core")
api(libs.jetty11Server) {
version {
strictly libs.jetty11Server.get().version
}
}
api libs.jetty11Server

testImplementation 'org.junit.jupiter:junit-jupiter'
testImplementation 'org.assertj:assertj-core'
Expand Down
6 changes: 3 additions & 3 deletions micrometer-test/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,9 @@ dependencies {
testImplementation 'org.apache.httpcomponents.client5:httpclient5'
testImplementation 'org.apache.activemq:artemis-junit-5'
testImplementation 'org.apache.activemq:artemis-jakarta-client'
testImplementation 'org.eclipse.jetty:jetty-client'
testImplementation 'org.eclipse.jetty:jetty-server'
testImplementation 'org.eclipse.jetty:jetty-servlet'
testImplementation libs.jetty9Client
testImplementation libs.jetty9Server
testImplementation libs.jetty9Servlet
testImplementation 'org.glassfish.jersey.core:jersey-server'
testImplementation libs.jersey2TestFrameworkJdkHttp
// necessary for Jersey test framework
Expand Down
13 changes: 13 additions & 0 deletions samples/micrometer-samples-jetty12/build.gradle
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
plugins {
id 'java'
}

dependencies {
implementation project(":micrometer-core")

testImplementation libs.jetty12Server
testImplementation libs.jetty12Client
testImplementation libs.httpcomponents.client
testImplementation libs.junitJupiter
testImplementation libs.assertj
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
/*
* Copyright 2023 VMware, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.micrometer.samples.jetty12;

import io.micrometer.core.instrument.MockClock;
import io.micrometer.core.instrument.binder.jetty.JettyConnectionMetrics;
import io.micrometer.core.instrument.simple.SimpleConfig;
import io.micrometer.core.instrument.simple.SimpleMeterRegistry;
import org.apache.http.client.methods.CloseableHttpResponse;
import org.apache.http.client.methods.HttpPost;
import org.apache.http.entity.StringEntity;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.HttpClients;
import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.client.Request;
import org.eclipse.jetty.client.StringRequestContent;
import org.eclipse.jetty.server.Connector;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.util.component.LifeCycle;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Test;

import java.util.concurrent.CountDownLatch;

import static java.util.concurrent.TimeUnit.SECONDS;
import static org.assertj.core.api.Assertions.assertThat;

class JettyConnectionMetricsTest {

private SimpleMeterRegistry registry = new SimpleMeterRegistry(SimpleConfig.DEFAULT, new MockClock());

private Server server = new Server(0);

private ServerConnector connector = new ServerConnector(server);

private CloseableHttpClient client = HttpClients.createDefault();

void setup() throws Exception {
connector.addBean(new JettyConnectionMetrics(registry));
server.setConnectors(new Connector[] { connector });
server.start();
}

@AfterEach
void teardown() throws Exception {
if (server.isRunning()) {
server.stop();
}
}

@Test
void contributesServerConnectorMetrics() throws Exception {
setup();
HttpPost post = new HttpPost("http://localhost:" + connector.getLocalPort());
post.setEntity(new StringEntity("123456"));

try (CloseableHttpResponse ignored = client.execute(post)) {
try (CloseableHttpResponse ignored2 = client.execute(post)) {
assertThat(registry.get("jetty.connections.current").gauge().value()).isEqualTo(2.0);
assertThat(registry.get("jetty.connections.max").gauge().value()).isEqualTo(2.0);
}
}

CountDownLatch latch = new CountDownLatch(1);
connector.addEventListener(new LifeCycle.Listener() {
@Override
public void lifeCycleStopped(LifeCycle event) {
latch.countDown();
}
});
// Convenient way to get Jetty to flush its connections, which is required to
// update the sent/received bytes metrics
server.stop();

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);
}

@Test
void contributesClientConnectorMetrics() throws Exception {
setup();
HttpClient httpClient = new HttpClient();
httpClient.setFollowRedirects(false);
httpClient.addBean(new JettyConnectionMetrics(registry));

CountDownLatch latch = new CountDownLatch(1);
httpClient.addEventListener(new LifeCycle.Listener() {
@Override
public void lifeCycleStopped(LifeCycle event) {
latch.countDown();
}
});

httpClient.start();

Request post = httpClient.POST("http://localhost:" + connector.getLocalPort());
post.body(new StringRequestContent("123456"));
post.send();
httpClient.stop();

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
void passingConnectorAddsConnectorNameTag() {
new JettyConnectionMetrics(registry, connector);

assertThat(registry.get("jetty.connections.messages.in").counter().getId().getTag("connector.name"))
.isEqualTo("unnamed");
}

@Test
void namedConnectorsGetTaggedWithName() {
connector.setName("super-fast-connector");
new JettyConnectionMetrics(registry, connector);

assertThat(registry.get("jetty.connections.messages.in").counter().getId().getTag("connector.name"))
.isEqualTo("super-fast-connector");
}

}
2 changes: 1 addition & 1 deletion settings.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ gradleEnterprise {

include 'micrometer-commons', 'micrometer-core', 'micrometer-observation'

['core', 'boot2', 'boot2-reactive', 'spring-integration', 'hazelcast', 'hazelcast3', 'javalin', 'jersey3'].each { sample ->
['core', 'boot2', 'boot2-reactive', 'spring-integration', 'hazelcast', 'hazelcast3', 'javalin', 'jersey3', 'jetty12'].each { sample ->
include "micrometer-samples-$sample"
project(":micrometer-samples-$sample").projectDir = new File(rootProject.projectDir, "samples/micrometer-samples-$sample")
}
Expand Down

0 comments on commit d5620fa

Please sign in to comment.