PYTHON-5947 Add OpenTelemetry Operation and Transaction Span Support - #2964
Draft
blink1073 wants to merge 53 commits into
Draft
PYTHON-5947 Add OpenTelemetry Operation and Transaction Span Support#2964blink1073 wants to merge 53 commits into
blink1073 wants to merge 53 commits into
Conversation
test_sensitive_command_produces_no_span previously asserted no span at all was produced for a sensitive command, but narrowing it to only check the (correctly suppressed) command span silently dropped that coverage. Add a companion assertion documenting that the wrapping operation span is not sensitivity-gated (start_operation_span has no such check, unlike start_command_span) and still exposes the bare command name, so the gap is tracked instead of silently uncovered.
Add integration tests for the STARTING/COMMITTED_EMPTY early-return in commit_transaction, the STARTING early-return in abort_transaction, and a retried commit, per code review: none of the prior tests exercised a transaction ended without ever sending a server command.
…e unified test format runner
Task 9's fix to bson/json_util._truncate_documents (removing a truthy
check that silently dropped falsy-but-valid field values like 0, False,
"", {}, []) was previously only covered incidentally via the vendored
OTel spec tests. Add a direct unit test so a future edit to this shared
production helper (also used by pymongo/logger.py for structured
command logging) can't silently reintroduce the bug.
…rom final review Final whole-branch review found that most _retry_internal call sites pass an _Op enum member (a str-mixin enum) as the operation name; Python 3.11 changed str-mixin Enum formatting so this corrupted span names/db.operation.name on every Python version from 3.11 through 3.14 (worked by accident on 3.10). Also fixes: Database.command() now produces a "runCommand" operation span per the OTel spec's driver-operation-name rule instead of leaking the underlying (possibly sensitive, e.g. saslStart) command name; the operation span's namespace/summary backfill now runs before the sensitive-command suppression check so it still gets its required attributes; and operation spans now carry exception.type/message/stacktrace attributes like command spans already did. Also merges two adjacent versionchanged:: 4.18 docstring blocks for the tracing option into one.
…span fixes Adds regression coverage for the final-review fixes: an operation-name normalization test using an _Op enum member (meaningful on every Python version, not just 3.11+, since 3.10's accidental correctness is what hid the bug), a runCommand operation-span-naming test (unit + live), and an operation-span exception-attribute assertion. Updates test_otel.py assertions that assumed the old (pre-fix) db.operation.name values, and updates or removes the now-resolved TODO(PYTHON-5947) comments. Adds one-line comments distinguishing the two hardcoded unified-format test skips (a removed API vs. a genuine fixture/driver mismatch) in unified_format.py.
test/open_telemetry/transaction/*.json and test_otel.py's
@require_transactions tests both require a replicaset/sharded topology, but
the OTel variant only ran against a standalone server, so transaction spans
never actually ran in CI. Adds a second task selector
(".test-non-standard .replica_set-noauth-ssl"), mirroring the existing
pattern of pairing a standard/standalone selector with a replica-set one
(e.g. create_pyopenssl_variants()). Regenerated via the generate-config
pre-commit hook rather than hand-editing the generated YAML.
docs/superpowers/plans/2026-07-27-python-5947-otel-operation-transaction-spans.md is an internal planning artifact from the implementation process, not project documentation (which lives under doc/, singular). The task list it describes is now complete.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
trace.use_span defaults record_exception/set_status_on_exception to True, so an exception propagating out of a `with use_operation_span():` block was auto-recorded there and then again by the caller's own end_operation_span_failure, producing two identical exception events on the finished span. Disable both, matching how the attached-mode path already avoids this via cm.__exit__(None, None, None). Also tighten start_operation_span's dbname/collection checks to `is not None` instead of truthiness.
…namespace AsyncDatabase.__getattr__ synthesizes a collection for any unknown attribute name, so the getattr(target, "database", None) probe used to distinguish an AsyncCollection target from an AsyncDatabase target returned a phantom collection for database/cluster-level change streams instead of None. This misclassified AsyncDatabaseChangeStream and AsyncClusterChangeStream targets as collections, putting the wrong values into db.namespace/db.collection.name. Switch to isinstance checks with a deferred (call-time) import to avoid the module-scope circular import between change_stream.py and collection.py/database.py.
Document that background monitoring spans no longer inherit a stale parent from client startup, split a run-on sentence into two, and use code spans for the concrete attribute names, per review feedback.
The otel test suite's InMemorySpanExporter/SimpleSpanProcessor for each tracing-enabled test class is registered on one shared, process-wide TracerProvider (trace.set_tracer_provider only takes effect once per process) and is never removed. A monitor that is still finishing an in-flight, cancelled heartbeat after its client's close() has already returned can therefore emit a stray "hello" command span that lands in whichever other test's span-capture window happens to be open at that moment, since: - The sync driver's Monitor.join() wraps two synchronous join() calls in an unawaited asyncio.gather(...), a leftover of generating the sync file from the async source, so it never actually blocks. - Neither close() path guarantees every monitor executor has fully exited by the time it returns. Wait for a tracing-enabled test client's monitor executors directly (bypassing Monitor.join()) as part of its cleanup, instead of a plain close(), and shut down each test class's own span exporter in tearDownClass so it stops accumulating spans from unrelated clients for the rest of the run.
The previous fix only joined monitors for clients created with an
explicit tracing={"enabled": True} kwarg, missing clients that become
tracing-enabled purely via the OTEL_PYTHON_INSTRUMENTATION_MONGODB_ENABLED
env var (e.g. TestOTelSpans's own env-var prose tests) -- exactly the
client shape identified as the root cause. There's no cheap, reliable
way to tell from outside a client whether it ended up tracing-enabled,
so route every managed client through the same close-and-join cleanup
unconditionally rather than trying to widen the gate. Confirmed via
timing (test_client.py + test_cursor.py, ~5 runs each before/after) that
this doesn't measurably slow the broader suite.
Routing every managed test client through the join-based cleanup
measurably slowed monitoring-heavy test files (test_streaming_protocol.py,
test_pooling.py, test_async_loop_unblocked.py) where clients' monitors
are still legitimately mid-flight at cleanup time, so the bounded
join(5) calls actually wait, sometimes close to the full timeout,
repeatedly. Only clients that can actually emit otel spans need this;
the rest can keep the plain client.close() cleanup.
Add _tracing_enabled_for_cleanup(), evaluated at client-creation time
(when the cleanup callable is chosen), reusing pymongo._otel's own
_OTEL_ENABLED_ENV/_env_truthy so the test harness can't disagree with
the driver about what "enabled" means. This covers both ways a client
ends up tracing-enabled -- the tracing={"enabled": True} kwarg, and the
OTEL_PYTHON_INSTRUMENTATION_MONGODB_ENABLED env var patched around
client creation in test_otel.py's env-var prose tests -- which was the
gap in the previous, kwarg-only gate.
… transaction's span with_transaction()'s finally block unconditionally nulled session._transaction.span, so calling it while a transaction started via the direct API was already active on the same session (an illegal but previously silent combination) would tear down the unrelated, still-active transaction's span: it was never ended or exported, and every later operation on that transaction became a spurious trace root. Only clear the span this call actually owns, and defer creating with_transaction()'s own shared span until it knows it will actually start one, so the illegal-combination path no longer even creates a spurious extra span.
…truction, not GC AsyncCommandCursor.__init__ marks a cursor _killed=True when its first batch already exhausts it, without calling close() -- no getMore is ever sent, so the usual span-ending path (_refresh -> _die_lock, or close()'s "if self._id == 0" check) never runs. The operation span for a single-batch aggregate/listIndexes/listCollections/listDatabases/etc. call was therefore only ended by an explicit close() or by __del__, inflating its duration to whenever GC happens to run and, if the cursor is retained, never ending or exporting it at all -- inconsistent with a multi-batch cursor of the same kind, which always ends promptly at exhaustion. Centralize span attachment in _AgnosticCursorBase._attach_operation_telemetry, which ends the span immediately when the cursor is already killed at construction, and route all 7 construction sites (aggregate, listIndexes, listSearchIndexes, listCollections, cursorCommand, listDatabases, and their sync mirrors) through it instead of poking _operation_telemetry directly.
…anup, consistency) - mongo_client.py _retry_internal: replace the bare assert guarding reuse_current_span/operation_telemetry mutual exclusion with a real raise ValueError, so it still holds under python -O. - change_stream.py: document why _run_aggregation_cmd deliberately omits operation_telemetry (a tailing change stream's operation span would never end), so the gap isn't mistaken for an oversight later. - client_bulk.py execute(): replace the hand-built span name/db.namespace/ db.operation.summary poking for the unacknowledged bulk-write path with _OperationTelemetry(..., dbname="admin"), which now produces the same attributes through the normal path. - _otel.py start_operation_span: use the same collection truthiness convention (if collection:) as _build_query_summary, instead of is not None, so an (unreachable today) empty-string collection can't set db.collection.name to empty string while being omitted from the summary.
These are process artifacts from the implementation session, not project documentation (which lives under doc/, singular). The task list they describe is complete.
blink1073
commented
Jul 29, 2026
| if hasattr(obj, "items"): | ||
| truncated: Any = {} | ||
| for k, v in obj.items(): | ||
| truncated_v, remaining = _truncate_documents(v, remaining) |
Member
Author
There was a problem hiding this comment.
These changes were needed for the new distinct.json, which has a falsey value for the field. These fields should not be dropped.
Both had grown by accretion across two rounds of work, reading as a base description followed by a list of later additions. Describe the feature as it ships instead.
…try args PYTHON-5963's test monkeypatches _CommandTelemetry.__init__ with a fixed positional signature. PYTHON-5945 added tracing_options and speculative_hello to that signature on this branch, so merging main in broke the test: git saw no conflict because the two changes touch different files. Forward the extra arguments instead of enumerating them.
db.namespace/db.collection.name are only "Required if available" per the OTel spec, and no vendored fixture covers the failure-before-any-command case they existed for, so remove the ~25-call-site eager threading of dbname/collection through _retry_internal/_retryable_read/_retryable_write/ _retry_with_session (and delete AsyncChangeStream._target_namespace, which only fed it). Instead, guarantee the always-Required db.operation.summary by falling back to the bare operation name in start_operation_span when no dbname is given. The lazy backfill in start_command_span still covers every success path, and direct _OperationTelemetry construction (cursors, killCursors, endSessions, unacknowledged bulk writes) is untouched.
Replace the hand-written "dbname.collname".split(".", 1) parsing
duplicated in command_cursor.py, cursor.py, mongo_client.py, and
aggregation.py with a single shared _split_namespace helper in
helpers_shared.py, keeping both the async and sync trees import-cycle
free without needing synchro generation for the helper itself.
Split with_transaction() into a thin wrapper that owns the shared "transaction" span plus a new _with_transaction_retry_loop() helper holding the retry loop at its original indentation, removing the ~51 lines of pure re-indentation diff noise the try/finally wrapping had introduced. Also trims the span bookkeeping comments down to the essential facts and drops the "Important #1" review-finding reference. No behavior change: the retry loop's statements, control flow, and this file's mirrored pymongo/synchronous/client_session.py (via `just synchro`) are unchanged apart from the de-indent.
…ites Adding dbname/collection arguments pushed several _retryable_read/write calls over the line limit, so ruff exploded them one-per-line with a magic trailing comma. Removing those arguments again left the trailing comma behind, which keeps ruff from collapsing the calls back. Drop it so these sites match their original single-line form.
Same magic-trailing-comma residue as the previous commit, at call sites whose base form spread arguments across one continuation line rather than fitting entirely on one line.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PYTHON-5947
Changes in this PR
Extends the OpenTelemetry support added in PYTHON-5945 with:
find_one,insert_one,bulk_write), spanning all retry attempts, with each attempt's command span nested underneath. A cursor's whole lifetime is a single operation span, sogetMorecommands nest under the originatingfind/aggregaterather than producing spans of their own."transaction"span wrappingstart_transaction()throughcommit_transaction()/abort_transaction(), with operations inside the transaction nested under it. A retriedwith_transaction()produces one span covering all of its attempts.observeTracingMessages/expectTracingMessages, plus the vendored spec test suite frommongodb/specifications.Spans stay conformant on failure paths too: an operation that fails before any command reaches the wire still carries
db.operation.nameanddb.operation.summary, andkillCursors/endSessionsnow get operation spans rather than leaving their command spans unparented.A few small pre-existing bugs surfaced by the new spec test coverage were fixed along the way (see commit history for details).
This is opt-in: no behavior changes for users who don't enable the
tracingclient option orOTEL_PYTHON_INSTRUMENTATION_MONGODB_ENABLEDenv var.Reviewer's guide
The diff is large but most of it isn't hand-written. Of ~9,800 added lines:
test/open_telemetry/*.json)mongodb/specificationsviaresync-specs.shpymongo/synchronous/*, mirroredtest/*)just synchrooutputtest_otel.pySuggested reading order. The commits are already in dependency order, so
git log -pworks, but if you'd rather read by file:pymongo/_otel.py(~500 lines, the core) — span creation/ending primitives. Note the three-way split: command spans, operation spans (with an eager-attribute and a detached mode), and the"transaction"pseudo-span. Theparent_spanparameter and the deliberate choice not to read ambient context for transaction parenting are the two decisions worth scrutinising.pymongo/_telemetry.py—_OperationTelemetry, the lifecycle wrapper. One line here (parent_span = session._transaction.span) is the only coupling between operation and transaction spans.pymongo/asynchronous/mongo_client.py—_retry_internalgained three span modes: owned (creates and ends its own), caller-owned (makes a caller's span current, creates/ends nothing), and passthrough (creates nothing, leaves ambient in place, used only by the client bulk-write results cursor). Also thekillCursors/endSessionswrappers.pymongo/cursor_shared.py+asynchronous/cursor.py+asynchronous/command_cursor.py— cursor-lifetime span ownership._end_operation_telemetryis idempotent and called from the two choke points every close/GC/exception path funnels through; the exactly-once property across those paths is the main thing to check.pymongo/asynchronous/client_session.py— transaction spans, including the shared-span-across-with_transaction-retries mechanism and its reentrancy guard.test/asynchronous/unified_format.py—observeTracingMessages/expectTracingMessageswiring, modelled on the existingexpectLogMessageshandling.Things you might reasonably ask about, already considered:
bson/json_util.pyis touched —_truncate_documentssilently dropped falsy values (0,False,"",{},[]) from truncated output. It's shared withpymongo/logger.py, so this was affecting structured command logging too, not just tracing. Has its own regression test.getMorenesting — a tailing stream's operation span would never end. Comment at the call site records this.test/__init__.pyandtest/asynchronous/__init__.pychanged — tracing-enabled test clients join their monitors on cleanup, so a stray heartbeat span can't land in another test's assertion window. Gated on tracing being enabled, so non-tracing tests keep the original cleanup path.Test Plan
mongodb/specifications.Checklist
Checklist for Author
No follow-up work outstanding — the span-coverage gaps originally deferred from this ticket are all closed here.
Checklist for Reviewer