diff --git a/components/environment/src/main/java/datadog/environment/EnvironmentVariables.java b/components/environment/src/main/java/datadog/environment/EnvironmentVariables.java index 753ea95acd8..5db48340e5c 100644 --- a/components/environment/src/main/java/datadog/environment/EnvironmentVariables.java +++ b/components/environment/src/main/java/datadog/environment/EnvironmentVariables.java @@ -16,6 +16,19 @@ public final class EnvironmentVariables { private EnvironmentVariables() {} + public static class EnvironmentVariablesProvider { + public String get(String name) { + return System.getenv(name); + } + + public Map getAll() { + return System.getenv(); + } + } + + // Make it accessible from tests. + public static EnvironmentVariablesProvider provider = new EnvironmentVariablesProvider(); + /** * Gets an environment variable value. * @@ -41,7 +54,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 +69,7 @@ public static String getOrDefault(String name, String defaultValue) { */ public static Map getAll() { try { - return unmodifiableMap(new HashMap<>(System.getenv())); + return unmodifiableMap(new HashMap<>(provider.getAll())); } catch (SecurityException e) { return emptyMap(); } diff --git a/dd-java-agent/agent-builder/src/test/groovy/datadog/trace/agent/test/DefaultInstrumenterForkedTest.groovy b/dd-java-agent/agent-builder/src/test/groovy/datadog/trace/agent/test/DefaultInstrumenterForkedTest.groovy index eb448819bd2..9be765964fe 100644 --- a/dd-java-agent/agent-builder/src/test/groovy/datadog/trace/agent/test/DefaultInstrumenterForkedTest.groovy +++ b/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: diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/ci/CIProviderInfoFactoryTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/ci/CIProviderInfoFactoryTest.groovy index 08b21a8d318..77efbd800b6 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/ci/CIProviderInfoFactoryTest.groovy +++ b/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: diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/ci/CITagsProviderTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/ci/CITagsProviderTest.groovy index 6939a087692..f8c95d78b74 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/ci/CITagsProviderTest.groovy +++ b/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,7 +124,7 @@ 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: @@ -142,7 +132,7 @@ abstract class CITagsProviderTest extends Specification { 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) } diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/ci/GithubActionsInfoTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/ci/GithubActionsInfoTest.groovy index 5475829870b..912b5201040 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/ci/GithubActionsInfoTest.groovy +++ b/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" diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/ci/UnknownCIInfoTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/ci/UnknownCIInfoTest.groovy index 22a7ce54825..652fecd470e 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/ci/UnknownCIInfoTest.groovy +++ b/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() diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/CIProviderGitInfoBuilderTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/CIProviderGitInfoBuilderTest.groovy index 1177e64183b..5e561c37cbb 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/CIProviderGitInfoBuilderTest.groovy +++ b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/CIProviderGitInfoBuilderTest.groovy @@ -2,25 +2,19 @@ package datadog.trace.civisibility.git 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 datadog.trace.test.util.ControllableEnvironmentVariables import spock.lang.Specification class CIProviderGitInfoBuilderTest extends Specification { + protected ControllableEnvironmentVariables env = ControllableEnvironmentVariables.setup() - @Rule - public final EnvironmentVariables environmentVariables = new EnvironmentVariables() - - 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 builds empty git info in an unknown repository"() { setup: - def builder = new CIProviderGitInfoBuilder(Config.get(), new CiEnvironmentImpl(System.getenv())) + def builder = new CIProviderGitInfoBuilder(Config.get(), new CiEnvironmentImpl(env.getAll())) when: def gitInfo = builder.build(null) diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/DebuggerAgentTest.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/DebuggerAgentTest.java index 24c29cb7bd6..d45fa5362ab 100644 --- a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/DebuggerAgentTest.java +++ b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/DebuggerAgentTest.java @@ -12,7 +12,6 @@ import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import static utils.TestHelper.setEnvVar; import static utils.TestHelper.setFieldInConfig; import com.datadog.debugger.util.RemoteConfigHelper; @@ -22,6 +21,7 @@ import datadog.trace.api.Config; import datadog.trace.api.git.GitInfoProvider; import datadog.trace.bootstrap.instrumentation.api.Tags; +import datadog.trace.test.util.ControllableEnvironmentVariables; import java.io.BufferedReader; import java.io.IOException; import java.io.InputStreamReader; @@ -54,6 +54,8 @@ public class DebuggerAgentTest { final MockWebServer server = new MockWebServer(); HttpUrl url; + private ControllableEnvironmentVariables env = ControllableEnvironmentVariables.setup(); + private static void setFieldInContainerInfo( ContainerInfo containerInfo, String fieldName, Object value) { try { @@ -67,6 +69,7 @@ private static void setFieldInContainerInfo( @BeforeEach public void setUp() { + env.clear(); url = server.url(URL_PATH); } @@ -175,14 +178,14 @@ public void tags() { when(config.getGlobalTags()).thenReturn(globalTags); // set env vars now to be cached by GitInfoProvider GitInfoProvider.INSTANCE.invalidateCache(); - setEnvVar("DD_GIT_COMMIT_SHA", "sha1"); - setEnvVar("DD_GIT_REPOSITORY_URL", "http://github.com"); + env.set("DD_GIT_COMMIT_SHA", "sha1"); + env.set("DD_GIT_REPOSITORY_URL", "http://github.com"); String tags; try { tags = DebuggerAgent.getDefaultTagsMergedWithGlobalTags(config); } finally { - setEnvVar("DD_GIT_COMMIT_SHA", null); - setEnvVar("DD_GIT_REPOSITORY_URL", null); + env.set("DD_GIT_COMMIT_SHA", null); + env.set("DD_GIT_REPOSITORY_URL", null); GitInfoProvider.INSTANCE.invalidateCache(); } Map resultTags = new HashMap<>(); diff --git a/dd-java-agent/agent-debugger/src/test/java/utils/TestHelper.java b/dd-java-agent/agent-debugger/src/test/java/utils/TestHelper.java index 364bf4b10c9..2ee8b170cc6 100644 --- a/dd-java-agent/agent-debugger/src/test/java/utils/TestHelper.java +++ b/dd-java-agent/agent-debugger/src/test/java/utils/TestHelper.java @@ -9,7 +9,6 @@ import java.nio.file.Paths; import java.time.Duration; import java.util.List; -import java.util.Map; import java.util.function.BooleanSupplier; public class TestHelper { @@ -47,19 +46,4 @@ public static void assertWithTimeout(BooleanSupplier predicate, Duration timeout } assertTrue(predicate.getAsBoolean()); } - - public static void setEnvVar(String envName, String envValue) { - try { - Class classOfMap = System.getenv().getClass(); - Field field = classOfMap.getDeclaredField("m"); - field.setAccessible(true); - if (envValue == null) { - ((Map) field.get(System.getenv())).remove(envName); - } else { - ((Map) field.get(System.getenv())).put(envName, envValue); - } - } catch (Exception ex) { - ex.printStackTrace(); - } - } } diff --git a/dd-java-agent/agent-profiling/profiling-uploader/src/test/java/com/datadog/profiling/uploader/ProfileUploaderTest.java b/dd-java-agent/agent-profiling/profiling-uploader/src/test/java/com/datadog/profiling/uploader/ProfileUploaderTest.java index 8e33c190f94..05bf54bb0e8 100644 --- a/dd-java-agent/agent-profiling/profiling-uploader/src/test/java/com/datadog/profiling/uploader/ProfileUploaderTest.java +++ b/dd-java-agent/agent-profiling/profiling-uploader/src/test/java/com/datadog/profiling/uploader/ProfileUploaderTest.java @@ -50,6 +50,7 @@ import datadog.trace.api.profiling.RecordingType; import datadog.trace.bootstrap.config.provider.ConfigProvider; import datadog.trace.relocate.api.IOLogger; +import datadog.trace.test.util.ControllableEnvironmentVariables; import datadog.trace.util.PidHelper; import delight.fileupload.FileUpload; import io.airlift.compress.zstd.ZstdInputStream; @@ -85,7 +86,6 @@ import org.apache.commons.fileupload.FileItem; import org.apache.commons.io.IOUtils; import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -157,19 +157,9 @@ public class ProfileUploaderTest { private ProfileUploader uploader; - @BeforeAll - static void setUpAll() throws Exception { - Map env = System.getenv(); - - Field field = env.getClass().getDeclaredField("m"); - field.setAccessible(true); - - @SuppressWarnings("unchecked") - Map modifiableEnv = (Map) field.get(env); - // hard-coding the env variable here; we could theoretically make the field from ServerlessInfo - // public instead - modifiableEnv.put("AWS_LAMBDA_FUNCTION_NAME", FUNCTION_NAME); - } + @SuppressWarnings("unused") + private static final ControllableEnvironmentVariables ENV = + ControllableEnvironmentVariables.setup("AWS_LAMBDA_FUNCTION_NAME", FUNCTION_NAME); @BeforeEach public void setup() throws IOException { diff --git a/dd-java-agent/instrumentation-testing/src/test/groovy/ConfigResetTest.groovy b/dd-java-agent/instrumentation-testing/src/test/groovy/ConfigResetTest.groovy index 065c5c0a2d3..d0649db5008 100644 --- a/dd-java-agent/instrumentation-testing/src/test/groovy/ConfigResetTest.groovy +++ b/dd-java-agent/instrumentation-testing/src/test/groovy/ConfigResetTest.groovy @@ -1,3 +1,4 @@ +import datadog.environment.EnvironmentVariables import datadog.trace.agent.test.InstrumentationSpecification import datadog.trace.api.Config import spock.lang.Shared @@ -19,7 +20,7 @@ class ConfigResetTest extends InstrumentationSpecification { static Object checkStaticAssertions() { assert System.getProperty("dd.trace.enabled") == null - assert System.getenv("DD_TRACE_ENABLED") == null + assert EnvironmentVariables.get("DD_TRACE_ENABLED") == null assert Config.get().isTraceEnabled() // Returning a new object so this can be used in field initializations @@ -152,14 +153,14 @@ class ConfigResetTest extends InstrumentationSpecification { injectEnvConfig("DD_TRACE_ENABLED", "true") then: - System.getenv("DD_TRACE_ENABLED") == "true" + EnvironmentVariables.get("DD_TRACE_ENABLED") == "true" Config.get().isTraceEnabled() when: injectEnvConfig("DD_TRACE_ENABLED", "false") then: - System.getenv("DD_TRACE_ENABLED") == "false" + EnvironmentVariables.get("DD_TRACE_ENABLED") == "false" !Config.get().isTraceEnabled() } @@ -171,14 +172,14 @@ class ConfigResetTest extends InstrumentationSpecification { injectEnvConfig("TRACE_ENABLED", "true") then: - System.getenv("DD_TRACE_ENABLED") == "true" + EnvironmentVariables.get("DD_TRACE_ENABLED") == "true" Config.get().isTraceEnabled() when: injectEnvConfig("TRACE_ENABLED", "false") then: - System.getenv("DD_TRACE_ENABLED") == "false" + EnvironmentVariables.get("DD_TRACE_ENABLED") == "false" !Config.get().isTraceEnabled() } @@ -190,28 +191,28 @@ class ConfigResetTest extends InstrumentationSpecification { injectEnvConfig("DD_TRACE_ENABLED", "false") then: - System.getenv("DD_TRACE_ENABLED") == "false" + EnvironmentVariables.get("DD_TRACE_ENABLED") == "false" !Config.get().isTraceEnabled() when: removeEnvConfig("DD_TRACE_ENABLED") then: - System.getenv("DD_TRACE_ENABLED") == null + EnvironmentVariables.get("DD_TRACE_ENABLED") == null Config.get().isTraceEnabled() when: injectEnvConfig("DD_TRACE_ENABLED", "false") then: - System.getenv("DD_TRACE_ENABLED") == "false" + EnvironmentVariables.get("DD_TRACE_ENABLED") == "false" !Config.get().isTraceEnabled() when: removeEnvConfig("TRACE_ENABLED") then: - System.getenv("DD_TRACE_ENABLED") == null + EnvironmentVariables.get("DD_TRACE_ENABLED") == null Config.get().isTraceEnabled() } } diff --git a/dd-java-agent/instrumentation/aws-java/aws-java-lambda-handler-1.2/build.gradle b/dd-java-agent/instrumentation/aws-java/aws-java-lambda-handler-1.2/build.gradle index 4790e3b512f..0eea7cd0b7f 100644 --- a/dd-java-agent/instrumentation/aws-java/aws-java-lambda-handler-1.2/build.gradle +++ b/dd-java-agent/instrumentation/aws-java/aws-java-lambda-handler-1.2/build.gradle @@ -10,18 +10,6 @@ apply from: "$rootDir/gradle/java.gradle" addTestSuiteForDir('latestDepTest', 'test') -tasks.named("test", Test) { - environment "_HANDLER", "Handler" -} - -tasks.named("forkedTest", Test) { - environment "_HANDLER", "Handler" -} - -tasks.named("latestDepTest", Test) { - environment "_HANDLER", "Handler" -} - dependencies { compileOnly group: 'com.amazonaws', name: 'aws-lambda-java-core', version: '1.2.1' testImplementation group: 'com.amazonaws', name: 'aws-lambda-java-core', version: '1.2.1' diff --git a/dd-java-agent/instrumentation/aws-java/aws-java-lambda-handler-1.2/src/test/groovy/LambdaHandlerInstrumentationTest.groovy b/dd-java-agent/instrumentation/aws-java/aws-java-lambda-handler-1.2/src/test/groovy/LambdaHandlerInstrumentationTest.groovy index 4efa77ff176..78314ada2da 100644 --- a/dd-java-agent/instrumentation/aws-java/aws-java-lambda-handler-1.2/src/test/groovy/LambdaHandlerInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/aws-java/aws-java-lambda-handler-1.2/src/test/groovy/LambdaHandlerInstrumentationTest.groovy @@ -5,6 +5,12 @@ import com.amazonaws.services.lambda.runtime.Context abstract class LambdaHandlerInstrumentationTest extends VersionedNamingTestBase { def requestId = "test-request-id" + // Must set this env var before the Datadog integration is initialized. + // If present at load time, the integration auto-enables. + static { + environmentVariables.set("_HANDLER", "Handler") + } + @Override String service() { null diff --git a/dd-smoke-tests/datastreams/kafkaschemaregistry/src/main/java/datadog/smoketest/datastreams/kafkaschemaregistry/KafkaProducerWithSchemaRegistry.java b/dd-smoke-tests/datastreams/kafkaschemaregistry/src/main/java/datadog/smoketest/datastreams/kafkaschemaregistry/KafkaProducerWithSchemaRegistry.java index 5a9edd105b2..5e1f3859193 100644 --- a/dd-smoke-tests/datastreams/kafkaschemaregistry/src/main/java/datadog/smoketest/datastreams/kafkaschemaregistry/KafkaProducerWithSchemaRegistry.java +++ b/dd-smoke-tests/datastreams/kafkaschemaregistry/src/main/java/datadog/smoketest/datastreams/kafkaschemaregistry/KafkaProducerWithSchemaRegistry.java @@ -57,7 +57,7 @@ public static void produce() { MyMessage.newBuilder().setId("1").setValue("Hello from Protobuf!").build(); ProducerRecord record = - new ProducerRecord(topicName, "testkey", message); + new ProducerRecord<>(topicName, "testkey", message); producer.send(record); Thread.sleep(1500); log.info("produced message"); diff --git a/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy index 9dc9955fb6b..2d5b1518f99 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/ConfigTest.groovy @@ -6,7 +6,6 @@ import datadog.trace.bootstrap.config.provider.ConfigConverter import datadog.trace.bootstrap.config.provider.ConfigProvider import datadog.trace.test.util.DDSpecification import datadog.trace.util.throwable.FatalAgentMisconfigurationError -import org.junit.Rule import static datadog.trace.api.ConfigDefaults.DEFAULT_HTTP_CLIENT_ERROR_STATUSES import static datadog.trace.api.ConfigDefaults.DEFAULT_HTTP_SERVER_ERROR_STATUSES @@ -140,12 +139,7 @@ import static datadog.trace.api.config.TracerConfig.TRACE_X_DATADOG_TAGS_MAX_LEN import static datadog.trace.api.config.TracerConfig.WRITER_TYPE class ConfigTest extends DDSpecification { - - static final String PREFIX = "dd." - - @Rule - public final FixedCapturedEnvironment fixedCapturedEnvironment = new FixedCapturedEnvironment() - + private static final String PREFIX = "dd." private static final DD_API_KEY_ENV = "DD_API_KEY" private static final DD_SERVICE_NAME_ENV = "DD_SERVICE_NAME" private static final DD_TRACE_ENABLED_ENV = "DD_TRACE_ENABLED" @@ -179,6 +173,10 @@ class ConfigTest extends DDSpecification { private static final DD_LLMOBS_ML_APP_ENV = "DD_LLMOBS_ML_APP" private static final DD_LLMOBS_AGENTLESS_ENABLED_ENV = "DD_LLMOBS_AGENTLESS_ENABLED" + def setup() { + FixedCapturedEnvironment.useFixedEnv([:]) + } + def "specify overrides via properties"() { setup: def prop = new Properties() @@ -830,15 +828,14 @@ class ConfigTest extends DDSpecification { def "captured env props override default props"() { setup: - def capturedEnv = new HashMap() - capturedEnv.put(SERVICE_NAME, "automatic service name") - fixedCapturedEnvironment.load(capturedEnv) + def capturedEnv = [(SERVICE_NAME): "test service name"] + FixedCapturedEnvironment.useFixedEnv(capturedEnv) when: def config = new Config() then: - config.serviceName == "automatic service name" + config.serviceName == "test service name" } def "specify props override captured env props"() { @@ -846,9 +843,8 @@ class ConfigTest extends DDSpecification { def prop = new Properties() prop.setProperty(SERVICE_NAME, "what actually wants") - def capturedEnv = new HashMap() - capturedEnv.put(SERVICE_NAME, "something else") - fixedCapturedEnvironment.load(capturedEnv) + def capturedEnv = [(SERVICE_NAME): "something else"] + FixedCapturedEnvironment.useFixedEnv(capturedEnv) when: def config = Config.get(prop) @@ -861,9 +857,8 @@ class ConfigTest extends DDSpecification { setup: System.setProperty(PREFIX + SERVICE_NAME, "what actually wants") - def capturedEnv = new HashMap() - capturedEnv.put(SERVICE_NAME, "something else") - fixedCapturedEnvironment.load(capturedEnv) + def capturedEnv = [(SERVICE_NAME): "something else"] + FixedCapturedEnvironment.useFixedEnv(capturedEnv) when: def config = new Config() @@ -876,9 +871,8 @@ class ConfigTest extends DDSpecification { setup: environmentVariables.set(DD_SERVICE_NAME_ENV, "what actually wants") - def capturedEnv = new HashMap() - capturedEnv.put(SERVICE_NAME, "something else") - fixedCapturedEnvironment.load(capturedEnv) + def capturedEnv = [(SERVICE_NAME): "something else"] + FixedCapturedEnvironment.useFixedEnv(capturedEnv) when: def config = new Config() @@ -1786,8 +1780,8 @@ class ConfigTest extends DDSpecification { then: //check that env wasn't set: - System.getenv(DD_ENV_ENV) == null - System.getenv(DD_VERSION_ENV) == null + environmentVariables.get(DD_ENV_ENV) == null + environmentVariables.get(DD_VERSION_ENV) == null //actual guard: config.mergedSpanTags == ["env": "production"] } @@ -1801,8 +1795,8 @@ class ConfigTest extends DDSpecification { then: //check that env wasn't set: - System.getenv(DD_ENV_ENV) == null - System.getenv(DD_VERSION_ENV) == null + environmentVariables.get(DD_ENV_ENV) == null + environmentVariables.get(DD_VERSION_ENV) == null //actual guard: config.mergedSpanTags == [(VERSION): "42"] } @@ -1815,7 +1809,7 @@ class ConfigTest extends DDSpecification { Config config = new Config() then: - System.getenv(DD_ENV_ENV) == null + environmentVariables.get(DD_ENV_ENV) == null config.mergedSpanTags.get("env") == null config.mergedSpanTags == [(VERSION): "3.2.1"] } @@ -2593,9 +2587,8 @@ class ConfigTest extends DDSpecification { setup: AgentArgsInjector.injectAgentArgsConfig([(PREFIX + SERVICE_NAME): "args service name"]) - def capturedEnv = new HashMap() - capturedEnv.put(SERVICE_NAME, "captured props service name") - fixedCapturedEnvironment.load(capturedEnv) + def capturedEnv = [(SERVICE_NAME): "captured props service name"] + FixedCapturedEnvironment.useFixedEnv(capturedEnv) when: def config = new Config() diff --git a/internal-api/src/test/groovy/datadog/trace/api/env/FixedCapturedEnvironment.java b/internal-api/src/test/groovy/datadog/trace/api/env/FixedCapturedEnvironment.java index 790d509f0e3..ab4f430a181 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/env/FixedCapturedEnvironment.java +++ b/internal-api/src/test/groovy/datadog/trace/api/env/FixedCapturedEnvironment.java @@ -1,26 +1,11 @@ package datadog.trace.api.env; -import java.util.Collections; import java.util.Map; -import org.junit.rules.ExternalResource; - -/** - * The {@code FixedCapturedEnvironment} rule cleans the {@code CapturedEnvironment} instance when - * the test starts. Additionally, this rule can be used to set fixed properties into that {@code - * CapturedEnvironment} instance. This is useful to be deterministic in those tests that are testing - * logic which interacts with {@code CapturedEnvironment} instance, because that object returns - * properties which are platform dependant (JDK, OS, etc...) - */ -public class FixedCapturedEnvironment extends ExternalResource { - - @Override - protected void before() throws Throwable { - // Clean CapturedEnvironment instance when test starts. - CapturedEnvironment.useFixedEnv(Collections.emptyMap()); - } +/** Helper class that has access to {@link CapturedEnvironment} */ +public class FixedCapturedEnvironment { /** Load properties instance into the {@code CapturedEnvironment} instance. */ - public void load(final Map properties) { + public static void useFixedEnv(final Map properties) { CapturedEnvironment.useFixedEnv(properties); } } diff --git a/internal-api/src/test/groovy/datadog/trace/api/git/UserSuppliedGitInfoBuilderTest.groovy b/internal-api/src/test/groovy/datadog/trace/api/git/UserSuppliedGitInfoBuilderTest.groovy index 21ea13d5ed9..50f3be0a301 100644 --- a/internal-api/src/test/groovy/datadog/trace/api/git/UserSuppliedGitInfoBuilderTest.groovy +++ b/internal-api/src/test/groovy/datadog/trace/api/git/UserSuppliedGitInfoBuilderTest.groovy @@ -6,12 +6,6 @@ import datadog.trace.test.util.DDSpecification import datadog.trace.util.ConfigStrings class UserSuppliedGitInfoBuilderTest extends DDSpecification { - - def setup() { - // Clear all environment variables to avoid clashes between - environmentVariables.clear(System.getenv().keySet() as String[]) - } - def "test no user supplied git info"() { when: def gitInfo = new UserSuppliedGitInfoBuilder().build(null) diff --git a/utils/config-utils/src/main/java/datadog/trace/api/env/CapturedEnvironment.java b/utils/config-utils/src/main/java/datadog/trace/api/env/CapturedEnvironment.java index 956db28c2de..7c4c3dcbe29 100644 --- a/utils/config-utils/src/main/java/datadog/trace/api/env/CapturedEnvironment.java +++ b/utils/config-utils/src/main/java/datadog/trace/api/env/CapturedEnvironment.java @@ -58,10 +58,7 @@ public ProcessInfo getProcessInfo() { // Testing purposes static void useFixedEnv(final Map props) { INSTANCE.properties.clear(); - - for (final Map.Entry entry : props.entrySet()) { - INSTANCE.properties.put(entry.getKey(), entry.getValue()); - } + INSTANCE.properties.putAll(props); } /** diff --git a/utils/test-utils/build.gradle.kts b/utils/test-utils/build.gradle.kts index 1299c1b62b3..5ddcf0b8dd1 100644 --- a/utils/test-utils/build.gradle.kts +++ b/utils/test-utils/build.gradle.kts @@ -11,7 +11,7 @@ dependencies { api(libs.bytebuddy) api(libs.bytebuddyagent) - api(group = "com.github.stefanbirkner", name = "system-rules", version = "1.19.0") + api(project(":components:environment")) api(group = "commons-fileupload", name = "commons-fileupload", version = "1.5") compileOnly(libs.logback.core) diff --git a/utils/test-utils/src/main/groovy/datadog/trace/test/util/ControllableEnvironmentVariables.groovy b/utils/test-utils/src/main/groovy/datadog/trace/test/util/ControllableEnvironmentVariables.groovy new file mode 100644 index 00000000000..76487cb07f7 --- /dev/null +++ b/utils/test-utils/src/main/groovy/datadog/trace/test/util/ControllableEnvironmentVariables.groovy @@ -0,0 +1,51 @@ +package datadog.trace.test.util + +import datadog.environment.EnvironmentVariables + +class ControllableEnvironmentVariables extends EnvironmentVariables.EnvironmentVariablesProvider { + private Map env = new HashMap<>() + + ControllableEnvironmentVariables(String... kv) { + if (kv) { + for (int i = 0; i + 1 < kv.length; i += 2) { + env[kv[i]] = kv[i + 1] + } + } + } + + @Override + String get(String name) { + return env.get(name) + } + + @Override + Map getAll() { + return env + } + + void set(String name, String value) { + env.put(name, value) + } + + void removePrefixed(String prefix) { + env.keySet().removeAll { k -> k.startsWith(prefix) } + } + + void clear() { + env.clear() + } + + static ControllableEnvironmentVariables setup(String... kv) { + ControllableEnvironmentVariables provider = new ControllableEnvironmentVariables(kv) + EnvironmentVariables.provider = provider + + // Propagate specified environment variables to test environment. + System.getenv("TEST_ENV_PROPAGATE_VARS") + ?.split(',') + ?.each { envVar -> + provider[envVar] = System.getenv(envVar) + } + + return provider + } +} diff --git a/utils/test-utils/src/main/groovy/datadog/trace/test/util/DDSpecification.groovy b/utils/test-utils/src/main/groovy/datadog/trace/test/util/DDSpecification.groovy index 43693470451..7b2ee0f2c27 100644 --- a/utils/test-utils/src/main/groovy/datadog/trace/test/util/DDSpecification.groovy +++ b/utils/test-utils/src/main/groovy/datadog/trace/test/util/DDSpecification.groovy @@ -1,7 +1,7 @@ package datadog.trace.test.util +import datadog.environment.EnvironmentVariables import de.thetaphi.forbiddenapis.SuppressForbidden -import org.junit.Rule import spock.lang.Shared import spock.lang.Specification @@ -34,12 +34,10 @@ abstract class DDSpecification extends Specification { private static isConfigInstanceModifiable = false static configModificationFailed = false - @Rule - public final ResetControllableEnvironmentVariables environmentVariables = new ResetControllableEnvironmentVariables() + @Shared + protected static ControllableEnvironmentVariables environmentVariables = ControllableEnvironmentVariables.setup() - // Intentionally not using the RestoreSystemProperties @Rule because this needs to save properties - // in the BeforeClass stage instead of Before stage. Even manually calling before()/after - // doesn't work because the properties object is not cloned for each invocation + // Intentionally saving and restoring System properties. private static Properties originalSystemProperties protected boolean assertThreadsEachCleanup = true @@ -104,13 +102,11 @@ abstract class DDSpecification extends Specification { copy.putAll(originalSystemProperties) System.setProperties(copy) } - - environmentVariables?.reset() } void setupSpec() { assert !configModificationFailed: "Config class modification failed. Ensure all test classes extend DDSpecification" - assert System.getenv().findAll { it.key.startsWith("DD_") }.isEmpty() + assert EnvironmentVariables.getAll().findAll { it.key.startsWith("DD_") }.isEmpty() assert systemPropertiesExceptAllowed().findAll { it.key.toString().startsWith("dd.") }.isEmpty() assert contextTestingAllowed: "Context not ready for testing. Ensure all test classes extend DDSpecification" @@ -127,7 +123,7 @@ abstract class DDSpecification extends Specification { void cleanupSpec() { restoreProperties() - assert System.getenv().findAll { it.key.startsWith("DD_") }.isEmpty() + assert EnvironmentVariables.getAll().findAll { it.key.startsWith("DD_") }.isEmpty() assert systemPropertiesExceptAllowed().findAll { it.key.toString().startsWith("dd.") }.isEmpty() if (isConfigInstanceModifiable) { @@ -150,7 +146,7 @@ abstract class DDSpecification extends Specification { void setup() { restoreProperties() - assert System.getenv().findAll { it.key.startsWith("DD_") }.isEmpty() + assert EnvironmentVariables.getAll().findAll { it.key.startsWith("DD_") }.isEmpty() assert systemPropertiesExceptAllowed().findAll { it.key.toString().startsWith("dd.") }.isEmpty() if (isConfigInstanceModifiable) { @@ -159,9 +155,11 @@ abstract class DDSpecification extends Specification { } void cleanup() { + environmentVariables.clear() + restoreProperties() - assert System.getenv().findAll { it.key.startsWith("DD_") }.isEmpty() + assert EnvironmentVariables.getAll().findAll { it.key.startsWith("DD_") }.isEmpty() assert systemPropertiesExceptAllowed().findAll { it.key.toString().startsWith("dd.") }.isEmpty() if (isConfigInstanceModifiable) { @@ -232,7 +230,7 @@ abstract class DDSpecification extends Specification { checkConfigTransformation() String prefixedName = name.startsWith("DD_") || !addPrefix ? name : "DD_" + name - environmentVariables.clear(prefixedName) + environmentVariables.removePrefixed(prefixedName) rebuildConfig() } diff --git a/utils/test-utils/src/main/groovy/datadog/trace/test/util/ResetControllableEnvironmentVariables.groovy b/utils/test-utils/src/main/groovy/datadog/trace/test/util/ResetControllableEnvironmentVariables.groovy deleted file mode 100644 index 7175152664d..00000000000 --- a/utils/test-utils/src/main/groovy/datadog/trace/test/util/ResetControllableEnvironmentVariables.groovy +++ /dev/null @@ -1,24 +0,0 @@ -package datadog.trace.test.util - -import org.junit.contrib.java.lang.system.EnvironmentVariables -import org.junit.runner.Description -import org.junit.runners.model.Statement - -/** - * This rule is needed to surface the reset method. The base class runs after - * DDSpecification.cleanup() not allowing it to rebuild Config with the restored environment - */ -class ResetControllableEnvironmentVariables extends EnvironmentVariables { - def delegate - - @Override - Statement apply(Statement base, Description description) { - delegate = super.apply(base, description) - - return delegate - } - - void reset() { - delegate?.restoreOriginalVariables() - } -}