Jetty-9 httpclient instrumentation, comments please (#3079)

* feat: jetty-9 http client, commit for PR comments

* chore: compensate for build break due to new TypeTransformer

* WIP: jetty9 with wrappers working

* feat: moved structure to match 9.2

* chore: fix package names again, now 9.2, clean up Nullables and Muzzle range

* chore: added latestDepTestLibrary for 10+ but class loader issues due to underlying jetty-server from the test framework

* chore: migrate to new instrumenter api

* chore: adjust to all the gradle plugin changes

* chore: added layers of builders

* chore: update final easy code review comments

* chore: remove comments in gradle

* chore: code cleanup, package cleanup, reduce surface area, iterators

* chore: cleanup of null checks, using HttpFlavorVals enum

* chore: null check removal again

* chore: set muzzle plugin correctly for 9.2 up to 9.4.+

* chore: convert gradle plugins to not use apply syntax

* Rename to build.gradle

* chore: move TypeInstrumention into standalone class

Co-authored-by: Anuraag Agrawal <[email protected]>

instrumentation/jetty-httpclient/jetty-httpclient-9.2/javaagent/build.gradle

@@ -0,0 +1,28 @@
+plugins {
+  id("otel.javaagent-instrumentation")
+}
+
+muzzle {
+  pass {
+    group = "org.eclipse.jetty"
+    module = 'jetty-client'
+    versions = "[9.2,9.4.+)"
+  }
+}
+
+
+//Jetty client 9.2 is the best starting point, HttpClient.send() is stable there
+def jettyVers_base9 = '9.2.0.v20140526'
+
+
+dependencies {
+  implementation project(':instrumentation:jetty-httpclient:jetty-httpclient-9.2:library')
+
+  library "org.eclipse.jetty:jetty-client:${jettyVers_base9}"
+  latestDepTestLibrary "org.eclipse.jetty:jetty-client:9.+"
+
+  testImplementation project(':instrumentation:jetty-httpclient:jetty-httpclient-9.2:testing')
+  testImplementation("org.eclipse.jetty:jetty-server:${jettyVers_base9}") {
+    exclude group: 'org.eclipse.jetty', module: 'jetty-client'
+  }
+}

instrumentation/jetty-httpclient/jetty-httpclient-9.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/httpclient/v9_2/JettyHttpClient9Instrumentation.java

@@ -0,0 +1,87 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.jetty.httpclient.v9_2;
+
+import static io.opentelemetry.instrumentation.jetty.httpclient.v9_2.internal.JettyClientWrapUtil.wrapResponseListeners;
+import static io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge.currentContext;
+import static io.opentelemetry.javaagent.instrumentation.jetty.httpclient.v9_2.JettyHttpClientSingletons.instrumenter;
+import static net.bytebuddy.matcher.ElementMatchers.isMethod;
+import static net.bytebuddy.matcher.ElementMatchers.named;
+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.javaagent.extension.instrumentation.TypeInstrumentation;
+import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
+import java.util.List;
+import net.bytebuddy.asm.Advice;
+import net.bytebuddy.description.type.TypeDescription;
+import net.bytebuddy.matcher.ElementMatcher;
+import org.eclipse.jetty.client.HttpRequest;
+import org.eclipse.jetty.client.api.Response;
+
+public class JettyHttpClient9Instrumentation implements TypeInstrumentation {
+  @Override
+  public ElementMatcher<TypeDescription> typeMatcher() {
+    return named("org.eclipse.jetty.client.HttpClient");
+  }
+
+  @Override
+  public void transform(TypeTransformer transformer) {
+    transformer.applyAdviceToMethod(
+        isMethod()
+            .and(named("send"))
+            .and(takesArgument(0, named("org.eclipse.jetty.client.HttpRequest")))
+            .and(takesArgument(1, List.class)),
+        JettyHttpClient9Instrumentation.class.getName() + "$JettyHttpClient9Advice");
+  }
+
+  @SuppressWarnings("unused")
+  public static class JettyHttpClient9Advice {
+
+    @Advice.OnMethodEnter(suppress = Throwable.class)
+    public static void addTracingEnter(
+        @Advice.Argument(value = 0) HttpRequest httpRequest,
+        @Advice.Argument(value = 1, readOnly = false) List<Response.ResponseListener> listeners,
+        @Advice.Local("otelContext") Context context,
+        @Advice.Local("otelScope") Scope scope) {
+      Context parentContext = currentContext();
+      if (!instrumenter().shouldStart(parentContext, httpRequest)) {
+        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();
+    }
+
+    @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class)
+    public static void exitTracingInterceptor(
+        @Advice.Argument(value = 0) HttpRequest httpRequest,
+        @Advice.Thrown Throwable throwable,
+        @Advice.Local("otelContext") Context context,
+        @Advice.Local("otelScope") Scope scope) {
+
+      if (scope == null) {
+        return;
+      }
+
+      // not ending span here unless error, span ended in the interceptor
+      scope.close();
+      if (throwable != null) {
+        instrumenter().end(context, httpRequest, null, throwable);
+      }
+    }
+  }
+}

instrumentation/jetty-httpclient/jetty-httpclient-9.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/httpclient/v9_2/JettyHttpClient9InstrumentationModule.java

@@ -0,0 +1,34 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.jetty.httpclient.v9_2;
+
+import static io.opentelemetry.javaagent.extension.matcher.ClassLoaderMatcher.hasClassesNamed;
+import static java.util.Collections.singletonList;
+
+import com.google.auto.service.AutoService;
+import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
+import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
+import java.util.List;
+import net.bytebuddy.matcher.ElementMatcher;
+
+@AutoService(InstrumentationModule.class)
+public class JettyHttpClient9InstrumentationModule extends InstrumentationModule {
+
+  public JettyHttpClient9InstrumentationModule() {
+    super("jetty-httpclient", "jetty-httpclient-9.2");
+  }
+
+  @Override
+  public List<TypeInstrumentation> typeInstrumentations() {
+    return singletonList(new JettyHttpClient9Instrumentation());
+  }
+
+  @Override
+  public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
+    // AbstractTypedContentProvider  showed up in version Jetty Client 9.2 on to 10.x
+    return hasClassesNamed("org.eclipse.jetty.client.util.AbstractTypedContentProvider");
+  }
+}

instrumentation/jetty-httpclient/jetty-httpclient-9.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jetty/httpclient/v9_2/JettyHttpClientSingletons.java

@@ -0,0 +1,34 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.jetty.httpclient.v9_2;
+
+import io.opentelemetry.api.GlobalOpenTelemetry;
+import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
+import io.opentelemetry.instrumentation.jetty.httpclient.v9_2.internal.JettyClientInstrumenterBuilder;
+import io.opentelemetry.instrumentation.jetty.httpclient.v9_2.internal.JettyHttpClientNetAttributesExtractor;
+import io.opentelemetry.javaagent.instrumentation.api.instrumenter.PeerServiceAttributesExtractor;
+import org.eclipse.jetty.client.api.Request;
+import org.eclipse.jetty.client.api.Response;
+
+public class JettyHttpClientSingletons {
+
+  private static final Instrumenter<Request, Response> INSTRUMENTER;
+
+  private JettyHttpClientSingletons() {}
+
+  static {
+    JettyClientInstrumenterBuilder builder =
+        new JettyClientInstrumenterBuilder(GlobalOpenTelemetry.get());
+
+    PeerServiceAttributesExtractor<Request, Response> peerServiceAttributesExtractor =
+        PeerServiceAttributesExtractor.create(new JettyHttpClientNetAttributesExtractor());
+    INSTRUMENTER = builder.addAttributeExtractor(peerServiceAttributesExtractor).build();
+  }
+
+  public static Instrumenter<Request, Response> instrumenter() {
+    return INSTRUMENTER;
+  }
+}

instrumentation/jetty-httpclient/jetty-httpclient-9.2/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/jetty/httpclient/v9_2/JettyHttpClient9AgentTest.groovy

@@ -0,0 +1,24 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.jetty.httpclient.v9_2
+
+import io.opentelemetry.instrumentation.jetty.httpclient.v9_2.AbstractJettyClient9Test
+import io.opentelemetry.instrumentation.test.AgentTestTrait
+import org.eclipse.jetty.client.HttpClient
+import org.eclipse.jetty.util.ssl.SslContextFactory
+
+class JettyHttpClient9AgentTest extends AbstractJettyClient9Test implements AgentTestTrait {
+
+  @Override
+  HttpClient createStandardClient() {
+    return new HttpClient()
+  }
+
+  @Override
+  HttpClient createHttpsClient(SslContextFactory sslContextFactory) {
+    return new HttpClient(sslContextFactory)
+  }
+}

instrumentation/jetty-httpclient/jetty-httpclient-9.2/library/build.gradle

@@ -0,0 +1,17 @@
+plugins {
+  id("otel.library-instrumentation")
+  id("net.ltgt.errorprone")
+}
+
+
+//Jetty client 9.2 is the best starting point, HttpClient.send() is stable there
+def jettyVers_base9 = '9.2.0.v20140526'
+
+dependencies {
+  library "org.eclipse.jetty:jetty-client:${jettyVers_base9}"
+  latestDepTestLibrary "org.eclipse.jetty:jetty-client:9.+"
+  testImplementation project(':instrumentation:jetty-httpclient::jetty-httpclient-9.2:testing')
+
+  implementation "org.slf4j:slf4j-api"
+}
+

instrumentation/jetty-httpclient/jetty-httpclient-9.2/library/src/main/java/io/opentelemetry/instrumentation/jetty/httpclient/v9_2/JettyClientTracing.java

@@ -0,0 +1,32 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.jetty.httpclient.v9_2;
+
+import io.opentelemetry.api.OpenTelemetry;
+import org.eclipse.jetty.client.HttpClient;
+
+/** JettyClientTracing, the Entrypoint for tracing Jetty client. */
+public final class JettyClientTracing {
+
+  private final HttpClient httpClient;
+
+  public static JettyClientTracing create(OpenTelemetry openTelemetry) {
+    JettyClientTracingBuilder builder = newBuilder(openTelemetry);
+    return builder.build();
+  }
+
+  public static JettyClientTracingBuilder newBuilder(OpenTelemetry openTelemetry) {
+    return new JettyClientTracingBuilder(openTelemetry);
+  }
+
+  public HttpClient getHttpClient() {
+    return httpClient;
+  }
+
+  JettyClientTracing(HttpClient httpClient) {
+    this.httpClient = httpClient;
+  }
+}

instrumentation/jetty-httpclient/jetty-httpclient-9.2/library/src/main/java/io/opentelemetry/instrumentation/jetty/httpclient/v9_2/JettyClientTracingBuilder.java

@@ -0,0 +1,46 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.jetty.httpclient.v9_2;
+
+import io.opentelemetry.api.OpenTelemetry;
+import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
+import io.opentelemetry.instrumentation.jetty.httpclient.v9_2.internal.JettyClientInstrumenterBuilder;
+import org.eclipse.jetty.client.HttpClientTransport;
+import org.eclipse.jetty.client.api.Request;
+import org.eclipse.jetty.client.api.Response;
+import org.eclipse.jetty.util.ssl.SslContextFactory;
+
+public final class JettyClientTracingBuilder {
+
+  private final OpenTelemetry openTelemetry;
+  private HttpClientTransport httpClientTransport;
+  private SslContextFactory sslContextFactory;
+
+  public JettyClientTracingBuilder(OpenTelemetry openTelemetry) {
+    this.openTelemetry = openTelemetry;
+  }
+
+  public JettyClientTracingBuilder setHttpClientTransport(HttpClientTransport httpClientTransport) {
+    this.httpClientTransport = httpClientTransport;
+    return this;
+  }
+
+  public JettyClientTracingBuilder setSslContextFactory(SslContextFactory sslContextFactory) {
+    this.sslContextFactory = sslContextFactory;
+    return this;
+  }
+
+  public JettyClientTracing build() {
+    JettyClientInstrumenterBuilder instrumenterBuilder =
+        new JettyClientInstrumenterBuilder(this.openTelemetry);
+    Instrumenter<Request, Response> instrumenter = instrumenterBuilder.build();
+
+    TracingHttpClient tracingHttpClient =
+        TracingHttpClient.buildNew(instrumenter, this.sslContextFactory, this.httpClientTransport);
+
+    return new JettyClientTracing(tracingHttpClient);
+  }
+}

instrumentation/jetty-httpclient/jetty-httpclient-9.2/library/src/main/java/io/opentelemetry/instrumentation/jetty/httpclient/v9_2/TracingHttpClient.java

@@ -0,0 +1,69 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.jetty.httpclient.v9_2;
+
+import static io.opentelemetry.instrumentation.jetty.httpclient.v9_2.internal.JettyClientWrapUtil.wrapResponseListeners;
+
+import io.opentelemetry.context.Context;
+import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
+import io.opentelemetry.instrumentation.jetty.httpclient.v9_2.internal.JettyHttpClient9TracingInterceptor;
+import java.util.List;
+import org.eclipse.jetty.client.HttpClient;
+import org.eclipse.jetty.client.HttpClientTransport;
+import org.eclipse.jetty.client.HttpRequest;
+import org.eclipse.jetty.client.api.Request;
+import org.eclipse.jetty.client.api.Response;
+import org.eclipse.jetty.util.ssl.SslContextFactory;
+
+class TracingHttpClient extends HttpClient {
+
+  private final Instrumenter<Request, Response> instrumenter;
+
+  TracingHttpClient(Instrumenter<Request, Response> instrumenter) {
+    super();
+    this.instrumenter = instrumenter;
+  }
+
+  TracingHttpClient(
+      Instrumenter<Request, Response> instrumenter, SslContextFactory sslContextFactory) {
+    super(sslContextFactory);
+    this.instrumenter = instrumenter;
+  }
+
+  TracingHttpClient(
+      Instrumenter<Request, Response> instrumenter,
+      HttpClientTransport transport,
+      SslContextFactory sslContextFactory) {
+    super(transport, sslContextFactory);
+    this.instrumenter = instrumenter;
+  }
+
+  static TracingHttpClient buildNew(
+      Instrumenter<Request, Response> instrumenter,
+      SslContextFactory sslContextFactory,
+      HttpClientTransport httpClientTransport) {
+    TracingHttpClient tracingHttpClient = null;
+    if (sslContextFactory != null && httpClientTransport != null) {
+      tracingHttpClient =
+          new TracingHttpClient(instrumenter, httpClientTransport, sslContextFactory);
+    } else if (sslContextFactory != null) {
+      tracingHttpClient = new TracingHttpClient(instrumenter, sslContextFactory);
+    } else {
+      tracingHttpClient = new TracingHttpClient(instrumenter);
+    }
+    return tracingHttpClient;
+  }
+
+  @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);
+  }
+}