dd-trace-py
dd-trace-py copied to clipboard
chore(tests): add testing to ensure patching does not import module
Following up on 5455 .
We should not be importing the module when we patch. This PR adds an assert to the patch testing to make sure that we do not. It should fail on Django, and likely other integrations currently. From there we'll need to implement fixes.
Checklist
- [ ] Change(s) are motivated and described in the PR description.
- [ ] Testing strategy is described if automated tests are not included in the PR.
- [ ] Risk is outlined (performance impact, potential for breakage, maintainability, etc).
- [ ] Change is maintainable (easy to change, telemetry, documentation).
- [ ] Library release note guidelines are followed.
- [ ] Documentation is included (in-code, generated user docs, public corp docs).
- [ ] PR description includes explicit acknowledgement/acceptance of the performance implications of this PR as reported in the benchmarks PR comment.
Reviewer Checklist
- [ ] Title is accurate.
- [ ] No unnecessary changes are introduced.
- [ ] Description motivates each change.
- [ ] Avoids breaking API 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.
Benchmarks
Comparing candidate commit 791c98627 in PR branch add_assert_module_not_imported_on_patch_testing
with baseline commit 7d875a495 in branch 1.x
.
Found 1 performance improvements and 0 performance regressions! Performance is the same for 93 cases.
scenario:flasksimple-tracer-and-profiler-and-appsec
- 🟩
execution_time
[-0.571ms; -0.473ms] or [-8.756%; -7.249%]
Should we convert this PR to a draft? There are 54 failing checks. Some additional work is probably required.