Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ muzzle {
javaVersion = "17"
assertInverse = true
}
pass {
group = "org.eclipse.jetty.websocket"
module = "jetty-websocket-core-client"
versions = "[12,)"
javaVersion = "17"
}
}

tracerJava {
Expand All @@ -18,10 +24,6 @@ tracerJava {

addTestSuiteForDir('latestDepTest', 'test')

tasks.named("compileMain_java17Java", JavaCompile) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove redundant settings. Additionally, I scanned the project for similar redundant settings and cleaned them up in #12689

configureCompiler(it, JavaVersion.VERSION_17)
}

configurations.matching { it.name.startsWith('test') || it.name.startsWith('latestDepTest') }.configureEach {
it.resolutionStrategy {
force group: 'org.slf4j', name: 'slf4j-api', version: libs.versions.slf4j.get()
Expand All @@ -30,6 +32,7 @@ configurations.matching { it.name.startsWith('test') || it.name.startsWith('late

dependencies {
main_java17CompileOnly group: 'org.eclipse.jetty', name: 'jetty-client', version: '12.0.0'
main_java17CompileOnly group: 'org.eclipse.jetty.websocket', name: 'jetty-websocket-core-client', version: '12.0.0'
// to test conflicts
testImplementation(project(':dd-java-agent:instrumentation:jetty:jetty-client:jetty-client-9.1'))
testImplementation(project(':dd-java-agent:instrumentation:jetty:jetty-client:jetty-client-10.0'))
Expand All @@ -40,5 +43,10 @@ dependencies {
}
testImplementation project(':dd-java-agent:instrumentation:jetty:jetty-util-9.4.31')
testImplementation group: 'org.eclipse.jetty', name: 'jetty-client', version: '12.0.0'
testImplementation 'org.eclipse.jetty.websocket:jetty-websocket-jetty-client:12.0.0'
testImplementation 'org.eclipse.jetty.websocket:jetty-websocket-jetty-server:12.0.0'
testRuntimeOnly project(':dd-java-agent:instrumentation:jetty:jetty-server:jetty-server-12.0')
latestDepTestImplementation group: 'org.eclipse.jetty', name: 'jetty-client', version: '12.+'
latestDepTestImplementation 'org.eclipse.jetty.websocket:jetty-websocket-jetty-client:12.+'
latestDepTestImplementation 'org.eclipse.jetty.websocket:jetty-websocket-jetty-server:12.+'
}
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,20 @@ org.codenarc:CodeNarc:3.7.0=codenarc
org.dom4j:dom4j:2.2.0=spotbugs
org.eclipse.jetty.compression:jetty-compression-common:12.1.13=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath
org.eclipse.jetty.compression:jetty-compression-gzip:12.1.13=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath
org.eclipse.jetty.websocket:jetty-websocket-core-client:12.0.0=main_java17CompileClasspath,testCompileClasspath,testRuntimeClasspath
org.eclipse.jetty.websocket:jetty-websocket-core-client:12.1.13=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath
org.eclipse.jetty.websocket:jetty-websocket-core-common:12.0.0=main_java17CompileClasspath,testCompileClasspath,testRuntimeClasspath
org.eclipse.jetty.websocket:jetty-websocket-core-common:12.1.13=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath
org.eclipse.jetty.websocket:jetty-websocket-core-server:12.0.0=testCompileClasspath,testRuntimeClasspath
org.eclipse.jetty.websocket:jetty-websocket-core-server:12.1.13=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath
org.eclipse.jetty.websocket:jetty-websocket-jetty-api:12.0.0=testCompileClasspath,testRuntimeClasspath
org.eclipse.jetty.websocket:jetty-websocket-jetty-api:12.1.13=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath
org.eclipse.jetty.websocket:jetty-websocket-jetty-client:12.0.0=testCompileClasspath,testRuntimeClasspath
org.eclipse.jetty.websocket:jetty-websocket-jetty-client:12.1.13=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath
org.eclipse.jetty.websocket:jetty-websocket-jetty-common:12.0.0=testCompileClasspath,testRuntimeClasspath
org.eclipse.jetty.websocket:jetty-websocket-jetty-common:12.1.13=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath
org.eclipse.jetty.websocket:jetty-websocket-jetty-server:12.0.0=testCompileClasspath,testRuntimeClasspath
org.eclipse.jetty.websocket:jetty-websocket-jetty-server:12.1.13=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath
org.eclipse.jetty:jetty-alpn-client:12.0.0=main_java17CompileClasspath,testCompileClasspath,testRuntimeClasspath
org.eclipse.jetty:jetty-alpn-client:12.1.13=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath
org.eclipse.jetty:jetty-client:12.0.0=main_java17CompileClasspath,testCompileClasspath,testRuntimeClasspath
Expand All @@ -90,6 +104,8 @@ org.eclipse.jetty:jetty-http:12.0.0=main_java17CompileClasspath,testCompileClass
org.eclipse.jetty:jetty-http:12.1.13=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath
org.eclipse.jetty:jetty-io:12.0.0=main_java17CompileClasspath,testCompileClasspath,testRuntimeClasspath
org.eclipse.jetty:jetty-io:12.1.13=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath
org.eclipse.jetty:jetty-server:12.0.0=testCompileClasspath,testRuntimeClasspath
org.eclipse.jetty:jetty-server:12.1.13=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath
org.eclipse.jetty:jetty-util:12.0.0=main_java17CompileClasspath,testCompileClasspath,testRuntimeClasspath
org.eclipse.jetty:jetty-util:12.1.13=latestDepTestCompileClasspath,latestDepTestRuntimeClasspath
org.gmetrics:GMetrics:2.1.0=codenarc
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ public JettyHttpClientInstrumentation() {
super("jetty-client");
}

@Override
public String muzzleDirective() {
return "jetty-client";
}

@Override
public String instrumentedType() {
return "org.eclipse.jetty.client.transport.HttpRequest";
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
package datadog.trace.instrumentation.jetty_client12;

import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan;
import static datadog.trace.instrumentation.jetty_client12.JettyClientDecorator.DECORATE;
import static java.util.Collections.singletonMap;
import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
import static net.bytebuddy.matcher.ElementMatchers.takesArguments;

import com.google.auto.service.AutoService;
import datadog.context.ContextScope;
import datadog.trace.agent.tooling.Instrumenter;
import datadog.trace.agent.tooling.InstrumenterModule;
import datadog.trace.bootstrap.InstrumentationContext;
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
import java.util.Map;
import net.bytebuddy.asm.Advice;
import org.eclipse.jetty.client.Request;
import org.eclipse.jetty.client.Response;
import org.eclipse.jetty.io.EndPoint;
import org.eclipse.jetty.websocket.core.client.CoreClientUpgradeRequest;

@AutoService(InstrumenterModule.class)
public class JettyWebSocketUpgradeInstrumentation extends InstrumenterModule.Tracing
implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice {
public JettyWebSocketUpgradeInstrumentation() {
super("jetty-client");
}

@Override
public String muzzleDirective() {
return "jetty-websocket-core-client";
}

@Override
public String instrumentedType() {
return "org.eclipse.jetty.websocket.core.client.CoreClientUpgradeRequest";
}

@Override
public String[] helperClassNames() {
return new String[] {packageName + ".JettyClientDecorator"};
}

@Override
public Map<String, String> contextStore() {
return singletonMap("org.eclipse.jetty.client.Request", AgentSpan.class.getName());
}

@Override
public void methodAdvice(MethodTransformer transformer) {
transformer.applyAdvice(
named("upgrade")
.and(takesArguments(2))
.and(takesArgument(0, named("org.eclipse.jetty.client.Response")))
.and(takesArgument(1, named("org.eclipse.jetty.io.EndPoint"))),
getClass().getName() + "$WebSocketUpgradeAdvice");
}

public static class WebSocketUpgradeAdvice {
@Advice.OnMethodEnter(suppress = Throwable.class)
public static ContextScope beforeUpgrade(@Advice.Argument(0) Response response) {
AgentSpan span =
InstrumentationContext.get(Request.class, AgentSpan.class).get(response.getRequest());
return span == null ? null : activateSpan(span);
}

@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
public static void afterUpgrade(
@Advice.Argument(0) Response response,
@Advice.Enter ContextScope scope,
@Advice.Thrown Throwable failure) {
AgentSpan span =
InstrumentationContext.get(Request.class, AgentSpan.class).get(response.getRequest());
try {
if (span != null && failure == null) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve failures swallowed by Jetty's upgrade method

When EndPoint.upgrade() fails after the HTTP 101 response, Jetty catches that throwable inside CoreClientUpgradeRequest.upgrade() and completes its session future exceptionally, so @Advice.Thrown is still null here; Jetty also marks the request upgraded, causing its response-completion path to skip listeners. The Jetty 12.0 implementation therefore makes this branch decorate and finish the handshake as a successful 101 even though connect() fails. The advice needs to observe that exceptional completion rather than treating every normal method return as a successful upgrade.

Useful? React with 👍 / 👎.

// Successful upgrades bypass the request's response completion listeners.
DECORATE.onResponse(span, response);
DECORATE.beforeFinish(span);
}
} finally {
if (scope != null) {
scope.close();
}
if (span != null && failure == null) {
span.finish();
}
}
}

/**
* Lets Muzzle fail CI if the upgrade method is removed or its signature changes, instead of
* silently skipping instrumentation.
*/
private void muzzleCheck(
CoreClientUpgradeRequest request, Response response, EndPoint endPoint) {
request.upgrade(response, endPoint);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import static datadog.trace.agent.test.assertions.SpanMatcher.span;
import static datadog.trace.agent.test.assertions.TraceMatcher.trace;
import static java.util.concurrent.TimeUnit.SECONDS;
import static java.util.regex.Pattern.compile;
import static java.util.regex.Pattern.quote;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;

import datadog.trace.agent.test.AbstractInstrumentationTest;
import datadog.trace.api.DDSpanTypes;
import datadog.trace.core.DDSpan;
import java.net.URI;
import java.util.List;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.server.handler.ContextHandler;
import org.eclipse.jetty.websocket.api.Session;
import org.eclipse.jetty.websocket.client.WebSocketClient;
import org.eclipse.jetty.websocket.server.WebSocketUpgradeHandler;
import org.junit.jupiter.api.Test;

class JettyWebSocketUpgradeTest extends AbstractInstrumentationTest {
@Test
void httpClientSpanFinishesWhenWebSocketUpgradeSucceeds() throws Exception {
Server server = new Server(0);
WebSocketClient client = new WebSocketClient();
try {
ContextHandler context = new ContextHandler("/");
server.setHandler(context);
context.setHandler(
WebSocketUpgradeHandler.from(server, context)
.configure(
container ->
container.addMapping(
"/upgrade", (request, response, callback) -> new Endpoint())));
server.start();
client.start();
URI uri =
URI.create(
"ws://localhost:"
+ ((ServerConnector) server.getConnectors()[0]).getLocalPort()
+ "/upgrade");

Session session = client.connect(new Endpoint(), uri).get(5, SECONDS);

assertTrue(session.isOpen());
// The HTTP handshake must be reported before the WebSocket connection closes.
assertTraces(
trace(
span()
.operationName(compile(quote("http.request")))
.resourceName(compile(quote("GET /upgrade")))
.type(DDSpanTypes.HTTP_CLIENT)
.root()
.error(false)),
trace(span().type(DDSpanTypes.HTTP_SERVER).error(false)));
DDSpan handshake =
writer.stream()
.flatMap(List::stream)
.filter(s -> "client".equals(s.getTag("span.kind")))
.findFirst()
.orElseThrow(() -> new AssertionError("Missing client handshake span"));
assertEquals("jetty-client", handshake.getTag("component").toString());
assertEquals("client", handshake.getTag("span.kind"));
assertEquals(101, handshake.getTag("http.status_code"));
} finally {
try {
client.stop();
} finally {
server.stop();
}
}
}

public static class Endpoint implements Session.Listener.AutoDemanding {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,13 @@ plugins {
}

muzzle {
pass {
name = 'jetty-websocket-12-native'
group = 'org.eclipse.jetty.websocket'
module = 'jetty-websocket-jetty-server'
versions = "[12,12.0.17]"
javaVersion = "17"
}
pass {
name = 'jetty-websocket-12ee8'
group = 'org.eclipse.jetty.ee8.websocket'
Expand Down Expand Up @@ -38,17 +45,31 @@ addTestSuiteForDir("latestDepTest", "test")
}
}

["compileTestJava", "compileLatestDepTestJava"].each { name ->
tasks.named(name, JavaCompile) {
configureCompiler(it, JavaVersion.VERSION_17)
}
}

dependencies {
testImplementation libs.bundles.mockito
compileOnly 'org.eclipse.jetty.websocket:jetty-websocket-jetty-common:12.0.0'
implementation project(":dd-java-agent:instrumentation:websocket:jetty-websocket:jetty-websocket-10.0")
testImplementation group: 'org.eclipse.jetty.ee8.websocket', name: 'jetty-ee8-websocket-javax-server', version: '12.0.0'
testImplementation group: 'org.eclipse.jetty.ee9.websocket', name: 'jetty-ee9-websocket-jakarta-server', version: '12.0.0'
testImplementation group: 'org.eclipse.jetty.ee10.websocket', name: 'jetty-ee10-websocket-jakarta-server', version: '12.0.0'
testImplementation 'org.eclipse.jetty.websocket:jetty-websocket-jetty-server:12.0.0'
testImplementation 'org.eclipse.jetty.websocket:jetty-websocket-jetty-client:12.0.0'
//TODO: jetty-12.1.0 is still alpha but wraps MethodHandle class into a MethodHolder class.
// Today that is not stable but we'll need to port those advices to support that once the code base will be a bit more stable
latestDepTestImplementation group: 'org.eclipse.jetty.ee8.websocket', name: 'jetty-ee8-websocket-javax-server', version: '12.0.17'
latestDepTestImplementation group: 'org.eclipse.jetty.ee9.websocket', name: 'jetty-ee9-websocket-jakarta-server', version: '12.0.17'
latestDepTestImplementation group: 'org.eclipse.jetty.ee10.websocket', name: 'jetty-ee10-websocket-jakarta-server', version: '12.0.17'
latestDepTestImplementation 'org.eclipse.jetty.websocket:jetty-websocket-jetty-server:12.0.17'
latestDepTestImplementation 'org.eclipse.jetty.websocket:jetty-websocket-jetty-client:12.0.17'

testRuntimeOnly project(":dd-java-agent:instrumentation:jetty:jetty-client:jetty-client-12.0")
testRuntimeOnly project(":dd-java-agent:instrumentation:jetty:jetty-server:jetty-server-12.0")
testRuntimeOnly project(":dd-java-agent:instrumentation:websocket:jetty-websocket:jetty-websocket-10.0")
testRuntimeOnly project(":dd-java-agent:instrumentation:websocket:jetty-websocket:jetty-websocket-11.0")
testRuntimeOnly project(":dd-java-agent:instrumentation:websocket:javax-websocket-1.0")
Expand Down
Loading
Loading