Fix pymongo (#3688)

Author: sentrivana

MIGRATION_GUIDE.md

@@ -10,6 +10,7 @@ Looking to upgrade from Sentry SDK 2.x to 3.x? Here's a comprehensive list of wh
 ### Changed
 
 - The SDK now supports Python 3.7 and higher.
+- Transaction names can no longer contain commas and equals signs. If present, these characters will be stripped.
 - `sentry_sdk.start_span` now only takes keyword arguments.
 - `sentry_sdk.start_span` no longer takes an explicit `span` argument.
 - The `Span()` constructor does not accept a `hub` parameter anymore.
@@ -24,6 +25,8 @@ Looking to upgrade from Sentry SDK 2.x to 3.x? Here's a comprehensive list of wh
 
 - Spans no longer have a `description`. Use `name` instead.
 - Dropped support for Python 3.6.
+- The PyMongo integration no longer sets tags. The data is still accessible via span attributes.
+- The PyMongo integration doesn't set `operation_ids` anymore. The individual IDs (`operation_id`, `request_id`, `session_id`) are now accessible as separate span attributes.
 - `sentry_sdk.metrics` and associated metrics APIs have been removed as Sentry no longer accepts metrics data in this form. See https://sentry.zendesk.com/hc/en-us/articles/26369339769883-Upcoming-API-Changes-to-Metrics
 - The experimental options `enable_metrics`, `before_emit_metric` and `metric_code_locations` have been removed.
 - When setting span status, the HTTP status code is no longer automatically added as a tag.

sentry_sdk/integrations/pymongo.py

@@ -1,12 +1,11 @@
 import copy
-import json
 
 import sentry_sdk
 from sentry_sdk.consts import SPANSTATUS, SPANDATA, OP
 from sentry_sdk.integrations import DidNotEnable, Integration
 from sentry_sdk.scope import should_send_default_pii
 from sentry_sdk.tracing import Span
-from sentry_sdk.utils import capture_internal_exceptions
+from sentry_sdk.utils import capture_internal_exceptions, _serialize_span_attribute
 
 try:
     from pymongo import monitoring
@@ -127,56 +126,49 @@ def started(self, event):
             command.pop("$clusterTime", None)
             command.pop("$signature", None)
 
-            tags = {
-                "db.name": event.database_name,
+            data = {
+                SPANDATA.DB_NAME: event.database_name,
                 SPANDATA.DB_SYSTEM: "mongodb",
                 SPANDATA.DB_OPERATION: event.command_name,
                 SPANDATA.DB_MONGODB_COLLECTION: command.get(event.command_name),
             }
 
             try:
-                tags["net.peer.name"] = event.connection_id[0]
-                tags["net.peer.port"] = str(event.connection_id[1])
+                data["net.peer.name"] = event.connection_id[0]
+                data["net.peer.port"] = str(event.connection_id[1])
             except TypeError:
                 pass
 
-            data = {"operation_ids": {}}  # type: Dict[str, Any]
-            data["operation_ids"]["operation"] = event.operation_id
-            data["operation_ids"]["request"] = event.request_id
-
-            data.update(_get_db_data(event))
-
             try:
                 lsid = command.pop("lsid")["id"]
-                data["operation_ids"]["session"] = str(lsid)
+                data["session_id"] = str(lsid)
             except KeyError:
                 pass
 
             if not should_send_default_pii():
                 command = _strip_pii(command)
 
-            query = json.dumps(command, default=str)
+            query = _serialize_span_attribute(command)
             span = sentry_sdk.start_span(
                 op=OP.DB,
                 name=query,
                 origin=PyMongoIntegration.origin,
             )
 
-            for tag, value in tags.items():
-                # set the tag for backwards-compatibility.
-                # TODO: remove the set_tag call in the next major release!
-                span.set_tag(tag, value)
-
-                span.set_data(tag, value)
-
-            for key, value in data.items():
-                span.set_data(key, value)
-
             with capture_internal_exceptions():
                 sentry_sdk.add_breadcrumb(
-                    message=query, category="query", type=OP.DB, data=tags
+                    message=query, category="query", type=OP.DB, data=data
                 )
 
+            for key, value in data.items():
+                span.set_attribute(key, value)
+
+            for key, value in _get_db_data(event).items():
+                span.set_attribute(key, value)
+
+            span.set_attribute("operation_id", event.operation_id)
+            span.set_attribute("request_id", event.request_id)
+
             self._ongoing_operations[self._operation_key(event)] = span.__enter__()
 
     def failed(self, event):

sentry_sdk/tracing.py

@@ -1189,11 +1189,6 @@ class POTelSpan:
     OTel span wrapper providing compatibility with the old span interface.
     """
 
-    # XXX Maybe it makes sense to repurpose the existing Span class for this.
-    # For now I'm keeping this class separate to have a clean slate.
-
-    # XXX The wrapper itself should have as little state as possible
-
     def __init__(
         self,
         *,
@@ -1225,14 +1220,14 @@ def __init__(
                 # OTel timestamps have nanosecond precision
                 start_timestamp = convert_to_otel_timestamp(start_timestamp)
 
-            self._otel_span = tracer.start_span(
-                name or description or op or "", start_time=start_timestamp
-            )
+            span_name = self._sanitize_name(name or description or op or "")
+            self._otel_span = tracer.start_span(span_name, start_time=start_timestamp)
 
             self.origin = origin or DEFAULT_SPAN_ORIGIN
             self.op = op
             self.description = description
-            self.name = name
+            self.name = span_name
+
             if status is not None:
                 self.set_status(status)
 
@@ -1602,6 +1597,10 @@ def set_context(self, key, value):
 
         self.set_attribute(f"{SentrySpanAttribute.CONTEXT}.{key}", value)
 
+    def _sanitize_name(self, name):
+        """No commas and equals allowed in tracestate."""
+        return name.replace(",", "").replace("=", "")
+
 
 if TYPE_CHECKING:
 

tests/integrations/pymongo/test_pymongo.py

@@ -1,3 +1,5 @@
+import re
+
 from sentry_sdk import capture_message, start_transaction
 from sentry_sdk.consts import SPANDATA
 from sentry_sdk.integrations.pymongo import PyMongoIntegration, _strip_pii
@@ -49,7 +51,7 @@ def test_transactions(sentry_init, capture_events, mongo_server, with_pii):
     (event,) = events
     (find, insert_success, insert_fail) = event["spans"]
 
-    common_tags = {
+    common_data = {
         "db.name": "test_db",
         "db.system": "mongodb",
         "net.peer.name": mongo_server.host,
@@ -60,31 +62,24 @@ def test_transactions(sentry_init, capture_events, mongo_server, with_pii):
         assert span["data"][SPANDATA.DB_NAME] == "test_db"
         assert span["data"][SPANDATA.SERVER_ADDRESS] == "localhost"
         assert span["data"][SPANDATA.SERVER_PORT] == mongo_server.port
-        for field, value in common_tags.items():
-            assert span["tags"][field] == value
+        for field, value in common_data.items():
             assert span["data"][field] == value
 
     assert find["op"] == "db"
     assert insert_success["op"] == "db"
     assert insert_fail["op"] == "db"
 
     assert find["data"]["db.operation"] == "find"
-    assert find["tags"]["db.operation"] == "find"
     assert insert_success["data"]["db.operation"] == "insert"
-    assert insert_success["tags"]["db.operation"] == "insert"
     assert insert_fail["data"]["db.operation"] == "insert"
-    assert insert_fail["tags"]["db.operation"] == "insert"
 
     assert find["description"].startswith('{"find')
-    assert insert_success["description"].startswith('{"insert')
-    assert insert_fail["description"].startswith('{"insert')
+    assert re.match("^{['\"]insert.*", insert_success["description"])
+    assert re.match("^{['\"]insert.*", insert_fail["description"])
 
     assert find["data"][SPANDATA.DB_MONGODB_COLLECTION] == "test_collection"
-    assert find["tags"][SPANDATA.DB_MONGODB_COLLECTION] == "test_collection"
     assert insert_success["data"][SPANDATA.DB_MONGODB_COLLECTION] == "test_collection"
-    assert insert_success["tags"][SPANDATA.DB_MONGODB_COLLECTION] == "test_collection"
     assert insert_fail["data"][SPANDATA.DB_MONGODB_COLLECTION] == "erroneous"
-    assert insert_fail["tags"][SPANDATA.DB_MONGODB_COLLECTION] == "erroneous"
     if with_pii:
         assert "1" in find["description"]
         assert "2" in insert_success["description"]
@@ -99,16 +94,22 @@ def test_transactions(sentry_init, capture_events, mongo_server, with_pii):
             and "4" not in insert_fail["description"]
         )
 
-    assert find["tags"]["status"] == "ok"
-    assert insert_success["tags"]["status"] == "ok"
-    assert insert_fail["tags"]["status"] == "internal_error"
-
 
-@pytest.mark.parametrize("with_pii", [False, True])
-def test_breadcrumbs(sentry_init, capture_events, mongo_server, with_pii):
+@pytest.mark.parametrize(
+    "with_pii,traces_sample_rate",
+    [
+        [False, 0.0],
+        [False, 1.0],
+        [True, 0.0],
+        [True, 1.0],
+    ],
+)
+def test_breadcrumbs(
+    sentry_init, capture_events, mongo_server, with_pii, traces_sample_rate
+):
     sentry_init(
         integrations=[PyMongoIntegration()],
-        traces_sample_rate=1.0,
+        traces_sample_rate=traces_sample_rate,
         send_default_pii=with_pii,
     )
     events = capture_events()
@@ -120,7 +121,11 @@ def test_breadcrumbs(sentry_init, capture_events, mongo_server, with_pii):
     )  # force query execution
     capture_message("hi")
 
-    (event,) = events
+    if traces_sample_rate:
+        event = events[1]
+    else:
+        event = events[0]
+
     (crumb,) = event["breadcrumbs"]["values"]
 
     assert crumb["category"] == "query"