Skip to content

Commit ac444eb

Browse files
authored
fix(tracing): ensure integrations can be disabled (backport #5856 to 1.13) (#5864)
Resolves: #5458 ## 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] OPTIONAL: PR description includes explicit acknowledgement of the performance implications of the change 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 has explicitly acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment.
1 parent c3e64c2 commit ac444eb

File tree

5 files changed

+49
-67
lines changed

5 files changed

+49
-67
lines changed

ddtrace/_monkey.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
"asyncio": True,
2929
"boto": True,
3030
"botocore": True,
31-
"bottle": False,
31+
"bottle": True,
3232
"cassandra": True,
3333
"celery": True,
3434
"consul": True,
@@ -72,9 +72,9 @@
7272
"kombu": False,
7373
"starlette": True,
7474
# Ignore some web framework integrations that might be configured explicitly in code
75-
"falcon": False,
76-
"pylons": False,
77-
"pyramid": False,
75+
"falcon": True,
76+
"pylons": True,
77+
"pyramid": True,
7878
# Auto-enable logging if the environment variable DD_LOGS_INJECTION is true
7979
"logging": config.logs_injection,
8080
"pynamodb": True,

ddtrace/bootstrap/sitecustomize.py

Lines changed: 4 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -71,26 +71,6 @@
7171
)
7272

7373

74-
EXTRA_PATCHED_MODULES = {
75-
"bottle": True,
76-
"django": True,
77-
"falcon": True,
78-
"flask": True,
79-
"pylons": True,
80-
"pyramid": True,
81-
}
82-
83-
84-
def update_patched_modules():
85-
modules_to_patch = os.getenv("DD_PATCH_MODULES")
86-
if not modules_to_patch:
87-
return
88-
89-
modules = parse_tags_str(modules_to_patch)
90-
for module, should_patch in modules.items():
91-
EXTRA_PATCHED_MODULES[module] = asbool(should_patch)
92-
93-
9474
if PY2:
9575
_unloaded_modules = []
9676

@@ -220,15 +200,16 @@ def _(threading):
220200
tracer.configure(**opts)
221201

222202
if trace_enabled:
223-
update_patched_modules()
224203
from ddtrace import patch_all
225204

226205
# We need to clean up after we have imported everything we need from
227206
# ddtrace, but before we register the patch-on-import hooks for the
228207
# integrations.
229208
cleanup_loaded_modules()
230-
231-
patch_all(**EXTRA_PATCHED_MODULES)
209+
modules_to_patch = os.getenv("DD_PATCH_MODULES")
210+
modules_to_str = parse_tags_str(modules_to_patch)
211+
modules_to_bool = {k: asbool(v) for k, v in modules_to_str.items()}
212+
patch_all(**modules_to_bool)
232213
else:
233214
cleanup_loaded_modules()
234215

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
fixes:
3+
- |
4+
tracing: Resolves an issue where ``DD_TRACE_<INTEGRATION>_ENABLED=False`` could not be used to disable
5+
the following integrations when ``ddtrace-run`` was used: flask, django, bottle, falcon, and pyramid.
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
if __name__ == "__main__":
2+
from ddtrace._monkey import _PATCHED_MODULES
3+
4+
assert (
5+
"flask" in _PATCHED_MODULES
6+
), "DD_PATCH_MODULES enables flask, this should override any value set by DD_TRACE_FLASK_ENABLED"
7+
8+
assert (
9+
"gevent" in _PATCHED_MODULES
10+
), "DD_PATCH_MODULES enables gevent, this should override any value set by DD_TRACE_GEVENT_ENABLED"
11+
12+
assert (
13+
"django" not in _PATCHED_MODULES
14+
), "DD_PATCH_MODULES enables django, this setting should override all other configurations"
15+
16+
assert (
17+
"boto" in _PATCHED_MODULES
18+
), "DD_PATCH_MODULES disables boto, this setting should override all other configurations"
19+
20+
assert (
21+
"falcon" not in _PATCHED_MODULES
22+
), "DD_PATCH_MODULES disables falcon, this setting override any value set by DD_TRACE_FALCON_ENABLED"
23+
24+
print("Test success")

tests/commands/test_runner.py

Lines changed: 12 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -157,46 +157,18 @@ def test_patch_modules_from_env(self):
157157
"""
158158
DD_PATCH_MODULES overrides the defaults for patch_all()
159159
"""
160-
from ddtrace.bootstrap.sitecustomize import EXTRA_PATCHED_MODULES
161-
from ddtrace.bootstrap.sitecustomize import update_patched_modules
162-
163-
orig = EXTRA_PATCHED_MODULES.copy()
164-
165-
# empty / malformed strings are no-ops
166-
with self.override_env(dict(DD_PATCH_MODULES="")):
167-
update_patched_modules()
168-
assert orig == EXTRA_PATCHED_MODULES
169-
170-
with self.override_env(dict(DD_PATCH_MODULES=":")):
171-
update_patched_modules()
172-
assert orig == EXTRA_PATCHED_MODULES
173-
174-
with self.override_env(dict(DD_PATCH_MODULES=",")):
175-
update_patched_modules()
176-
assert orig == EXTRA_PATCHED_MODULES
177-
178-
with self.override_env(dict(DD_PATCH_MODULES=",:")):
179-
update_patched_modules()
180-
assert orig == EXTRA_PATCHED_MODULES
181-
182-
# overrides work in either direction
183-
with self.override_env(dict(DD_PATCH_MODULES="django:false")):
184-
update_patched_modules()
185-
assert EXTRA_PATCHED_MODULES["django"] is False
186-
187-
with self.override_env(dict(DD_PATCH_MODULES="boto:true")):
188-
update_patched_modules()
189-
assert EXTRA_PATCHED_MODULES["boto"] is True
190-
191-
with self.override_env(dict(DD_PATCH_MODULES="django:true,boto:false")):
192-
update_patched_modules()
193-
assert EXTRA_PATCHED_MODULES["boto"] is False
194-
assert EXTRA_PATCHED_MODULES["django"] is True
195-
196-
with self.override_env(dict(DD_PATCH_MODULES="django:false,boto:true")):
197-
update_patched_modules()
198-
assert EXTRA_PATCHED_MODULES["boto"] is True
199-
assert EXTRA_PATCHED_MODULES["django"] is False
160+
with self.override_env(
161+
env=dict(
162+
DD_PATCH_MODULES="flask:true,gevent:true,django:false,boto:true,falcon:false",
163+
DD_TRACE_FLASK_ENABLED="false",
164+
DD_TRACE_GEVENT_ENABLED="false",
165+
DD_TRACE_FALCON_ENABLED="true",
166+
)
167+
):
168+
out = subprocess.check_output(
169+
["ddtrace-run", "python", "tests/commands/ddtrace_run_patched_modules_overrides.py"],
170+
)
171+
assert out.startswith(b"Test success"), out
200172

201173
def test_sitecustomize_without_ddtrace_run_command(self):
202174
# [Regression test]: ensure `sitecustomize` path is removed only if it's

0 commit comments

Comments
 (0)