Okhttp autoinstrumentation stabilization

auto-instrumentation/okhttp/okhttp-3.0/README.md

@@ -1,7 +1,5 @@
 # Android Instrumentation for OkHttp version 3.0 and higher
 
-# 🚧Work in progress 🚧
-
 Provides OpenTelemetry instrumentation for [okhttp3](https://square.github.io/okhttp/).
 
 ## Quickstart

auto-instrumentation/okhttp/okhttp-3.0/agent/build.gradle.kts

@@ -1,10 +1,14 @@
 plugins {
-    id("otel.java-library-conventions")
+    id("otel.android-library-conventions")
     id("otel.publish-conventions")
 }
 
 description = "OpenTelemetry build-time auto-instrumentation for OkHttp on Android"
 
+android {
+    namespace = "io.opentelemetry.android.okhttp.agent"
+}
+
 dependencies {
     implementation(project(":auto-instrumentation:okhttp:okhttp-3.0:library"))
     implementation(libs.okhttp)

auto-instrumentation/okhttp/okhttp-3.0/library/build.gradle.kts

@@ -1,10 +1,18 @@
 plugins {
-    id("otel.java-library-conventions")
+    id("otel.android-library-conventions")
     id("otel.publish-conventions")
 }
 
 description = "OpenTelemetry OkHttp library instrumentation for Android"
 
+android {
+    namespace = "io.opentelemetry.android.okhttp.library"
+
+    defaultConfig {
+        consumerProguardFiles("consumer-rules.pro")
+    }
+}
+
 dependencies {
     compileOnly(libs.okhttp)
     api(libs.opentelemetry.instrumentation.okhttp)

auto-instrumentation/okhttp/okhttp-3.0/library/consumer-rules.pro

@@ -0,0 +1 @@
+-keep class io.opentelemetry.exporter.internal.InstrumentationUtil { *; }

auto-instrumentation/okhttp/okhttp-3.0/library/src/main/java/io/opentelemetry/instrumentation/library/okhttp/v3_0/internal/CachedSupplier.java

@@ -0,0 +1,36 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.library.okhttp.v3_0.internal;
+
+import java.util.function.Supplier;
+
+/**
+ * This class is internal and is hence not for public use. Its APIs are unstable and can change at
+ * any time.
+ */
+public class CachedSupplier<T> implements Supplier<T> {
+    private Supplier<T> supplier;
+    private T instance;
+
+    public static <T> CachedSupplier<T> create(Supplier<T> instance) {
+        return new CachedSupplier<>(instance);
+    }
+
+    private CachedSupplier(Supplier<T> supplier) {
+        this.supplier = supplier;
+    }
+
+    @Override
+    public T get() {
+        synchronized (this) {
+            if (instance == null) {
+                instance = supplier.get();
+                supplier = null;
+            }
+            return instance;
+        }
+    }
+}

auto-instrumentation/okhttp/okhttp-3.0/library/src/main/java/io/opentelemetry/instrumentation/library/okhttp/v3_0/internal/LazyInterceptor.java

@@ -0,0 +1,29 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.library.okhttp.v3_0.internal;
+
+import java.io.IOException;
+import okhttp3.Interceptor;
+import okhttp3.Response;
+import org.jetbrains.annotations.NotNull;
+
+/**
+ * This class is internal and is hence not for public use. Its APIs are unstable and can change at
+ * any time.
+ */
+public class LazyInterceptor<T extends Interceptor> implements Interceptor {
+    private final CachedSupplier<T> interceptorSupplier;
+
+    public LazyInterceptor(CachedSupplier<T> interceptorSupplier) {
+        this.interceptorSupplier = interceptorSupplier;
+    }
+
+    @NotNull
+    @Override
+    public Response intercept(@NotNull Chain chain) throws IOException {
+        return interceptorSupplier.get().intercept(chain);
+    }
+}

auto-instrumentation/okhttp/okhttp-3.0/library/src/main/java/io/opentelemetry/instrumentation/library/okhttp/v3_0/internal/OkHttp3Singletons.java

@@ -18,6 +18,7 @@
 import io.opentelemetry.instrumentation.okhttp.v3_0.internal.OkHttpAttributesGetter;
 import io.opentelemetry.instrumentation.okhttp.v3_0.internal.OkHttpInstrumenterFactory;
 import io.opentelemetry.instrumentation.okhttp.v3_0.internal.TracingInterceptor;
+import java.util.function.Supplier;
 import okhttp3.Interceptor;
 import okhttp3.Request;
 import okhttp3.Response;
@@ -28,24 +29,31 @@
  */
 public final class OkHttp3Singletons {
 
-    private static final Instrumenter<Request, Response> INSTRUMENTER =
-            OkHttpInstrumenterFactory.create(
-                    GlobalOpenTelemetry.get(),
-                    builder ->
-                            builder.setCapturedRequestHeaders(
-                                            OkHttpInstrumentationConfig.getCapturedRequestHeaders())
-                                    .setCapturedResponseHeaders(
-                                            OkHttpInstrumentationConfig
-                                                    .getCapturedResponseHeaders())
-                                    .setKnownMethods(OkHttpInstrumentationConfig.getKnownMethods()),
-                    spanNameExtractorConfigurer ->
-                            spanNameExtractorConfigurer.setKnownMethods(
-                                    OkHttpInstrumentationConfig.getKnownMethods()),
-                    singletonList(
-                            PeerServiceAttributesExtractor.create(
-                                    OkHttpAttributesGetter.INSTANCE,
-                                    OkHttpInstrumentationConfig.newPeerServiceResolver())),
-                    OkHttpInstrumentationConfig.emitExperimentalHttpClientMetrics());
+    private static final Supplier<Instrumenter<Request, Response>> INSTRUMENTER =
+            CachedSupplier.create(
+                    () ->
+                            OkHttpInstrumenterFactory.create(
+                                    GlobalOpenTelemetry.get(),
+                                    builder ->
+                                            builder.setCapturedRequestHeaders(
+                                                            OkHttpInstrumentationConfig
+                                                                    .getCapturedRequestHeaders())
+                                                    .setCapturedResponseHeaders(
+                                                            OkHttpInstrumentationConfig
+                                                                    .getCapturedResponseHeaders())
+                                                    .setKnownMethods(
+                                                            OkHttpInstrumentationConfig
+                                                                    .getKnownMethods()),
+                                    spanNameExtractorConfigurer ->
+                                            spanNameExtractorConfigurer.setKnownMethods(
+                                                    OkHttpInstrumentationConfig.getKnownMethods()),
+                                    singletonList(
+                                            PeerServiceAttributesExtractor.create(
+                                                    OkHttpAttributesGetter.INSTANCE,
+                                                    OkHttpInstrumentationConfig
+                                                            .newPeerServiceResolver())),
+                                    OkHttpInstrumentationConfig
+                                            .emitExperimentalHttpClientMetrics()));
 
     public static final Interceptor CALLBACK_CONTEXT_INTERCEPTOR =
             chain -> {
@@ -70,10 +78,17 @@ public final class OkHttp3Singletons {
             };
 
     public static final Interceptor CONNECTION_ERROR_INTERCEPTOR =
-            new ConnectionErrorSpanInterceptor(INSTRUMENTER);
+            new LazyInterceptor<>(
+                    CachedSupplier.create(
+                            () -> new ConnectionErrorSpanInterceptor(INSTRUMENTER.get())));
 
     public static final Interceptor TRACING_INTERCEPTOR =
-            new TracingInterceptor(INSTRUMENTER, GlobalOpenTelemetry.getPropagators());
+            new LazyInterceptor<>(
+                    CachedSupplier.create(
+                            () ->
+                                    new TracingInterceptor(
+                                            INSTRUMENTER.get(),
+                                            GlobalOpenTelemetry.getPropagators())));
 
     private OkHttp3Singletons() {}
 }

auto-instrumentation/okhttp/okhttp-3.0/testing/build.gradle.kts

@@ -3,9 +3,20 @@ plugins {
     id("net.bytebuddy.byte-buddy-gradle-plugin")
 }
 
+android {
+    buildTypes {
+        debug {
+            isMinifyEnabled = true
+            proguardFile("proguard-rules.pro")
+            testProguardFile("proguard-test-rules.pro")
+        }
+    }
+}
+
 dependencies {
     byteBuddy(project(":auto-instrumentation:okhttp:okhttp-3.0:agent"))
     implementation(project(":auto-instrumentation:okhttp:okhttp-3.0:library"))
     implementation(libs.okhttp)
+    implementation(libs.opentelemetry.exporter.otlp)
     androidTestImplementation(libs.okhttp.mockwebserver)
 }

auto-instrumentation/okhttp/okhttp-3.0/testing/proguard-rules.pro

@@ -0,0 +1,7 @@
+-keep class kotlin.** { *; }
+-keep class okhttp3.** { *; }
+-keep class io.opentelemetry.api.** { *; }
+-keep class io.opentelemetry.sdk.** { *; }
+-keep class io.opentelemetry.exporter.otlp.http.trace.OtlpHttpSpanExporter { *; }
+-keep class io.opentelemetry.exporter.otlp.http.trace.OtlpHttpSpanExporterBuilder { *; }
+-dontwarn *

auto-instrumentation/okhttp/okhttp-3.0/testing/proguard-test-rules.pro

@@ -0,0 +1 @@
+-include proguard-rules.pro

auto-instrumentation/okhttp/okhttp-3.0/testing/src/androidTest/java/io/opentelemetry/instrumentation/library/okhttp/v3_0/InstrumentationTest.java

@@ -12,10 +12,12 @@
 import io.opentelemetry.api.trace.Span;
 import io.opentelemetry.api.trace.SpanContext;
 import io.opentelemetry.context.Scope;
+import io.opentelemetry.exporter.otlp.http.trace.OtlpHttpSpanExporter;
 import io.opentelemetry.sdk.OpenTelemetrySdk;
 import io.opentelemetry.sdk.testing.exporter.InMemorySpanExporter;
 import io.opentelemetry.sdk.trace.SdkTracerProvider;
 import io.opentelemetry.sdk.trace.export.SimpleSpanProcessor;
+import io.opentelemetry.sdk.trace.export.SpanExporter;
 import java.io.IOException;
 import java.util.concurrent.CountDownLatch;
 import okhttp3.Call;
@@ -27,17 +29,11 @@
 import okhttp3.mockwebserver.MockWebServer;
 import org.junit.After;
 import org.junit.Before;
-import org.junit.BeforeClass;
 import org.junit.Test;
 
 public class InstrumentationTest {
     private MockWebServer server;
-    private static InMemorySpanExporter spanExporter;
-
-    @BeforeClass
-    public static void setUpAll() {
-        setUpInMemorySpanExporter();
-    }
+    private static final InMemorySpanExporter inMemorySpanExporter = InMemorySpanExporter.create();
 
     @Before
     public void setUp() throws IOException {
@@ -48,11 +44,12 @@ public void setUp() throws IOException {
     @After
     public void tearDown() throws IOException {
         server.shutdown();
-        spanExporter.reset();
+        inMemorySpanExporter.reset();
     }
 
     @Test
     public void okhttpTraces() throws IOException {
+        setUpSpanExporter(inMemorySpanExporter);
         server.enqueue(new MockResponse().setResponseCode(200));
 
         Span span = getSpan();
@@ -74,11 +71,12 @@ public void okhttpTraces() throws IOException {
 
         span.end();
 
-        assertEquals(2, spanExporter.getFinishedSpanItems().size());
+        assertEquals(2, inMemorySpanExporter.getFinishedSpanItems().size());
     }
 
     @Test
     public void okhttpTraces_with_callback() throws InterruptedException {
+        setUpSpanExporter(inMemorySpanExporter);
         CountDownLatch lock = new CountDownLatch(1);
         Span span = getSpan();
 
@@ -117,18 +115,49 @@ public void onResponse(
         lock.await();
         span.end();
 
-        assertEquals(2, spanExporter.getFinishedSpanItems().size());
+        assertEquals(2, inMemorySpanExporter.getFinishedSpanItems().size());
+    }
+
+    @Test
+    public void avoidCreatingSpansForInternalOkhttpRequests() throws InterruptedException {
+        // NOTE: For some reason this test always passes when running all the tests in this file at
+        // once,
+        // so it should be run isolated to actually get it to fail when it's expected to fail.
+        OtlpHttpSpanExporter exporter =
+                OtlpHttpSpanExporter.builder().setEndpoint(server.url("").toString()).build();
+        setUpSpanExporter(exporter);
+
+        server.enqueue(new MockResponse().setResponseCode(200));
+
+        // This span should trigger 1 export okhttp call, which is the only okhttp call expected
+        // for this test case.
+        getSpan().end();
+
+        // Wait for unwanted extra okhttp requests.
+        int loop = 0;
+        while (loop < 10) {
+            Thread.sleep(100);
+            // Stop waiting if we get at least one unwanted request.
+            if (server.getRequestCount() > 1) {
+                break;
+            }
+            loop++;
+        }
+
+        assertEquals(1, server.getRequestCount());
     }
 
     private static Span getSpan() {
         return GlobalOpenTelemetry.getTracer("TestTracer").spanBuilder("A Span").startSpan();
     }
 
-    private static void setUpInMemorySpanExporter() {
-        spanExporter = InMemorySpanExporter.create();
-        OpenTelemetrySdk.builder()
-                .setTracerProvider(getSimpleTracerProvider(spanExporter))
-                .buildAndRegisterGlobal();
+    private void setUpSpanExporter(SpanExporter spanExporter) {
+        OpenTelemetrySdk openTelemetry =
+                OpenTelemetrySdk.builder()
+                        .setTracerProvider(getSimpleTracerProvider(spanExporter))
+                        .build();
+        GlobalOpenTelemetry.resetForTest();
+        GlobalOpenTelemetry.set(openTelemetry);
     }
 
     private Call createCall(OkHttpClient client, String urlPath) {
@@ -137,7 +166,7 @@ private Call createCall(OkHttpClient client, String urlPath) {
     }
 
     @NonNull
-    private static SdkTracerProvider getSimpleTracerProvider(InMemorySpanExporter spanExporter) {
+    private SdkTracerProvider getSimpleTracerProvider(SpanExporter spanExporter) {
         return SdkTracerProvider.builder()
                 .addSpanProcessor(SimpleSpanProcessor.create(spanExporter))
                 .build();

buildSrc/src/main/kotlin/otel.android-library-conventions.gradle.kts

@@ -24,10 +24,12 @@ android {
         val javaVersion = rootProject.extra["java_version"] as JavaVersion
         sourceCompatibility(javaVersion)
         targetCompatibility(javaVersion)
+        isCoreLibraryDesugaringEnabled = true
     }
 }
 
 val libs = extensions.getByType<VersionCatalogsExtension>().named("libs")
 dependencies {
     implementation(libs.findLibrary("findbugs-jsr305").get())
+    coreLibraryDesugaring(libs.findLibrary("desugarJdkLibs").get())
 }

gradle/libs.versions.toml

@@ -28,6 +28,7 @@ opentelemetry-exporter-zipkin = { module = "io.opentelemetry:opentelemetry-expor
 opentelemetry-exporter-logging = { module = "io.opentelemetry:opentelemetry-exporter-logging" }
 zipkin-sender-okhttp3 = { module = "io.zipkin.reporter2:zipkin-sender-okhttp3", version.ref = "zipkin-reporter" }
 opentelemetry-diskBuffering = { module = "io.opentelemetry.contrib:opentelemetry-disk-buffering", version.ref = "opentelemetry-contrib" }
+opentelemetry-exporter-otlp = { module = "io.opentelemetry:opentelemetry-exporter-otlp", version.ref = "opentelemetry" }
 
 #Test tools
 opentelemetry-sdk-testing = { module = "io.opentelemetry:opentelemetry-sdk-testing", version.ref = "opentelemetry" }

instrumentation/build.gradle.kts

@@ -47,10 +47,6 @@ android {
         }
     }
 
-    compileOptions {
-        isCoreLibraryDesugaringEnabled = true
-    }
-
     testOptions {
         unitTests.isReturnDefaultValues = true
         unitTests.isIncludeAndroidResources = true
@@ -79,8 +75,6 @@ dependencies {
     testImplementation(libs.androidx.test.core)
     testImplementation(libs.assertj.core)
     testImplementation(libs.awaitility)
-
-    coreLibraryDesugaring(libs.desugarJdkLibs)
 }
 
 tasks.withType<Test> {