You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
fix(sanic): avoids unloading re module when gevent is installed (backport #5867 to 1.9) (#5900)
## Background
When gevent is installed (or
`DD_UNLOAD_MODULES_FROM_SITECUSTOMIZE=auto`), all modules imported by
[ddtrace.bootstrap.sitecustomize](https://github.com/DataDog/dd-trace-py/blob/1.x/ddtrace/bootstrap/sitecustomize.py#L91)
are unloaded from sys.modules (except for a small [exclude
list](https://github.com/DataDog/dd-trace-py/blob/1.x/ddtrace/bootstrap/sitecustomize.py#L103-L113)).
## Description
Currently the [typing
module](https://github.com/DataDog/dd-trace-py/blob/1.x/ddtrace/bootstrap/sitecustomize.py#L110)
is excluded from module unloading but re is NOT. This causes
[isinstance(some_re_pattern,
typing.Pattern)](https://github.com/sanic-org/sanic-routing/blob/main/sanic_routing/router.py#L289)
to return False (here typing holds a stale reference to
[re.Pattern](https://github.com/python/cpython/blob/main/Lib/typing.py#L3422)).
## Risks
This bug was called out and reproduced in the following github issue:
#5722. However this issue
could not be reproduced using riot due to how packages are installed. To
workaround this limitation a framework test was added for sanic which
reproduced this error. This is less than ideal. Module cloning is a
vital part of our codebase and should have test coverage for all common
use-cases.
### Explanation
Riot uses `pip install -e` to install dependencies. When packages are
installed with the `-e` option a `.pth` file is added to
`site-packages/` which get run even before `sitecustomize.py`. This is
done to extend the Python path to include additional packages. The
creation of the `.pth` file has a side-effect of that masks this bug. It
loads the re module before importing ddtrace and before LOADED_MODULES
can get set, so it is basically the same impact as ignoring/not cleaning
up re module.
## 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.
Co-authored-by: Brett Langdon <[email protected]>
Copy file name to clipboardExpand all lines: .github/workflows/test_frameworks.yml
+27Lines changed: 27 additions & 0 deletions
Original file line number
Diff line number
Diff line change
@@ -52,6 +52,33 @@ jobs:
52
52
# log output and it contains phony error messages.
53
53
run: PYTHONPATH=../ddtrace/tests/debugging/exploration/ ddtrace-run pytest test --continue-on-collection-errors -v -k 'not test_simple'
54
54
55
+
sanic-testsuite-22_12:
56
+
runs-on: ubuntu-20.04
57
+
env:
58
+
# Regression test for DataDog/dd-trace-py/issues/5722
59
+
DD_UNLOAD_MODULES_FROM_SITECUSTOMIZE: true
60
+
defaults:
61
+
run:
62
+
working-directory: sanic
63
+
steps:
64
+
- uses: actions/checkout@v3
65
+
with:
66
+
path: ddtrace
67
+
- uses: actions/checkout@v3
68
+
with:
69
+
repository: sanic-org/sanic
70
+
ref: v22.12.0
71
+
path: sanic
72
+
- uses: actions/setup-python@v4
73
+
with:
74
+
python-version: "3.11"
75
+
- name: Install sanic and dependencies required to run tests
76
+
run: pip3 install '.[test]' aioquic
77
+
- name: Install ddtrace
78
+
run: pip3 install ../ddtrace
79
+
- name: Run tests
80
+
run: ddtrace-run pytest -k "not test_add_signal and not test_ode_removes and not test_skip_touchup and not test_dispatch_signal_triggers and not test_keep_alive_connection_context"
0 commit comments