Skip to content

Commit c7fbbb3

Browse files
mtoffl01mtoffl01
committed
Modify ConfigCollector to store configs in a Map<ConfigOrigin, Map<String, ConfigSetting>>
1 parent 7db5a70 commit c7fbbb3

File tree

4 files changed

+60
-49
lines changed

4 files changed

+60
-49
lines changed

‎internal-api/src/main/java/datadog/trace/api/ConfigCollector.java‎

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -16,42 +16,34 @@ public class ConfigCollector {
1616
private static final AtomicReferenceFieldUpdater<ConfigCollector, Map> COLLECTED_UPDATER =
1717
AtomicReferenceFieldUpdater.newUpdater(ConfigCollector.class, Map.class, "collected");
1818

19-
private volatile Map<String, ConfigSetting> collected = new ConcurrentHashMap<>();
19+
private volatile Map<ConfigOrigin, Map<String, ConfigSetting>> collected =
20+
new ConcurrentHashMap<>();
2021

2122
public static ConfigCollector get() {
2223
return INSTANCE;
2324
}
2425

2526
public void put(String key, Object value, ConfigOrigin origin) {
2627
ConfigSetting setting = ConfigSetting.of(key, value, origin);
27-
collected.put(key, setting);
28+
Map<String, ConfigSetting> configMap =
29+
collected.computeIfAbsent(origin, k -> new ConcurrentHashMap<>());
30+
configMap.put(key, setting); // replaces any previous value for this key at origin
2831
}
2932

3033
public void putAll(Map<String, Object> keysAndValues, ConfigOrigin origin) {
31-
// attempt merge+replace to avoid collector seeing partial update
32-
Map<String, ConfigSetting> merged =
33-
new ConcurrentHashMap<>(keysAndValues.size() + collected.size());
3434
for (Map.Entry<String, Object> entry : keysAndValues.entrySet()) {
35-
ConfigSetting setting = ConfigSetting.of(entry.getKey(), entry.getValue(), origin);
36-
merged.put(entry.getKey(), setting);
37-
}
38-
while (true) {
39-
Map<String, ConfigSetting> current = collected;
40-
current.forEach(merged::putIfAbsent);
41-
if (COLLECTED_UPDATER.compareAndSet(this, current, merged)) {
42-
break; // success
43-
}
44-
// roll back to original update before next attempt
45-
merged.keySet().retainAll(keysAndValues.keySet());
35+
put(entry.getKey(), entry.getValue(), origin);
4636
}
4737
}
4838

4939
@SuppressWarnings("unchecked")
50-
public Map<String, ConfigSetting> collect() {
40+
public Map<ConfigOrigin, Map<String, ConfigSetting>> collect() {
5141
if (!collected.isEmpty()) {
5242
return COLLECTED_UPDATER.getAndSet(this, new ConcurrentHashMap<>());
5343
} else {
5444
return Collections.emptyMap();
5545
}
5646
}
5747
}
48+
49+
// public ConfigSetting getAppliedConfigSetting(String key) {}

‎internal-api/src/test/groovy/datadog/trace/api/ConfigCollectorTest.groovy‎

Lines changed: 41 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,13 @@ class ConfigCollectorTest extends DDSpecification {
2020
def "non-default config settings get collected"() {
2121
setup:
2222
injectEnvConfig(Strings.toEnvVar(configKey), configValue)
23+
def origin = ConfigOrigin.ENV
2324

2425
expect:
25-
def setting = ConfigCollector.get().collect().get(configKey)
26-
setting.stringValue() == configValue
27-
setting.origin == ConfigOrigin.ENV
26+
def envConfigByKey = ConfigCollector.get().collect().get(origin)
27+
def config = envConfigByKey.get(configKey)
28+
config.stringValue() == configValue
29+
config.origin == ConfigOrigin.ENV
2830

2931
where:
3032
configKey | configValue
@@ -64,16 +66,27 @@ class ConfigCollectorTest extends DDSpecification {
6466

6567
def "should collect merged data from multiple sources"() {
6668
setup:
67-
injectEnvConfig(Strings.toEnvVar(configKey), configValue1)
68-
injectSysConfig(configKey, configValue2)
69+
injectEnvConfig(Strings.toEnvVar(configKey), envConfigValue)
70+
injectSysConfig(configKey, jvmConfigValue)
6971

70-
expect:
71-
def setting = ConfigCollector.get().collect().get(configKey)
72-
setting.stringValue() == expectedValue
73-
setting.origin == ConfigOrigin.JVM_PROP
72+
when:
73+
def collected = ConfigCollector.get().collect()
74+
75+
then:
76+
def envSetting = collected.get(ConfigOrigin.ENV)
77+
def envConfig = envSetting.get(configKey)
78+
envConfig.stringValue() == envConfigValue
79+
envConfig.origin == ConfigOrigin.ENV
80+
81+
def jvmSetting = collected.get(ConfigOrigin.JVM_PROP)
82+
def jvmConfig = jvmSetting.get(configKey)
83+
jvmConfig.stringValue().split(',').toList().toSet() == jvmConfigValue.split(',').toList().toSet()
84+
jvmConfig.origin == ConfigOrigin.JVM_PROP
85+
86+
// TODO: Add a check for which setting the collector recognizes as highest precedence
7487

7588
where:
76-
configKey | configValue1 | configValue2 | expectedValue
89+
configKey | envConfigValue | jvmConfigValue | expectedValue
7790
// ConfigProvider.getMergedMap
7891
TracerConfig.TRACE_PEER_SERVICE_MAPPING | "service1:best_service,userService:my_service" | "service2:backup_service" | "service2:backup_service,service1:best_service,userService:my_service"
7992
// ConfigProvider.getOrderedMap
@@ -84,7 +97,8 @@ class ConfigCollectorTest extends DDSpecification {
8497

8598
def "default not-null config settings are collected"() {
8699
expect:
87-
def setting = ConfigCollector.get().collect().get(configKey)
100+
def defaultConfigByKey = ConfigCollector.get().collect().get(ConfigOrigin.DEFAULT)
101+
def setting = defaultConfigByKey.get(configKey)
88102
setting.origin == ConfigOrigin.DEFAULT
89103
setting.stringValue() == defaultValue
90104

@@ -100,7 +114,8 @@ class ConfigCollectorTest extends DDSpecification {
100114

101115
def "default null config settings are also collected"() {
102116
when:
103-
ConfigSetting cs = ConfigCollector.get().collect().get(configKey)
117+
def defaultConfigByKey = ConfigCollector.get().collect().get(ConfigOrigin.DEFAULT)
118+
ConfigSetting cs = defaultConfigByKey.get(configKey)
104119

105120
then:
106121
cs.key == configKey
@@ -121,7 +136,8 @@ class ConfigCollectorTest extends DDSpecification {
121136

122137
def "default empty maps and list config settings are collected as empty strings"() {
123138
when:
124-
ConfigSetting cs = ConfigCollector.get().collect().get(configKey)
139+
def defaultConfigByKey = ConfigCollector.get().collect().get(ConfigOrigin.DEFAULT)
140+
ConfigSetting cs = defaultConfigByKey.get(configKey)
125141

126142
then:
127143
cs.key == configKey
@@ -143,15 +159,15 @@ class ConfigCollectorTest extends DDSpecification {
143159
when:
144160
ConfigCollector.get().put('key1', 'value1', ConfigOrigin.DEFAULT)
145161
ConfigCollector.get().put('key2', 'value2', ConfigOrigin.ENV)
146-
ConfigCollector.get().put('key1', 'replaced', ConfigOrigin.REMOTE)
162+
ConfigCollector.get().put('key1', 'value4', ConfigOrigin.REMOTE)
147163
ConfigCollector.get().put('key3', 'value3', ConfigOrigin.JVM_PROP)
148164

149165
then:
150-
ConfigCollector.get().collect().values().toSet() == [
151-
ConfigSetting.of('key1', 'replaced', ConfigOrigin.REMOTE),
152-
ConfigSetting.of('key2', 'value2', ConfigOrigin.ENV),
153-
ConfigSetting.of('key3', 'value3', ConfigOrigin.JVM_PROP)
154-
] as Set
166+
def collected = ConfigCollector.get().collect()
167+
collected.get(ConfigOrigin.REMOTE).get('key1') == ConfigSetting.of('key1', 'value4', ConfigOrigin.REMOTE)
168+
collected.get(ConfigOrigin.ENV).get('key2') == ConfigSetting.of('key2', 'value2', ConfigOrigin.ENV)
169+
collected.get(ConfigOrigin.JVM_PROP).get('key3') == ConfigSetting.of('key3', 'value3', ConfigOrigin.JVM_PROP)
170+
collected.get(ConfigOrigin.DEFAULT).get('key1') == ConfigSetting.of('key1', 'value1', ConfigOrigin.DEFAULT)
155171
}
156172

157173

@@ -163,16 +179,16 @@ class ConfigCollectorTest extends DDSpecification {
163179
ConfigCollector.get().put('DD_API_KEY', 'sensitive data', ConfigOrigin.ENV)
164180

165181
then:
166-
ConfigCollector.get().collect().get('DD_API_KEY').stringValue() == '<hidden>'
182+
def collected = ConfigCollector.get().collect()
183+
collected.get(ConfigOrigin.ENV).get('DD_API_KEY').stringValue() == '<hidden>'
167184
}
168185

169186
def "collects common setting default values"() {
170187
when:
171-
def settings = ConfigCollector.get().collect()
188+
def defaultConfigByKey = ConfigCollector.get().collect().get(ConfigOrigin.DEFAULT)
172189

173190
then:
174-
def setting = settings.get(key)
175-
191+
def setting = defaultConfigByKey.get(key)
176192
setting.key == key
177193
setting.stringValue() == value
178194
setting.origin == ConfigOrigin.DEFAULT
@@ -202,11 +218,10 @@ class ConfigCollectorTest extends DDSpecification {
202218
injectEnvConfig("DD_TRACE_SAMPLE_RATE", "0.3")
203219

204220
when:
205-
def settings = ConfigCollector.get().collect()
221+
def envConfigByKey = ConfigCollector.get().collect().get(ConfigOrigin.ENV)
206222

207223
then:
208-
def setting = settings.get(key)
209-
224+
def setting = envConfigByKey.get(key)
210225
setting.key == key
211226
setting.stringValue() == value
212227
setting.origin == ConfigOrigin.ENV

‎telemetry/src/main/java/datadog/telemetry/TelemetryRunnable.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import datadog.telemetry.metric.MetricPeriodicAction;
44
import datadog.trace.api.Config;
55
import datadog.trace.api.ConfigCollector;
6+
import datadog.trace.api.ConfigOrigin;
67
import datadog.trace.api.ConfigSetting;
78
import datadog.trace.api.time.SystemTimeSource;
89
import datadog.trace.api.time.TimeSource;
@@ -141,7 +142,7 @@ private void mainLoopIteration() throws InterruptedException {
141142
}
142143

143144
private void collectConfigChanges() {
144-
Map<String, ConfigSetting> collectedConfig = ConfigCollector.get().collect();
145+
Map<ConfigOrigin, Map<String, ConfigSetting>> collectedConfig = ConfigCollector.get().collect();
145146
if (!collectedConfig.isEmpty()) {
146147
telemetryService.addConfiguration(collectedConfig);
147148
}

‎telemetry/src/main/java/datadog/telemetry/TelemetryService.java‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import datadog.telemetry.api.Metric;
88
import datadog.telemetry.api.RequestType;
99
import datadog.telemetry.dependency.Dependency;
10+
import datadog.trace.api.ConfigOrigin;
1011
import datadog.trace.api.ConfigSetting;
1112
import datadog.trace.api.telemetry.Endpoint;
1213
import datadog.trace.api.telemetry.ProductChange;
@@ -84,11 +85,13 @@ public static TelemetryService build(
8485
this.debug = debug;
8586
}
8687

87-
public boolean addConfiguration(Map<String, ConfigSetting> configuration) {
88-
for (ConfigSetting cs : configuration.values()) {
89-
extendedHeartbeatData.pushConfigSetting(cs);
90-
if (!this.configurations.offer(cs)) {
91-
return false;
88+
public boolean addConfiguration(Map<ConfigOrigin, Map<String, ConfigSetting>> configuration) {
89+
for (Map<String, ConfigSetting> settings : configuration.values()) {
90+
for (ConfigSetting cs : settings.values()) {
91+
extendedHeartbeatData.pushConfigSetting(cs);
92+
if (!this.configurations.offer(cs)) {
93+
return false;
94+
}
9295
}
9396
}
9497
return true;

0 commit comments

Comments
 (0)