fix(ci-visibility): do not enable CIVisibility service in agentless m… (#6647)

…ode if DD_API_KEY is not set (#6583)

Fixes the behavior of the CIVisibility service to simply not enable
(instead of raising an exception) if the `DD_API_KEY` environment
variable is not set when the `DD_CIVISIBILITY_AGENTLESS_ENABLE` one is
set and truthy.

- [x] Change(s) are motivated and described in the PR description.
- [x] Testing strategy is described if automated tests are not included
in the PR.
- [x] Risk is outlined (performance impact, potential for breakage,
maintainability, etc).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] [Library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
are followed. If no release note is required, add label
`changelog/no-changelog`.
- [x] Documentation is included (in-code, generated user docs, [public
corp docs](https://github.com/DataDog/documentation/)).
- [x] Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

- [x] Title is accurate.
- [x] No unnecessary changes are introduced.
- [x] Description motivates each change.
- [x] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes unless absolutely necessary.
- [x] Testing strategy adequately addresses listed risk(s).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] Release note makes sense to a user of the library.
- [x] Reviewer has explicitly acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment.
- [x] 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)

(cherry picked from commit 2067422)

Co-authored-by: Romain Komorn <[email protected]>

Author: gnufede

ddtrace/internal/ci_visibility/recorder.py

@@ -338,11 +338,21 @@ def _should_skip_path(self, path, name):
     @classmethod
     def enable(cls, tracer=None, config=None, service=None):
         # type: (Optional[Tracer], Optional[Any], Optional[str]) -> None
+        log.debug("Enabling %s", cls.__name__)
+
+        if ddconfig._ci_visibility_agentless_enabled:
+            if not os.getenv("_CI_DD_API_KEY", os.getenv("DD_API_KEY")):
+                log.critical(
+                    "%s disabled: environment variable DD_CIVISIBILITY_AGENTLESS_ENABLED is true but"
+                    " DD_API_KEY is not set",
+                    cls.__name__,
+                )
+                cls.enabled = False
+                return
 
         if cls._instance is not None:
             log.debug("%s already enabled", cls.__name__)
             return
-        log.debug("Enabling %s", cls.__name__)
 
         cls._instance = cls(tracer=tracer, config=config, service=service)
         cls.enabled = True

releasenotes/notes/fix-civisibility-dont-enable-if-agentless-without-api-f57875b2477862f7.yaml

@@ -0,0 +1,4 @@
+---
+fixes:
+  - |
+    CI Visibility: fixes an issue where the CIVisibility client would raise an exception if it was started in agentless mode without the DD_API_KEY set

tests/integration/test_integration.py

@@ -535,34 +535,39 @@ def test_priority_sampling_rate_honored(encoding, monkeypatch):
 
 @pytest.mark.skipif(AGENT_VERSION == "testagent", reason="Test agent doesn't support evp proxy.")
 def test_civisibility_intake_with_evp_available():
-    with override_env(dict(DD_API_KEY="foobar.baz", DD_SITE="foo.bar")):
-        with override_global_config({"_ci_visibility_agentless_enabled": False}):
-            t = Tracer()
-            CIVisibility.enable(tracer=t)
-            assert CIVisibility._instance.tracer._writer._endpoint == EVP_PROXY_AGENT_ENDPOINT
-            assert CIVisibility._instance.tracer._writer.intake_url == agent.get_trace_url()
-            assert (
-                CIVisibility._instance.tracer._writer._headers[EVP_SUBDOMAIN_HEADER_NAME]
-                == EVP_SUBDOMAIN_HEADER_EVENT_VALUE
-            )
-            CIVisibility.disable()
+    with override_env(dict(DD_API_KEY="foobar.baz", DD_SITE="foo.bar", DD_CIVISIBILITY_AGENTLESS_ENABLED="0")):
+        ddtrace.internal.ci_visibility.recorder.ddconfig = ddtrace.settings.Config()
+        t = Tracer()
+        CIVisibility.enable(tracer=t)
+        assert CIVisibility._instance.tracer._writer._endpoint == EVP_PROXY_AGENT_ENDPOINT
+        assert CIVisibility._instance.tracer._writer.intake_url == agent.get_trace_url()
+        assert (
+            CIVisibility._instance.tracer._writer._headers[EVP_SUBDOMAIN_HEADER_NAME]
+            == EVP_SUBDOMAIN_HEADER_EVENT_VALUE
+        )
+        CIVisibility.disable()
 
 
 def test_civisibility_intake_with_missing_apikey():
-    with override_env(dict(DD_SITE="foobar.baz")):
-        with override_global_config({"_ci_visibility_agentless_enabled": True}):
-            with pytest.raises(EnvironmentError):
+    with override_env(dict(DD_SITE="foobar.baz", DD_CIVISIBILITY_AGENTLESS_ENABLED="1")):
+        with mock.patch.object(CIVisibility, "__init__", return_value=None) as mock_CIVisibility_init:
+            with mock.patch.object(CIVisibility, "start") as mock_CIVisibility_start:
+                ddtrace.internal.ci_visibility.recorder.ddconfig = ddtrace.settings.Config()
                 CIVisibility.enable()
+                assert CIVisibility.enabled is False
+                assert CIVisibility._instance is None
+                mock_CIVisibility_init.assert_not_called()
+                mock_CIVisibility_start.assert_not_called()
 
 
 def test_civisibility_intake_with_apikey():
-    with override_env(dict(DD_API_KEY="foobar.baz", DD_SITE="foo.bar")):
-        with override_global_config({"_ci_visibility_agentless_enabled": True}):
-            t = Tracer()
-            CIVisibility.enable(tracer=t)
-            assert CIVisibility._instance.tracer._writer._endpoint == AGENTLESS_ENDPOINT
-            assert CIVisibility._instance.tracer._writer.intake_url == "https://citestcycle-intake.foo.bar"
-            CIVisibility.disable()
+    with override_env(dict(DD_API_KEY="foobar.baz", DD_SITE="foo.bar", DD_CIVISIBILITY_AGENTLESS_ENABLED="1")):
+        ddtrace.internal.ci_visibility.recorder.ddconfig = ddtrace.settings.Config()
+        t = Tracer()
+        CIVisibility.enable(tracer=t)
+        assert CIVisibility._instance.tracer._writer._endpoint == AGENTLESS_ENDPOINT
+        assert CIVisibility._instance.tracer._writer.intake_url == "https://citestcycle-intake.foo.bar"
+        CIVisibility.disable()
 
 
 def test_bad_endpoint():