fix(rcm): validate remote config is enabled [backport #5241 to 1.9] (#5353)

Backports: https://github.com/DataDog/dd-trace-py/pull/5241/files

Resolves: #5343

## Checklist

- [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/contributing.html#Release-Note-Guidelines)
are followed.
- [x] Documentation is included (in-code, generated user docs, [public
corp docs](https://github.com/DataDog/documentation/)).
- [x] Author is aware of the performance implications of this PR as
reported in the benchmarks PR comment.

## Reviewer Checklist

- [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 is aware of, and discussed the performance implications
of this PR as reported in the benchmarks PR comment.

---------

Co-authored-by: Alberto Vara <[email protected]>
Co-authored-by: Juanjo Alvarez Martinez <[email protected]>
Co-authored-by: Gabriele N. Tornetta <[email protected]>
Co-authored-by: Christophe Papazian <[email protected]>

Author: 5 people

ddtrace/debugging/_debugger.py

@@ -1,5 +1,6 @@
 from collections import defaultdict
 from itertools import chain
+import os
 import sys
 import threading
 from types import FunctionType
@@ -57,6 +58,7 @@
 from ddtrace.internal.remoteconfig import RemoteConfig
 from ddtrace.internal.safety import _isinstance
 from ddtrace.internal.service import Service
+from ddtrace.internal.utils.formats import asbool
 from ddtrace.internal.wrapping import Wrapper
 
 
@@ -245,6 +247,12 @@ def __init__(self, tracer=None):
             raise_on_exceed=False,
         )
 
+        # TODO: this is only temporary and will be reverted once the DD_REMOTE_CONFIGURATION_ENABLED variable
+        #  has been removed
+        if asbool(os.environ.get("DD_REMOTE_CONFIGURATION_ENABLED", True)) is False:
+            os.environ["DD_REMOTE_CONFIGURATION_ENABLED"] = "true"
+            log.info("Disabled Remote Configuration enabled by Dynamic Instrumentation.")
+
         # Register the debugger with the RCM client.
         RemoteConfig.register("LIVE_DEBUGGING", self.__rc_adapter__(self._on_configuration))
 

ddtrace/internal/remoteconfig/__init__.py

@@ -46,7 +46,9 @@ def _check_remote_config_enable_in_agent(cls):
     @classmethod
     def enable(cls):
         # type: () -> bool
-        if cls._check_remote_config_enable_in_agent():
+        # TODO: this is only temporary. DD_REMOTE_CONFIGURATION_ENABLED variable will be deprecated
+        rc_env_enabled = asbool(os.environ.get("DD_REMOTE_CONFIGURATION_ENABLED", "true"))
+        if rc_env_enabled and cls._check_remote_config_enable_in_agent():
             with cls._worker_lock:
                 if cls._worker is None:
                     cls._worker = RemoteConfigWorker()

releasenotes/notes/rcm-fix-disable-f45787a4e6576f53.yaml

@@ -0,0 +1,4 @@
+---
+fixes:
+  - |
+    remote config: Ensure DD_REMOTE_CONFIGURATION_ENABLED environment variable disables remote config if set to False

tests/contrib/gunicorn/test_gunicorn.py

@@ -108,7 +108,7 @@ def gunicorn_server(gunicorn_server_settings, tmp_path):
         cmd = ["ddtrace-run"]
     cmd += ["gunicorn", "--config", str(cfg_file), str(gunicorn_server_settings.app_path)]
     print("Running %r with configuration file %s" % (" ".join(cmd), cfg))
-
+    gunicorn_server_settings.env["DD_REMOTE_CONFIGURATION_ENABLED"] = "true"
     server_process = subprocess.Popen(
         cmd,
         env=gunicorn_server_settings.env,

tests/internal/remoteconfig/test_remoteconfig.py

@@ -92,40 +92,58 @@ def get_mock_encoded_msg(msg):
 @mock.patch.object(RemoteConfig, "_check_remote_config_enable_in_agent")
 def test_remote_config_register_auto_enable(mock_check_remote_config_enable_in_agent):
     # ASM_FEATURES product is enabled by default, but LIVE_DEBUGGER isn't
+    with override_env(dict(DD_REMOTE_CONFIGURATION_ENABLED="true")):
+        assert RemoteConfig._worker is None
+
+        RemoteConfig.register("LIVE_DEBUGGER", lambda m, c: None)
+
+        assert RemoteConfig._worker._client._products["LIVE_DEBUGGER"] is not None
+
+        RemoteConfig.disable()
+
+
+def test_remote_config_register_validate_rc_disabled():
     assert RemoteConfig._worker is None
 
-    mock_check_remote_config_enable_in_agent.return_value = True
-    RemoteConfig.register("LIVE_DEBUGGER", lambda m, c: None)
+    with override_env(dict(DD_REMOTE_CONFIGURATION_ENABLED="false")):
+        RemoteConfig.register("LIVE_DEBUGGER", lambda m, c: None)
 
-    assert RemoteConfig._worker._client._products["LIVE_DEBUGGER"] is not None
+        assert RemoteConfig._worker is None
 
-    RemoteConfig.disable()
 
+def test_remote_config_enable_validate_rc_disabled():
     assert RemoteConfig._worker is None
 
+    with override_env(dict(DD_REMOTE_CONFIGURATION_ENABLED="false")):
+        RemoteConfig.enable()
+
+        assert RemoteConfig._worker is None
+
 
 @pytest.mark.subprocess
 def test_remote_config_forksafe():
     import mock
 
     from ddtrace.internal.remoteconfig import RemoteConfig
+    from tests.utils import override_env
 
-    with mock.patch.object(
-        RemoteConfig, "_check_remote_config_enable_in_agent"
-    ) as mock_check_remote_config_enable_in_agent:
-        mock_check_remote_config_enable_in_agent.return_value = True
+    with override_env(dict(DD_REMOTE_CONFIGURATION_ENABLED="true")):
+        with mock.patch.object(
+            RemoteConfig, "_check_remote_config_enable_in_agent"
+        ) as mock_check_remote_config_enable_in_agent:
+            mock_check_remote_config_enable_in_agent.return_value = True
 
-        import os
+            import os
 
-        RemoteConfig.enable()
+            RemoteConfig.enable()
 
-        parent_worker = RemoteConfig._worker
-        assert parent_worker is not None
+            parent_worker = RemoteConfig._worker
+            assert parent_worker is not None
 
-        if os.fork() == 0:
-            assert RemoteConfig._worker is not None
-            assert RemoteConfig._worker is not parent_worker
-            exit(0)
+            if os.fork() == 0:
+                assert RemoteConfig._worker is not None
+                assert RemoteConfig._worker is not parent_worker
+                exit(0)
 
 
 @mock.patch.object(RemoteConfigClient, "_send_request")
@@ -139,7 +157,7 @@ def _reload_features(self, metadata, features):
 
     callback = Callback()
 
-    with override_env(dict(DD_REMOTE_CONFIG_POLL_INTERVAL_SECONDS="0.1")):
+    with override_env(dict(DD_REMOTE_CONFIGURATION_ENABLED="true", DD_REMOTE_CONFIG_POLL_INTERVAL_SECONDS="0.1")):
         mock_check_remote_config_enable_in_agent.return_value = True
         mock_send_request.return_value = get_mock_encoded_msg(b'{"asm":{"enabled":true}}')
         rc = RemoteConfig()
@@ -183,7 +201,7 @@ def _reload_features(self, metadata, features):
 
     callback = Callback()
 
-    with override_env(dict(DD_REMOTE_CONFIG_POLL_INTERVAL_SECONDS="0.1")):
+    with override_env(dict(DD_REMOTE_CONFIGURATION_ENABLED="true", DD_REMOTE_CONFIG_POLL_INTERVAL_SECONDS="0.1")):
         mock_check_remote_config_enable_in_agent.return_value = True
         mock_send_request.return_value = get_mock_encoded_msg(
             b'{"rules_data": [{"data": [{"expiration": 1662804872, "value": "127.0.0.0"}, '