fix(profiling): add debug asserts for our assumption on the stack frame

[taegyunkim]

As suggested in #9615 (comment)

Checklist

• Change(s) are motivated and described in the PR description
• Testing strategy is described if automated tests are not included in the PR
• Risks are described (performance impact, potential for breakage, maintainability)
• Change is maintainable (easy to change, telemetry, documentation)
• Library release note guidelines are followed or label changelog/no-changelog is set
• Documentation is included (in-code, generated user docs, public corp docs)
• Backport labels are set (if applicable)
• If this PR changes the public interface, I've notified @DataDog/apm-tees.

Reviewer Checklist

• Title is accurate
• All changes are related to the pull request's stated goal
• Description motivates each change
• Avoids breaking API changes
• Testing strategy adequately addresses listed risks
• Change is maintainable (easy to change, telemetry, documentation)
• Release note makes sense to a user of the library
• Author has 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

[datadog-dd-trace-py-rkomorn]

Datadog Report

Branch report: taegyunkim/lock-asserts
Commit report: 1a42a60
Test service: dd-trace-py

✅ 0 Failed, 175319 Passed, 1179 Skipped, 11h 29m 53.36s Total duration (27m 16.57s time saved)
❄️ 1 New Flaky

New Flaky Tests (1)

• test_iter_events_multi_thread - test_memalloc.py - Last Failure

  Expand for error

   assert 766 >= 1000
  

[pr-commenter]

Benchmarks

Benchmark execution time: 2024-07-01 17:58:12

Comparing candidate commit 1a42a60 in PR branch taegyunkim/lock-asserts with baseline commit 9c9b5a7 in branch main.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 221 metrics, 9 unstable metrics.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 0% with 60 lines in your changes missing coverage. Please review.

Project coverage is 27.23%. Comparing base (4acfde7) to head (6f18866).
Report is 13 commits behind head on main.

❗ Current head 6f18866 differs from pull request most recent head 1a42a60

Please upload reports for the commit 1a42a60 to get more accurate results.

Files	Patch %	Lines
tests/profiling/collector/test_threading.py	0.00%	49 Missing ⚠️
ddtrace/profiling/collector/_lock.py	0.00%	10 Missing ⚠️
ddtrace/settings/profiling.py	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #9672       +/-   ##
===========================================
- Coverage   74.65%   27.23%   -47.42%     
===========================================
  Files        1381     1370       -11     
  Lines      127965   127928       -37     
===========================================
- Hits        95531    34843    -60688     
- Misses      32434    93085    +60651     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[sanchda]

Yeah, I think this looks good to me. Checking config is a pretty reasonable strategy, since it'll just be the lookup for the object and then the subsequent attribute. I think that's in the same category as wrapping an os.environ check, and IMHO much more readable and intuitive.

I also see that we dropped the WRAPT_C_EXT check. I'm assuming that this is OK due to how we vendor wrapt, but I don't know for certain.

[taegyunkim]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• chore(profiling): add tests for wrapt disable extensions #9688
• fix(profiling): add debug asserts for our assumption on the stack frame #9672 👈
• add debug asserts #9687
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @taegyunkim and the rest of your teammates on Graphite

[taegyunkim]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• chore(profiling): add tests for wrapt disable extensions #9688
• fix(profiling): add debug asserts for our assumption on the stack frame #9672 👈
• add debug asserts #9687
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @taegyunkim and the rest of your teammates on Graphite

[taegyunkim]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• chore(profiling): add tests for wrapt disable extensions #9688
• fix(profiling): add debug asserts for our assumption on the stack frame #9672 👈
• add debug asserts #9687
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @taegyunkim and the rest of your teammates on Graphite