Fix race condition of GlobalOpenTelemetry initialization with AutoConfiguredOpenTelemetrySdkBuilder (#7365)

Co-authored-by: Jack Berg <[email protected]>
Co-authored-by: jack-berg <[email protected]>

Author: fandreuz

api/all/src/main/java/io/opentelemetry/api/GlobalOpenTelemetry.java

@@ -17,6 +17,7 @@
 import io.opentelemetry.context.propagation.ContextPropagators;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
+import java.util.function.Supplier;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import javax.annotation.Nullable;
@@ -116,6 +117,19 @@ public static void set(OpenTelemetry openTelemetry) {
     }
   }
 
+  /**
+   * Sets the {@link OpenTelemetry} that should be the global instance.
+   *
+   * <p>This method calls the given {@code supplier} and calls {@link #set(OpenTelemetry)}, all
+   * while holding the {@link GlobalOpenTelemetry} mutex.
+   */
+  public static void set(Supplier<OpenTelemetry> supplier) {
+    synchronized (mutex) {
+      OpenTelemetry openTelemetry = supplier.get();
+      set(openTelemetry);
+    }
+  }
+
   /** Returns the globally registered {@link TracerProvider}. */
   public static TracerProvider getTracerProvider() {
     return get().getTracerProvider();

docs/apidiffs/current_vs_latest/opentelemetry-api.txt

@@ -1,2 +1,4 @@
 Comparing source compatibility of opentelemetry-api-1.52.0-SNAPSHOT.jar against opentelemetry-api-1.51.0.jar
-No changes.
+***  MODIFIED CLASS: PUBLIC FINAL io.opentelemetry.api.GlobalOpenTelemetry  (not serializable)
+	===  CLASS FILE FORMAT VERSION: 52.0 <- 52.0
+	+++  NEW METHOD: PUBLIC(+) STATIC(+) void set(java.util.function.Supplier<io.opentelemetry.api.OpenTelemetry>)

sdk-extensions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/AutoConfiguredOpenTelemetrySdkBuilder.java

@@ -41,6 +41,8 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.BiFunction;
 import java.util.function.Function;
 import java.util.function.Supplier;
@@ -424,6 +426,25 @@ AutoConfiguredOpenTelemetrySdkBuilder setComponentLoader(ComponentLoader compone
    * the settings of this {@link AutoConfiguredOpenTelemetrySdkBuilder}.
    */
   public AutoConfiguredOpenTelemetrySdk build() {
+    if (!setResultAsGlobal) {
+      return buildImpl();
+    }
+    AtomicReference<AutoConfiguredOpenTelemetrySdk> autoConfiguredRef = new AtomicReference<>();
+    GlobalOpenTelemetry.set(
+        () -> {
+          AutoConfiguredOpenTelemetrySdk sdk = buildImpl();
+          autoConfiguredRef.set(sdk);
+          return sdk.getOpenTelemetrySdk();
+        });
+    AutoConfiguredOpenTelemetrySdk sdk = Objects.requireNonNull(autoConfiguredRef.get());
+    logger.log(
+        Level.FINE,
+        "Global OpenTelemetry set to {0} by autoconfiguration",
+        sdk.getOpenTelemetrySdk());
+    return sdk;
+  }
+
+  private AutoConfiguredOpenTelemetrySdk buildImpl() {
     SpiHelper spiHelper = SpiHelper.create(componentLoader);
     if (!customized) {
       customized = true;
@@ -440,8 +461,10 @@ public AutoConfiguredOpenTelemetrySdk build() {
         maybeConfigureFromFile(config, componentLoader);
     if (fromFileConfiguration != null) {
       maybeRegisterShutdownHook(fromFileConfiguration.getOpenTelemetrySdk());
-      maybeSetAsGlobal(
-          fromFileConfiguration.getOpenTelemetrySdk(), fromFileConfiguration.getConfigProvider());
+      Object configProvider = fromFileConfiguration.getConfigProvider();
+      if (setResultAsGlobal && INCUBATOR_AVAILABLE && configProvider != null) {
+        IncubatingUtil.setGlobalConfigProvider(configProvider);
+      }
       return fromFileConfiguration;
     }
 
@@ -467,7 +490,6 @@ public AutoConfiguredOpenTelemetrySdk build() {
 
       OpenTelemetrySdk openTelemetrySdk = sdkBuilder.build();
       maybeRegisterShutdownHook(openTelemetrySdk);
-      maybeSetAsGlobal(openTelemetrySdk, null);
       callAutoConfigureListeners(spiHelper, openTelemetrySdk);
 
       return AutoConfiguredOpenTelemetrySdk.create(openTelemetrySdk, resource, config, null);
@@ -572,19 +594,6 @@ private void maybeRegisterShutdownHook(OpenTelemetrySdk openTelemetrySdk) {
     Runtime.getRuntime().addShutdownHook(shutdownHook(openTelemetrySdk));
   }
 
-  private void maybeSetAsGlobal(
-      OpenTelemetrySdk openTelemetrySdk, @Nullable Object configProvider) {
-    if (!setResultAsGlobal) {
-      return;
-    }
-    GlobalOpenTelemetry.set(openTelemetrySdk);
-    if (INCUBATOR_AVAILABLE && configProvider != null) {
-      IncubatingUtil.setGlobalConfigProvider(configProvider);
-    }
-    logger.log(
-        Level.FINE, "Global OpenTelemetry set to {0} by autoconfiguration", openTelemetrySdk);
-  }
-
   // Visible for testing
   void callAutoConfigureListeners(SpiHelper spiHelper, OpenTelemetrySdk openTelemetrySdk) {
     for (AutoConfigureListener listener : spiHelper.getListeners()) {

sdk-extensions/autoconfigure/src/test/java/io/opentelemetry/sdk/autoconfigure/AutoConfiguredOpenTelemetrySdkTest.java

@@ -58,14 +58,21 @@
 import io.opentelemetry.sdk.trace.export.SimpleSpanProcessor;
 import io.opentelemetry.sdk.trace.samplers.Sampler;
 import java.io.IOException;
+import java.lang.reflect.Field;
 import java.math.BigDecimal;
 import java.math.BigInteger;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
 import java.util.function.BiFunction;
 import java.util.function.Consumer;
 import java.util.function.Supplier;
@@ -674,4 +681,66 @@ void configurationError_ClosesResources() {
 
     logs.assertContains("Error closing io.opentelemetry.sdk.trace.SdkTracerProvider: Error!");
   }
+
+  @Test
+  void globalOpenTelemetryLock() throws InterruptedException, ExecutionException, TimeoutException {
+    CountDownLatch autoconfigStarted = new CountDownLatch(1);
+    CountDownLatch completeAutoconfig = new CountDownLatch(1);
+    ExecutorService executorService = Executors.newFixedThreadPool(2);
+
+    // Submit a future to autoconfigure the SDK and set the result as global. Add a customization
+    // hook which blocks until we say so.
+    CompletableFuture<OpenTelemetrySdk> autoConfiguredOpenTelemetryFuture =
+        CompletableFuture.supplyAsync(
+            () ->
+                builder
+                    .addLoggerProviderCustomizer(
+                        (sdkLoggerProviderBuilder, configProperties) -> {
+                          autoconfigStarted.countDown();
+                          try {
+                            completeAutoconfig.await();
+                          } catch (InterruptedException e) {
+                            Thread.currentThread().interrupt();
+                          }
+                          return sdkLoggerProviderBuilder;
+                        })
+                    .setResultAsGlobal()
+                    .build()
+                    .getOpenTelemetrySdk(),
+            executorService);
+
+    // Wait for autoconfiguration to enter our callback, then try to get an instance of
+    // GlobalOpenTelemetry. GlobalOpenTelemetry.get() should block until we release the
+    // completeAutoconfig latch and allow autoconfiguration to complete.
+    autoconfigStarted.await();
+    CompletableFuture<OpenTelemetry> globalOpenTelemetryFuture =
+        CompletableFuture.supplyAsync(GlobalOpenTelemetry::get, executorService);
+    Thread.sleep(10);
+    assertThat(globalOpenTelemetryFuture.isDone()).isFalse();
+    assertThat(autoConfiguredOpenTelemetryFuture.isDone()).isFalse();
+
+    // Release the latch, allowing autoconfiguration to complete. Confirm that our
+    // GlobalOpenTelemetry.get() future resolved to the same instance as autoconfiguration.
+    completeAutoconfig.countDown();
+    assertThat(unobfuscate(globalOpenTelemetryFuture.get(10, TimeUnit.SECONDS)))
+        .isSameAs(autoConfiguredOpenTelemetryFuture.get(10, TimeUnit.SECONDS));
+
+    // Cleanup
+    executorService.shutdown();
+    autoConfiguredOpenTelemetryFuture.get().shutdown().join(10, TimeUnit.SECONDS);
+    GlobalOpenTelemetry.resetForTest();
+  }
+
+  private static OpenTelemetry unobfuscate(OpenTelemetry openTelemetry) {
+    try {
+      Field delegateField =
+          Class.forName("io.opentelemetry.api.GlobalOpenTelemetry$ObfuscatedOpenTelemetry")
+              .getDeclaredField("delegate");
+      delegateField.setAccessible(true);
+      Object delegate = delegateField.get(openTelemetry);
+      return (OpenTelemetry) delegate;
+    } catch (NoSuchFieldException | IllegalAccessException | ClassNotFoundException e) {
+      throw new IllegalStateException("Error unobfuscating OpenTelemetry", e);
+    }
+  }
 }