From ea0a59452466c9fe9631107741a8511e968b0b0b Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Wed, 10 Jul 2024 18:15:11 +0200 Subject: [PATCH 1/3] Support jetty client 12 --- .../jetty-client-12.0/build.gradle | 42 ++++++ ...FutureResponseListenerInstrumentation.java | 45 +++++++ .../JettyHttpClientInstrumentation.java | 70 ++++++++++ .../jetty_client12/CallbackWrapper.java | 127 ++++++++++++++++++ .../jetty_client12/HeadersInjectAdapter.java | 14 ++ .../jetty_client12/JettyClientDecorator.java | 48 +++++++ .../jetty_client12/LinkListenerAdvice.java | 14 ++ .../jetty_client12/RequestCreateAdvice.java | 16 +++ .../jetty_client12/SendAdvice.java | 40 ++++++ .../SpanFinishingCompleteListener.java | 29 ++++ .../jetty_client12/WrapListenerAdvice.java | 25 ++++ .../src/test/groovy/JettyClientTest.groovy | 109 +++++++++++++++ settings.gradle | 1 + 13 files changed, 580 insertions(+) create mode 100644 dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/build.gradle create mode 100644 dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java/datadog/trace/instrumentation/jetty_client12/FutureResponseListenerInstrumentation.java create mode 100644 dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java/datadog/trace/instrumentation/jetty_client12/JettyHttpClientInstrumentation.java create mode 100644 dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/CallbackWrapper.java create mode 100644 dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/HeadersInjectAdapter.java create mode 100644 dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/JettyClientDecorator.java create mode 100644 dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/LinkListenerAdvice.java create mode 100644 dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/RequestCreateAdvice.java create mode 100644 dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/SendAdvice.java create mode 100644 dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/SpanFinishingCompleteListener.java create mode 100644 dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/WrapListenerAdvice.java create mode 100644 dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/test/groovy/JettyClientTest.groovy diff --git a/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/build.gradle b/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/build.gradle new file mode 100644 index 00000000000..3df7a537917 --- /dev/null +++ b/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/build.gradle @@ -0,0 +1,42 @@ +ext { + minJavaVersionForTests = JavaVersion.VERSION_17 +} +muzzle { + pass { + group = "org.eclipse.jetty" + module = "jetty-client" + versions = "[12,)" + javaVersion = "17" + assertInverse = true + } +} + +apply from: "$rootDir/gradle/java.gradle" + +addTestSuiteForDir('latestDepTest', 'test') + +compileMain_java17Java.configure { + setJavaVersion(it, 17) +} + +configurations.matching({ it.name.startsWith('test') || it.name.startsWith('latestDepTest') }).each({ + it.resolutionStrategy { + force group: 'org.slf4j', name: 'slf4j-api', version: libs.versions.slf4j.get() + } +}) + +dependencies { + main_java17CompileOnly group: 'org.eclipse.jetty', name: 'jetty-client', version: '12.0.0' + + //because contains some instrumentation that still apply + testImplementation(project(':dd-java-agent:instrumentation:jetty-client:jetty-client-9.1')) + // to test conflicts + testImplementation(project(':dd-java-agent:instrumentation:jetty-client:jetty-client-10.0')) + testImplementation(project(path:':dd-java-agent:testing', configuration:'shadow')) { + // explicitly declared below. + exclude group: 'org.eclipse.jetty' + } + testImplementation project(':dd-java-agent:instrumentation:jetty-util') + testImplementation group: 'org.eclipse.jetty', name: 'jetty-client', version: '12.0.0' + latestDepTestImplementation group: 'org.eclipse.jetty', name: 'jetty-client', version: '12.+' +} diff --git a/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java/datadog/trace/instrumentation/jetty_client12/FutureResponseListenerInstrumentation.java b/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java/datadog/trace/instrumentation/jetty_client12/FutureResponseListenerInstrumentation.java new file mode 100644 index 00000000000..7b664ee29be --- /dev/null +++ b/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java/datadog/trace/instrumentation/jetty_client12/FutureResponseListenerInstrumentation.java @@ -0,0 +1,45 @@ +package datadog.trace.instrumentation.jetty_client12; + +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.isConstructor; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.agent.tooling.InstrumenterModule; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import java.util.HashMap; +import java.util.Map; + +@AutoService(InstrumenterModule.class) +public final class FutureResponseListenerInstrumentation extends InstrumenterModule.Tracing + implements Instrumenter.ForSingleType { + + public FutureResponseListenerInstrumentation() { + super("jetty-client"); + } + + @Override + public String instrumentedType() { + return "org.eclipse.jetty.client.FutureResponseListener"; + } + + @Override + public Map contextStore() { + Map contextStore = new HashMap<>(4); + contextStore.put("org.eclipse.jetty.client.Request", AgentSpan.class.getName()); + contextStore.put( + "org.eclipse.jetty.client.FutureResponseListener", "org.eclipse.jetty.client.Request"); + return contextStore; + } + + @Override + public void methodAdvice(MethodTransformer transformer) { + transformer.applyAdvice( + isConstructor() + .and( + takesArgument(0, named("org.eclipse.jetty.client.Request")).and(takesArguments(2))), + packageName + ".LinkListenerAdvice"); + } +} diff --git a/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java/datadog/trace/instrumentation/jetty_client12/JettyHttpClientInstrumentation.java b/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java/datadog/trace/instrumentation/jetty_client12/JettyHttpClientInstrumentation.java new file mode 100644 index 00000000000..e305ff71965 --- /dev/null +++ b/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java/datadog/trace/instrumentation/jetty_client12/JettyHttpClientInstrumentation.java @@ -0,0 +1,70 @@ +package datadog.trace.instrumentation.jetty_client12; + +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.namedOneOf; +import static datadog.trace.bootstrap.instrumentation.java.concurrent.ExcludeFilter.ExcludeType.RUNNABLE; +import static java.util.Collections.singletonList; +import static java.util.Collections.singletonMap; +import static net.bytebuddy.matcher.ElementMatchers.isConstructor; +import static net.bytebuddy.matcher.ElementMatchers.isMethod; +import static net.bytebuddy.matcher.ElementMatchers.isPublic; +import static net.bytebuddy.matcher.ElementMatchers.takesArgument; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.ExcludeFilterProvider; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.agent.tooling.InstrumenterModule; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.java.concurrent.ExcludeFilter; +import java.util.Collection; +import java.util.Map; + +@AutoService(InstrumenterModule.class) +public class JettyHttpClientInstrumentation extends InstrumenterModule.Tracing + implements Instrumenter.ForSingleType, ExcludeFilterProvider { + public JettyHttpClientInstrumentation() { + super("jetty-client"); + } + + @Override + public String instrumentedType() { + return "org.eclipse.jetty.client.transport.HttpRequest"; + } + + @Override + public String[] helperClassNames() { + return new String[] { + packageName + ".JettyClientDecorator", + packageName + ".HeadersInjectAdapter", + packageName + ".SpanFinishingCompleteListener", + packageName + ".CallbackWrapper", + }; + } + + @Override + public Map contextStore() { + return singletonMap("org.eclipse.jetty.client.Request", AgentSpan.class.getName()); + } + + @Override + public void methodAdvice(MethodTransformer transformer) { + transformer.applyAdvice(isConstructor(), packageName + ".RequestCreateAdvice"); + transformer.applyAdvice( + isMethod() + .and(named("send")) + .and(takesArgument(0, named("org.eclipse.jetty.client.Response$CompleteListener"))), + packageName + ".SendAdvice"); + transformer.applyAdvice( + isMethod() + .and(isPublic()) + .and(namedOneOf("listener", "onSuccess", "onFailure", "onComplete")) + .and(takesArguments(1)), + packageName + ".WrapListenerAdvice"); + } + + @Override + public Map> excludedClasses() { + return singletonMap(RUNNABLE, singletonList("org.eclipse.jetty.util.SocketAddressResolver$1")); + } +} diff --git a/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/CallbackWrapper.java b/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/CallbackWrapper.java new file mode 100644 index 00000000000..dd17a887f3a --- /dev/null +++ b/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/CallbackWrapper.java @@ -0,0 +1,127 @@ +package datadog.trace.instrumentation.jetty_client12; + +import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan; + +import datadog.trace.bootstrap.instrumentation.api.AgentScope; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import org.eclipse.jetty.client.Request; +import org.eclipse.jetty.client.Response; +import org.eclipse.jetty.client.Result; + +public class CallbackWrapper implements Response.Listener, Request.Listener { + + private final AgentSpan parent; + private final AgentSpan span; + private final Object delegate; + + public CallbackWrapper(AgentSpan parent, AgentSpan span, Object delegate) { + this.parent = parent; + this.span = span; + this.delegate = delegate; + } + + @Override + public void onBegin(Response response) { + if (delegate instanceof Response.BeginListener) { + try (AgentScope scope = activate(span)) { + ((Response.BeginListener) delegate).onBegin(response); + } + } + } + + @Override + public void onComplete(Result result) { + if (delegate instanceof Response.CompleteListener) { + // this probably does the wrong thing, but preserves old behaviour and is consistent + // with other http clients with completion callback registration + try (AgentScope scope = activate(parent)) { + ((Response.CompleteListener) delegate).onComplete(result); + } + } + } + + @Override + public void onFailure(Response response, Throwable failure) { + if (delegate instanceof Response.FailureListener) { + try (AgentScope scope = activate(span)) { + ((Response.FailureListener) delegate).onFailure(response, failure); + } + } + } + + @Override + public void onHeaders(Response response) { + if (delegate instanceof Response.HeadersListener) { + try (AgentScope scope = activate(span)) { + ((Response.HeadersListener) delegate).onHeaders(response); + } + } + } + + @Override + public void onSuccess(Response response) { + if (delegate instanceof Response.SuccessListener) { + try (AgentScope scope = activate(span)) { + ((Response.SuccessListener) delegate).onSuccess(response); + } + } + } + + @Override + public void onBegin(Request request) { + if (delegate instanceof Request.BeginListener) { + try (AgentScope scope = activate(span)) { + ((Request.SuccessListener) delegate).onSuccess(request); + } + } + } + + @Override + public void onCommit(Request request) { + if (delegate instanceof Request.CommitListener) { + try (AgentScope scope = activate(span)) { + ((Request.CommitListener) delegate).onCommit(request); + } + } + } + + @Override + public void onFailure(Request request, Throwable failure) { + if (delegate instanceof Request.FailureListener) { + try (AgentScope scope = activate(span)) { + ((Request.FailureListener) delegate).onFailure(request, failure); + } + } + } + + @Override + public void onHeaders(Request request) { + if (delegate instanceof Request.HeadersListener) { + try (AgentScope scope = activate(span)) { + ((Request.HeadersListener) delegate).onHeaders(request); + } + } + } + + @Override + public void onQueued(Request request) { + if (delegate instanceof Request.QueuedListener) { + try (AgentScope scope = activate(span)) { + ((Request.QueuedListener) delegate).onQueued(request); + } + } + } + + @Override + public void onSuccess(Request request) { + if (delegate instanceof Request.SuccessListener) { + try (AgentScope scope = activate(span)) { + ((Request.SuccessListener) delegate).onSuccess(request); + } + } + } + + private AgentScope activate(AgentSpan span) { + return null == span ? null : activateSpan(span); + } +} diff --git a/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/HeadersInjectAdapter.java b/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/HeadersInjectAdapter.java new file mode 100644 index 00000000000..94a16ded1bb --- /dev/null +++ b/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/HeadersInjectAdapter.java @@ -0,0 +1,14 @@ +package datadog.trace.instrumentation.jetty_client12; + +import datadog.trace.bootstrap.instrumentation.api.AgentPropagation; +import org.eclipse.jetty.client.Request; + +public class HeadersInjectAdapter implements AgentPropagation.Setter { + + public static final HeadersInjectAdapter SETTER = new HeadersInjectAdapter(); + + @Override + public void set(final Request carrier, final String key, final String value) { + carrier.headers(httpFields -> httpFields.add(key, value)); + } +} diff --git a/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/JettyClientDecorator.java b/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/JettyClientDecorator.java new file mode 100644 index 00000000000..28ef7bb44dd --- /dev/null +++ b/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/JettyClientDecorator.java @@ -0,0 +1,48 @@ +package datadog.trace.instrumentation.jetty_client12; + +import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString; +import datadog.trace.bootstrap.instrumentation.decorator.HttpClientDecorator; +import java.net.URI; +import org.eclipse.jetty.client.Request; +import org.eclipse.jetty.client.Response; + +public class JettyClientDecorator extends HttpClientDecorator { + public static final CharSequence JETTY_CLIENT = UTF8BytesString.create("jetty-client"); + public static final JettyClientDecorator DECORATE = new JettyClientDecorator(); + public static final CharSequence HTTP_REQUEST = UTF8BytesString.create(DECORATE.operationName()); + + @Override + protected String[] instrumentationNames() { + return new String[] {"jetty-client"}; + } + + @Override + protected CharSequence component() { + return JETTY_CLIENT; + } + + @Override + protected String method(final Request httpRequest) { + return httpRequest.getMethod(); + } + + @Override + protected URI url(final Request httpRequest) { + return httpRequest.getURI(); + } + + @Override + protected int status(final Response httpResponse) { + return httpResponse.getStatus(); + } + + @Override + protected String getRequestHeader(Request request, String headerName) { + return request.getHeaders().get(headerName); + } + + @Override + protected String getResponseHeader(Response response, String headerName) { + return response.getHeaders().get(headerName); + } +} diff --git a/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/LinkListenerAdvice.java b/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/LinkListenerAdvice.java new file mode 100644 index 00000000000..fc8da11d15c --- /dev/null +++ b/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/LinkListenerAdvice.java @@ -0,0 +1,14 @@ +package datadog.trace.instrumentation.jetty_client12; + +import datadog.trace.bootstrap.InstrumentationContext; +import net.bytebuddy.asm.Advice; +import org.eclipse.jetty.client.FutureResponseListener; +import org.eclipse.jetty.client.Request; + +public class LinkListenerAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + public static void link( + @Advice.This FutureResponseListener listener, @Advice.Argument(0) Request request) { + InstrumentationContext.get(FutureResponseListener.class, Request.class).put(listener, request); + } +} diff --git a/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/RequestCreateAdvice.java b/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/RequestCreateAdvice.java new file mode 100644 index 00000000000..a0e54017e6f --- /dev/null +++ b/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/RequestCreateAdvice.java @@ -0,0 +1,16 @@ +package datadog.trace.instrumentation.jetty_client12; + +import datadog.trace.bootstrap.InstrumentationContext; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import net.bytebuddy.asm.Advice; +import org.eclipse.jetty.client.Request; +import org.eclipse.jetty.client.transport.HttpRequest; + +public class RequestCreateAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + public static void afterCreate(@Advice.This HttpRequest self) { + self.onComplete( + new SpanFinishingCompleteListener( + InstrumentationContext.get(Request.class, AgentSpan.class))); + } +} diff --git a/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/SendAdvice.java b/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/SendAdvice.java new file mode 100644 index 00000000000..2e09929c4a6 --- /dev/null +++ b/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/SendAdvice.java @@ -0,0 +1,40 @@ +package datadog.trace.instrumentation.jetty_client12; + +import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan; +import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.propagate; +import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan; +import static datadog.trace.instrumentation.jetty_client12.HeadersInjectAdapter.SETTER; + +import datadog.trace.bootstrap.InstrumentationContext; +import datadog.trace.bootstrap.instrumentation.api.AgentScope; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import datadog.trace.bootstrap.instrumentation.decorator.HttpClientDecorator; +import net.bytebuddy.asm.Advice; +import org.eclipse.jetty.client.Request; +import org.eclipse.jetty.client.transport.HttpRequest; + +public class SendAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static AgentScope methodEnter(@Advice.This final HttpRequest request) { + AgentSpan span = startSpan(JettyClientDecorator.HTTP_REQUEST); + InstrumentationContext.get(Request.class, AgentSpan.class).put(request, span); + JettyClientDecorator.DECORATE.afterStart(span); + JettyClientDecorator.DECORATE.onRequest(span, request); + propagate().inject(span, request, SETTER); + propagate() + .injectPathwayContext(span, request, SETTER, HttpClientDecorator.CLIENT_PATHWAY_EDGE_TAGS); + return activateSpan(span); + } + + @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static void methodExit( + @Advice.Enter final AgentScope scope, @Advice.Thrown final Throwable throwable) { + try (scope) { + if (throwable != null) { + JettyClientDecorator.DECORATE.onError(scope, throwable); + JettyClientDecorator.DECORATE.beforeFinish(scope); + scope.span().finish(); + } + } + } +} diff --git a/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/SpanFinishingCompleteListener.java b/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/SpanFinishingCompleteListener.java new file mode 100644 index 00000000000..088a782a9e4 --- /dev/null +++ b/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/SpanFinishingCompleteListener.java @@ -0,0 +1,29 @@ +package datadog.trace.instrumentation.jetty_client12; + +import datadog.trace.bootstrap.ContextStore; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import org.eclipse.jetty.client.Request; +import org.eclipse.jetty.client.Response; +import org.eclipse.jetty.client.Result; + +public class SpanFinishingCompleteListener implements Response.CompleteListener { + private final ContextStore contextStore; + + public SpanFinishingCompleteListener(ContextStore contextStore) { + this.contextStore = contextStore; + } + + @Override + public void onComplete(Result result) { + AgentSpan span = contextStore.get(result.getRequest()); + if (span == null) { + return; + } + if (result.getResponse().getStatus() <= 0) { + JettyClientDecorator.DECORATE.onError(span, result.getFailure()); + } + JettyClientDecorator.DECORATE.onResponse(span, result.getResponse()); + JettyClientDecorator.DECORATE.beforeFinish(span); + span.finish(); + } +} diff --git a/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/WrapListenerAdvice.java b/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/WrapListenerAdvice.java new file mode 100644 index 00000000000..c8062aeba55 --- /dev/null +++ b/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/WrapListenerAdvice.java @@ -0,0 +1,25 @@ +package datadog.trace.instrumentation.jetty_client12; + +import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeSpan; + +import datadog.trace.bootstrap.InstrumentationContext; +import datadog.trace.bootstrap.instrumentation.api.AgentSpan; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.implementation.bytecode.assign.Assigner; +import org.eclipse.jetty.client.Request; + +public class WrapListenerAdvice { + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void methodEnter( + @Advice.This Request request, + @Advice.Argument(value = 0, readOnly = false, typing = Assigner.Typing.DYNAMIC) + Object listener) { + if (!(listener instanceof CallbackWrapper)) { + listener = + new CallbackWrapper( + activeSpan(), + InstrumentationContext.get(Request.class, AgentSpan.class).get(request), + listener); + } + } +} diff --git a/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/test/groovy/JettyClientTest.groovy b/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/test/groovy/JettyClientTest.groovy new file mode 100644 index 00000000000..8c34e9da103 --- /dev/null +++ b/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/test/groovy/JettyClientTest.groovy @@ -0,0 +1,109 @@ +import datadog.trace.agent.test.base.HttpClientTest +import datadog.trace.agent.test.naming.TestingGenericHttpNamingConventions +import org.eclipse.jetty.client.HttpClient +import org.eclipse.jetty.client.HttpProxy +import org.eclipse.jetty.client.HttpResponseException +import org.eclipse.jetty.client.Request +import org.eclipse.jetty.client.Response +import org.eclipse.jetty.client.Result +import org.eclipse.jetty.client.StringRequestContent +import org.eclipse.jetty.client.transport.HttpClientTransportDynamic +import org.eclipse.jetty.io.ClientConnector +import org.eclipse.jetty.util.ssl.SslContextFactory +import spock.lang.Shared +import spock.lang.Subject + +import java.util.concurrent.ExecutionException + +abstract class JettyClientTest extends HttpClientTest { + + @Shared + @Subject + HttpClient client = createHttpClient() + + @Shared + HttpClient proxiedClient = createHttpClient() + + def createHttpClient() { + SslContextFactory.Client sslContextFactory = new SslContextFactory.Client(true) + ClientConnector clientConnector = new ClientConnector() + clientConnector.setSslContextFactory(sslContextFactory) + return new HttpClient(new HttpClientTransportDynamic(clientConnector)) + } + + def setupSpec() { + client.connectTimeout = CONNECT_TIMEOUT_MS + client.addressResolutionTimeout = CONNECT_TIMEOUT_MS + client.idleTimeout = READ_TIMEOUT_MS + client.start() + + proxiedClient.proxyConfiguration.addProxy (new HttpProxy("localhost", proxy.port)) + proxiedClient.connectTimeout = CONNECT_TIMEOUT_MS + proxiedClient.addressResolutionTimeout = CONNECT_TIMEOUT_MS + proxiedClient.idleTimeout = READ_TIMEOUT_MS + proxiedClient.start() + } + + @Override + int doRequest(String method, URI uri, Map headers, String body, Closure callback) { + def proxy = uri.fragment != null && uri.fragment.equals("proxy") + Request req = (proxy ? proxiedClient : client).newRequest(uri).method(method) + headers.entrySet().each { h -> + req.headers {it.add(h.key, h.value)} + } + if (body) { + req.body(new StringRequestContent(body)) + } + + if (callback) { + req.onComplete (new Response.CompleteListener() { + @Override + void onComplete(Result result) { + callback.call() + } + }) + } + try { + def resp = req.send() + blockUntilChildSpansFinished(1) + //assert propagatesContext + return resp.status + } catch (ExecutionException ex) { + if (ex.cause instanceof HttpResponseException) { + return (ex.cause as HttpResponseException).response.status + } + throw ex + } + } + + @Override + CharSequence component() { + return "jetty-client" + } + + @Override + boolean testRedirects() { + false + } + + @Override + boolean testRemoteConnection() { + false + } + + @Override + boolean testSecure() { + true + } + + @Override + boolean testProxy() { + false // doesn't produce CONNECT span. + } +} + +class JettyClientV0Test extends JettyClientTest implements TestingGenericHttpNamingConventions.ClientV0 { +} + +class JettyClientV1ForkedTest extends JettyClientTest implements TestingGenericHttpNamingConventions.ClientV1 { +} diff --git a/settings.gradle b/settings.gradle index 53384c4822a..8424cfdce39 100644 --- a/settings.gradle +++ b/settings.gradle @@ -304,6 +304,7 @@ include ':dd-java-agent:instrumentation:jetty-appsec-9.3' include ':dd-java-agent:instrumentation:jetty-client:jetty-client-common' include ':dd-java-agent:instrumentation:jetty-client:jetty-client-9.1' include ':dd-java-agent:instrumentation:jetty-client:jetty-client-10.0' +include ':dd-java-agent:instrumentation:jetty-client:jetty-client-12.0' include ':dd-java-agent:instrumentation:jetty-common' include ':dd-java-agent:instrumentation:jetty-util' include ':dd-java-agent:instrumentation:jms' From 0902b4b74368590983d272dd7a9cf033e44c0e5c Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Thu, 11 Jul 2024 15:08:08 +0200 Subject: [PATCH 2/3] Use noop scope when no parent --- .../trace/instrumentation/jetty_client12/CallbackWrapper.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/CallbackWrapper.java b/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/CallbackWrapper.java index dd17a887f3a..dabe4f20cf3 100644 --- a/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/CallbackWrapper.java +++ b/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/CallbackWrapper.java @@ -1,6 +1,7 @@ package datadog.trace.instrumentation.jetty_client12; import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan; +import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.noopSpan; import datadog.trace.bootstrap.instrumentation.api.AgentScope; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; @@ -15,7 +16,7 @@ public class CallbackWrapper implements Response.Listener, Request.Listener { private final Object delegate; public CallbackWrapper(AgentSpan parent, AgentSpan span, Object delegate) { - this.parent = parent; + this.parent = parent != null ? parent : noopSpan(); this.span = span; this.delegate = delegate; } From 50e50cd22e558e65a63a7b6c113551a67e473e28 Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Thu, 11 Jul 2024 15:10:06 +0200 Subject: [PATCH 3/3] clean up --- .../jetty-client/jetty-client-12.0/build.gradle | 5 ++--- .../instrumentation/jetty_client12/CallbackWrapper.java | 2 -- .../jetty-client-12.0/src/test/groovy/JettyClientTest.groovy | 1 - 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/build.gradle b/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/build.gradle index 3df7a537917..f726648b022 100644 --- a/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/build.gradle +++ b/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/build.gradle @@ -27,11 +27,10 @@ configurations.matching({ it.name.startsWith('test') || it.name.startsWith('late dependencies { main_java17CompileOnly group: 'org.eclipse.jetty', name: 'jetty-client', version: '12.0.0' - - //because contains some instrumentation that still apply - testImplementation(project(':dd-java-agent:instrumentation:jetty-client:jetty-client-9.1')) // to test conflicts + testImplementation(project(':dd-java-agent:instrumentation:jetty-client:jetty-client-9.1')) testImplementation(project(':dd-java-agent:instrumentation:jetty-client:jetty-client-10.0')) + testImplementation(project(path:':dd-java-agent:testing', configuration:'shadow')) { // explicitly declared below. exclude group: 'org.eclipse.jetty' diff --git a/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/CallbackWrapper.java b/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/CallbackWrapper.java index dabe4f20cf3..74e393a47c2 100644 --- a/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/CallbackWrapper.java +++ b/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/main/java17/datadog/trace/instrumentation/jetty_client12/CallbackWrapper.java @@ -33,8 +33,6 @@ public void onBegin(Response response) { @Override public void onComplete(Result result) { if (delegate instanceof Response.CompleteListener) { - // this probably does the wrong thing, but preserves old behaviour and is consistent - // with other http clients with completion callback registration try (AgentScope scope = activate(parent)) { ((Response.CompleteListener) delegate).onComplete(result); } diff --git a/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/test/groovy/JettyClientTest.groovy b/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/test/groovy/JettyClientTest.groovy index 8c34e9da103..41aa79f1a94 100644 --- a/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/test/groovy/JettyClientTest.groovy +++ b/dd-java-agent/instrumentation/jetty-client/jetty-client-12.0/src/test/groovy/JettyClientTest.groovy @@ -66,7 +66,6 @@ abstract class JettyClientTest extends HttpClientTest { try { def resp = req.send() blockUntilChildSpansFinished(1) - //assert propagatesContext return resp.status } catch (ExecutionException ex) { if (ex.cause instanceof HttpResponseException) {