[DRAFT] APM AI Toolkit: Add Feign HTTP client instrumentation (v8.0+) 2nd run with add-apm-integrations.md

dd-java-agent/instrumentation/feign/feign-8.0/build.gradle

@@ -0,0 +1,22 @@
+muzzle {
+  pass {
+    group = "com.netflix.feign"
+    module = "feign-core"
+    versions = "[8.0,9.0)"
+    assertInverse = true
+  }
+}
+
+apply from: "$rootDir/gradle/java.gradle"
+
+addTestSuiteForDir('latestDepTest', 'test')
+
+dependencies {
+  compileOnly group: 'com.netflix.feign', name: 'feign-core', version: '8.0.0'
+
+  testImplementation group: 'com.netflix.feign', name: 'feign-core', version: '8.18.0'
+  testImplementation group: 'com.netflix.feign', name: 'feign-okhttp', version: '8.18.0'
+
+  latestDepTestImplementation group: 'com.netflix.feign', name: 'feign-core', version: '8.+'
+  latestDepTestImplementation group: 'com.netflix.feign', name: 'feign-okhttp', version: '8.+'
+}

dd-java-agent/instrumentation/feign/feign-8.0/src/main/java/datadog/trace/instrumentation/feign/FeignClientDecorator.java

@@ -0,0 +1,67 @@
+package datadog.trace.instrumentation.feign;
+
+import static datadog.context.Context.current;
+import static datadog.trace.instrumentation.feign.RequestInjectAdapter.SETTER;
+
+import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString;
+import datadog.trace.bootstrap.instrumentation.decorator.HttpClientDecorator;
+import feign.Request;
+import feign.Response;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.util.Collection;
+import java.util.Map;
+
+public class FeignClientDecorator extends HttpClientDecorator<Request, Response> {
+  public static final CharSequence FEIGN = UTF8BytesString.create("feign");
+  public static final FeignClientDecorator DECORATE = new FeignClientDecorator();
+  public static final CharSequence FEIGN_REQUEST = UTF8BytesString.create(DECORATE.operationName());
+
+  @Override
+  protected String[] instrumentationNames() {
+    return new String[] {"feign"};
+  }
+
+  @Override
+  protected CharSequence component() {
+    return FEIGN;
+  }
+
+  @Override
+  protected String method(final Request request) {
+    return request.method();
+  }
+
+  @Override
+  protected URI url(final Request request) throws URISyntaxException {
+    return new URI(request.url());
+  }
+
+  @Override
+  protected int status(final Response response) {
+    return response.status();
+  }
+
+  @Override
+  protected String getRequestHeader(Request request, String headerName) {
+    Collection<String> values = request.headers().get(headerName);
+    if (values != null && !values.isEmpty()) {
+      return values.iterator().next();
+    }
+    return null;
+  }
+
+  @Override
+  protected String getResponseHeader(Response response, String headerName) {
+    Collection<String> values = response.headers().get(headerName);
+    if (values != null && !values.isEmpty()) {
+      return values.iterator().next();
+    }
+    return null;
+  }
+
+  /** Inject trace headers into the Feign request headers map. */
+  public void injectHeaders(Map<String, Collection<String>> headers) {
+    injectContext(current(), headers, SETTER);
+  }
+}

dd-java-agent/instrumentation/feign/feign-8.0/src/main/java/datadog/trace/instrumentation/feign/FeignInstrumentation.java

@@ -0,0 +1,99 @@
+package datadog.trace.instrumentation.feign;
+
+import static datadog.trace.agent.tooling.bytebuddy.matcher.ClassLoaderMatchers.hasClassNamed;
+import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
+import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activateSpan;
+import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.startSpan;
+import static datadog.trace.instrumentation.feign.FeignClientDecorator.DECORATE;
+import static datadog.trace.instrumentation.feign.FeignClientDecorator.FEIGN_REQUEST;
+import static net.bytebuddy.matcher.ElementMatchers.isMethod;
+import static net.bytebuddy.matcher.ElementMatchers.isPublic;
+import static net.bytebuddy.matcher.ElementMatchers.not;
+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.AgentScope;
+import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
+import feign.Request;
+import feign.Response;
+import net.bytebuddy.asm.Advice;
+import net.bytebuddy.matcher.ElementMatcher;
+
+@AutoService(InstrumenterModule.class)
+public class FeignInstrumentation extends InstrumenterModule.Tracing
+    implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice {
+
+  public FeignInstrumentation() {
+    super("feign");
+  }
+
+  @Override
+  public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
+    // Feign 8.0 removed Dagger dependency, so check for absence of dagger-related inject adapters
+    return hasClassNamed("feign.Param")
+        .and(not(hasClassNamed("feign.Client$Default$$InjectAdapter")));
+  }
+
+  @Override
+  public String instrumentedType() {
+    return "feign.Client";
+  }
+
+  @Override
+  public String[] helperClassNames() {
+    return new String[] {
+      packageName + ".FeignClientDecorator", packageName + ".RequestInjectAdapter",
+    };
+  }
+
+  @Override
+  public void methodAdvice(MethodTransformer transformer) {
+    transformer.applyAdvice(
+        isMethod()
+            .and(isPublic())
+            .and(named("execute"))
+            .and(takesArguments(2))
+            .and(takesArgument(0, named("feign.Request")))
+            .and(takesArgument(1, named("feign.Request$Options"))),
+        FeignInstrumentation.class.getName() + "$FeignClientAdvice");
+  }
+
+  public static class FeignClientAdvice {
+    @Advice.OnMethodEnter(suppress = Throwable.class)
+    public static AgentScope methodEnter(@Advice.Argument(0) Request request) {
+      AgentSpan span = startSpan(FEIGN_REQUEST);
+      DECORATE.afterStart(span);
+      DECORATE.onRequest(span, request);
+
+      // Inject headers into the request's mutable headers map
+      DECORATE.injectHeaders(request.headers());
+
+      return activateSpan(span);
+    }
+
+    @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
+    public static void methodExit(
+        @Advice.Enter final AgentScope scope,
+        @Advice.Return final Response response,
+        @Advice.Thrown final Throwable throwable) {
+      if (scope == null) {
+        return;
+      }
+
+      try {
+        AgentSpan span = scope.span();
+        DECORATE.onError(span, throwable);
+        if (response != null) {
+          DECORATE.onResponse(span, response);
+        }
+        DECORATE.beforeFinish(span);
+        span.finish();
+      } finally {
+        scope.close();
+      }
+    }
+  }
+}

dd-java-agent/instrumentation/feign/feign-8.0/src/main/java/datadog/trace/instrumentation/feign/RequestInjectAdapter.java

@@ -0,0 +1,22 @@
+package datadog.trace.instrumentation.feign;
+
+import datadog.context.propagation.CarrierSetter;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Map;
+
+public class RequestInjectAdapter implements CarrierSetter<Map<String, Collection<String>>> {
+
+  public static final RequestInjectAdapter SETTER = new RequestInjectAdapter();
+
+  @Override
+  public void set(
+      final Map<String, Collection<String>> carrier, final String key, final String value) {
+    Collection<String> values = carrier.get(key);
+    if (values == null) {
+      values = new ArrayList<>();
+      carrier.put(key, values);
+    }
+    values.add(value);
+  }
+}

dd-java-agent/instrumentation/feign/feign-8.0/src/test/groovy/FeignTest.groovy

@@ -0,0 +1,69 @@
+import datadog.trace.agent.test.base.HttpClientTest
+import datadog.trace.agent.test.naming.TestingGenericHttpNamingConventions
+import datadog.trace.instrumentation.feign.FeignClientDecorator
+import feign.Feign
+import feign.Request
+import feign.RequestLine
+import feign.Util
+import spock.lang.Shared
+
+abstract class FeignTest extends HttpClientTest {
+
+  @Shared
+  def client
+
+  def setupSpec() {
+    client = Feign.builder()
+      .target(TestInterface, "http://localhost:${server.address.port}")
+  }
+
+  @Override
+  int doRequest(String method, URI uri, Map<String, String> headers, String body, Closure callback) {
+    def request = Request.create(
+      method,
+      uri.toString(),
+      headers.collectEntries { k, v -> [(k): [v]] },
+      body ? body.bytes : null,
+      Util.UTF_8
+      )
+
+    def options = new Request.Options()
+    def feignClient = new feign.Client.Default(null, null)
+    def response = feignClient.execute(request, options)
+
+    callback?.call(response.body().asInputStream())
+    return response.status()
+  }
+
+  @Override
+  CharSequence component() {
+    return FeignClientDecorator.DECORATE.component()
+  }
+
+  @Override
+  boolean testRedirects() {
+    false
+  }
+
+  @Override
+  boolean testConnectionFailure() {
+    false
+  }
+
+  @Override
+  boolean testRemoteConnection() {
+    // Feign doesn't work well with redirects in test harness
+    return false
+  }
+
+  interface TestInterface {
+    @RequestLine("GET /success")
+    String success()
+  }
+}
+
+class FeignV0ForkedTest extends FeignTest implements TestingGenericHttpNamingConventions.ClientV0 {
+}
+
+class FeignV1ForkedTest extends FeignTest implements TestingGenericHttpNamingConventions.ClientV1 {
+}

settings.gradle.kts

@@ -340,6 +340,7 @@ include(
   ":dd-java-agent:instrumentation:elasticsearch:elasticsearch-transport:elasticsearch-transport-7.3",
   ":dd-java-agent:instrumentation:elasticsearch:elasticsearch-transport:elasticsearch-transport-common",
   ":dd-java-agent:instrumentation:elasticsearch:elasticsearch-common",
+  ":dd-java-agent:instrumentation:feign:feign-8.0",
   ":dd-java-agent:instrumentation:finatra-2.9",
   ":dd-java-agent:instrumentation:freemarker:freemarker-2.3.24",
   ":dd-java-agent:instrumentation:freemarker:freemarker-2.3.9",