Refactor method signatures

Author: jack-berg

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

@@ -48,9 +48,13 @@ static Map<String, Object> toMap(DeclarativeConfigProperties declarativeConfigPr
   private static Object resolveValue(
       String key, DeclarativeConfigProperties declarativeConfigProperties) {
     for (int i = 0; i < valueResolvers.size(); i++) {
-      Object value = valueResolvers.get(i).apply(key, declarativeConfigProperties);
-      if (value != null) {
-        return value;
+      try {
+        Object value = valueResolvers.get(i).apply(key, declarativeConfigProperties);
+        if (value != null) {
+          return value;
+        }
+      } catch (DeclarativeConfigException e) {
+        // Ignore and continue
       }
     }
     return null;

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

@@ -151,24 +151,46 @@ public static <T> T getOrNull(
   private InstrumentationConfigUtil() {}
 
   /**
-   * Convert the {@code declarativeConfigProperties} to an instance of a given {@code modelType}.
+   * Return {@code .instrumentation.java.<instrumentationName>}, after converting it to the {@code
+   * modelType} using the {@code objectMapper}. If no configuration exists for the {@code }
    *
-   * <p>This method is a simple wrapper of {@link ObjectMapper#convertValue(Object, Class)} combined
-   * with a call to {@link DeclarativeConfigProperties#toMap(DeclarativeConfigProperties)}.
+   * <p>This method is a convenience method for a common instrumentation library workflow:
+   *
+   * <ul>
+   *   <li>During initialization, an instrumentation library is given an {@link ConfigProvider} and
+   *       must initialize according to the relevant config
+   *   <li>It checks if the user has provided configuration for it, and if so...
+   *   <li>It converts the configuration to an in-memory model representing all of its relevant
+   *       properties
+   *   <li>It initializes using the strongly typed in-memory model
+   * </ul>
+   *
+   * <p>Conversion is done using {@link ObjectMapper#convertValue(Object, Class)} from {@code
+   * com.fasterxml.jackson.databind}, and assumes the {@code modelType} is a POJO written /
+   * annotated to support jackson databinding.
    *
    * <p>NOTE: callers MUST add their own dependency on {@code
    * com.fasterxml.jackson.core:jackson-databind}. This module's dependency is {@code compileOnly}
    * since jackson is a large dependency that many users will not require. It's very possible to
    * convert between {@link DeclarativeConfigProperties} (or a map representation from {@link
    * DeclarativeConfigProperties#toMap(DeclarativeConfigProperties)}) and a target model type
-   * without jackson. This method is provided a convenience method for demonstration purposes.
+   * without jackson. This method is provided as an optional convenience method.
+   *
+   * @throws IllegalArgumentException if conversion fails. See {@link
+   *     ObjectMapper#convertValue(Object, Class)} for details.
    */
-  public static <T> T convertToModel(
+  @Nullable
+  public static <T> T getInstrumentationConfigModel(
+      ConfigProvider configProvider,
+      String instrumentationName,
       ObjectMapper objectMapper,
-      DeclarativeConfigProperties declarativeConfigProperties,
       Class<T> modelType) {
-    Map<String, Object> configPropertiesMap =
-        DeclarativeConfigProperties.toMap(declarativeConfigProperties);
+    DeclarativeConfigProperties properties =
+        javaInstrumentationConfig(configProvider, instrumentationName);
+    if (properties == null) {
+      return null;
+    }
+    Map<String, Object> configPropertiesMap = DeclarativeConfigProperties.toMap(properties);
     return objectMapper.convertValue(configPropertiesMap, modelType);
   }
 }

…ry/api/incubator/ConvertToModelTest.java …bator/InstrumentationConfigUtilTest.javaapi/incubator/src/testConvertToModel/java/io/opentelemetry/api/incubator/ConvertToModelTest.java renamed to api/incubator/src/testConvertToModel/java/io/opentelemetry/api/incubator/InstrumentationConfigUtilTest.java api/incubator/src/testConvertToModel/java/io/opentelemetry/api/incubator/ConvertToModelTest.java renamed to api/incubator/src/testConvertToModel/java/io/opentelemetry/api/incubator/InstrumentationConfigUtilTest.java

@@ -11,12 +11,14 @@
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.google.common.collect.ImmutableMap;
+import io.opentelemetry.api.incubator.config.ConfigProvider;
 import io.opentelemetry.api.incubator.config.DeclarativeConfigProperties;
 import io.opentelemetry.api.incubator.config.InstrumentationConfigUtil;
-import io.opentelemetry.sdk.autoconfigure.internal.ComponentLoader;
-import io.opentelemetry.sdk.autoconfigure.internal.SpiHelper;
 import io.opentelemetry.sdk.extension.incubator.fileconfig.DeclarativeConfiguration;
-import io.opentelemetry.sdk.extension.incubator.fileconfig.YamlDeclarativeConfigProperties;
+import io.opentelemetry.sdk.extension.incubator.fileconfig.SdkConfigProvider;
+import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.ExperimentalLanguageSpecificInstrumentationModel;
+import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.InstrumentationModel;
+import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.OpenTelemetryConfigurationModel;
 import java.io.ByteArrayInputStream;
 import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
@@ -27,7 +29,7 @@
 import java.util.Objects;
 import org.junit.jupiter.api.Test;
 
-class ConvertToModelTest {
+class InstrumentationConfigUtilTest {
 
   private static final ObjectMapper MAPPER = new ObjectMapper();
 
@@ -58,17 +60,31 @@ void toMap_RoundTrip() throws JsonProcessingException {
   }
 
   @Test
-  void convertToModel_Empty() {
-    DeclarativeConfigProperties properties = ofMap(Collections.emptyMap());
+  void getInstrumentationConfigModel_UnsetConfig() {
+    ConfigProvider configProvider = () -> null;
 
-    assertThat(InstrumentationConfigUtil.convertToModel(MAPPER, properties, Model.class))
+    assertThat(
+            InstrumentationConfigUtil.getInstrumentationConfigModel(
+                configProvider, "my_instrumentation_library", MAPPER, Model.class))
+        .isEqualTo(null);
+  }
+
+  @Test
+  void getInstrumentationConfigModel_EmptyConfig() {
+    ConfigProvider configProvider =
+        withInstrumentationConfig("my_instrumentation_library", Collections.emptyMap());
+
+    assertThat(
+            InstrumentationConfigUtil.getInstrumentationConfigModel(
+                configProvider, "my_instrumentation_library", MAPPER, Model.class))
         .isEqualTo(new Model());
   }
 
   @Test
-  void convertToModel_KitchenSink() {
-    DeclarativeConfigProperties properties =
-        ofMap(
+  void getInstrumentationConfigModel_KitchenSink() {
+    ConfigProvider configProvider =
+        withInstrumentationConfig(
+            "my_instrumentation_library",
             ImmutableMap.<String, Object>builder()
                 .put("string_property", "value")
                 .put("boolean_property", true)
@@ -84,6 +100,7 @@ void convertToModel_KitchenSink() {
                     Collections.singletonList(
                         ImmutableMap.of("key", "the_key", "value", "the_value")))
                 .build());
+
     Model expected = new Model();
     expected.stringProperty = "value";
     expected.booleanProperty = true;
@@ -99,15 +116,21 @@ void convertToModel_KitchenSink() {
     listEntryModel.value = "the_value";
     expected.structuredListProperty = Collections.singletonList(listEntryModel);
 
-    assertThat(InstrumentationConfigUtil.convertToModel(MAPPER, properties, Model.class))
+    assertThat(
+            InstrumentationConfigUtil.getInstrumentationConfigModel(
+                configProvider, "my_instrumentation_library", MAPPER, Model.class))
         .isEqualTo(expected);
   }
 
-  private static final ComponentLoader componentLoader =
-      SpiHelper.serviceComponentLoader(ConvertToModelTest.class.getClassLoader());
+  private static ConfigProvider withInstrumentationConfig(
+      String instrumentationName, Map<String, Object> instrumentationConfig) {
+    ExperimentalLanguageSpecificInstrumentationModel javaConfig =
+        new ExperimentalLanguageSpecificInstrumentationModel();
+    javaConfig.setAdditionalProperty(instrumentationName, instrumentationConfig);
 
-  private static YamlDeclarativeConfigProperties ofMap(Map<String, Object> map) {
-    return YamlDeclarativeConfigProperties.create(map, componentLoader);
+    return SdkConfigProvider.create(
+        new OpenTelemetryConfigurationModel()
+            .withInstrumentationDevelopment(new InstrumentationModel().withJava(javaConfig)));
   }
 
   private static class Model {

sdk-extensions/incubator/src/main/java/io/opentelemetry/sdk/extension/incubator/fileconfig/DeclarativeConfiguration.java

@@ -11,7 +11,6 @@
 import com.fasterxml.jackson.databind.ObjectMapper;
 import io.opentelemetry.api.incubator.config.DeclarativeConfigException;
 import io.opentelemetry.api.incubator.config.DeclarativeConfigProperties;
-import io.opentelemetry.api.incubator.config.InstrumentationConfigUtil;
 import io.opentelemetry.sdk.OpenTelemetrySdk;
 import io.opentelemetry.sdk.autoconfigure.internal.ComponentLoader;
 import io.opentelemetry.sdk.autoconfigure.internal.SpiHelper;
@@ -203,8 +202,8 @@ public static Sampler createSampler(DeclarativeConfigProperties genericSamplerMo
     YamlDeclarativeConfigProperties yamlDeclarativeConfigProperties =
         requireYamlDeclarativeConfigProperties(genericSamplerModel);
     SamplerModel samplerModel =
-        InstrumentationConfigUtil.convertToModel(
-            MAPPER, yamlDeclarativeConfigProperties, SamplerModel.class);
+        MAPPER.convertValue(
+            DeclarativeConfigProperties.toMap(yamlDeclarativeConfigProperties), SamplerModel.class);
     return createAndMaybeCleanup(
         SamplerFactory.getInstance(),
         SpiHelper.create(yamlDeclarativeConfigProperties.getComponentLoader()),