Skip to content

Commit 7169626

Browse files
mtoffl01mcculls
mtoffl01
and
mcculls
authored
Stable Config file: target system properties in process_arguments and support template variables in YamlParser (#8690)
* migrate CLIHelper to use a Map, parse system properties in key-vals * Fix processTemplateVar logic to remove superfluous ':' * nits: javadocs * Move writeFileRaw helper to testutils * Add YamlParser tests * Revert CLIHelper to use List<String>; lazily load a Map cache for querying key-vals * reuse logic for adding to the vm args cache * apply github suggestions * Move processTemplate yaml helper fns into StableConfigParser, along with tests * Remove changes to CLIHelper; rely on System.getProperty instead * optimize: return from processTemplate early if no {{ found * Add more test coverage to StableConfigParserTest * Optimize template processing to reduce use of substrings * Introduce constants for repeated strings * Remove FileUtils and all its references --------- Co-authored-by: Stuart McCulloch <[email protected]>
1 parent 0e18e0e commit 7169626

File tree

4 files changed

+179
-46
lines changed

4 files changed

+179
-46
lines changed

‎components/yaml/src/main/java/datadog/yaml/YamlParser.java‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,15 @@
11
package datadog.yaml;
22

3-
import java.io.FileInputStream;
4-
import java.io.IOException;
53
import org.yaml.snakeyaml.Yaml;
64

75
public class YamlParser {
86
// Supports clazz == null for default yaml parsing
9-
public static <T> T parse(String filePath, Class<T> clazz) throws IOException {
7+
public static <T> T parse(String content, Class<T> clazz) {
108
Yaml yaml = new Yaml();
11-
try (FileInputStream fis = new FileInputStream(filePath)) {
12-
if (clazz == null) {
13-
return yaml.load(fis);
14-
} else {
15-
return yaml.loadAs(fis, clazz);
16-
}
9+
if (clazz == null) {
10+
return yaml.load(content);
11+
} else {
12+
return yaml.loadAs(content, clazz);
1713
}
1814
}
1915
}

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

Lines changed: 100 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,27 @@
11
package datadog.trace.bootstrap.config.provider;
22

3-
import datadog.cli.CLIHelper;
43
import datadog.trace.bootstrap.config.provider.stableconfigyaml.ConfigurationMap;
54
import datadog.trace.bootstrap.config.provider.stableconfigyaml.Rule;
65
import datadog.trace.bootstrap.config.provider.stableconfigyaml.Selector;
76
import datadog.trace.bootstrap.config.provider.stableconfigyaml.StableConfigYaml;
87
import datadog.yaml.YamlParser;
98
import java.io.IOException;
9+
import java.nio.charset.StandardCharsets;
10+
import java.nio.file.Files;
11+
import java.nio.file.Paths;
1012
import java.util.Collections;
1113
import java.util.HashMap;
12-
import java.util.HashSet;
1314
import java.util.List;
14-
import java.util.Set;
1515
import java.util.function.BiPredicate;
1616
import org.slf4j.Logger;
1717
import org.slf4j.LoggerFactory;
1818

1919
public class StableConfigParser {
2020
private static final Logger log = LoggerFactory.getLogger(StableConfigParser.class);
2121

22-
private static final Set<String> VM_ARGS = new HashSet<>(CLIHelper.getVmArgs());
22+
private static final String ENVIRONMENT_VARIABLES_PREFIX = "environment_variables['";
23+
private static final String PROCESS_ARGUMENTS_PREFIX = "process_arguments['";
24+
private static final String UNDEFINED_VALUE = "UNDEFINED";
2325

2426
/**
2527
* Parses a configuration file and returns a stable configuration object.
@@ -37,7 +39,9 @@ public class StableConfigParser {
3739
*/
3840
public static StableConfigSource.StableConfig parse(String filePath) throws IOException {
3941
try {
40-
StableConfigYaml data = YamlParser.parse(filePath, StableConfigYaml.class);
42+
String content = new String(Files.readAllBytes(Paths.get(filePath)), StandardCharsets.UTF_8);
43+
String processedContent = processTemplate(content);
44+
StableConfigYaml data = YamlParser.parse(processedContent, StableConfigYaml.class);
4145

4246
String configId = data.getConfig_id();
4347
ConfigurationMap configMap = data.getApm_configuration_default();
@@ -66,7 +70,9 @@ public static StableConfigSource.StableConfig parse(String filePath) throws IOEx
6670

6771
} catch (IOException e) {
6872
log.debug(
69-
"Stable configuration file either not found or not readable at filepath {}", filePath);
73+
"Stable configuration file either not found or not readable at filepath {}. Error: {}",
74+
filePath,
75+
e.getMessage());
7076
}
7177
return StableConfigSource.StableConfig.EMPTY;
7278
}
@@ -166,14 +172,100 @@ static boolean selectorMatch(String origin, List<String> matches, String operato
166172
return false;
167173
}
168174
case "process_arguments":
169-
// For now, always return true if `key` exists in the JVM Args
170175
// TODO: flesh out the meaning of each operator for process_arguments
171-
return VM_ARGS.contains(key);
176+
if (!key.startsWith("-D")) {
177+
log.warn(
178+
"Ignoring unsupported process_arguments entry in selector match, '{}'. Only system properties specified with the '-D' prefix are supported.",
179+
key);
180+
return false;
181+
}
182+
// Cut the -D prefix
183+
return System.getProperty(key.substring(2)) != null;
172184
case "tags":
173185
// TODO: Support this down the line (Must define the source of "tags" first)
174186
return false;
175187
default:
176188
return false;
177189
}
178190
}
191+
192+
static String processTemplate(String content) throws IOException {
193+
// Do nothing if there are no variables to process
194+
int openIndex = content.indexOf("{{");
195+
if (openIndex == -1) {
196+
return content;
197+
}
198+
199+
StringBuilder result = new StringBuilder(content.length());
200+
201+
// Add everything before the opening braces
202+
result.append(content, 0, openIndex);
203+
204+
while (true) {
205+
206+
// Find the closing braces
207+
int closeIndex = content.indexOf("}}", openIndex);
208+
if (closeIndex == -1) {
209+
throw new IOException("Unterminated template in config");
210+
}
211+
212+
// Extract the template variable
213+
String templateVar = content.substring(openIndex + 2, closeIndex).trim();
214+
215+
// Process the template variable and get its value
216+
String value = processTemplateVar(templateVar);
217+
218+
// Add the processed value
219+
result.append(value);
220+
221+
// Continue with the next template variable
222+
openIndex = content.indexOf("{{", closeIndex);
223+
if (openIndex == -1) {
224+
// Stop and add everything left after the final closing braces
225+
result.append(content, closeIndex + 2, content.length());
226+
break;
227+
} else {
228+
// Add everything between the last braces and the next
229+
result.append(content, closeIndex + 2, openIndex);
230+
}
231+
}
232+
233+
return result.toString();
234+
}
235+
236+
private static String processTemplateVar(String templateVar) throws IOException {
237+
if (templateVar.startsWith(ENVIRONMENT_VARIABLES_PREFIX) && templateVar.endsWith("']")) {
238+
String envVar =
239+
templateVar
240+
.substring(ENVIRONMENT_VARIABLES_PREFIX.length(), templateVar.length() - 2)
241+
.trim();
242+
if (envVar.isEmpty()) {
243+
throw new IOException("Empty environment variable name in template");
244+
}
245+
String value = System.getenv(envVar.toUpperCase());
246+
if (value == null || value.isEmpty()) {
247+
return UNDEFINED_VALUE;
248+
}
249+
return value;
250+
} else if (templateVar.startsWith(PROCESS_ARGUMENTS_PREFIX) && templateVar.endsWith("']")) {
251+
String processArg =
252+
templateVar.substring(PROCESS_ARGUMENTS_PREFIX.length(), templateVar.length() - 2).trim();
253+
if (processArg.isEmpty()) {
254+
throw new IOException("Empty process argument in template");
255+
}
256+
if (!processArg.startsWith("-D")) {
257+
log.warn(
258+
"Ignoring unsupported process_arguments entry in template variable, '{}'. Only system properties specified with the '-D' prefix are supported.",
259+
processArg);
260+
return UNDEFINED_VALUE;
261+
}
262+
String value = System.getProperty(processArg.substring(2));
263+
if (value == null || value.isEmpty()) {
264+
return UNDEFINED_VALUE;
265+
}
266+
return value;
267+
} else {
268+
return UNDEFINED_VALUE;
269+
}
270+
}
179271
}

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

Lines changed: 71 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,12 @@
11
package datadog.trace.bootstrap.config.provider
2-
32
import datadog.trace.test.util.DDSpecification
4-
53
import java.nio.file.Files
64
import java.nio.file.Path
75

86
class StableConfigParserTest extends DDSpecification {
97
def "test parse valid"() {
108
when:
11-
Path filePath = StableConfigSourceTest.tempFile()
9+
Path filePath = Files.createTempFile("testFile_", ".yaml")
1210
if (filePath == null) {
1311
throw new AssertionError("Failed to create test file")
1412
}
@@ -42,7 +40,7 @@ apm_configuration_rules:
4240
KEY_FOUR: "ignored"
4341
"""
4442
try {
45-
StableConfigSourceTest.writeFileRaw(filePath, yaml)
43+
Files.write(filePath, yaml.getBytes())
4644
} catch (IOException e) {
4745
throw new AssertionError("Failed to write to file: ${e.message}")
4846
}
@@ -82,6 +80,8 @@ apm_configuration_rules:
8280
"language" | ["java", "golang"] | "equals" | "" | true
8381
"language" | ["java"] | "starts_with" | "" | true
8482
"language" | ["golang"] | "equals" | "" | false
83+
"language" | ["java"] | "exists" | "" | false
84+
"language" | ["java"] | "something unexpected" | "" | false
8585
"environment_variables" | [] | "exists" | "DD_TAGS" | true
8686
"environment_variables" | ["team:apm"] | "contains" | "DD_TAGS" | true
8787
"ENVIRONMENT_VARIABLES" | ["TeAm:ApM"] | "CoNtAiNs" | "Dd_TaGs" | true // check case insensitivity
@@ -96,13 +96,12 @@ apm_configuration_rules:
9696
"environment_variables" | ["svc"] | "contains" | "DD_SERVICE" | true
9797
"environment_variables" | ["other"] | "contains" | "DD_SERVICE" | false
9898
"environment_variables" | [null] | "contains" | "DD_SERVICE" | false
99-
// "process_arguments" | null | "equals" | "-DCustomKey" | true
10099
}
101100

102101
def "test duplicate entries"() {
103102
// When duplicate keys are encountered, snakeyaml preserves the last value by default
104103
when:
105-
Path filePath = StableConfigSourceTest.tempFile()
104+
Path filePath = Files.createTempFile("testFile_", ".yaml")
106105
if (filePath == null) {
107106
throw new AssertionError("Failed to create test file")
108107
}
@@ -116,7 +115,7 @@ apm_configuration_rules:
116115
"""
117116

118117
try {
119-
StableConfigSourceTest.writeFileRaw(filePath, yaml)
118+
Files.write(filePath, yaml.getBytes())
120119
} catch (IOException e) {
121120
throw new AssertionError("Failed to write to file: ${e.message}")
122121
}
@@ -134,10 +133,38 @@ apm_configuration_rules:
134133
cfg.get("DD_KEY") == "value_2"
135134
}
136135

136+
def "test config_id only"() {
137+
when:
138+
Path filePath = Files.createTempFile("testFile_", ".yaml")
139+
if (filePath == null) {
140+
throw new AssertionError("Failed to create test file")
141+
}
142+
String yaml = """
143+
config_id: 12345
144+
"""
145+
try {
146+
Files.write(filePath, yaml.getBytes())
147+
} catch (IOException e) {
148+
throw new AssertionError("Failed to write to file: ${e.message}")
149+
}
150+
151+
StableConfigSource.StableConfig cfg
152+
try {
153+
cfg = StableConfigParser.parse(filePath.toString())
154+
} catch (Exception e) {
155+
throw new AssertionError("Failed to parse the file: ${e.message}")
156+
}
157+
158+
then:
159+
cfg != null
160+
cfg.getConfigId() == "12345"
161+
cfg.getKeys().size() == 0
162+
}
163+
137164
def "test parse invalid"() {
138165
// If any piece of the file is invalid, the whole file is rendered invalid and an exception is thrown
139166
when:
140-
Path filePath = StableConfigSourceTest.tempFile()
167+
Path filePath = Files.createTempFile("testFile_", ".yaml")
141168
if (filePath == null) {
142169
throw new AssertionError("Failed to create test file")
143170
}
@@ -159,7 +186,7 @@ apm_configuration_rules:
159186
something-else-irrelevant: value-irrelevant
160187
"""
161188
try {
162-
StableConfigSourceTest.writeFileRaw(filePath, yaml)
189+
Files.write(filePath, yaml.getBytes())
163190
} catch (IOException e) {
164191
throw new AssertionError("Failed to write to file: ${e.message}")
165192
}
@@ -177,4 +204,39 @@ apm_configuration_rules:
177204
cfg == null
178205
Files.delete(filePath)
179206
}
207+
208+
def "test processTemplate valid cases"() {
209+
when:
210+
if (envKey != null) {
211+
injectEnvConfig(envKey, envVal)
212+
}
213+
214+
then:
215+
StableConfigParser.processTemplate(templateVar) == expect
216+
217+
where:
218+
templateVar | envKey | envVal | expect
219+
"{{environment_variables['DD_KEY']}}" | "DD_KEY" | "value" | "value"
220+
"{{environment_variables['DD_KEY']}}" | null | null | "UNDEFINED"
221+
"{{}}" | null | null | "UNDEFINED"
222+
"{}" | null | null | "{}"
223+
"{{environment_variables['dd_key']}}" | "DD_KEY" | "value" | "value"
224+
"{{environment_variables['DD_KEY}}" | "DD_KEY" | "value" | "UNDEFINED"
225+
"header-{{environment_variables['DD_KEY']}}-footer" | "DD_KEY" | "value" | "header-value-footer"
226+
"{{environment_variables['HEADER']}}{{environment_variables['DD_KEY']}}{{environment_variables['FOOTER']}}" | "DD_KEY" | "value" | "UNDEFINEDvalueUNDEFINED"
227+
}
228+
229+
def "test processTemplate error cases"() {
230+
when:
231+
StableConfigParser.processTemplate(templateVar)
232+
233+
then:
234+
def e = thrown(IOException)
235+
e.message == expect
236+
237+
where:
238+
templateVar | expect
239+
"{{environment_variables['']}}" | "Empty environment variable name in template"
240+
"{{environment_variables['DD_KEY']}" | "Unterminated template in config"
241+
}
180242
}

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

Lines changed: 3 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ class StableConfigSourceTest extends DDSpecification {
3333

3434
def "test empty file"() {
3535
when:
36-
Path filePath = tempFile()
36+
Path filePath = Files.createTempFile("testFile_", ".yaml")
3737
if (filePath == null) {
3838
throw new AssertionError("Failed to create test file")
3939
}
@@ -47,7 +47,7 @@ class StableConfigSourceTest extends DDSpecification {
4747
def "test file invalid format"() {
4848
// StableConfigSource must handle the exception thrown by StableConfigParser.parse(filePath) gracefully
4949
when:
50-
Path filePath = tempFile()
50+
Path filePath = Files.createTempFile("testFile_", ".yaml")
5151
if (filePath == null) {
5252
throw new AssertionError("Failed to create test file")
5353
}
@@ -73,7 +73,7 @@ class StableConfigSourceTest extends DDSpecification {
7373

7474
def "test file valid format"() {
7575
when:
76-
Path filePath = tempFile()
76+
Path filePath = Files.createTempFile("testFile_", ".yaml")
7777
if (filePath == null) {
7878
throw new AssertionError("Failed to create test file")
7979
}
@@ -133,16 +133,6 @@ class StableConfigSourceTest extends DDSpecification {
133133
def static sampleNonMatchingRule = new Rule(Arrays.asList(new Selector("origin", "language", Arrays.asList("Golang"), null)), new ConfigurationMap(singletonMap("DD_KEY_FOUR", new ConfigurationValue("four"))))
134134

135135
// Helper functions
136-
static Path tempFile() {
137-
try {
138-
return Files.createTempFile("testFile_", ".yaml")
139-
} catch (IOException e) {
140-
println "Error creating file: ${e.message}"
141-
e.printStackTrace()
142-
return null // or throw new RuntimeException("File creation failed", e)
143-
}
144-
}
145-
146136
def stableConfigYamlWriter = getStableConfigYamlWriter()
147137

148138
Yaml getStableConfigYamlWriter() {
@@ -166,7 +156,6 @@ class StableConfigSourceTest extends DDSpecification {
166156
}
167157

168158
def writeFileYaml(Path filePath, StableConfigYaml stableConfigs) {
169-
// Yaml yaml = getStableConfigYaml();
170159
try (FileWriter writer = new FileWriter(filePath.toString())) {
171160
stableConfigYamlWriter.dump(stableConfigs, writer)
172161
} catch (IOException e) {
@@ -180,10 +169,4 @@ class StableConfigSourceTest extends DDSpecification {
180169
StandardOpenOption[] openOpts = [StandardOpenOption.WRITE] as StandardOpenOption[]
181170
Files.write(filePath, data.getBytes(), openOpts)
182171
}
183-
184-
// Use this for writing a string directly into a file
185-
static writeFileRaw(Path filePath, String data) {
186-
StandardOpenOption[] openOpts = [StandardOpenOption.WRITE] as StandardOpenOption[]
187-
Files.write(filePath, data.getBytes(), openOpts)
188-
}
189172
}

0 commit comments

Comments
 (0)