Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .changelog/5329.changed
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
opentelemetry-sdk: revert BoundedAttributes RLock back to Lock
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ def __init__(
MutableMapping[str, types.AnyValue]
| OrderedDict[str, types.AnyValue]
) = {}
self._lock = threading.RLock()
self._lock = threading.Lock()
if attributes:
for key, value in attributes.items():
self[key] = value
Expand Down
50 changes: 40 additions & 10 deletions opentelemetry-api/tests/attributes/test_attributes.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
# type: ignore

import copy
import logging
import threading
import unittest
import unittest.mock
from collections.abc import MutableSequence
Expand Down Expand Up @@ -269,19 +271,47 @@ def test_immutable(self):
with self.assertRaises(TypeError):
bdict["should-not-work"] = "dict immutable"

def test_locking(self):
"""Supporting test case for a commit titled: Fix class BoundedAttributes to have RLock rather than Lock. See #3858.
The change was introduced because __iter__ of the class BoundedAttributes holds lock, and we observed some deadlock symptoms
in the codebase. This test case is to verify that the fix works as expected.
def test_no_deadlock_on_reentrant_logging(self):
"""Regression test for #3858.

The deadlock scenario: a logging handler intercepts the warning
emitted by _clean_attribute for an invalid value and calls __setitem__
on the same BoundedAttributes instance from the same thread.
With _clean_attribute called inside the lock this caused a deadlock.
With _clean_attribute called before the lock is acquired, no deadlock
occurs.
"""
bdict = BoundedAttributes(immutable=False)

with bdict._lock: # pylint: disable=protected-access
for num in range(100):
bdict[str(num)] = num

for num in range(100):
self.assertEqual(bdict[str(num)], num)
class ReentrantHandler(logging.Handler):
def emit(self, _record):
# Simulates Sentry intercepting the OTel warning and writing
# back into the same BoundedAttributes on the same thread.
bdict["reentrant.key"] = "set_by_handler"

otel_logger = logging.getLogger("opentelemetry.attributes")
handler = ReentrantHandler()
otel_logger.addHandler(handler)
try:
completed = threading.Event()

def run():
# None is an invalid attribute value and triggers _logger.warning
# in _clean_attribute, which fires the ReentrantHandler above.
bdict["trigger.key"] = None
completed.set()

thread = threading.Thread(target=run, daemon=True)
thread.start()
thread.join(timeout=2.0)

self.assertTrue(
completed.is_set(),
"Deadlock detected: __setitem__ did not complete within 2s",
)
self.assertEqual(bdict.get("reentrant.key"), "set_by_handler")
finally:
otel_logger.removeHandler(handler)

# pylint: disable=no-self-use
def test_extended_attributes(self):
Expand Down
Loading