fix(pymongo): decode bytes values instead of str()-ing them to avoid BytesWarning (#4782)

sentry_sdk/integrations/pymongo.py

@@ -86,6 +86,36 @@ def _strip_pii(command: "Dict[str, Any]") -> "Dict[str, Any]":
     return command
 
 
+def _bytes_safe_str(value: "Any") -> str:
+    """
+    Convert a BSON value to a string without triggering ``BytesWarning``.
+
+    ``str()`` on a ``bytes`` instance emits ``BytesWarning`` under
+    ``python -b`` and produces a useless ``b'...'`` representation, so
+    decode instead. This is for human-readable BSON values (collection
+    names etc.); for opaque identifiers such as the ``lsid`` session id
+    use ``_bytes_to_hex()`` instead, since decoding random bytes as UTF-8
+    is lossy and would make different ids collide.
+    """
+    if isinstance(value, bytes):
+        return value.decode("utf-8", errors="replace")
+    return str(value)
+
+
+def _bytes_to_hex(value: "Any") -> str:
+    """
+    Render a binary identifier (e.g. the BSON ``lsid.id`` ``Binary``, a
+    ``bytes`` subclass) as a stable, lossless hex string.
+
+    ``str()`` on it emits ``BytesWarning`` under ``python -b`` (see #4782),
+    and decoding it as UTF-8 would be lossy: session ids are random bytes,
+    so replacement characters would make distinct ids collide.
+    """
+    if isinstance(value, bytes):
+        return value.hex()
+    return str(value)
+
+
 def _get_db_data(event: "Any") -> "Dict[str, Any]":
     data = {}
 
@@ -152,7 +182,7 @@ def started(self, event: "CommandStartedEvent") -> None:
             elif not should_send_default_pii():
                 command = _strip_pii(command)
 
-            query = json.dumps(command, default=str)
+            query = json.dumps(command, default=_bytes_safe_str)
 
             if has_span_streaming_enabled(client.options):
                 span_first_data = {
@@ -205,7 +235,7 @@ def started(self, event: "CommandStartedEvent") -> None:
                 try:
                     if lsid:
                         lsid_id = lsid["id"]
-                        data["operation_ids"]["session"] = str(lsid_id)
+                        data["operation_ids"]["session"] = _bytes_to_hex(lsid_id)
                 except KeyError:
                     pass
 

tests/integrations/pymongo/test_pymongo.py

@@ -1,11 +1,20 @@
+import datetime
+import warnings
+
 import pytest
+from bson.binary import Binary
 from mockupdb import MockupDB, OpQuery
 from pymongo import MongoClient
+from pymongo.monitoring import CommandStartedEvent, CommandSucceededEvent
 
 import sentry_sdk
 from sentry_sdk import capture_message, start_transaction
 from sentry_sdk.consts import SPANDATA
-from sentry_sdk.integrations.pymongo import PyMongoIntegration, _strip_pii
+from sentry_sdk.integrations.pymongo import (
+    CommandTracer,
+    PyMongoIntegration,
+    _strip_pii,
+)
 
 
 @pytest.fixture(scope="session")
@@ -840,3 +849,124 @@ def test_span_streaming_status_on_failure(sentry_init, capture_items, mongo_serv
 
     assert segment["name"] == "test_segment"
     assert db_span["status"] == "error"
+
+
+def test_bytes_safe_str():
+    """_bytes_safe_str decodes bytes instead of str()-ing them (#4782)."""
+    from sentry_sdk.integrations.pymongo import _bytes_safe_str, _bytes_to_hex
+
+    with warnings.catch_warnings():
+        warnings.simplefilter("error", BytesWarning)
+        assert _bytes_safe_str(b"test_collection") == "test_collection"
+        # Invalid UTF-8 must not raise, it is replaced instead
+        assert _bytes_safe_str(b"caf\xe9") == "caf\ufffd"
+        # Non-bytes values keep the normal str() behavior
+        assert _bytes_safe_str(42) == "42"
+        assert _bytes_safe_str("plain") == "plain"
+
+        # Binary identifiers must be hex-encoded, not decoded: session ids
+        # are random bytes, and UTF-8 decoding them is lossy (collisions).
+        assert _bytes_to_hex(b"\x00\x01\xff") == "0001ff"
+        assert _bytes_to_hex(42) == "42"
+        assert _bytes_to_hex("plain") == "plain"
+        # Distinct random ids stay distinct (the collision Bugbot flagged)
+        id1 = Binary(bytes(range(16)), 4)
+        id2 = Binary(bytes(range(1, 17)), 4)
+        assert _bytes_to_hex(id1) != _bytes_to_hex(id2)
+        assert _bytes_to_hex(id1) == "000102030405060708090a0b0c0d0e0f"
+
+
+def _make_started_event(command, request_id=1):
+    return CommandStartedEvent(
+        command=command,
+        database_name="test_db",
+        request_id=request_id,
+        connection_id=("localhost", 27017),
+        operation_id=request_id,
+    )
+
+
+def _make_succeeded_event(command_name, request_id=1):
+    return CommandSucceededEvent(
+        duration=datetime.timedelta(seconds=1),
+        reply={"ok": 1},
+        command_name=command_name,
+        request_id=request_id,
+        connection_id=("localhost", 27017),
+        operation_id=request_id,
+    )
+
+
+@pytest.mark.parametrize("with_pii", [False, True])
+def test_bytes_lsid_does_not_raise_byteswarning(sentry_init, capture_events, with_pii):
+    """
+    The BSON logical session id (``lsid.id``) is a ``Binary`` instance, which
+    is a subclass of ``bytes``. ``str()`` on it raises ``BytesWarning`` under
+    ``python -b`` (see #4782). Turn BytesWarning into an error and make sure
+    the span is created with a decoded session id.
+    """
+    sentry_init(
+        integrations=[PyMongoIntegration()],
+        traces_sample_rate=1.0,
+        send_default_pii=with_pii,
+    )
+    events = capture_events()
+
+    tracer = CommandTracer()
+    session_id = Binary(bytes(range(16)), 4)
+    started = _make_started_event(
+        {
+            "find": "test_collection",
+            "lsid": {"id": session_id},
+        }
+    )
+    succeeded = _make_succeeded_event("find")
+
+    with warnings.catch_warnings():
+        warnings.simplefilter("error", BytesWarning)
+        with start_transaction():
+            tracer.started(started)
+            tracer.succeeded(succeeded)
+
+    (event,) = events
+    (span,) = event["spans"]
+
+    session = span["data"]["operation_ids"]["session"]
+    assert isinstance(session, str)
+    assert "b'" not in session
+    # Random binary ids must round-trip losslessly (hex), not be decoded
+    # as UTF-8 -- decoding random bytes loses information and collides.
+    assert session == bytes(range(16)).hex()
+
+
+def test_bytes_collection_name_in_query_does_not_raise_byteswarning(
+    sentry_init, capture_events
+):
+    """
+    BSON values in the command can be ``bytes`` (e.g. a bytes collection
+    name). ``json.dumps(..., default=str)`` calls ``str()`` on them, which
+    raises ``BytesWarning`` under ``python -b`` (#4782). The query JSON must
+    contain the decoded name, not ``b'...'``.
+    """
+    sentry_init(
+        integrations=[PyMongoIntegration()],
+        traces_sample_rate=1.0,
+        send_default_pii=True,
+    )
+    events = capture_events()
+
+    tracer = CommandTracer()
+    started = _make_started_event({"find": b"test_collection", "limit": 1})
+    succeeded = _make_succeeded_event("find")
+
+    with warnings.catch_warnings():
+        warnings.simplefilter("error", BytesWarning)
+        with start_transaction():
+            tracer.started(started)
+            tracer.succeeded(succeeded)
+
+    (event,) = events
+    (span,) = event["spans"]
+
+    assert "test_collection" in span["description"]
+    assert "b'" not in span["description"]