Review actions.

- Moved the functions in JdkHttpUtil to JdkHttpSender.
- Updated test.

Author: SoftlySplinter

exporters/sender/jdk/src/main/java/io/opentelemetry/exporter/sender/jdk/internal/JdkHttpSender.java

@@ -13,6 +13,7 @@
 import io.opentelemetry.sdk.common.CompletableResultCode;
 import io.opentelemetry.sdk.common.export.ProxyOptions;
 import io.opentelemetry.sdk.common.export.RetryPolicy;
+import io.opentelemetry.sdk.internal.DaemonThreadFactory;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.OutputStream;
@@ -32,7 +33,9 @@
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ConcurrentLinkedQueue;
 import java.util.concurrent.ExecutorService;
+import java.util.concurrent.SynchronousQueue;
 import java.util.concurrent.ThreadLocalRandom;
+import java.util.concurrent.ThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Consumer;
 import java.util.function.Predicate;
@@ -72,6 +75,23 @@ public final class JdkHttpSender implements HttpSender {
   @Nullable private final RetryPolicy retryPolicy;
   private final Predicate<IOException> retryExceptionPredicate;
 
+  /**
+   * Returns an {@link ExecutorService} using daemon threads.
+   *
+   * @param propagateContextForTesting For tests only. When enabled, the current thread's Context
+   *     will be passed over to the new threads, this is useful for validating scenarios where
+   *     context propagation is available through bytecode instrumentation.
+   */
+  public static ExecutorService newExecutor(boolean propagateContextForTesting) {
+    return new ThreadPoolExecutor(
+        0,
+        Integer.MAX_VALUE,
+        60,
+        TimeUnit.SECONDS,
+        new SynchronousQueue<>(),
+        new DaemonThreadFactory("jdkhttp-executor", propagateContextForTesting));
+  }
+
   // Visible for testing
   JdkHttpSender(
       HttpClient client,
@@ -82,7 +102,8 @@ public final class JdkHttpSender implements HttpSender {
       long timeoutNanos,
       Supplier<Map<String, List<String>>> headerSupplier,
       @Nullable RetryPolicy retryPolicy,
-      @Nullable ExecutorService executorService) {
+      @Nullable ExecutorService executorService,
+      boolean propagateContextForTesting) {
     this.client = client;
     try {
       this.uri = new URI(endpoint);
@@ -100,7 +121,7 @@ public final class JdkHttpSender implements HttpSender {
             .map(RetryPolicy::getRetryExceptionPredicate)
             .orElse(JdkHttpSender::isRetryableException);
     if (executorService == null) {
-      this.executorService = JdkHtttpUtil.newExecutor();
+      this.executorService = JdkHttpSender.newExecutor(propagateContextForTesting);
       this.managedExecutor = true;
     } else {
       this.executorService = executorService;
@@ -129,7 +150,8 @@ public final class JdkHttpSender implements HttpSender {
         timeoutNanos,
         headerSupplier,
         retryPolicy,
-        executorService);
+        executorService,
+        /* propagateContextForTesting= */ false);
   }
 
   private static HttpClient configureClient(
@@ -223,7 +245,8 @@ HttpResponse<byte[]> sendInternal(Marshaler marshaler) throws IOException {
           Thread.currentThread().interrupt();
           break; // Break out and return response or throw
         }
-        // If after sleeping we've exceeded timeoutNanos, break out and return response or throw
+        // If after sleeping we've exceeded timeoutNanos, break out and return
+        // response or throw
         if ((System.nanoTime() - startTimeNanos) >= timeoutNanos) {
           break;
         }
@@ -304,12 +327,15 @@ private HttpResponse<byte[]> sendRequest(
   }
 
   private static boolean isRetryableException(IOException throwable) {
-    // Almost all IOExceptions we've encountered are transient retryable, so we opt out of specific
+    // Almost all IOExceptions we've encountered are transient retryable, so we
+    // opt out of specific
     // IOExceptions that are unlikely to resolve rather than opting in.
-    // Known retryable IOException messages: "Connection reset", "/{remote ip}:{remote port} GOAWAY
+    // Known retryable IOException messages: "Connection reset", "/{remote
+    // ip}:{remote port} GOAWAY
     // received"
     // Known retryable HttpTimeoutException messages: "request timed out"
-    // Known retryable HttpConnectTimeoutException messages: "HTTP connect timed out"
+    // Known retryable HttpConnectTimeoutException messages: "HTTP connect timed
+    // out"
     return !(throwable instanceof SSLException);
   }
 

exporters/sender/jdk/src/main/java/io/opentelemetry/exporter/sender/jdk/internal/JdkHtttpUtil.java

@@ -27,7 +27,6 @@
 import java.util.concurrent.TimeUnit;
 import javax.net.ssl.SSLException;
 import org.assertj.core.api.InstanceOfAssertFactories;
-import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
@@ -47,8 +46,6 @@ class JdkHttpSenderTest {
 
   @BeforeEach
   void setup() throws IOException, InterruptedException {
-    JdkHtttpUtil.setPropagateContextForTestingInDispatcher(true);
-
     // Can't directly spy on HttpClient for some reason, so create a real instance and a mock that
     // delegates to the real thing
     when(mockHttpClient.send(any(), any()))
@@ -66,12 +63,8 @@ void setup() throws IOException, InterruptedException {
             Duration.ofSeconds(10).toNanos(),
             Collections::emptyMap,
             RetryPolicy.builder().setMaxAttempts(2).setInitialBackoff(Duration.ofMillis(1)).build(),
-            null);
-  }
-
-  @AfterEach
-  void tearDown() {
-    JdkHtttpUtil.setPropagateContextForTestingInDispatcher(false);
+            null,
+            true);
   }
 
   @Test
@@ -97,7 +90,8 @@ void sendInternal_RetryableConnectException() throws IOException, InterruptedExc
             Duration.ofSeconds(10).toNanos(),
             Collections::emptyMap,
             RetryPolicy.builder().setMaxAttempts(2).setInitialBackoff(Duration.ofMillis(1)).build(),
-            null);
+            null,
+            true);
 
     assertThatThrownBy(() -> sender.sendInternal(new NoOpMarshaler()))
         .satisfies(