Skip to content

Commit 2c56ffe

Browse files
committed
Clean up
1 parent ec92fee commit 2c56ffe

File tree

6 files changed

+80
-47
lines changed

6 files changed

+80
-47
lines changed

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,14 @@
44
import org.snakeyaml.engine.v2.api.LoadSettings;
55

66
public class YamlParser {
7+
/**
8+
* Parses YAML content. Duplicate keys are not allowed and will result in a runtime exception..
9+
*
10+
* @param content - text context to be parsed as YAML
11+
* @return - a parsed representation as a composition of map and list objects.
12+
*/
713
public static Object parse(String content) {
8-
LoadSettings settings = LoadSettings.builder().setAllowDuplicateKeys(true).build();
14+
LoadSettings settings = LoadSettings.builder().build();
915
Load yaml = new Load(settings);
1016
return yaml.loadFromString(content);
1117
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ public static StableConfigSource.StableConfig parse(String filePath) throws IOEx
5050

5151
if (!rules.isEmpty()) {
5252
for (Rule rule : rules) {
53+
// Use the first matching rule
5354
if (doesRuleMatch(rule)) {
5455
// Merge configs found in apm_configuration_rules with those found in
5556
// apm_configuration_default
@@ -61,7 +62,7 @@ public static StableConfigSource.StableConfig parse(String filePath) throws IOEx
6162
}
6263
// If configs were found in apm_configuration_default, use them
6364
if (!configMap.isEmpty()) {
64-
return new StableConfigSource.StableConfig(configId, new LinkedHashMap<>(configMap));
65+
return new StableConfigSource.StableConfig(configId, configMap);
6566
}
6667

6768
// If there's a configId but no configMap, use configId but return an empty map

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

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,23 @@
11
package datadog.trace.bootstrap.config.provider.stableconfig;
22

3-
import java.util.ArrayList;
4-
import java.util.Collections;
5-
import java.util.LinkedHashMap;
3+
import static java.util.Collections.*;
4+
import static java.util.stream.Collectors.toList;
5+
66
import java.util.List;
77
import java.util.Map;
88
import java.util.Objects;
9-
import java.util.stream.Collectors;
109

11-
// Rule represents a set of selectors and their corresponding configurations found in stable
12-
// configuration files
10+
/**
11+
* Rule represents a set of selectors and their corresponding configurations found in stable
12+
* configuration files
13+
*/
1314
public final class Rule {
1415
private final List<Selector> selectors;
1516
private final Map<String, Object> configuration;
1617

1718
public Rule() {
18-
this.selectors = Collections.unmodifiableList(new ArrayList<>());
19-
this.configuration = Collections.unmodifiableMap(new LinkedHashMap<>());
19+
this.selectors = emptyList();
20+
this.configuration = emptyMap();
2021
}
2122

2223
public Rule(List<Selector> selectors, Map<String, Object> configuration) {
@@ -27,10 +28,10 @@ public Rule(List<Selector> selectors, Map<String, Object> configuration) {
2728
public Rule(Object yaml) {
2829
Map map = (Map) yaml;
2930
selectors =
30-
Collections.unmodifiableList(
31+
unmodifiableList(
3132
((List<Object>) map.get("selectors"))
32-
.stream().filter(Objects::nonNull).map(Selector::new).collect(Collectors.toList()));
33-
configuration = Collections.unmodifiableMap((Map<String, Object>) map.get("configuration"));
33+
.stream().filter(Objects::nonNull).map(Selector::new).collect(toList()));
34+
configuration = unmodifiableMap((Map<String, Object>) map.get("configuration"));
3435
}
3536

3637
public List<Selector> getSelectors() {

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

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
package datadog.trace.bootstrap.config.provider.stableconfig;
22

3-
import java.util.ArrayList;
4-
import java.util.Collections;
5-
import java.util.LinkedHashMap;
3+
import static java.util.Collections.emptyList;
4+
import static java.util.Collections.unmodifiableList;
5+
import static java.util.Collections.unmodifiableMap;
6+
import static java.util.stream.Collectors.toList;
7+
68
import java.util.List;
79
import java.util.Map;
8-
import java.util.stream.Collectors;
910

1011
public final class StableConfig {
1112
private final String configId;
@@ -16,19 +17,12 @@ public StableConfig(Object yaml) {
1617
Map<Object, Object> map = (Map<Object, Object>) yaml;
1718
this.configId = String.valueOf(map.get("config_id"));
1819
this.apmConfigurationDefault =
19-
Collections.unmodifiableMap(
20-
(Map<String, Object>)
21-
map.getOrDefault("apm_configuration_default", new LinkedHashMap<>()));
20+
unmodifiableMap(
21+
(Map<String, Object>) map.getOrDefault("apm_configuration_default", emptyList()));
2222
this.apmConfigurationRules =
23-
Collections.unmodifiableList(
24-
((List<Object>) map.getOrDefault("apm_configuration_rules", new ArrayList<>()))
25-
.stream().map(Rule::new).collect(Collectors.toList()));
26-
}
27-
28-
public StableConfig(String configId, Map<String, Object> apmConfigurationDefault) {
29-
this.configId = configId;
30-
this.apmConfigurationDefault = apmConfigurationDefault;
31-
this.apmConfigurationRules = new ArrayList<>();
23+
unmodifiableList(
24+
((List<Object>) map.getOrDefault("apm_configuration_rules", emptyList()))
25+
.stream().map(Rule::new).collect(toList()));
3226
}
3327

3428
public String getConfigId() {

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

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,16 @@ import java.nio.file.Files
44
import java.nio.file.Path
55

66
class StableConfigParserTest extends DDSpecification {
7+
78
def "test parse valid"() {
89
when:
910
Path filePath = Files.createTempFile("testFile_", ".yaml")
11+
then:
12+
if (filePath == null) {
13+
throw new AssertionError("Failed to create: " + filePath)
14+
}
15+
16+
when:
1017
injectEnvConfig("DD_SERVICE", "mysvc")
1118
// From the below yaml, only apm_configuration_default and the second selector should be applied: We use the first matching rule and discard the rest
1219
String yaml = """
@@ -40,7 +47,6 @@ apm_configuration_rules:
4047
StableConfigSource.StableConfig cfg = StableConfigParser.parse(filePath.toString())
4148

4249
then:
43-
filePath != null
4450
def keys = cfg.getKeys()
4551
keys.size() == 3
4652
cfg.getConfigId().trim() == ("12345")
@@ -86,39 +92,45 @@ apm_configuration_rules:
8692
"environment_variables" | [null] | "contains" | "DD_SERVICE" | false
8793
}
8894

89-
def "test duplicate entries"() {
90-
// When duplicate keys are encountered, snakeyaml preserves the last value by default
95+
def "test duplicate entries not allowed"() {
9196
when:
9297
Path filePath = Files.createTempFile("testFile_", ".yaml")
98+
then:
99+
if (filePath == null) {
100+
throw new AssertionError("Failed to create: " + filePath)
101+
}
102+
103+
when:
93104
String yaml = """
94105
config_id: 12345
95106
config_id: 67890
96-
apm_configuration_default:
97-
DD_KEY: value_1
98-
apm_configuration_default:
99-
DD_KEY: value_2
100107
"""
101108
Files.write(filePath, yaml.getBytes())
102-
StableConfigSource.StableConfig cfg = StableConfigParser.parse(filePath.toString())
109+
StableConfigParser.parse(filePath.toString())
103110

104111
then:
105-
filePath != null
106-
cfg != null
107-
cfg.getConfigId() == "67890"
108-
cfg.get("DD_KEY") == "value_2"
112+
def ex = thrown(RuntimeException)
113+
114+
and:
115+
ex.message.contains "found duplicate key config_id"
109116
}
110117

111118
def "test config_id only"() {
112119
when:
113120
Path filePath = Files.createTempFile("testFile_", ".yaml")
121+
then:
122+
if (filePath == null) {
123+
throw new AssertionError("Failed to create: " + filePath)
124+
}
125+
126+
when:
114127
String yaml = """
115128
config_id: 12345
116129
"""
117130
Files.write(filePath, yaml.getBytes())
118131
StableConfigSource.StableConfig cfg = StableConfigParser.parse(filePath.toString())
119132

120133
then:
121-
filePath != null
122134
cfg != null
123135
cfg.getConfigId() == "12345"
124136
cfg.getKeys().size() == 0
@@ -128,6 +140,12 @@ apm_configuration_rules:
128140
// If any piece of the file is invalid, the whole file is rendered invalid and an exception is thrown
129141
when:
130142
Path filePath = Files.createTempFile("testFile_", ".yaml")
143+
then:
144+
if (filePath == null) {
145+
throw new AssertionError("Failed to create: " + filePath)
146+
}
147+
148+
when:
131149
String yaml = """
132150
something-irrelevant: ""
133151
config_id: 12345
@@ -155,7 +173,6 @@ apm_configuration_rules:
155173
}
156174

157175
then:
158-
filePath != null
159176
exception != null
160177
cfg == null
161178
Files.delete(filePath)

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

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,14 @@ class StableConfigSourceTest extends DDSpecification {
2828
def "test empty file"() {
2929
when:
3030
Path filePath = Files.createTempFile("testFile_", ".yaml")
31-
StableConfigSource config = new StableConfigSource(filePath.toString(), ConfigOrigin.LOCAL_STABLE_CONFIG)
31+
then:
32+
if (filePath == null) {
33+
throw new AssertionError("Failed to create: " + filePath)
34+
}
3235

36+
when:
37+
StableConfigSource config = new StableConfigSource(filePath.toString(), ConfigOrigin.LOCAL_STABLE_CONFIG)
3338
then:
34-
filePath != null
3539
config.getKeys().size() == 0
3640
config.getConfigId() == null
3741
}
@@ -40,11 +44,16 @@ class StableConfigSourceTest extends DDSpecification {
4044
// StableConfigSource must handle the exception thrown by StableConfigParser.parse(filePath) gracefully
4145
when:
4246
Path filePath = Files.createTempFile("testFile_", ".yaml")
47+
then:
48+
if (filePath == null) {
49+
throw new AssertionError("Failed to create: " + filePath)
50+
}
51+
52+
when:
4353
writeFileRaw(filePath, configId, data)
4454
StableConfigSource stableCfg = new StableConfigSource(filePath.toString(), ConfigOrigin.LOCAL_STABLE_CONFIG)
4555

4656
then:
47-
filePath != null
4857
stableCfg.getConfigId() == null
4958
stableCfg.getKeys().size() == 0
5059
Files.delete(filePath)
@@ -58,12 +67,17 @@ class StableConfigSourceTest extends DDSpecification {
5867
def "test file valid format"() {
5968
when:
6069
Path filePath = Files.createTempFile("testFile_", ".yaml")
70+
then:
71+
if (filePath == null) {
72+
throw new AssertionError("Failed to create: " + filePath)
73+
}
74+
75+
when:
6176
StableConfig stableConfigYaml = new StableConfig(configId, defaultConfigs)
6277
writeFileYaml(filePath, stableConfigYaml)
6378
StableConfigSource stableCfg = new StableConfigSource(filePath.toString(), ConfigOrigin.LOCAL_STABLE_CONFIG)
6479

6580
then:
66-
filePath != null
6781
for (key in defaultConfigs.keySet()) {
6882
String keyString = (String) key
6983
keyString = keyString.substring(4) // Cut `DD_`

0 commit comments

Comments
 (0)