partially incorporate pr feedback

Author: jack-berg

api/incubator/src/main/java/io/opentelemetry/api/incubator/config/ConfigProvider.java

@@ -23,7 +23,7 @@ public interface ConfigProvider {
   /**
    * Returns the {@link StructuredConfigProperties} corresponding to <a
    * href="https://github.com/open-telemetry/opentelemetry-configuration/blob/main/schema/instrumentation.json">instrumentation
-   * config</a>, or {@code null} if file configuration is not used.
+   * config</a>, or {@code null} if unavailable.
    *
    * @return the instrumentation {@link StructuredConfigProperties}
    */

api/incubator/src/main/java/io/opentelemetry/api/incubator/config/GlobalConfigProvider.java

@@ -37,6 +37,8 @@ public static ConfigProvider get() {
    * Sets the global {@link ConfigProvider}. Future calls to {@link #get()} will return the provided
    * {@link ConfigProvider} instance. This should be called once as early as possible in your
    * application initialization logic.
+   *
+   * @throws IllegalStateException when called more than once
    */
   public static void set(ConfigProvider configProvider) {
     boolean changed = instance.compareAndSet(ConfigProvider.noop(), configProvider);

api/incubator/src/main/java/io/opentelemetry/api/incubator/config/InstrumentationConfigUtil.java

@@ -5,7 +5,7 @@
 
 package io.opentelemetry.api.incubator.config;
 
-import java.util.HashMap;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -22,6 +22,8 @@ public class InstrumentationConfigUtil {
   /**
    * Return a map representation of the peer service map entries in {@code
    * .instrumentation.general.peer.service_mapping}, or null if none is configured.
+   *
+   * @throws StructuredConfigException if an unexpected type is encountered accessing the property
    */
   @Nullable
   public static Map<String, String> peerServiceMapping(ConfigProvider configProvider) {
@@ -33,7 +35,7 @@ public static Map<String, String> peerServiceMapping(ConfigProvider configProvid
     if (!optServiceMappingList.isPresent()) {
       return null;
     }
-    Map<String, String> serviceMapping = new HashMap<>();
+    Map<String, String> serviceMapping = new LinkedHashMap<>();
     optServiceMappingList
         .get()
         .forEach(
@@ -44,12 +46,14 @@ public static Map<String, String> peerServiceMapping(ConfigProvider configProvid
                 serviceMapping.put(peer, service);
               }
             });
-    return serviceMapping;
+    return serviceMapping.isEmpty() ? null : serviceMapping;
   }
 
   /**
    * Return {@code .instrumentation.general.http.client.request_captured_headers}, or null if none
    * is configured.
+   *
+   * @throws StructuredConfigException if an unexpected type is encountered accessing the property
    */
   @Nullable
   public static List<String> httpClientRequestCapturedHeaders(ConfigProvider configProvider) {
@@ -58,12 +62,15 @@ public static List<String> httpClientRequestCapturedHeaders(ConfigProvider confi
         .map(generalConfig -> generalConfig.getStructured("http"))
         .map(httpConfig -> httpConfig.getStructured("client"))
         .map(clientConfig -> clientConfig.getScalarList("request_captured_headers", String.class))
+        .filter(list -> !list.isEmpty())
         .orElse(null);
   }
 
   /**
    * Return {@code .instrumentation.general.http.client.response_captured_headers}, or null if none
    * is configured.
+   *
+   * @throws StructuredConfigException if an unexpected type is encountered accessing the property
    */
   @Nullable
   public static List<String> httpClientResponseCapturedHeaders(ConfigProvider configProvider) {
@@ -72,12 +79,15 @@ public static List<String> httpClientResponseCapturedHeaders(ConfigProvider conf
         .map(generalConfig -> generalConfig.getStructured("http"))
         .map(httpConfig -> httpConfig.getStructured("client"))
         .map(clientConfig -> clientConfig.getScalarList("response_captured_headers", String.class))
+        .filter(list -> !list.isEmpty())
         .orElse(null);
   }
 
   /**
    * Return {@code .instrumentation.general.http.server.request_captured_headers}, or null if none
    * is configured.
+   *
+   * @throws StructuredConfigException if an unexpected type is encountered accessing the property
    */
   @Nullable
   public static List<String> httpServerRequestCapturedHeaders(ConfigProvider configProvider) {
@@ -86,12 +96,15 @@ public static List<String> httpServerRequestCapturedHeaders(ConfigProvider confi
         .map(generalConfig -> generalConfig.getStructured("http"))
         .map(httpConfig -> httpConfig.getStructured("server"))
         .map(clientConfig -> clientConfig.getScalarList("request_captured_headers", String.class))
+        .filter(list -> !list.isEmpty())
         .orElse(null);
   }
 
   /**
    * Return {@code .instrumentation.general.http.server.response_captured_headers}, or null if none
    * is configured.
+   *
+   * @throws StructuredConfigException if an unexpected type is encountered accessing the property
    */
   @Nullable
   public static List<String> httpSeverResponseCapturedHeaders(ConfigProvider configProvider) {
@@ -100,10 +113,15 @@ public static List<String> httpSeverResponseCapturedHeaders(ConfigProvider confi
         .map(generalConfig -> generalConfig.getStructured("http"))
         .map(httpConfig -> httpConfig.getStructured("server"))
         .map(clientConfig -> clientConfig.getScalarList("response_captured_headers", String.class))
+        .filter(list -> !list.isEmpty())
         .orElse(null);
   }
 
-  /** Return {@code .instrumentation.java.<instrumentationName>}, or null if none is configured. */
+  /**
+   * Return {@code .instrumentation.java.<instrumentationName>}, or null if none is configured.
+   *
+   * @throws StructuredConfigException if an unexpected type is encountered accessing the property
+   */
   @Nullable
   public static StructuredConfigProperties javaInstrumentationConfig(
       ConfigProvider configProvider, String instrumentationName) {

api/incubator/src/main/java/io/opentelemetry/api/incubator/config/internal/YamlStructuredConfigProperties.java

@@ -287,10 +287,7 @@ public String toString() {
     return joiner.toString();
   }
 
-  /**
-   * Returns a map representation of this {@link YamlStructuredConfigProperties}, which is useful
-   * for testing.
-   */
+  /** Returns a map representation of this, which is useful for testing. */
   public Map<String, Object> asMap() {
     Map<String, Object> response = new HashMap<>();
     response.putAll(simpleEntries);

api/incubator/src/test/java/io/opentelemetry/api/incubator/ConfigProviderTest.java

@@ -0,0 +1,20 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.api.incubator;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import io.opentelemetry.api.incubator.config.ConfigProvider;
+import org.junit.jupiter.api.Test;
+
+class ConfigProviderTest {
+
+  @Test
+  void noopEquality() {
+    ConfigProvider noop = ConfigProvider.noop();
+    assertThat(ConfigProvider.noop()).isSameAs(noop);
+  }
+}

api/incubator/src/test/java/io/opentelemetry/api/incubator/config/GlobalConfigProviderTest.java

@@ -37,11 +37,10 @@ void setAndGet() {
 
   @Test
   void setThenSet() {
-    GlobalConfigProvider.set(() -> YamlStructuredConfigProperties.create(Collections.emptyMap()));
-    assertThatThrownBy(
-            () ->
-                GlobalConfigProvider.set(
-                    () -> YamlStructuredConfigProperties.create(Collections.emptyMap())))
+    ConfigProvider configProvider =
+        () -> YamlStructuredConfigProperties.create(Collections.emptyMap());
+    GlobalConfigProvider.set(configProvider);
+    assertThatThrownBy(() -> GlobalConfigProvider.set(configProvider))
         .isInstanceOf(IllegalStateException.class)
         .hasMessageContaining("GlobalConfigProvider.set has already been called")
         .hasStackTraceContaining("setThenSet");

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

@@ -52,6 +52,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.function.BiFunction;
 import java.util.function.Function;
 import java.util.function.Supplier;
@@ -435,7 +436,9 @@ public AutoConfiguredOpenTelemetrySdk build() {
     if (fromFileConfiguration != null) {
       maybeRegisterShutdownHook(fromFileConfiguration.getOpenTelemetrySdk());
       maybeSetAsGlobal(
-          fromFileConfiguration.getOpenTelemetrySdk(), fromFileConfiguration.getConfigProvider());
+          fromFileConfiguration.getOpenTelemetrySdk(),
+          Optional.ofNullable(fromFileConfiguration.getConfigProvider())
+              .orElse(ConfigProvider.noop()));
       return fromFileConfiguration;
     }
 
@@ -507,7 +510,7 @@ public AutoConfiguredOpenTelemetrySdk build() {
       }
 
       maybeRegisterShutdownHook(openTelemetrySdk);
-      maybeSetAsGlobal(openTelemetrySdk, null);
+      maybeSetAsGlobal(openTelemetrySdk, ConfigProvider.noop());
       callAutoConfigureListeners(spiHelper, openTelemetrySdk);
 
       return AutoConfiguredOpenTelemetrySdk.create(openTelemetrySdk, resource, config, null);
@@ -593,15 +596,14 @@ private void maybeRegisterShutdownHook(OpenTelemetrySdk openTelemetrySdk) {
     Runtime.getRuntime().addShutdownHook(shutdownHook(openTelemetrySdk));
   }
 
-  private void maybeSetAsGlobal(
-      OpenTelemetrySdk openTelemetrySdk, @Nullable ConfigProvider configProvider) {
+  private void maybeSetAsGlobal(OpenTelemetrySdk openTelemetrySdk, ConfigProvider configProvider) {
     if (!setResultAsGlobal) {
       return;
     }
     GlobalOpenTelemetry.set(openTelemetrySdk);
     GlobalEventLoggerProvider.set(
         SdkEventLoggerProvider.create(openTelemetrySdk.getSdkLoggerProvider()));
-    GlobalConfigProvider.set(configProvider != null ? configProvider : ConfigProvider.noop());
+    GlobalConfigProvider.set(configProvider);
     logger.log(
         Level.FINE, "Global OpenTelemetry set to {0} by autoconfiguration", openTelemetrySdk);
   }