fix(ci_visibility): get default environment from agent when DD_ENV is…

… not set (#11478)

This fixes an issue where, if we are in EVP mode, and the agent has a
custom default environment set (eg: using `DD_APM_ENV`), but `DD_ENV` is
not set in the environment, we would incorrectly set the environment to
`None`.

Instead, we now query the agent's info page and use the
`config.default_env` key to choose the environment value.

In somewhat-related changes, we now also explicitly default the
environment to `"none"` in agentless mode if it is not set by the user,
whereas before we would use a `None` value which would serialize to
`null`, and we would rely on the backend to perform the `null` ->
`"none"` change. Since the backend was already enforcing this behavior,
it should be a no-op from the user's perspective.

## Checklist
- [x] PR author has checked that all the criteria below are met
- The PR description includes an overview of the change
- The PR description articulates the motivation for the change
- The change includes tests OR the PR description describes a testing
strategy
- The PR description notes risks associated with the change, if any
- Newly-added code is easy to change
- The change follows the [library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
- The change includes or references documentation updates if necessary
- Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist
- [x] Reviewer has checked that all the criteria below are met 
- Title is accurate
- All changes are related to the pull request's stated goal
- Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes
- Testing strategy adequately addresses listed risks
- Newly-added code is easy to change
- Release note makes sense to a user of the library
- If necessary, author has acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment
- Backport labels are set in a manner that is consistent with the
[release branch maintenance
policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)

ddtrace/internal/ci_visibility/recorder.py

@@ -222,8 +222,14 @@ def __init__(self, tracer=None, config=None, service=None):
         self._git_data: GitData = get_git_data_from_tags(self._tags)
 
         dd_env = os.getenv("_CI_DD_ENV", ddconfig.env)
+        dd_env_msg = ""
 
         if ddconfig._ci_visibility_agentless_enabled:
+            # In agentless mode, normalize an unset env to none (this is already done by the backend in most cases, so
+            # it does not override default behavior)
+            if dd_env is None:
+                dd_env = "none"
+                dd_env_msg = " (not set in environment)"
             if not self._api_key:
                 raise EnvironmentError(
                     "DD_CIVISIBILITY_AGENTLESS_ENABLED is set, but DD_API_KEY is not set, so ddtrace "
@@ -242,6 +248,11 @@ def __init__(self, tracer=None, config=None, service=None):
                 dd_env,
             )
         elif self._agent_evp_proxy_is_available():
+            # In EVP-proxy cases, if an env is not provided, we need to get the agent's default env in order to make
+            # the correct decision:
+            if dd_env is None:
+                dd_env = self._agent_get_default_env()
+                dd_env_msg = " (default environment provided by agent)"
             self._requests_mode = REQUESTS_MODE.EVP_PROXY_EVENTS
             requests_mode_str = "EVP Proxy"
             self._api_client = EVPProxyTestVisibilityAPIClient(
@@ -269,7 +280,7 @@ def __init__(self, tracer=None, config=None, service=None):
 
         self._configure_writer(coverage_enabled=self._collect_coverage_enabled, url=self.tracer._agent_url)
 
-        log.info("Service: %s (env: %s)", self._service, dd_env)
+        log.info("Service: %s (env: %s%s)", self._service, dd_env, dd_env_msg)
         log.info("Requests mode: %s", requests_mode_str)
         log.info("Git metadata upload enabled: %s", self._should_upload_git_metadata)
         log.info("API-provided settings: coverage collection: %s", self._api_settings.coverage_enabled)
@@ -394,6 +405,17 @@ def _agent_evp_proxy_is_available(self):
                 return True
         return False
 
+    def _agent_get_default_env(self):
+        # type: () -> Optional[str]
+        try:
+            info = agent.info(self.tracer._agent_url)
+        except Exception:
+            return "none"
+
+        if info:
+            return info.get("config", {}).get("default_env", "none")
+        return "none"
+
     @classmethod
     def is_itr_enabled(cls):
         # cls.enabled guarantees _instance is not None

releasenotes/notes/ci_visibility-get_agent_default_env-bf4a11283dccdf87.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    CI Visibility: fixes an issue where the CIVisbility service would incorrectly default the tracer env to ``None``
+    in  EVP proxy mode if ``DD_ENV`` was not specified but the agent had a default environment set to a value other
+    than ``none`` (eg: using ``DD_APM_ENV`` in the agent's environment).

tests/ci_visibility/api_client/test_ci_visibility_api_client.py

@@ -353,6 +353,8 @@ def test_civisibility_api_client_agentless_env_config_success(self, env_vars, ex
             configurations["custom"] = _expected_config.pop("custom_configurations")
         if "dd_service" not in _expected_config:
             _expected_config["dd_service"] = "dd-test-py"
+        if "dd_env" not in _expected_config:
+            _expected_config["dd_env"] = "none"
 
         git_data = GitData("[email protected]:TestDog/dd-test-py.git", "notmainbranch", "mytestcommitsha1234")
         with _ci_override_env(_env_vars, full_clear=True), _patch_env_for_testing():
@@ -425,6 +427,7 @@ def test_civisibility_api_client_evp_proxy_config_success(self, env_vars, expect
                     configurations=configurations,
                     git_data=git_data,
                     agent_url="http://patchedagenturl:6218",
+                    dd_env="none",
                     **_expected_config,
                 )
                 CIVisibility.enable()
@@ -435,3 +438,106 @@ def test_civisibility_api_client_evp_proxy_config_success(self, env_vars, expect
                 assert CIVisibility._instance._api_client.__dict__ == expected_client.__dict__
             finally:
                 CIVisibility.disable()
+
+    def test_civisibility_api_client_evp_respects_agent_default_config(self):
+        """Tests that, if no DD_ENV is provided in EVP mode, the agent's default env is used"""
+        agent_info_response = json.loads(
+            """
+            {
+              "version": "7.49.1",
+              "git_commit": "1790cab",
+              "endpoints": [
+                "/v0.3/traces",
+                "/v0.3/services",
+                "/v0.4/traces",
+                "/v0.4/services",
+                "/v0.5/traces",
+                "/v0.7/traces",
+                "/profiling/v1/input",
+                "/telemetry/proxy/",
+                "/v0.6/stats",
+                "/v0.1/pipeline_stats",
+                "/evp_proxy/v1/",
+                "/evp_proxy/v2/",
+                "/evp_proxy/v3/",
+                "/debugger/v1/input",
+                "/debugger/v1/diagnostics",
+                "/symdb/v1/input",
+                "/dogstatsd/v1/proxy",
+                "/dogstatsd/v2/proxy",
+                "/v0.7/config",
+                "/config/set"
+              ],
+              "client_drop_p0s": true,
+              "span_meta_structs": true,
+              "long_running_spans": true,
+              "config": {
+                "default_env": "not_the_default_default_env",
+                "target_tps": 10,
+                "max_eps": 200,
+                "receiver_port": 8126,
+                "receiver_socket": "",
+                "connection_limit": 0,
+                "receiver_timeout": 0,
+                "max_request_bytes": 26214400,
+                "statsd_port": 8125,
+                "max_memory": 0,
+                "max_cpu": 0,
+                "analyzed_spans_by_service": {},
+                "obfuscation": {
+                  "elastic_search": true,
+                  "mongo": true,
+                  "sql_exec_plan": false,
+                  "sql_exec_plan_normalize": false,
+                  "http": {
+                    "remove_query_string": false,
+                    "remove_path_digits": false
+                  },
+                  "remove_stack_traces": false,
+                  "redis": {
+                    "Enabled": true,
+                    "RemoveAllArgs": false
+                  },
+                  "memcached": {
+                    "Enabled": true,
+                    "KeepCommand": false
+                  }
+                }
+              }
+            }
+            """
+        )
+
+        configurations = {
+            "os.architecture": "testarch64",
+            "os.platform": "Not Actually Linux",
+            "os.version": "1.2.3-test",
+            "runtime.name": "CPythonTest",
+            "runtime.version": "1.2.3",
+        }
+
+        git_data = GitData("[email protected]:TestDog/dd-test-py.git", "notmainbranch", "mytestcommitsha1234")
+        with _ci_override_env(full_clear=True), _patch_env_for_testing(), mock.patch(
+            "ddtrace.internal.ci_visibility.recorder.CIVisibility._agent_evp_proxy_is_available", return_value=True
+        ), mock.patch("ddtrace.internal.agent.info", return_value=agent_info_response), mock.patch(
+            "ddtrace.internal.agent.get_trace_url", return_value="http://shouldntbeused:6218"
+        ), mock.patch(
+            "ddtrace.internal.ci_visibility.recorder.ddtrace.tracer._agent_url", "http://patchedagenturl:6218"
+        ):
+            try:
+                expected_client = EVPProxyTestVisibilityAPIClient(
+                    itr_skipping_level=ITR_SKIPPING_LEVEL.TEST,
+                    configurations=configurations,
+                    git_data=git_data,
+                    agent_url="http://patchedagenturl:6218",
+                    dd_env="not_the_default_default_env",
+                    dd_service="dd-test-py",
+                )
+                CIVisibility.enable()
+                assert CIVisibility.enabled is True
+                assert CIVisibility._instance is not None
+                assert CIVisibility._instance._api_client is not None
+
+                assert CIVisibility._instance._api_client.__dict__ == expected_client.__dict__
+            finally:
+                CIVisibility.disable()