Migrate Querying of Environment Variables to to ConfigHelper

build.gradle.kts

@@ -9,6 +9,7 @@ plugins {
   id("de.thetaphi.forbiddenapis") version "3.8"
 
   id("tracer-version")
+  id("config-inversion-linter")
   id("io.github.gradle-nexus.publish-plugin") version "2.0.0"
 
   id("com.gradleup.shadow") version "8.3.6" apply false

buildSrc/src/main/kotlin/datadog/gradle/plugin/config/ConfigInversionLinter.kt

@@ -99,9 +99,9 @@ private fun registerCheckEnvironmentVariablesUsage(project: Project) {
       val javaFiles = project.fileTree(project.projectDir) {
         include("**/src/main/java/**/*.java")
         exclude("**/build/**")
-        exclude("internal-api/src/main/java/datadog/trace/api/ConfigHelper.java")
+        exclude("utils/config-utils/src/main/java/datadog/trace/config/inversion/ConfigHelper.java")
         exclude("dd-java-agent/agent-bootstrap/**")
-        exclude("dd-java-agent/src/main/java/datadog/trace/bootstrap/BootstrapInitializationTelemetry.java")
+        exclude("dd-java-agent/src/main/java/datadog/trace/bootstrap/**")
       }
 
       val pattern = Regex("""EnvironmentVariables\.get\s*\(""")

components/environment/src/main/java/datadog/environment/EnvironmentVariables.java

@@ -3,6 +3,7 @@
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.unmodifiableMap;
 
+import de.thetaphi.forbiddenapis.SuppressForbidden;
 import java.util.HashMap;
 import java.util.Map;
 import javax.annotation.Nullable;
@@ -16,6 +17,24 @@
 public final class EnvironmentVariables {
   private EnvironmentVariables() {}
 
+  public static class EnvironmentVariablesProvider {
+
+    // Environment Component has SecurityException handling, so it is safe to call System.getenv
+    // here
+    @SuppressForbidden
+    public String get(String name) {
+      return System.getenv(name);
+    }
+
+    @SuppressForbidden
+    public Map<String, String> getAll() {
+      return System.getenv();
+    }
+  }
+
+  // Make it accessible from tests.
+  public static volatile EnvironmentVariablesProvider provider = new EnvironmentVariablesProvider();
+
   /**
    * Gets an environment variable value.
    *
@@ -41,7 +60,7 @@ public static String getOrDefault(String name, String defaultValue) {
       return defaultValue;
     }
     try {
-      String value = System.getenv(name);
+      String value = provider.get(name);
       return value == null ? defaultValue : value;
     } catch (SecurityException e) {
       return defaultValue;
@@ -56,7 +75,7 @@ public static String getOrDefault(String name, String defaultValue) {
    */
   public static Map<String, String> getAll() {
     try {
-      return unmodifiableMap(new HashMap<>(System.getenv()));
+      return unmodifiableMap(new HashMap<>(provider.getAll()));
     } catch (SecurityException e) {
       return emptyMap();
     }

dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Constants.java

@@ -22,6 +22,7 @@ public final class Constants {
     "datadog.appsec.api",
     "datadog.trace.api",
     "datadog.trace.bootstrap",
+    "datadog.trace.config.inversion",
     "datadog.trace.context",
     "datadog.trace.instrumentation.api",
     "datadog.trace.logging",

dd-java-agent/agent-builder/src/test/groovy/datadog/trace/agent/test/DefaultInstrumenterForkedTest.groovy

@@ -1,6 +1,6 @@
 package datadog.trace.agent.test
 
-
+import datadog.environment.EnvironmentVariables
 import datadog.trace.agent.tooling.InstrumenterModule
 import datadog.trace.agent.tooling.bytebuddy.matcher.DDElementMatchers
 import datadog.trace.agent.tooling.bytebuddy.outline.TypePoolFacade
@@ -118,7 +118,7 @@ class DefaultInstrumenterForkedTest extends DDSpecification {
     def target = new TestDefaultInstrumenter(name, altName)
 
     then:
-    System.getenv("DD_INTEGRATION_${value}_ENABLED") == "true"
+    EnvironmentVariables.get("DD_INTEGRATION_${value}_ENABLED") == "true"
     target.enabled == enabled
 
     where:

dd-java-agent/agent-ci-visibility/civisibility-instrumentation-test-fixtures/src/main/groovy/datadog/trace/civisibility/CiVisibilityInstrumentationTest.groovy

@@ -3,6 +3,7 @@ package datadog.trace.civisibility
 import com.fasterxml.jackson.databind.ObjectMapper
 import datadog.communication.serialization.GrowableBuffer
 import datadog.communication.serialization.msgpack.MsgPackWriter
+import datadog.environment.EnvironmentVariables
 import datadog.trace.agent.test.InstrumentationSpecification
 import datadog.trace.agent.test.asserts.ListWriterAssert
 import datadog.trace.api.Config
@@ -367,7 +368,7 @@ abstract class CiVisibilityInstrumentationTest extends InstrumentationSpecificat
 
     def additionalIgnoredTags = CiVisibilityTestUtils.IGNORED_TAGS + ignoredTags
 
-    if (System.getenv().get("GENERATE_TEST_FIXTURES") != null) {
+    if (EnvironmentVariables.get("GENERATE_TEST_FIXTURES") != null) {
       return generateTestFixtures(testcaseName, events, coverages, additionalReplacements, additionalIgnoredTags)
     }
 

dd-java-agent/agent-ci-visibility/civisibility-test-fixtures/src/main/groovy/datadog/trace/civisibility/CiVisibilitySmokeTest.groovy

@@ -1,5 +1,6 @@
 package datadog.trace.civisibility
 
+import datadog.environment.EnvironmentVariables
 import datadog.trace.api.Config
 import datadog.trace.api.civisibility.config.TestFQN
 import datadog.trace.api.config.CiVisibilityConfig
@@ -21,9 +22,9 @@ abstract class CiVisibilitySmokeTest extends Specification {
 
   protected static String buildJavaHome() {
     if (Jvm.current.isJava8()) {
-      return System.getenv("JAVA_8_HOME")
+      return EnvironmentVariables.get("JAVA_8_HOME")
     }
-    return System.getenv("JAVA_" + Jvm.current.getJavaSpecificationVersion() + "_HOME")
+    return EnvironmentVariables.get("JAVA_" + Jvm.current.getJavaSpecificationVersion() + "_HOME")
   }
 
   protected static String javaPath() {
@@ -67,10 +68,10 @@ abstract class CiVisibilitySmokeTest extends Specification {
     Map<String, String> argMap = buildJvmArgMap(mockBackendIntakeUrl, serviceName, additionalArgs)
 
     // for convenience when debugging locally
-    if (System.getenv("DD_CIVISIBILITY_SMOKETEST_DEBUG_PARENT") != null) {
+    if (EnvironmentVariables.get("DD_CIVISIBILITY_SMOKETEST_DEBUG_PARENT") != null) {
       arguments +=  "-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=5005"
     }
-    if (System.getenv("DD_CIVISIBILITY_SMOKETEST_DEBUG_CHILD") != null) {
+    if (EnvironmentVariables.get("DD_CIVISIBILITY_SMOKETEST_DEBUG_CHILD") != null) {
       argMap.put(CiVisibilityConfig.CIVISIBILITY_DEBUG_PORT, "5055")
     }
 
@@ -83,7 +84,7 @@ abstract class CiVisibilitySmokeTest extends Specification {
   protected verifyEventsAndCoverages(String projectName, String toolchain, String toolchainVersion, List<Map<String, Object>> events, List<Map<String, Object>> coverages, List<String> additionalDynamicTags = []) {
     def additionalReplacements = ["content.meta.['test.toolchain']": "$toolchain:$toolchainVersion"]
 
-    if (System.getenv().get("GENERATE_TEST_FIXTURES") != null) {
+    if (EnvironmentVariables.get("GENERATE_TEST_FIXTURES") != null) {
       def baseTemplatesPath = CiVisibilitySmokeTest.classLoader.getResource(projectName).toURI().schemeSpecificPart.replace('build/resources/test', 'src/test/resources')
       CiVisibilityTestUtils.generateTemplates(baseTemplatesPath, events, coverages, additionalReplacements.keySet() + additionalDynamicTags, SMOKE_IGNORED_TAGS)
     } else {

dd-java-agent/agent-ci-visibility/civisibility-test-fixtures/src/main/groovy/datadog/trace/civisibility/CiVisibilityTestUtils.groovy

@@ -7,6 +7,7 @@ import com.jayway.jsonpath.JsonPath
 import com.jayway.jsonpath.Option
 import com.jayway.jsonpath.ReadContext
 import com.jayway.jsonpath.WriteContext
+import datadog.environment.EnvironmentVariables
 import datadog.trace.api.DDSpanTypes
 import datadog.trace.api.civisibility.config.LibraryCapability
 import datadog.trace.api.civisibility.config.TestFQN
@@ -162,7 +163,7 @@ abstract class CiVisibilityTestUtils {
   }
 
   private static void compareJson(String expectedJson, String actualJson) {
-    def environment = System.getenv()
+    def environment = EnvironmentVariables.getAll()
     def ciRun = environment.get("GITHUB_ACTION") != null || environment.get("GITLAB_CI") != null
     def comparisonMode = ciRun ? JSONCompareMode.LENIENT : JSONCompareMode.NON_EXTENSIBLE
 

dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/ci/CIProviderInfoFactoryTest.groovy

@@ -2,9 +2,7 @@ package datadog.trace.civisibility.ci
 
 import datadog.trace.api.Config
 import datadog.trace.civisibility.ci.env.CiEnvironmentImpl
-import org.junit.Rule
-import org.junit.contrib.java.lang.system.EnvironmentVariables
-import org.junit.contrib.java.lang.system.RestoreSystemProperties
+import datadog.trace.test.util.ControllableEnvironmentVariables
 import spock.lang.Specification
 
 import java.nio.file.Paths
@@ -21,25 +19,18 @@ import static datadog.trace.civisibility.ci.JenkinsInfo.JENKINS
 import static datadog.trace.civisibility.ci.TravisInfo.TRAVIS
 
 class CIProviderInfoFactoryTest extends Specification {
-  @Rule
-  public final EnvironmentVariables environmentVariables = new EnvironmentVariables()
+  protected ControllableEnvironmentVariables env = ControllableEnvironmentVariables.setup()
 
-  @Rule
-  public final RestoreSystemProperties restoreSystemProperties = new RestoreSystemProperties()
-
-  def setup() {
-    // Clear all environment variables to avoid clashes between
-    // real CI/Git environment variables and the spec CI/Git
-    // environment variables.
-    environmentVariables.clear(System.getenv().keySet() as String[])
+  void cleanup() {
+    env.clear()
   }
 
   def "test correct info is selected"() {
     setup:
-    environmentVariables.set(ciKeySelector, "true")
+    env.set(ciKeySelector, "true")
 
     when:
-    def ciProviderInfoFactory = new CIProviderInfoFactory(Config.get(), new CiEnvironmentImpl(System.getenv()))
+    def ciProviderInfoFactory = new CIProviderInfoFactory(Config.get(), new CiEnvironmentImpl(env.getAll()))
     def ciProviderInfo = ciProviderInfoFactory.createCIProviderInfo(Paths.get("").toAbsolutePath())
 
     then:

dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/ci/CITagsProviderTest.groovy

@@ -8,9 +8,7 @@ import datadog.trace.civisibility.ci.env.CiEnvironmentImpl
 import datadog.trace.civisibility.git.CILocalGitInfoBuilder
 import datadog.trace.civisibility.git.CIProviderGitInfoBuilder
 import datadog.trace.civisibility.git.tree.GitClient
-import org.junit.Rule
-import org.junit.contrib.java.lang.system.EnvironmentVariables
-import org.junit.contrib.java.lang.system.RestoreSystemProperties
+import datadog.trace.test.util.ControllableEnvironmentVariables
 import spock.lang.Specification
 
 import java.nio.file.Path
@@ -19,36 +17,28 @@ import java.nio.file.Paths
 import static datadog.trace.util.ConfigStrings.propertyNameToEnvironmentVariableName
 
 abstract class CITagsProviderTest extends Specification {
-
   static final CI_WORKSPACE_PATH_FOR_TESTS = "ci/ci_workspace_for_tests"
   static final GIT_FOLDER_FOR_TESTS = "git_folder_for_tests"
 
-  @Rule
-  public final EnvironmentVariables environmentVariables = new EnvironmentVariables()
-
-  @Rule
-  public final RestoreSystemProperties restoreSystemProperties = new RestoreSystemProperties()
+  protected ControllableEnvironmentVariables env = ControllableEnvironmentVariables.setup()
 
   protected final String localFSGitWorkspace = resolve(CI_WORKSPACE_PATH_FOR_TESTS)
 
-  def setup() {
-    // Clear all environment variables to avoid clashes between
-    // real CI/Git environment variables and the spec CI/Git
-    // environment variables.
-    environmentVariables.clear(System.getenv().keySet() as String[])
+  void cleanup() {
+    env.clear()
   }
 
   def "test ci provider info is set properly: #ciSpec.providerName #ciSpec.idx #ciSpec.testCaseName"() {
     setup:
     ciSpec.env.each {
-      environmentVariables.set(it.key, it.value.toString())
+      env.set(it.key, it.value.toString())
       if (it.key == "HOME") {
         System.setProperty("user.home", it.value)
       }
     }
 
     when:
-    CIProviderInfoFactory ciProviderInfoFactory = new CIProviderInfoFactory(Config.get(), GIT_FOLDER_FOR_TESTS, new CiEnvironmentImpl(System.getenv()))
+    CIProviderInfoFactory ciProviderInfoFactory = new CIProviderInfoFactory(Config.get(), GIT_FOLDER_FOR_TESTS, new CiEnvironmentImpl(env.getAll()))
     def ciProviderInfo = ciProviderInfoFactory.createCIProviderInfo(getWorkspacePath())
     def ciInfo = ciProviderInfo.buildCIInfo()
     def prInfo = ciProviderInfo.buildPullRequestInfo()
@@ -68,13 +58,13 @@ abstract class CITagsProviderTest extends Specification {
   def "test user supplied commit hash takes precedence over auto-detected git info"() {
     setup:
     buildRemoteGitInfoEmpty().each {
-      environmentVariables.set(it.key, it.value)
+      env.set(it.key, it.value)
     }
 
-    environmentVariables.set(propertyNameToEnvironmentVariableName(UserSuppliedGitInfoBuilder.DD_GIT_COMMIT_SHA), "1234567890123456789012345678901234567890")
+    env.set(propertyNameToEnvironmentVariableName(UserSuppliedGitInfoBuilder.DD_GIT_COMMIT_SHA), "1234567890123456789012345678901234567890")
 
     when:
-    CIProviderInfoFactory ciProviderInfoFactory = new CIProviderInfoFactory(Config.get(), GIT_FOLDER_FOR_TESTS, new CiEnvironmentImpl(System.getenv()))
+    CIProviderInfoFactory ciProviderInfoFactory = new CIProviderInfoFactory(Config.get(), GIT_FOLDER_FOR_TESTS, new CiEnvironmentImpl(env.getAll()))
     def ciProviderInfo = ciProviderInfoFactory.createCIProviderInfo(getWorkspacePath())
     def ciInfo = ciProviderInfo.buildCIInfo()
     def ciTagsProvider = ciTagsProvider()
@@ -87,13 +77,13 @@ abstract class CITagsProviderTest extends Specification {
   def "test user supplied repo url takes precedence over auto-detected git info"() {
     setup:
     buildRemoteGitInfoEmpty().each {
-      environmentVariables.set(it.key, it.value)
+      env.set(it.key, it.value)
     }
 
-    environmentVariables.set(propertyNameToEnvironmentVariableName(UserSuppliedGitInfoBuilder.DD_GIT_REPOSITORY_URL), "local supplied repo url")
+    env.set(propertyNameToEnvironmentVariableName(UserSuppliedGitInfoBuilder.DD_GIT_REPOSITORY_URL), "local supplied repo url")
 
     when:
-    CIProviderInfoFactory ciProviderInfoFactory = new CIProviderInfoFactory(Config.get(), GIT_FOLDER_FOR_TESTS, new CiEnvironmentImpl(System.getenv()))
+    CIProviderInfoFactory ciProviderInfoFactory = new CIProviderInfoFactory(Config.get(), GIT_FOLDER_FOR_TESTS, new CiEnvironmentImpl(env.getAll()))
     def ciProviderInfo = ciProviderInfoFactory.createCIProviderInfo(getWorkspacePath())
     def ciInfo = ciProviderInfo.buildCIInfo()
     def ciTagsProvider = ciTagsProvider()
@@ -106,11 +96,11 @@ abstract class CITagsProviderTest extends Specification {
   def "test set local git info if remote git info is not present"() {
     setup:
     buildRemoteGitInfoEmpty().each {
-      environmentVariables.set(it.key, it.value)
+      env.set(it.key, it.value)
     }
 
     when:
-    CIProviderInfoFactory ciProviderInfoFactory = new CIProviderInfoFactory(Config.get(), GIT_FOLDER_FOR_TESTS, new CiEnvironmentImpl(System.getenv()))
+    CIProviderInfoFactory ciProviderInfoFactory = new CIProviderInfoFactory(Config.get(), GIT_FOLDER_FOR_TESTS, new CiEnvironmentImpl(env.getAll()))
     def ciProviderInfo = ciProviderInfoFactory.createCIProviderInfo(getWorkspacePath())
     def ciInfo = ciProviderInfo.buildCIInfo()
     def ciTagsProvider = ciTagsProvider()
@@ -134,15 +124,15 @@ abstract class CITagsProviderTest extends Specification {
   def "test avoid setting local git info if remote commit does not match"() {
     setup:
     buildRemoteGitInfoMismatchLocalGit().each {
-      environmentVariables.set(it.key, it.value)
+      env.set(it.key, it.value)
     }
 
     when:
     def ciTagsProvider = ciTagsProvider()
 
     then:
     if (isCi()) {
-      CIProviderInfoFactory ciProviderInfoFactory = new CIProviderInfoFactory(Config.get(), GIT_FOLDER_FOR_TESTS, new CiEnvironmentImpl(System.getenv()))
+      CIProviderInfoFactory ciProviderInfoFactory = new CIProviderInfoFactory(Config.get(), GIT_FOLDER_FOR_TESTS, new CiEnvironmentImpl(env.getAll()))
       def ciProviderInfo = ciProviderInfoFactory.createCIProviderInfo(getWorkspacePath())
       def ciInfo = ciProviderInfo.buildCIInfo()
       def tags = ciTagsProvider.getCiTags(ciInfo, PullRequestInfo.EMPTY)
@@ -176,7 +166,7 @@ abstract class CITagsProviderTest extends Specification {
 
     GitInfoProvider gitInfoProvider = new GitInfoProvider()
     gitInfoProvider.registerGitInfoBuilder(new UserSuppliedGitInfoBuilder())
-    gitInfoProvider.registerGitInfoBuilder(new CIProviderGitInfoBuilder(Config.get(), new CiEnvironmentImpl(System.getenv())))
+    gitInfoProvider.registerGitInfoBuilder(new CIProviderGitInfoBuilder(Config.get(), new CiEnvironmentImpl(env.getAll())))
     gitInfoProvider.registerGitInfoBuilder(new CILocalGitInfoBuilder(gitClientFactory, GIT_FOLDER_FOR_TESTS))
     return new CITagsProvider(gitInfoProvider)
   }

dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/ci/GithubActionsInfoTest.groovy

@@ -33,11 +33,11 @@ class GithubActionsInfoTest extends CITagsProviderTest {
     def githubEvent = GithubActionsInfoTest.getResource("/ci/github-event.json")
     def githubEventPath = Paths.get(githubEvent.toURI())
 
-    environmentVariables.set(GithubActionsInfo.GITHUB_BASE_REF, "base-ref")
-    environmentVariables.set(GithubActionsInfo.GITHUB_EVENT_PATH, githubEventPath.toString())
+    env.set(GithubActionsInfo.GITHUB_BASE_REF, "base-ref")
+    env.set(GithubActionsInfo.GITHUB_EVENT_PATH, githubEventPath.toString())
 
     when:
-    def pullRequestInfo = new GithubActionsInfo(new CiEnvironmentImpl(System.getenv())).buildPullRequestInfo()
+    def pullRequestInfo = new GithubActionsInfo(new CiEnvironmentImpl(env.getAll())).buildPullRequestInfo()
 
     then:
     pullRequestInfo.getBaseBranch() == "base-ref"

dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/ci/UnknownCIInfoTest.groovy

@@ -45,7 +45,7 @@ class UnknownCIInfoTest extends CITagsProviderTest {
     ]
 
     when:
-    CIProviderInfoFactory ciProviderInfoFactory = new CIProviderInfoFactory(Config.get(), GIT_FOLDER_FOR_TESTS, new CiEnvironmentImpl(System.getenv()))
+    CIProviderInfoFactory ciProviderInfoFactory = new CIProviderInfoFactory(Config.get(), GIT_FOLDER_FOR_TESTS, new CiEnvironmentImpl(env.getAll()))
     def ciProviderInfo = ciProviderInfoFactory.createCIProviderInfo(workspaceForTests)
     def ciInfo = ciProviderInfo.buildCIInfo()
     def ciTagsProvider = ciTagsProvider()
@@ -62,9 +62,9 @@ class UnknownCIInfoTest extends CITagsProviderTest {
 
     GitInfoProvider gitInfoProvider = new GitInfoProvider()
     gitInfoProvider.registerGitInfoBuilder(new UserSuppliedGitInfoBuilder())
-    gitInfoProvider.registerGitInfoBuilder(new CIProviderGitInfoBuilder(Config.get(), new CiEnvironmentImpl(System.getenv())))
+    gitInfoProvider.registerGitInfoBuilder(new CIProviderGitInfoBuilder(Config.get(), new CiEnvironmentImpl(env.getAll())))
     gitInfoProvider.registerGitInfoBuilder(new CILocalGitInfoBuilder(gitClientFactory, "this-target-folder-does-not-exist"))
-    CIProviderInfoFactory ciProviderInfoFactory = new CIProviderInfoFactory(Config.get(), "this-target-folder-does-not-exist", new CiEnvironmentImpl(System.getenv()))
+    CIProviderInfoFactory ciProviderInfoFactory = new CIProviderInfoFactory(Config.get(), "this-target-folder-does-not-exist", new CiEnvironmentImpl(env.getAll()))
 
     def ciProviderInfo = ciProviderInfoFactory.createCIProviderInfo(workspaceForTests)
     def ciInfo = ciProviderInfo.buildCIInfo()