Skip to content

Migrate Config system to environment component #9191

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
@@ -1,6 +1,5 @@
package datadog.environment;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;

/**
Expand Down Expand Up @@ -32,7 +31,7 @@ private EnvironmentVariables() {}
* @return The environment variable value, {@code defaultValue} if missing, can't be retrieved or
* the environment variable name is {@code null}.
*/
public static String getOrDefault(@Nonnull String name, String defaultValue) {
public static String getOrDefault(String name, String defaultValue) {
if (name == null) {
return defaultValue;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package datadog.environment;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;

/**
* Safely queries system properties against security manager.
Expand All @@ -18,7 +18,7 @@ private SystemProperties() {}
* @return The system property value, {@code null} if missing, can't be retrieved, or the system
* property name is {@code null}.
*/
public static String get(String property) {
public static @Nullable String get(String property) {
return getOrDefault(property, null);
}

Expand All @@ -31,7 +31,7 @@ public static String get(String property) {
* @return The system property value, {@code defaultValue} if missing, can't be retrieved, or the
* system property name is {@code null}.
*/
public static String getOrDefault(@Nonnull String property, String defaultValue) {
public static String getOrDefault(String property, String defaultValue) {
if (property == null) {
return defaultValue;
}
Expand All @@ -50,11 +50,32 @@ public static String getOrDefault(@Nonnull String property, String defaultValue)
* @return {@code true} if the system property was successfully set, {@code} false otherwise.
*/
public static boolean set(String property, String value) {
if (property == null || value == null) {
return false;
}
try {
System.setProperty(property, value);
return true;
} catch (SecurityException ignored) {
return false;
}
}

/**
* Clears a system property.
*
* @param property The system property name to clear.
* @return The previous value of the system property, {@code null} if there was no prior property
* and property can't be cleared.
*/
public static @Nullable String clear(String property) {
if (property == null) {
return null;
}
try {
return System.clearProperty(property);
} catch (SecurityException ignored) {
return null;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
@ParametersAreNonnullByDefault
package datadog.environment;

import javax.annotation.ParametersAreNonnullByDefault;
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,29 @@ void testGetOrDefault() {

@Test
void testSet() {
String testProperty = "test.property";
String testValue = "test-value";
assumeTrue(SystemProperties.get(testProperty) == null);

String testProperty = "test.set.property";
String testValue = "test.set.value";
assertNull(SystemProperties.get(testProperty));
assertTrue(SystemProperties.set(testProperty, testValue));
assertEquals(testValue, SystemProperties.get(testProperty));
// Null values
assertDoesNotThrow(() -> SystemProperties.set(testProperty, null));
assertFalse(SystemProperties.set(testProperty, null));
assertDoesNotThrow(() -> SystemProperties.set(null, testValue));
assertFalse(SystemProperties.set(null, testValue));
}

@Test
void testClear() {
String testProperty = "test.clear.property";
String testValue = "test.clear.value";
assertNull(SystemProperties.get(testProperty));
assertNull(SystemProperties.clear(testProperty));
assumeTrue(SystemProperties.set(testProperty, testValue));
assertEquals(testValue, SystemProperties.clear(testProperty));
assertNull(SystemProperties.clear(testProperty));
// Null values
assertDoesNotThrow(() -> SystemProperties.clear(null));
assertNull(SystemProperties.clear(null));
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package datadog.trace.bootstrap.benchmark;

import datadog.environment.SystemProperties;
import java.io.*;
import java.nio.charset.StandardCharsets;
import java.util.Objects;
Expand All @@ -18,8 +19,8 @@ public class StaticEventLogger {
static {
BufferedWriter writer = null;

if ("true".equalsIgnoreCase(System.getProperty("dd.benchmark.enabled"))) {
String dir = System.getProperty("dd.benchmark.output.dir");
if ("true".equalsIgnoreCase(SystemProperties.get("dd.benchmark.enabled"))) {
String dir = SystemProperties.get("dd.benchmark.output.dir");
dir = (dir != null ? dir + File.separator : "");
String fileName = dir + "startup_" + System.currentTimeMillis() + ".csv";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import static datadog.trace.agent.tooling.bytebuddy.matcher.GlobalIgnoresMatcher.globalIgnoresMatcher;
import static net.bytebuddy.matcher.ElementMatchers.isDefaultFinalizer;

import datadog.environment.SystemProperties;
import datadog.trace.agent.tooling.bytebuddy.SharedTypePools;
import datadog.trace.agent.tooling.bytebuddy.iast.TaintableRedefinitionStrategyListener;
import datadog.trace.agent.tooling.bytebuddy.matcher.DDElementMatchers;
Expand Down Expand Up @@ -318,18 +319,17 @@ public static Set<InstrumenterModule.TargetSystem> getEnabledSystems() {
}

private static void addByteBuddyRawSetting() {
final String savedPropertyValue = System.getProperty(TypeDefinition.RAW_TYPES_PROPERTY);
try {
System.setProperty(TypeDefinition.RAW_TYPES_PROPERTY, "true");
final String savedPropertyValue = SystemProperties.get(TypeDefinition.RAW_TYPES_PROPERTY);
if (SystemProperties.set(TypeDefinition.RAW_TYPES_PROPERTY, "true")) {
final boolean rawTypes = TypeDescription.AbstractBase.RAW_TYPES;
if (!rawTypes && DEBUG) {
log.debug("Too late to enable {}", TypeDefinition.RAW_TYPES_PROPERTY);
}
} finally {
} else {
if (savedPropertyValue == null) {
System.clearProperty(TypeDefinition.RAW_TYPES_PROPERTY);
SystemProperties.clear(TypeDefinition.RAW_TYPES_PROPERTY);
} else {
System.setProperty(TypeDefinition.RAW_TYPES_PROPERTY, savedPropertyValue);
SystemProperties.set(TypeDefinition.RAW_TYPES_PROPERTY, savedPropertyValue);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,15 @@ class InitializationTelemetryTest extends Specification {

def "incomplete agent start-up"() {
// In this case, the SecurityManager blocks a custom permission that is checked by bytebuddy causing
// agent initialization to fail. However, we should catch the exception allowing the application
// agent initialization to fail. However, we should catch the exception allowing the application
// to run normally.
when:
def result = InitializationTelemetryCheck.runTestJvm(InitializationTelemetryCheck.BlockByteBuddy)

then:
then: 'should complete successfully and catch error'
result.exitCode == 0
!result.telemetryJson.contains('library_entrypoint.complete')
result.telemetryJson.contains('error_type:java.lang.IllegalStateException')
result.telemetryJson.contains('error_type:')
}

def "block forwarder env var"() {
Expand Down
22 changes: 12 additions & 10 deletions internal-api/src/main/java/datadog/trace/api/Config.java
Original file line number Diff line number Diff line change
Expand Up @@ -630,8 +630,10 @@
import static datadog.trace.util.CollectionUtils.tryMakeImmutableSet;
import static datadog.trace.util.Strings.propertyNameToEnvironmentVariableName;

import datadog.environment.EnvironmentVariables;
import datadog.environment.JavaVirtualMachine;
import datadog.environment.OperatingSystem;
import datadog.environment.SystemProperties;
import datadog.trace.api.civisibility.CiVisibilityWellKnownTags;
import datadog.trace.api.config.GeneralConfig;
import datadog.trace.api.config.ProfilingConfig;
Expand Down Expand Up @@ -1236,7 +1238,7 @@ private Config(final ConfigProvider configProvider, final InstrumenterConfig ins
configFileStatus = configProvider.getConfigFileStatus();
runtimeIdEnabled =
configProvider.getBoolean(RUNTIME_ID_ENABLED, true, RUNTIME_METRICS_RUNTIME_ID_ENABLED);
runtimeVersion = System.getProperty("java.version", "unknown");
runtimeVersion = SystemProperties.getOrDefault("java.version", "unknown");

// Note: We do not want APiKey to be loaded from property for security reasons
// Note: we do not use defined default here
Expand Down Expand Up @@ -3397,7 +3399,7 @@ public static boolean isDatadogProfilerSafeInCurrentEnvironment() {
// don't want to put this logic (which will evolve) in the public ProfilingConfig, and can't
// access Platform there
if (!JavaVirtualMachine.isJ9() && isJavaVersion(8)) {
String arch = System.getProperty("os.arch");
String arch = SystemProperties.get("os.arch");
if ("aarch64".equalsIgnoreCase(arch) || "arm64".equalsIgnoreCase(arch)) {
return false;
}
Expand Down Expand Up @@ -4441,12 +4443,12 @@ public CiVisibilityWellKnownTags getCiVisibilityWellKnownTags() {
getRuntimeId(),
getEnv(),
LANGUAGE_TAG_VALUE,
System.getProperty("java.runtime.name"),
System.getProperty("java.version"),
System.getProperty("java.vendor"),
System.getProperty("os.arch"),
System.getProperty("os.name"),
System.getProperty("os.version"),
SystemProperties.get("java.runtime.name"),
SystemProperties.get("java.version"),
SystemProperties.get("java.vendor"),
SystemProperties.get("os.arch"),
SystemProperties.get("os.name"),
SystemProperties.get("os.version"),
isServiceNameSetByUser() ? "true" : "false");
}

Expand Down Expand Up @@ -5185,7 +5187,7 @@ private static boolean isWindowsOS() {
}

private static String getEnv(String name) {
String value = System.getenv(name);
String value = EnvironmentVariables.get(name);
if (value != null) {
ConfigCollector.get().put(name, value, ConfigOrigin.ENV);
}
Expand All @@ -5208,7 +5210,7 @@ private static String getProp(String name) {
}

private static String getProp(String name, String def) {
String value = System.getProperty(name, def);
String value = SystemProperties.getOrDefault(name, def);
if (value != null) {
ConfigCollector.get().put(name, value, ConfigOrigin.JVM_PROP);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
import static datadog.environment.JavaVirtualMachine.isJavaVersionAtLeast;
import static datadog.environment.JavaVirtualMachine.isOracleJDK8;

import datadog.environment.SystemProperties;

/**
* This class is used early on during premain; it must not touch features like JMX or JUL in case
* they trigger early loading/binding.
Expand Down Expand Up @@ -51,7 +53,7 @@ private static boolean checkForJfr() {

private static boolean checkForNativeImageBuilder() {
try {
return "org.graalvm.nativeimage.builder".equals(System.getProperty("jdk.module.main"));
return "org.graalvm.nativeimage.builder".equals(SystemProperties.get("jdk.module.main"));
} catch (Throwable e) {
return false;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package datadog.trace.api.env;

import datadog.environment.EnvironmentVariables;
import datadog.environment.JavaVirtualMachine;
import datadog.trace.api.config.GeneralConfig;
import java.io.File;
Expand Down Expand Up @@ -77,8 +78,8 @@ static void useFixedProcessInfo(final ProcessInfo processInfo) {
* autodetection will return either the JAR filename or the java main class.
*/
private String autodetectServiceName() {
String inAas = System.getenv("DD_AZURE_APP_SERVICES");
String siteName = System.getenv("WEBSITE_SITE_NAME");
String inAas = EnvironmentVariables.get("DD_AZURE_APP_SERVICES");
String siteName = EnvironmentVariables.get("WEBSITE_SITE_NAME");

if (("true".equalsIgnoreCase(inAas) || "1".equals(inAas)) && siteName != null) {
return siteName;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package datadog.trace.bootstrap.config.provider;

import datadog.environment.EnvironmentVariables;
import datadog.environment.SystemProperties;
import datadog.trace.util.Strings;
import java.util.Map;

Expand All @@ -21,14 +23,14 @@ public static void injectAgentArgsConfig(Map<String, String> args) {
}
for (Map.Entry<String, String> e : args.entrySet()) {
String propertyName = e.getKey();
String existingPropertyValue = System.getProperty(propertyName);
String existingPropertyValue = SystemProperties.get(propertyName);
if (existingPropertyValue != null) {
// system properties should have higher priority than agent arguments
continue;
}

String envVarName = Strings.toEnvVar(propertyName);
String envVarValue = System.getenv(envVarName);
String envVarValue = EnvironmentVariables.get(envVarName);
if (envVarValue != null) {
// env variables should have higher priority than agent arguments
continue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static datadog.trace.api.config.GeneralConfig.CONFIGURATION_FILE;

import datadog.environment.SystemProperties;
import datadog.trace.api.ConfigCollector;
import datadog.trace.api.ConfigOrigin;
import de.thetaphi.forbiddenapis.SuppressForbidden;
Expand Down Expand Up @@ -471,8 +472,11 @@ private static Properties loadConfigurationFile(ConfigProvider configProvider) {
}

// Normalizing tilde (~) paths for unix systems
configurationFilePath =
configurationFilePath.replaceFirst("^~", System.getProperty("user.home"));
String home;
if (configurationFilePath.charAt(0) == '~'
&& (home = SystemProperties.get("user.home")) != null) {
configurationFilePath = home + configurationFilePath.substring(1);
}

// Configuration properties file is optional
final File configurationFile = new File(configurationFilePath);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,22 +1,19 @@
package datadog.trace.bootstrap.config.provider;

import static datadog.trace.api.ConfigOrigin.ENV;
import static datadog.trace.util.Strings.propertyNameToEnvironmentVariableName;

import datadog.environment.EnvironmentVariables;
import datadog.trace.api.ConfigOrigin;

final class EnvironmentConfigSource extends ConfigProvider.Source {

@Override
protected String get(String key) {
try {
return System.getenv(propertyNameToEnvironmentVariableName(key));
} catch (SecurityException e) {
return null;
}
return EnvironmentVariables.get(propertyNameToEnvironmentVariableName(key));
}

@Override
public ConfigOrigin origin() {
return ConfigOrigin.ENV;
return ENV;
}
}
Loading