Simplify jetty9 http client instrumentation (#11595)

Author: laurit

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 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 static void addTracingEnter(
         @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();
     }
 

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 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 @@ static TracingHttpClient buildNew(
   @Override
   protected void send(HttpRequest request, List<Response.ResponseListener> listeners) {
     Context parentContext = Context.current();
-    JettyHttpClient9TracingInterceptor requestInterceptor =
-        new JettyHttpClient9TracingInterceptor(parentContext, this.instrumenter);
-    requestInterceptor.attachToRequest(request);
-    List<Response.ResponseListener> 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);
   }
 }

instrumentation/jetty-httpclient/jetty-httpclient-9.2/library/src/main/java/io/opentelemetry/instrumentation/jetty/httpclient/v9_2/internal/JettyHttpClient9TracingInterceptor.java 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.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
  *
  * <p>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<Request, Response> instrumenter;
 
-  public JettyHttpClient9TracingInterceptor(
-      Context parentCtx, Instrumenter<Request, Response> instrumenter) {
-    this.parentContext = parentCtx;
+  private JettyClientTracingListener(
+      Context context, Instrumenter<Request, Response> instrumenter) {
+    this.context = context;
     this.instrumenter = instrumenter;
   }
 
-  public void attachToRequest(Request jettyRequest) {
-    List<JettyHttpClient9TracingInterceptor> current =
-        jettyRequest.getRequestListeners(JettyHttpClient9TracingInterceptor.class);
-
+  @Nullable
+  public static Context handleRequest(
+      Context parentContext, HttpRequest request, Instrumenter<Request, Response> instrumenter) {
+    List<JettyClientTracingListener> 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;
+    }
+
+    if (!instrumenter.shouldStart(parentContext, request)) {
+      return null;
     }
-    startSpan(jettyRequest);
+
+    Context context = instrumenter.start(parentContext, request);
 
     // wrap all important request-based listeners that may already be attached, null should ensure
-    // are returned here
-    List<Request.RequestListener> existingListeners = jettyRequest.getRequestListeners(null);
-    wrapRequestListeners(existingListeners);
-
-    jettyRequest
-        .onRequestBegin(this)
-        .onRequestFailure(this)
-        .onResponseFailure(this)
-        .onResponseSuccess(this);
+    // that all listeners are returned here
+    List<Request.RequestListener> existingListeners = request.getRequestListeners(null);
+    wrapRequestListeners(existingListeners, context);
+
+    JettyClientTracingListener listener = new JettyClientTracingListener(context, instrumenter);
+    request
+        .onRequestBegin(listener)
+        .onRequestFailure(listener)
+        .onResponseFailure(listener)
+        .onResponseSuccess(listener);
+
+    return context;
   }
 
-  private void wrapRequestListeners(List<Request.RequestListener> requestListeners) {
+  private static void wrapRequestListeners(
+      List<Request.RequestListener> requestListeners, Context context) {
     ListIterator<Request.RequestListener> iterator = requestListeners.listIterator();
 
     while (iterator.hasNext()) {
@@ -121,34 +123,21 @@ private void wrapRequestListeners(List<Request.RequestListener> requestListeners
     }
   }
 
-  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);
   }
 }

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 static List<Response.ResponseListener> wrapResponseListeners(
 
   private static Response.ResponseListener wrapTheListener(
       Response.ResponseListener listener, Context context) {
-    if (listener == null || listener instanceof JettyHttpClient9TracingInterceptor) {
+    if (listener == null || listener instanceof JettyClientTracingListener) {
       return listener;
     }