opentelemetry-sdk: revert RLock back to Lock (#5329)

* opentelemetry-sdk: revert RLock back to Lock

With the changes in clean_attributes, the original issue that required the reentrant lock in the first place is no longer an issue. Added a test that was run with the old code to validate that the regression does not occur.

Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com>

* changelog

Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com>

* Update .changelog/5329.changed

Co-authored-by: Riccardo Magliocchetti <riccardo.magliocchetti@gmail.com>

* fix lint

Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com>

* remove old test

Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com>

---------

Signed-off-by: Alex Boten <223565+codeboten@users.noreply.github.com>
Co-authored-by: Riccardo Magliocchetti <riccardo.magliocchetti@gmail.com>

Author: codeboten

.changelog/5329.changed

@@ -0,0 +1 @@
+opentelemetry-sdk: revert BoundedAttributes RLock back to Lock

opentelemetry-api/src/opentelemetry/attributes/__init__.py

@@ -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

opentelemetry-api/tests/attributes/test_attributes.py

@@ -4,6 +4,8 @@
 # type: ignore
 
 import copy
+import logging
+import threading
 import unittest
 import unittest.mock
 from collections.abc import MutableSequence
@@ -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):