Skip to content

Commit

Permalink
fix(debugging): ensure dynamic instrumentation enabled (#6618)
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
P403n1x87 authored Aug 25, 2023
1 parent ba5ae85 commit c98875a
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 2 deletions.
6 changes: 6 additions & 0 deletions ddtrace/debugging/_debugger.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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)
Expand All @@ -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):
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
10 changes: 8 additions & 2 deletions tests/debugging/test_api.py
Original file line number Diff line number Diff line change
@@ -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():
Expand All @@ -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)
Expand Down

0 comments on commit c98875a

Please sign in to comment.