Match environment variables for underscore keys in DefaultConfigProperties

sdk-extensions/autoconfigure-spi/build.gradle.kts

@@ -12,3 +12,8 @@ dependencies {
   api(project(":sdk:all"))
   compileOnly(project(":api:incubator"))
 }
+
+tasks.test {
+  // Configure environment variable for ConfigPropertiesTest
+  environment("OTEL_JAVA_EXPORTER_MEMORY_MODE", "immutable_data")
+}

sdk-extensions/autoconfigure-spi/src/main/java/io/opentelemetry/sdk/autoconfigure/spi/internal/DefaultConfigProperties.java

@@ -28,9 +28,9 @@
 import javax.annotation.Nullable;
 
 /**
- * Properties are normalized to The properties for both of these will be normalized to be all lower
- * case, dashses are replaces with periods, and environment variable underscores are replaces with
- * periods.
+ * Property keys from all sources are normalized to be all lower case, with dashes and underscores
+ * replaced with periods. For example, {@code otel.java.exporter.memory_mode} matches the
+ * environment variable {@code OTEL_JAVA_EXPORTER_MEMORY_MODE}.
  *
  * <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
  * at any time.
@@ -71,13 +71,11 @@ private DefaultConfigProperties(
       Map<String, String> defaultProperties,
       ComponentLoader componentLoader) {
     Map<String, String> config = new HashMap<>();
-    defaultProperties.forEach(
-        (name, value) -> config.put(ConfigUtil.normalizePropertyKey(name), value));
+    defaultProperties.forEach((name, value) -> config.put(normalizeKey(name), value));
     environmentVariables.forEach(
         (name, value) -> config.put(ConfigUtil.normalizeEnvironmentVariableKey(name), value));
     systemProperties.forEach(
-        (key, value) ->
-            config.put(ConfigUtil.normalizePropertyKey(key.toString()), value.toString()));
+        (key, value) -> config.put(normalizeKey(key.toString()), value.toString()));
 
     this.config = config;
     this.componentLoader = componentLoader;
@@ -87,7 +85,7 @@ private DefaultConfigProperties(
       DefaultConfigProperties previousProperties, Map<String, String> overrides) {
     // previousProperties are already normalized, they can be copied as they are
     Map<String, String> config = new HashMap<>(previousProperties.config);
-    overrides.forEach((name, value) -> config.put(ConfigUtil.normalizePropertyKey(name), value));
+    overrides.forEach((name, value) -> config.put(normalizeKey(name), value));
 
     this.config = config;
     this.componentLoader = previousProperties.componentLoader;
@@ -96,13 +94,13 @@ private DefaultConfigProperties(
   @Override
   @Nullable
   public String getString(String name) {
-    return config.get(ConfigUtil.normalizePropertyKey(name));
+    return config.get(normalizeKey(name));
   }
 
   @Override
   @Nullable
   public Boolean getBoolean(String name) {
-    String value = config.get(ConfigUtil.normalizePropertyKey(name));
+    String value = config.get(normalizeKey(name));
     if (value == null || value.isEmpty()) {
       return null;
     }
@@ -113,7 +111,7 @@ public Boolean getBoolean(String name) {
   @Nullable
   @SuppressWarnings("UnusedException")
   public Integer getInt(String name) {
-    String value = config.get(ConfigUtil.normalizePropertyKey(name));
+    String value = config.get(normalizeKey(name));
     if (value == null || value.isEmpty()) {
       return null;
     }
@@ -128,7 +126,7 @@ public Integer getInt(String name) {
   @Nullable
   @SuppressWarnings("UnusedException")
   public Long getLong(String name) {
-    String value = config.get(ConfigUtil.normalizePropertyKey(name));
+    String value = config.get(normalizeKey(name));
     if (value == null || value.isEmpty()) {
       return null;
     }
@@ -143,7 +141,7 @@ public Long getLong(String name) {
   @Nullable
   @SuppressWarnings("UnusedException")
   public Double getDouble(String name) {
-    String value = config.get(ConfigUtil.normalizePropertyKey(name));
+    String value = config.get(normalizeKey(name));
     if (value == null || value.isEmpty()) {
       return null;
     }
@@ -158,7 +156,7 @@ public Double getDouble(String name) {
   @Nullable
   @SuppressWarnings("UnusedException")
   public Duration getDuration(String name) {
-    String value = config.get(ConfigUtil.normalizePropertyKey(name));
+    String value = config.get(normalizeKey(name));
     if (value == null || value.isEmpty()) {
       return null;
     }
@@ -187,7 +185,7 @@ public Duration getDuration(String name) {
 
   @Override
   public List<String> getList(String name) {
-    String value = config.get(ConfigUtil.normalizePropertyKey(name));
+    String value = config.get(normalizeKey(name));
     if (value == null) {
       return Collections.emptyList();
     }
@@ -260,6 +258,14 @@ private static ConfigurationException newInvalidPropertyException(
         "Invalid value for property " + name + "=" + value + ". Must be a " + type + ".");
   }
 
+  /**
+   * Normalize a property key the same way environment variable keys are normalized, so that keys
+   * containing "_" match their environment variable.
+   */
+  private static String normalizeKey(String key) {
+    return ConfigUtil.normalizeEnvironmentVariableKey(ConfigUtil.normalizePropertyKey(key));
+  }
+
   private static List<String> filterBlanksAndNulls(String[] values) {
     return Arrays.stream(values)
         .map(String::trim)

sdk-extensions/autoconfigure-spi/src/test/java/io/opentelemetry/sdk/autoconfigure/spi/internal/ConfigPropertiesTest.java

@@ -20,7 +20,12 @@
 import java.util.HashMap;
 import java.util.Map;
 import org.junit.jupiter.api.Test;
+import org.junitpioneer.jupiter.SetSystemProperty;
 
+/**
+ * Relies on environment configuration in {@code
+ * ./sdk-extensions/autoconfigure-spi/build.gradle.kts}.
+ */
 class ConfigPropertiesTest {
 
   private static final ComponentLoader COMPONENT_LOADER =
@@ -288,6 +293,28 @@ void defaultCollectionTypes() {
     assertThat(properties.getMap("foo")).isEmpty();
   }
 
+  @Test
+  void environmentVariableForKeyWithUnderscore() {
+    ConfigProperties properties = DefaultConfigProperties.create(emptyMap(), COMPONENT_LOADER);
+    assertThat(properties.getString("otel.java.exporter.memory_mode")).isEqualTo("immutable_data");
+  }
+
+  @Test
+  void environmentVariableTakesPriorityOverDefaultForKeyWithUnderscore() {
+    ConfigProperties properties =
+        DefaultConfigProperties.create(
+            Collections.singletonMap("otel.java.exporter.memory_mode", "reusable_data"),
+            COMPONENT_LOADER);
+    assertThat(properties.getString("otel.java.exporter.memory_mode")).isEqualTo("immutable_data");
+  }
+
+  @Test
+  @SetSystemProperty(key = "otel.java.exporter.memory_mode", value = "reusable_data")
+  void systemPropertyTakesPriorityOverEnvironmentVariableForKeyWithUnderscore() {
+    ConfigProperties properties = DefaultConfigProperties.create(emptyMap(), COMPONENT_LOADER);
+    assertThat(properties.getString("otel.java.exporter.memory_mode")).isEqualTo("reusable_data");
+  }
+
   private static Map<String, String> makeTestProps() {
     Map<String, String> properties = new HashMap<>();
     properties.put("test.string", "str");