Skip to content

Commit 06d572d

Browse files
committed
Introduce StableConfigSource to the list of ConfigProvider sources
1 parent 9f03e04 commit 06d572d

File tree

3 files changed

+155
-78
lines changed

3 files changed

+155
-78
lines changed

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

+23-4
Original file line numberDiff line numberDiff line change
@@ -353,19 +353,29 @@ public static ConfigProvider createDefault() {
353353
Properties configProperties =
354354
loadConfigurationFile(
355355
new ConfigProvider(new SystemPropertiesConfigSource(), new EnvironmentConfigSource()));
356+
// MIKAYLA: What is the significance of configProperties, of this isEmpty check?
356357
if (configProperties.isEmpty()) {
357358
return new ConfigProvider(
358359
new SystemPropertiesConfigSource(),
360+
new StableConfigSource(
361+
StableConfigSource.MANAGED_STABLE_CONFIG_PATH, ConfigOrigin.MANAGED_STABLE_CONFIG),
359362
new EnvironmentConfigSource(),
360363
new OtelEnvironmentConfigSource(),
361-
new CapturedEnvironmentConfigSource());
364+
new CapturedEnvironmentConfigSource(), // MIKAYLA: what is
365+
// CapturedEnvironmentConfigSource?
366+
new StableConfigSource(
367+
StableConfigSource.USER_STABLE_CONFIG_PATH, ConfigOrigin.USER_STABLE_CONFIG));
362368
} else {
363369
return new ConfigProvider(
364370
new SystemPropertiesConfigSource(),
371+
new StableConfigSource(
372+
StableConfigSource.MANAGED_STABLE_CONFIG_PATH, ConfigOrigin.MANAGED_STABLE_CONFIG),
365373
new EnvironmentConfigSource(),
366374
new PropertiesConfigSource(configProperties, true),
367375
new OtelEnvironmentConfigSource(configProperties),
368-
new CapturedEnvironmentConfigSource());
376+
new CapturedEnvironmentConfigSource(),
377+
new StableConfigSource(
378+
StableConfigSource.USER_STABLE_CONFIG_PATH, ConfigOrigin.USER_STABLE_CONFIG));
369379
}
370380
}
371381

@@ -378,20 +388,29 @@ public static ConfigProvider withoutCollector() {
378388
return new ConfigProvider(
379389
false,
380390
new SystemPropertiesConfigSource(),
391+
new StableConfigSource(
392+
StableConfigSource.MANAGED_STABLE_CONFIG_PATH, ConfigOrigin.MANAGED_STABLE_CONFIG),
381393
new EnvironmentConfigSource(),
382394
new OtelEnvironmentConfigSource(),
383-
new CapturedEnvironmentConfigSource());
395+
new CapturedEnvironmentConfigSource(),
396+
new StableConfigSource(
397+
StableConfigSource.USER_STABLE_CONFIG_PATH, ConfigOrigin.USER_STABLE_CONFIG));
384398
} else {
385399
return new ConfigProvider(
386400
false,
387401
new SystemPropertiesConfigSource(),
402+
new StableConfigSource(
403+
StableConfigSource.MANAGED_STABLE_CONFIG_PATH, ConfigOrigin.MANAGED_STABLE_CONFIG),
388404
new EnvironmentConfigSource(),
389405
new PropertiesConfigSource(configProperties, true),
390406
new OtelEnvironmentConfigSource(configProperties),
391-
new CapturedEnvironmentConfigSource());
407+
new CapturedEnvironmentConfigSource(),
408+
new StableConfigSource(
409+
StableConfigSource.USER_STABLE_CONFIG_PATH, ConfigOrigin.USER_STABLE_CONFIG));
392410
}
393411
}
394412

413+
// MIKAYLA: What is providedConfigSource, and how should it stand up against stableconfig?
395414
public static ConfigProvider withPropertiesOverride(Properties properties) {
396415
PropertiesConfigSource providedConfigSource = new PropertiesConfigSource(properties, false);
397416
Properties configProperties =

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

+27-26
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package datadog.trace.bootstrap.config.provider;
22

3+
import static datadog.trace.util.Strings.propertyNameToEnvironmentVariableName;
4+
35
import datadog.trace.api.ConfigOrigin;
46
import java.io.File;
57
import java.io.IOException;
@@ -20,29 +22,40 @@ public final class StableConfigSource extends ConfigProvider.Source {
2022
"/etc/datadog-agent/managed/datadog-apm-libraries/stable/application_monitoring.yaml ";
2123
private static final Logger log = LoggerFactory.getLogger(StableConfigSource.class);
2224

23-
final ConfigOrigin fileOrigin;
24-
Map<String, Object> configuration;
25-
String configId;
25+
private final ConfigOrigin fileOrigin;
26+
private final Map<String, Object> configuration;
27+
private final String configId;
2628

27-
StableConfigSource(String file, ConfigOrigin origin) throws IOException {
29+
StableConfigSource(String file, ConfigOrigin origin) {
2830
this.fileOrigin = origin;
29-
HashMap<String, Object> data = readFile(file);
31+
HashMap<String, Object> data = readYamlFromFile(file);
3032
if (data == null) {
3133
this.configuration = new HashMap<>();
3234
this.configId = null;
3335
} else {
34-
this.configId = (String) data.get("config_id"); // could be ""
36+
this.configId = (String) data.get("config_id");
3537
this.configuration = parseStableConfig(data);
3638
}
3739
}
3840

39-
private static HashMap<String, Object> readFile(String filePath) {
40-
// Check if the file exists
41+
/**
42+
* Reads configuration data from the YAML file located at the specified file path.
43+
*
44+
* <p>If the file is in a valid YAML format, this method returns a {@link HashMap} containing the
45+
* configuration information.
46+
*
47+
* <p>If the file is in an invalid format, the method returns <code>null</code>.
48+
*
49+
* @param filePath The path to the YAML file to be read.
50+
* @return A {@link HashMap} containing the configuration data if the file is valid, or <code>null
51+
* </code> if the file is in an invalid format.
52+
*/
53+
private static HashMap<String, Object> readYamlFromFile(String filePath) {
4154
File file = new File(filePath);
4255
if (!file.exists()) {
4356
log.debug(
4457
"Stable configuration file does not exist at the specified path: {}, ignoring", filePath);
45-
return new HashMap<>();
58+
return null;
4659
}
4760

4861
Yaml yaml = new Yaml(new SafeConstructor(new LoaderOptions()));
@@ -52,20 +65,21 @@ private static HashMap<String, Object> readFile(String filePath) {
5265
} catch (IOException e) {
5366
// throw new RuntimeException(e); // Do we want to do this? Or fail more gracefully?
5467
log.error("Unable to read from stable config file {}, dropping input", filePath);
55-
return new HashMap<>();
68+
return null;
5669
}
5770
try {
5871
return yaml.load(input);
5972
} catch (Exception e) {
6073
log.error("YAML parsing error in stable config file {}: {}", filePath, e.getMessage());
61-
return new HashMap<>();
74+
return null;
6275
}
6376
}
6477

6578
private static Map<String, Object> parseStableConfig(HashMap<String, Object> data) {
6679
HashMap<String, Object> config = new HashMap<>();
6780
Object apmConfig = data.get("apm_configuration_default");
6881
if (apmConfig == null) {
82+
6983
return config;
7084
}
7185
if (apmConfig instanceof HashMap<?, ?>) {
@@ -76,7 +90,7 @@ private static Map<String, Object> parseStableConfig(HashMap<String, Object> dat
7690
Object value = entry.getValue();
7791
config.put(key, value);
7892
} else {
79-
log.debug("Configuration {} in unexpected format", entry.getKey());
93+
log.debug("Config key {} in unexpected format", entry.getKey());
8094
}
8195
}
8296
} else {
@@ -88,7 +102,7 @@ private static Map<String, Object> parseStableConfig(HashMap<String, Object> dat
88102

89103
@Override
90104
public String get(String key) {
91-
return (String) configuration.get(key);
105+
return (String) this.configuration.get(propertyNameToEnvironmentVariableName(key));
92106
}
93107

94108
@Override
@@ -103,17 +117,4 @@ public Set<String> getKeys() {
103117
public String getConfigId() {
104118
return this.configId;
105119
}
106-
107-
// private static class StableConfig {
108-
// private String config_id;
109-
// private Map<String, Object> apm_configuration_default;
110-
//
111-
// private void setApmConfigurationDefault(HashMap<String, Object> m) {
112-
// apm_configuration_default = m;
113-
// }
114-
//
115-
// private void setConfigId(String i) {
116-
// config_id = i;
117-
// }
118-
// }
119120
}

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

+105-48
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import datadog.trace.api.ConfigOrigin
44
import datadog.trace.test.util.DDSpecification
55
import org.yaml.snakeyaml.DumperOptions
66
import org.yaml.snakeyaml.Yaml
7+
import spock.lang.Shared
78

89
import java.nio.file.Path
910
import java.nio.file.Files
@@ -18,7 +19,6 @@ class StableConfigSourceTest extends DDSpecification {
1819
expect:
1920
config.getKeys().size() == 0
2021
config.getConfigId() == null
21-
// How to check that the "file does not exist" error was logged?
2222
}
2323

2424
def "test empty file"() {
@@ -44,75 +44,132 @@ class StableConfigSourceTest extends DDSpecification {
4444
config.getConfigId() == null
4545
}
4646

47-
def "test populated file"() {
47+
def "test get"() {
4848
when:
49-
Path filePath
50-
StableConfigSource stableCfg
49+
Path filePath = tempFile()
50+
if (filePath == null) {
51+
return // fail?
52+
}
53+
54+
def configs = new HashMap<>() << ["DD_SERVICE": "svc", "DD_ENV": "env", "CONFIG_NO_DD": "value123"]
55+
5156
try {
52-
filePath = Files.createTempFile("testFile_", ".yaml")
57+
writeFileYaml(filePath, "12345", configs)
5358
} catch (IOException e) {
54-
println "Error creating file: ${e.message}"
55-
e.printStackTrace()
56-
return // or throw new RuntimeException("File creation failed", e)
59+
println "Error writing to file: ${e.message}"
60+
return // fail?
5761
}
62+
63+
StableConfigSource cfg = new StableConfigSource(filePath.toString(), ConfigOrigin.USER_STABLE_CONFIG)
64+
65+
then:
66+
cfg.getKeys().size() == 3
67+
cfg.get("service") == "svc"
68+
cfg.get("env") == "env"
69+
cfg.get("config_no_dd") == null
70+
cfg.get("config_nonexistent") == null
71+
}
72+
73+
def "test file invalid format"() {
74+
when:
75+
Path filePath = tempFile()
5876
if (filePath == null) {
59-
return
77+
return // fail?
6078
}
6179

62-
DumperOptions options = new DumperOptions()
63-
options.setDefaultFlowStyle(DumperOptions.FlowStyle.BLOCK)
80+
try {
81+
writeFileRaw(filePath, configId, configs)
82+
} catch (IOException e) {
83+
println "Error writing to file: ${e.message}"
84+
return // fail?
85+
}
6486

65-
// Prepare to write the data map to the file in yaml format
66-
Yaml yaml = new Yaml(options)
67-
String yamlString
68-
if (corrupt == true) {
69-
yamlString = '''
70-
abc: 123
71-
def
72-
ghi "jkl"
73-
lmn: 456
74-
'''
75-
} else {
76-
Map<String, Object> data = new HashMap<>()
77-
if (configId != null) {
78-
data.put("config_id", configId)
79-
}
80-
if (configs != null) {
81-
data.put("apm_configuration_default", configs)
82-
}
83-
84-
yamlString = yaml.dump(data)
87+
StableConfigSource stableCfg = new StableConfigSource(filePath.toString(), ConfigOrigin.USER_STABLE_CONFIG)
88+
89+
then:
90+
stableCfg.getConfigId() == null
91+
stableCfg.getKeys().size() == 0
92+
Files.delete(filePath)
93+
94+
where:
95+
configId | configs
96+
null | corruptYaml
97+
"12345" | "this is not yaml format!"
98+
}
99+
100+
def "test file valid format"() {
101+
when:
102+
Path filePath = tempFile()
103+
if (filePath == null) {
104+
return // fail?
85105
}
86106

87107
try {
88-
StandardOpenOption[] openOpts = [StandardOpenOption.WRITE] as StandardOpenOption[]
89-
Files.write(filePath, yamlString.getBytes(), openOpts)
90-
println "YAML written to: $filePath"
108+
writeFileYaml(filePath, configId, configs)
91109
} catch (IOException e) {
92110
println "Error writing to file: ${e.message}"
93-
// fail fast?
111+
return // fail?
94112
}
95113

96-
stableCfg = new StableConfigSource(filePath.toString(), ConfigOrigin.USER_STABLE_CONFIG)
114+
StableConfigSource stableCfg = new StableConfigSource(filePath.toString(), ConfigOrigin.USER_STABLE_CONFIG)
97115

98116
then:
99117
stableCfg.getConfigId() == configId
100-
if (configs == null) {
101-
stableCfg.getKeys().size() == 0
102-
} else {
103-
stableCfg.getKeys().size() == configs.size()
118+
stableCfg.getKeys().size() == configs.size()
119+
for (key in stableCfg.getKeys()) {
120+
key = key.substring(4) // Cut `DD_`
121+
stableCfg.get(key) == configs.get(key)
104122
}
105123
Files.delete(filePath)
106124

107125
where:
108-
key_one = "key_one"
109-
val_one = "val_one"
110-
key_two = "key_two"
111-
val_two = "val_2"
112-
configId | configs | corrupt
113-
null | null | true
114-
"" | new HashMap<>() | false
115-
"12345" | new HashMap<>() << ["key_one": "val_one", "key_two": "val_two"] | false
126+
configId | configs
127+
"" | new HashMap<>()
128+
"12345" | new HashMap<>() << ["DD_KEY_ONE": "one", "DD_KEY_TWO": "two"]
129+
}
130+
// Corrupt YAML string variable used for testing, defined outside the 'where' block for readability
131+
@Shared
132+
def corruptYaml = '''
133+
abc: 123
134+
def:
135+
ghi: "jkl"
136+
lmn: 456
137+
'''
138+
139+
Path tempFile() {
140+
try {
141+
return Files.createTempFile("testFile_", ".yaml")
142+
} catch (IOException e) {
143+
println "Error creating file: ${e.message}"
144+
e.printStackTrace()
145+
return null // or throw new RuntimeException("File creation failed", e)
146+
}
147+
}
148+
149+
def writeFileYaml(Path filePath, String configId, Map configs) {
150+
DumperOptions options = new DumperOptions()
151+
options.setDefaultFlowStyle(DumperOptions.FlowStyle.BLOCK)
152+
153+
// Prepare to write the data map to the file in yaml format
154+
Yaml yaml = new Yaml(options)
155+
String yamlString
156+
Map<String, Object> data = new HashMap<>()
157+
if (configId != null) {
158+
data.put("config_id", configId)
159+
}
160+
if (configs instanceof HashMap<?, ?>) {
161+
data.put("apm_configuration_default", configs)
162+
}
163+
164+
yamlString = yaml.dump(data)
165+
166+
StandardOpenOption[] openOpts = [StandardOpenOption.WRITE] as StandardOpenOption[]
167+
Files.write(filePath, yamlString.getBytes(), openOpts)
168+
}
116169

170+
def writeFileRaw(Path filePath, String configId, String configs) {
171+
String data = configId + "\n" + configs
172+
StandardOpenOption[] openOpts = [StandardOpenOption.WRITE] as StandardOpenOption[]
173+
Files.write(filePath, data.getBytes(), openOpts)
117174
}
118175
}

0 commit comments

Comments
 (0)