Skip to content

Migrate internal apis to environment component #9291

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

Merged
merged 2 commits into from
Aug 14, 2025
Merged
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,5 +1,10 @@
package datadog.environment;

import static java.util.Collections.emptyMap;
import static java.util.Collections.unmodifiableMap;

import java.util.HashMap;
import java.util.Map;
import javax.annotation.Nullable;

/**
Expand Down Expand Up @@ -42,4 +47,18 @@ public static String getOrDefault(String name, String defaultValue) {
return defaultValue;
}
}

/**
* Gets all environment variables.
*
* @return All environment variables captured in an unmodifiable {@link Map}, or an empty {@link
* Map} if they can't be retrieved.
*/
public static Map<String, String> getAll() {
try {
return unmodifiableMap(new HashMap<>(System.getenv()));
} catch (SecurityException e) {
return emptyMap();
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
package datadog.environment;

import static java.util.Collections.emptyMap;
import static java.util.Collections.unmodifiableMap;

import java.util.HashMap;
import java.util.Map;
import javax.annotation.Nullable;

/**
Expand Down Expand Up @@ -42,6 +47,24 @@ public static String getOrDefault(String property, String defaultValue) {
}
}

/**
* Convert system properties to an unmodifiable {@link Map}.
*
* @return All system properties captured in an unmodifiable {@link Map}, or an empty {@link Map}
* if they can't be retrieved.
*/
public static Map<String, String> asStringMap() {
try {
Map<String, String> map = new HashMap<>();
for (String propertyName : System.getProperties().stringPropertyNames()) {
map.put(propertyName, System.getProperty(propertyName));
}
return unmodifiableMap(map);
} catch (SecurityException ignored) {
return emptyMap();
}
}
Comment on lines +57 to +66
Copy link
Contributor

Choose a reason for hiding this comment

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

Just thinking out loud here...
Is that OK that we stop iterating on first SecurityException?
Would it make sense to test each property and ignore only those that failed?
But potentially that could be slow...


/**
* Sets a system property value.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@

import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;

import java.util.Map;
import org.junit.jupiter.api.Test;

class EnvironmentVariablesTest {
Expand Down Expand Up @@ -36,4 +39,13 @@ void testGetOrDefault() {
assertDoesNotThrow(() -> EnvironmentVariables.getOrDefault(MISSING_ENV_VAR, null));
assertNull(EnvironmentVariables.getOrDefault(MISSING_ENV_VAR, null));
}

@Test
void testGetAll() {
Map<String, String> all = EnvironmentVariables.getAll();
assertNotNull(all);
assertFalse(all.isEmpty());
// Unmodifiable collection
assertThrows(UnsupportedOperationException.class, all::clear);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import static org.junit.jupiter.api.Assertions.*;
import static org.junit.jupiter.api.Assumptions.assumeTrue;

import java.util.Map;
import org.junit.jupiter.api.Test;

class SystemPropertiesTest {
Expand Down Expand Up @@ -63,4 +64,15 @@ void testClear() {
assertDoesNotThrow(() -> SystemProperties.clear(null));
assertNull(SystemProperties.clear(null));
}

@Test
void testAsStringMap() {
Map<String, String> stringMap = SystemProperties.asStringMap();
assertNotNull(stringMap);
assertFalse(stringMap.isEmpty());
// Unmodifiable collection
assertThrows(
UnsupportedOperationException.class,
() -> stringMap.put(MISSING_SYSTEM_PROPERTY, DEFAULT_VALUE));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,22 @@
import datadog.trace.bootstrap.instrumentation.api.AgentPropagation;
import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext;
import java.net.InetSocketAddress;
import java.util.Properties;
import java.util.Map;
import javax.annotation.Nullable;

public class ProcessHierarchy {

private static final class SystemPropertiesPropagationGetter
implements AgentPropagation.ContextVisitor<Properties> {
static final AgentPropagation.ContextVisitor<Properties> INSTANCE =
implements AgentPropagation.ContextVisitor<Map<String, String>> {
static final AgentPropagation.ContextVisitor<Map<String, String>> INSTANCE =
new SystemPropertiesPropagationGetter();

private SystemPropertiesPropagationGetter() {}

@Override
public void forEachKey(Properties carrier, AgentPropagation.KeyClassifier classifier) {
for (String propertyName : carrier.stringPropertyNames()) {
if (!classifier.accept(propertyName, carrier.getProperty(propertyName))) {
public void forEachKey(Map<String, String> carrier, AgentPropagation.KeyClassifier classifier) {
for (Map.Entry<String, String> property : carrier.entrySet()) {
if (!classifier.accept(property.getKey(), property.getValue())) {
return;
}
}
Expand All @@ -36,7 +36,7 @@ public void forEachKey(Properties carrier, AgentPropagation.KeyClassifier classi
ProcessHierarchy() {
parentProcessModuleContext =
extractContextAndGetSpanContext(
System.getProperties(), SystemPropertiesPropagationGetter.INSTANCE);
SystemProperties.asStringMap(), SystemPropertiesPropagationGetter.INSTANCE);
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package datadog.trace.civisibility.ci.env;

import java.util.Collections;
import datadog.environment.EnvironmentVariables;
import java.util.Map;

public class CiEnvironmentImpl implements CiEnvironment {
Expand All @@ -12,13 +12,7 @@ public CiEnvironmentImpl(Map<String, String> env) {
}

public static CiEnvironment local() {
Map<String, String> env;
try {
env = System.getenv();
} catch (SecurityException e) {
env = Collections.emptyMap();
}
return new CiEnvironmentImpl(env);
return new CiEnvironmentImpl(EnvironmentVariables.getAll());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import datadog.communication.ddagent.TracerVersion;
import datadog.context.propagation.CarrierSetter;
import datadog.environment.SystemProperties;
import datadog.trace.api.Config;
import datadog.trace.api.DDTags;
import datadog.trace.api.civisibility.domain.BuildModuleLayout;
Expand Down Expand Up @@ -37,7 +38,6 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Properties;
import java.util.concurrent.atomic.LongAdder;
import java.util.function.Consumer;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -137,18 +137,14 @@ private Map<String, String> getPropertiesPropagatedToChildProcess(
ExecutionSettings executionSettings,
BuildSessionSettings sessionSettings) {
Map<String, String> propagatedSystemProperties = new HashMap<>();
try {
Properties systemProperties = System.getProperties();
for (Map.Entry<Object, Object> e : systemProperties.entrySet()) {
String propertyName = (String) e.getKey();
Object propertyValue = e.getValue();
if ((propertyName.startsWith(Config.PREFIX)
|| propertyName.startsWith("datadog.slf4j.simpleLogger.defaultLogLevel"))
&& propertyValue != null) {
propagatedSystemProperties.put(propertyName, propertyValue.toString());
}
for (Map.Entry<String, String> p : SystemProperties.asStringMap().entrySet()) {
String propertyName = p.getKey();
String propertyValue = p.getValue();
if ((propertyName.startsWith(Config.PREFIX)
|| propertyName.startsWith("datadog.slf4j.simpleLogger.defaultLogLevel"))
&& propertyValue != null) {
propagatedSystemProperties.put(propertyName, propertyValue);
}
} catch (SecurityException ignored) {
}

propagatedSystemProperties.put(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import java.nio.file.Path;
import java.util.HashMap;
import java.util.Map;
import java.util.Properties;

public class GradleDaemonInjectionUtils {

Expand All @@ -28,20 +27,16 @@ public static Map<String, String> addJavaagentToGradleDaemonProperties(
Path agentJarPath = agentJar.toPath();
StringBuilder agentArg = new StringBuilder("-javaagent:").append(agentJarPath).append('=');

try {
Properties systemProperties = System.getProperties();
for (Map.Entry<Object, Object> e : systemProperties.entrySet()) {
String propertyName = (String) e.getKey();
Object propertyValue = e.getValue();
if (propertyName.startsWith(Config.PREFIX)) {
agentArg
.append(propertyName)
.append("='")
.append(String.valueOf(propertyValue).replace("'", "'\\''"))
.append("',");
}
for (Map.Entry<String, String> p : SystemProperties.asStringMap().entrySet()) {
String propertyName = p.getKey();
String propertyValue = p.getValue();
if (propertyName.startsWith(Config.PREFIX)) {
agentArg
.append(propertyName)
.append("='")
.append(propertyValue.replace("'", "'\\''"))
.append("',");
}
} catch (SecurityException ignored) {
}

// creating a new map in case jvmOptions is immutable
Expand Down
24 changes: 7 additions & 17 deletions internal-api/src/main/java/datadog/trace/api/ProcessTags.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package datadog.trace.api;

import datadog.environment.EnvironmentVariables;
import datadog.environment.SystemProperties;
import datadog.trace.api.env.CapturedEnvironment;
import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString;
import datadog.trace.util.TraceUtils;
Expand Down Expand Up @@ -27,7 +29,7 @@ public class ProcessTags {
public static final String ENTRYPOINT_WORKDIR = "entrypoint.workdir";

// visible for testing
static Function<String, String> envGetter = System::getenv;
static Function<String, String> envGetter = EnvironmentVariables::get;

private static class Lazy {
// the tags are used to compute a hash for dsm hence that map must be sorted.
Expand Down Expand Up @@ -58,20 +60,12 @@ private static void fillJeeTags(SortedMap<String, String> tags) {

private static void insertTagFromSysPropIfPresent(
Map<String, String> tags, String propKey, String tagKey) {
String value = maybeGetSystemProperty(propKey);
String value = SystemProperties.get(propKey);
if (value != null) {
tags.put(tagKey, value);
}
}

private static String maybeGetSystemProperty(String propKey) {
try {
return System.getProperty(propKey);
} catch (Throwable ignored) {
}
return null;
}

private static boolean insertTagFromEnvIfPresent(
Map<String, String> tags, String envKey, String tagKey) {
try {
Expand Down Expand Up @@ -102,11 +96,7 @@ private static boolean insertLastPathSegmentIfPresent(
}

private static boolean hasSystemProperty(String propKey) {
try {
return System.getProperties().containsKey(propKey);
} catch (Throwable ignored) {
}
return false;
return SystemProperties.get(propKey) != null;
}

private static void fillBaseTags(Map<String, String> tags) {
Expand All @@ -123,12 +113,12 @@ private static void fillBaseTags(Map<String, String> tags) {
insertLastPathSegmentIfPresent(tags, processInfo.jarFile.getParent(), ENTRYPOINT_BASEDIR);
}

insertLastPathSegmentIfPresent(tags, maybeGetSystemProperty("user.dir"), ENTRYPOINT_WORKDIR);
insertLastPathSegmentIfPresent(tags, SystemProperties.get("user.dir"), ENTRYPOINT_WORKDIR);
}

private static boolean fillJbossTags(Map<String, String> tags) {
if (insertLastPathSegmentIfPresent(
tags, maybeGetSystemProperty("jboss.home.dir"), "jboss.home")) {
tags, SystemProperties.get("jboss.home.dir"), "jboss.home")) {
insertTagFromSysPropIfPresent(tags, "jboss.server.name", SERVER_NAME);
tags.put("jboss.mode", hasSystemProperty("[Standalone]") ? "standalone" : "domain");
tags.put(SERVER_TYPE, "jboss");
Expand Down
18 changes: 12 additions & 6 deletions internal-api/src/main/java/datadog/trace/util/PidHelper.java
Original file line number Diff line number Diff line change
@@ -1,15 +1,19 @@
package datadog.trace.util;

import datadog.environment.EnvironmentVariables;
import datadog.environment.JavaVirtualMachine;
import datadog.environment.OperatingSystem;
import datadog.environment.SystemProperties;
import de.thetaphi.forbiddenapis.SuppressForbidden;
import java.io.IOException;
import java.lang.management.ManagementFactory;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Collections;
import java.util.Objects;
import java.util.Set;
import java.util.function.Predicate;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import java.util.stream.Stream;
Expand Down Expand Up @@ -75,14 +79,16 @@ private static String getTempDir() {
if (OperatingSystem.isLinux()) {
return "/tmp";
} else if (OperatingSystem.isWindows()) {
return Stream.of(System.getenv("TMP"), System.getenv("TEMP"), System.getenv("USERPROFILE"))
.filter(String::isEmpty)
return Stream.of("TMP", "TEMP", "USERPROFILE")
.map(EnvironmentVariables::get)
.filter(Objects::nonNull)
.filter(((Predicate<String>) String::isEmpty).negate())
Copy link
Contributor Author

@PerfectSlayer PerfectSlayer Aug 1, 2025

Choose a reason for hiding this comment

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

📝 notes: This behavior change (empty --> !empty) is expected. It’s a fix to the PidHelper on Windows platform and was confirmed with the author first.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add some comment about that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For clarification, what kind of comment do you expect here?

One of the next step for the environment component will be to handle the "temp dir" computation.
So feedback about it would be useful for this part too 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

Comment that will explain this non obvious empty --> !empty on Windows.

.findFirst()
.orElse("C:\\Windows");
} else if (OperatingSystem.isMacOs()) {
return System.getenv("TMPDIR");
return EnvironmentVariables.get("TMPDIR");
} else {
return System.getProperty("java.io.tmpdir");
return SystemProperties.get("java.io.tmpdir");
}
} else {
try {
Expand All @@ -93,7 +99,7 @@ private static String getTempDir() {
.invoke(null);
} catch (Throwable t) {
// Fall back to constants based on J9 source code, may not have perfect coverage
String tmpDir = System.getProperty("java.io.tmpdir");
String tmpDir = SystemProperties.get("java.io.tmpdir");
if (tmpDir != null && !tmpDir.isEmpty()) {
return tmpDir;
} else if (OperatingSystem.isWindows()) {
Expand All @@ -114,7 +120,7 @@ private static Path getJavaProcessesDir() {
} else {
// Emulating the hotspot way to enumerate the JVM processes using the perfdata file
// https://github.com/openjdk/jdk/blob/d7cb933b89839b692f5562aeeb92076cd25a99f6/src/hotspot/share/runtime/perfMemory.cpp#L244
return Paths.get(getTempDir(), "hsperfdata_" + System.getProperty("user.name"));
return Paths.get(getTempDir(), "hsperfdata_" + SystemProperties.get("user.name"));
}
}

Expand Down
Loading