From de11929beaca8a55bc8adf3cf34db01a4dd85c95 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Wed, 19 Jun 2024 02:09:34 +0300 Subject: [PATCH] Simplify jetty9 http client instrumentation (#11595) --- .../v9_2/JettyHttpClient9Instrumentation.java | 14 +-- .../httpclient/v9_2/TracingHttpClient.java | 14 +-- ...r.java => JettyClientTracingListener.java} | 91 ++++++++----------- .../v9_2/internal/JettyClientWrapUtil.java | 2 +- 4 files changed, 53 insertions(+), 68 deletions(-) rename instrumentation/jetty-httpclient/jetty-httpclient-9.2/library/src/main/java/io/opentelemetry/instrumentation/jetty/httpclient/v9_2/internal/{JettyHttpClient9TracingInterceptor.java => JettyClientTracingListener.java} (58%) diff --git a/instrumentation/jetty-httpclient/jetty-httpclient-9.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/httpclient/v9_2/JettyHttpClient9Instrumentation.java b/instrumentation/jetty-httpclient/jetty-httpclient-9.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/httpclient/v9_2/JettyHttpClient9Instrumentation.java index 7ad866f0f5..4215289766 100644 --- a/instrumentation/jetty-httpclient/jetty-httpclient-9.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/httpclient/v9_2/JettyHttpClient9Instrumentation.java +++ b/instrumentation/jetty-httpclient/jetty-httpclient-9.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/httpclient/v9_2/JettyHttpClient9Instrumentation.java @@ -14,7 +14,7 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArgument; import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; -import io.opentelemetry.instrumentation.jetty.httpclient.v9_2.internal.JettyHttpClient9TracingInterceptor; +import io.opentelemetry.instrumentation.jetty.httpclient.v9_2.internal.JettyClientTracingListener; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; import java.util.List; @@ -50,19 +50,13 @@ public class JettyHttpClient9Instrumentation implements TypeInstrumentation { @Advice.Local("otelContext") Context context, @Advice.Local("otelScope") Scope scope) { Context parentContext = currentContext(); - if (!instrumenter().shouldStart(parentContext, httpRequest)) { + context = + JettyClientTracingListener.handleRequest(parentContext, httpRequest, instrumenter()); + if (context == null) { return; } - // First step is to attach the tracer to the Jetty request. Request listeners are wrapped here - JettyHttpClient9TracingInterceptor requestInterceptor = - new JettyHttpClient9TracingInterceptor(parentContext, instrumenter()); - requestInterceptor.attachToRequest(httpRequest); - - // Second step is to wrap all the important result callback listeners = wrapResponseListeners(parentContext, listeners); - - context = requestInterceptor.getContext(); scope = context.makeCurrent(); } diff --git a/instrumentation/jetty-httpclient/jetty-httpclient-9.2/library/src/main/java/io/opentelemetry/instrumentation/jetty/httpclient/v9_2/TracingHttpClient.java b/instrumentation/jetty-httpclient/jetty-httpclient-9.2/library/src/main/java/io/opentelemetry/instrumentation/jetty/httpclient/v9_2/TracingHttpClient.java index 094b38ced3..e89708a146 100644 --- a/instrumentation/jetty-httpclient/jetty-httpclient-9.2/library/src/main/java/io/opentelemetry/instrumentation/jetty/httpclient/v9_2/TracingHttpClient.java +++ b/instrumentation/jetty-httpclient/jetty-httpclient-9.2/library/src/main/java/io/opentelemetry/instrumentation/jetty/httpclient/v9_2/TracingHttpClient.java @@ -9,7 +9,7 @@ import static io.opentelemetry.instrumentation.jetty.httpclient.v9_2.internal.Je import io.opentelemetry.context.Context; import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter; -import io.opentelemetry.instrumentation.jetty.httpclient.v9_2.internal.JettyHttpClient9TracingInterceptor; +import io.opentelemetry.instrumentation.jetty.httpclient.v9_2.internal.JettyClientTracingListener; import java.util.List; import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.client.HttpClientTransport; @@ -60,10 +60,12 @@ class TracingHttpClient extends HttpClient { @Override protected void send(HttpRequest request, List listeners) { Context parentContext = Context.current(); - JettyHttpClient9TracingInterceptor requestInterceptor = - new JettyHttpClient9TracingInterceptor(parentContext, this.instrumenter); - requestInterceptor.attachToRequest(request); - List wrapped = wrapResponseListeners(parentContext, listeners); - super.send(request, wrapped); + Context context = + JettyClientTracingListener.handleRequest(parentContext, request, instrumenter); + // wrap listeners only when a span was started (context is not null) + if (context != null) { + listeners = wrapResponseListeners(parentContext, listeners); + } + super.send(request, listeners); } } diff --git a/instrumentation/jetty-httpclient/jetty-httpclient-9.2/library/src/main/java/io/opentelemetry/instrumentation/jetty/httpclient/v9_2/internal/JettyHttpClient9TracingInterceptor.java b/instrumentation/jetty-httpclient/jetty-httpclient-9.2/library/src/main/java/io/opentelemetry/instrumentation/jetty/httpclient/v9_2/internal/JettyClientTracingListener.java similarity index 58% rename from instrumentation/jetty-httpclient/jetty-httpclient-9.2/library/src/main/java/io/opentelemetry/instrumentation/jetty/httpclient/v9_2/internal/JettyHttpClient9TracingInterceptor.java rename to instrumentation/jetty-httpclient/jetty-httpclient-9.2/library/src/main/java/io/opentelemetry/instrumentation/jetty/httpclient/v9_2/internal/JettyClientTracingListener.java index 71c3e4778a..844929afe8 100644 --- a/instrumentation/jetty-httpclient/jetty-httpclient-9.2/library/src/main/java/io/opentelemetry/instrumentation/jetty/httpclient/v9_2/internal/JettyHttpClient9TracingInterceptor.java +++ b/instrumentation/jetty-httpclient/jetty-httpclient-9.2/library/src/main/java/io/opentelemetry/instrumentation/jetty/httpclient/v9_2/internal/JettyClientTracingListener.java @@ -15,26 +15,26 @@ import java.util.List; import java.util.ListIterator; import java.util.logging.Logger; import javax.annotation.Nullable; +import org.eclipse.jetty.client.HttpRequest; import org.eclipse.jetty.client.api.Request; import org.eclipse.jetty.client.api.Response; /** - * JettyHttpClient9TracingInterceptor does three jobs stimulated from the Jetty Request object from - * attachToRequest() 1. Start the CLIENT span and create the tracer 2. Set the listener callbacks - * for each important lifecycle actions that would cause the start and close of the span 3. Set - * callback wrappers on two important request-based callbacks + * JettyClientTracingListener performs three actions when {@link #handleRequest(Context, + * HttpRequest, Instrumenter)} is called 1. Start the CLIENT span 2. Set the listener callbacks for + * each lifecycle action that signal end of the request 3. Wrap request listeners to propagate + * context into the listeners * *

This class is internal and is hence not for public use. Its APIs are unstable and can change * at any time. */ -public final class JettyHttpClient9TracingInterceptor +public final class JettyClientTracingListener implements Request.BeginListener, Request.FailureListener, Response.SuccessListener, Response.FailureListener { - private static final Logger logger = - Logger.getLogger(JettyHttpClient9TracingInterceptor.class.getName()); + private static final Logger logger = Logger.getLogger(JettyClientTracingListener.class.getName()); private static final Class[] requestlistenerInterfaces = { Request.BeginListener.class, @@ -46,46 +46,48 @@ public final class JettyHttpClient9TracingInterceptor Request.QueuedListener.class }; - @Nullable private Context context; - - @Nullable - public Context getContext() { - return this.context; - } - - private final Context parentContext; - + private final Context context; private final Instrumenter instrumenter; - public JettyHttpClient9TracingInterceptor( - Context parentCtx, Instrumenter instrumenter) { - this.parentContext = parentCtx; + private JettyClientTracingListener( + Context context, Instrumenter instrumenter) { + this.context = context; this.instrumenter = instrumenter; } - public void attachToRequest(Request jettyRequest) { - List current = - jettyRequest.getRequestListeners(JettyHttpClient9TracingInterceptor.class); - + @Nullable + public static Context handleRequest( + Context parentContext, HttpRequest request, Instrumenter instrumenter) { + List current = + request.getRequestListeners(JettyClientTracingListener.class); if (!current.isEmpty()) { - logger.warning("A tracing interceptor is already in place for this request!"); - return; + logger.warning("A tracing request listener is already in place for this request!"); + return null; } - startSpan(jettyRequest); + + if (!instrumenter.shouldStart(parentContext, request)) { + return null; + } + + Context context = instrumenter.start(parentContext, request); // wrap all important request-based listeners that may already be attached, null should ensure - // are returned here - List existingListeners = jettyRequest.getRequestListeners(null); - wrapRequestListeners(existingListeners); + // that all listeners are returned here + List existingListeners = request.getRequestListeners(null); + wrapRequestListeners(existingListeners, context); - jettyRequest - .onRequestBegin(this) - .onRequestFailure(this) - .onResponseFailure(this) - .onResponseSuccess(this); + JettyClientTracingListener listener = new JettyClientTracingListener(context, instrumenter); + request + .onRequestBegin(listener) + .onRequestFailure(listener) + .onResponseFailure(listener) + .onResponseSuccess(listener); + + return context; } - private void wrapRequestListeners(List requestListeners) { + private static void wrapRequestListeners( + List requestListeners, Context context) { ListIterator iterator = requestListeners.listIterator(); while (iterator.hasNext()) { @@ -121,34 +123,21 @@ public final class JettyHttpClient9TracingInterceptor } } - private void startSpan(Request request) { - if (!instrumenter.shouldStart(this.parentContext, request)) { - return; - } - this.context = instrumenter.start(this.parentContext, request); - } - @Override public void onBegin(Request request) {} @Override public void onSuccess(Response response) { - if (this.context != null) { - instrumenter.end(this.context, response.getRequest(), response, null); - } + instrumenter.end(this.context, response.getRequest(), response, null); } @Override public void onFailure(Request request, Throwable t) { - if (this.context != null) { - instrumenter.end(this.context, request, null, t); - } + instrumenter.end(this.context, request, null, t); } @Override public void onFailure(Response response, Throwable t) { - if (this.context != null) { - instrumenter.end(this.context, response.getRequest(), response, t); - } + instrumenter.end(this.context, response.getRequest(), response, t); } } diff --git a/instrumentation/jetty-httpclient/jetty-httpclient-9.2/library/src/main/java/io/opentelemetry/instrumentation/jetty/httpclient/v9_2/internal/JettyClientWrapUtil.java b/instrumentation/jetty-httpclient/jetty-httpclient-9.2/library/src/main/java/io/opentelemetry/instrumentation/jetty/httpclient/v9_2/internal/JettyClientWrapUtil.java index b6885bd3ed..53a76256da 100644 --- a/instrumentation/jetty-httpclient/jetty-httpclient-9.2/library/src/main/java/io/opentelemetry/instrumentation/jetty/httpclient/v9_2/internal/JettyClientWrapUtil.java +++ b/instrumentation/jetty-httpclient/jetty-httpclient-9.2/library/src/main/java/io/opentelemetry/instrumentation/jetty/httpclient/v9_2/internal/JettyClientWrapUtil.java @@ -50,7 +50,7 @@ public final class JettyClientWrapUtil { private static Response.ResponseListener wrapTheListener( Response.ResponseListener listener, Context context) { - if (listener == null || listener instanceof JettyHttpClient9TracingInterceptor) { + if (listener == null || listener instanceof JettyClientTracingListener) { return listener; }