Extract okhttp3 library instrumentation. (#2489)

* Extract okhttp3 library instrumentation.

* bad spotless

instrumentation/kubernetes-client-7.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/kubernetesclient/KubernetesClientTracer.java

@@ -11,7 +11,6 @@ import io.opentelemetry.api.trace.Span;
 import io.opentelemetry.context.propagation.TextMapSetter;
 import io.opentelemetry.instrumentation.api.config.Config;
 import io.opentelemetry.instrumentation.api.tracer.HttpClientTracer;
-import io.opentelemetry.javaagent.instrumentation.okhttp.v3_0.RequestBuilderInjectAdapter;
 import io.opentelemetry.semconv.trace.attributes.SemanticAttributes;
 import java.net.URI;
 import java.util.Collections;

instrumentation/kubernetes-client-7.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/kubernetesclient/RequestBuilderInjectAdapter.java

@@ -0,0 +1,29 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.kubernetesclient;
+
+import io.opentelemetry.context.propagation.TextMapSetter;
+import okhttp3.Request;
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+/**
+ * Helper class to inject span context into request headers.
+ *
+ * @author Pavol Loffay
+ */
+// TODO(anuraaga): Figure out a way to avoid copying this from okhttp instrumentation.
+final class RequestBuilderInjectAdapter implements TextMapSetter<Request.Builder> {
+
+  static final RequestBuilderInjectAdapter SETTER = new RequestBuilderInjectAdapter();
+
+  @Override
+  public void set(Request.@Nullable Builder carrier, String key, String value) {
+    if (carrier == null) {
+      return;
+    }
+    carrier.addHeader(key, value);
+  }
+}

instrumentation/okhttp/okhttp-3.0/javaagent/okhttp-3.0-javaagent.gradle

@@ -19,12 +19,9 @@ because it looks like exclusions using configurations excludes dependency even i
 not transitive.
  */
 dependencies {
+  implementation project(':instrumentation:okhttp:okhttp-3.0:library')
+
   library(group: 'com.squareup.okhttp3', name: 'okhttp', version: '3.0.0')
 
-  testImplementation(project(':testing-common')) {
-    exclude module: 'okhttp'
-  }
-
-  // 4.x.x-alpha has been released and it looks like there are lots of incompatible changes
-  latestDepTestLibrary group: 'com.squareup.okhttp3', name: 'okhttp', version: '[3.11.0, 4.0.0*)'
+  testImplementation project(':instrumentation:okhttp:okhttp-3.0:testing')
 }

instrumentation/okhttp/okhttp-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/okhttp/v3_0/OkHttp3InstrumentationModule.java

@@ -20,7 +20,6 @@ import net.bytebuddy.asm.Advice;
 import net.bytebuddy.description.method.MethodDescription;
 import net.bytebuddy.description.type.TypeDescription;
 import net.bytebuddy.matcher.ElementMatcher;
-import okhttp3.Interceptor;
 import okhttp3.OkHttpClient;
 
 @AutoService(InstrumentationModule.class)
@@ -52,13 +51,7 @@ public class OkHttp3InstrumentationModule extends InstrumentationModule {
   public static class OkHttp3Advice {
     @Advice.OnMethodEnter(suppress = Throwable.class)
     public static void addTracingInterceptor(@Advice.Argument(0) OkHttpClient.Builder builder) {
-      for (Interceptor interceptor : builder.interceptors()) {
-        if (interceptor instanceof TracingInterceptor) {
-          return;
-        }
-      }
-      TracingInterceptor interceptor = new TracingInterceptor();
-      builder.addInterceptor(interceptor);
+      builder.addInterceptor(OkHttp3Interceptors.TRACING_INTERCEPTOR);
     }
   }
 }

instrumentation/okhttp/okhttp-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/okhttp/v3_0/OkHttp3Interceptors.java

@@ -0,0 +1,17 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.okhttp.v3_0;
+
+import io.opentelemetry.api.GlobalOpenTelemetry;
+import io.opentelemetry.instrumentation.okhttp.v3_0.OkHttpTracing;
+import okhttp3.Interceptor;
+
+/** Holder of singleton interceptors for adding to instrumented clients. */
+public class OkHttp3Interceptors {
+
+  public static final Interceptor TRACING_INTERCEPTOR =
+      OkHttpTracing.create(GlobalOpenTelemetry.get()).newInterceptor();
+}

instrumentation/okhttp/okhttp-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/okhttp/v3_0/RequestBuilderInjectAdapter.java

@@ -1,24 +0,0 @@
-/*
- * Copyright The OpenTelemetry Authors
- * SPDX-License-Identifier: Apache-2.0
- */
-
-package io.opentelemetry.javaagent.instrumentation.okhttp.v3_0;
-
-import io.opentelemetry.context.propagation.TextMapSetter;
-import okhttp3.Request;
-
-/**
- * Helper class to inject span context into request headers.
- *
- * @author Pavol Loffay
- */
-public class RequestBuilderInjectAdapter implements TextMapSetter<Request.Builder> {
-
-  public static final RequestBuilderInjectAdapter SETTER = new RequestBuilderInjectAdapter();
-
-  @Override
-  public void set(Request.Builder carrier, String key, String value) {
-    carrier.addHeader(key, value);
-  }
-}

instrumentation/okhttp/okhttp-3.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/okhttp/v3_0/OkHttp3AsyncTest.groovy

@@ -0,0 +1,17 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.okhttp.v3_0
+
+import io.opentelemetry.instrumentation.okhttp.v3_0.AbstractOkHttp3AsyncTest
+import io.opentelemetry.instrumentation.test.AgentTestTrait
+import okhttp3.OkHttpClient
+
+class OkHttp3AsyncTest extends AbstractOkHttp3AsyncTest implements AgentTestTrait {
+  @Override
+  OkHttpClient.Builder configureClient(OkHttpClient.Builder clientBuilder) {
+    return clientBuilder
+  }
+}

instrumentation/okhttp/okhttp-3.0/javaagent/src/test/groovy/io/opentelemetry/javaagent/instrumentation/okhttp/v3_0/OkHttp3Test.groovy

@@ -0,0 +1,17 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.instrumentation.okhttp.v3_0
+
+import io.opentelemetry.instrumentation.okhttp.v3_0.AbstractOkHttp3Test
+import io.opentelemetry.instrumentation.test.AgentTestTrait
+import okhttp3.OkHttpClient
+
+class OkHttp3Test extends AbstractOkHttp3Test implements AgentTestTrait {
+  @Override
+  OkHttpClient.Builder configureClient(OkHttpClient.Builder clientBuilder) {
+    return clientBuilder
+  }
+}

instrumentation/okhttp/okhttp-3.0/library/okhttp-3.0-library.gradle

@@ -0,0 +1,8 @@
+apply from: "$rootDir/gradle/instrumentation-library.gradle"
+apply plugin: "net.ltgt.errorprone"
+
+dependencies {
+  library group: 'com.squareup.okhttp3', name: 'okhttp', version: '3.0.0'
+
+  testImplementation project(':instrumentation:okhttp:okhttp-3.0:testing')
+}

instrumentation/okhttp/okhttp-3.0/library/src/main/java/io/opentelemetry/instrumentation/okhttp/v3_0/OkHttpClientTracer.java

@@ -3,21 +3,21 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-package io.opentelemetry.javaagent.instrumentation.okhttp.v3_0;
+package io.opentelemetry.instrumentation.okhttp.v3_0;
 
-import static io.opentelemetry.javaagent.instrumentation.okhttp.v3_0.RequestBuilderInjectAdapter.SETTER;
+import static io.opentelemetry.instrumentation.okhttp.v3_0.RequestBuilderInjectAdapter.SETTER;
 
+import io.opentelemetry.api.OpenTelemetry;
 import io.opentelemetry.context.propagation.TextMapSetter;
 import io.opentelemetry.instrumentation.api.tracer.HttpClientTracer;
 import java.net.URI;
 import okhttp3.Request;
 import okhttp3.Response;
 
-public class OkHttpClientTracer extends HttpClientTracer<Request, Request.Builder, Response> {
-  private static final OkHttpClientTracer TRACER = new OkHttpClientTracer();
+final class OkHttpClientTracer extends HttpClientTracer<Request, Request.Builder, Response> {
 
-  public static OkHttpClientTracer tracer() {
-    return TRACER;
+  OkHttpClientTracer(OpenTelemetry openTelemetry) {
+    super(openTelemetry);
   }
 
   @Override

instrumentation/okhttp/okhttp-3.0/library/src/main/java/io/opentelemetry/instrumentation/okhttp/v3_0/OkHttpTracing.java

@@ -0,0 +1,32 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.okhttp.v3_0;
+
+import io.opentelemetry.api.OpenTelemetry;
+import okhttp3.Interceptor;
+
+/** Entrypoint for tracing OkHttp clients. */
+public final class OkHttpTracing {
+
+  /** Returns a new {@link OkHttpTracing} configured with the given {@link OpenTelemetry}. */
+  public static OkHttpTracing create(OpenTelemetry openTelemetry) {
+    return new OkHttpTracing(openTelemetry);
+  }
+
+  private final OkHttpClientTracer tracer;
+
+  private OkHttpTracing(OpenTelemetry openTelemetry) {
+    this.tracer = new OkHttpClientTracer(openTelemetry);
+  }
+
+  /**
+   * Returns a new {@link Interceptor} that can be used with methods like {@link
+   * okhttp3.OkHttpClient.Builder#addInterceptor(Interceptor)}.
+   */
+  public Interceptor newInterceptor() {
+    return new TracingInterceptor(tracer);
+  }
+}

instrumentation/okhttp/okhttp-3.0/library/src/main/java/io/opentelemetry/instrumentation/okhttp/v3_0/RequestBuilderInjectAdapter.java

@@ -0,0 +1,28 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.okhttp.v3_0;
+
+import io.opentelemetry.context.propagation.TextMapSetter;
+import okhttp3.Request;
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+/**
+ * Helper class to inject span context into request headers.
+ *
+ * @author Pavol Loffay
+ */
+final class RequestBuilderInjectAdapter implements TextMapSetter<Request.Builder> {
+
+  static final RequestBuilderInjectAdapter SETTER = new RequestBuilderInjectAdapter();
+
+  @Override
+  public void set(Request.@Nullable Builder carrier, String key, String value) {
+    if (carrier == null) {
+      return;
+    }
+    carrier.addHeader(key, value);
+  }
+}

instrumentation/okhttp/okhttp-3.0/library/src/main/java/io/opentelemetry/instrumentation/okhttp/v3_0/TracingInterceptor.java

@@ -3,9 +3,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-package io.opentelemetry.javaagent.instrumentation.okhttp.v3_0;
-
-import static io.opentelemetry.javaagent.instrumentation.okhttp.v3_0.OkHttpClientTracer.tracer;
+package io.opentelemetry.instrumentation.okhttp.v3_0;
 
 import io.opentelemetry.context.Context;
 import io.opentelemetry.context.Scope;
@@ -14,26 +12,32 @@ import okhttp3.Interceptor;
 import okhttp3.Request;
 import okhttp3.Response;
 
-public class TracingInterceptor implements Interceptor {
+final class TracingInterceptor implements Interceptor {
+
+  private final OkHttpClientTracer tracer;
+
+  TracingInterceptor(OkHttpClientTracer tracer) {
+    this.tracer = tracer;
+  }
 
   @Override
   public Response intercept(Chain chain) throws IOException {
     Context parentContext = Context.current();
-    if (!tracer().shouldStartSpan(parentContext)) {
+    if (!tracer.shouldStartSpan(parentContext)) {
       return chain.proceed(chain.request());
     }
 
     Request.Builder requestBuilder = chain.request().newBuilder();
-    Context context = tracer().startSpan(parentContext, chain.request(), requestBuilder);
+    Context context = tracer.startSpan(parentContext, chain.request(), requestBuilder);
 
     Response response;
     try (Scope ignored = context.makeCurrent()) {
       response = chain.proceed(requestBuilder.build());
     } catch (Exception e) {
-      tracer().endExceptionally(context, e);
+      tracer.endExceptionally(context, e);
       throw e;
     }
-    tracer().end(context, response);
+    tracer.end(context, response);
     return response;
   }
 }

instrumentation/okhttp/okhttp-3.0/library/src/test/groovy/io/opentelemetry/instrumentation/okhttp/v3_0/OkHttp3AsyncTest.groovy

@@ -0,0 +1,19 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.okhttp.v3_0
+
+import io.opentelemetry.instrumentation.test.LibraryTestTrait
+import okhttp3.OkHttpClient
+import spock.lang.Ignore
+
+// Async test relies on javaagent concurrency instrumentation currently.
+@Ignore
+class OkHttp3AsyncTest extends AbstractOkHttp3AsyncTest implements LibraryTestTrait {
+  @Override
+  OkHttpClient.Builder configureClient(OkHttpClient.Builder clientBuilder) {
+    return clientBuilder.addInterceptor(OkHttpTracing.create(getOpenTelemetry()).newInterceptor())
+  }
+}

instrumentation/okhttp/okhttp-3.0/library/src/test/groovy/io/opentelemetry/instrumentation/okhttp/v3_0/OkHttp3Test.groovy

@@ -0,0 +1,16 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.okhttp.v3_0
+
+import io.opentelemetry.instrumentation.test.LibraryTestTrait
+import okhttp3.OkHttpClient
+
+class OkHttp3Test extends AbstractOkHttp3Test implements LibraryTestTrait {
+  @Override
+  OkHttpClient.Builder configureClient(OkHttpClient.Builder clientBuilder) {
+    return clientBuilder.addInterceptor(OkHttpTracing.create(getOpenTelemetry()).newInterceptor())
+  }
+}

instrumentation/okhttp/okhttp-3.0/testing/okhttp-3.0-testing.gradle

@@ -0,0 +1,15 @@
+apply from: "$rootDir/gradle/java.gradle"
+
+dependencies {
+  api(project(':testing-common')) {
+    exclude module: 'okhttp'
+  }
+
+  api group: 'com.squareup.okhttp3', name: 'okhttp', version: '3.0.0'
+
+  implementation deps.guava
+
+  implementation deps.groovy
+  implementation deps.opentelemetryApi
+  implementation deps.spock
+}

instrumentation/okhttp/okhttp-3.0/testing/src/main/groovy/io/opentelemetry/instrumentation/okhttp/v3_0/AbstractOkHttp3AsyncTest.groovy

@@ -3,6 +3,8 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+package io.opentelemetry.instrumentation.okhttp.v3_0
+
 import static java.util.concurrent.TimeUnit.SECONDS
 
 import java.util.concurrent.CountDownLatch
@@ -16,7 +18,7 @@ import okhttp3.RequestBody
 import okhttp3.Response
 import okhttp3.internal.http.HttpMethod
 
-class OkHttp3AsyncTest extends OkHttp3Test {
+abstract class AbstractOkHttp3AsyncTest extends AbstractOkHttp3Test {
 
   @Override
   int doRequest(String method, URI uri, Map<String, String> headers, Closure callback) {

instrumentation/okhttp/okhttp-3.0/testing/src/main/groovy/io/opentelemetry/instrumentation/okhttp/v3_0/AbstractOkHttp3Test.groovy

@@ -3,7 +3,8 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import io.opentelemetry.instrumentation.test.AgentTestTrait
+package io.opentelemetry.instrumentation.okhttp.v3_0
+
 import io.opentelemetry.instrumentation.test.base.HttpClientTest
 import java.util.concurrent.TimeUnit
 import okhttp3.Headers
@@ -12,14 +13,19 @@ import okhttp3.OkHttpClient
 import okhttp3.Request
 import okhttp3.RequestBody
 import okhttp3.internal.http.HttpMethod
+import spock.lang.Shared
 import spock.lang.Timeout
 
 @Timeout(5)
-class OkHttp3Test extends HttpClientTest implements AgentTestTrait {
+abstract class AbstractOkHttp3Test extends HttpClientTest {
 
-  def client = new OkHttpClient.Builder()
-    .connectTimeout(CONNECT_TIMEOUT_MS, TimeUnit.MILLISECONDS)
-    .retryOnConnectionFailure(false)
+  abstract OkHttpClient.Builder configureClient(OkHttpClient.Builder clientBuilder)
+
+  @Shared
+  def client = configureClient(
+    new OkHttpClient.Builder()
+      .connectTimeout(CONNECT_TIMEOUT_MS, TimeUnit.MILLISECONDS)
+      .retryOnConnectionFailure(false))
     .build()
 
   @Override

settings.gradle

@@ -182,6 +182,8 @@ include ':instrumentation:netty:netty-4.1:library'
 include ':instrumentation:netty:netty-4.1:javaagent'
 include ':instrumentation:okhttp:okhttp-2.2:javaagent'
 include ':instrumentation:okhttp:okhttp-3.0:javaagent'
+include ':instrumentation:okhttp:okhttp-3.0:library'
+include ':instrumentation:okhttp:okhttp-3.0:testing'
 include ':instrumentation:opentelemetry-annotations-1.0:javaagent'
 include ':instrumentation:opentelemetry-api-1.0:javaagent'
 include ':instrumentation:opentelemetry-api-metrics-1.0:javaagent'