Skip to content

ConfigProvider iterates over all sources and reports all non-null val… #9327

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 16 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,8 @@ private void setAppSecActivation(final AppSecFeatures.Asm asm) {
} else {
newState = asm.enabled;
// Report AppSec activation change via telemetry when modified via remote config
ConfigCollector.get().put(APPSEC_ENABLED, asm.enabled, ConfigOrigin.REMOTE);
ConfigCollector.get()
.put(APPSEC_ENABLED, asm.enabled, ConfigOrigin.REMOTE); // TODO: Report seq id?
}
if (AppSecSystem.isActive() != newState) {
log.info("AppSec {} (runtime)", newState ? "enabled" : "disabled");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ public void run() throws InterruptedException {
}

private static Object getLogInjectionEnabled() {
ConfigSetting configSetting = ConfigCollector.get().collect().get(LOGS_INJECTION_ENABLED);
ConfigSetting configSetting =
ConfigCollector.get().getAppliedConfigSetting(LOGS_INJECTION_ENABLED);
if (configSetting == null) {
return null;
}
Expand Down
2 changes: 2 additions & 0 deletions internal-api/src/main/java/datadog/trace/api/Config.java
Original file line number Diff line number Diff line change
Expand Up @@ -5217,6 +5217,7 @@ private static boolean isWindowsOS() {
private static String getEnv(String name) {
String value = EnvironmentVariables.get(name);
if (value != null) {
// TODO: Report seqID?
ConfigCollector.get().put(name, value, ConfigOrigin.ENV);
}
return value;
Expand All @@ -5240,6 +5241,7 @@ private static String getProp(String name) {
private static String getProp(String name, String def) {
String value = SystemProperties.getOrDefault(name, def);
if (value != null) {
// TODO: Report seqId?
ConfigCollector.get().put(name, value, ConfigOrigin.JVM_PROP);
}
return value;
Expand Down
44 changes: 27 additions & 17 deletions internal-api/src/main/java/datadog/trace/api/ConfigCollector.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,42 +16,52 @@ public class ConfigCollector {
private static final AtomicReferenceFieldUpdater<ConfigCollector, Map> COLLECTED_UPDATER =
AtomicReferenceFieldUpdater.newUpdater(ConfigCollector.class, Map.class, "collected");

private volatile Map<String, ConfigSetting> collected = new ConcurrentHashMap<>();
private volatile Map<ConfigOrigin, Map<String, ConfigSetting>> collected =
new ConcurrentHashMap<>();

public static ConfigCollector get() {
return INSTANCE;
}

public void put(String key, Object value, ConfigOrigin origin) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I mark this one as deprecated?

ConfigSetting setting = ConfigSetting.of(key, value, origin);
collected.put(key, setting);
Map<String, ConfigSetting> configMap =
collected.computeIfAbsent(origin, k -> new ConcurrentHashMap<>());
configMap.put(key, setting); // replaces any previous value for this key at origin
}

public void put(String key, Object value, ConfigOrigin origin, int seqId) {
ConfigSetting setting = ConfigSetting.of(key, value, origin, seqId);
Map<String, ConfigSetting> configMap =
collected.computeIfAbsent(origin, k -> new ConcurrentHashMap<>());
configMap.put(key, setting); // replaces any previous value for this key at origin
}

public void putAll(Map<String, Object> keysAndValues, ConfigOrigin origin) {
// attempt merge+replace to avoid collector seeing partial update
Map<String, ConfigSetting> merged =
new ConcurrentHashMap<>(keysAndValues.size() + collected.size());
for (Map.Entry<String, Object> entry : keysAndValues.entrySet()) {
ConfigSetting setting = ConfigSetting.of(entry.getKey(), entry.getValue(), origin);
merged.put(entry.getKey(), setting);
}
while (true) {
Map<String, ConfigSetting> current = collected;
current.forEach(merged::putIfAbsent);
if (COLLECTED_UPDATER.compareAndSet(this, current, merged)) {
break; // success
}
// roll back to original update before next attempt
merged.keySet().retainAll(keysAndValues.keySet());
put(entry.getKey(), entry.getValue(), origin);
}
}

@SuppressWarnings("unchecked")
public Map<String, ConfigSetting> collect() {
public Map<ConfigOrigin, Map<String, ConfigSetting>> collect() {
if (!collected.isEmpty()) {
return COLLECTED_UPDATER.getAndSet(this, new ConcurrentHashMap<>());
} else {
return Collections.emptyMap();
}
}

public ConfigSetting getAppliedConfigSetting(String key) {
ConfigSetting best = null;
for (Map<String, ConfigSetting> configMap : collected.values()) {
ConfigSetting setting = configMap.get(key);
if (setting != null) {
if (best == null || setting.seqId > best.seqId) {
best = setting;
}
}
}
return best;
}
}
31 changes: 27 additions & 4 deletions internal-api/src/main/java/datadog/trace/api/ConfigSetting.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,35 @@ public final class ConfigSetting {
public final String key;
public final Object value;
public final ConfigOrigin origin;
public final int seqId;
public final boolean error;

public static final int DEFAULT_SEQ_ID = 1;
private static final int ABSENT_SEQ_ID = 0;

private static final Set<String> CONFIG_FILTER_LIST =
new HashSet<>(
Arrays.asList("DD_API_KEY", "dd.api-key", "dd.profiling.api-key", "dd.profiling.apikey"));

public static ConfigSetting of(String key, Object value, ConfigOrigin origin) {
return new ConfigSetting(key, value, origin);
return new ConfigSetting(key, value, origin, ABSENT_SEQ_ID, false);
}

public static ConfigSetting of(String key, Object value, ConfigOrigin origin, int seqId) {
return new ConfigSetting(key, value, origin, seqId, false);
}

public static ConfigSetting of(
String key, Object value, ConfigOrigin origin, int seqId, boolean error) {
return new ConfigSetting(key, value, origin, seqId, error);
}

private ConfigSetting(String key, Object value, ConfigOrigin origin) {
private ConfigSetting(String key, Object value, ConfigOrigin origin, int seqId, boolean error) {
this.key = key;
this.value = CONFIG_FILTER_LIST.contains(key) ? "<hidden>" : value;
this.origin = origin;
this.seqId = seqId;
this.error = error;
}

public String normalizedKey() {
Expand Down Expand Up @@ -99,12 +115,16 @@ public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
ConfigSetting that = (ConfigSetting) o;
return key.equals(that.key) && Objects.equals(value, that.value) && origin == that.origin;
return key.equals(that.key)
&& Objects.equals(value, that.value)
&& origin == that.origin
&& seqId == that.seqId
&& error == that.error;
}

@Override
public int hashCode() {
return Objects.hash(key, value, origin);
return Objects.hash(key, value, origin, seqId);
}

@Override
Expand All @@ -117,6 +137,9 @@ public String toString() {
+ stringValue()
+ ", origin="
+ origin
+ ", seqId="
+ seqId
+ '}';
// whether to write errors
}
}
Loading
Loading