From d112dc78f41d9569488555feb05d2b814cbbe09f Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Thu, 10 Apr 2025 12:38:57 -0400 Subject: [PATCH 01/15] migrate CLIHelper to use a Map, parse system properties in key-vals --- .../src/main/java/datadog/cli/CLIHelper.java | 51 ++++++++++++++++--- .../trace/bootstrap/AgentBootstrap.java | 7 ++- .../config/provider/StableConfigParser.java | 7 ++- .../provider/StableConfigParserTest.groovy | 1 - 4 files changed, 52 insertions(+), 14 deletions(-) diff --git a/components/cli/src/main/java/datadog/cli/CLIHelper.java b/components/cli/src/main/java/datadog/cli/CLIHelper.java index 0f37953a92f..3fd318e2c6d 100644 --- a/components/cli/src/main/java/datadog/cli/CLIHelper.java +++ b/components/cli/src/main/java/datadog/cli/CLIHelper.java @@ -9,25 +9,30 @@ import java.nio.file.Paths; import java.util.Arrays; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Locale; +import java.util.Map; public final class CLIHelper { - private static final List VM_ARGS = findVmArgs(); + private static final Map VM_ARGS = findVmArgs(); - public static List getVmArgs() { + public static Map getVmArgs() { return VM_ARGS; } @SuppressForbidden - private static List findVmArgs() { + private static Map findVmArgs() { + List rawArgs; + // Try ProcFS on Linux try { if (isLinux()) { Path cmdlinePath = Paths.get("/proc/self/cmdline"); if (Files.exists(cmdlinePath)) { try (BufferedReader in = Files.newBufferedReader(cmdlinePath)) { - return Arrays.asList(in.readLine().split("\0")); + rawArgs = Arrays.asList(in.readLine().split("\0")); + return parseVmArgs(rawArgs); } } } @@ -57,7 +62,8 @@ private static List findVmArgs() { } //noinspection unchecked - return (List) vmManagementClass.getMethod("getVmArguments").invoke(vmManagement); + rawArgs = (List) vmManagementClass.getMethod("getVmArguments").invoke(vmManagement); + return parseVmArgs(rawArgs); } catch (final ReflectiveOperationException | UnsatisfiedLinkError ignored) { // Ignored exception } @@ -66,20 +72,49 @@ private static List findVmArgs() { try { final Class VMClass = Class.forName("com.ibm.oti.vm.VM"); final String[] argArray = (String[]) VMClass.getMethod("getVMArgs").invoke(null); - return Arrays.asList(argArray); + rawArgs = Arrays.asList(argArray); + return parseVmArgs(rawArgs); } catch (final ReflectiveOperationException ignored) { // Ignored exception } // Fallback to default try { - return ManagementFactory.getRuntimeMXBean().getInputArguments(); + rawArgs = ManagementFactory.getRuntimeMXBean().getInputArguments(); + return parseVmArgs(rawArgs); } catch (final Throwable t) { // Throws InvocationTargetException on modularized applications // with non-opened java.management module System.err.println("WARNING: Unable to get VM args using managed beans"); } - return Collections.emptyList(); + return Collections.emptyMap(); + } + + private static Map parseVmArgs(List args) { + Map result = new HashMap<>(); + + // For now, we only support values on system properties (-D arguments) + for (String arg : args) { + if (arg.startsWith("-D")) { + // Handle system properties (-D arguments) + int equalsIndex = arg.indexOf('='); + + if (equalsIndex >= 0) { + // Key-value pair + String key = arg.substring(0, equalsIndex); + String value = arg.substring(equalsIndex + 1); + result.put(key, value); + } else { + // Just a key with no value + result.put(arg, ""); + } + } else { + // Any other type of VM argument + result.put(arg, ""); + } + } + + return result; } private static boolean isLinux() { diff --git a/dd-java-agent/src/main/java/datadog/trace/bootstrap/AgentBootstrap.java b/dd-java-agent/src/main/java/datadog/trace/bootstrap/AgentBootstrap.java index f70038f26ba..b7f95818865 100644 --- a/dd-java-agent/src/main/java/datadog/trace/bootstrap/AgentBootstrap.java +++ b/dd-java-agent/src/main/java/datadog/trace/bootstrap/AgentBootstrap.java @@ -19,6 +19,7 @@ import java.util.Collections; import java.util.IdentityHashMap; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.jar.JarFile; @@ -381,8 +382,12 @@ private static List getAgentFilesFromVMArguments() { // - On IBM-based JDKs since at least 1.7 // This prevents custom log managers from working correctly // Use reflection to bypass the loading of the class~ - for (final String argument : CLIHelper.getVmArgs()) { + Map vmArgs = CLIHelper.getVmArgs(); + for (final Map.Entry entry : vmArgs.entrySet()) { + final String argument = entry.getKey(); if (argument.startsWith(JAVA_AGENT_ARGUMENT)) { + // TODO: Modify CLIHelper to parse key-vals on JAVA_AGENT_ARGUMENT arguments, so we don't + // have to do the parsing here. int index = argument.indexOf('=', JAVA_AGENT_ARGUMENT.length()); String agentPathname = argument.substring( 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 0c7554f2e0e..55523ae35ed 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 @@ -9,9 +9,8 @@ import java.io.IOException; import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; import java.util.List; -import java.util.Set; +import java.util.Map; import java.util.function.BiPredicate; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -19,7 +18,7 @@ public class StableConfigParser { private static final Logger log = LoggerFactory.getLogger(StableConfigParser.class); - private static final Set VM_ARGS = new HashSet<>(CLIHelper.getVmArgs()); + private static final Map VM_ARGS = CLIHelper.getVmArgs(); /** * Parses a configuration file and returns a stable configuration object. @@ -168,7 +167,7 @@ static boolean selectorMatch(String origin, List matches, String operato case "process_arguments": // For now, always return true if `key` exists in the JVM Args // TODO: flesh out the meaning of each operator for process_arguments - return VM_ARGS.contains(key); + return VM_ARGS.containsKey(key); case "tags": // TODO: Support this down the line (Must define the source of "tags" first) return false; 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 2df560a0eeb..6f19536a78d 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 @@ -96,7 +96,6 @@ apm_configuration_rules: "environment_variables" | ["svc"] | "contains" | "DD_SERVICE" | true "environment_variables" | ["other"] | "contains" | "DD_SERVICE" | false "environment_variables" | [null] | "contains" | "DD_SERVICE" | false - // "process_arguments" | null | "equals" | "-DCustomKey" | true } def "test duplicate entries"() { From 3ba99afb831f0e133a63b927242e6787f7bc1489 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Wed, 16 Apr 2025 15:12:32 -0400 Subject: [PATCH 02/15] Fix processTemplateVar logic to remove superfluous ':' --- .../src/main/java/datadog/cli/CLIHelper.java | 3 +- components/yaml/build.gradle.kts | 1 + .../main/java/datadog/yaml/YamlParser.java | 82 +++++++++++++++++-- .../config/provider/StableConfigParser.java | 4 +- 4 files changed, 80 insertions(+), 10 deletions(-) diff --git a/components/cli/src/main/java/datadog/cli/CLIHelper.java b/components/cli/src/main/java/datadog/cli/CLIHelper.java index 3fd318e2c6d..625a5c4356b 100644 --- a/components/cli/src/main/java/datadog/cli/CLIHelper.java +++ b/components/cli/src/main/java/datadog/cli/CLIHelper.java @@ -31,8 +31,7 @@ private static Map findVmArgs() { Path cmdlinePath = Paths.get("/proc/self/cmdline"); if (Files.exists(cmdlinePath)) { try (BufferedReader in = Files.newBufferedReader(cmdlinePath)) { - rawArgs = Arrays.asList(in.readLine().split("\0")); - return parseVmArgs(rawArgs); + return parseVmArgs(Arrays.asList(in.readLine().split("\0"))); } } } diff --git a/components/yaml/build.gradle.kts b/components/yaml/build.gradle.kts index 5acc8dd4cc2..d0d80f55015 100644 --- a/components/yaml/build.gradle.kts +++ b/components/yaml/build.gradle.kts @@ -11,4 +11,5 @@ jmh { // https://repo1.maven.org/maven2/org/yaml/snakeyaml/2.4/snakeyaml-2.4.pom dependencies { implementation("org.yaml", "snakeyaml", "2.4") + implementation(project(":components:cli")) } diff --git a/components/yaml/src/main/java/datadog/yaml/YamlParser.java b/components/yaml/src/main/java/datadog/yaml/YamlParser.java index 01aa7216fc1..a2b2a3daf63 100644 --- a/components/yaml/src/main/java/datadog/yaml/YamlParser.java +++ b/components/yaml/src/main/java/datadog/yaml/YamlParser.java @@ -1,19 +1,87 @@ package datadog.yaml; -import java.io.FileInputStream; +import datadog.cli.CLIHelper; import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.util.Map; import org.yaml.snakeyaml.Yaml; public class YamlParser { - // Supports clazz == null for default yaml parsing + private static final Map VM_ARGS = CLIHelper.getVmArgs(); + public static T parse(String filePath, Class clazz) throws IOException { Yaml yaml = new Yaml(); - try (FileInputStream fis = new FileInputStream(filePath)) { - if (clazz == null) { - return yaml.load(fis); - } else { - return yaml.loadAs(fis, clazz); + String content = new String(Files.readAllBytes(Paths.get(filePath))); + String processedContent = processTemplate(content); + + if (clazz == null) { + return yaml.load(processedContent); + } else { + return yaml.loadAs(processedContent, clazz); + } + } + + private static String processTemplate(String content) throws IOException { + StringBuilder result = new StringBuilder(content.length()); + String rest = content; + + while (true) { + int openIndex = rest.indexOf("{{"); + if (openIndex == -1) { + result.append(rest); + break; + } + + // Add everything before the template + result.append(rest.substring(0, openIndex)); + + // Find the closing braces + int closeIndex = rest.indexOf("}}", openIndex); + if (closeIndex == -1) { + throw new IOException("Unterminated template in config"); + } + + // Extract the template variable + String templateVar = rest.substring(openIndex + 2, closeIndex).trim(); + + // Process the template variable and get its value + String value = processTemplateVar(templateVar); + + // Add the processed value + result.append(value); + + // Continue with the rest of the string + rest = rest.substring(closeIndex + 2); + } + + return result.toString(); + } + + private static String processTemplateVar(String templateVar) throws IOException { + if (templateVar.startsWith("environment_variables[") && templateVar.endsWith("]")) { + String envVar = + templateVar.substring("environment_variables[".length(), templateVar.length() - 1).trim(); + if (envVar.isEmpty()) { + throw new IOException("Empty environment variable name in template"); + } + String value = System.getenv(envVar); + if (value == null || value.isEmpty()) { + return "UNDEFINED"; + } + return value; + } else if (templateVar.startsWith("process_arguments[") && templateVar.endsWith("]")) { + String processArg = + templateVar.substring("process_arguments[".length(), templateVar.length() - 1).trim(); + if (processArg.isEmpty()) { + throw new IOException("Empty process argument in template"); + } + String value = VM_ARGS.get(processArg); + if (value == null || value.isEmpty()) { + return "UNDEFINED"; } + return value; } + return "UNDEFINED"; } } 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 55523ae35ed..35e29e22d3d 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 @@ -65,7 +65,9 @@ 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 {}", filePath); + "Stable configuration file either not found or not readable at filepath {}. Error: {}", + filePath, + e.getMessage()); } return StableConfigSource.StableConfig.EMPTY; } From 9caeb5daf369a54bdba9e62df59261b5bfda7268 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Thu, 17 Apr 2025 15:54:25 -0400 Subject: [PATCH 03/15] nits: javadocs --- .../cli/src/main/java/datadog/cli/CLIHelper.java | 7 +++++++ .../src/main/java/datadog/yaml/YamlParser.java | 14 ++++++++++++-- .../test/groovy/datadog/yaml/YamlParserTest.groovy | 6 ++++++ 3 files changed, 25 insertions(+), 2 deletions(-) create mode 100644 components/yaml/src/test/groovy/datadog/yaml/YamlParserTest.groovy diff --git a/components/cli/src/main/java/datadog/cli/CLIHelper.java b/components/cli/src/main/java/datadog/cli/CLIHelper.java index 625a5c4356b..670af6247bd 100644 --- a/components/cli/src/main/java/datadog/cli/CLIHelper.java +++ b/components/cli/src/main/java/datadog/cli/CLIHelper.java @@ -14,6 +14,7 @@ import java.util.Locale; import java.util.Map; +/** Helper class for retrieving and parsing JVM arguments. */ public final class CLIHelper { private static final Map VM_ARGS = findVmArgs(); @@ -89,11 +90,17 @@ private static Map findVmArgs() { return Collections.emptyMap(); } + /** + * Parses a list of VM arguments into a map of key-value pairs. For system properties (-D + * arguments), the key includes the -D prefix and the value is everything after the = sign. For + * all other VM arguments, the key is the full argument and the value is an empty string. + */ private static Map parseVmArgs(List args) { Map result = new HashMap<>(); // For now, we only support values on system properties (-D arguments) for (String arg : args) { + // TODO: Handle other types of VM arguments if (arg.startsWith("-D")) { // Handle system properties (-D arguments) int equalsIndex = arg.indexOf('='); diff --git a/components/yaml/src/main/java/datadog/yaml/YamlParser.java b/components/yaml/src/main/java/datadog/yaml/YamlParser.java index a2b2a3daf63..144d857295e 100644 --- a/components/yaml/src/main/java/datadog/yaml/YamlParser.java +++ b/components/yaml/src/main/java/datadog/yaml/YamlParser.java @@ -22,7 +22,12 @@ public static T parse(String filePath, Class clazz) throws IOException { } } - private static String processTemplate(String content) throws IOException { + /** + * Processes a YAML template by replacing all template variables with their corresponding values. + * Template variables are enclosed in double curly braces, e.g. {{variable}}. Returns the + * processed content with all template variables resolved. + */ + static String processTemplate(String content) throws IOException { StringBuilder result = new StringBuilder(content.length()); String rest = content; @@ -58,7 +63,12 @@ private static String processTemplate(String content) throws IOException { return result.toString(); } - private static String processTemplateVar(String templateVar) throws IOException { + /** + * Processes a template variable by extracting its value from either environment variables or VM + * arguments. Template variables should be in the format "{{environment_variables[VAR_NAME]}}" or + * "{{process_arguments[-ARG_NAME]}}". Returns "UNDEFINED" if the variable is not found or empty. + */ + static String processTemplateVar(String templateVar) throws IOException { if (templateVar.startsWith("environment_variables[") && templateVar.endsWith("]")) { String envVar = templateVar.substring("environment_variables[".length(), templateVar.length() - 1).trim(); diff --git a/components/yaml/src/test/groovy/datadog/yaml/YamlParserTest.groovy b/components/yaml/src/test/groovy/datadog/yaml/YamlParserTest.groovy new file mode 100644 index 00000000000..22636316b89 --- /dev/null +++ b/components/yaml/src/test/groovy/datadog/yaml/YamlParserTest.groovy @@ -0,0 +1,6 @@ +package datadog.yaml + +import datadog.trace.test.util.DDSpecification + +class YamlParserTest extends DDSpecification{ +} From d0f194b1930a32b1182c49b05b2c9905f94375e7 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Mon, 21 Apr 2025 10:21:04 -0400 Subject: [PATCH 04/15] Move writeFileRaw helper to testutils --- components/yaml/build.gradle.kts | 1 + .../groovy/datadog/yaml/YamlParserTest.groovy | 23 ++++++++++++++++++ .../provider/StableConfigParserTest.groovy | 14 +++++------ .../provider/StableConfigSourceTest.groovy | 24 ++++--------------- .../datadog/trace/test/util/FileUtils.groovy | 23 ++++++++++++++++++ 5 files changed, 58 insertions(+), 27 deletions(-) create mode 100644 utils/test-utils/src/main/groovy/datadog/trace/test/util/FileUtils.groovy diff --git a/components/yaml/build.gradle.kts b/components/yaml/build.gradle.kts index d0d80f55015..a066062782a 100644 --- a/components/yaml/build.gradle.kts +++ b/components/yaml/build.gradle.kts @@ -12,4 +12,5 @@ jmh { dependencies { implementation("org.yaml", "snakeyaml", "2.4") implementation(project(":components:cli")) + testImplementation(project(":utils:test-utils")) } diff --git a/components/yaml/src/test/groovy/datadog/yaml/YamlParserTest.groovy b/components/yaml/src/test/groovy/datadog/yaml/YamlParserTest.groovy index 22636316b89..2a6f653811a 100644 --- a/components/yaml/src/test/groovy/datadog/yaml/YamlParserTest.groovy +++ b/components/yaml/src/test/groovy/datadog/yaml/YamlParserTest.groovy @@ -1,6 +1,29 @@ package datadog.yaml import datadog.trace.test.util.DDSpecification +import java.nio.file.Path +import datadog.trace.test.util.FileUtils class YamlParserTest extends DDSpecification{ + def "test parse"() { + // when: + // String yaml = """ + //apm_configuration_rules: + // - selectors: + // - origin: language + // matches: ["java"] + // operator: equals + // configuration: + // DD_SERVICE: "${templateVar}" + //""" + // Path filePath = FileUtils.tempFile() + // try { + // FileUtils.writeFileRaw(filePath, yaml) + // } catch (IOException e) { + // throw new AssertionError("Failed to write to file: ${e.message}") + // } + // where: + // templateVar | envKey | envVal | expect + // "{{environment_variables['DD_KEY']}}" | "DD_KEY" | "value" | "value" + } } 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 6f19536a78d..8c548c70b7f 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 @@ -1,14 +1,14 @@ package datadog.trace.bootstrap.config.provider import datadog.trace.test.util.DDSpecification - +import datadog.trace.test.util.FileUtils import java.nio.file.Files import java.nio.file.Path class StableConfigParserTest extends DDSpecification { def "test parse valid"() { when: - Path filePath = StableConfigSourceTest.tempFile() + Path filePath = FileUtils.tempFile() if (filePath == null) { throw new AssertionError("Failed to create test file") } @@ -42,7 +42,7 @@ apm_configuration_rules: KEY_FOUR: "ignored" """ try { - StableConfigSourceTest.writeFileRaw(filePath, yaml) + FileUtils.writeFileRaw(filePath, yaml) } catch (IOException e) { throw new AssertionError("Failed to write to file: ${e.message}") } @@ -101,7 +101,7 @@ apm_configuration_rules: def "test duplicate entries"() { // When duplicate keys are encountered, snakeyaml preserves the last value by default when: - Path filePath = StableConfigSourceTest.tempFile() + Path filePath = FileUtils.tempFile() if (filePath == null) { throw new AssertionError("Failed to create test file") } @@ -115,7 +115,7 @@ apm_configuration_rules: """ try { - StableConfigSourceTest.writeFileRaw(filePath, yaml) + FileUtils.writeFileRaw(filePath, yaml) } catch (IOException e) { throw new AssertionError("Failed to write to file: ${e.message}") } @@ -136,7 +136,7 @@ apm_configuration_rules: def "test parse invalid"() { // If any piece of the file is invalid, the whole file is rendered invalid and an exception is thrown when: - Path filePath = StableConfigSourceTest.tempFile() + Path filePath = FileUtils.tempFile() if (filePath == null) { throw new AssertionError("Failed to create test file") } @@ -158,7 +158,7 @@ apm_configuration_rules: something-else-irrelevant: value-irrelevant """ try { - StableConfigSourceTest.writeFileRaw(filePath, yaml) + FileUtils.writeFileRaw(filePath, yaml) } catch (IOException e) { throw new AssertionError("Failed to write to file: ${e.message}") } 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 b6596174985..dc431aba755 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,7 @@ 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.test.util.DDSpecification +import datadog.trace.test.util.FileUtils import org.yaml.snakeyaml.DumperOptions import org.yaml.snakeyaml.Yaml import org.yaml.snakeyaml.introspector.Property @@ -33,7 +34,7 @@ class StableConfigSourceTest extends DDSpecification { def "test empty file"() { when: - Path filePath = tempFile() + Path filePath = FileUtils.tempFile() if (filePath == null) { throw new AssertionError("Failed to create test file") } @@ -47,7 +48,7 @@ class StableConfigSourceTest extends DDSpecification { def "test file invalid format"() { // StableConfigSource must handle the exception thrown by StableConfigParser.parse(filePath) gracefully when: - Path filePath = tempFile() + Path filePath = FileUtils.tempFile() if (filePath == null) { throw new AssertionError("Failed to create test file") } @@ -73,7 +74,7 @@ class StableConfigSourceTest extends DDSpecification { def "test file valid format"() { when: - Path filePath = tempFile() + Path filePath = FileUtils.tempFile() if (filePath == null) { throw new AssertionError("Failed to create test file") } @@ -133,16 +134,6 @@ class StableConfigSourceTest extends DDSpecification { def static sampleNonMatchingRule = new Rule(Arrays.asList(new Selector("origin", "language", Arrays.asList("Golang"), null)), new ConfigurationMap(singletonMap("DD_KEY_FOUR", new ConfigurationValue("four")))) // Helper functions - static Path tempFile() { - try { - return Files.createTempFile("testFile_", ".yaml") - } catch (IOException e) { - println "Error creating file: ${e.message}" - e.printStackTrace() - return null // or throw new RuntimeException("File creation failed", e) - } - } - def stableConfigYamlWriter = getStableConfigYamlWriter() Yaml getStableConfigYamlWriter() { @@ -166,7 +157,6 @@ class StableConfigSourceTest extends DDSpecification { } def writeFileYaml(Path filePath, StableConfigYaml stableConfigs) { - // Yaml yaml = getStableConfigYaml(); try (FileWriter writer = new FileWriter(filePath.toString())) { stableConfigYamlWriter.dump(stableConfigs, writer) } catch (IOException e) { @@ -180,10 +170,4 @@ class StableConfigSourceTest extends DDSpecification { StandardOpenOption[] openOpts = [StandardOpenOption.WRITE] as StandardOpenOption[] Files.write(filePath, data.getBytes(), openOpts) } - - // Use this for writing a string directly into a file - static writeFileRaw(Path filePath, String data) { - StandardOpenOption[] openOpts = [StandardOpenOption.WRITE] as StandardOpenOption[] - Files.write(filePath, data.getBytes(), openOpts) - } } diff --git a/utils/test-utils/src/main/groovy/datadog/trace/test/util/FileUtils.groovy b/utils/test-utils/src/main/groovy/datadog/trace/test/util/FileUtils.groovy new file mode 100644 index 00000000000..d82c0fd1585 --- /dev/null +++ b/utils/test-utils/src/main/groovy/datadog/trace/test/util/FileUtils.groovy @@ -0,0 +1,23 @@ +package datadog.trace.test.util + +import java.nio.file.Files +import java.nio.file.Path +import java.nio.file.StandardOpenOption + +class FileUtils { + // Use this for writing a string directly into a file + static writeFileRaw(Path filePath, String data) { + StandardOpenOption[] openOpts = [StandardOpenOption.WRITE] as StandardOpenOption[] + Files.write(filePath, data.getBytes(), openOpts) + } + + static Path tempFile() { + try { + return Files.createTempFile("testFile_", ".yaml") + } catch (IOException e) { + println "Error creating file: ${e.message}" + e.printStackTrace() + return null + } + } +} From 457c2882697fc837f71d6861ae94d4c9b540c19b Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Mon, 21 Apr 2025 12:15:53 -0400 Subject: [PATCH 05/15] Add YamlParser tests --- components/yaml/build.gradle.kts | 1 + .../main/java/datadog/yaml/YamlParser.java | 12 ++-- .../groovy/datadog/yaml/YamlParserTest.groovy | 66 +++++++++++++------ 3 files changed, 55 insertions(+), 24 deletions(-) diff --git a/components/yaml/build.gradle.kts b/components/yaml/build.gradle.kts index a066062782a..5d9ff3a6f37 100644 --- a/components/yaml/build.gradle.kts +++ b/components/yaml/build.gradle.kts @@ -13,4 +13,5 @@ dependencies { implementation("org.yaml", "snakeyaml", "2.4") implementation(project(":components:cli")) testImplementation(project(":utils:test-utils")) + testImplementation(project(":internal-api")) } diff --git a/components/yaml/src/main/java/datadog/yaml/YamlParser.java b/components/yaml/src/main/java/datadog/yaml/YamlParser.java index 144d857295e..5a397f27331 100644 --- a/components/yaml/src/main/java/datadog/yaml/YamlParser.java +++ b/components/yaml/src/main/java/datadog/yaml/YamlParser.java @@ -69,20 +69,22 @@ static String processTemplate(String content) throws IOException { * "{{process_arguments[-ARG_NAME]}}". Returns "UNDEFINED" if the variable is not found or empty. */ static String processTemplateVar(String templateVar) throws IOException { - if (templateVar.startsWith("environment_variables[") && templateVar.endsWith("]")) { + if (templateVar.startsWith("environment_variables['") && templateVar.endsWith("']")) { String envVar = - templateVar.substring("environment_variables[".length(), templateVar.length() - 1).trim(); + templateVar + .substring("environment_variables['".length(), templateVar.length() - 2) + .trim(); if (envVar.isEmpty()) { throw new IOException("Empty environment variable name in template"); } - String value = System.getenv(envVar); + String value = System.getenv(envVar.toUpperCase()); if (value == null || value.isEmpty()) { return "UNDEFINED"; } return value; - } else if (templateVar.startsWith("process_arguments[") && templateVar.endsWith("]")) { + } else if (templateVar.startsWith("process_arguments['") && templateVar.endsWith("']")) { String processArg = - templateVar.substring("process_arguments[".length(), templateVar.length() - 1).trim(); + templateVar.substring("process_arguments['".length(), templateVar.length() - 2).trim(); if (processArg.isEmpty()) { throw new IOException("Empty process argument in template"); } diff --git a/components/yaml/src/test/groovy/datadog/yaml/YamlParserTest.groovy b/components/yaml/src/test/groovy/datadog/yaml/YamlParserTest.groovy index 2a6f653811a..132a43eaf10 100644 --- a/components/yaml/src/test/groovy/datadog/yaml/YamlParserTest.groovy +++ b/components/yaml/src/test/groovy/datadog/yaml/YamlParserTest.groovy @@ -1,29 +1,57 @@ package datadog.yaml +import datadog.trace.bootstrap.config.provider.stableconfigyaml.StableConfigYaml import datadog.trace.test.util.DDSpecification import java.nio.file.Path import datadog.trace.test.util.FileUtils class YamlParserTest extends DDSpecification{ def "test parse"() { - // when: - // String yaml = """ - //apm_configuration_rules: - // - selectors: - // - origin: language - // matches: ["java"] - // operator: equals - // configuration: - // DD_SERVICE: "${templateVar}" - //""" - // Path filePath = FileUtils.tempFile() - // try { - // FileUtils.writeFileRaw(filePath, yaml) - // } catch (IOException e) { - // throw new AssertionError("Failed to write to file: ${e.message}") - // } - // where: - // templateVar | envKey | envVal | expect - // "{{environment_variables['DD_KEY']}}" | "DD_KEY" | "value" | "value" + when: + String yaml = """ + apm_configuration_rules: + - selectors: + - origin: language + matches: ["java"] + operator: equals + configuration: + DD_SERVICE: "${templateVar}" + """ + Path filePath = FileUtils.tempFile() + try { + FileUtils.writeFileRaw(filePath, yaml) + } catch (IOException e) { + throw new AssertionError("Failed to write to file: ${e.message}") + } + + if (envKey != null) { + injectEnvConfig(envKey, envVal) + } + String service + try { + def data = YamlParser.parse(filePath as String, StableConfigYaml) + def configs = data.getApm_configuration_rules().get(0).getConfiguration() + service = configs.get("DD_SERVICE").toString() + } catch (IOException e) { + // parse failed, stable config will be dropped + service = null + } + + then: + service == expect + + where: + templateVar | envKey | envVal | expect + "{{environment_variables['DD_KEY']}}" | "DD_KEY" | "value" | "value" + "{{environment_variables['DD_KEY']}}" | null | null | "UNDEFINED" + "{{environment_variables['DD_KEY}}" | "DD_KEY" | "value" | "UNDEFINED" + "{{DD_KEY}}" | "DD_KEY" | "value" | "UNDEFINED" + "{{environment_variables['']}}" | null | null | null + "{{environment_variables['OTHER_KEY']}}" | "DD_KEY" | "value" | "UNDEFINED" + "{{}}" | null | null | "UNDEFINED" + "{}" | null | null | "{}" + "{{environment_variables[DD_KEY]}}" | "DD_KEY" | "value" | "UNDEFINED" + "{{environment_variables['DD_KEY']}" | null | null | null + "{{environment_variables['dd_key']}}" | "DD_KEY" | "value" | "value" } } From 06ae9acb25acd9e3e46b5306ee3a290d794b4471 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Tue, 29 Apr 2025 15:10:52 -0400 Subject: [PATCH 06/15] Revert CLIHelper to use List; lazily load a Map cache for querying key-vals --- .../src/main/java/datadog/cli/CLIHelper.java | 136 ++++++++++++------ .../main/java/datadog/yaml/YamlParser.java | 6 +- .../trace/bootstrap/AgentBootstrap.java | 7 +- .../config/provider/StableConfigParser.java | 5 +- 4 files changed, 99 insertions(+), 55 deletions(-) diff --git a/components/cli/src/main/java/datadog/cli/CLIHelper.java b/components/cli/src/main/java/datadog/cli/CLIHelper.java index 670af6247bd..e77da7cbe5d 100644 --- a/components/cli/src/main/java/datadog/cli/CLIHelper.java +++ b/components/cli/src/main/java/datadog/cli/CLIHelper.java @@ -16,14 +16,101 @@ /** Helper class for retrieving and parsing JVM arguments. */ public final class CLIHelper { - private static final Map VM_ARGS = findVmArgs(); + private static final List VM_ARGS = findVmArgs(); + private static Map VM_ARGS_MAP = Collections.emptyMap(); + private static int currentIndex = 0; // Tracks the last processed index in VM_ARGS - public static Map getVmArgs() { + public static List getVmArgs() { return VM_ARGS; } + public static String getArgValue(String key) { + int numArgs = VM_ARGS.size(); + // Lazy population of cache + synchronized (VM_ARGS_MAP) { + // Double check after acquiring lock + if (VM_ARGS_MAP.containsKey(key)) { + return VM_ARGS_MAP.get(key); + } + if (currentIndex >= numArgs) { + return null; + } + + // Initialize cache if empty + if (VM_ARGS_MAP.isEmpty()) { + VM_ARGS_MAP = new HashMap<>(); + } + + // Process remaining args + while (currentIndex < numArgs) { + String arg = VM_ARGS.get(currentIndex); + if (arg.startsWith("-D")) { + int equalsIndex = arg.indexOf('='); + if (equalsIndex >= 0) { + String argKey = arg.substring(0, equalsIndex); + String argValue = arg.substring(equalsIndex + 1); + VM_ARGS_MAP.put(argKey, argValue); + } else { + VM_ARGS_MAP.put(arg, ""); + } + } else { + VM_ARGS_MAP.put(arg, ""); + } + currentIndex++; + + // Check if we found our key + if (VM_ARGS_MAP.containsKey(key)) { + return VM_ARGS_MAP.get(key); + } + } + + return null; + } + } + + public static boolean argExists(String key) { + int numArgs = VM_ARGS.size(); + // Lazy population of cache + synchronized (VM_ARGS_MAP) { + // Double check after acquiring lock + if (currentIndex >= numArgs) { + return VM_ARGS_MAP.containsKey(key); + } + + // Initialize cache if empty + if (VM_ARGS_MAP.isEmpty()) { + VM_ARGS_MAP = new HashMap<>(); + } + + // Process remaining args + while (currentIndex < numArgs) { + String arg = VM_ARGS.get(currentIndex); + if (arg.startsWith("-D")) { + int equalsIndex = arg.indexOf('='); + if (equalsIndex >= 0) { + String argKey = arg.substring(0, equalsIndex); + String argValue = arg.substring(equalsIndex + 1); + VM_ARGS_MAP.put(argKey, argValue); + } else { + VM_ARGS_MAP.put(arg, ""); + } + } else { + VM_ARGS_MAP.put(arg, ""); + } + currentIndex++; + + // Check if we found our key + if (key.equals(arg)) { + return true; + } + } + + return false; + } + } + @SuppressForbidden - private static Map findVmArgs() { + private static List findVmArgs() { List rawArgs; // Try ProcFS on Linux @@ -32,7 +119,7 @@ private static Map findVmArgs() { Path cmdlinePath = Paths.get("/proc/self/cmdline"); if (Files.exists(cmdlinePath)) { try (BufferedReader in = Files.newBufferedReader(cmdlinePath)) { - return parseVmArgs(Arrays.asList(in.readLine().split("\0"))); + return Arrays.asList(in.readLine().split("\0")); } } } @@ -63,7 +150,7 @@ private static Map findVmArgs() { //noinspection unchecked rawArgs = (List) vmManagementClass.getMethod("getVmArguments").invoke(vmManagement); - return parseVmArgs(rawArgs); + return rawArgs; } catch (final ReflectiveOperationException | UnsatisfiedLinkError ignored) { // Ignored exception } @@ -73,7 +160,7 @@ private static Map findVmArgs() { final Class VMClass = Class.forName("com.ibm.oti.vm.VM"); final String[] argArray = (String[]) VMClass.getMethod("getVMArgs").invoke(null); rawArgs = Arrays.asList(argArray); - return parseVmArgs(rawArgs); + return rawArgs; } catch (final ReflectiveOperationException ignored) { // Ignored exception } @@ -81,46 +168,13 @@ private static Map findVmArgs() { // Fallback to default try { rawArgs = ManagementFactory.getRuntimeMXBean().getInputArguments(); - return parseVmArgs(rawArgs); + return rawArgs; } catch (final Throwable t) { // Throws InvocationTargetException on modularized applications // with non-opened java.management module System.err.println("WARNING: Unable to get VM args using managed beans"); } - return Collections.emptyMap(); - } - - /** - * Parses a list of VM arguments into a map of key-value pairs. For system properties (-D - * arguments), the key includes the -D prefix and the value is everything after the = sign. For - * all other VM arguments, the key is the full argument and the value is an empty string. - */ - private static Map parseVmArgs(List args) { - Map result = new HashMap<>(); - - // For now, we only support values on system properties (-D arguments) - for (String arg : args) { - // TODO: Handle other types of VM arguments - if (arg.startsWith("-D")) { - // Handle system properties (-D arguments) - int equalsIndex = arg.indexOf('='); - - if (equalsIndex >= 0) { - // Key-value pair - String key = arg.substring(0, equalsIndex); - String value = arg.substring(equalsIndex + 1); - result.put(key, value); - } else { - // Just a key with no value - result.put(arg, ""); - } - } else { - // Any other type of VM argument - result.put(arg, ""); - } - } - - return result; + return Collections.emptyList(); } private static boolean isLinux() { diff --git a/components/yaml/src/main/java/datadog/yaml/YamlParser.java b/components/yaml/src/main/java/datadog/yaml/YamlParser.java index 5a397f27331..0d0821933e7 100644 --- a/components/yaml/src/main/java/datadog/yaml/YamlParser.java +++ b/components/yaml/src/main/java/datadog/yaml/YamlParser.java @@ -4,12 +4,10 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Paths; -import java.util.Map; import org.yaml.snakeyaml.Yaml; public class YamlParser { - private static final Map VM_ARGS = CLIHelper.getVmArgs(); - + // Supports clazz == null for default yaml parsing public static T parse(String filePath, Class clazz) throws IOException { Yaml yaml = new Yaml(); String content = new String(Files.readAllBytes(Paths.get(filePath))); @@ -88,7 +86,7 @@ static String processTemplateVar(String templateVar) throws IOException { if (processArg.isEmpty()) { throw new IOException("Empty process argument in template"); } - String value = VM_ARGS.get(processArg); + String value = CLIHelper.getArgValue(processArg); if (value == null || value.isEmpty()) { return "UNDEFINED"; } diff --git a/dd-java-agent/src/main/java/datadog/trace/bootstrap/AgentBootstrap.java b/dd-java-agent/src/main/java/datadog/trace/bootstrap/AgentBootstrap.java index b7f95818865..f70038f26ba 100644 --- a/dd-java-agent/src/main/java/datadog/trace/bootstrap/AgentBootstrap.java +++ b/dd-java-agent/src/main/java/datadog/trace/bootstrap/AgentBootstrap.java @@ -19,7 +19,6 @@ import java.util.Collections; import java.util.IdentityHashMap; import java.util.List; -import java.util.Map; import java.util.Set; import java.util.jar.JarFile; @@ -382,12 +381,8 @@ private static List getAgentFilesFromVMArguments() { // - On IBM-based JDKs since at least 1.7 // This prevents custom log managers from working correctly // Use reflection to bypass the loading of the class~ - Map vmArgs = CLIHelper.getVmArgs(); - for (final Map.Entry entry : vmArgs.entrySet()) { - final String argument = entry.getKey(); + for (final String argument : CLIHelper.getVmArgs()) { if (argument.startsWith(JAVA_AGENT_ARGUMENT)) { - // TODO: Modify CLIHelper to parse key-vals on JAVA_AGENT_ARGUMENT arguments, so we don't - // have to do the parsing here. int index = argument.indexOf('=', JAVA_AGENT_ARGUMENT.length()); String agentPathname = argument.substring( 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 35e29e22d3d..8d107e928da 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 @@ -10,7 +10,6 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.function.BiPredicate; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -18,8 +17,6 @@ public class StableConfigParser { private static final Logger log = LoggerFactory.getLogger(StableConfigParser.class); - private static final Map VM_ARGS = CLIHelper.getVmArgs(); - /** * Parses a configuration file and returns a stable configuration object. * @@ -169,7 +166,7 @@ static boolean selectorMatch(String origin, List matches, String operato case "process_arguments": // For now, always return true if `key` exists in the JVM Args // TODO: flesh out the meaning of each operator for process_arguments - return VM_ARGS.containsKey(key); + return CLIHelper.argExists(key); case "tags": // TODO: Support this down the line (Must define the source of "tags" first) return false; From 06712dc39ebba2d4b8ec78ba425cd6c615db0ab4 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Tue, 29 Apr 2025 15:40:53 -0400 Subject: [PATCH 07/15] reuse logic for adding to the vm args cache --- .../src/main/java/datadog/cli/CLIHelper.java | 42 ++++++++----------- 1 file changed, 17 insertions(+), 25 deletions(-) diff --git a/components/cli/src/main/java/datadog/cli/CLIHelper.java b/components/cli/src/main/java/datadog/cli/CLIHelper.java index e77da7cbe5d..40505a886c6 100644 --- a/components/cli/src/main/java/datadog/cli/CLIHelper.java +++ b/components/cli/src/main/java/datadog/cli/CLIHelper.java @@ -43,19 +43,7 @@ public static String getArgValue(String key) { // Process remaining args while (currentIndex < numArgs) { - String arg = VM_ARGS.get(currentIndex); - if (arg.startsWith("-D")) { - int equalsIndex = arg.indexOf('='); - if (equalsIndex >= 0) { - String argKey = arg.substring(0, equalsIndex); - String argValue = arg.substring(equalsIndex + 1); - VM_ARGS_MAP.put(argKey, argValue); - } else { - VM_ARGS_MAP.put(arg, ""); - } - } else { - VM_ARGS_MAP.put(arg, ""); - } + populateCache(VM_ARGS.get(currentIndex)); currentIndex++; // Check if we found our key @@ -68,6 +56,21 @@ public static String getArgValue(String key) { } } + private static void populateCache(String arg) { + if (arg.startsWith("-D")) { + int equalsIndex = arg.indexOf('='); + if (equalsIndex >= 0) { + String argKey = arg.substring(0, equalsIndex); + String argValue = arg.substring(equalsIndex + 1); + VM_ARGS_MAP.put(argKey, argValue); + } else { + VM_ARGS_MAP.put(arg, ""); + } + } else { + VM_ARGS_MAP.put(arg, ""); + } + } + public static boolean argExists(String key) { int numArgs = VM_ARGS.size(); // Lazy population of cache @@ -85,18 +88,7 @@ public static boolean argExists(String key) { // Process remaining args while (currentIndex < numArgs) { String arg = VM_ARGS.get(currentIndex); - if (arg.startsWith("-D")) { - int equalsIndex = arg.indexOf('='); - if (equalsIndex >= 0) { - String argKey = arg.substring(0, equalsIndex); - String argValue = arg.substring(equalsIndex + 1); - VM_ARGS_MAP.put(argKey, argValue); - } else { - VM_ARGS_MAP.put(arg, ""); - } - } else { - VM_ARGS_MAP.put(arg, ""); - } + populateCache(arg); currentIndex++; // Check if we found our key From 0a1da8ff3e91a5a8dd7d4eecf2f5b437786dacc0 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Thu, 1 May 2025 09:18:19 -0400 Subject: [PATCH 08/15] apply github suggestions --- .../src/main/java/datadog/cli/CLIHelper.java | 61 ++++--------------- .../main/java/datadog/yaml/YamlParser.java | 3 +- 2 files changed, 14 insertions(+), 50 deletions(-) diff --git a/components/cli/src/main/java/datadog/cli/CLIHelper.java b/components/cli/src/main/java/datadog/cli/CLIHelper.java index 40505a886c6..5ef6cf8f2d3 100644 --- a/components/cli/src/main/java/datadog/cli/CLIHelper.java +++ b/components/cli/src/main/java/datadog/cli/CLIHelper.java @@ -17,7 +17,7 @@ /** Helper class for retrieving and parsing JVM arguments. */ public final class CLIHelper { private static final List VM_ARGS = findVmArgs(); - private static Map VM_ARGS_MAP = Collections.emptyMap(); + private static final Map VM_ARGS_MAP = new HashMap<>(); private static int currentIndex = 0; // Tracks the last processed index in VM_ARGS public static List getVmArgs() { @@ -25,30 +25,23 @@ public static List getVmArgs() { } public static String getArgValue(String key) { - int numArgs = VM_ARGS.size(); // Lazy population of cache synchronized (VM_ARGS_MAP) { - // Double check after acquiring lock - if (VM_ARGS_MAP.containsKey(key)) { - return VM_ARGS_MAP.get(key); - } - if (currentIndex >= numArgs) { - return null; - } - - // Initialize cache if empty - if (VM_ARGS_MAP.isEmpty()) { - VM_ARGS_MAP = new HashMap<>(); + String value = VM_ARGS_MAP.get(key); + if (value != null) { + return value; } // Process remaining args + int numArgs = VM_ARGS.size(); while (currentIndex < numArgs) { populateCache(VM_ARGS.get(currentIndex)); currentIndex++; // Check if we found our key - if (VM_ARGS_MAP.containsKey(key)) { - return VM_ARGS_MAP.get(key); + value = VM_ARGS_MAP.get(key); + if (value != null) { + return value; } } @@ -72,38 +65,11 @@ private static void populateCache(String arg) { } public static boolean argExists(String key) { - int numArgs = VM_ARGS.size(); - // Lazy population of cache - synchronized (VM_ARGS_MAP) { - // Double check after acquiring lock - if (currentIndex >= numArgs) { - return VM_ARGS_MAP.containsKey(key); - } - - // Initialize cache if empty - if (VM_ARGS_MAP.isEmpty()) { - VM_ARGS_MAP = new HashMap<>(); - } - - // Process remaining args - while (currentIndex < numArgs) { - String arg = VM_ARGS.get(currentIndex); - populateCache(arg); - currentIndex++; - - // Check if we found our key - if (key.equals(arg)) { - return true; - } - } - - return false; - } + return getArgValue(key) != null; } @SuppressForbidden private static List findVmArgs() { - List rawArgs; // Try ProcFS on Linux try { @@ -141,8 +107,7 @@ private static List findVmArgs() { } //noinspection unchecked - rawArgs = (List) vmManagementClass.getMethod("getVmArguments").invoke(vmManagement); - return rawArgs; + return (List) vmManagementClass.getMethod("getVmArguments").invoke(vmManagement); } catch (final ReflectiveOperationException | UnsatisfiedLinkError ignored) { // Ignored exception } @@ -151,16 +116,14 @@ private static List findVmArgs() { try { final Class VMClass = Class.forName("com.ibm.oti.vm.VM"); final String[] argArray = (String[]) VMClass.getMethod("getVMArgs").invoke(null); - rawArgs = Arrays.asList(argArray); - return rawArgs; + return Arrays.asList(argArray); } catch (final ReflectiveOperationException ignored) { // Ignored exception } // Fallback to default try { - rawArgs = ManagementFactory.getRuntimeMXBean().getInputArguments(); - return rawArgs; + return ManagementFactory.getRuntimeMXBean().getInputArguments(); } catch (final Throwable t) { // Throws InvocationTargetException on modularized applications // with non-opened java.management module diff --git a/components/yaml/src/main/java/datadog/yaml/YamlParser.java b/components/yaml/src/main/java/datadog/yaml/YamlParser.java index 0d0821933e7..9d8682c38c8 100644 --- a/components/yaml/src/main/java/datadog/yaml/YamlParser.java +++ b/components/yaml/src/main/java/datadog/yaml/YamlParser.java @@ -2,6 +2,7 @@ import datadog.cli.CLIHelper; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Paths; import org.yaml.snakeyaml.Yaml; @@ -10,7 +11,7 @@ public class YamlParser { // Supports clazz == null for default yaml parsing public static T parse(String filePath, Class clazz) throws IOException { Yaml yaml = new Yaml(); - String content = new String(Files.readAllBytes(Paths.get(filePath))); + String content = new String(Files.readAllBytes(Paths.get(filePath)), StandardCharsets.UTF_8); String processedContent = processTemplate(content); if (clazz == null) { From e1064160dfec10b9f4e765a160700864f88a7c66 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Thu, 1 May 2025 12:04:54 -0400 Subject: [PATCH 09/15] Move processTemplate yaml helper fns into StableConfigParser, along with tests --- .../src/main/java/datadog/cli/CLIHelper.java | 1 - components/yaml/build.gradle.kts | 2 - .../main/java/datadog/yaml/YamlParser.java | 89 +------------------ .../groovy/datadog/yaml/YamlParserTest.groovy | 57 ------------ .../config/provider/StableConfigParser.java | 72 ++++++++++++++- .../provider/StableConfigParserTest.groovy | 34 +++++++ 6 files changed, 108 insertions(+), 147 deletions(-) delete mode 100644 components/yaml/src/test/groovy/datadog/yaml/YamlParserTest.groovy diff --git a/components/cli/src/main/java/datadog/cli/CLIHelper.java b/components/cli/src/main/java/datadog/cli/CLIHelper.java index 5ef6cf8f2d3..7b591d32d3e 100644 --- a/components/cli/src/main/java/datadog/cli/CLIHelper.java +++ b/components/cli/src/main/java/datadog/cli/CLIHelper.java @@ -70,7 +70,6 @@ public static boolean argExists(String key) { @SuppressForbidden private static List findVmArgs() { - // Try ProcFS on Linux try { if (isLinux()) { diff --git a/components/yaml/build.gradle.kts b/components/yaml/build.gradle.kts index 5d9ff3a6f37..d0d80f55015 100644 --- a/components/yaml/build.gradle.kts +++ b/components/yaml/build.gradle.kts @@ -12,6 +12,4 @@ jmh { dependencies { implementation("org.yaml", "snakeyaml", "2.4") implementation(project(":components:cli")) - testImplementation(project(":utils:test-utils")) - testImplementation(project(":internal-api")) } diff --git a/components/yaml/src/main/java/datadog/yaml/YamlParser.java b/components/yaml/src/main/java/datadog/yaml/YamlParser.java index 9d8682c38c8..7af6d22303b 100644 --- a/components/yaml/src/main/java/datadog/yaml/YamlParser.java +++ b/components/yaml/src/main/java/datadog/yaml/YamlParser.java @@ -1,98 +1,15 @@ package datadog.yaml; -import datadog.cli.CLIHelper; -import java.io.IOException; -import java.nio.charset.StandardCharsets; -import java.nio.file.Files; -import java.nio.file.Paths; import org.yaml.snakeyaml.Yaml; public class YamlParser { // Supports clazz == null for default yaml parsing - public static T parse(String filePath, Class clazz) throws IOException { + public static T parse(String content, Class clazz) { Yaml yaml = new Yaml(); - String content = new String(Files.readAllBytes(Paths.get(filePath)), StandardCharsets.UTF_8); - String processedContent = processTemplate(content); - if (clazz == null) { - return yaml.load(processedContent); + return yaml.load(content); } else { - return yaml.loadAs(processedContent, clazz); - } - } - - /** - * Processes a YAML template by replacing all template variables with their corresponding values. - * Template variables are enclosed in double curly braces, e.g. {{variable}}. Returns the - * processed content with all template variables resolved. - */ - static String processTemplate(String content) throws IOException { - StringBuilder result = new StringBuilder(content.length()); - String rest = content; - - while (true) { - int openIndex = rest.indexOf("{{"); - if (openIndex == -1) { - result.append(rest); - break; - } - - // Add everything before the template - result.append(rest.substring(0, openIndex)); - - // Find the closing braces - int closeIndex = rest.indexOf("}}", openIndex); - if (closeIndex == -1) { - throw new IOException("Unterminated template in config"); - } - - // Extract the template variable - String templateVar = rest.substring(openIndex + 2, closeIndex).trim(); - - // Process the template variable and get its value - String value = processTemplateVar(templateVar); - - // Add the processed value - result.append(value); - - // Continue with the rest of the string - rest = rest.substring(closeIndex + 2); - } - - return result.toString(); - } - - /** - * Processes a template variable by extracting its value from either environment variables or VM - * arguments. Template variables should be in the format "{{environment_variables[VAR_NAME]}}" or - * "{{process_arguments[-ARG_NAME]}}". Returns "UNDEFINED" if the variable is not found or empty. - */ - static String processTemplateVar(String templateVar) throws IOException { - if (templateVar.startsWith("environment_variables['") && templateVar.endsWith("']")) { - String envVar = - templateVar - .substring("environment_variables['".length(), templateVar.length() - 2) - .trim(); - if (envVar.isEmpty()) { - throw new IOException("Empty environment variable name in template"); - } - String value = System.getenv(envVar.toUpperCase()); - if (value == null || value.isEmpty()) { - return "UNDEFINED"; - } - return value; - } else if (templateVar.startsWith("process_arguments['") && templateVar.endsWith("']")) { - String processArg = - templateVar.substring("process_arguments['".length(), templateVar.length() - 2).trim(); - if (processArg.isEmpty()) { - throw new IOException("Empty process argument in template"); - } - String value = CLIHelper.getArgValue(processArg); - if (value == null || value.isEmpty()) { - return "UNDEFINED"; - } - return value; + return yaml.loadAs(content, clazz); } - return "UNDEFINED"; } } diff --git a/components/yaml/src/test/groovy/datadog/yaml/YamlParserTest.groovy b/components/yaml/src/test/groovy/datadog/yaml/YamlParserTest.groovy deleted file mode 100644 index 132a43eaf10..00000000000 --- a/components/yaml/src/test/groovy/datadog/yaml/YamlParserTest.groovy +++ /dev/null @@ -1,57 +0,0 @@ -package datadog.yaml - -import datadog.trace.bootstrap.config.provider.stableconfigyaml.StableConfigYaml -import datadog.trace.test.util.DDSpecification -import java.nio.file.Path -import datadog.trace.test.util.FileUtils - -class YamlParserTest extends DDSpecification{ - def "test parse"() { - when: - String yaml = """ - apm_configuration_rules: - - selectors: - - origin: language - matches: ["java"] - operator: equals - configuration: - DD_SERVICE: "${templateVar}" - """ - Path filePath = FileUtils.tempFile() - try { - FileUtils.writeFileRaw(filePath, yaml) - } catch (IOException e) { - throw new AssertionError("Failed to write to file: ${e.message}") - } - - if (envKey != null) { - injectEnvConfig(envKey, envVal) - } - String service - try { - def data = YamlParser.parse(filePath as String, StableConfigYaml) - def configs = data.getApm_configuration_rules().get(0).getConfiguration() - service = configs.get("DD_SERVICE").toString() - } catch (IOException e) { - // parse failed, stable config will be dropped - service = null - } - - then: - service == expect - - where: - templateVar | envKey | envVal | expect - "{{environment_variables['DD_KEY']}}" | "DD_KEY" | "value" | "value" - "{{environment_variables['DD_KEY']}}" | null | null | "UNDEFINED" - "{{environment_variables['DD_KEY}}" | "DD_KEY" | "value" | "UNDEFINED" - "{{DD_KEY}}" | "DD_KEY" | "value" | "UNDEFINED" - "{{environment_variables['']}}" | null | null | null - "{{environment_variables['OTHER_KEY']}}" | "DD_KEY" | "value" | "UNDEFINED" - "{{}}" | null | null | "UNDEFINED" - "{}" | null | null | "{}" - "{{environment_variables[DD_KEY]}}" | "DD_KEY" | "value" | "UNDEFINED" - "{{environment_variables['DD_KEY']}" | null | null | null - "{{environment_variables['dd_key']}}" | "DD_KEY" | "value" | "value" - } -} 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 8d107e928da..50f46dcb11f 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 @@ -7,6 +7,9 @@ import datadog.trace.bootstrap.config.provider.stableconfigyaml.StableConfigYaml; import datadog.yaml.YamlParser; import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Paths; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -33,7 +36,9 @@ public class StableConfigParser { */ public static StableConfigSource.StableConfig parse(String filePath) throws IOException { try { - StableConfigYaml data = YamlParser.parse(filePath, StableConfigYaml.class); + String content = new String(Files.readAllBytes(Paths.get(filePath)), StandardCharsets.UTF_8); + String processedContent = processTemplate(content); + StableConfigYaml data = YamlParser.parse(processedContent, StableConfigYaml.class); String configId = data.getConfig_id(); ConfigurationMap configMap = data.getApm_configuration_default(); @@ -174,4 +179,69 @@ static boolean selectorMatch(String origin, List matches, String operato return false; } } + + static String processTemplate(String content) throws IOException { + StringBuilder result = new StringBuilder(content.length()); + String rest = content; + + while (true) { + int openIndex = rest.indexOf("{{"); + if (openIndex == -1) { + result.append(rest); + break; + } + + // Add everything before the template + result.append(rest.substring(0, openIndex)); + + // Find the closing braces + int closeIndex = rest.indexOf("}}", openIndex); + if (closeIndex == -1) { + throw new IOException("Unterminated template in config"); + } + + // Extract the template variable + String templateVar = rest.substring(openIndex + 2, closeIndex).trim(); + + // Process the template variable and get its value + String value = processTemplateVar(templateVar); + + // Add the processed value + result.append(value); + + // Continue with the rest of the string + rest = rest.substring(closeIndex + 2); + } + + return result.toString(); + } + + private static String processTemplateVar(String templateVar) throws IOException { + if (templateVar.startsWith("environment_variables['") && templateVar.endsWith("']")) { + String envVar = + templateVar + .substring("environment_variables['".length(), templateVar.length() - 2) + .trim(); + if (envVar.isEmpty()) { + throw new IOException("Empty environment variable name in template"); + } + String value = System.getenv(envVar.toUpperCase()); + if (value == null || value.isEmpty()) { + return "UNDEFINED"; + } + return value; + } else if (templateVar.startsWith("process_arguments['") && templateVar.endsWith("']")) { + String processArg = + templateVar.substring("process_arguments['".length(), templateVar.length() - 2).trim(); + if (processArg.isEmpty()) { + throw new IOException("Empty process argument in template"); + } + String value = CLIHelper.getArgValue(processArg); + if (value == null || value.isEmpty()) { + return "UNDEFINED"; + } + return value; + } + return "UNDEFINED"; + } } 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 8c548c70b7f..2199e9fb9de 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 @@ -2,6 +2,7 @@ package datadog.trace.bootstrap.config.provider import datadog.trace.test.util.DDSpecification import datadog.trace.test.util.FileUtils + import java.nio.file.Files import java.nio.file.Path @@ -176,4 +177,37 @@ apm_configuration_rules: cfg == null Files.delete(filePath) } + + def "test processTemplate valid cases"() { + when: + if (envKey != null) { + injectEnvConfig(envKey, envVal) + } + + then: + 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" + } + + def "test processTemplate error cases"() { + when: + StableConfigParser.processTemplate(templateVar) + + then: + def e = thrown(IOException) + e.message == expect + + where: + templateVar | expect + "{{environment_variables['']}}" | "Empty environment variable name in template" + "{{environment_variables['DD_KEY']}" | "Unterminated template in config" + } } From 48adeaaf6f15b97471001e003ce3f4ca7ab43637 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Thu, 1 May 2025 12:55:00 -0400 Subject: [PATCH 10/15] Remove changes to CLIHelper; rely on System.getProperty instead --- .../src/main/java/datadog/cli/CLIHelper.java | 49 ------------------- components/yaml/build.gradle.kts | 1 - .../config/provider/StableConfigParser.java | 19 +++++-- .../provider/StableConfigParserTest.groovy | 2 - 4 files changed, 15 insertions(+), 56 deletions(-) diff --git a/components/cli/src/main/java/datadog/cli/CLIHelper.java b/components/cli/src/main/java/datadog/cli/CLIHelper.java index 7b591d32d3e..0f37953a92f 100644 --- a/components/cli/src/main/java/datadog/cli/CLIHelper.java +++ b/components/cli/src/main/java/datadog/cli/CLIHelper.java @@ -9,65 +9,16 @@ import java.nio.file.Paths; import java.util.Arrays; import java.util.Collections; -import java.util.HashMap; import java.util.List; import java.util.Locale; -import java.util.Map; -/** Helper class for retrieving and parsing JVM arguments. */ public final class CLIHelper { private static final List VM_ARGS = findVmArgs(); - private static final Map VM_ARGS_MAP = new HashMap<>(); - private static int currentIndex = 0; // Tracks the last processed index in VM_ARGS public static List getVmArgs() { return VM_ARGS; } - public static String getArgValue(String key) { - // Lazy population of cache - synchronized (VM_ARGS_MAP) { - String value = VM_ARGS_MAP.get(key); - if (value != null) { - return value; - } - - // Process remaining args - int numArgs = VM_ARGS.size(); - while (currentIndex < numArgs) { - populateCache(VM_ARGS.get(currentIndex)); - currentIndex++; - - // Check if we found our key - value = VM_ARGS_MAP.get(key); - if (value != null) { - return value; - } - } - - return null; - } - } - - private static void populateCache(String arg) { - if (arg.startsWith("-D")) { - int equalsIndex = arg.indexOf('='); - if (equalsIndex >= 0) { - String argKey = arg.substring(0, equalsIndex); - String argValue = arg.substring(equalsIndex + 1); - VM_ARGS_MAP.put(argKey, argValue); - } else { - VM_ARGS_MAP.put(arg, ""); - } - } else { - VM_ARGS_MAP.put(arg, ""); - } - } - - public static boolean argExists(String key) { - return getArgValue(key) != null; - } - @SuppressForbidden private static List findVmArgs() { // Try ProcFS on Linux diff --git a/components/yaml/build.gradle.kts b/components/yaml/build.gradle.kts index d0d80f55015..5acc8dd4cc2 100644 --- a/components/yaml/build.gradle.kts +++ b/components/yaml/build.gradle.kts @@ -11,5 +11,4 @@ jmh { // https://repo1.maven.org/maven2/org/yaml/snakeyaml/2.4/snakeyaml-2.4.pom dependencies { implementation("org.yaml", "snakeyaml", "2.4") - implementation(project(":components:cli")) } 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 50f46dcb11f..cec19fb35bf 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.cli.CLIHelper; import datadog.trace.bootstrap.config.provider.stableconfigyaml.ConfigurationMap; import datadog.trace.bootstrap.config.provider.stableconfigyaml.Rule; import datadog.trace.bootstrap.config.provider.stableconfigyaml.Selector; @@ -169,9 +168,15 @@ static boolean selectorMatch(String origin, List matches, String operato return false; } case "process_arguments": - // For now, always return true if `key` exists in the JVM Args // TODO: flesh out the meaning of each operator for process_arguments - return CLIHelper.argExists(key); + if (!key.startsWith("-D")) { + log.warn( + "Ignoring unsupported process_arguments entry in selector match, '{}'. Only system properties specified with the '-D' prefix are supported.", + key); + return false; + } + // Cut the -D prefix + return System.getProperty(key.substring(2)) != null; case "tags": // TODO: Support this down the line (Must define the source of "tags" first) return false; @@ -236,7 +241,13 @@ private static String processTemplateVar(String templateVar) throws IOException if (processArg.isEmpty()) { throw new IOException("Empty process argument in template"); } - String value = CLIHelper.getArgValue(processArg); + if (!processArg.startsWith("-D")) { + log.warn( + "Ignoring unsupported process_arguments entry in template variable, '{}'. Only system properties specified with the '-D' prefix are supported.", + processArg); + return "UNDEFINED"; + } + String value = System.getProperty(processArg.substring(2)); if (value == null || value.isEmpty()) { return "UNDEFINED"; } 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 2199e9fb9de..f2095e5e93f 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 @@ -1,8 +1,6 @@ package datadog.trace.bootstrap.config.provider - import datadog.trace.test.util.DDSpecification import datadog.trace.test.util.FileUtils - import java.nio.file.Files import java.nio.file.Path From 24b86158accc9c48c5ab98a48e2b38e9f6278d9d Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Thu, 1 May 2025 13:04:39 -0400 Subject: [PATCH 11/15] optimize: return from processTemplate early if no {{ found --- .../bootstrap/config/provider/StableConfigParser.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) 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 cec19fb35bf..0506b6985ce 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 @@ -186,11 +186,16 @@ static boolean selectorMatch(String origin, List matches, String operato } static String processTemplate(String content) throws IOException { + int openIndex = content.indexOf("{{"); + if (openIndex == -1) { + return content; + } + StringBuilder result = new StringBuilder(content.length()); String rest = content; while (true) { - int openIndex = rest.indexOf("{{"); + openIndex = rest.indexOf("{{"); if (openIndex == -1) { result.append(rest); break; From cc769546a3f00fcabe6f6e1eac07864322a655d7 Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Thu, 1 May 2025 14:14:14 -0400 Subject: [PATCH 12/15] Add more test coverage to StableConfigParserTest --- .../provider/StableConfigParserTest.groovy | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) 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 f2095e5e93f..db84e9b2bbd 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 @@ -81,6 +81,8 @@ apm_configuration_rules: "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 @@ -132,6 +134,34 @@ apm_configuration_rules: cfg.get("DD_KEY") == "value_2" } + def "test config_id only"() { + when: + Path filePath = FileUtils.tempFile() + if (filePath == null) { + throw new AssertionError("Failed to create test file") + } + String yaml = """ + config_id: 12345 + """ + try { + FileUtils.writeFileRaw(filePath, yaml) + } catch (IOException e) { + throw new AssertionError("Failed to write to file: ${e.message}") + } + + StableConfigSource.StableConfig cfg + try { + cfg = StableConfigParser.parse(filePath.toString()) + } catch (Exception e) { + throw new AssertionError("Failed to parse the file: ${e.message}") + } + + then: + cfg != null + cfg.getConfigId() == "12345" + cfg.getKeys().size() == 0 + } + def "test parse invalid"() { // If any piece of the file is invalid, the whole file is rendered invalid and an exception is thrown when: From fc4652cc9a8f2a81636b75c34e026308ead4f5f9 Mon Sep 17 00:00:00 2001 From: Stuart McCulloch Date: Fri, 2 May 2025 10:15:17 +0100 Subject: [PATCH 13/15] Optimize template processing to reduce use of substrings --- .../config/provider/StableConfigParser.java | 29 ++++++++++--------- .../provider/StableConfigParserTest.groovy | 2 ++ 2 files changed, 18 insertions(+), 13 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 0506b6985ce..54f5f1a813a 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 @@ -186,32 +186,27 @@ static boolean selectorMatch(String origin, List matches, String operato } static String processTemplate(String content) throws IOException { + // Do nothing if there are no variables to process int openIndex = content.indexOf("{{"); if (openIndex == -1) { return content; } StringBuilder result = new StringBuilder(content.length()); - String rest = content; - while (true) { - openIndex = rest.indexOf("{{"); - if (openIndex == -1) { - result.append(rest); - break; - } + // Add everything before the opening braces + result.append(content, 0, openIndex); - // Add everything before the template - result.append(rest.substring(0, openIndex)); + while (true) { // Find the closing braces - int closeIndex = rest.indexOf("}}", openIndex); + int closeIndex = content.indexOf("}}", openIndex); if (closeIndex == -1) { throw new IOException("Unterminated template in config"); } // Extract the template variable - String templateVar = rest.substring(openIndex + 2, closeIndex).trim(); + String templateVar = content.substring(openIndex + 2, closeIndex).trim(); // Process the template variable and get its value String value = processTemplateVar(templateVar); @@ -219,8 +214,16 @@ static String processTemplate(String content) throws IOException { // Add the processed value result.append(value); - // Continue with the rest of the string - rest = rest.substring(closeIndex + 2); + // Continue with the next template variable + openIndex = content.indexOf("{{", closeIndex); + if (openIndex == -1) { + // Stop and add everything left after the final closing braces + result.append(content, closeIndex + 2, content.length()); + break; + } else { + // Add everything between the last braces and the next + result.append(content, closeIndex + 2, openIndex); + } } return result.toString(); 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 db84e9b2bbd..f5c6b8a3c7c 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 @@ -223,6 +223,8 @@ apm_configuration_rules: "{}" | 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" } def "test processTemplate error cases"() { From e633f267cd8e953431c0d7473eeb2090ad54900b Mon Sep 17 00:00:00 2001 From: Stuart McCulloch Date: Fri, 2 May 2025 10:25:52 +0100 Subject: [PATCH 14/15] Introduce constants for repeated strings --- .../config/provider/StableConfigParser.java | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 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 54f5f1a813a..3721528f81f 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 @@ -19,6 +19,10 @@ public class StableConfigParser { private static final Logger log = LoggerFactory.getLogger(StableConfigParser.class); + private static final String ENVIRONMENT_VARIABLES_PREFIX = "environment_variables['"; + private static final String PROCESS_ARGUMENTS_PREFIX = "process_arguments['"; + private static final String UNDEFINED_VALUE = "UNDEFINED"; + /** * Parses a configuration file and returns a stable configuration object. * @@ -230,22 +234,22 @@ static String processTemplate(String content) throws IOException { } private static String processTemplateVar(String templateVar) throws IOException { - if (templateVar.startsWith("environment_variables['") && templateVar.endsWith("']")) { + if (templateVar.startsWith(ENVIRONMENT_VARIABLES_PREFIX) && templateVar.endsWith("']")) { String envVar = templateVar - .substring("environment_variables['".length(), templateVar.length() - 2) + .substring(ENVIRONMENT_VARIABLES_PREFIX.length(), templateVar.length() - 2) .trim(); if (envVar.isEmpty()) { throw new IOException("Empty environment variable name in template"); } String value = System.getenv(envVar.toUpperCase()); if (value == null || value.isEmpty()) { - return "UNDEFINED"; + return UNDEFINED_VALUE; } return value; - } else if (templateVar.startsWith("process_arguments['") && templateVar.endsWith("']")) { + } else if (templateVar.startsWith(PROCESS_ARGUMENTS_PREFIX) && templateVar.endsWith("']")) { String processArg = - templateVar.substring("process_arguments['".length(), templateVar.length() - 2).trim(); + templateVar.substring(PROCESS_ARGUMENTS_PREFIX.length(), templateVar.length() - 2).trim(); if (processArg.isEmpty()) { throw new IOException("Empty process argument in template"); } @@ -253,14 +257,15 @@ private static String processTemplateVar(String templateVar) throws IOException log.warn( "Ignoring unsupported process_arguments entry in template variable, '{}'. Only system properties specified with the '-D' prefix are supported.", processArg); - return "UNDEFINED"; + return UNDEFINED_VALUE; } String value = System.getProperty(processArg.substring(2)); if (value == null || value.isEmpty()) { - return "UNDEFINED"; + return UNDEFINED_VALUE; } return value; + } else { + return UNDEFINED_VALUE; } - return "UNDEFINED"; } } From 3c8b646f0fe490d80e5fcde7a4175af5862596ce Mon Sep 17 00:00:00 2001 From: Mikayla Toffler Date: Mon, 5 May 2025 10:25:48 -0400 Subject: [PATCH 15/15] Remove FileUtils and all its references --- .../provider/StableConfigParserTest.groovy | 17 +++++++------- .../provider/StableConfigSourceTest.groovy | 7 +++--- .../datadog/trace/test/util/FileUtils.groovy | 23 ------------------- 3 files changed, 11 insertions(+), 36 deletions(-) delete mode 100644 utils/test-utils/src/main/groovy/datadog/trace/test/util/FileUtils.groovy 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 f5c6b8a3c7c..686648471a1 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 @@ -1,13 +1,12 @@ package datadog.trace.bootstrap.config.provider import datadog.trace.test.util.DDSpecification -import datadog.trace.test.util.FileUtils import java.nio.file.Files import java.nio.file.Path class StableConfigParserTest extends DDSpecification { def "test parse valid"() { when: - Path filePath = FileUtils.tempFile() + Path filePath = Files.createTempFile("testFile_", ".yaml") if (filePath == null) { throw new AssertionError("Failed to create test file") } @@ -41,7 +40,7 @@ apm_configuration_rules: KEY_FOUR: "ignored" """ try { - FileUtils.writeFileRaw(filePath, yaml) + Files.write(filePath, yaml.getBytes()) } catch (IOException e) { throw new AssertionError("Failed to write to file: ${e.message}") } @@ -102,7 +101,7 @@ apm_configuration_rules: def "test duplicate entries"() { // When duplicate keys are encountered, snakeyaml preserves the last value by default when: - Path filePath = FileUtils.tempFile() + Path filePath = Files.createTempFile("testFile_", ".yaml") if (filePath == null) { throw new AssertionError("Failed to create test file") } @@ -116,7 +115,7 @@ apm_configuration_rules: """ try { - FileUtils.writeFileRaw(filePath, yaml) + Files.write(filePath, yaml.getBytes()) } catch (IOException e) { throw new AssertionError("Failed to write to file: ${e.message}") } @@ -136,7 +135,7 @@ apm_configuration_rules: def "test config_id only"() { when: - Path filePath = FileUtils.tempFile() + Path filePath = Files.createTempFile("testFile_", ".yaml") if (filePath == null) { throw new AssertionError("Failed to create test file") } @@ -144,7 +143,7 @@ apm_configuration_rules: config_id: 12345 """ try { - FileUtils.writeFileRaw(filePath, yaml) + Files.write(filePath, yaml.getBytes()) } catch (IOException e) { throw new AssertionError("Failed to write to file: ${e.message}") } @@ -165,7 +164,7 @@ apm_configuration_rules: def "test parse invalid"() { // If any piece of the file is invalid, the whole file is rendered invalid and an exception is thrown when: - Path filePath = FileUtils.tempFile() + Path filePath = Files.createTempFile("testFile_", ".yaml") if (filePath == null) { throw new AssertionError("Failed to create test file") } @@ -187,7 +186,7 @@ apm_configuration_rules: something-else-irrelevant: value-irrelevant """ try { - FileUtils.writeFileRaw(filePath, yaml) + Files.write(filePath, yaml.getBytes()) } catch (IOException e) { throw new AssertionError("Failed to write to file: ${e.message}") } 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 dc431aba755..f9a6281f337 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,7 +9,6 @@ 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.test.util.DDSpecification -import datadog.trace.test.util.FileUtils import org.yaml.snakeyaml.DumperOptions import org.yaml.snakeyaml.Yaml import org.yaml.snakeyaml.introspector.Property @@ -34,7 +33,7 @@ class StableConfigSourceTest extends DDSpecification { def "test empty file"() { when: - Path filePath = FileUtils.tempFile() + Path filePath = Files.createTempFile("testFile_", ".yaml") if (filePath == null) { throw new AssertionError("Failed to create test file") } @@ -48,7 +47,7 @@ class StableConfigSourceTest extends DDSpecification { def "test file invalid format"() { // StableConfigSource must handle the exception thrown by StableConfigParser.parse(filePath) gracefully when: - Path filePath = FileUtils.tempFile() + Path filePath = Files.createTempFile("testFile_", ".yaml") if (filePath == null) { throw new AssertionError("Failed to create test file") } @@ -74,7 +73,7 @@ class StableConfigSourceTest extends DDSpecification { def "test file valid format"() { when: - Path filePath = FileUtils.tempFile() + Path filePath = Files.createTempFile("testFile_", ".yaml") if (filePath == null) { throw new AssertionError("Failed to create test file") } diff --git a/utils/test-utils/src/main/groovy/datadog/trace/test/util/FileUtils.groovy b/utils/test-utils/src/main/groovy/datadog/trace/test/util/FileUtils.groovy deleted file mode 100644 index d82c0fd1585..00000000000 --- a/utils/test-utils/src/main/groovy/datadog/trace/test/util/FileUtils.groovy +++ /dev/null @@ -1,23 +0,0 @@ -package datadog.trace.test.util - -import java.nio.file.Files -import java.nio.file.Path -import java.nio.file.StandardOpenOption - -class FileUtils { - // Use this for writing a string directly into a file - static writeFileRaw(Path filePath, String data) { - StandardOpenOption[] openOpts = [StandardOpenOption.WRITE] as StandardOpenOption[] - Files.write(filePath, data.getBytes(), openOpts) - } - - static Path tempFile() { - try { - return Files.createTempFile("testFile_", ".yaml") - } catch (IOException e) { - println "Error creating file: ${e.message}" - e.printStackTrace() - return null - } - } -}