Add shutdown / close to OpenTelemetrySdk (#5100)

* Add shutdown / close to OpenTelemetrySdk * Shutdown hook closes instead of shutdown
open-telemetry · Jan 10, 2023 · 4df4a0a · 4df4a0a
1 parent 50e7a17
commit 4df4a0a
Show file tree

Hide file tree

Showing 7 changed files with 99 additions and 43 deletions.
diff --git a/docs/apidiffs/current_vs_latest/opentelemetry-sdk.txt b/docs/apidiffs/current_vs_latest/opentelemetry-sdk.txt
@@ -1,2 +1,5 @@
 Comparing source compatibility of  against 
-No changes.
+***  MODIFIED CLASS: PUBLIC FINAL io.opentelemetry.sdk.OpenTelemetrySdk  (not serializable)
+	===  CLASS FILE FORMAT VERSION: 52.0 <- 52.0
+	+++  NEW METHOD: PUBLIC(+) void close()
+	+++  NEW METHOD: PUBLIC(+) io.opentelemetry.sdk.common.CompletableResultCode shutdown()
diff --git a/perf-harness/src/test/java/io/opentelemetry/perf/OtlpPipelineStressTest.java b/perf-harness/src/test/java/io/opentelemetry/perf/OtlpPipelineStressTest.java
@@ -10,7 +10,6 @@
 import eu.rekawek.toxiproxy.model.ToxicDirection;
 import eu.rekawek.toxiproxy.model.ToxicList;
 import eu.rekawek.toxiproxy.model.toxic.Timeout;
-import io.opentelemetry.api.OpenTelemetry;
 import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.api.trace.Span;
 import io.opentelemetry.api.trace.Tracer;
@@ -106,9 +105,7 @@ public class OtlpPipelineStressTest {
 
   private final InMemoryMetricExporter metricExporter = InMemoryMetricExporter.create();
 
-  private SdkTracerProvider sdkTracerProvider;
-  private OpenTelemetry openTelemetry;
-  private SdkMeterProvider meterProvider;
+  private OpenTelemetrySdk openTelemetry;
   private Proxy collectorProxy;
   private ToxiproxyClient toxiproxyClient;
 
@@ -134,8 +131,7 @@ void setUp() throws IOException {
 
   @AfterEach
   void tearDown() throws IOException {
-    meterProvider.close();
-    sdkTracerProvider.shutdown();
+    openTelemetry.close();
 
     toxiproxyClient.reset();
     collectorProxy.delete();
@@ -188,7 +184,7 @@ void oltpExportWithFlakyCollector() throws IOException, InterruptedException {
 
     Thread.sleep(10000);
     List<MetricData> finishedMetricItems = metricExporter.getFinishedMetricItems();
-    meterProvider.close();
+    openTelemetry.getSdkMeterProvider().close();
     Thread.sleep(1000);
     reportMetrics(finishedMetricItems);
     Thread.sleep(10000);
@@ -250,7 +246,7 @@ private void setupSdk() {
                 .build());
 
     // set up the metric exporter and wire it into the SDK and a timed reader.
-    meterProvider =
+    SdkMeterProvider meterProvider =
         SdkMeterProvider.builder()
             .setResource(resource)
             .registerMetricReader(
@@ -281,7 +277,9 @@ private void setupSdk() {
     SdkTracerProvider tracerProvider =
         SdkTracerProvider.builder().addSpanProcessor(spanProcessor).build();
     openTelemetry =
-        OpenTelemetrySdk.builder().setTracerProvider(tracerProvider).buildAndRegisterGlobal();
-    sdkTracerProvider = tracerProvider;
+        OpenTelemetrySdk.builder()
+            .setTracerProvider(tracerProvider)
+            .setMeterProvider(meterProvider)
+            .buildAndRegisterGlobal();
   }
 }
diff --git a/...c/main/java/io/opentelemetry/sdk/autoconfigure/AutoConfiguredOpenTelemetrySdkBuilder.java b/...c/main/java/io/opentelemetry/sdk/autoconfigure/AutoConfiguredOpenTelemetrySdkBuilder.java
@@ -17,7 +17,6 @@
 import io.opentelemetry.sdk.autoconfigure.spi.AutoConfigurationCustomizerProvider;
 import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
 import io.opentelemetry.sdk.autoconfigure.spi.internal.DefaultConfigProperties;
-import io.opentelemetry.sdk.common.CompletableResultCode;
 import io.opentelemetry.sdk.logs.SdkLoggerProvider;
 import io.opentelemetry.sdk.logs.SdkLoggerProviderBuilder;
 import io.opentelemetry.sdk.logs.export.LogRecordExporter;
@@ -35,7 +34,6 @@
 import java.util.List;
 import java.util.Map;
 import java.util.ServiceLoader;
-import java.util.concurrent.TimeUnit;
 import java.util.function.BiFunction;
 import java.util.function.Function;
 import java.util.function.Supplier;
@@ -365,19 +363,6 @@ public AutoConfiguredOpenTelemetrySdk build() {
       loggerProviderBuilder = loggerProviderCustomizer.apply(loggerProviderBuilder, config);
       SdkLoggerProvider loggerProvider = loggerProviderBuilder.build();
 
-      if (registerShutdownHook) {
-        Runtime.getRuntime()
-            .addShutdownHook(
-                new Thread(
-                    () -> {
-                      List<CompletableResultCode> shutdown = new ArrayList<>();
-                      shutdown.add(tracerProvider.shutdown());
-                      shutdown.add(meterProvider.shutdown());
-                      shutdown.add(loggerProvider.shutdown());
-                      CompletableResultCode.ofAll(shutdown).join(10, TimeUnit.SECONDS);
-                    }));
-      }
-
       ContextPropagators propagators =
           PropagatorConfiguration.configurePropagators(
               config, serviceClassLoader, propagatorCustomizer);
@@ -390,6 +375,10 @@ public AutoConfiguredOpenTelemetrySdk build() {
               .setPropagators(propagators);
 
       openTelemetrySdk = sdkBuilder.build();
+
+      if (registerShutdownHook) {
+        Runtime.getRuntime().addShutdownHook(new Thread(openTelemetrySdk::close));
+      }
     }
 
     if (setResultAsGlobal) {

diff --git a/.../src/test/java/io/opentelemetry/sdk/autoconfigure/AutoConfiguredOpenTelemetrySdkTest.java b/.../src/test/java/io/opentelemetry/sdk/autoconfigure/AutoConfiguredOpenTelemetrySdkTest.java
@@ -413,7 +413,7 @@ void tracerProviderCustomizer() {
     assertThat(spanData.getResource().getAttribute(stringKey("cat"))).isEqualTo("meow");
 
     // Ensures the export happened.
-    sdk.getSdkTracerProvider().shutdown().join(10, TimeUnit.SECONDS);
+    sdk.shutdown().join(10, TimeUnit.SECONDS);
   }
 
   @Test

diff --git a/...oconfigure/src/testFullConfig/java/io/opentelemetry/sdk/autoconfigure/FullConfigTest.java b/...oconfigure/src/testFullConfig/java/io/opentelemetry/sdk/autoconfigure/FullConfigTest.java
@@ -163,21 +163,7 @@ void setUp() {
 
   @AfterEach
   void afterEach() {
-    autoConfiguredOpenTelemetrySdk
-        .getOpenTelemetrySdk()
-        .getSdkMeterProvider()
-        .shutdown()
-        .join(10, TimeUnit.SECONDS);
-    autoConfiguredOpenTelemetrySdk
-        .getOpenTelemetrySdk()
-        .getSdkLoggerProvider()
-        .shutdown()
-        .join(10, TimeUnit.SECONDS);
-    autoConfiguredOpenTelemetrySdk
-        .getOpenTelemetrySdk()
-        .getSdkTracerProvider()
-        .shutdown()
-        .join(10, TimeUnit.SECONDS);
+    autoConfiguredOpenTelemetrySdk.getOpenTelemetrySdk().shutdown().join(10, TimeUnit.SECONDS);
     GlobalOpenTelemetry.resetForTest();
     GlobalLoggerProvider.resetForTest();
   }

diff --git a/sdk/all/src/main/java/io/opentelemetry/sdk/OpenTelemetrySdk.java b/sdk/all/src/main/java/io/opentelemetry/sdk/OpenTelemetrySdk.java
@@ -12,14 +12,25 @@
 import io.opentelemetry.api.trace.TracerBuilder;
 import io.opentelemetry.api.trace.TracerProvider;
 import io.opentelemetry.context.propagation.ContextPropagators;
+import io.opentelemetry.sdk.common.CompletableResultCode;
 import io.opentelemetry.sdk.logs.SdkLoggerProvider;
 import io.opentelemetry.sdk.metrics.SdkMeterProvider;
 import io.opentelemetry.sdk.trace.SdkTracerProvider;
+import java.io.Closeable;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.logging.Logger;
 import javax.annotation.concurrent.ThreadSafe;
 
 /** The SDK implementation of {@link OpenTelemetry}. */
 @ThreadSafe
-public final class OpenTelemetrySdk implements OpenTelemetry {
+public final class OpenTelemetrySdk implements OpenTelemetry, Closeable {
+
+  private static final Logger LOGGER = Logger.getLogger(OpenTelemetrySdk.class.getName());
+
+  private final AtomicBoolean isShutdown = new AtomicBoolean(false);
   private final ObfuscatedTracerProvider tracerProvider;
   private final ObfuscatedMeterProvider meterProvider;
   private final SdkLoggerProvider loggerProvider;
@@ -78,6 +89,29 @@ public ContextPropagators getPropagators() {
     return propagators;
   }
 
+  /**
+   * Shutdown the SDK. Calls {@link SdkTracerProvider#shutdown()}, {@link
+   * SdkMeterProvider#shutdown()}, and {@link SdkLoggerProvider#shutdown()}.
+   *
+   * @return a {@link CompletableResultCode} which completes when all providers are shutdown
+   */
+  public CompletableResultCode shutdown() {
+    if (!isShutdown.compareAndSet(false, true)) {
+      LOGGER.info("Multiple shutdown calls");
+      return CompletableResultCode.ofSuccess();
+    }
+    List<CompletableResultCode> results = new ArrayList<>();
+    results.add(tracerProvider.unobfuscate().shutdown());
+    results.add(meterProvider.unobfuscate().shutdown());
+    results.add(loggerProvider.shutdown());
+    return CompletableResultCode.ofAll(results);
+  }
+
+  @Override
+  public void close() {
+    shutdown().join(10, TimeUnit.SECONDS);
+  }
+
   @Override
   public String toString() {
     return "OpenTelemetrySdk{"

diff --git a/sdk/all/src/test/java/io/opentelemetry/sdk/OpenTelemetrySdkTest.java b/sdk/all/src/test/java/io/opentelemetry/sdk/OpenTelemetrySdkTest.java
@@ -10,14 +10,18 @@
 import static org.assertj.core.api.InstanceOfAssertFactories.type;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
+import io.github.netmikey.logunit.api.LogCapturer;
 import io.opentelemetry.api.GlobalOpenTelemetry;
 import io.opentelemetry.api.OpenTelemetry;
 import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.context.propagation.ContextPropagators;
 import io.opentelemetry.context.propagation.TextMapPropagator;
 import io.opentelemetry.sdk.common.Clock;
+import io.opentelemetry.sdk.common.CompletableResultCode;
 import io.opentelemetry.sdk.logs.SdkLoggerProvider;
 import io.opentelemetry.sdk.logs.export.LogRecordExporter;
 import io.opentelemetry.sdk.logs.export.SimpleLogRecordProcessor;
@@ -36,15 +40,21 @@
 import io.opentelemetry.sdk.trace.export.SpanExporter;
 import io.opentelemetry.sdk.trace.samplers.Sampler;
 import java.time.Duration;
+import java.util.concurrent.TimeUnit;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.api.extension.RegisterExtension;
 import org.mockito.Mock;
+import org.mockito.Mockito;
 import org.mockito.junit.jupiter.MockitoExtension;
 
 @ExtendWith(MockitoExtension.class)
 class OpenTelemetrySdkTest {
 
+  @RegisterExtension
+  LogCapturer logCapturer = LogCapturer.create().captureForLogger(OpenTelemetrySdk.class.getName());
+
   @Mock private MetricExporter metricExporter;
   @Mock private SdkTracerProvider tracerProvider;
   @Mock private SdkMeterProvider meterProvider;
@@ -318,6 +328,42 @@ void minimalOpenTelemetrySdkConfigurationDemo() {
         .build();
   }
 
+  @Test
+  void shutdown() {
+    when(tracerProvider.shutdown()).thenReturn(CompletableResultCode.ofSuccess());
+    when(meterProvider.shutdown()).thenReturn(CompletableResultCode.ofSuccess());
+    when(loggerProvider.shutdown()).thenReturn(CompletableResultCode.ofSuccess());
+
+    OpenTelemetrySdk sdk =
+        OpenTelemetrySdk.builder()
+            .setTracerProvider(tracerProvider)
+            .setMeterProvider(meterProvider)
+            .setLoggerProvider(loggerProvider)
+            .build();
+
+    // First call should call shutdown
+    assertThat(sdk.shutdown().join(10, TimeUnit.SECONDS).isSuccess()).isTrue();
+    verify(tracerProvider).shutdown();
+    verify(meterProvider).shutdown();
+    verify(loggerProvider).shutdown();
+    assertThat(logCapturer.getEvents()).isEmpty();
+
+    // Subsequent calls should log not call shutdown
+    Mockito.reset(tracerProvider, meterProvider, loggerProvider);
+    assertThat(sdk.shutdown().join(10, TimeUnit.SECONDS).isSuccess()).isTrue();
+    sdk.close();
+
+    verify(tracerProvider, never()).shutdown();
+    verify(meterProvider, never()).shutdown();
+    verify(loggerProvider, never()).shutdown();
+
+    assertThat(logCapturer.getEvents())
+        .hasSize(2)
+        .allSatisfy(
+            loggingEvent ->
+                assertThat(loggingEvent.getMessage()).isEqualTo("Multiple shutdown calls"));
+  }
+
   @Test
   void stringRepresentation() {
     SpanExporter spanExporter = mock(SpanExporter.class);