PYTHON-5947 Add OpenTelemetry operation spans by blink1073 · Pull Request #2991 · mongodb/mongo-python-driver

blink1073 · 2026-08-17T14:44:03Z

First of four PRs splitting #2964. Base is otel; review in order.

	Branch	Contents
1	`PYTHON-5947-otel-1-operations`	operation spans (this PR)
2	`PYTHON-5947-otel-2-transactions`	transaction spans
3	`PYTHON-5947-otel-3-unified`	unified runner and vendored fixtures
4	`PYTHON-5947-otel-4-getmore`	getMore spans
5	`PYTHON-5947-otel-5-error-type`	`error.type` command span attribute

Changes in this PR

Adds one span per public API call, containing one command span per command sent to the server, per the OpenTelemetry driver specification. Without it a retried operation's command spans share no parent, so a reader cannot tell which attempts belong to the same call.

One operation span covers every retry attempt of a call, so retries show up as sibling command spans instead of collapsing into one.
killCursors, endSessions, and unacknowledged client bulk writes get operation spans too.
An operation that fails before anything reaches the wire still produces a conformant span, so a server selection timeout is reported rather than lost.
Cursor-creating calls (find, aggregate, listCollections, listIndexes) are covered, but only for the command that creates the cursor. Caller-driven getMores arrive in PR 4.
Fixed truncated output dropping falsy values (0, False, ""). This affected structured command logging as well as spans.
Background tasks no longer inherit a caller's span.
The otel Evergreen variant now covers all three topologies, held at 22 tasks.

Opt-in: no behavior change unless the tracing client option or OTEL_PYTHON_INSTRUMENTATION_MONGODB_ENABLED is set. With tracing off there is no added allocation per command.

Around 1,000 of the added lines are just synchro output and another ~745 are tests.

Test Plan

49 new tests each side: span attributes and naming, retry and failure paths, sensitive-command redaction, and config handling, plus a regression test that enabling tracing through the environment variable does not trace background heartbeats.
Regression tests for the truncation bug and for collection names containing dots.
Locally against a replica set: 98 otel tests pass, along with the cursor, collection, common, json_util, and operation-id suites. just lint clean.

Checklist

Checklist for Author

Did you update the changelog (if necessary)?
Is there test coverage?
Is any followup work tracked in a JIRA ticket? If so, add link(s).

Checklist for Reviewer

Does the title of the PR reference a JIRA Ticket?
Do you fully understand the implementation? (Would you be comfortable explaining how this code works to someone else?)
Is all relevant documentation (README or docstring) updated?

codecov · 2026-08-17T23:33:41Z

Codecov Report

❌ Patch coverage is 97.04918% with 9 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
pymongo/_otel.py	97.52%	2 Missing and 1 partial ⚠️
pymongo/asynchronous/client_bulk.py	77.77%	1 Missing and 1 partial ⚠️
pymongo/synchronous/client_bulk.py	77.77%	1 Missing and 1 partial ⚠️
pymongo/asynchronous/collection.py	66.66%	1 Missing ⚠️
pymongo/synchronous/collection.py	66.66%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

Give every public API call an operation span containing one command span per command sent to the server, per the OpenTelemetry driver specification. The span covers all retry attempts of one _retry_internal call, so retries appear as sibling command spans rather than being collapsed into one. killCursors and endSessions bypass the retry layer and start their spans at the call site instead, as does an unacknowledged client bulk write, which never reaches the command-span code that would fill in its namespace. Cursor-creating operations (find, aggregate, listCollections, listIndexes) are covered here, but only for the command that creates the cursor. Spans for caller-driven getMores come in a later change, as do transaction spans. _otel.py also takes over the specification's naming and attribute rules, so _telemetry.py deals only with span lifecycles and a specification change need not touch it. Namespace parsing moves to helpers_shared._split_namespace.

Lead with the condition rather than the bare adjective.

The vendored unified spec fixtures it pointed at arrive in a later change, so the note described tests absent from this one.

…d test Evergreen only runs on PRs whose base is a configured branch, so the upper PRs in this stack get no Evergreen coverage, and the otel tests only run in Evergreen's otel variant. Add a temporary GitHub Actions job that runs them on every branch in the stack, against a replica set so the transaction span tests are included. Remove the job before merging. Also drop test_operation_name_normalizes_enum_operation. All 22 vendored fixtures assert db.operation.name with literal values, so the _Op formatting regression it guards fails them loudly, and adding a test here only to delete it once the fixtures land is churn.

semgrep-code-mongodb · 2026-08-18T14:07:02Z

Semgrep found 2 github-actions-mutable-action-tag findings:

.github/workflows/test-python.yml
- L321 - Triage
- L329 - Triage

GitHub Actions step uses a mutable tag or branch reference. Tags and branch names can be silently repointed by the action owner, enabling supply-chain attacks — as seen in the trivy-action and kics-github-action compromises. Pin the reference to a full 40-character commit SHA instead, e.g. uses: actions/checkout@8ade135a41bc03ea155e62e844d188df1ea18608.

🛟 Help? Slack #semgrep-help or go/semgrep-help.

Resolution Options:

Fix the code
Reply /fp $reason (if security gap doesn’t exist)
Reply /ar $reason (if gap is valid but intentional; add mitigations/monitoring)
Reply /other $reason (e.g., test-only)

…the end The mutable-action-tag scanners reported two findings in test_minimum, which this branch does not touch: inserting the otel job mid-file shifted those lines by 31 and the diff-based scan attributed them to this change. Appending the job instead leaves every pre-existing line where it was. Pin the job's own actions to commit hashes so the newly added lines do not trip the same rule. drivers-evergreen-tools has no pinned use elsewhere in this file, so v1.0.1 is spelled out; its commit is the same one master points at, so the topology input this job relies on is unchanged.

NoahStapp

Can you run a micro benchmark measuring performance impact both when OTel is enabled as well as disabled?

NoahStapp · 2026-08-18T19:44:01Z

        :param is_run_command: If this is a runCommand operation, defaults to False.
        :param is_aggregate_write: If this is a aggregate operation with a write, defaults to False.
        :param operation_id: Stable operation id shared across retries, defaults to None
+        :param operation_telemetry: A caller-owned operation span outliving this call,


How can the operation span outlive this call and this method own a fresh span? Not clear what that means.

I clarified that AsyncCursor._refresh is the intended user.

NoahStapp · 2026-08-18T19:45:41Z

+        dbname: str,
+        collection: Optional[str] = None,
+    ) -> _CommandCursor:
+        """Run a command-cursor read within its own operation span.


Suggested change

"""Run a command-cursor read within its own operation span.

"""Run a command cursor read within its own operation span.

No hyphen between command and cursor.

NoahStapp · 2026-08-18T19:48:49Z

+        # One span covering every attempt. A caller needing it to outlive this
+        # object (a cursor) passes its own and keeps ownership.


self._owns_telemetry = True means that we create the span here, otherwise it's passed in because a cursor owns it? Comment doesn't make that clear.

Clarified the comment

NoahStapp · 2026-08-18T19:56:35Z

+    ``query_text_max_length`` is None as validated from user input; the options
+    a client holds have been through :func:`_resolve_tracing_options`, so both
+    fields are resolved.


Does this mean that query_text_max_length is always None? Not sure what this comment means.

None is to allow the env var to override when resolved. I created _UnresolvedTracingOptions to distinguish between the two states.

NoahStapp · 2026-08-18T20:02:10Z

    return f"{command_name} {dbname}"


+# Some `_Op` values are the wire command name ("drop"/"create") rather than the


Why not rename _Op for consistency if it's entirely internal?

Logging wants the wire name, OTel wants the logical name (good times).

Do you know the motivation behind that choice in the spec? Having two of our telemetry APIs record different names for the same operation seems very confusing for both us and users.

Turns out I was confused as well, there are two different fields, db.operation.name and db.command.name. I made the comment more explicit.

NoahStapp · 2026-08-18T21:04:05Z

+# spec's db.operation.name ("dropCollection"/"createCollection"). Translate here
+# instead of renaming `_Op`: `_WRITES_WITH_CLUSTER_TIME` in operations.py matches
+# these exact strings to pick which writes get afterClusterTime.
+_OPERATION_NAME_OVERRIDES = {


I think we need "rename -> renameCollection" here too.

Rename is not one of the supported operations.

Why is that list prefaced with "including but not limited to the following operations"? Does that imply we should support rename if it's easy to do so?

NoahStapp · 2026-08-18T21:06:02Z

+    # built inside it. Before the sensitive-command return below, since the
+    # operation span needs those attributes even when the command gets no span.
+    current_operation = _CURRENT_OPERATION_NAME.get()
+    if current_operation is not None:


Does this backfill run on each retry attempt too? If so, does it need to?

Clarified the comment. Yes, repeating is necessary in case an attempt dies before the command is built, and it is idempotent.

NoahStapp · 2026-08-18T21:09:27Z

            # ___init__ did not run to completion (or at all).
            return

+        self._end_operation_telemetry()


Shouldn't this come after the actual close, so if the close fails this also records the failure?

No, added a comment above this line.

NoahStapp · 2026-08-18T21:10:19Z

+            except BaseException as exc:
+                if operation_telemetry is not None:
+                    operation_telemetry.failed(exc)
+                raise
+            else:
+                if operation_telemetry is not None:
+                    operation_telemetry.succeeded()


Can this not use the with telemetry or contextlib.nullcontext() pattern used elsewhere instead? Same question twice in mongo_client.py too.

Good call, done

The parameter's description packed two mutually exclusive branches into one clause, so it read as though the span both outlived the call and was freshly owned by it. Split the two cases apart and name AsyncCursor as the one caller that passes a span, along with why it keeps ownership. Also drops the claim that a cursor's span is shared by its getMores, which is not true yet: the cursor clears its span once the creating command completes.

blink1073 · 2026-08-19T11:26:45Z

Can you run a micro benchmark measuring performance impact both when OTel is enabled as well as disabled?

I ran a test with find_one() on a spawn host with PyPy 3.9, Python 3.14, and Python 3.14t. When tracing is disabled the the op/s is not measurably affected. Adding tracing costs 32–48% of throughput across all variations of interpreter and sync/async.

The unacknowledged path skips _retryable_write, which is what opens the operation span on the acknowledged path, so open one at the call site. Matches what the client-level bulk write already did, and both now use the telemetry object as a context manager instead of hand-written try/except/else, as does _retryable_read_cursor. Split the tracing option TypedDict in two. As validated from user input, query_text_max_length is None when unset, which is distinct from an explicit 0; once _resolve_tracing_options has folded in the environment variables it is always an int. Giving the two states separate types lets the resolved reader stop re-checking for None. Also record why the operation-name overrides exist: "dropCollection" and "createCollection" are deviations the spec is expected to correct, and "dropSearchIndexes" is only our `_Op` member being plural.

blink1073 force-pushed the PYTHON-5947-otel-1-operations branch 3 times, most recently from 4a8db0c to ba21bb8 Compare August 17, 2026 23:09

blink1073 force-pushed the PYTHON-5947-otel-1-operations branch 11 times, most recently from c841c9f to 4e0c468 Compare August 18, 2026 12:02

blink1073 force-pushed the PYTHON-5947-otel-1-operations branch from 4e0c468 to 6638ae1 Compare August 18, 2026 12:55

blink1073 added 3 commits August 18, 2026 08:33

PYTHON-5947 Reword the tracing-disabled test comment

35c3c5e

Lead with the condition rather than the bare adjective.

PYTHON-5947 Drop a forward reference from the validator test comment

9b44925

The vendored unified spec fixtures it pointed at arrive in a later change, so the note described tests absent from this one.

github-advanced-security Bot found potential problems Aug 18, 2026

View reviewed changes

Comment thread .github/workflows/test-python.yml Fixed

blink1073 requested a review from NoahStapp August 18, 2026 17:24

blink1073 marked this pull request as ready for review August 18, 2026 17:24

blink1073 requested a review from a team as a code owner August 18, 2026 17:24

NoahStapp requested changes Aug 18, 2026

View reviewed changes

	"""Run a command-cursor read within its own operation span.
	"""Run a command cursor read within its own operation span.

		# One span covering every attempt. A caller needing it to outlive this
		# object (a cursor) passes its own and keeps ownership.

		return f"{command_name} {dbname}"


		# Some `_Op` values are the wire command name ("drop"/"create") rather than the

Uh oh!

Conversation

blink1073 commented Aug 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes in this PR

Test Plan

Checklist

Checklist for Author

Checklist for Reviewer

Uh oh!

codecov Bot commented Aug 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

semgrep-code-mongodb Bot commented Aug 18, 2026

Uh oh!

NoahStapp left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blink1073 commented Aug 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

blink1073 commented Aug 17, 2026 •

edited

Loading

codecov Bot commented Aug 17, 2026 •

edited

Loading