From 9a3ba871874a7430ce84d8a8bd6166490c7b86ec Mon Sep 17 00:00:00 2001 From: Yury Gribkov Date: Thu, 8 May 2025 18:18:50 -0700 Subject: [PATCH 1/6] Migrate from snakeyaml to snakeyaml-engine --- components/cli/build.gradle.kts | 8 ----- components/context/build.gradle.kts | 8 ----- components/yaml/build.gradle.kts | 7 +---- .../main/java/datadog/yaml/YamlParser.java | 15 ++++------ dd-java-agent/build.gradle | 4 +-- dd-java-agent/instrumentation/build.gradle | 2 +- dd-java-agent/testing/build.gradle | 2 +- gradle/dependencies.gradle | 2 +- internal-api/build.gradle | 2 +- .../config/provider/StableConfigParser.java | 23 +++++++-------- .../stableconfigyaml/ConfigurationMap.java | 23 --------------- .../provider/stableconfigyaml/Rule.java | 29 ++++++++++--------- .../provider/stableconfigyaml/Selector.java | 18 ++++++------ .../stableconfigyaml/StableConfigYaml.java | 26 +++++++++++------ .../provider/StableConfigParserTest.groovy | 16 +++++----- .../provider/StableConfigSourceTest.groovy | 14 ++++----- 16 files changed, 79 insertions(+), 120 deletions(-) delete mode 100644 internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfigyaml/ConfigurationMap.java diff --git a/components/cli/build.gradle.kts b/components/cli/build.gradle.kts index 4dca7fc3036..b8314ee7f33 100644 --- a/components/cli/build.gradle.kts +++ b/components/cli/build.gradle.kts @@ -1,9 +1 @@ -plugins { - id("me.champeau.jmh") -} - apply(from = "$rootDir/gradle/java.gradle") - -jmh { - version = "1.28" -} diff --git a/components/context/build.gradle.kts b/components/context/build.gradle.kts index c737d9fefdd..cda16b4cb17 100644 --- a/components/context/build.gradle.kts +++ b/components/context/build.gradle.kts @@ -1,13 +1,5 @@ -plugins { - id("me.champeau.jmh") -} - apply(from = "$rootDir/gradle/java.gradle") -jmh { - version = "1.28" -} - val excludedClassesInstructionCoverage by extra { listOf("datadog.context.ContextProviders") // covered by forked test } diff --git a/components/yaml/build.gradle.kts b/components/yaml/build.gradle.kts index 5acc8dd4cc2..dbf51963008 100644 --- a/components/yaml/build.gradle.kts +++ b/components/yaml/build.gradle.kts @@ -4,11 +4,6 @@ plugins { apply(from = "$rootDir/gradle/java.gradle") -jmh { - version = "1.28" -} - -// https://repo1.maven.org/maven2/org/yaml/snakeyaml/2.4/snakeyaml-2.4.pom dependencies { - implementation("org.yaml", "snakeyaml", "2.4") + implementation("org.snakeyaml", "snakeyaml-engine", "2.9") } diff --git a/components/yaml/src/main/java/datadog/yaml/YamlParser.java b/components/yaml/src/main/java/datadog/yaml/YamlParser.java index 7af6d22303b..b9d714c6fbf 100644 --- a/components/yaml/src/main/java/datadog/yaml/YamlParser.java +++ b/components/yaml/src/main/java/datadog/yaml/YamlParser.java @@ -1,15 +1,12 @@ package datadog.yaml; -import org.yaml.snakeyaml.Yaml; +import org.snakeyaml.engine.v2.api.Load; +import org.snakeyaml.engine.v2.api.LoadSettings; public class YamlParser { - // Supports clazz == null for default yaml parsing - public static T parse(String content, Class clazz) { - Yaml yaml = new Yaml(); - if (clazz == null) { - return yaml.load(content); - } else { - return yaml.loadAs(content, clazz); - } + public static Object parse(String content) { + LoadSettings settings = LoadSettings.builder().setAllowDuplicateKeys(true).build(); + Load yaml = new Load(settings); + return yaml.loadFromString(content); } } diff --git a/dd-java-agent/build.gradle b/dd-java-agent/build.gradle index 1b2c3804acb..4413097cc79 100644 --- a/dd-java-agent/build.gradle +++ b/dd-java-agent/build.gradle @@ -38,7 +38,7 @@ ext.generalShadowJarConfig = { // used to report our own dependencies, but we should remove the top-level metadata // of vendored packages because those could trigger unwanted framework checks. exclude '/META-INF/maven/org.slf4j/**' - exclude '/META-INF/maven/org.yaml/**' + exclude '/META-INF/maven/org.snakeyaml/**' exclude '**/META-INF/maven/**/pom.xml' exclude '**/META-INF/proguard/' exclude '**/META-INF/*.kotlin_module' @@ -66,7 +66,7 @@ ext.generalShadowJarConfig = { // Prevents conflict with other instances, but doesn't relocate instrumentation if (!projectName.equals('instrumentation')) { - relocate 'org.yaml.snakeyaml', 'datadog.snakeyaml' + relocate 'org.snakeyaml.engine', 'datadog.snakeyaml.engine' relocate 'okhttp3', 'datadog.okhttp3' relocate 'okio', 'datadog.okio' } diff --git a/dd-java-agent/instrumentation/build.gradle b/dd-java-agent/instrumentation/build.gradle index 2d19b5f5b6f..e7d93411ed7 100644 --- a/dd-java-agent/instrumentation/build.gradle +++ b/dd-java-agent/instrumentation/build.gradle @@ -60,7 +60,7 @@ subprojects { Project subProj -> jdkCompile = "main_${name}Implementation" } configurations.muzzleBootstrap { - exclude group: 'org.yaml', module : 'snakeyaml' // we vendor this in the agent jar + exclude group: 'org.snakeyaml', module : 'snakeyaml-engine' // we vendor this in the agent jar } dependencies { // Apply common dependencies for instrumentation. diff --git a/dd-java-agent/testing/build.gradle b/dd-java-agent/testing/build.gradle index 00394d6f480..df2430f9433 100644 --- a/dd-java-agent/testing/build.gradle +++ b/dd-java-agent/testing/build.gradle @@ -39,7 +39,7 @@ excludedClassesCoverage += [ ] configurations.api { - exclude group: 'org.yaml', module: 'snakeyaml' // we vendor this in the agent jar + exclude group: 'org.snakeyaml', module: 'snakeyaml-engine' // we vendor this in the agent jar } dependencies { diff --git a/gradle/dependencies.gradle b/gradle/dependencies.gradle index 4fbdac06696..d407da73747 100644 --- a/gradle/dependencies.gradle +++ b/gradle/dependencies.gradle @@ -52,7 +52,7 @@ final class CachedData { exclude(dependency('cafe.cryptography::')) // snakeyaml and its transitives - exclude(dependency('org.yaml:snakeyaml')) + exclude(dependency('org.snakeyaml:snakeyaml')) } ] } diff --git a/internal-api/build.gradle b/internal-api/build.gradle index d137456f9fe..4c82b076d70 100644 --- a/internal-api/build.gradle +++ b/internal-api/build.gradle @@ -251,7 +251,7 @@ dependencies { // it contains annotations that are also present in the instrumented application classes api "com.datadoghq:dd-javac-plugin-client:0.2.2" - testImplementation("org.yaml:snakeyaml:2.4") + testImplementation("org.snakeyaml:snakeyaml-engine:2.9") testImplementation project(":utils:test-utils") testImplementation("org.assertj:assertj-core:3.20.2") testImplementation libs.bundles.junit5 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 3721528f81f..fc4b8987c26 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 @@ -1,6 +1,5 @@ package datadog.trace.bootstrap.config.provider; -import datadog.trace.bootstrap.config.provider.stableconfigyaml.ConfigurationMap; import datadog.trace.bootstrap.config.provider.stableconfigyaml.Rule; import datadog.trace.bootstrap.config.provider.stableconfigyaml.Selector; import datadog.trace.bootstrap.config.provider.stableconfigyaml.StableConfigYaml; @@ -10,8 +9,9 @@ import java.nio.file.Files; import java.nio.file.Paths; import java.util.Collections; -import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; import java.util.function.BiPredicate; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -41,10 +41,11 @@ public static StableConfigSource.StableConfig parse(String filePath) throws IOEx try { String content = new String(Files.readAllBytes(Paths.get(filePath)), StandardCharsets.UTF_8); String processedContent = processTemplate(content); - StableConfigYaml data = YamlParser.parse(processedContent, StableConfigYaml.class); + Object parsedYaml = YamlParser.parse(processedContent); + StableConfigYaml data = new StableConfigYaml(parsedYaml); String configId = data.getConfig_id(); - ConfigurationMap configMap = data.getApm_configuration_default(); + Map configMap = data.getApm_configuration_default(); List rules = data.getApm_configuration_rules(); if (!rules.isEmpty()) { @@ -53,14 +54,16 @@ public static StableConfigSource.StableConfig parse(String filePath) throws IOEx if (doesRuleMatch(rule)) { // Merge configs found in apm_configuration_rules with those found in // apm_configuration_default - configMap.putAll(rule.getConfiguration()); - return createStableConfig(configId, configMap); + Map mergedConfigMap = new LinkedHashMap<>(configMap); + mergedConfigMap.putAll(rule.getConfiguration()); + return new StableConfigSource.StableConfig(configId, mergedConfigMap); } } } // If configs were found in apm_configuration_default, use them if (!configMap.isEmpty()) { - return createStableConfig(configId, configMap); + return new StableConfigSource.StableConfig( + configId, new LinkedHashMap(configMap)); } // If there's a configId but no configMap, use configId but return an empty map @@ -91,12 +94,6 @@ private static boolean doesRuleMatch(Rule rule) { return true; // Return true if all selectors match } - /** Creates a StableConfig object from the provided configId and configMap. */ - private static StableConfigSource.StableConfig createStableConfig( - String configId, ConfigurationMap configMap) { - return new StableConfigSource.StableConfig(configId, new HashMap<>(configMap)); - } - private static boolean validOperatorForLanguageOrigin(String operator) { operator = operator.toLowerCase(); // "exists" is not valid diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfigyaml/ConfigurationMap.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfigyaml/ConfigurationMap.java deleted file mode 100644 index 20940733fb2..00000000000 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfigyaml/ConfigurationMap.java +++ /dev/null @@ -1,23 +0,0 @@ -package datadog.trace.bootstrap.config.provider.stableconfigyaml; - -import java.util.HashMap; -import java.util.Map; - -// ConfigurationMap represents configuration key-values found in stable configuration files -public class ConfigurationMap extends HashMap { - public ConfigurationMap() { - super(); - } - - public ConfigurationMap(Map map) { - super(map); - } -} - -class ConfigurationValue { - private final String value; - - public ConfigurationValue(String value) { - this.value = value; - } -} diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfigyaml/Rule.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfigyaml/Rule.java index dbcca5792e3..0c4be55c282 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfigyaml/Rule.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfigyaml/Rule.java @@ -1,38 +1,41 @@ package datadog.trace.bootstrap.config.provider.stableconfigyaml; import java.util.ArrayList; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.stream.Collectors; // Rule represents a set of selectors and their corresponding configurations found in stable // configuration files public class Rule { private List selectors; - private ConfigurationMap configuration; + private Map configuration; public Rule() { this.selectors = new ArrayList<>(); - this.configuration = new ConfigurationMap(); + this.configuration = new LinkedHashMap<>(); } - public Rule(List selectors, ConfigurationMap configuration) { + public Rule(List selectors, Map configuration) { this.selectors = selectors; this.configuration = configuration; } - // Getters and setters - public List getSelectors() { - return selectors; + public Rule(Object yaml) { + Map map = (Map) yaml; + selectors = + ((List) map.get("selectors")) + .stream().filter(Objects::nonNull).map(Selector::new).collect(Collectors.toList()); + configuration = (Map) map.get("configuration"); } - public void setSelectors(List selectors) { - this.selectors = selectors; + public List getSelectors() { + return selectors; } - public ConfigurationMap getConfiguration() { + public Map getConfiguration() { return configuration; } - - public void setConfiguration(ConfigurationMap configuration) { - this.configuration = configuration; - } } diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfigyaml/Selector.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfigyaml/Selector.java index 05ce5cb9c6b..e9f296907d2 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfigyaml/Selector.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfigyaml/Selector.java @@ -1,7 +1,7 @@ package datadog.trace.bootstrap.config.provider.stableconfigyaml; -import java.util.ArrayList; import java.util.List; +import java.util.Map; public class Selector { private String origin; @@ -9,13 +9,6 @@ public class Selector { private List matches; private String operator; - public Selector() { - this.origin = null; - this.key = null; - this.matches = new ArrayList<>(); - this.operator = null; - } - public Selector(String origin, String key, List matches, String operator) { this.origin = origin; this.key = key; @@ -23,7 +16,14 @@ public Selector(String origin, String key, List matches, String operator this.operator = operator; } - // Getters and setters + public Selector(Object yaml) { + Map map = (Map) yaml; + origin = (String) map.get("origin"); + key = (String) map.get("key"); + matches = (List) map.get("matches"); + operator = (String) map.get("operator"); + } + public String getOrigin() { return origin; } diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfigyaml/StableConfigYaml.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfigyaml/StableConfigYaml.java index ec1a2816ed1..7ce5e1cd728 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfigyaml/StableConfigYaml.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfigyaml/StableConfigYaml.java @@ -1,20 +1,32 @@ package datadog.trace.bootstrap.config.provider.stableconfigyaml; import java.util.ArrayList; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; public class StableConfigYaml { private String config_id; // optional - private ConfigurationMap apm_configuration_default; + private Map apm_configuration_default; private List apm_configuration_rules; // optional + public StableConfigYaml(Object yaml) { + Map map = (Map) yaml; + this.config_id = String.valueOf(map.get("config_id")); + this.apm_configuration_default = + (Map) map.getOrDefault("apm_configuration_default", new LinkedHashMap<>()); + this.apm_configuration_rules = + ((List) map.getOrDefault("apm_configuration_rules", new ArrayList<>())) + .stream().map(Rule::new).collect(Collectors.toList()); + } + public StableConfigYaml() { this.config_id = null; - this.apm_configuration_default = new ConfigurationMap(); + this.apm_configuration_default = new LinkedHashMap<>(); this.apm_configuration_rules = new ArrayList<>(); } - // Getters and setters public String getConfig_id() { return config_id; } @@ -23,19 +35,15 @@ public void setConfig_id(String config_id) { this.config_id = config_id; } - public ConfigurationMap getApm_configuration_default() { + public Map getApm_configuration_default() { return apm_configuration_default; } - public void setApm_configuration_default(ConfigurationMap apm_configuration_default) { + public void setApm_configuration_default(Map apm_configuration_default) { this.apm_configuration_default = apm_configuration_default; } public List getApm_configuration_rules() { return apm_configuration_rules; } - - public void setApm_configuration_rules(List apm_configuration_rules) { - this.apm_configuration_rules = apm_configuration_rules; - } } diff --git a/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigParserTest.groovy b/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigParserTest.groovy index 686648471a1..d16475abadb 100644 --- a/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigParserTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigParserTest.groovy @@ -42,14 +42,14 @@ apm_configuration_rules: try { Files.write(filePath, yaml.getBytes()) } catch (IOException e) { - throw new AssertionError("Failed to write to file: ${e.message}") + throw new AssertionError("Failed to write to file: ${e.message}", e) } StableConfigSource.StableConfig cfg try { cfg = StableConfigParser.parse(filePath.toString()) } catch (Exception e) { - throw new AssertionError("Failed to parse the file: ${e.message}") + throw new AssertionError("Failed to parse the file: ${e.message}", e) } then: @@ -117,14 +117,14 @@ apm_configuration_rules: try { Files.write(filePath, yaml.getBytes()) } catch (IOException e) { - throw new AssertionError("Failed to write to file: ${e.message}") + throw new AssertionError("Failed to write to file: ${e.message}", e) } StableConfigSource.StableConfig cfg try { cfg = StableConfigParser.parse(filePath.toString()) } catch (Exception e) { - throw new AssertionError("Failed to parse the file: ${e.message}") + throw new AssertionError("Failed to parse the file: ${e.message}", e) } then: @@ -137,7 +137,7 @@ apm_configuration_rules: when: Path filePath = Files.createTempFile("testFile_", ".yaml") if (filePath == null) { - throw new AssertionError("Failed to create test file") + throw new AssertionError("Failed to create test file", e) } String yaml = """ config_id: 12345 @@ -145,14 +145,14 @@ apm_configuration_rules: try { Files.write(filePath, yaml.getBytes()) } catch (IOException e) { - throw new AssertionError("Failed to write to file: ${e.message}") + throw new AssertionError("Failed to write to file: ${e.message}", e) } StableConfigSource.StableConfig cfg try { cfg = StableConfigParser.parse(filePath.toString()) } catch (Exception e) { - throw new AssertionError("Failed to parse the file: ${e.message}") + throw new AssertionError("Failed to parse the file: ${e.message}", e) } then: @@ -166,7 +166,7 @@ apm_configuration_rules: when: Path filePath = Files.createTempFile("testFile_", ".yaml") if (filePath == null) { - throw new AssertionError("Failed to create test file") + throw new AssertionError("Failed to create test file", e) } String yaml = """ something-irrelevant: "" 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 f9a6281f337..c4a8b2ea307 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 @@ -3,8 +3,6 @@ package datadog.trace.bootstrap.config.provider import static java.util.Collections.singletonMap import datadog.trace.api.ConfigOrigin -import datadog.trace.bootstrap.config.provider.stableconfigyaml.ConfigurationMap -import datadog.trace.bootstrap.config.provider.stableconfigyaml.ConfigurationValue import datadog.trace.bootstrap.config.provider.stableconfigyaml.Rule import datadog.trace.bootstrap.config.provider.stableconfigyaml.Selector import datadog.trace.bootstrap.config.provider.stableconfigyaml.StableConfigYaml @@ -79,7 +77,7 @@ class StableConfigSourceTest extends DDSpecification { } StableConfigYaml stableConfigYaml = new StableConfigYaml() stableConfigYaml.setConfig_id(configId) - stableConfigYaml.setApm_configuration_default(defaultConfigs as ConfigurationMap) + stableConfigYaml.setApm_configuration_default(defaultConfigs as Map) try { writeFileYaml(filePath, stableConfigYaml) @@ -115,9 +113,9 @@ class StableConfigSourceTest extends DDSpecification { Files.delete(filePath) where: - configId | defaultConfigs | ruleConfigs - "" | new HashMap<>() | Arrays.asList(new Rule()) - "12345" | new HashMap<>() << ["DD_KEY_ONE": "one", "DD_KEY_TWO": "two"] | Arrays.asList(sampleMatchingRule, sampleNonMatchingRule) + configId | defaultConfigs | ruleConfigs + "" | [:] | Arrays.asList(new Rule()) + "12345" | ["DD_KEY_ONE": "one", "DD_KEY_TWO": "two"] | Arrays.asList(sampleMatchingRule, sampleNonMatchingRule) } // Corrupt YAML string variable used for testing, defined outside the 'where' block for readability @@ -129,8 +127,8 @@ class StableConfigSourceTest extends DDSpecification { ''' // Matching and non-matching Rules used for testing, defined outside the 'where' block for readability - def static sampleMatchingRule = new Rule(Arrays.asList(new Selector("origin", "language", Arrays.asList("Java"), null)), new ConfigurationMap(singletonMap("DD_KEY_THREE", new ConfigurationValue("three")))) - def static sampleNonMatchingRule = new Rule(Arrays.asList(new Selector("origin", "language", Arrays.asList("Golang"), null)), new ConfigurationMap(singletonMap("DD_KEY_FOUR", new ConfigurationValue("four")))) + def static sampleMatchingRule = new Rule(Arrays.asList(new Selector("origin", "language", Arrays.asList("Java"), null)), singletonMap("DD_KEY_THREE", "three")) + def static sampleNonMatchingRule = new Rule(Arrays.asList(new Selector("origin", "language", Arrays.asList("Golang"), null)), singletonMap("DD_KEY_FOUR", "four")) // Helper functions def stableConfigYamlWriter = getStableConfigYamlWriter() From 68d5f1d3cdd8cf5a04f459c8dd7d06989d00c9c3 Mon Sep 17 00:00:00 2001 From: Yury Gribkov Date: Fri, 9 May 2025 10:27:56 -0700 Subject: [PATCH 2/6] Clean up --- .../graal/nativeimage/ResourcesFeatureInstrumentation.java | 3 +-- dd-smoke-tests/spring-boot-3.0-native/application/build.gradle | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/dd-java-agent/instrumentation/graal/native-image/src/main/java/datadog/trace/instrumentation/graal/nativeimage/ResourcesFeatureInstrumentation.java b/dd-java-agent/instrumentation/graal/native-image/src/main/java/datadog/trace/instrumentation/graal/nativeimage/ResourcesFeatureInstrumentation.java index bf546d0b97f..47e5a2a62b1 100644 --- a/dd-java-agent/instrumentation/graal/native-image/src/main/java/datadog/trace/instrumentation/graal/nativeimage/ResourcesFeatureInstrumentation.java +++ b/dd-java-agent/instrumentation/graal/native-image/src/main/java/datadog/trace/instrumentation/graal/nativeimage/ResourcesFeatureInstrumentation.java @@ -50,8 +50,7 @@ public static void onExit() { tracerResources.add("profiling/jfr/overrides/minimal.jfp"); // jmxfetch configs - tracerResources.add( - "metrics/project.properties"); // org.datadog.jmxfetch.AppConfig reads its version + tracerResources.add("metrics/project.properties"); tracerResources.add("metrics/org/datadog/jmxfetch/default-jmx-metrics.yaml"); tracerResources.add("metrics/org/datadog/jmxfetch/new-gc-default-jmx-metrics.yaml"); tracerResources.add("metrics/org/datadog/jmxfetch/old-gc-default-jmx-metrics.yaml"); diff --git a/dd-smoke-tests/spring-boot-3.0-native/application/build.gradle b/dd-smoke-tests/spring-boot-3.0-native/application/build.gradle index 992fa0eaa6f..868dcf3239e 100644 --- a/dd-smoke-tests/spring-boot-3.0-native/application/build.gradle +++ b/dd-smoke-tests/spring-boot-3.0-native/application/build.gradle @@ -39,7 +39,6 @@ if (hasProperty('agentPath')) { if (withProfiler && property('profiler') == 'true') { buildArgs.add("-J-Ddd.profiling.enabled=true") } - buildArgs.add("--enable-monitoring=jmxserver") } } } From a4deaea65eebfe6a72c3063247c17e9c5388db4c Mon Sep 17 00:00:00 2001 From: Yury Gribkov Date: Fri, 9 May 2025 11:22:22 -0700 Subject: [PATCH 3/6] Clean up stableconfig stuff --- .../config/provider/StableConfigParser.java | 23 ++- .../Rule.java | 20 +-- .../Selector.java | 31 ++-- .../provider/stableconfig/StableConfig.java | 45 ++++++ .../stableconfigyaml/StableConfigYaml.java | 49 ------- .../provider/StableConfigParserTest.groovy | 132 ++++++------------ .../provider/StableConfigSourceTest.groovy | 87 ++++-------- 7 files changed, 143 insertions(+), 244 deletions(-) rename internal-api/src/main/java/datadog/trace/bootstrap/config/provider/{stableconfigyaml => stableconfig}/Rule.java (51%) rename internal-api/src/main/java/datadog/trace/bootstrap/config/provider/{stableconfigyaml => stableconfig}/Selector.java (54%) create mode 100644 internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfig.java delete mode 100644 internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfigyaml/StableConfigYaml.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 fc4b8987c26..35be63e5fcd 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 @@ -1,8 +1,8 @@ package datadog.trace.bootstrap.config.provider; -import datadog.trace.bootstrap.config.provider.stableconfigyaml.Rule; -import datadog.trace.bootstrap.config.provider.stableconfigyaml.Selector; -import datadog.trace.bootstrap.config.provider.stableconfigyaml.StableConfigYaml; +import datadog.trace.bootstrap.config.provider.stableconfig.Rule; +import datadog.trace.bootstrap.config.provider.stableconfig.Selector; +import datadog.trace.bootstrap.config.provider.stableconfig.StableConfig; import datadog.yaml.YamlParser; import java.io.IOException; import java.nio.charset.StandardCharsets; @@ -42,15 +42,14 @@ public static StableConfigSource.StableConfig parse(String filePath) throws IOEx String content = new String(Files.readAllBytes(Paths.get(filePath)), StandardCharsets.UTF_8); String processedContent = processTemplate(content); Object parsedYaml = YamlParser.parse(processedContent); - StableConfigYaml data = new StableConfigYaml(parsedYaml); + StableConfig data = new StableConfig(parsedYaml); - String configId = data.getConfig_id(); - Map configMap = data.getApm_configuration_default(); - List rules = data.getApm_configuration_rules(); + String configId = data.getConfigId(); + Map configMap = data.getApmConfigurationDefault(); + List rules = data.getApmConfigurationRules(); if (!rules.isEmpty()) { for (Rule rule : rules) { - // Use the first matching rule if (doesRuleMatch(rule)) { // Merge configs found in apm_configuration_rules with those found in // apm_configuration_default @@ -62,8 +61,7 @@ public static StableConfigSource.StableConfig parse(String filePath) throws IOEx } // If configs were found in apm_configuration_default, use them if (!configMap.isEmpty()) { - return new StableConfigSource.StableConfig( - configId, new LinkedHashMap(configMap)); + return new StableConfigSource.StableConfig(configId, new LinkedHashMap<>(configMap)); } // If there's a configId but no configMap, use configId but return an empty map @@ -72,10 +70,7 @@ public static StableConfigSource.StableConfig parse(String filePath) throws IOEx } } catch (IOException e) { - log.debug( - "Stable configuration file either not found or not readable at filepath {}. Error: {}", - filePath, - e.getMessage()); + log.debug("Failed to read the stable configuration file: {}", filePath, e); } return StableConfigSource.StableConfig.EMPTY; } diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfigyaml/Rule.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java similarity index 51% rename from internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfigyaml/Rule.java rename to internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java index 0c4be55c282..cf8432b0656 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfigyaml/Rule.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Rule.java @@ -1,6 +1,7 @@ -package datadog.trace.bootstrap.config.provider.stableconfigyaml; +package datadog.trace.bootstrap.config.provider.stableconfig; import java.util.ArrayList; +import java.util.Collections; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -9,13 +10,13 @@ // Rule represents a set of selectors and their corresponding configurations found in stable // configuration files -public class Rule { - private List selectors; - private Map configuration; +public final class Rule { + private final List selectors; + private final Map configuration; public Rule() { - this.selectors = new ArrayList<>(); - this.configuration = new LinkedHashMap<>(); + this.selectors = Collections.unmodifiableList(new ArrayList<>()); + this.configuration = Collections.unmodifiableMap(new LinkedHashMap<>()); } public Rule(List selectors, Map configuration) { @@ -26,9 +27,10 @@ public Rule(List selectors, Map configuration) { public Rule(Object yaml) { Map map = (Map) yaml; selectors = - ((List) map.get("selectors")) - .stream().filter(Objects::nonNull).map(Selector::new).collect(Collectors.toList()); - configuration = (Map) map.get("configuration"); + Collections.unmodifiableList( + ((List) map.get("selectors")) + .stream().filter(Objects::nonNull).map(Selector::new).collect(Collectors.toList())); + configuration = Collections.unmodifiableMap((Map) map.get("configuration")); } public List getSelectors() { diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfigyaml/Selector.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Selector.java similarity index 54% rename from internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfigyaml/Selector.java rename to internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Selector.java index e9f296907d2..acf396053fd 100644 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfigyaml/Selector.java +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/Selector.java @@ -1,13 +1,14 @@ -package datadog.trace.bootstrap.config.provider.stableconfigyaml; +package datadog.trace.bootstrap.config.provider.stableconfig; +import java.util.Collections; import java.util.List; import java.util.Map; -public class Selector { - private String origin; - private String key; - private List matches; - private String operator; +public final class Selector { + private final String origin; + private final String key; + private final List matches; + private final String operator; public Selector(String origin, String key, List matches, String operator) { this.origin = origin; @@ -20,7 +21,7 @@ public Selector(Object yaml) { Map map = (Map) yaml; origin = (String) map.get("origin"); key = (String) map.get("key"); - matches = (List) map.get("matches"); + matches = Collections.unmodifiableList((List) map.get("matches")); operator = (String) map.get("operator"); } @@ -28,31 +29,15 @@ public String getOrigin() { return origin; } - public void setOrigin(String origin) { - this.origin = origin; - } - public String getKey() { return key; } - public void setKey(String key) { - this.key = key; - } - public List getMatches() { return matches; } - public void setMatches(List matches) { - this.matches = matches; - } - public String getOperator() { return operator; } - - public void setOperator(String operator) { - this.operator = operator; - } } 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 new file mode 100644 index 00000000000..7950500f24b --- /dev/null +++ b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfig/StableConfig.java @@ -0,0 +1,45 @@ +package datadog.trace.bootstrap.config.provider.stableconfig; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; + +public final class StableConfig { + private final String configId; + private final Map apmConfigurationDefault; + private final List apmConfigurationRules; + + public StableConfig(Object yaml) { + Map map = (Map) yaml; + this.configId = String.valueOf(map.get("config_id")); + this.apmConfigurationDefault = + Collections.unmodifiableMap( + (Map) + map.getOrDefault("apm_configuration_default", new LinkedHashMap<>())); + this.apmConfigurationRules = + Collections.unmodifiableList( + ((List) map.getOrDefault("apm_configuration_rules", new ArrayList<>())) + .stream().map(Rule::new).collect(Collectors.toList())); + } + + public StableConfig(String configId, Map apmConfigurationDefault) { + this.configId = configId; + this.apmConfigurationDefault = apmConfigurationDefault; + this.apmConfigurationRules = new ArrayList<>(); + } + + public String getConfigId() { + return configId; + } + + public Map getApmConfigurationDefault() { + return apmConfigurationDefault; + } + + public List getApmConfigurationRules() { + return apmConfigurationRules; + } +} diff --git a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfigyaml/StableConfigYaml.java b/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfigyaml/StableConfigYaml.java deleted file mode 100644 index 7ce5e1cd728..00000000000 --- a/internal-api/src/main/java/datadog/trace/bootstrap/config/provider/stableconfigyaml/StableConfigYaml.java +++ /dev/null @@ -1,49 +0,0 @@ -package datadog.trace.bootstrap.config.provider.stableconfigyaml; - -import java.util.ArrayList; -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Map; -import java.util.stream.Collectors; - -public class StableConfigYaml { - private String config_id; // optional - private Map apm_configuration_default; - private List apm_configuration_rules; // optional - - public StableConfigYaml(Object yaml) { - Map map = (Map) yaml; - this.config_id = String.valueOf(map.get("config_id")); - this.apm_configuration_default = - (Map) map.getOrDefault("apm_configuration_default", new LinkedHashMap<>()); - this.apm_configuration_rules = - ((List) map.getOrDefault("apm_configuration_rules", new ArrayList<>())) - .stream().map(Rule::new).collect(Collectors.toList()); - } - - public StableConfigYaml() { - this.config_id = null; - this.apm_configuration_default = new LinkedHashMap<>(); - this.apm_configuration_rules = new ArrayList<>(); - } - - public String getConfig_id() { - return config_id; - } - - public void setConfig_id(String config_id) { - this.config_id = config_id; - } - - public Map getApm_configuration_default() { - return apm_configuration_default; - } - - public void setApm_configuration_default(Map apm_configuration_default) { - this.apm_configuration_default = apm_configuration_default; - } - - public List getApm_configuration_rules() { - return apm_configuration_rules; - } -} diff --git a/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigParserTest.groovy b/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigParserTest.groovy index d16475abadb..ec6c88c08cd 100644 --- a/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigParserTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigParserTest.groovy @@ -7,9 +7,6 @@ class StableConfigParserTest extends DDSpecification { def "test parse valid"() { when: Path filePath = Files.createTempFile("testFile_", ".yaml") - if (filePath == null) { - throw new AssertionError("Failed to create test file") - } injectEnvConfig("DD_SERVICE", "mysvc") // From the below yaml, only apm_configuration_default and the second selector should be applied: We use the first matching rule and discard the rest String yaml = """ @@ -39,20 +36,11 @@ apm_configuration_rules: configuration: KEY_FOUR: "ignored" """ - try { - Files.write(filePath, yaml.getBytes()) - } catch (IOException e) { - throw new AssertionError("Failed to write to file: ${e.message}", e) - } - - StableConfigSource.StableConfig cfg - try { - cfg = StableConfigParser.parse(filePath.toString()) - } catch (Exception e) { - throw new AssertionError("Failed to parse the file: ${e.message}", e) - } + Files.write(filePath, yaml.getBytes()) + StableConfigSource.StableConfig cfg = StableConfigParser.parse(filePath.toString()) then: + filePath != null def keys = cfg.getKeys() keys.size() == 3 cfg.getConfigId().trim() == ("12345") @@ -74,37 +62,34 @@ apm_configuration_rules: match == expectMatch where: - origin | matches | operator | key | expectMatch - "language" | ["java"] | "equals" | "" | true - "LANGUAGE" | ["JaVa"] | "EQUALS" | "" | true // check case insensitivity - "language" | ["java", "golang"] | "equals" | "" | true - "language" | ["java"] | "starts_with" | "" | true - "language" | ["golang"] | "equals" | "" | false - "language" | ["java"] | "exists" | "" | false - "language" | ["java"] | "something unexpected" | "" | false - "environment_variables" | [] | "exists" | "DD_TAGS" | true - "environment_variables" | ["team:apm"] | "contains" | "DD_TAGS" | true - "ENVIRONMENT_VARIABLES" | ["TeAm:ApM"] | "CoNtAiNs" | "Dd_TaGs" | true // check case insensitivity - "environment_variables" | ["team:apm"] | "equals" | "DD_TAGS" | false - "environment_variables" | ["team:apm"] | "starts_with" | "DD_TAGS" | true - "environment_variables" | ["true"] | "equals" | "DD_PROFILING_ENABLED" | true - "environment_variables" | ["abcdefg"] | "equals" | "DD_PROFILING_ENABLED" | false - "environment_variables" | ["true"] | "equals" | "DD_PROFILING_ENABLED" | true - "environment_variables" | ["mysvc", "othersvc"] | "equals" | "DD_SERVICE" | true - "environment_variables" | ["my"] | "starts_with" | "DD_SERVICE" | true - "environment_variables" | ["svc"] | "ends_with" | "DD_SERVICE" | true - "environment_variables" | ["svc"] | "contains" | "DD_SERVICE" | true - "environment_variables" | ["other"] | "contains" | "DD_SERVICE" | false - "environment_variables" | [null] | "contains" | "DD_SERVICE" | false + origin | matches | operator | key | expectMatch + "language" | ["java"] | "equals" | "" | true + "LANGUAGE" | ["JaVa"] | "EQUALS" | "" | true // check case insensitivity + "language" | ["java", "golang"] | "equals" | "" | true + "language" | ["java"] | "starts_with" | "" | true + "language" | ["golang"] | "equals" | "" | false + "language" | ["java"] | "exists" | "" | false + "language" | ["java"] | "something unexpected" | "" | false + "environment_variables" | [] | "exists" | "DD_TAGS" | true + "environment_variables" | ["team:apm"] | "contains" | "DD_TAGS" | true + "ENVIRONMENT_VARIABLES" | ["TeAm:ApM"] | "CoNtAiNs" | "Dd_TaGs" | true // check case insensitivity + "environment_variables" | ["team:apm"] | "equals" | "DD_TAGS" | false + "environment_variables" | ["team:apm"] | "starts_with" | "DD_TAGS" | true + "environment_variables" | ["true"] | "equals" | "DD_PROFILING_ENABLED" | true + "environment_variables" | ["abcdefg"] | "equals" | "DD_PROFILING_ENABLED" | false + "environment_variables" | ["true"] | "equals" | "DD_PROFILING_ENABLED" | true + "environment_variables" | ["mysvc", "othersvc"] | "equals" | "DD_SERVICE" | true + "environment_variables" | ["my"] | "starts_with" | "DD_SERVICE" | true + "environment_variables" | ["svc"] | "ends_with" | "DD_SERVICE" | true + "environment_variables" | ["svc"] | "contains" | "DD_SERVICE" | true + "environment_variables" | ["other"] | "contains" | "DD_SERVICE" | false + "environment_variables" | [null] | "contains" | "DD_SERVICE" | false } def "test duplicate entries"() { // When duplicate keys are encountered, snakeyaml preserves the last value by default when: Path filePath = Files.createTempFile("testFile_", ".yaml") - if (filePath == null) { - throw new AssertionError("Failed to create test file") - } String yaml = """ config_id: 12345 config_id: 67890 @@ -113,21 +98,11 @@ apm_configuration_rules: apm_configuration_default: DD_KEY: value_2 """ - - try { - Files.write(filePath, yaml.getBytes()) - } catch (IOException e) { - throw new AssertionError("Failed to write to file: ${e.message}", e) - } - - StableConfigSource.StableConfig cfg - try { - cfg = StableConfigParser.parse(filePath.toString()) - } catch (Exception e) { - throw new AssertionError("Failed to parse the file: ${e.message}", e) - } + Files.write(filePath, yaml.getBytes()) + StableConfigSource.StableConfig cfg = StableConfigParser.parse(filePath.toString()) then: + filePath != null cfg != null cfg.getConfigId() == "67890" cfg.get("DD_KEY") == "value_2" @@ -136,26 +111,14 @@ apm_configuration_rules: def "test config_id only"() { when: Path filePath = Files.createTempFile("testFile_", ".yaml") - if (filePath == null) { - throw new AssertionError("Failed to create test file", e) - } String yaml = """ config_id: 12345 """ - try { - Files.write(filePath, yaml.getBytes()) - } catch (IOException e) { - throw new AssertionError("Failed to write to file: ${e.message}", e) - } - - StableConfigSource.StableConfig cfg - try { - cfg = StableConfigParser.parse(filePath.toString()) - } catch (Exception e) { - throw new AssertionError("Failed to parse the file: ${e.message}", e) - } + Files.write(filePath, yaml.getBytes()) + StableConfigSource.StableConfig cfg = StableConfigParser.parse(filePath.toString()) then: + filePath != null cfg != null cfg.getConfigId() == "12345" cfg.getKeys().size() == 0 @@ -165,9 +128,6 @@ apm_configuration_rules: // If any piece of the file is invalid, the whole file is rendered invalid and an exception is thrown when: Path filePath = Files.createTempFile("testFile_", ".yaml") - if (filePath == null) { - throw new AssertionError("Failed to create test file", e) - } String yaml = """ something-irrelevant: "" config_id: 12345 @@ -185,13 +145,8 @@ apm_configuration_rules: KEY_FIVE: [a,b,c,d] something-else-irrelevant: value-irrelevant """ - try { - Files.write(filePath, yaml.getBytes()) - } catch (IOException e) { - throw new AssertionError("Failed to write to file: ${e.message}") - } - - StableConfigSource.StableConfig cfg + Files.write(filePath, yaml.getBytes()) + StableConfigSource.StableConfig cfg = null Exception exception = null try { cfg = StableConfigParser.parse(filePath.toString()) @@ -200,6 +155,7 @@ apm_configuration_rules: } then: + filePath != null exception != null cfg == null Files.delete(filePath) @@ -215,14 +171,14 @@ apm_configuration_rules: StableConfigParser.processTemplate(templateVar) == expect where: - templateVar | envKey | envVal | expect - "{{environment_variables['DD_KEY']}}" | "DD_KEY" | "value" | "value" - "{{environment_variables['DD_KEY']}}" | null | null | "UNDEFINED" - "{{}}" | null | null | "UNDEFINED" - "{}" | null | null | "{}" - "{{environment_variables['dd_key']}}" | "DD_KEY" | "value" | "value" - "{{environment_variables['DD_KEY}}" | "DD_KEY" | "value" | "UNDEFINED" - "header-{{environment_variables['DD_KEY']}}-footer" | "DD_KEY" | "value" | "header-value-footer" + templateVar | envKey | envVal | expect + "{{environment_variables['DD_KEY']}}" | "DD_KEY" | "value" | "value" + "{{environment_variables['DD_KEY']}}" | null | null | "UNDEFINED" + "{{}}" | null | null | "UNDEFINED" + "{}" | null | null | "{}" + "{{environment_variables['dd_key']}}" | "DD_KEY" | "value" | "value" + "{{environment_variables['DD_KEY}}" | "DD_KEY" | "value" | "UNDEFINED" + "header-{{environment_variables['DD_KEY']}}-footer" | "DD_KEY" | "value" | "header-value-footer" "{{environment_variables['HEADER']}}{{environment_variables['DD_KEY']}}{{environment_variables['FOOTER']}}" | "DD_KEY" | "value" | "UNDEFINEDvalueUNDEFINED" } @@ -235,8 +191,8 @@ apm_configuration_rules: e.message == expect where: - templateVar | expect - "{{environment_variables['']}}" | "Empty environment variable name in template" + templateVar | expect + "{{environment_variables['']}}" | "Empty environment variable name in template" "{{environment_variables['DD_KEY']}" | "Unterminated template in config" } } 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 c4a8b2ea307..cda61425bd3 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 @@ -3,16 +3,12 @@ package datadog.trace.bootstrap.config.provider import static java.util.Collections.singletonMap import datadog.trace.api.ConfigOrigin -import datadog.trace.bootstrap.config.provider.stableconfigyaml.Rule -import datadog.trace.bootstrap.config.provider.stableconfigyaml.Selector -import datadog.trace.bootstrap.config.provider.stableconfigyaml.StableConfigYaml +import datadog.trace.bootstrap.config.provider.stableconfig.Rule +import datadog.trace.bootstrap.config.provider.stableconfig.Selector +import datadog.trace.bootstrap.config.provider.stableconfig.StableConfig import datadog.trace.test.util.DDSpecification -import org.yaml.snakeyaml.DumperOptions -import org.yaml.snakeyaml.Yaml -import org.yaml.snakeyaml.introspector.Property -import org.yaml.snakeyaml.nodes.NodeTuple -import org.yaml.snakeyaml.nodes.Tag -import org.yaml.snakeyaml.representer.Representer +import org.snakeyaml.engine.v2.api.Dump +import org.snakeyaml.engine.v2.api.DumpSettings import java.nio.file.Files import java.nio.file.Path @@ -32,12 +28,10 @@ class StableConfigSourceTest extends DDSpecification { def "test empty file"() { when: Path filePath = Files.createTempFile("testFile_", ".yaml") - if (filePath == null) { - throw new AssertionError("Failed to create test file") - } StableConfigSource config = new StableConfigSource(filePath.toString(), ConfigOrigin.LOCAL_STABLE_CONFIG) then: + filePath != null config.getKeys().size() == 0 config.getConfigId() == null } @@ -46,19 +40,11 @@ class StableConfigSourceTest extends DDSpecification { // StableConfigSource must handle the exception thrown by StableConfigParser.parse(filePath) gracefully when: Path filePath = Files.createTempFile("testFile_", ".yaml") - if (filePath == null) { - throw new AssertionError("Failed to create test file") - } - - try { - writeFileRaw(filePath, configId, data) - } catch (IOException e) { - throw new AssertionError("Failed to write to file: ${e.message}") - } - + writeFileRaw(filePath, configId, data) StableConfigSource stableCfg = new StableConfigSource(filePath.toString(), ConfigOrigin.LOCAL_STABLE_CONFIG) then: + filePath != null stableCfg.getConfigId() == null stableCfg.getKeys().size() == 0 Files.delete(filePath) @@ -72,22 +58,12 @@ class StableConfigSourceTest extends DDSpecification { def "test file valid format"() { when: Path filePath = Files.createTempFile("testFile_", ".yaml") - if (filePath == null) { - throw new AssertionError("Failed to create test file") - } - StableConfigYaml stableConfigYaml = new StableConfigYaml() - stableConfigYaml.setConfig_id(configId) - stableConfigYaml.setApm_configuration_default(defaultConfigs as Map) - - try { - writeFileYaml(filePath, stableConfigYaml) - } catch (IOException e) { - throw new AssertionError("Failed to write to file: ${e.message}") - } - + StableConfig stableConfigYaml = new StableConfig(configId, defaultConfigs) + writeFileYaml(filePath, stableConfigYaml) StableConfigSource stableCfg = new StableConfigSource(filePath.toString(), ConfigOrigin.LOCAL_STABLE_CONFIG) then: + filePath != null for (key in defaultConfigs.keySet()) { String keyString = (String) key keyString = keyString.substring(4) // Cut `DD_` @@ -130,34 +106,23 @@ class StableConfigSourceTest extends DDSpecification { def static sampleMatchingRule = new Rule(Arrays.asList(new Selector("origin", "language", Arrays.asList("Java"), null)), singletonMap("DD_KEY_THREE", "three")) def static sampleNonMatchingRule = new Rule(Arrays.asList(new Selector("origin", "language", Arrays.asList("Golang"), null)), singletonMap("DD_KEY_FOUR", "four")) - // Helper functions - def stableConfigYamlWriter = getStableConfigYamlWriter() - - Yaml getStableConfigYamlWriter() { - DumperOptions options = new DumperOptions() - // Create the Representer, configure it to omit nulls - Representer representer = new Representer(options) { - @Override - protected NodeTuple representJavaBeanProperty(Object javaBean, Property property, Object propertyValue, Tag customTag) { - if (propertyValue == null) { - return null // Skip null values - } else { - return super.representJavaBeanProperty(javaBean, property, propertyValue, customTag) - } - } - } - // Exclude class tag from the resulting yaml string - representer.addClassTag(StableConfigYaml.class, Tag.MAP) + def writeFileYaml(Path filePath, StableConfig stableConfigs) { + Map yamlData = new HashMap<>() - // YAML instance with custom Representer - return new Yaml(representer, options) - } + if (stableConfigs.getConfigId() != null && !stableConfigs.getConfigId().isEmpty()) { + yamlData.put("config_id", stableConfigs.getConfigId()) + } + + if (stableConfigs.getApmConfigurationDefault() != null && !stableConfigs.getApmConfigurationDefault().isEmpty()) { + yamlData.put("apm_configuration_default", stableConfigs.getApmConfigurationDefault()) + } + + DumpSettings settings = DumpSettings.builder().build() + Dump dump = new Dump(settings) + String yamlContent = dump.dumpToString(yamlData) - def writeFileYaml(Path filePath, StableConfigYaml stableConfigs) { - try (FileWriter writer = new FileWriter(filePath.toString())) { - stableConfigYamlWriter.dump(stableConfigs, writer) - } catch (IOException e) { - e.printStackTrace() + try (FileWriter writer = new FileWriter(filePath.toFile())) { + writer.write(yamlContent) } } From 4ac352a1abd2607f651a8bc60718af35acc31866 Mon Sep 17 00:00:00 2001 From: Yury Gribkov Date: Mon, 12 May 2025 11:33:56 -0700 Subject: [PATCH 4/6] Properly exclude snakeyaml-engine --- gradle/dependencies.gradle | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gradle/dependencies.gradle b/gradle/dependencies.gradle index d407da73747..cb46e079d9a 100644 --- a/gradle/dependencies.gradle +++ b/gradle/dependencies.gradle @@ -51,8 +51,8 @@ final class CachedData { // cafe_crypto and its transitives exclude(dependency('cafe.cryptography::')) - // snakeyaml and its transitives - exclude(dependency('org.snakeyaml:snakeyaml')) + // snakeyaml-engine and its transitives + exclude(dependency('org.snakeyaml:snakeyaml-engine')) } ] } From 2c56ffe9dde3ac5b6398b14e227a4c8f6cec2a0e Mon Sep 17 00:00:00 2001 From: Yury Gribkov Date: Tue, 20 May 2025 11:01:27 +0200 Subject: [PATCH 5/6] Clean up --- .../main/java/datadog/yaml/YamlParser.java | 8 +++- .../config/provider/StableConfigParser.java | 3 +- .../config/provider/stableconfig/Rule.java | 23 +++++----- .../provider/stableconfig/StableConfig.java | 26 +++++------ .../provider/StableConfigParserTest.groovy | 45 +++++++++++++------ .../provider/StableConfigSourceTest.groovy | 22 +++++++-- 6 files changed, 80 insertions(+), 47 deletions(-) diff --git a/components/yaml/src/main/java/datadog/yaml/YamlParser.java b/components/yaml/src/main/java/datadog/yaml/YamlParser.java index b9d714c6fbf..1284ef0833a 100644 --- a/components/yaml/src/main/java/datadog/yaml/YamlParser.java +++ b/components/yaml/src/main/java/datadog/yaml/YamlParser.java @@ -4,8 +4,14 @@ import org.snakeyaml.engine.v2.api.LoadSettings; public class YamlParser { + /** + * Parses YAML content. Duplicate keys are not allowed and will result in a runtime exception.. + * + * @param content - text context to be parsed as YAML + * @return - a parsed representation as a composition of map and list objects. + */ public static Object parse(String content) { - LoadSettings settings = LoadSettings.builder().setAllowDuplicateKeys(true).build(); + LoadSettings settings = LoadSettings.builder().build(); Load yaml = new Load(settings); return yaml.loadFromString(content); } 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 35be63e5fcd..47f3ea53a7d 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 @@ -50,6 +50,7 @@ public static StableConfigSource.StableConfig parse(String filePath) throws IOEx if (!rules.isEmpty()) { for (Rule rule : rules) { + // Use the first matching rule if (doesRuleMatch(rule)) { // Merge configs found in apm_configuration_rules with those found in // apm_configuration_default @@ -61,7 +62,7 @@ public static StableConfigSource.StableConfig parse(String filePath) throws IOEx } // If configs were found in apm_configuration_default, use them if (!configMap.isEmpty()) { - return new StableConfigSource.StableConfig(configId, new LinkedHashMap<>(configMap)); + return new StableConfigSource.StableConfig(configId, configMap); } // If there's a configId but no configMap, use configId but return an empty map 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 cf8432b0656..2d464b41ed1 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,22 +1,23 @@ package datadog.trace.bootstrap.config.provider.stableconfig; -import java.util.ArrayList; -import java.util.Collections; -import java.util.LinkedHashMap; +import static java.util.Collections.*; +import static java.util.stream.Collectors.toList; + import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.stream.Collectors; -// Rule represents a set of selectors and their corresponding configurations found in stable -// configuration files +/** + * Rule represents a set of selectors and their corresponding configurations found in stable + * configuration files + */ public final class Rule { private final List selectors; private final Map configuration; public Rule() { - this.selectors = Collections.unmodifiableList(new ArrayList<>()); - this.configuration = Collections.unmodifiableMap(new LinkedHashMap<>()); + this.selectors = emptyList(); + this.configuration = emptyMap(); } public Rule(List selectors, Map configuration) { @@ -27,10 +28,10 @@ public Rule(List selectors, Map configuration) { public Rule(Object yaml) { Map map = (Map) yaml; selectors = - Collections.unmodifiableList( + unmodifiableList( ((List) map.get("selectors")) - .stream().filter(Objects::nonNull).map(Selector::new).collect(Collectors.toList())); - configuration = Collections.unmodifiableMap((Map) map.get("configuration")); + .stream().filter(Objects::nonNull).map(Selector::new).collect(toList())); + configuration = unmodifiableMap((Map) map.get("configuration")); } public List getSelectors() { 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 7950500f24b..c8385546cf8 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,11 +1,12 @@ package datadog.trace.bootstrap.config.provider.stableconfig; -import java.util.ArrayList; -import java.util.Collections; -import java.util.LinkedHashMap; +import static java.util.Collections.emptyList; +import static java.util.Collections.unmodifiableList; +import static java.util.Collections.unmodifiableMap; +import static java.util.stream.Collectors.toList; + import java.util.List; import java.util.Map; -import java.util.stream.Collectors; public final class StableConfig { private final String configId; @@ -16,19 +17,12 @@ public StableConfig(Object yaml) { Map map = (Map) yaml; this.configId = String.valueOf(map.get("config_id")); this.apmConfigurationDefault = - Collections.unmodifiableMap( - (Map) - map.getOrDefault("apm_configuration_default", new LinkedHashMap<>())); + unmodifiableMap( + (Map) map.getOrDefault("apm_configuration_default", emptyList())); this.apmConfigurationRules = - Collections.unmodifiableList( - ((List) map.getOrDefault("apm_configuration_rules", new ArrayList<>())) - .stream().map(Rule::new).collect(Collectors.toList())); - } - - public StableConfig(String configId, Map apmConfigurationDefault) { - this.configId = configId; - this.apmConfigurationDefault = apmConfigurationDefault; - this.apmConfigurationRules = new ArrayList<>(); + unmodifiableList( + ((List) map.getOrDefault("apm_configuration_rules", emptyList())) + .stream().map(Rule::new).collect(toList())); } public String getConfigId() { diff --git a/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigParserTest.groovy b/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigParserTest.groovy index ec6c88c08cd..4a0bbdbf39f 100644 --- a/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigParserTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigParserTest.groovy @@ -4,9 +4,16 @@ import java.nio.file.Files import java.nio.file.Path class StableConfigParserTest extends DDSpecification { + def "test parse valid"() { when: Path filePath = Files.createTempFile("testFile_", ".yaml") + then: + if (filePath == null) { + throw new AssertionError("Failed to create: " + filePath) + } + + when: injectEnvConfig("DD_SERVICE", "mysvc") // From the below yaml, only apm_configuration_default and the second selector should be applied: We use the first matching rule and discard the rest String yaml = """ @@ -40,7 +47,6 @@ apm_configuration_rules: StableConfigSource.StableConfig cfg = StableConfigParser.parse(filePath.toString()) then: - filePath != null def keys = cfg.getKeys() keys.size() == 3 cfg.getConfigId().trim() == ("12345") @@ -86,31 +92,38 @@ apm_configuration_rules: "environment_variables" | [null] | "contains" | "DD_SERVICE" | false } - def "test duplicate entries"() { - // When duplicate keys are encountered, snakeyaml preserves the last value by default + def "test duplicate entries not allowed"() { when: Path filePath = Files.createTempFile("testFile_", ".yaml") + then: + if (filePath == null) { + throw new AssertionError("Failed to create: " + filePath) + } + + when: String yaml = """ config_id: 12345 config_id: 67890 - apm_configuration_default: - DD_KEY: value_1 - apm_configuration_default: - DD_KEY: value_2 """ Files.write(filePath, yaml.getBytes()) - StableConfigSource.StableConfig cfg = StableConfigParser.parse(filePath.toString()) + StableConfigParser.parse(filePath.toString()) then: - filePath != null - cfg != null - cfg.getConfigId() == "67890" - cfg.get("DD_KEY") == "value_2" + def ex = thrown(RuntimeException) + + and: + ex.message.contains "found duplicate key config_id" } def "test config_id only"() { when: Path filePath = Files.createTempFile("testFile_", ".yaml") + then: + if (filePath == null) { + throw new AssertionError("Failed to create: " + filePath) + } + + when: String yaml = """ config_id: 12345 """ @@ -118,7 +131,6 @@ apm_configuration_rules: StableConfigSource.StableConfig cfg = StableConfigParser.parse(filePath.toString()) then: - filePath != null cfg != null cfg.getConfigId() == "12345" cfg.getKeys().size() == 0 @@ -128,6 +140,12 @@ apm_configuration_rules: // If any piece of the file is invalid, the whole file is rendered invalid and an exception is thrown when: Path filePath = Files.createTempFile("testFile_", ".yaml") + then: + if (filePath == null) { + throw new AssertionError("Failed to create: " + filePath) + } + + when: String yaml = """ something-irrelevant: "" config_id: 12345 @@ -155,7 +173,6 @@ apm_configuration_rules: } then: - filePath != null exception != null cfg == null Files.delete(filePath) 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 cda61425bd3..b3ec755ddc1 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 @@ -28,10 +28,14 @@ class StableConfigSourceTest extends DDSpecification { def "test empty file"() { when: Path filePath = Files.createTempFile("testFile_", ".yaml") - StableConfigSource config = new StableConfigSource(filePath.toString(), ConfigOrigin.LOCAL_STABLE_CONFIG) + then: + if (filePath == null) { + throw new AssertionError("Failed to create: " + filePath) + } + when: + StableConfigSource config = new StableConfigSource(filePath.toString(), ConfigOrigin.LOCAL_STABLE_CONFIG) then: - filePath != null config.getKeys().size() == 0 config.getConfigId() == null } @@ -40,11 +44,16 @@ class StableConfigSourceTest extends DDSpecification { // StableConfigSource must handle the exception thrown by StableConfigParser.parse(filePath) gracefully when: Path filePath = Files.createTempFile("testFile_", ".yaml") + then: + if (filePath == null) { + throw new AssertionError("Failed to create: " + filePath) + } + + when: writeFileRaw(filePath, configId, data) StableConfigSource stableCfg = new StableConfigSource(filePath.toString(), ConfigOrigin.LOCAL_STABLE_CONFIG) then: - filePath != null stableCfg.getConfigId() == null stableCfg.getKeys().size() == 0 Files.delete(filePath) @@ -58,12 +67,17 @@ class StableConfigSourceTest extends DDSpecification { def "test file valid format"() { when: Path filePath = Files.createTempFile("testFile_", ".yaml") + then: + if (filePath == null) { + throw new AssertionError("Failed to create: " + filePath) + } + + when: StableConfig stableConfigYaml = new StableConfig(configId, defaultConfigs) writeFileYaml(filePath, stableConfigYaml) StableConfigSource stableCfg = new StableConfigSource(filePath.toString(), ConfigOrigin.LOCAL_STABLE_CONFIG) then: - filePath != null for (key in defaultConfigs.keySet()) { String keyString = (String) key keyString = keyString.substring(4) // Cut `DD_` From 24f89bce264f5600585cbaede5fe401576fb336c Mon Sep 17 00:00:00 2001 From: Yury Gribkov Date: Wed, 21 May 2025 14:59:03 +0200 Subject: [PATCH 6/6] Fix broken tests --- .../config/provider/stableconfig/StableConfig.java | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) 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 c8385546cf8..58fa3c4f826 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,10 +1,12 @@ package datadog.trace.bootstrap.config.provider.stableconfig; import static java.util.Collections.emptyList; +import static java.util.Collections.emptyMap; import static java.util.Collections.unmodifiableList; import static java.util.Collections.unmodifiableMap; import static java.util.stream.Collectors.toList; +import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -18,13 +20,20 @@ public StableConfig(Object yaml) { this.configId = String.valueOf(map.get("config_id")); this.apmConfigurationDefault = unmodifiableMap( - (Map) map.getOrDefault("apm_configuration_default", emptyList())); + (Map) map.getOrDefault("apm_configuration_default", emptyMap())); this.apmConfigurationRules = unmodifiableList( ((List) map.getOrDefault("apm_configuration_rules", emptyList())) .stream().map(Rule::new).collect(toList())); } + // test only + private StableConfig(String configId, Map apmConfigurationDefault) { + this.configId = configId; + this.apmConfigurationDefault = apmConfigurationDefault; + this.apmConfigurationRules = new ArrayList<>(); + } + public String getConfigId() { return configId; }