From 370e894f826f807d1e9a9dc419e9194e3f1ce425 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Mon, 28 Jul 2025 12:02:44 -0400 Subject: [PATCH 01/28] Refactor matchOperator to use method reference comparator for efficiency --- .../config/provider/StableConfigParser.java | 33 ++++++++++++------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigParser.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigParser.java index 90a17667c56..3d3ef351993 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigParser.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigParser.java @@ -14,6 +14,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; @@ -115,22 +116,32 @@ private static boolean matchOperator(String value, String operator, List return false; } value = value.toLowerCase(Locale.ROOT); + + Predicate 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; From df4da2f4a331e424ff15cd27ae58c6188bc38a10 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Mon, 28 Jul 2025 16:32:35 -0400 Subject: [PATCH 02/28] Implement better exception handling and debug logging for StableConfig parsing errors --- .../config/provider/StableConfigParser.java | 1 - .../config/provider/StableConfigSource.java | 21 +++-- .../config/provider/stableconfig/Rule.java | 26 ++++++- .../provider/stableconfig/Selector.java | 38 +++++++++- .../StableConfigMappingException.java | 11 +++ .../provider/StableConfigSourceTest.groovy | 76 +++++++++++++++++++ 6 files changed, 161 insertions(+), 12 deletions(-) create mode 100644 internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfigMappingException.java diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigParser.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigParser.java index 3d3ef351993..8b05418a40b 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigParser.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigParser.java @@ -84,7 +84,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); } diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigSource.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigSource.java index ab634b6fc14..02ae1b71ef3 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigSource.java +++ b/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; @@ -37,11 +38,21 @@ public final class StableConfigSource extends ConfigProvider.Source { try { 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); + } catch (StableConfigMappingException + | IllegalArgumentException + | ClassCastException + | NullPointerException e) { + log.warn("YAML mapping error in stable configuration file {}: {}", filePath, e.getMessage()); + cfg = StableConfig.EMPTY; + } catch (Exception e) { + 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; diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java index 2d464b41ed1..1a23417b391 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java @@ -26,12 +26,34 @@ public Rule(List selectors, Map configuration) { } public Rule(Object yaml) { + if (!(yaml instanceof Map)) { + throw new StableConfigMappingException( + "Rule must be a map, but got: " + yaml.getClass().getSimpleName()); + } Map map = (Map) yaml; + + Object selectorsObj = map.get("selectors"); + if (selectorsObj == null) { + throw new StableConfigMappingException("Missing 'selectors' in rule: " + map); + } + if (!(selectorsObj instanceof List)) { + throw new StableConfigMappingException( + "'selectors' must be a list, but got: " + selectorsObj.getClass().getSimpleName()); + } selectors = unmodifiableList( - ((List) map.get("selectors")) + ((List) selectorsObj) .stream().filter(Objects::nonNull).map(Selector::new).collect(toList())); - configuration = unmodifiableMap((Map) map.get("configuration")); + + Object configObj = map.get("configuration"); + if (configObj == null) { + throw new StableConfigMappingException("Missing 'configuration' in rule: " + map); + } + if (!(configObj instanceof Map)) { + throw new StableConfigMappingException( + "'configuration' must be a map, but got: " + configObj.getClass().getSimpleName()); + } + configuration = unmodifiableMap((Map) configObj); } public List getSelectors() { diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Selector.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Selector.java index ac611751fa5..384f551b7d0 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Selector.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Selector.java @@ -18,13 +18,43 @@ public Selector(String origin, String key, List matches, String operator } public Selector(Object yaml) { + if (!(yaml instanceof Map)) { + throw new StableConfigMappingException( + "Selector must be a map, but got: " + yaml.getClass().getSimpleName()); + } Map map = (Map) yaml; - origin = (String) map.get("origin"); - key = (String) map.get("key"); - List rawMatches = (List) map.get("matches"); + + Object originObj = map.get("origin"); + if (originObj == null) { + throw new StableConfigMappingException("Missing 'origin' in selector: " + map); + } + if (!(originObj instanceof String)) { + throw new StableConfigMappingException( + "'origin' must be a string, but got: " + originObj.getClass().getSimpleName()); + } + origin = (String) originObj; + + Object keyObj = map.get("key"); + key = (keyObj instanceof String) ? (String) keyObj : null; + + Object matchesObj = map.get("matches"); + if (matchesObj != null && !(matchesObj instanceof List)) { + throw new StableConfigMappingException( + "'matches' must be a list, but got: " + matchesObj.getClass().getSimpleName()); + } + List rawMatches = (List) matchesObj; matches = rawMatches != null ? Collections.unmodifiableList(rawMatches) : Collections.emptyList(); - operator = (String) map.get("operator"); + + Object operatorObj = map.get("operator"); + if (operatorObj == null) { + throw new StableConfigMappingException("Missing 'operator' in selector: " + map); + } + if (!(operatorObj instanceof String)) { + throw new StableConfigMappingException( + "'operator' must be a string, but got: " + operatorObj.getClass().getSimpleName()); + } + operator = (String) operatorObj; } public String getOrigin() { diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfigMappingException.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfigMappingException.java new file mode 100644 index 00000000000..662a657b883 --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfigMappingException.java @@ -0,0 +1,11 @@ +package datadog.trace.bootstrap.config.provider.stableconfig; + +public class StableConfigMappingException extends RuntimeException { + public StableConfigMappingException(String message) { + super(message); + } + + public StableConfigMappingException(String message, Throwable cause) { + super(message, cause); + } +} diff --git a/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigSourceTest.groovy b/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigSourceTest.groovy index b3ec755ddc1..d87d4396e75 100644 --- a/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigSourceTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigSourceTest.groovy @@ -9,6 +9,10 @@ import datadog.trace.bootstrap.config.provider.stableconfig.StableConfig import datadog.trace.test.util.DDSpecification import org.snakeyaml.engine.v2.api.Dump import org.snakeyaml.engine.v2.api.DumpSettings +import ch.qos.logback.classic.Logger +import ch.qos.logback.classic.spi.ILoggingEvent +import ch.qos.logback.core.read.ListAppender +import org.slf4j.LoggerFactory import java.nio.file.Files import java.nio.file.Path @@ -108,6 +112,78 @@ class StableConfigSourceTest extends DDSpecification { "12345" | ["DD_KEY_ONE": "one", "DD_KEY_TWO": "two"] | Arrays.asList(sampleMatchingRule, sampleNonMatchingRule) } + def "test parse invalid logs mapping errors"() { + given: + Logger logbackLogger = (Logger) LoggerFactory.getLogger(datadog.trace.bootstrap.config.provider.StableConfigSource) + def listAppender = new ListAppender() + listAppender.start() + logbackLogger.addAppender(listAppender) + + def tempFile = File.createTempFile("testFile_", ".yaml") + tempFile.text = yaml + + when: + def stableCfg = new StableConfigSource(tempFile.absolutePath, ConfigOrigin.LOCAL_STABLE_CONFIG) + + then: + stableCfg.config == StableConfigSource.StableConfig.EMPTY + def warnLogs = listAppender.list.findAll { it.level.toString() == 'WARN' } + warnLogs.any { it.formattedMessage.contains(expectedLogSubstring) } + + cleanup: + tempFile.delete() + logbackLogger.detachAppender(listAppender) + + where: + yaml | expectedLogSubstring + // Missing 'origin' in selector + '''apm_configuration_rules: + - selectors: + - key: "someKey" + matches: ["someValue"] + operator: equals + configuration: + DD_SERVICE: "test" + ''' | "Missing 'origin' in selector" + // Missing 'configuration' in rule + '''apm_configuration_rules: + - selectors: + - origin: process_arguments + key: "-Dfoo" + matches: ["bar"] + operator: equals + ''' | "Missing 'configuration' in rule" + // Missing 'selectors' in rule + '''apm_configuration_rules: + - configuration: + DD_SERVICE: "test" + ''' | "Missing 'selectors' in rule" + // Triggers ClassCastException (selectors should be a list, not a string) + '''apm_configuration_rules: + - selectors: "not-a-list" + configuration: + DD_SERVICE: "test" + ''' | "'selectors' must be a list, but got: String" + // configuration present but not a map + '''apm_configuration_rules: + - selectors: + - origin: process_arguments + key: "-Dfoo" + matches: ["bar"] + operator: equals + configuration: "not-a-map" + ''' | "'configuration' must be a map, but got: String" + // configuration present but not a map (integer) + '''apm_configuration_rules: + - selectors: + - origin: process_arguments + key: "-Dfoo" + matches: ["bar"] + operator: equals + configuration: 12345 + ''' | "'configuration' must be a map, but got: Integer" + } + // Corrupt YAML string variable used for testing, defined outside the 'where' block for readability def static corruptYaml = ''' abc: 123 From 7049d3676b808a7abec5c512ebbc753eed825eab Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Mon, 28 Jul 2025 16:43:39 -0400 Subject: [PATCH 03/28] Add a debug log about no rules getting applied --- .../trace/bootstrap/config/provider/StableConfigParser.java | 1 + 1 file changed, 1 insertion(+) diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigParser.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigParser.java index 8b05418a40b..a076934e927 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigParser.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigParser.java @@ -74,6 +74,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()) { From e730cbe49dec0ed728c775f0883c6d247b745e20 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Tue, 29 Jul 2025 11:37:01 -0400 Subject: [PATCH 04/28] Improve test file logic and cleanup in StableConfigSourceTest class --- .../provider/StableConfigSourceTest.groovy | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigSourceTest.groovy b/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigSourceTest.groovy index d87d4396e75..85c803a302d 100644 --- a/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigSourceTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigSourceTest.groovy @@ -30,18 +30,17 @@ class StableConfigSourceTest extends DDSpecification { } def "test empty file"() { - when: + given: Path filePath = Files.createTempFile("testFile_", ".yaml") - then: - if (filePath == null) { - throw new AssertionError("Failed to create: " + filePath) - } when: StableConfigSource config = new StableConfigSource(filePath.toString(), ConfigOrigin.LOCAL_STABLE_CONFIG) then: config.getKeys().size() == 0 config.getConfigId() == null + + cleanup: + Files.delete(filePath) } def "test file invalid format"() { @@ -60,6 +59,8 @@ class StableConfigSourceTest extends DDSpecification { then: stableCfg.getConfigId() == null stableCfg.getKeys().size() == 0 + + cleanup: Files.delete(filePath) where: @@ -69,12 +70,8 @@ class StableConfigSourceTest extends DDSpecification { } def "test file valid format"() { - when: + given: Path filePath = Files.createTempFile("testFile_", ".yaml") - then: - if (filePath == null) { - throw new AssertionError("Failed to create: " + filePath) - } when: StableConfig stableConfigYaml = new StableConfig(configId, defaultConfigs) @@ -104,6 +101,8 @@ class StableConfigSourceTest extends DDSpecification { !cfgKeys.contains(keyString) } } + + cleanup: Files.delete(filePath) where: @@ -114,7 +113,7 @@ class StableConfigSourceTest extends DDSpecification { def "test parse invalid logs mapping errors"() { given: - Logger logbackLogger = (Logger) LoggerFactory.getLogger(datadog.trace.bootstrap.config.provider.StableConfigSource) + Logger logbackLogger = (Logger) LoggerFactory.getLogger(StableConfigSource) def listAppender = new ListAppender() listAppender.start() logbackLogger.addAppender(listAppender) From 5b639aab11c64eb5531394d2174a9dcd00fc86c6 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Thu, 31 Jul 2025 12:44:10 -0400 Subject: [PATCH 05/28] Github suggestions: Use static factory methods as constructors for Rule and Selector classes --- .../config/provider/stableconfig/Rule.java | 28 +++++++++++-------- .../provider/stableconfig/Selector.java | 18 +++++------- .../provider/stableconfig/StableConfig.java | 27 ++++++++++++++---- 3 files changed, 46 insertions(+), 27 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java index 1a23417b391..66f71c192a6 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java @@ -25,13 +25,7 @@ public Rule(List selectors, Map configuration) { this.configuration = configuration; } - public Rule(Object yaml) { - if (!(yaml instanceof Map)) { - throw new StableConfigMappingException( - "Rule must be a map, but got: " + yaml.getClass().getSimpleName()); - } - Map map = (Map) yaml; - + public static Rule from(Map map) { Object selectorsObj = map.get("selectors"); if (selectorsObj == null) { throw new StableConfigMappingException("Missing 'selectors' in rule: " + map); @@ -40,10 +34,20 @@ public Rule(Object yaml) { throw new StableConfigMappingException( "'selectors' must be a list, but got: " + selectorsObj.getClass().getSimpleName()); } - selectors = + List selectors = unmodifiableList( - ((List) selectorsObj) - .stream().filter(Objects::nonNull).map(Selector::new).collect(toList())); + ((List) selectorsObj) + .stream() + .filter(Objects::nonNull) + .map( + s -> { + if (!(s instanceof Map)) { + throw new StableConfigMappingException( + "Selector must be a map, but got: " + s.getClass().getSimpleName()); + } + return Selector.from((Map) s); + }) + .collect(toList())); Object configObj = map.get("configuration"); if (configObj == null) { @@ -53,7 +57,9 @@ public Rule(Object yaml) { throw new StableConfigMappingException( "'configuration' must be a map, but got: " + configObj.getClass().getSimpleName()); } - configuration = unmodifiableMap((Map) configObj); + Map configuration = (Map) configObj; + + return new Rule(selectors, configuration); } public List getSelectors() { diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Selector.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Selector.java index 384f551b7d0..ef59dffdf4d 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Selector.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Selector.java @@ -17,13 +17,7 @@ public Selector(String origin, String key, List matches, String operator this.operator = operator; } - public Selector(Object yaml) { - if (!(yaml instanceof Map)) { - throw new StableConfigMappingException( - "Selector must be a map, but got: " + yaml.getClass().getSimpleName()); - } - Map map = (Map) yaml; - + public static Selector from(Map map) { Object originObj = map.get("origin"); if (originObj == null) { throw new StableConfigMappingException("Missing 'origin' in selector: " + map); @@ -32,10 +26,10 @@ public Selector(Object yaml) { throw new StableConfigMappingException( "'origin' must be a string, but got: " + originObj.getClass().getSimpleName()); } - origin = (String) originObj; + String origin = (String) originObj; Object keyObj = map.get("key"); - key = (keyObj instanceof String) ? (String) keyObj : null; + String key = (keyObj instanceof String) ? (String) keyObj : null; Object matchesObj = map.get("matches"); if (matchesObj != null && !(matchesObj instanceof List)) { @@ -43,7 +37,7 @@ public Selector(Object yaml) { "'matches' must be a list, but got: " + matchesObj.getClass().getSimpleName()); } List rawMatches = (List) matchesObj; - matches = + List matches = rawMatches != null ? Collections.unmodifiableList(rawMatches) : Collections.emptyList(); Object operatorObj = map.get("operator"); @@ -54,7 +48,9 @@ public Selector(Object yaml) { throw new StableConfigMappingException( "'operator' must be a string, but got: " + operatorObj.getClass().getSimpleName()); } - operator = (String) operatorObj; + String operator = (String) operatorObj; + + return new Selector(origin, key, matches, operator); } public String getOrigin() { diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfig.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfig.java index 58fa3c4f826..5beacd08c6e 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfig.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfig.java @@ -4,7 +4,6 @@ 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 +20,7 @@ public StableConfig(Object yaml) { this.apmConfigurationDefault = unmodifiableMap( (Map) map.getOrDefault("apm_configuration_default", emptyMap())); - this.apmConfigurationRules = - unmodifiableList( - ((List) map.getOrDefault("apm_configuration_rules", emptyList())) - .stream().map(Rule::new).collect(toList())); + this.apmConfigurationRules = parseRules(map); } // test only @@ -45,4 +41,25 @@ public Map getApmConfigurationDefault() { public List getApmConfigurationRules() { return apmConfigurationRules; } + + private List parseRules(Map map) { + Object rulesObj = map.get("apm_configuration_rules"); + if (rulesObj instanceof List) { + List rulesList = (List) rulesObj; + List rules = new ArrayList<>(); + for (Object ruleObj : rulesList) { + if (ruleObj instanceof Map) { + rules.add(Rule.from((Map) ruleObj)); + } else { + // Log or throw with a detailed message + throw new StableConfigMappingException( + "Rule must be a map, but got: " + + (ruleObj == null ? "null" : ruleObj.getClass().getSimpleName())); + } + } + return unmodifiableList(rules); + } else { + return emptyList(); + } + } } From 85d11cfd04038d8f53c8a76b05834438910175ac Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Fri, 1 Aug 2025 10:03:37 -0400 Subject: [PATCH 06/28] Improve exception message: include value of object that caused the failure --- .../config/provider/stableconfig/Rule.java | 15 +++- .../provider/stableconfig/Selector.java | 15 +++- .../provider/stableconfig/StableConfig.java | 4 +- .../StableConfigMappingException.java | 9 +++ .../provider/StableConfigSourceTest.groovy | 75 ++++++++++--------- 5 files changed, 74 insertions(+), 44 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java index 66f71c192a6..a5b7af8bc6e 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java @@ -32,7 +32,10 @@ public static Rule from(Map map) { } if (!(selectorsObj instanceof List)) { throw new StableConfigMappingException( - "'selectors' must be a list, but got: " + selectorsObj.getClass().getSimpleName()); + "'selectors' must be a list, but got: " + + selectorsObj.getClass().getSimpleName() + + ", value: " + + StableConfigMappingException.safeToString(selectorsObj)); } List selectors = unmodifiableList( @@ -43,7 +46,10 @@ public static Rule from(Map map) { s -> { if (!(s instanceof Map)) { throw new StableConfigMappingException( - "Selector must be a map, but got: " + s.getClass().getSimpleName()); + "Each selector must be a map, but got: " + + s.getClass().getSimpleName() + + ", value: " + + StableConfigMappingException.safeToString(s)); } return Selector.from((Map) s); }) @@ -55,7 +61,10 @@ public static Rule from(Map map) { } if (!(configObj instanceof Map)) { throw new StableConfigMappingException( - "'configuration' must be a map, but got: " + configObj.getClass().getSimpleName()); + "'configuration' must be a map, but got: " + + configObj.getClass().getSimpleName() + + ", value: " + + StableConfigMappingException.safeToString(configObj)); } Map configuration = (Map) configObj; diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Selector.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Selector.java index ef59dffdf4d..71674c2a6d3 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Selector.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Selector.java @@ -24,7 +24,10 @@ public static Selector from(Map map) { } if (!(originObj instanceof String)) { throw new StableConfigMappingException( - "'origin' must be a string, but got: " + originObj.getClass().getSimpleName()); + "'origin' must be a string, but got: " + + originObj.getClass().getSimpleName() + + ", value: " + + StableConfigMappingException.safeToString(originObj)); } String origin = (String) originObj; @@ -34,7 +37,10 @@ public static Selector from(Map map) { Object matchesObj = map.get("matches"); if (matchesObj != null && !(matchesObj instanceof List)) { throw new StableConfigMappingException( - "'matches' must be a list, but got: " + matchesObj.getClass().getSimpleName()); + "'matches' must be a list, but got: " + + matchesObj.getClass().getSimpleName() + + ", value: " + + StableConfigMappingException.safeToString(matchesObj)); } List rawMatches = (List) matchesObj; List matches = @@ -46,7 +52,10 @@ public static Selector from(Map map) { } if (!(operatorObj instanceof String)) { throw new StableConfigMappingException( - "'operator' must be a string, but got: " + operatorObj.getClass().getSimpleName()); + "'operator' must be a string, but got: " + + operatorObj.getClass().getSimpleName() + + ", value: " + + StableConfigMappingException.safeToString(operatorObj)); } String operator = (String) operatorObj; diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfig.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfig.java index 5beacd08c6e..fbb0aa6c4e1 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfig.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfig.java @@ -54,7 +54,9 @@ private List parseRules(Map map) { // Log or throw with a detailed message throw new StableConfigMappingException( "Rule must be a map, but got: " - + (ruleObj == null ? "null" : ruleObj.getClass().getSimpleName())); + + (ruleObj == null ? "null" : ruleObj.getClass().getSimpleName()) + + ", value: " + + StableConfigMappingException.safeToString(ruleObj)); } } return unmodifiableList(rules); diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfigMappingException.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfigMappingException.java index 662a657b883..d634df5189a 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfigMappingException.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfigMappingException.java @@ -8,4 +8,13 @@ public StableConfigMappingException(String message) { public StableConfigMappingException(String message, Throwable cause) { super(message, cause); } + + public static String safeToString(Object value) { + if (value == null) return "null"; + String str = value.toString(); + if (str.length() > 100) { + return str.substring(0, 100) + "...(truncated)"; + } + return str; + } } diff --git a/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigSourceTest.groovy b/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigSourceTest.groovy index 85c803a302d..211f39dd83d 100644 --- a/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigSourceTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigSourceTest.groovy @@ -135,52 +135,53 @@ class StableConfigSourceTest extends DDSpecification { where: yaml | expectedLogSubstring - // Missing 'origin' in selector '''apm_configuration_rules: - - selectors: - - key: "someKey" - matches: ["someValue"] - operator: equals - configuration: - DD_SERVICE: "test" - ''' | "Missing 'origin' in selector" - // Missing 'configuration' in rule + - selectors: + - key: "someKey" + matches: ["someValue"] + operator: equals + configuration: + DD_SERVICE: "test" + ''' | "Missing 'origin' in selector" '''apm_configuration_rules: - - selectors: - - origin: process_arguments - key: "-Dfoo" - matches: ["bar"] - operator: equals - ''' | "Missing 'configuration' in rule" - // Missing 'selectors' in rule + - selectors: + - origin: process_arguments + key: "-Dfoo" + matches: ["bar"] + operator: equals + ''' | "Missing 'configuration' in rule" '''apm_configuration_rules: - configuration: DD_SERVICE: "test" ''' | "Missing 'selectors' in rule" - // Triggers ClassCastException (selectors should be a list, not a string) '''apm_configuration_rules: - - selectors: "not-a-list" - configuration: - DD_SERVICE: "test" - ''' | "'selectors' must be a list, but got: String" - // configuration present but not a map + - selectors: "not-a-list" + configuration: + DD_SERVICE: "test" + ''' | "'selectors' must be a list, but got: String" + '''apm_configuration_rules: + - selectors: + - "not-a-map" + ''' | "Each selector must be a map, but got: String" + '''apm_configuration_rules: + - selectors: + - origin: process_arguments + key: "-Dfoo" + matches: ["bar"] + operator: equals + configuration: "not-a-map" + ''' | "'configuration' must be a map, but got: String" '''apm_configuration_rules: - - selectors: - - origin: process_arguments - key: "-Dfoo" - matches: ["bar"] - operator: equals - configuration: "not-a-map" - ''' | "'configuration' must be a map, but got: String" - // configuration present but not a map (integer) + - selectors: + - origin: process_arguments + key: "-Dfoo" + matches: ["bar"] + operator: equals + configuration: 12345 + ''' | "'configuration' must be a map, but got: Integer" '''apm_configuration_rules: - - selectors: - - origin: process_arguments - key: "-Dfoo" - matches: ["bar"] - operator: equals - configuration: 12345 - ''' | "'configuration' must be a map, but got: Integer" + - "not-a-map" + ''' | "Rule must be a map, but got: String" } // Corrupt YAML string variable used for testing, defined outside the 'where' block for readability From 6ab4b6c0c42e820ba4d453b71e5c0e02c5372a87 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Fri, 1 Aug 2025 10:11:02 -0400 Subject: [PATCH 07/28] Add more test cases to StableConfigSourceTest to improve codecov --- .../provider/StableConfigSourceTest.groovy | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigSourceTest.groovy b/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigSourceTest.groovy index 211f39dd83d..16b1d268044 100644 --- a/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigSourceTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigSourceTest.groovy @@ -182,6 +182,32 @@ class StableConfigSourceTest extends DDSpecification { '''apm_configuration_rules: - "not-a-map" ''' | "Rule must be a map, but got: String" + '''apm_configuration_rules: + - selectors: + - origin: process_arguments + key: "-Dfoo" + matches: "not-a-list" + operator: equals + configuration: + DD_SERVICE: "test" + ''' | "'matches' must be a list, but got: String" + '''apm_configuration_rules: + - selectors: + - origin: process_arguments + key: "-Dfoo" + matches: ["bar"] + configuration: + DD_SERVICE: "test" + ''' | "Missing 'operator' in selector" + '''apm_configuration_rules: + - selectors: + - origin: process_arguments + key: "-Dfoo" + matches: ["bar"] + operator: 12345 + configuration: + DD_SERVICE: "test" + ''' | "'operator' must be a string, but got: Integer" } // Corrupt YAML string variable used for testing, defined outside the 'where' block for readability From 007f58a5dce195704cdef356c12c665ae1c313c8 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Fri, 1 Aug 2025 16:28:19 -0400 Subject: [PATCH 08/28] Add test coverage for StableConfigMappingException --- .../StableConfigMappingException.java | 4 --- .../StableConfigMappingExceptionTest.groovy | 35 +++++++++++++++++++ 2 files changed, 35 insertions(+), 4 deletions(-) create mode 100644 internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigMappingExceptionTest.groovy diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfigMappingException.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfigMappingException.java index d634df5189a..e1df66863d8 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfigMappingException.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfigMappingException.java @@ -5,10 +5,6 @@ public StableConfigMappingException(String message) { super(message); } - public StableConfigMappingException(String message, Throwable cause) { - super(message, cause); - } - public static String safeToString(Object value) { if (value == null) return "null"; String str = value.toString(); diff --git a/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigMappingExceptionTest.groovy b/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigMappingExceptionTest.groovy new file mode 100644 index 00000000000..6096099ea1c --- /dev/null +++ b/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)" + } +} From 473c59bebe36dd929e574af770b453ecc00e3b52 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler <46911781+mtoffl01@users.noreply.github.com> Date: Fri, 8 Aug 2025 11:55:36 -0400 Subject: [PATCH 09/28] Update internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java Co-authored-by: Alexey Kuznetsov --- .../trace/bootstrap/config/provider/stableconfig/Rule.java | 1 + 1 file changed, 1 insertion(+) diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java index a5b7af8bc6e..cdc323495cf 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java @@ -30,6 +30,7 @@ public static Rule from(Map map) { if (selectorsObj == null) { throw new StableConfigMappingException("Missing 'selectors' in rule: " + map); } + if (!(selectorsObj instanceof List)) { throw new StableConfigMappingException( "'selectors' must be a list, but got: " From feb1ba04f2c0c070146e2c9c6bd59841ef2f5bee Mon Sep 17 00:00:00 2001 From: Mikayla Toffler <46911781+mtoffl01@users.noreply.github.com> Date: Fri, 8 Aug 2025 11:55:46 -0400 Subject: [PATCH 10/28] Update internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java Co-authored-by: Alexey Kuznetsov --- .../trace/bootstrap/config/provider/stableconfig/Rule.java | 1 + 1 file changed, 1 insertion(+) diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java index cdc323495cf..49a2fa76e0e 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java @@ -38,6 +38,7 @@ public static Rule from(Map map) { + ", value: " + StableConfigMappingException.safeToString(selectorsObj)); } + List selectors = unmodifiableList( ((List) selectorsObj) From f93546abda86fe60a9222abbba7f954997e64657 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Fri, 8 Aug 2025 13:32:14 -0400 Subject: [PATCH 11/28] Use safeToString when printing stableconfig map --- .../trace/bootstrap/config/provider/stableconfig/Rule.java | 6 ++++-- .../bootstrap/config/provider/stableconfig/Selector.java | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java index 49a2fa76e0e..86fcf5fbac5 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java @@ -28,7 +28,8 @@ public Rule(List selectors, Map configuration) { public static Rule from(Map map) { Object selectorsObj = map.get("selectors"); if (selectorsObj == null) { - throw new StableConfigMappingException("Missing 'selectors' in rule: " + map); + throw new StableConfigMappingException( + "Missing 'selectors' in rule: " + StableConfigMappingException.safeToString(map)); } if (!(selectorsObj instanceof List)) { @@ -59,7 +60,8 @@ public static Rule from(Map map) { Object configObj = map.get("configuration"); if (configObj == null) { - throw new StableConfigMappingException("Missing 'configuration' in rule: " + map); + throw new StableConfigMappingException( + "Missing 'configuration' in rule: " + StableConfigMappingException.safeToString(map)); } if (!(configObj instanceof Map)) { throw new StableConfigMappingException( diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Selector.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Selector.java index 71674c2a6d3..a08853ce93e 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Selector.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Selector.java @@ -20,7 +20,8 @@ public Selector(String origin, String key, List matches, String operator public static Selector from(Map map) { Object originObj = map.get("origin"); if (originObj == null) { - throw new StableConfigMappingException("Missing 'origin' in selector: " + map); + throw new StableConfigMappingException( + "Missing 'origin' in selector: " + StableConfigMappingException.safeToString(map)); } if (!(originObj instanceof String)) { throw new StableConfigMappingException( @@ -48,7 +49,8 @@ public static Selector from(Map map) { Object operatorObj = map.get("operator"); if (operatorObj == null) { - throw new StableConfigMappingException("Missing 'operator' in selector: " + map); + throw new StableConfigMappingException( + "Missing 'operator' in selector: " + StableConfigMappingException.safeToString(map)); } if (!(operatorObj instanceof String)) { throw new StableConfigMappingException( From 780aee31a99c510dd4ffb0c72ea7cd771cde395d Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Fri, 8 Aug 2025 13:40:34 -0400 Subject: [PATCH 12/28] rearrange logic in selectors creation for better readability --- .../config/provider/stableconfig/Rule.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java index 86fcf5fbac5..d2798e07f9f 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java @@ -47,14 +47,14 @@ public static Rule from(Map map) { .filter(Objects::nonNull) .map( s -> { - if (!(s instanceof Map)) { - throw new StableConfigMappingException( - "Each selector must be a map, but got: " - + s.getClass().getSimpleName() - + ", value: " - + StableConfigMappingException.safeToString(s)); + if (s instanceof Map) { + return Selector.from((Map) s); } - return Selector.from((Map) s); + throw new StableConfigMappingException( + "Each selector must be a map, but got: " + + s.getClass().getSimpleName() + + ", value: " + + StableConfigMappingException.safeToString(s)); }) .collect(toList())); From ac5e05acc6b8eea9e22a8b005d5bec414a66e74a Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Fri, 8 Aug 2025 13:46:47 -0400 Subject: [PATCH 13/28] delete superfluous else --- .../bootstrap/config/provider/stableconfig/StableConfig.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfig.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfig.java index fbb0aa6c4e1..d89a29ebb17 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfig.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfig.java @@ -60,8 +60,7 @@ private List parseRules(Map map) { } } return unmodifiableList(rules); - } else { - return emptyList(); } + return emptyList(); } } From bcebbcb79080150fb72530c9f35f4518029bbcd6 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Fri, 8 Aug 2025 13:48:55 -0400 Subject: [PATCH 14/28] Use constant to represent max length in safeToString --- .../provider/stableconfig/StableConfigMappingException.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfigMappingException.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfigMappingException.java index e1df66863d8..9320e0901b2 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfigMappingException.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfigMappingException.java @@ -1,6 +1,8 @@ package datadog.trace.bootstrap.config.provider.stableconfig; public class StableConfigMappingException extends RuntimeException { + private static final int maxLen = 10; + public StableConfigMappingException(String message) { super(message); } @@ -8,8 +10,8 @@ public StableConfigMappingException(String message) { public static String safeToString(Object value) { if (value == null) return "null"; String str = value.toString(); - if (str.length() > 100) { - return str.substring(0, 100) + "...(truncated)"; + if (str.length() > maxLen) { + return str.substring(0, maxLen) + "...(truncated)"; } return str; } From 0baccfdb19ffe9915cd400a123c163f271312f92 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Mon, 11 Aug 2025 14:00:31 -0400 Subject: [PATCH 15/28] use throwStableConfigMappingException helper function --- .../config/provider/stableconfig/Rule.java | 32 ++++++++----------- .../provider/stableconfig/Selector.java | 31 +++++++----------- .../provider/stableconfig/StableConfig.java | 9 ++---- .../StableConfigMappingException.java | 8 +++-- 4 files changed, 35 insertions(+), 45 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java index d2798e07f9f..eaee26e82ec 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java @@ -28,16 +28,14 @@ public Rule(List selectors, Map configuration) { public static Rule from(Map map) { Object selectorsObj = map.get("selectors"); if (selectorsObj == null) { - throw new StableConfigMappingException( - "Missing 'selectors' in rule: " + StableConfigMappingException.safeToString(map)); + StableConfigMappingException.throwStableConfigMappingException( + "Missing 'selectors' in rule", map); } if (!(selectorsObj instanceof List)) { - throw new StableConfigMappingException( - "'selectors' must be a list, but got: " - + selectorsObj.getClass().getSimpleName() - + ", value: " - + StableConfigMappingException.safeToString(selectorsObj)); + StableConfigMappingException.throwStableConfigMappingException( + "'selectors' must be a list, but got: " + selectorsObj.getClass().getSimpleName(), + selectorsObj); } List selectors = @@ -50,25 +48,23 @@ public static Rule from(Map map) { if (s instanceof Map) { return Selector.from((Map) s); } - throw new StableConfigMappingException( + StableConfigMappingException.throwStableConfigMappingException( "Each selector must be a map, but got: " - + s.getClass().getSimpleName() - + ", value: " - + StableConfigMappingException.safeToString(s)); + + s.getClass().getSimpleName(), + s); + return null; }) .collect(toList())); Object configObj = map.get("configuration"); if (configObj == null) { - throw new StableConfigMappingException( - "Missing 'configuration' in rule: " + StableConfigMappingException.safeToString(map)); + StableConfigMappingException.throwStableConfigMappingException( + "Missing 'configuration' in rule", map); } if (!(configObj instanceof Map)) { - throw new StableConfigMappingException( - "'configuration' must be a map, but got: " - + configObj.getClass().getSimpleName() - + ", value: " - + StableConfigMappingException.safeToString(configObj)); + StableConfigMappingException.throwStableConfigMappingException( + "'configuration' must be a map, but got: " + configObj.getClass().getSimpleName(), + configObj); } Map configuration = (Map) configObj; diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Selector.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Selector.java index a08853ce93e..5e58235b226 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Selector.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Selector.java @@ -20,15 +20,12 @@ public Selector(String origin, String key, List matches, String operator public static Selector from(Map map) { Object originObj = map.get("origin"); if (originObj == null) { - throw new StableConfigMappingException( - "Missing 'origin' in selector: " + StableConfigMappingException.safeToString(map)); + StableConfigMappingException.throwStableConfigMappingException( + "Missing 'origin' in selector", map); } if (!(originObj instanceof String)) { - throw new StableConfigMappingException( - "'origin' must be a string, but got: " - + originObj.getClass().getSimpleName() - + ", value: " - + StableConfigMappingException.safeToString(originObj)); + StableConfigMappingException.throwStableConfigMappingException( + "'origin' must be a string, but got: " + originObj.getClass().getSimpleName(), originObj); } String origin = (String) originObj; @@ -37,11 +34,9 @@ public static Selector from(Map map) { Object matchesObj = map.get("matches"); if (matchesObj != null && !(matchesObj instanceof List)) { - throw new StableConfigMappingException( - "'matches' must be a list, but got: " - + matchesObj.getClass().getSimpleName() - + ", value: " - + StableConfigMappingException.safeToString(matchesObj)); + StableConfigMappingException.throwStableConfigMappingException( + "'matches' must be a list, but got: " + matchesObj.getClass().getSimpleName(), + matchesObj); } List rawMatches = (List) matchesObj; List matches = @@ -49,15 +44,13 @@ public static Selector from(Map map) { Object operatorObj = map.get("operator"); if (operatorObj == null) { - throw new StableConfigMappingException( - "Missing 'operator' in selector: " + StableConfigMappingException.safeToString(map)); + StableConfigMappingException.throwStableConfigMappingException( + "Missing 'operator' in selector", map); } if (!(operatorObj instanceof String)) { - throw new StableConfigMappingException( - "'operator' must be a string, but got: " - + operatorObj.getClass().getSimpleName() - + ", value: " - + StableConfigMappingException.safeToString(operatorObj)); + StableConfigMappingException.throwStableConfigMappingException( + "'operator' must be a string, but got: " + operatorObj.getClass().getSimpleName(), + operatorObj); } String operator = (String) operatorObj; diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfig.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfig.java index d89a29ebb17..04ebc40e59d 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfig.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfig.java @@ -51,12 +51,9 @@ private List parseRules(Map map) { if (ruleObj instanceof Map) { rules.add(Rule.from((Map) ruleObj)); } else { - // Log or throw with a detailed message - throw new StableConfigMappingException( - "Rule must be a map, but got: " - + (ruleObj == null ? "null" : ruleObj.getClass().getSimpleName()) - + ", value: " - + StableConfigMappingException.safeToString(ruleObj)); + StableConfigMappingException.throwStableConfigMappingException( + "Rule must be a map, but got: " + ruleObj.getClass().getSimpleName(), ruleObj); + return emptyList(); } } return unmodifiableList(rules); diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfigMappingException.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfigMappingException.java index 9320e0901b2..f5a2d6ab679 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfigMappingException.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfigMappingException.java @@ -1,13 +1,13 @@ package datadog.trace.bootstrap.config.provider.stableconfig; public class StableConfigMappingException extends RuntimeException { - private static final int maxLen = 10; + private static final int maxLen = 100; public StableConfigMappingException(String message) { super(message); } - public static String safeToString(Object value) { + static String safeToString(Object value) { if (value == null) return "null"; String str = value.toString(); if (str.length() > maxLen) { @@ -15,4 +15,8 @@ public static String safeToString(Object value) { } return str; } + + public static void throwStableConfigMappingException(String message, Object value) { + throw new StableConfigMappingException(message + " " + safeToString(value)); + } } From 957d9c936742f2d1a08bc1d620236f3f8d2b9c89 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Mon, 11 Aug 2025 14:07:03 -0400 Subject: [PATCH 16/28] refactor exception catching in StableConfigSource --- .../config/provider/StableConfigSource.java | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigSource.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigSource.java index 02ae1b71ef3..93d512cef94 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigSource.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigSource.java @@ -38,21 +38,24 @@ public final class StableConfigSource extends ConfigProvider.Source { try { log.debug("Stable configuration file found at path: {}", file); cfg = StableConfigParser.parse(filePath); - } catch (StableConfigMappingException - | IllegalArgumentException - | ClassCastException - | NullPointerException e) { - log.warn("YAML mapping error in stable configuration file {}: {}", filePath, e.getMessage()); - cfg = StableConfig.EMPTY; - } catch (Exception e) { - if (log.isDebugEnabled()) { + } catch (Throwable 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()); + cfg = StableConfig.EMPTY; + } else if (log.isDebugEnabled()) { log.error("Unexpected error while reading stable configuration file {}: {}", filePath, e); + cfg = StableConfig.EMPTY; } else { log.error( "Unexpected error while reading stable configuration file {}: {}", filePath, e.getMessage()); } + cfg = StableConfig.EMPTY; } this.config = cfg; From b9870bd0c5d1cf98ec5f38fcf0c0fca15b0c34a2 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Mon, 11 Aug 2025 15:43:34 -0400 Subject: [PATCH 17/28] restore semicolon in calls to throwStableConfigMappingException --- .../trace/bootstrap/config/provider/stableconfig/Rule.java | 2 +- .../bootstrap/config/provider/stableconfig/Selector.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java index eaee26e82ec..e5c795590fb 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java @@ -59,7 +59,7 @@ public static Rule from(Map map) { Object configObj = map.get("configuration"); if (configObj == null) { StableConfigMappingException.throwStableConfigMappingException( - "Missing 'configuration' in rule", map); + "Missing 'configuration' in rule:", map); } if (!(configObj instanceof Map)) { StableConfigMappingException.throwStableConfigMappingException( diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Selector.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Selector.java index 5e58235b226..bfee429fa01 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Selector.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Selector.java @@ -21,7 +21,7 @@ public static Selector from(Map map) { Object originObj = map.get("origin"); if (originObj == null) { StableConfigMappingException.throwStableConfigMappingException( - "Missing 'origin' in selector", map); + "Missing 'origin' in selector:", map); } if (!(originObj instanceof String)) { StableConfigMappingException.throwStableConfigMappingException( @@ -45,7 +45,7 @@ public static Selector from(Map map) { Object operatorObj = map.get("operator"); if (operatorObj == null) { StableConfigMappingException.throwStableConfigMappingException( - "Missing 'operator' in selector", map); + "Missing 'operator' in selector:", map); } if (!(operatorObj instanceof String)) { StableConfigMappingException.throwStableConfigMappingException( From 5019584025ddb7d99a465f2c2182403a3cc61f9f Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Mon, 11 Aug 2025 15:52:00 -0400 Subject: [PATCH 18/28] Use static import for throwStableConfigMappingException --- .../config/provider/stableconfig/Rule.java | 13 ++++++------- .../config/provider/stableconfig/Selector.java | 14 +++++++------- .../config/provider/stableconfig/StableConfig.java | 3 ++- .../stableconfig/StableConfigMappingException.java | 6 +++--- 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java index e5c795590fb..8b0de61e483 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java +++ b/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; @@ -28,12 +29,11 @@ public Rule(List selectors, Map configuration) { public static Rule from(Map map) { Object selectorsObj = map.get("selectors"); if (selectorsObj == null) { - StableConfigMappingException.throwStableConfigMappingException( - "Missing 'selectors' in rule", map); + throwStableConfigMappingException("Missing 'selectors' in rule", map); } if (!(selectorsObj instanceof List)) { - StableConfigMappingException.throwStableConfigMappingException( + throwStableConfigMappingException( "'selectors' must be a list, but got: " + selectorsObj.getClass().getSimpleName(), selectorsObj); } @@ -48,7 +48,7 @@ public static Rule from(Map map) { if (s instanceof Map) { return Selector.from((Map) s); } - StableConfigMappingException.throwStableConfigMappingException( + throwStableConfigMappingException( "Each selector must be a map, but got: " + s.getClass().getSimpleName(), s); @@ -58,11 +58,10 @@ public static Rule from(Map map) { Object configObj = map.get("configuration"); if (configObj == null) { - StableConfigMappingException.throwStableConfigMappingException( - "Missing 'configuration' in rule:", map); + throwStableConfigMappingException("Missing 'configuration' in rule:", map); } if (!(configObj instanceof Map)) { - StableConfigMappingException.throwStableConfigMappingException( + throwStableConfigMappingException( "'configuration' must be a map, but got: " + configObj.getClass().getSimpleName(), configObj); } diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Selector.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Selector.java index bfee429fa01..be83dc404fd 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Selector.java +++ b/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; @@ -20,11 +22,10 @@ public Selector(String origin, String key, List matches, String operator public static Selector from(Map map) { Object originObj = map.get("origin"); if (originObj == null) { - StableConfigMappingException.throwStableConfigMappingException( - "Missing 'origin' in selector:", map); + throwStableConfigMappingException("Missing 'origin' in selector:", map); } if (!(originObj instanceof String)) { - StableConfigMappingException.throwStableConfigMappingException( + throwStableConfigMappingException( "'origin' must be a string, but got: " + originObj.getClass().getSimpleName(), originObj); } String origin = (String) originObj; @@ -34,7 +35,7 @@ public static Selector from(Map map) { Object matchesObj = map.get("matches"); if (matchesObj != null && !(matchesObj instanceof List)) { - StableConfigMappingException.throwStableConfigMappingException( + throwStableConfigMappingException( "'matches' must be a list, but got: " + matchesObj.getClass().getSimpleName(), matchesObj); } @@ -44,11 +45,10 @@ public static Selector from(Map map) { Object operatorObj = map.get("operator"); if (operatorObj == null) { - StableConfigMappingException.throwStableConfigMappingException( - "Missing 'operator' in selector:", map); + throwStableConfigMappingException("Missing 'operator' in selector:", map); } if (!(operatorObj instanceof String)) { - StableConfigMappingException.throwStableConfigMappingException( + throwStableConfigMappingException( "'operator' must be a string, but got: " + operatorObj.getClass().getSimpleName(), operatorObj); } diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfig.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfig.java index 04ebc40e59d..55e7b3acfdc 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfig.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfig.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.emptyList; import static java.util.Collections.emptyMap; import static java.util.Collections.unmodifiableList; @@ -51,7 +52,7 @@ private List parseRules(Map map) { if (ruleObj instanceof Map) { rules.add(Rule.from((Map) ruleObj)); } else { - StableConfigMappingException.throwStableConfigMappingException( + throwStableConfigMappingException( "Rule must be a map, but got: " + ruleObj.getClass().getSimpleName(), ruleObj); return emptyList(); } diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfigMappingException.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfigMappingException.java index f5a2d6ab679..2901e6d7926 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfigMappingException.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfigMappingException.java @@ -1,7 +1,7 @@ package datadog.trace.bootstrap.config.provider.stableconfig; public class StableConfigMappingException extends RuntimeException { - private static final int maxLen = 100; + private static final int MAX_LEN = 100; public StableConfigMappingException(String message) { super(message); @@ -10,8 +10,8 @@ public StableConfigMappingException(String message) { static String safeToString(Object value) { if (value == null) return "null"; String str = value.toString(); - if (str.length() > maxLen) { - return str.substring(0, maxLen) + "...(truncated)"; + if (str.length() > MAX_LEN) { + return str.substring(0, MAX_LEN) + "...(truncated)"; } return str; } From e8d6e8f500e1f28ab09d60ed450129d3477e721d Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Mon, 11 Aug 2025 16:22:59 -0400 Subject: [PATCH 19/28] test other kinds of exceptions in StableConfigSource --- .../provider/StableConfigSourceTest.groovy | 144 ++++++++++-------- 1 file changed, 77 insertions(+), 67 deletions(-) diff --git a/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigSourceTest.groovy b/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigSourceTest.groovy index 16b1d268044..4348d73eb07 100644 --- a/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigSourceTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigSourceTest.groovy @@ -135,79 +135,89 @@ class StableConfigSourceTest extends DDSpecification { where: yaml | expectedLogSubstring + // '''apm_configuration_rules: + // - selectors: + // - key: "someKey" + // matches: ["someValue"] + // operator: equals + // configuration: + // DD_SERVICE: "test" + // ''' | "Missing 'origin' in selector" + // '''apm_configuration_rules: + // - selectors: + // - origin: process_arguments + // key: "-Dfoo" + // matches: ["bar"] + // operator: equals + // ''' | "Missing 'configuration' in rule" + // '''apm_configuration_rules: + // - configuration: + // DD_SERVICE: "test" + // ''' | "Missing 'selectors' in rule" + // '''apm_configuration_rules: + // - selectors: "not-a-list" + // configuration: + // DD_SERVICE: "test" + // ''' | "'selectors' must be a list, but got: String" + // '''apm_configuration_rules: + // - selectors: + // - "not-a-map" + // ''' | "Each selector must be a map, but got: String" + // '''apm_configuration_rules: + // - selectors: + // - origin: process_arguments + // key: "-Dfoo" + // matches: ["bar"] + // operator: equals + // configuration: "not-a-map" + // ''' | "'configuration' must be a map, but got: String" + // '''apm_configuration_rules: + // - selectors: + // - origin: process_arguments + // key: "-Dfoo" + // matches: ["bar"] + // operator: equals + // configuration: 12345 + // ''' | "'configuration' must be a map, but got: Integer" + // '''apm_configuration_rules: + // - "not-a-map" + // ''' | "Rule must be a map, but got: String" + // '''apm_configuration_rules: + // - selectors: + // - origin: process_arguments + // key: "-Dfoo" + // matches: "not-a-list" + // operator: equals + // configuration: + // DD_SERVICE: "test" + // ''' | "'matches' must be a list, but got: String" + // '''apm_configuration_rules: + // - selectors: + // - origin: process_arguments + // key: "-Dfoo" + // matches: ["bar"] + // operator: 12345 + // configuration: + // DD_SERVICE: "test" + // ''' | "Missing 'operator' in selector" + // '''apm_configuration_rules: + // - selectors: + // - origin: process_arguments + // key: "-Dfoo" + // matches: ["bar"] + // operator: 12345 + // configuration: + // DD_SERVICE: "test" + // ''' | "'operator' must be a string, but got: Integer" | "Exception while parsing stable config" '''apm_configuration_rules: - selectors: - - key: "someKey" - matches: ["someValue"] - operator: equals - configuration: - DD_SERVICE: "test" - ''' | "Missing 'origin' in selector" - '''apm_configuration_rules: - - selectors: - - origin: process_arguments - key: "-Dfoo" + # origin is missing entirely, should trigger NullPointerException + - key: "-Dfoo" matches: ["bar"] operator: equals - ''' | "Missing 'configuration' in rule" - '''apm_configuration_rules: - - configuration: - DD_SERVICE: "test" - ''' | "Missing 'selectors' in rule" - '''apm_configuration_rules: - - selectors: "not-a-list" configuration: DD_SERVICE: "test" - ''' | "'selectors' must be a list, but got: String" - '''apm_configuration_rules: - - selectors: - - "not-a-map" - ''' | "Each selector must be a map, but got: String" - '''apm_configuration_rules: - - selectors: - - origin: process_arguments - key: "-Dfoo" - matches: ["bar"] - operator: equals - configuration: "not-a-map" - ''' | "'configuration' must be a map, but got: String" - '''apm_configuration_rules: - - selectors: - - origin: process_arguments - key: "-Dfoo" - matches: ["bar"] - operator: equals - configuration: 12345 - ''' | "'configuration' must be a map, but got: Integer" - '''apm_configuration_rules: - - "not-a-map" - ''' | "Rule must be a map, but got: String" - '''apm_configuration_rules: - - selectors: - - origin: process_arguments - key: "-Dfoo" - matches: "not-a-list" - operator: equals - configuration: - DD_SERVICE: "test" - ''' | "'matches' must be a list, but got: String" - '''apm_configuration_rules: - - selectors: - - origin: process_arguments - key: "-Dfoo" - matches: ["bar"] - configuration: - DD_SERVICE: "test" - ''' | "Missing 'operator' in selector" - '''apm_configuration_rules: - - selectors: - - origin: process_arguments - key: "-Dfoo" - matches: ["bar"] - operator: 12345 - configuration: - DD_SERVICE: "test" - ''' | "'operator' must be a string, but got: Integer" + ''' | "YAML mapping error in stable configuration file" } // Corrupt YAML string variable used for testing, defined outside the 'where' block for readability From 1fefb17be212501afbde7268c155118f783e4c4a Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Mon, 11 Aug 2025 16:31:32 -0400 Subject: [PATCH 20/28] remove duplicate cfg assignments --- .../trace/bootstrap/config/provider/StableConfigSource.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigSource.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigSource.java index 93d512cef94..c349c79588b 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigSource.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigSource.java @@ -45,10 +45,8 @@ public final class StableConfigSource extends ConfigProvider.Source { || e instanceof NullPointerException) { log.warn( "YAML mapping error in stable configuration file {}: {}", filePath, e.getMessage()); - cfg = StableConfig.EMPTY; } else if (log.isDebugEnabled()) { log.error("Unexpected error while reading stable configuration file {}: {}", filePath, e); - cfg = StableConfig.EMPTY; } else { log.error( "Unexpected error while reading stable configuration file {}: {}", From 058cbdac0afe13ab330d2055dabc301e043dc813 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Tue, 12 Aug 2025 11:07:33 -0400 Subject: [PATCH 21/28] fix missing operator test case --- .../provider/StableConfigSourceTest.groovy | 149 +++++++++--------- 1 file changed, 73 insertions(+), 76 deletions(-) diff --git a/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigSourceTest.groovy b/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigSourceTest.groovy index 4348d73eb07..4114656c31d 100644 --- a/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigSourceTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigSourceTest.groovy @@ -135,88 +135,85 @@ class StableConfigSourceTest extends DDSpecification { where: yaml | expectedLogSubstring - // '''apm_configuration_rules: - // - selectors: - // - key: "someKey" - // matches: ["someValue"] - // operator: equals - // configuration: - // DD_SERVICE: "test" - // ''' | "Missing 'origin' in selector" - // '''apm_configuration_rules: - // - selectors: - // - origin: process_arguments - // key: "-Dfoo" - // matches: ["bar"] - // operator: equals - // ''' | "Missing 'configuration' in rule" - // '''apm_configuration_rules: - // - configuration: - // DD_SERVICE: "test" - // ''' | "Missing 'selectors' in rule" - // '''apm_configuration_rules: - // - selectors: "not-a-list" - // configuration: - // DD_SERVICE: "test" - // ''' | "'selectors' must be a list, but got: String" - // '''apm_configuration_rules: - // - selectors: - // - "not-a-map" - // ''' | "Each selector must be a map, but got: String" - // '''apm_configuration_rules: - // - selectors: - // - origin: process_arguments - // key: "-Dfoo" - // matches: ["bar"] - // operator: equals - // configuration: "not-a-map" - // ''' | "'configuration' must be a map, but got: String" - // '''apm_configuration_rules: - // - selectors: - // - origin: process_arguments - // key: "-Dfoo" - // matches: ["bar"] - // operator: equals - // configuration: 12345 - // ''' | "'configuration' must be a map, but got: Integer" - // '''apm_configuration_rules: - // - "not-a-map" - // ''' | "Rule must be a map, but got: String" - // '''apm_configuration_rules: - // - selectors: - // - origin: process_arguments - // key: "-Dfoo" - // matches: "not-a-list" - // operator: equals - // configuration: - // DD_SERVICE: "test" - // ''' | "'matches' must be a list, but got: String" - // '''apm_configuration_rules: - // - selectors: - // - origin: process_arguments - // key: "-Dfoo" - // matches: ["bar"] - // operator: 12345 - // configuration: - // DD_SERVICE: "test" - // ''' | "Missing 'operator' in selector" - // '''apm_configuration_rules: - // - selectors: - // - origin: process_arguments - // key: "-Dfoo" - // matches: ["bar"] - // operator: 12345 - // configuration: - // DD_SERVICE: "test" - // ''' | "'operator' must be a string, but got: Integer" | "Exception while parsing stable config" '''apm_configuration_rules: - selectors: - # origin is missing entirely, should trigger NullPointerException - - key: "-Dfoo" + - key: "someKey" + matches: ["someValue"] + operator: equals + configuration: + DD_SERVICE: "test" + ''' | "Missing 'origin' in selector" + '''apm_configuration_rules: + - selectors: + - origin: process_arguments + key: "-Dfoo" matches: ["bar"] operator: equals + ''' | "Missing 'configuration' in rule" + '''apm_configuration_rules: + - configuration: + DD_SERVICE: "test" + ''' | "Missing 'selectors' in rule" + '''apm_configuration_rules: + - selectors: "not-a-list" configuration: DD_SERVICE: "test" + ''' | "'selectors' must be a list, but got: String" + '''apm_configuration_rules: + - selectors: + - "not-a-map" + ''' | "Each selector must be a map, but got: String" + '''apm_configuration_rules: + - selectors: + - origin: process_arguments + key: "-Dfoo" + matches: ["bar"] + operator: equals + configuration: "not-a-map" + ''' | "'configuration' must be a map, but got: String" + '''apm_configuration_rules: + - selectors: + - origin: process_arguments + key: "-Dfoo" + matches: ["bar"] + operator: equals + configuration: 12345 + ''' | "'configuration' must be a map, but got: Integer" + '''apm_configuration_rules: + - "not-a-map" + ''' | "Rule must be a map, but got: String" + '''apm_configuration_rules: + - selectors: + - origin: process_arguments + key: "-Dfoo" + matches: "not-a-list" + operator: equals + configuration: + DD_SERVICE: "test" + ''' | "'matches' must be a list, but got: String" + '''apm_configuration_rules: + - selectors: + - origin: process_arguments + key: "-Dfoo" + matches: ["bar"] + configuration: + DD_SERVICE: "test" + ''' | "Missing 'operator' in selector" + '''apm_configuration_rules: + - selectors: + - origin: process_arguments + key: "-Dfoo" + matches: ["bar"] + operator: 12345 + configuration: + DD_SERVICE: "test" + ''' | "'operator' must be a string, but got: Integer" + '''apm_configuration_rules: + - selectors: + # origin is missing entirely, should trigger NullPointerException + - key: "-Dfoo" + matches: ["bar"] + operator: equals ''' | "YAML mapping error in stable configuration file" } From 5460c8abeb6434a53f4551e330a13f3abf7a3682 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler <46911781+mtoffl01@users.noreply.github.com> Date: Tue, 12 Aug 2025 14:59:51 -0400 Subject: [PATCH 22/28] Update internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java Co-authored-by: Alexey Kuznetsov --- .../trace/bootstrap/config/provider/stableconfig/Rule.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java index 8b0de61e483..363c511fca3 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java @@ -29,7 +29,7 @@ public Rule(List selectors, Map configuration) { public static Rule from(Map map) { Object selectorsObj = map.get("selectors"); if (selectorsObj == null) { - throwStableConfigMappingException("Missing 'selectors' in rule", map); + throwStableConfigMappingException("Missing 'selectors' in rule:", map); } if (!(selectorsObj instanceof List)) { From 37da711ff70a6968687515f8bbfdc132f1e7342f Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Tue, 12 Aug 2025 15:54:27 -0400 Subject: [PATCH 23/28] Improve readability of StableConfigMappingException and its function invocations --- .../config/provider/stableconfig/Rule.java | 7 ++++--- .../config/provider/stableconfig/Selector.java | 7 ++++--- .../provider/stableconfig/StableConfig.java | 2 +- .../StableConfigMappingException.java | 15 +++++++++++++++ 4 files changed, 24 insertions(+), 7 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java index 8b0de61e483..848d96310b7 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java @@ -34,7 +34,7 @@ public static Rule from(Map map) { if (!(selectorsObj instanceof List)) { throwStableConfigMappingException( - "'selectors' must be a list, but got: " + selectorsObj.getClass().getSimpleName(), + "'selectors' must be a list, but got: " + selectorsObj.getClass().getSimpleName() + ": ", selectorsObj); } @@ -50,7 +50,8 @@ public static Rule from(Map map) { } throwStableConfigMappingException( "Each selector must be a map, but got: " - + s.getClass().getSimpleName(), + + s.getClass().getSimpleName() + + ": ", s); return null; }) @@ -62,7 +63,7 @@ public static Rule from(Map map) { } if (!(configObj instanceof Map)) { throwStableConfigMappingException( - "'configuration' must be a map, but got: " + configObj.getClass().getSimpleName(), + "'configuration' must be a map, but got: " + configObj.getClass().getSimpleName() + ": ", configObj); } Map configuration = (Map) configObj; diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Selector.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Selector.java index be83dc404fd..b06362cd0cd 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Selector.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Selector.java @@ -26,7 +26,8 @@ public static Selector from(Map map) { } if (!(originObj instanceof String)) { throwStableConfigMappingException( - "'origin' must be a string, but got: " + originObj.getClass().getSimpleName(), originObj); + "'origin' must be a string, but got: " + originObj.getClass().getSimpleName() + ": ", + originObj); } String origin = (String) originObj; @@ -36,7 +37,7 @@ public static Selector from(Map map) { Object matchesObj = map.get("matches"); if (matchesObj != null && !(matchesObj instanceof List)) { throwStableConfigMappingException( - "'matches' must be a list, but got: " + matchesObj.getClass().getSimpleName(), + "'matches' must be a list, but got: " + matchesObj.getClass().getSimpleName() + ": ", matchesObj); } List rawMatches = (List) matchesObj; @@ -49,7 +50,7 @@ public static Selector from(Map map) { } if (!(operatorObj instanceof String)) { throwStableConfigMappingException( - "'operator' must be a string, but got: " + operatorObj.getClass().getSimpleName(), + "'operator' must be a string, but got: " + operatorObj.getClass().getSimpleName() + ": ", operatorObj); } String operator = (String) operatorObj; diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfig.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfig.java index 55e7b3acfdc..f5b09d0d579 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfig.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfig.java @@ -53,7 +53,7 @@ private List parseRules(Map map) { rules.add(Rule.from((Map) ruleObj)); } else { throwStableConfigMappingException( - "Rule must be a map, but got: " + ruleObj.getClass().getSimpleName(), ruleObj); + "Rule must be a map, but got: " + ruleObj.getClass().getSimpleName() + ": ", ruleObj); return emptyList(); } } diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfigMappingException.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfigMappingException.java index 2901e6d7926..817573a76b9 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfigMappingException.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfigMappingException.java @@ -7,6 +7,13 @@ public StableConfigMappingException(String message) { super(message); } + /** + * Safely converts an object to a string for error reporting, truncating the result if it exceeds + * a maximum length. + * + * @param value the object to convert to a string + * @return a string representation of the object, truncated if necessary + */ static String safeToString(Object value) { if (value == null) return "null"; String str = value.toString(); @@ -16,6 +23,14 @@ static String safeToString(Object value) { return str; } + /** + * Throws a StableConfigMappingException with a message that includes a safe string representation + * of the provided value. + * + * @param message the error message to include + * @param value the value to include in the error message, safely stringified + * @throws StableConfigMappingException always thrown by this method + */ public static void throwStableConfigMappingException(String message, Object value) { throw new StableConfigMappingException(message + " " + safeToString(value)); } From 5dac7f878411f674de0d25c408396cb4d0e7b4fa Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Tue, 12 Aug 2025 15:59:46 -0400 Subject: [PATCH 24/28] modify safeToString logic to print first 50 and last 50 chars --- .../provider/stableconfig/StableConfigMappingException.java | 5 ++++- .../config/provider/StableConfigMappingExceptionTest.groovy | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfigMappingException.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfigMappingException.java index 817573a76b9..5b0f10a3cda 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfigMappingException.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfigMappingException.java @@ -18,7 +18,10 @@ 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)"; + int partLen = MAX_LEN / 2; + return str.substring(0, partLen) + + "...(truncated)..." + + str.substring(str.length() - partLen); } return str; } diff --git a/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigMappingExceptionTest.groovy b/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigMappingExceptionTest.groovy index 6096099ea1c..2a1af178562 100644 --- a/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigMappingExceptionTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigMappingExceptionTest.groovy @@ -30,6 +30,6 @@ class StableConfigMappingExceptionTest extends Specification { given: def longStr = "a" * 101 expect: - StableConfigMappingException.safeToString(longStr) == ("a" * 100) + "...(truncated)" + StableConfigMappingException.safeToString(longStr) == ("a" * 50) + "...(truncated)..." + ("a" * 51).substring(1) } } From 13bef8db9543bb928503642e058a299c291cd234 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Tue, 12 Aug 2025 16:48:40 -0400 Subject: [PATCH 25/28] remove extra placeholder in string message when printing exception --- .../trace/bootstrap/config/provider/StableConfigSource.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigSource.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigSource.java index c349c79588b..c3455d1dc51 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigSource.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigSource.java @@ -46,7 +46,7 @@ public final class StableConfigSource extends ConfigProvider.Source { 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); + log.error("Unexpected error while reading stable configuration file {}: ", filePath, e); } else { log.error( "Unexpected error while reading stable configuration file {}: {}", From 2706bcab10ca91eca403e7f468b2251843688e81 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler <46911781+mtoffl01@users.noreply.github.com> Date: Tue, 12 Aug 2025 16:58:51 -0400 Subject: [PATCH 26/28] Update internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigSource.java Co-authored-by: Alexey Kuznetsov --- .../trace/bootstrap/config/provider/StableConfigSource.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigSource.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigSource.java index c3455d1dc51..c7e9b7c0a44 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigSource.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigSource.java @@ -44,12 +44,12 @@ public final class StableConfigSource extends ConfigProvider.Source { || e instanceof ClassCastException || e instanceof NullPointerException) { log.warn( - "YAML mapping error in stable configuration file {}: {}", filePath, e.getMessage()); + "YAML mapping error in stable configuration file: {}, error: {}", filePath, e.getMessage()); } else if (log.isDebugEnabled()) { - log.error("Unexpected error while reading stable configuration file {}: ", filePath, e); + log.error("Unexpected error while reading stable configuration file: {}", filePath, e); } else { log.error( - "Unexpected error while reading stable configuration file {}: {}", + "Unexpected error while reading stable configuration file: {}, error: {}", filePath, e.getMessage()); } From 4dde2169c2391192fac727c941bb88d7004faa47 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Thu, 14 Aug 2025 16:56:40 -0400 Subject: [PATCH 27/28] refactor Rules.from to perform type assertions before heavyweight list manipulation --- .../config/provider/stableconfig/Rule.java | 40 +++++++++---------- .../provider/StableConfigSourceTest.groovy | 8 ++-- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java index b811a9af0de..673c7f8c2a2 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java @@ -38,25 +38,6 @@ public static Rule from(Map map) { selectorsObj); } - List selectors = - unmodifiableList( - ((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); @@ -66,9 +47,26 @@ public static Rule from(Map map) { "'configuration' must be a map, but got: " + configObj.getClass().getSimpleName() + ": ", configObj); } - Map configuration = (Map) configObj; - return new Rule(selectors, configuration); + List selectors = + ((List) selectorsObj) + .stream() + .filter(Objects::nonNull) + .map( + s -> { + if (!(s instanceof Map)) { + throwStableConfigMappingException( + "Each selector must be a map, but got: " + + s.getClass().getSimpleName() + + ": ", + s); + } + + return Selector.from((Map) s); + }) + .collect(toList()); + + return new Rule(unmodifiableList(selectors), (Map) configObj); } public List getSelectors() { diff --git a/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigSourceTest.groovy b/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigSourceTest.groovy index 4114656c31d..4dee86f5ae7 100644 --- a/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigSourceTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigSourceTest.groovy @@ -160,9 +160,11 @@ class StableConfigSourceTest extends DDSpecification { DD_SERVICE: "test" ''' | "'selectors' must be a list, but got: String" '''apm_configuration_rules: - - selectors: - - "not-a-map" - ''' | "Each selector must be a map, but got: String" + - selectors: + - "not-a-map" + configuration: + DD_SERVICE: "test" + ''' | "Each selector must be a map, but got: String" '''apm_configuration_rules: - selectors: - origin: process_arguments From 16d5f3683675c1eb92d64774c0ac2c240295b91d Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Fri, 15 Aug 2025 09:32:31 -0400 Subject: [PATCH 28/28] run spotless --- .../trace/bootstrap/config/provider/StableConfigSource.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigSource.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigSource.java index c7e9b7c0a44..917fbec2bda 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigSource.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigSource.java @@ -44,7 +44,9 @@ public final class StableConfigSource extends ConfigProvider.Source { || e instanceof ClassCastException || e instanceof NullPointerException) { log.warn( - "YAML mapping error in stable configuration file: {}, error: {}", filePath, e.getMessage()); + "YAML mapping error in stable configuration file: {}, error: {}", + filePath, + e.getMessage()); } else if (log.isDebugEnabled()) { log.error("Unexpected error while reading stable configuration file: {}", filePath, e); } else {