Skip to content

Commit a03562c

Browse files
mtoffl01mtoffl01
committed
add comments to current tests
1 parent 06d572d commit a03562c

5 files changed

Lines changed: 126 additions & 18 deletions

File tree

‎dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import datadog.trace.api.profiling.ProfilingEnablement;
3434
import datadog.trace.api.scopemanager.ScopeListener;
3535
import datadog.trace.bootstrap.benchmark.StaticEventLogger;
36+
import datadog.trace.bootstrap.config.provider.StableConfigSource;
3637
import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
3738
import datadog.trace.bootstrap.instrumentation.api.AgentTracer.TracerAPI;
3839
import datadog.trace.bootstrap.instrumentation.api.ProfilingContextIntegration;
@@ -49,6 +50,7 @@
4950
import java.net.URL;
5051
import java.security.CodeSource;
5152
import java.util.EnumSet;
53+
import java.util.Map;
5254
import java.util.concurrent.TimeUnit;
5355
import java.util.concurrent.atomic.AtomicBoolean;
5456
import java.util.regex.PatternSyntaxException;
@@ -1203,9 +1205,18 @@ private static boolean isFeatureEnabled(AgentFeature feature) {
12031205
// must be kept in sync with logic from Config!
12041206
final String featureEnabledSysprop = feature.getSystemProp();
12051207
String featureEnabled = System.getProperty(featureEnabledSysprop);
1208+
// MIKAYLA: Where to write tests for this?
1209+
if (featureEnabled == null) {
1210+
featureEnabled =
1211+
getStableConfig(featureEnabledSysprop, StableConfigSource.MANAGED_STABLE_CONFIG_PATH);
1212+
}
12061213
if (featureEnabled == null) {
12071214
featureEnabled = ddGetEnv(featureEnabledSysprop);
12081215
}
1216+
if (featureEnabled == null) {
1217+
featureEnabled =
1218+
getStableConfig(featureEnabledSysprop, StableConfigSource.USER_STABLE_CONFIG_PATH);
1219+
}
12091220

12101221
if (feature.isEnabledByDefault()) {
12111222
// true unless it's explicitly set to "false"
@@ -1215,6 +1226,7 @@ private static boolean isFeatureEnabled(AgentFeature feature) {
12151226
// We need this hack because profiling in SSI can receive 'auto' value in
12161227
// the enablement config
12171228
return ProfilingEnablement.of(featureEnabled).isActive();
1229+
// MIKAYLA: How does this order of precedence compete with stable config?
12181230
}
12191231
// false unless it's explicitly set to "true"
12201232
return Boolean.parseBoolean(featureEnabled) || "1".equals(featureEnabled);
@@ -1342,6 +1354,15 @@ private static String ddGetProperty(final String sysProp) {
13421354
return value;
13431355
}
13441356

1357+
private static String getStableConfig(final String sysProp, String filepath) {
1358+
String key = toEnvVar(sysProp);
1359+
Map<String, Object> data = StableConfigSource.readYamlFromFile(filepath);
1360+
if (data != null) {
1361+
return (String) data.get(key);
1362+
}
1363+
return null;
1364+
}
1365+
13451366
/** Looks for the "DD_" environment variable equivalent of the given "dd." system property. */
13461367
private static String ddGetEnv(final String sysProp) {
13471368
return System.getenv(toEnvVar(sysProp));

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

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -353,18 +353,16 @@ 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?
357356
if (configProperties.isEmpty()) {
358357
return new ConfigProvider(
359358
new SystemPropertiesConfigSource(),
360359
new StableConfigSource(
361360
StableConfigSource.MANAGED_STABLE_CONFIG_PATH, ConfigOrigin.MANAGED_STABLE_CONFIG),
362361
new EnvironmentConfigSource(),
363362
new OtelEnvironmentConfigSource(),
364-
new CapturedEnvironmentConfigSource(), // MIKAYLA: what is
365-
// CapturedEnvironmentConfigSource?
366363
new StableConfigSource(
367-
StableConfigSource.USER_STABLE_CONFIG_PATH, ConfigOrigin.USER_STABLE_CONFIG));
364+
StableConfigSource.USER_STABLE_CONFIG_PATH, ConfigOrigin.USER_STABLE_CONFIG),
365+
new CapturedEnvironmentConfigSource());
368366
} else {
369367
return new ConfigProvider(
370368
new SystemPropertiesConfigSource(),
@@ -373,9 +371,9 @@ public static ConfigProvider createDefault() {
373371
new EnvironmentConfigSource(),
374372
new PropertiesConfigSource(configProperties, true),
375373
new OtelEnvironmentConfigSource(configProperties),
376-
new CapturedEnvironmentConfigSource(),
377374
new StableConfigSource(
378-
StableConfigSource.USER_STABLE_CONFIG_PATH, ConfigOrigin.USER_STABLE_CONFIG));
375+
StableConfigSource.USER_STABLE_CONFIG_PATH, ConfigOrigin.USER_STABLE_CONFIG),
376+
new CapturedEnvironmentConfigSource());
379377
}
380378
}
381379

@@ -392,9 +390,9 @@ public static ConfigProvider withoutCollector() {
392390
StableConfigSource.MANAGED_STABLE_CONFIG_PATH, ConfigOrigin.MANAGED_STABLE_CONFIG),
393391
new EnvironmentConfigSource(),
394392
new OtelEnvironmentConfigSource(),
395-
new CapturedEnvironmentConfigSource(),
396393
new StableConfigSource(
397-
StableConfigSource.USER_STABLE_CONFIG_PATH, ConfigOrigin.USER_STABLE_CONFIG));
394+
StableConfigSource.USER_STABLE_CONFIG_PATH, ConfigOrigin.USER_STABLE_CONFIG),
395+
new CapturedEnvironmentConfigSource());
398396
} else {
399397
return new ConfigProvider(
400398
false,
@@ -404,35 +402,43 @@ public static ConfigProvider withoutCollector() {
404402
new EnvironmentConfigSource(),
405403
new PropertiesConfigSource(configProperties, true),
406404
new OtelEnvironmentConfigSource(configProperties),
407-
new CapturedEnvironmentConfigSource(),
408405
new StableConfigSource(
409-
StableConfigSource.USER_STABLE_CONFIG_PATH, ConfigOrigin.USER_STABLE_CONFIG));
406+
StableConfigSource.USER_STABLE_CONFIG_PATH, ConfigOrigin.USER_STABLE_CONFIG),
407+
new CapturedEnvironmentConfigSource());
410408
}
411409
}
412410

413-
// MIKAYLA: What is providedConfigSource, and how should it stand up against stableconfig?
414411
public static ConfigProvider withPropertiesOverride(Properties properties) {
415412
PropertiesConfigSource providedConfigSource = new PropertiesConfigSource(properties, false);
416413
Properties configProperties =
417414
loadConfigurationFile(
418415
new ConfigProvider(
419416
new SystemPropertiesConfigSource(),
417+
// MIKAYLA: To add StableConfig?
420418
new EnvironmentConfigSource(),
421419
providedConfigSource));
422420
if (configProperties.isEmpty()) {
423421
return new ConfigProvider(
424422
new SystemPropertiesConfigSource(),
423+
new StableConfigSource(
424+
StableConfigSource.MANAGED_STABLE_CONFIG_PATH, ConfigOrigin.MANAGED_STABLE_CONFIG),
425425
new EnvironmentConfigSource(),
426426
providedConfigSource,
427427
new OtelEnvironmentConfigSource(),
428+
new StableConfigSource(
429+
StableConfigSource.USER_STABLE_CONFIG_PATH, ConfigOrigin.USER_STABLE_CONFIG),
428430
new CapturedEnvironmentConfigSource());
429431
} else {
430432
return new ConfigProvider(
431433
providedConfigSource,
432434
new SystemPropertiesConfigSource(),
435+
new StableConfigSource(
436+
StableConfigSource.MANAGED_STABLE_CONFIG_PATH, ConfigOrigin.MANAGED_STABLE_CONFIG),
433437
new EnvironmentConfigSource(),
434438
new PropertiesConfigSource(configProperties, true),
435439
new OtelEnvironmentConfigSource(configProperties),
440+
new StableConfigSource(
441+
StableConfigSource.USER_STABLE_CONFIG_PATH, ConfigOrigin.USER_STABLE_CONFIG),
436442
new CapturedEnvironmentConfigSource());
437443
}
438444
}

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

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,20 @@
1717
import org.yaml.snakeyaml.constructor.SafeConstructor;
1818

1919
public final class StableConfigSource extends ConfigProvider.Source {
20-
static final String USER_STABLE_CONFIG_PATH = "/etc/datadog-agent/application_monitoring.yaml";
21-
static final String MANAGED_STABLE_CONFIG_PATH =
22-
"/etc/datadog-agent/managed/datadog-apm-libraries/stable/application_monitoring.yaml ";
20+
public static String USER_STABLE_CONFIG_PATH =
21+
"/etc/datadog-agent/application_monitoring.yaml"; // MIKAYLA: this should be final, but I need
22+
// to modify it in my tests because I can't
23+
// write to /etc/ without sudo persmission.
24+
public static String MANAGED_STABLE_CONFIG_PATH =
25+
"/etc/datadog-agent/managed/datadog-apm-libraries/stable/application_monitoring.yaml "; // MIKAYLA: Same for this var.
2326
private static final Logger log = LoggerFactory.getLogger(StableConfigSource.class);
2427

2528
private final ConfigOrigin fileOrigin;
2629
private final Map<String, Object> configuration;
2730
private final String configId;
2831

32+
// MIKAYLA: improvement - if we see that some cached map is already not null by the name the
33+
// StableConfigurationSource constructor is called, we can skip calling it again.
2934
StableConfigSource(String file, ConfigOrigin origin) {
3035
this.fileOrigin = origin;
3136
HashMap<String, Object> data = readYamlFromFile(file);
@@ -50,14 +55,18 @@ public final class StableConfigSource extends ConfigProvider.Source {
5055
* @return A {@link HashMap} containing the configuration data if the file is valid, or <code>null
5156
* </code> if the file is in an invalid format.
5257
*/
53-
private static HashMap<String, Object> readYamlFromFile(String filePath) {
58+
// MIKAYLA: Should improve this so that it caches the resulting map the first time it's called,
59+
// that way we don't need to call readFromYaml twice;
60+
// if we see that said cached map is already not null by the name the StableConfigurationSource
61+
// constructor is called, we can skip calling it again.
62+
public static HashMap<String, Object> readYamlFromFile(String filePath) {
5463
File file = new File(filePath);
5564
if (!file.exists()) {
5665
log.debug(
5766
"Stable configuration file does not exist at the specified path: {}, ignoring", filePath);
5867
return null;
5968
}
60-
69+
System.out.println("file exists");
6170
Yaml yaml = new Yaml(new SafeConstructor(new LoaderOptions()));
6271
InputStream input;
6372
try {

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

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,14 @@ import datadog.trace.api.env.FixedCapturedEnvironment
44
import datadog.trace.bootstrap.config.provider.AgentArgsInjector
55
import datadog.trace.bootstrap.config.provider.ConfigConverter
66
import datadog.trace.bootstrap.config.provider.ConfigProvider
7+
import datadog.trace.bootstrap.config.provider.StableConfigSource
8+
import datadog.trace.bootstrap.config.provider.StableConfigSourceTest
79
import datadog.trace.test.util.DDSpecification
810
import datadog.trace.util.throwable.FatalAgentMisconfigurationError
911
import org.junit.Rule
1012

13+
import java.nio.file.Path
14+
1115
import static datadog.trace.api.ConfigDefaults.DEFAULT_HTTP_CLIENT_ERROR_STATUSES
1216
import static datadog.trace.api.ConfigDefaults.DEFAULT_HTTP_SERVER_ERROR_STATUSES
1317
import static datadog.trace.api.ConfigDefaults.DEFAULT_PARTIAL_FLUSH_MIN_SPANS
@@ -26,6 +30,8 @@ import static datadog.trace.api.TracePropagationStyle.B3SINGLE
2630
import static datadog.trace.api.TracePropagationStyle.DATADOG
2731
import static datadog.trace.api.TracePropagationStyle.HAYSTACK
2832
import static datadog.trace.api.TracePropagationStyle.TRACECONTEXT
33+
import static datadog.trace.api.config.AppSecConfig.APPSEC_ENABLED
34+
import static datadog.trace.api.config.AppSecConfig.APPSEC_SCA_ENABLED
2935
import static datadog.trace.api.config.CiVisibilityConfig.CIVISIBILITY_AGENTLESS_ENABLED
3036
import static datadog.trace.api.config.CiVisibilityConfig.CIVISIBILITY_ENABLED
3137
import static datadog.trace.api.config.DebuggerConfig.DEBUGGER_CLASSFILE_DUMP_ENABLED
@@ -56,6 +62,9 @@ import static datadog.trace.api.config.GeneralConfig.SITE
5662
import static datadog.trace.api.config.GeneralConfig.TAGS
5763
import static datadog.trace.api.config.GeneralConfig.TRACER_METRICS_IGNORED_RESOURCES
5864
import static datadog.trace.api.config.GeneralConfig.VERSION
65+
import static datadog.trace.api.config.GeneralConfig.DATA_STREAMS_ENABLED
66+
import static datadog.trace.api.config.GeneralConfig.DATA_JOBS_ENABLED
67+
import static datadog.trace.api.config.IastConfig.IAST_ENABLED
5968
import static datadog.trace.api.config.JmxFetchConfig.JMX_FETCH_CHECK_PERIOD
6069
import static datadog.trace.api.config.JmxFetchConfig.JMX_FETCH_ENABLED
6170
import static datadog.trace.api.config.JmxFetchConfig.JMX_FETCH_METRICS_CONFIGS
@@ -97,6 +106,7 @@ import static datadog.trace.api.config.TraceInstrumentationConfig.DB_CLIENT_HOST
97106
import static datadog.trace.api.config.TraceInstrumentationConfig.HTTP_CLIENT_HOST_SPLIT_BY_DOMAIN
98107
import static datadog.trace.api.config.TraceInstrumentationConfig.RUNTIME_CONTEXT_FIELD_INJECTION
99108
import static datadog.trace.api.config.TraceInstrumentationConfig.TRACE_ENABLED
109+
import static datadog.trace.api.config.TraceInstrumentationConfig.LOGS_INJECTION
100110
import static datadog.trace.api.config.TracerConfig.AGENT_HOST
101111
import static datadog.trace.api.config.TracerConfig.AGENT_PORT_LEGACY
102112
import static datadog.trace.api.config.TracerConfig.AGENT_UNIX_DOMAIN_SOCKET
@@ -567,6 +577,68 @@ class ConfigTest extends DDSpecification {
567577
config.getLongRunningTraceFlushInterval() == 81
568578
}
569579

580+
def "specify overrides stable config"() {
581+
setup:
582+
Path filePath = StableConfigSourceTest.tempFile()
583+
if (filePath == null) {
584+
// fail fast?
585+
return
586+
}
587+
// Override for testing
588+
String originalPath
589+
if (user == true) {
590+
originalPath = StableConfigSource.USER_STABLE_CONFIG_PATH
591+
StableConfigSource.USER_STABLE_CONFIG_PATH = filePath.toAbsolutePath().toString()
592+
} else if (managed == true) {
593+
originalPath = StableConfigSource.MANAGED_STABLE_CONFIG_PATH
594+
StableConfigSource.MANAGED_STABLE_CONFIG_PATH = filePath.toAbsolutePath().toString()
595+
}
596+
def configs = new HashMap<>()
597+
configs.put(TRACE_ENABLED, "false")
598+
// configs.put(DD_RUNTIME_METRICS_ENABLED_ENV, "false")
599+
// configs.put(LOGS_INJECTION, "false")
600+
// configs.put(PROFILING_ENABLED, "true")
601+
// configs.put(DATA_STREAMS_ENABLED, "true")
602+
// configs.put(APPSEC_ENABLED, "true")
603+
// configs.put(IAST_ENABLED, "true")
604+
// configs.put(DEBUGGER_ENABLED, "true")
605+
// configs.put(DATA_JOBS_ENABLED, "true")
606+
// configs.put(APPSEC_SCA_ENABLED, "true")
607+
608+
try {
609+
StableConfigSourceTest.writeFileYaml(filePath, "12345", configs)
610+
} catch (IOException e) {
611+
println "Error writing to file: ${e.message}"
612+
return // fail?
613+
}
614+
615+
when:
616+
def config = new Config()
617+
618+
then:
619+
!config.traceEnabled
620+
// !config.runtimeMetricsEnabled
621+
// !config.logsInjectionEnabled
622+
// config.profilingEnabled
623+
// config.dataStreamsEnabled
624+
// //config.appSecEnabled?
625+
// config.iastDebugEnabled
626+
// config.debuggerEnabled
627+
// config.dataJobsEnabled
628+
// config.appSecScaEnabled
629+
if (user == true) {
630+
StableConfigSource.USER_STABLE_CONFIG_PATH = originalPath
631+
} else if (managed == true) {
632+
StableConfigSource.MANAGED_STABLE_CONFIG_PATH = originalPath
633+
}
634+
635+
where:
636+
user | managed
637+
true | false
638+
// false | true
639+
640+
}
641+
570642
def "sys props override env vars"() {
571643
setup:
572644
environmentVariables.set(DD_SERVICE_NAME_ENV, "still something else")

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ class StableConfigSourceTest extends DDSpecification {
136136
lmn: 456
137137
'''
138138

139-
Path tempFile() {
139+
static Path tempFile() {
140140
try {
141141
return Files.createTempFile("testFile_", ".yaml")
142142
} catch (IOException e) {
@@ -146,7 +146,7 @@ class StableConfigSourceTest extends DDSpecification {
146146
}
147147
}
148148

149-
def writeFileYaml(Path filePath, String configId, Map configs) {
149+
static writeFileYaml(Path filePath, String configId, Map configs) {
150150
DumperOptions options = new DumperOptions()
151151
options.setDefaultFlowStyle(DumperOptions.FlowStyle.BLOCK)
152152

0 commit comments

Comments
 (0)