Stable Config improvements

internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigParser.java

@@ -16,6 +16,7 @@
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.function.Predicate;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -75,6 +76,7 @@ public static StableConfigSource.StableConfig parse(String filePath) throws IOEx
             return new StableConfigSource.StableConfig(configId, mergedConfigMap);
           }
         }
+        log.debug("No matching rule found in stable configuration file {}", filePath);
       }
       // If configs were found in apm_configuration_default, use them
       if (!configMap.isEmpty()) {
@@ -85,7 +87,6 @@ public static StableConfigSource.StableConfig parse(String filePath) throws IOEx
       if (configId != null) {
         return new StableConfigSource.StableConfig(configId, Collections.emptyMap());
       }
-
     } catch (IOException e) {
       log.debug("Failed to read the stable configuration file: {}", filePath, e);
     }
@@ -117,22 +118,32 @@ private static boolean matchOperator(String value, String operator, List<String>
       return false;
     }
     value = value.toLowerCase(Locale.ROOT);
+
+    Predicate<String> comparator;
+    switch (operator) {
+      case "equals":
+        comparator = value::equals;
+        break;
+      case "starts_with":
+        comparator = value::startsWith;
+        break;
+      case "ends_with":
+        comparator = value::endsWith;
+        break;
+      case "contains":
+        comparator = value::contains;
+        break;
+      default:
+        return false;
+    }
+
     for (String match : matches) {
       if (match == null) {
         continue;
       }
       match = match.toLowerCase(Locale.ROOT);
-      switch (operator) {
-        case "equals":
-          return value.equals(match);
-        case "starts_with":
-          return value.startsWith(match);
-        case "ends_with":
-          return value.endsWith(match);
-        case "contains":
-          return value.contains(match);
-        default:
-          return false;
+      if (comparator.test(match)) {
+        return true;
       }
     }
     return false;

internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigSource.java

@@ -3,6 +3,7 @@
 import static datadog.trace.util.Strings.propertyNameToEnvironmentVariableName;
 
 import datadog.trace.api.ConfigOrigin;
+import datadog.trace.bootstrap.config.provider.stableconfig.StableConfigMappingException;
 import java.io.File;
 import java.util.Collections;
 import java.util.Map;
@@ -38,10 +39,21 @@ public final class StableConfigSource extends ConfigProvider.Source {
       log.debug("Stable configuration file found at path: {}", file);
       cfg = StableConfigParser.parse(filePath);
     } catch (Throwable e) {
-      log.warn(
-          "Encountered the following exception when attempting to read stable configuration file at path: {}, dropping configs.",
-          file,
-          e);
+      if (e instanceof StableConfigMappingException
+          || e instanceof IllegalArgumentException
+          || e instanceof ClassCastException
+          || e instanceof NullPointerException) {
+        log.warn(
+            "YAML mapping error in stable configuration file {}: {}", filePath, e.getMessage());
+      } else if (log.isDebugEnabled()) {
+        log.error("Unexpected error while reading stable configuration file {}: {}", filePath, e);
+      } else {
+        log.error(
+            "Unexpected error while reading stable configuration file {}: {}",
+            filePath,
+            e.getMessage());
+      }
+
       cfg = StableConfig.EMPTY;
     }
     this.config = cfg;

internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java

@@ -1,5 +1,6 @@
 package datadog.trace.bootstrap.config.provider.stableconfig;
 
+import static datadog.trace.bootstrap.config.provider.stableconfig.StableConfigMappingException.throwStableConfigMappingException;
 import static java.util.Collections.*;
 import static java.util.stream.Collectors.toList;
 
@@ -25,13 +26,48 @@ public Rule(List<Selector> selectors, Map<String, Object> configuration) {
     this.configuration = configuration;
   }
 
-  public Rule(Object yaml) {
-    Map map = (Map) yaml;
-    selectors =
+  public static Rule from(Map<?, ?> map) {
+    Object selectorsObj = map.get("selectors");
+    if (selectorsObj == null) {
+      throwStableConfigMappingException("Missing 'selectors' in rule", map);
+    }
+
+    if (!(selectorsObj instanceof List)) {
+      throwStableConfigMappingException(
+          "'selectors' must be a list, but got: " + selectorsObj.getClass().getSimpleName(),
+          selectorsObj);
+    }
+
+    List<Selector> selectors =
         unmodifiableList(
-            ((List<Object>) map.get("selectors"))
-                .stream().filter(Objects::nonNull).map(Selector::new).collect(toList()));
-    configuration = unmodifiableMap((Map<String, Object>) map.get("configuration"));
+            ((List<?>) selectorsObj)
+                .stream()
+                    .filter(Objects::nonNull)
+                    .map(
+                        s -> {
+                          if (s instanceof Map) {
+                            return Selector.from((Map<?, ?>) s);
+                          }
+                          throwStableConfigMappingException(
+                              "Each selector must be a map, but got: "
+                                  + s.getClass().getSimpleName(),
+                              s);
+                          return null;
+                        })
+                    .collect(toList()));
+
+    Object configObj = map.get("configuration");
+    if (configObj == null) {
+      throwStableConfigMappingException("Missing 'configuration' in rule:", map);
+    }
+    if (!(configObj instanceof Map)) {
+      throwStableConfigMappingException(
+          "'configuration' must be a map, but got: " + configObj.getClass().getSimpleName(),
+          configObj);
+    }
+    Map<String, Object> configuration = (Map<String, Object>) configObj;
+
+    return new Rule(selectors, configuration);
   }
 
   public List<Selector> getSelectors() {

internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Selector.java

@@ -1,5 +1,7 @@
 package datadog.trace.bootstrap.config.provider.stableconfig;
 
+import static datadog.trace.bootstrap.config.provider.stableconfig.StableConfigMappingException.throwStableConfigMappingException;
+
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
@@ -17,14 +19,42 @@ public Selector(String origin, String key, List<String> matches, String operator
     this.operator = operator;
   }
 
-  public Selector(Object yaml) {
-    Map map = (Map) yaml;
-    origin = (String) map.get("origin");
-    key = (String) map.get("key");
-    List<String> rawMatches = (List<String>) map.get("matches");
-    matches =
+  public static Selector from(Map<?, ?> map) {
+    Object originObj = map.get("origin");
+    if (originObj == null) {
+      throwStableConfigMappingException("Missing 'origin' in selector:", map);
+    }
+    if (!(originObj instanceof String)) {
+      throwStableConfigMappingException(
+          "'origin' must be a string, but got: " + originObj.getClass().getSimpleName(), originObj);
+    }
+    String origin = (String) originObj;
+
+    Object keyObj = map.get("key");
+    String key = (keyObj instanceof String) ? (String) keyObj : null;
+
+    Object matchesObj = map.get("matches");
+    if (matchesObj != null && !(matchesObj instanceof List)) {
+      throwStableConfigMappingException(
+          "'matches' must be a list, but got: " + matchesObj.getClass().getSimpleName(),
+          matchesObj);
+    }
+    List<String> rawMatches = (List<String>) matchesObj;
+    List<String> matches =
         rawMatches != null ? Collections.unmodifiableList(rawMatches) : Collections.emptyList();
-    operator = (String) map.get("operator");
+
+    Object operatorObj = map.get("operator");
+    if (operatorObj == null) {
+      throwStableConfigMappingException("Missing 'operator' in selector:", map);
+    }
+    if (!(operatorObj instanceof String)) {
+      throwStableConfigMappingException(
+          "'operator' must be a string, but got: " + operatorObj.getClass().getSimpleName(),
+          operatorObj);
+    }
+    String operator = (String) operatorObj;
+
+    return new Selector(origin, key, matches, operator);
   }
 
   public String getOrigin() {

internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfig.java

@@ -1,10 +1,10 @@
 package datadog.trace.bootstrap.config.provider.stableconfig;
 
+import static datadog.trace.bootstrap.config.provider.stableconfig.StableConfigMappingException.throwStableConfigMappingException;
 import static java.util.Collections.emptyList;
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.unmodifiableList;
 import static java.util.Collections.unmodifiableMap;
-import static java.util.stream.Collectors.toList;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -21,10 +21,7 @@ public StableConfig(Object yaml) {
     this.apmConfigurationDefault =
         unmodifiableMap(
             (Map<String, Object>) map.getOrDefault("apm_configuration_default", emptyMap()));
-    this.apmConfigurationRules =
-        unmodifiableList(
-            ((List<Object>) map.getOrDefault("apm_configuration_rules", emptyList()))
-                .stream().map(Rule::new).collect(toList()));
+    this.apmConfigurationRules = parseRules(map);
   }
 
   // test only
@@ -45,4 +42,23 @@ public Map<String, Object> getApmConfigurationDefault() {
   public List<Rule> getApmConfigurationRules() {
     return apmConfigurationRules;
   }
+
+  private List<Rule> parseRules(Map<?, ?> map) {
+    Object rulesObj = map.get("apm_configuration_rules");
+    if (rulesObj instanceof List) {
+      List<?> rulesList = (List<?>) rulesObj;
+      List<Rule> rules = new ArrayList<>();
+      for (Object ruleObj : rulesList) {
+        if (ruleObj instanceof Map) {
+          rules.add(Rule.from((Map<?, ?>) ruleObj));
+        } else {
+          throwStableConfigMappingException(
+              "Rule must be a map, but got: " + ruleObj.getClass().getSimpleName(), ruleObj);
+          return emptyList();
+        }
+      }
+      return unmodifiableList(rules);
+    }
+    return emptyList();
+  }
 }

internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfigMappingException.java

@@ -0,0 +1,22 @@
+package datadog.trace.bootstrap.config.provider.stableconfig;
+
+public class StableConfigMappingException extends RuntimeException {
+  private static final int MAX_LEN = 100;
+
+  public StableConfigMappingException(String message) {
+    super(message);
+  }
+
+  static String safeToString(Object value) {
+    if (value == null) return "null";
+    String str = value.toString();
+    if (str.length() > MAX_LEN) {
+      return str.substring(0, MAX_LEN) + "...(truncated)";
+    }
+    return str;
+  }
+
+  public static void throwStableConfigMappingException(String message, Object value) {
+    throw new StableConfigMappingException(message + " " + safeToString(value));
+  }
+}

internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigMappingExceptionTest.groovy

@@ -0,0 +1,35 @@
+package datadog.trace.bootstrap.config.provider
+
+import datadog.trace.bootstrap.config.provider.stableconfig.StableConfigMappingException
+import spock.lang.Specification
+
+class StableConfigMappingExceptionTest extends Specification {
+
+  def "constructors work as expected"() {
+    when:
+    def ex1 = new StableConfigMappingException("msg")
+    def ex2 = new StableConfigMappingException("msg2")
+
+    then:
+    ex1.message == "msg"
+    ex1.cause == null
+    ex2.message == "msg2"
+  }
+
+  def "safeToString handles null"() {
+    expect:
+    StableConfigMappingException.safeToString(null) == "null"
+  }
+
+  def "safeToString handles short string"() {
+    expect:
+    StableConfigMappingException.safeToString("short string") == "short string"
+  }
+
+  def "safeToString handles long string"() {
+    given:
+    def longStr = "a" * 101
+    expect:
+    StableConfigMappingException.safeToString(longStr) == ("a" * 100) + "...(truncated)"
+  }
+}