pr review

Author: zeitlinger

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

@@ -71,61 +71,37 @@ final class DeclarativeConfigPropertiesBridge implements ConfigProperties {
   @Nullable
   @Override
   public String getString(String propertyName) {
-    Object value = getOverride(propertyName);
-    if (value != null) {
-      return value.toString();
-    }
-    return getPropertyValue(propertyName, DeclarativeConfigProperties::getString);
+    return getPropertyValue(propertyName, String.class, DeclarativeConfigProperties::getString);
   }
 
   @Nullable
   @Override
   public Boolean getBoolean(String propertyName) {
-    Object value = getOverride(propertyName);
-    if (value != null) {
-      return (Boolean) value;
-    }
-    return getPropertyValue(propertyName, DeclarativeConfigProperties::getBoolean);
+    return getPropertyValue(propertyName, Boolean.class, DeclarativeConfigProperties::getBoolean);
   }
 
   @Nullable
   @Override
   public Integer getInt(String propertyName) {
-    Object value = getOverride(propertyName);
-    if (value != null) {
-      return (Integer) value;
-    }
-    return getPropertyValue(propertyName, DeclarativeConfigProperties::getInt);
+    return getPropertyValue(propertyName, Integer.class, DeclarativeConfigProperties::getInt);
   }
 
   @Nullable
   @Override
   public Long getLong(String propertyName) {
-    Object value = getOverride(propertyName);
-    if (value != null) {
-      return (Long) value;
-    }
-    return getPropertyValue(propertyName, DeclarativeConfigProperties::getLong);
+    return getPropertyValue(propertyName, Long.class, DeclarativeConfigProperties::getLong);
   }
 
   @Nullable
   @Override
   public Double getDouble(String propertyName) {
-    Object value = getOverride(propertyName);
-    if (value != null) {
-      return (Double) value;
-    }
-    return getPropertyValue(propertyName, DeclarativeConfigProperties::getDouble);
+    return getPropertyValue(propertyName, Double.class, DeclarativeConfigProperties::getDouble);
   }
 
   @Nullable
   @Override
   public Duration getDuration(String propertyName) {
-    Object value = getOverride(propertyName);
-    if (value != null) {
-      return (Duration) value;
-    }
-    Long millis = getPropertyValue(propertyName, DeclarativeConfigProperties::getLong);
+    Long millis = getPropertyValue(propertyName, Long.class, DeclarativeConfigProperties::getLong);
     if (millis == null) {
       return null;
     }
@@ -135,26 +111,19 @@ public Duration getDuration(String propertyName) {
   @SuppressWarnings("unchecked")
   @Override
   public List<String> getList(String propertyName) {
-    Object value = getOverride(propertyName);
-    if (value != null) {
-      return (List<String>) value;
-    }
     List<String> propertyValue =
         getPropertyValue(
             propertyName,
+            List.class,
             (properties, lastPart) -> properties.getScalarList(lastPart, String.class));
     return propertyValue == null ? Collections.emptyList() : propertyValue;
   }
 
   @SuppressWarnings("unchecked")
   @Override
   public Map<String, String> getMap(String propertyName) {
-    Object fixed = getOverride(propertyName);
-    if (fixed != null) {
-      return (Map<String, String>) fixed;
-    }
     DeclarativeConfigProperties propertyValue =
-        getPropertyValue(propertyName, DeclarativeConfigProperties::getStructured);
+        getPropertyValue(propertyName, DeclarativeConfigProperties.class, DeclarativeConfigProperties::getStructured);
     if (propertyValue == null) {
       return Collections.emptyMap();
     }
@@ -174,7 +143,12 @@ public Map<String, String> getMap(String propertyName) {
 
   @Nullable
   private <T> T getPropertyValue(
-      String property, BiFunction<DeclarativeConfigProperties, String, T> extractor) {
+      String property, Class<T> clazz, BiFunction<DeclarativeConfigProperties, String, T> extractor) {
+    T override = clazz.cast(overrideValues.get(property));
+    if (override != null) {
+      return override;
+    }
+
     if (baseNode == null) {
       return null;
     }
@@ -212,9 +186,4 @@ private String translateProperty(String property) {
     }
     return property;
   }
-
-  @Nullable
-  private Object getOverride(String propertyName) {
-    return overrideValues.get(propertyName);
-  }
 }

javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/internal/DeclarativeConfigPropertiesBridgeBuilder.java

@@ -13,6 +13,7 @@
 import io.opentelemetry.sdk.autoconfigure.AutoConfiguredOpenTelemetrySdk;
 import io.opentelemetry.sdk.autoconfigure.internal.AutoConfigureUtil;
 import io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties;
+import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.Map;
 import javax.annotation.Nullable;
@@ -25,8 +26,13 @@
  * at any time.
  */
 public class DeclarativeConfigPropertiesBridgeBuilder {
-  private final LinkedHashMap<String, String> mappings = new LinkedHashMap<>();
-  private final Map<String, Object> overrideValues = new LinkedHashMap<>();
+  /**
+   * order is important here, so we use LinkedHashMap - see {@link #addMapping(String, String)} for
+   * more details
+   */
+  private final Map<String, String> mappings = new LinkedHashMap<>();
+
+  private final Map<String, Object> overrideValues = new HashMap<>();
 
   public DeclarativeConfigPropertiesBridgeBuilder() {}