Skip to content

Commit

Permalink
fix(profiling): add debug asserts for our stack assumptions (#9689) […
Browse files Browse the repository at this point in the history
…backport-2.10] (#10065)

Manual backport of #9689 to 2.10

## 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] Risks are described (performance impact, potential for breakage,
maintainability)
- [x] Change is maintainable (easy to change, telemetry, documentation)
- [x] [Library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
are followed or label `changelog/no-changelog` is set
- [x] Documentation is included (in-code, generated user docs, [public
corp docs](https://github.com/DataDog/documentation/))
- [x] Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))
- [x] If this PR changes the public interface, I've notified
`@DataDog/apm-tees`.

## Reviewer Checklist
- [x] Title is accurate
- [x] All changes are related to the pull request's stated goal
- [x] Description motivates each change
- [x] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes
- [x] Testing strategy adequately addresses listed risks
- [x] Change is maintainable (easy to change, telemetry, documentation)
- [x] Release note makes sense to a user of the library
- [x] Author has acknowledged and discussed the performance implications
of this PR as reported in the benchmarks PR comment
- [x] Backport labels are set in a manner that is consistent with the
[release branch maintenance

policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)

(cherry picked from commit 0767725)
  • Loading branch information
taegyunkim authored Aug 5, 2024
1 parent 337a2d5 commit a554900
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 2 deletions.
10 changes: 8 additions & 2 deletions ddtrace/profiling/collector/_lock.py
Original file line number Diff line number Diff line change
Expand Up @@ -260,8 +260,14 @@ def _get_lock_call_loc_with_name(self) -> typing.Optional[str]:
# 1: _acquire/_release
# 2: acquire/release (or __enter__/__exit__)
# 3: caller frame
# And we expect additional frame if WRAPT_C_EXT is False
frame = sys._getframe(3 if WRAPT_C_EXT else 4)
if config.enable_asserts:
frame = sys._getframe(1)
if frame.f_code.co_name not in {"_acquire", "_release"}:
raise AssertionError("Unexpected frame %s" % frame.f_code.co_name)
frame = sys._getframe(2)
if frame.f_code.co_name not in {"acquire", "release", "__enter__", "__exit__"}:
raise AssertionError("Unexpected frame %s" % frame.f_code.co_name)
frame = sys._getframe(3)
code = frame.f_code
call_loc = "%s:%d" % (os.path.basename(code.co_filename), frame.f_lineno)

Expand Down
8 changes: 8 additions & 0 deletions ddtrace/settings/profiling.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,14 @@ class ProfilingConfig(En):
help="The tags to apply to uploaded profile. Must be a list in the ``key1:value,key2:value2`` format",
)

enable_asserts = En.v(
bool,
"enable_asserts",
default=False,
help_type="Boolean",
help="Whether to enable debug assertions in the profiler code",
)

class Stack(En):
__item__ = __prefix__ = "stack"

Expand Down
3 changes: 3 additions & 0 deletions riotfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -2677,6 +2677,9 @@ def select_pys(min_version=MIN_PYTHON_VERSION, max_version=MAX_PYTHON_VERSION):
name="profile",
# NB riot commands that use this Venv must include --pass-env to work properly
command="python -m tests.profiling.run pytest -v --no-cov --capture=no --benchmark-disable {cmdargs} tests/profiling", # noqa: E501
env={
"DD_PROFILING_ENABLE_ASSERTS": "1",
},
pkgs={
"gunicorn": latest,
#
Expand Down

0 comments on commit a554900

Please sign in to comment.