add logging when incompatible types are found (#7693)

Author: zeitlinger

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

@@ -20,6 +20,8 @@
 import java.util.Objects;
 import java.util.Set;
 import java.util.StringJoiner;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 import javax.annotation.Nullable;
 
 /**
@@ -36,6 +38,9 @@
  */
 public final class YamlDeclarativeConfigProperties implements DeclarativeConfigProperties {
 
+  private static final Logger logger =
+      Logger.getLogger(YamlDeclarativeConfigProperties.class.getName());
+
   private static final Set<Class<?>> SUPPORTED_SCALAR_TYPES =
       Collections.unmodifiableSet(
           new LinkedHashSet<>(
@@ -151,13 +156,13 @@ private static boolean isMap(Object object) {
   @Nullable
   @Override
   public String getString(String name) {
-    return stringOrNull(simpleEntries.get(name));
+    return stringOrNull(simpleEntries.get(name), name);
   }
 
   @Nullable
   @Override
   public Boolean getBoolean(String name) {
-    return booleanOrNull(simpleEntries.get(name));
+    return booleanOrNull(simpleEntries.get(name), name);
   }
 
   @Nullable
@@ -176,13 +181,13 @@ public Integer getInt(String name) {
   @Nullable
   @Override
   public Long getLong(String name) {
-    return longOrNull(simpleEntries.get(name));
+    return longOrNull(simpleEntries.get(name), name);
   }
 
   @Nullable
   @Override
   public Double getDouble(String name) {
-    return doubleOrNull(simpleEntries.get(name));
+    return doubleOrNull(simpleEntries.get(name), name);
   }
 
   @Nullable
@@ -210,13 +215,13 @@ public <T> List<T> getScalarList(String name, Class<T> scalarType) {
                   .map(
                       entry -> {
                         if (scalarType == String.class) {
-                          return stringOrNull(entry);
+                          return stringOrNull(entry, name);
                         } else if (scalarType == Boolean.class) {
-                          return booleanOrNull(entry);
+                          return booleanOrNull(entry, name);
                         } else if (scalarType == Long.class) {
-                          return longOrNull(entry);
+                          return longOrNull(entry, name);
                         } else if (scalarType == Double.class) {
-                          return doubleOrNull(entry);
+                          return doubleOrNull(entry, name);
                         }
                         return null;
                       })
@@ -231,40 +236,52 @@ public <T> List<T> getScalarList(String name, Class<T> scalarType) {
   }
 
   @Nullable
-  private static String stringOrNull(@Nullable Object value) {
+  private static String stringOrNull(@Nullable Object value, String name) {
     if (value instanceof String) {
       return (String) value;
     }
+    if (value != null) {
+      logTypeWarning(name, value, String.class);
+    }
     return null;
   }
 
   @Nullable
-  private static Boolean booleanOrNull(@Nullable Object value) {
+  private static Boolean booleanOrNull(@Nullable Object value, String name) {
     if (value instanceof Boolean) {
       return (Boolean) value;
     }
+    if (value != null) {
+      logTypeWarning(name, value, Boolean.class);
+    }
     return null;
   }
 
   @Nullable
-  private static Long longOrNull(@Nullable Object value) {
+  private static Long longOrNull(@Nullable Object value, String name) {
     if (value instanceof Integer) {
       return ((Integer) value).longValue();
     }
     if (value instanceof Long) {
       return (Long) value;
     }
+    if (value != null) {
+      logTypeWarning(name, value, Long.class);
+    }
     return null;
   }
 
   @Nullable
-  private static Double doubleOrNull(@Nullable Object value) {
+  private static Double doubleOrNull(@Nullable Object value, String name) {
     if (value instanceof Float) {
       return ((Float) value).doubleValue();
     }
     if (value instanceof Double) {
       return (Double) value;
     }
+    if (value != null) {
+      logTypeWarning(name, value, Double.class);
+    }
     return null;
   }
 
@@ -307,4 +324,11 @@ public String toString() {
   public ComponentLoader getComponentLoader() {
     return componentLoader;
   }
+
+  private static void logTypeWarning(String key, Object value, Class<?> expected) {
+    logger.log(
+        Level.WARNING,
+        "Ignoring value for key [{0}] because it is {1} instead of {2}: {3}",
+        new Object[] {key, value.getClass().getSimpleName(), expected.getSimpleName(), value});
+  }
 }

sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/YamlDeclarativeConfigPropertiesTest.java

@@ -9,6 +9,7 @@
 import static org.assertj.core.api.Assertions.assertThat;
 
 import com.google.common.collect.ImmutableSet;
+import io.github.netmikey.logunit.api.LogCapturer;
 import io.opentelemetry.api.incubator.config.DeclarativeConfigProperties;
 import io.opentelemetry.sdk.extension.incubator.fileconfig.internal.model.OpenTelemetryConfigurationModel;
 import java.io.ByteArrayInputStream;
@@ -18,9 +19,13 @@
 import java.util.List;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
 
 class YamlDeclarativeConfigPropertiesTest {
 
+  @RegisterExtension
+  LogCapturer logs = LogCapturer.create().captureForType(YamlDeclarativeConfigProperties.class);
+
   private static final String extendedSchema =
       "file_format: \"1.0-rc.1\"\n"
           + "disabled: false\n"
@@ -225,6 +230,22 @@ void wrongType() {
     assertThat(otherProps.getScalarList("str_key", String.class)).isNull();
     assertThat(otherProps.getStructured("str_key")).isNull();
     assertThat(otherProps.getStructuredList("str_key")).isNull();
+
+    assertWarning("Ignoring value for key [int_key] because it is Integer instead of String: 1");
+    assertWarning(
+        "Ignoring value for key [str_key] because it is String instead of Long: str_value");
+    assertWarning(
+        "Ignoring value for key [str_key] because it is String instead of Double: str_value");
+    assertWarning(
+        "Ignoring value for key [str_key] because it is String instead of Boolean: str_value");
+  }
+
+  private void assertWarning(String message) {
+    logs.assertContains(
+        e ->
+            String.format(e.getMessage().replaceAll("\\{\\d}", "%s"), e.getArgumentArray())
+                .contains(message),
+        message);
   }
 
   @Test