From c98875a0db32d30ef7c6e9b658077888a1704cf7 Mon Sep 17 00:00:00 2001 From: "Gabriele N. Tornetta" Date: Fri, 25 Aug 2023 16:40:52 +0100 Subject: [PATCH] fix(debugging): ensure dynamic instrumentation enabled (#6618) With the introduction of exception debugging, the new enablement logic caused the dynamic instrumentation to not register with the remoteconfig client when the programmatic API was used, unless `DD_DYNAMIC_INSTRUMENTATION_ENABLED` is also set. Since this is not the documented behaviour, we make this fix to align with the expected behaviour of the programmatic API. ## 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/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)) ## Reviewer Checklist - [ ] Title is accurate. - [ ] No unnecessary changes are introduced. - [ ] Description motivates each change. - [ ] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [ ] Testing strategy adequately addresses listed risk(s). - [ ] Change is maintainable (easy to change, telemetry, documentation). - [ ] Release note makes sense to a user of the library. - [ ] Reviewer has explicitly 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/debugging/_debugger.py | 6 ++++++ ...ix-di-enable-programmatic-api-2dd583ac87af5451.yaml | 6 ++++++ tests/debugging/test_api.py | 10 ++++++++-- 3 files changed, 20 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/fix-di-enable-programmatic-api-2dd583ac87af5451.yaml diff --git a/ddtrace/debugging/_debugger.py b/ddtrace/debugging/_debugger.py index 22f9a32cd20..4a27995aa78 100644 --- a/ddtrace/debugging/_debugger.py +++ b/ddtrace/debugging/_debugger.py @@ -186,6 +186,8 @@ def enable(cls, run_module=False): log.debug("Enabling %s", cls.__name__) + di_config.enabled = True + cls.__watchdog__.install() if di_config.metrics: @@ -215,6 +217,8 @@ def disable(cls, join=True): log.debug("Disabling %s", cls.__name__) + remoteconfig_poller.unregister("LIVE_DEBUGGING") + forksafe.unregister(cls._restart) atexit.unregister(cls.disable) unregister_post_run_module_hook(cls._on_run_module) @@ -229,6 +233,8 @@ def disable(cls, join=True): if di_config.metrics: metrics.disable() + di_config.enabled = False + log.debug("%s disabled", cls.__name__) def __init__(self, tracer=None): diff --git a/releasenotes/notes/fix-di-enable-programmatic-api-2dd583ac87af5451.yaml b/releasenotes/notes/fix-di-enable-programmatic-api-2dd583ac87af5451.yaml new file mode 100644 index 00000000000..ef0cfe68ee0 --- /dev/null +++ b/releasenotes/notes/fix-di-enable-programmatic-api-2dd583ac87af5451.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + dynamic instrumentation: Fixed the programmatic API to ensure that the + dynamic instrumentation service is fully enabled when + ``Dynamic Instrumentation.enable()`` is called. diff --git a/tests/debugging/test_api.py b/tests/debugging/test_api.py index 5bdb99eda2a..58c8c12a399 100644 --- a/tests/debugging/test_api.py +++ b/tests/debugging/test_api.py @@ -1,7 +1,5 @@ import pytest -from ddtrace.debugging import DynamicInstrumentation - @pytest.mark.subprocess(ddtrace_run=True, env=dict(DD_DYNAMIC_INSTRUMENTATION_ENABLED="true"), err=None) def test_debugger_enabled_ddtrace_run(): @@ -17,12 +15,20 @@ def test_debugger_disabled_ddtrace_run(): assert DynamicInstrumentation._instance is None +@pytest.mark.subprocess def test_debugger_enabled_programmatically(): + from ddtrace.internal.remoteconfig.worker import remoteconfig_poller + + assert "LIVE_DEBUGGING" not in remoteconfig_poller._client._products + from ddtrace.debugging import DynamicInstrumentation + DynamicInstrumentation.enable() assert DynamicInstrumentation._instance is not None + assert "LIVE_DEBUGGING" in remoteconfig_poller._client._products DynamicInstrumentation.disable() assert DynamicInstrumentation._instance is None + assert "LIVE_DEBUGGING" not in remoteconfig_poller._client._products @pytest.mark.subprocess(err=None)