Commit 979e756
authored
fix(profiling): fix silent suppression of AssertionError exceptions in _ProfiledLock::_acquire (#15089)
https://datadoghq.atlassian.net/browse/PROF-12903
## Description
`_ProfiledLock::_maybe_update_self_name` raises `AssertionError` in two
places when `config.enable_asserts == True`.
However, they're being silently suppressed by the try/catch block in the
caller, `_ProfiledLock::_acquire`, method.
Current estimated impact is low, as it's only ever triggered for
development workflows, but still, worth making it right. This PR fixes
the bug by propagating `AssertionError`'s by the caller.
## Testing
Added two new tests: one for `AssertionError` and another for a
non-`AssertionError` exception.
**PROD**
_Test fails, since the `AssertionError` exception is not propagated_
```
$ ./scripts/ddtest riot -v run --pass-env d667cb4 -- -k test_assertion_error_suppressed_in_acquire
...
FAILED tests/profiling_v2/collector/test_threading.py::TestThreadingLockCollector::test_assertion_error_suppressed_in_acquire - Failed: DID NOT RAISE <class 'AssertionError'>
FAILED tests/profiling_v2/collector/test_threading.py::TestThreadingRLockCollector::test_assertion_error_suppressed_in_acquire - Failed: DID NOT RAISE <class 'AssertionError'>
```
_Highlighted code is for demonstration purposes only - not being merged_
<img width="703" height="735" alt="Screenshot 2025-10-30 at 10 36 28 AM"
src="https://github.com/user-attachments/assets/4c3d8066-3aa2-465e-8386-0f13a3de0cf6"
/>
**BRANCH**
_Test passes_
```
$ ./scripts/ddtest riot -v run --pass-env d667cb4 -- -k test_assertion_error_suppressed_in_acquire
...
tests/profiling_v2/collector/test_threading.py::TestThreadingLockCollector::test_assertion_error_suppressed_in_acquire PASSED
tests/profiling_v2/collector/test_threading.py::TestThreadingRLockCollector::test_assertion_error_suppressed_in_acquire PASSED
```
## Risks
none
[PROF-12903]:
https://datadoghq.atlassian.net/browse/PROF-12903?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ1 parent 06ea45c commit 979e756
3 files changed
Lines changed: 86 additions & 17 deletions
File tree
- ddtrace/profiling/collector
- releasenotes/notes
- tests/profiling_v2/collector
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
29 | 29 | | |
30 | 30 | | |
31 | 31 | | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
32 | 38 | | |
33 | 39 | | |
34 | 40 | | |
| |||
91 | 97 | | |
92 | 98 | | |
93 | 99 | | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
94 | 104 | | |
| 105 | + | |
95 | 106 | | |
96 | 107 | | |
97 | 108 | | |
| |||
202 | 213 | | |
203 | 214 | | |
204 | 215 | | |
205 | | - | |
206 | | - | |
207 | | - | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
208 | 219 | | |
209 | | - | |
210 | | - | |
211 | | - | |
212 | | - | |
213 | | - | |
214 | | - | |
215 | | - | |
216 | | - | |
217 | | - | |
218 | | - | |
| 220 | + | |
| 221 | + | |
219 | 222 | | |
220 | 223 | | |
221 | | - | |
222 | | - | |
223 | | - | |
224 | | - | |
| 224 | + | |
| 225 | + | |
225 | 226 | | |
226 | 227 | | |
227 | 228 | | |
| |||
Lines changed: 4 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
360 | 360 | | |
361 | 361 | | |
362 | 362 | | |
| 363 | + | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
| 370 | + | |
| 371 | + | |
| 372 | + | |
| 373 | + | |
| 374 | + | |
| 375 | + | |
| 376 | + | |
| 377 | + | |
| 378 | + | |
| 379 | + | |
| 380 | + | |
| 381 | + | |
| 382 | + | |
| 383 | + | |
| 384 | + | |
| 385 | + | |
| 386 | + | |
| 387 | + | |
| 388 | + | |
| 389 | + | |
| 390 | + | |
| 391 | + | |
| 392 | + | |
| 393 | + | |
| 394 | + | |
| 395 | + | |
| 396 | + | |
| 397 | + | |
| 398 | + | |
| 399 | + | |
| 400 | + | |
| 401 | + | |
| 402 | + | |
| 403 | + | |
| 404 | + | |
| 405 | + | |
| 406 | + | |
| 407 | + | |
| 408 | + | |
| 409 | + | |
| 410 | + | |
| 411 | + | |
| 412 | + | |
| 413 | + | |
| 414 | + | |
| 415 | + | |
| 416 | + | |
| 417 | + | |
| 418 | + | |
| 419 | + | |
| 420 | + | |
| 421 | + | |
| 422 | + | |
| 423 | + | |
| 424 | + | |
| 425 | + | |
| 426 | + | |
363 | 427 | | |
364 | 428 | | |
365 | 429 | | |
| |||
0 commit comments