feat(pymongo): Add span streaming support#6253
Conversation
Add span streaming support to the PyMongo integration, mirroring the pattern used in the asyncpg integration. When trace_lifecycle=stream is enabled, spans are emitted using the new StreamedSpan API with OTel-style attributes. The legacy code path is preserved for non-streaming clients. Fixes PY-2349 Fixes #6047
|
bugbot run |
|
@sentry review |
Codecov Results 📊✅ 146 passed | Total: 146 | Pass Rate: 100% | Execution Time: 22.83s 📊 Comparison with Base Branch
✨ No test changes detected All tests are passing successfully. ❌ Patch coverage is 5.36%. Project has 14368 uncovered lines. Files with missing lines (1)
Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
+ Coverage 30.12% 34.92% +4.8%
==========================================
Files 190 190 —
Lines 21907 22079 +172
Branches 7818 7424 -394
==========================================
+ Hits 6598 7711 +1113
- Misses 15309 14368 -941
- Partials 695 807 +112Generated by Codecov Action |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
|
||
| data: "Dict[str, Any]" = {"operation_ids": {}} | ||
| data["operation_ids"]["operation"] = event.operation_id | ||
| data["operation_ids"]["request"] = event.request_id |
There was a problem hiding this comment.
These are also kept for legacy reasons, are being removed for streamed spans.
sentrivana
left a comment
sentrivana
left a comment
There was a problem hiding this comment.
Looking good, but we're using some deprecated attributes here, I've highlighted the ones I noticed but please have another look at the rest we're setting on the span first path as well 🙏🏻 Approving to not block.
| if type(span) is StreamedSpan: | ||
| span.status = SpanStatus.OK |
There was a problem hiding this comment.
OK is the default value for StreamedSpans, so technically we don't need to set it here
There was a problem hiding this comment.
IIRC this is deprecated and superseded by SPANDATA.DB_NAMESPACE, so we should use that in span first
| "db.name": event.database_name, | ||
| SPANDATA.DB_SYSTEM: "mongodb", | ||
| SPANDATA.DB_DRIVER_NAME: "pymongo", | ||
| SPANDATA.DB_OPERATION: event.command_name, |
There was a problem hiding this comment.
| SPANDATA.DB_OPERATION: event.command_name, | |
| SPANDATA.DB_OPERATION_NAME: event.command_name, |
https://getsentry.github.io/sentry-conventions/attributes/db/#db-operation
| if has_span_streaming_enabled(client.options): | ||
| span_first_data = { | ||
| "db.name": event.database_name, | ||
| SPANDATA.DB_SYSTEM: "mongodb", |
There was a problem hiding this comment.
| SPANDATA.DB_SYSTEM: "mongodb", | |
| SPANDATA.DB_SYSTEM_NAME: "mongodb", |
|
|
||
|
|
||
| @pytest.mark.parametrize("with_pii", [False, True]) | ||
| def test_transactions_span_streaming( |
There was a problem hiding this comment.
Would be good to not use the term transaction in the span streaming tests as there are no transactions anymore. Maybe just span or segment if we want to emphasize that a span is root-level.
Looks like we're mentioning transactions in this test name and then also when we're creating spans by hand in this test and others we're calling them test_transaction
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| if db_name is not None: | ||
| data[SPANDATA.DB_NAME] = db_name | ||
|
|
||
| server_address = event.connection_id[0] |
There was a problem hiding this comment.
Bug: Accessing a non-indexable event.connection_id in _get_db_data raises a TypeError that is silently caught, causing the entire pymongo span to be dropped.
Severity: MEDIUM
Suggested Fix
Wrap the access to event.connection_id within a try...except TypeError block. This will prevent the unhandled exception from halting execution, allowing the span to be created even if peer address data cannot be extracted.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: sentry_sdk/integrations/pymongo.py#L98
Potential issue: The function `_get_db_data` directly accesses `event.connection_id` to
retrieve database connection details. If `event.connection_id` is not an indexable type
(e.g., an integer), a `TypeError` is raised. This exception is caught and suppressed by
the `capture_internal_exceptions` context manager, which then prematurely exits the
`with` block. As a consequence, the associated Sentry span is never created, and the
entire pymongo event is silently dropped. This is a regression from previous behavior
where this error was handled, and a span was still created.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 30ba6da. Configure here.
2 participants
Add span streaming support to the PyMongo integration. The legacy code path is preserved unchanged for non-streaming clients.
Fixes PY-2349
Fixes #6047