Dont shutdown executor on shutdown when setExecutorService is called

Author: jack-berg

exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/http/logs/OtlpHttpLogRecordExporterBuilder.java

@@ -233,7 +233,13 @@ public OtlpHttpLogRecordExporterBuilder setServiceClassLoader(ClassLoader servic
     return this;
   }
 
-  /** Set the {@link ExecutorService} used to execute requests. */
+  /**
+   * Set the {@link ExecutorService} used to execute requests.
+   *
+   * <p>NOTE: By calling this method, you are opting into managing the lifecycle of the {@code
+   * executorService}. {@link ExecutorService#shutdown()} will NOT be called when this exporter is
+   * shutdown.
+   */
   public OtlpHttpLogRecordExporterBuilder setExecutorService(ExecutorService executorService) {
     requireNonNull(executorService, "executorService");
     delegate.setExecutorService(executorService);

exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/http/metrics/OtlpHttpMetricExporterBuilder.java

@@ -261,7 +261,13 @@ public OtlpHttpMetricExporterBuilder setServiceClassLoader(ClassLoader serviceCl
     return this;
   }
 
-  /** Set the {@link ExecutorService} used to execute requests. */
+  /**
+   * Set the {@link ExecutorService} used to execute requests.
+   *
+   * <p>NOTE: By calling this method, you are opting into managing the lifecycle of the {@code
+   * executorService}. {@link ExecutorService#shutdown()} will NOT be called when this exporter is
+   * shutdown.
+   */
   public OtlpHttpMetricExporterBuilder setExecutorService(ExecutorService executorService) {
     requireNonNull(executorService, "executorService");
     delegate.setExecutorService(executorService);

exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/http/trace/OtlpHttpSpanExporterBuilder.java

@@ -234,7 +234,13 @@ public OtlpHttpSpanExporterBuilder setServiceClassLoader(ClassLoader serviceClas
     return this;
   }
 
-  /** Set the {@link ExecutorService} used to execute requests. */
+  /**
+   * Set the {@link ExecutorService} used to execute requests.
+   *
+   * <p>NOTE: By calling this method, you are opting into managing the lifecycle of the {@code
+   * executorService}. {@link ExecutorService#shutdown()} will NOT be called when this exporter is
+   * shutdown.
+   */
   public OtlpHttpSpanExporterBuilder setExecutorService(ExecutorService executorService) {
     requireNonNull(executorService, "executorService");
     delegate.setExecutorService(executorService);

exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/logs/OtlpGrpcLogRecordExporterBuilder.java

@@ -266,7 +266,13 @@ public OtlpGrpcLogRecordExporterBuilder setServiceClassLoader(ClassLoader servic
     return this;
   }
 
-  /** Set the {@link ExecutorService} used to execute requests. */
+  /**
+   * Set the {@link ExecutorService} used to execute requests.
+   *
+   * <p>NOTE: By calling this method, you are opting into managing the lifecycle of the {@code
+   * executorService}. {@link ExecutorService#shutdown()} will NOT be called when this exporter is
+   * shutdown.
+   */
   public OtlpGrpcLogRecordExporterBuilder setExecutorService(ExecutorService executorService) {
     requireNonNull(executorService, "executorService");
     delegate.setExecutorService(executorService);

exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/metrics/OtlpGrpcMetricExporterBuilder.java

@@ -294,7 +294,13 @@ public OtlpGrpcMetricExporterBuilder setServiceClassLoader(ClassLoader serviceCl
     return this;
   }
 
-  /** Set the {@link ExecutorService} used to execute requests. */
+  /**
+   * Set the {@link ExecutorService} used to execute requests.
+   *
+   * <p>NOTE: By calling this method, you are opting into managing the lifecycle of the {@code
+   * executorService}. {@link ExecutorService#shutdown()} will NOT be called when this exporter is
+   * shutdown.
+   */
   public OtlpGrpcMetricExporterBuilder setExecutorService(ExecutorService executorService) {
     requireNonNull(executorService, "executorService");
     delegate.setExecutorService(executorService);

exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/trace/OtlpGrpcSpanExporterBuilder.java

@@ -263,7 +263,13 @@ public OtlpGrpcSpanExporterBuilder setServiceClassLoader(ClassLoader serviceClas
     return this;
   }
 
-  /** Set the {@link ExecutorService} used to execute requests. */
+  /**
+   * Set the {@link ExecutorService} used to execute requests.
+   *
+   * <p>NOTE: By calling this method, you are opting into managing the lifecycle of the {@code
+   * executorService}. {@link ExecutorService#shutdown()} will NOT be called when this exporter is
+   * shutdown.
+   */
   public OtlpGrpcSpanExporterBuilder setExecutorService(ExecutorService executorService) {
     requireNonNull(executorService, "executorService");
     delegate.setExecutorService(executorService);

exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/AbstractGrpcTelemetryExporterTest.java

@@ -834,6 +834,8 @@ void executorService() {
       assertThat(executorService.getTaskCount()).isPositive();
     } finally {
       exporter.shutdown();
+      // If setting executor, the user is responsible for calling shutdown
+      assertThat(executorService.isShutdown()).isFalse();
       executorService.shutdown();
     }
   }

exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/AbstractHttpTelemetryExporterTest.java

@@ -717,6 +717,8 @@ void executorService() {
       assertThat(executorService.getTaskCount()).isPositive();
     } finally {
       exporter.shutdown();
+      // If setting executor, the user is responsible for calling shutdown
+      assertThat(executorService.isShutdown()).isFalse();
       executorService.shutdown();
     }
   }

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

@@ -61,6 +61,7 @@ public final class JdkHttpSender implements HttpSender {
 
   private static final Logger logger = Logger.getLogger(JdkHttpSender.class.getName());
 
+  private final boolean managedExecutor;
   private final ExecutorService executorService;
   private final HttpClient client;
   private final URI uri;
@@ -99,8 +100,13 @@ public final class JdkHttpSender implements HttpSender {
         Optional.ofNullable(retryPolicy)
             .map(RetryPolicy::getRetryExceptionPredicate)
             .orElse(JdkHttpSender::isRetryableException);
-    this.executorService =
-        executorService == null ? Executors.newFixedThreadPool(5) : executorService;
+    if (executorService == null) {
+      this.executorService = Executors.newFixedThreadPool(5);
+      this.managedExecutor = true;
+    } else {
+      this.executorService = executorService;
+      this.managedExecutor = false;
+    }
   }
 
   JdkHttpSender(
@@ -368,7 +374,9 @@ private void resetPool() {
 
   @Override
   public CompletableResultCode shutdown() {
-    executorService.shutdown();
+    if (managedExecutor) {
+      executorService.shutdown();
+    }
     return CompletableResultCode.ofSuccess();
   }
 }

exporters/sender/okhttp/src/main/java/io/opentelemetry/exporter/sender/okhttp/internal/OkHttpGrpcSender.java

@@ -68,6 +68,7 @@ public final class OkHttpGrpcSender<T extends Marshaler> implements GrpcSender<T
   private static final String GRPC_STATUS = "grpc-status";
   private static final String GRPC_MESSAGE = "grpc-message";
 
+  private final boolean managedExecutor;
   private final OkHttpClient client;
   private final HttpUrl url;
   private final Supplier<Map<String, List<String>>> headersSupplier;
@@ -89,8 +90,15 @@ public OkHttpGrpcSender(
     int connectTimeoutMillis =
         (int) Math.min(Duration.ofNanos(connectTimeoutNanos).toMillis(), Integer.MAX_VALUE);
 
-    Dispatcher dispatcher =
-        executorService == null ? OkHttpUtil.newDispatcher() : new Dispatcher(executorService);
+    Dispatcher dispatcher;
+    if (executorService == null) {
+      dispatcher = OkHttpUtil.newDispatcher();
+      this.managedExecutor = true;
+    } else {
+      dispatcher = new Dispatcher(executorService);
+      ;
+      this.managedExecutor = false;
+    }
 
     OkHttpClient.Builder clientBuilder =
         new OkHttpClient.Builder()
@@ -205,7 +213,9 @@ private static String grpcMessage(Response response) {
   @Override
   public CompletableResultCode shutdown() {
     client.dispatcher().cancelAll();
-    client.dispatcher().executorService().shutdownNow();
+    if (managedExecutor) {
+      client.dispatcher().executorService().shutdownNow();
+    }
     client.connectionPool().evictAll();
     return CompletableResultCode.ofSuccess();
   }