fix(tracing): Fix add_query_source with modules outside of project root

[rominf]

Fix: #3312

Previously, when packages added in in_app_include were installed to a location outside of the project root directory, span from those packages were not extended with OTel compatible source code information. Cases include running Python from virtualenv created outside of the project root directory or Python packages installed into the system using package managers. This resulted in an inconsistency: spans from the same project would be different, depending on the deployment method.

The first change moves should_be_included logic into the function to enable unit testing.

In the second change, the logic was slightly changed to avoid these discrepancies and conform to the requirements, described in the PR with a better setting of in-app in stack frames: #1894 (comment).

---

General Notes

Thank you for contributing to sentry-python!

Please add tests to validate your changes, and lint your code using tox -e linters.

Running the test suite on your PR might require maintainer approval. Some tests (AWS Lambda) additionally require a maintainer to add a special label to run and will fail if the label is not present.

For maintainers

Sensitive test suites require maintainer review to ensure that tests do not compromise our secrets. This review must be repeated after any code revisions.

Before running sensitive test suites, please carefully check the PR. Then, apply the Trigger: tests using secrets label. The label will be removed after any code changes to enforce our policy requiring maintainers to review all code revisions before running sensitive tests.

[antonpirker]

Hey @rominf ,
I just glanced over it, but thanks for this contribution and having a great description in issue and PR!
We will look at this next week!

[antonpirker]

FYI @rominf ignore the failing AWS tests, they need to be started by us for contributions from outside our organisation.

[rominf]

I noticed some mistakes in the first commit message in the reproduction steps. Since I've fixed them in the issue, I'll push updated commits when/if needed, to avoid extra CI runs.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.65%. Comparing base (6814df9) to head (c6ce9c7).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3313      +/-   ##
==========================================
- Coverage   79.68%   79.65%   -0.04%     
==========================================
  Files         136      136              
  Lines       14666    14669       +3     
  Branches     3080     3080              
==========================================
- Hits        11687    11684       -3     
- Misses       2139     2150      +11     
+ Partials      840      835       -5     

Files with missing lines	Coverage Δ
sentry_sdk/tracing_utils.py	83.56% <100.00%> (+0.42%)	⬆️

... and 4 files with indirect coverage changes

[rominf]

Comments have been addressed, and the code has become much simpler than in the first iteration. Thank you, @szokeasaurusrex, for asking me to split my PR into two (refactoring and actual logic change)!

[szokeasaurusrex]

Hey @rominf, I am a bit confused by this PR now that you split the PR into two pieces.

Has the logic changed at all in this PR, and if so, then how? Or, is this simply a refactor of the existing logic, plus a new test?

[rominf]

Hi @szokeasaurusrex. Yes, the logic has changed. Previously when packages added in in_app_include were installed to a location outside of the project root directory, span from those packages were not extended with OTel compatible source code information. This issue and reproduction steps are described in #3312.

In the first commit I introduce testing suite with one test marked with xfail, as it should pass according to my understanding of the requirements described in the comment of introduction PR, but it does not pass currently. You can check the failure of meeting requirements by doing a checkout of the first commit, running the testing suite, and observing XFAIL instead of XPASS:

$ git checkout -d rominf-tracing-fix-logic-in_app~
$ pytest tests/test_tracing_utils.py
...
tests/test_tracing_utils.py::test_should_be_included[Frame from system-wide installed Django with `django` in `in_app_include`-True] XFAIL  # <--- XFAIL shows that the test with marker `xfail` failed

In the second commit I update the program logic to make it pass and remove xfail marker (as you can see, all tests pass now). I split should_be_included to two variables (should_be_included and should_be_excluded) to model the requirements referenced above.

I believe it is easier to review the logic in the commits by comparing the requirements with the code and checking that all cases are covered, but not by looking at the diff. If you think the logic is not fully covered by tests, please tell me about my oversight – I am open to add yet another test(s) for full coverage.