Code review changes.

UT added

Author: robsunday

javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/ConfigPropertiesUtil.java

@@ -11,7 +11,6 @@
 import io.opentelemetry.sdk.autoconfigure.internal.AutoConfigureUtil;
 import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
 
-// TODO: Add unit test
 public class ConfigPropertiesUtil {
   private ConfigPropertiesUtil() {}
 
@@ -28,9 +27,11 @@ public static ConfigProperties resolveConfigProperties(
     if (configProvider != null) {
       DeclarativeConfigProperties instrumentationConfig = configProvider.getInstrumentationConfig();
 
-      if (instrumentationConfig != null) {
-        return new DeclarativeConfigPropertiesBridge(instrumentationConfig);
+      if (instrumentationConfig == null) {
+        instrumentationConfig = DeclarativeConfigProperties.empty();
       }
+
+      return new DeclarativeConfigPropertiesBridge(instrumentationConfig);
     }
     // Should never happen
     throw new IllegalStateException(

javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/DeclarativeConfigPropertiesBridge.java

@@ -130,51 +130,28 @@ public Map<String, String> getMap(String propertyName) {
   @Nullable
   private <T> T getPropertyValue(
       String property, BiFunction<DeclarativeConfigProperties, String, T> extractor) {
-    T value = getOtelPropertyValue(property, extractor);
-    if (value == null) {
-      value = getVendorPropertyValue(property, extractor);
+    if (instrumentationJavaNode == null) {
+      return null;
     }
-    return value;
-  }
 
-  @Nullable
-  private <T> T getPropertyValue(
-      String[] segments, BiFunction<DeclarativeConfigProperties, String, T> extractor) {
+    if (property.startsWith(OTEL_INSTRUMENTATION_PREFIX)) {
+      property = property.substring(OTEL_INSTRUMENTATION_PREFIX.length());
+    }
+    // Split the remainder of the property on "."
+    String[] segments = property.split("\\.");
+    if (segments.length == 0) {
+      return null;
+    }
 
+    // Extract the value by walking to the N-1 entry
     DeclarativeConfigProperties target = instrumentationJavaNode;
     if (segments.length > 1) {
       for (int i = 0; i < segments.length - 1; i++) {
         target = target.getStructured(segments[i], empty());
       }
     }
     String lastPart = segments[segments.length - 1];
-    return extractor.apply(target, lastPart);
-  }
 
-  @Nullable
-  private <T> T getOtelPropertyValue(
-      String property, BiFunction<DeclarativeConfigProperties, String, T> extractor) {
-    if (!property.startsWith(OTEL_INSTRUMENTATION_PREFIX)) {
-      return null;
-    }
-    String suffix = property.substring(OTEL_INSTRUMENTATION_PREFIX.length());
-    // Split the remainder of the property on ".", and walk to the N-1 entry
-    String[] segments = suffix.split("\\.");
-    if (segments.length == 0) {
-      return null;
-    }
-
-    return getPropertyValue(segments, extractor);
-  }
-
-  @Nullable
-  private <T> T getVendorPropertyValue(
-      String property, BiFunction<DeclarativeConfigProperties, String, T> extractor) {
-    String[] segments = property.split("\\.");
-    if (segments.length == 0) {
-      return null;
-    }
-
-    return getPropertyValue(segments, extractor);
+    return extractor.apply(target, lastPart);
   }
 }

javaagent-extension-api/src/test/java/io/opentelemetry/javaagent/extension/ConfigPropertiesUtilTest.java

@@ -0,0 +1,87 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.javaagent.extension;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import io.opentelemetry.api.incubator.config.ConfigProvider;
+import io.opentelemetry.api.incubator.config.DeclarativeConfigProperties;
+import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk;
+import io.opentelemetry.sdk.autoconfigure.internal.AutoConfigureUtil;
+import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
+import org.junit.jupiter.api.Test;
+import org.mockito.MockedStatic;
+import org.mockito.Mockito;
+
+@SuppressWarnings("DoNotMockAutoValue")
+class ConfigPropertiesUtilTest {
+  @Test
+  void shouldUseConfigPropertiesForAutoConfiguration() {
+    ConfigProperties configPropertiesMock = mock(ConfigProperties.class);
+    AutoConfiguredOpenTelemetrySdk sdkMock = mock(AutoConfiguredOpenTelemetrySdk.class);
+    try (MockedStatic<AutoConfigureUtil> autoConfigureUtilMock =
+        Mockito.mockStatic(AutoConfigureUtil.class)) {
+      autoConfigureUtilMock
+          .when(() -> AutoConfigureUtil.getConfig(sdkMock))
+          .thenReturn(configPropertiesMock);
+
+      ConfigProperties configProperties = ConfigPropertiesUtil.resolveConfigProperties(sdkMock);
+
+      assertThat(configProperties).isSameAs(configPropertiesMock);
+    }
+  }
+
+  @Test
+  void shouldUseConfigProviderForDeclarativeConfiguration() {
+    String propertyName = "testProperty";
+    String expectedValue = "the value";
+    DeclarativeConfigProperties javaNodeMock = mock(DeclarativeConfigProperties.class);
+    when(javaNodeMock.getString(propertyName)).thenReturn(expectedValue);
+
+    DeclarativeConfigProperties instrumentationConfigMock = mock(DeclarativeConfigProperties.class);
+    when(instrumentationConfigMock.getStructured(eq("java"), any())).thenReturn(javaNodeMock);
+
+    ConfigProvider configProviderMock = mock(ConfigProvider.class);
+    when(configProviderMock.getInstrumentationConfig()).thenReturn(instrumentationConfigMock);
+
+    AutoConfiguredOpenTelemetrySdk sdkMock = mock(AutoConfiguredOpenTelemetrySdk.class);
+
+    try (MockedStatic<AutoConfigureUtil> autoConfigureUtilMock =
+        Mockito.mockStatic(AutoConfigureUtil.class)) {
+      autoConfigureUtilMock.when(() -> AutoConfigureUtil.getConfig(sdkMock)).thenReturn(null);
+      autoConfigureUtilMock
+          .when(() -> AutoConfigureUtil.getConfigProvider(sdkMock))
+          .thenReturn(configProviderMock);
+
+      ConfigProperties configProperties = ConfigPropertiesUtil.resolveConfigProperties(sdkMock);
+
+      assertThat(configProperties.getString(propertyName)).isEqualTo(expectedValue);
+    }
+  }
+
+  @Test
+  void shouldUseConfigProviderForDeclarativeConfiguration_noInstrumentationConfig() {
+    AutoConfiguredOpenTelemetrySdk sdkMock = mock(AutoConfiguredOpenTelemetrySdk.class);
+    ConfigProvider configProviderMock = mock(ConfigProvider.class);
+    when(configProviderMock.getInstrumentationConfig()).thenReturn(null);
+
+    try (MockedStatic<AutoConfigureUtil> autoConfigureUtilMock =
+        Mockito.mockStatic(AutoConfigureUtil.class)) {
+      autoConfigureUtilMock.when(() -> AutoConfigureUtil.getConfig(sdkMock)).thenReturn(null);
+      autoConfigureUtilMock
+          .when(() -> AutoConfigureUtil.getConfigProvider(sdkMock))
+          .thenReturn(configProviderMock);
+
+      ConfigProperties configProperties = ConfigPropertiesUtil.resolveConfigProperties(sdkMock);
+
+      assertThat(configProperties.getString("testProperty")).isEqualTo(null);
+    }
+  }
+}