PYTHON-5947 Trim the operation telemetry comments and docstrings

Several of these grew while answering review questions and ended up explaining
spec history or restating the code. Cut each to the invariant a reader needs.

Author: blink1073

pymongo/_otel.py

@@ -75,10 +75,9 @@
 class _UnresolvedTracingOptions(TypedDict):
     """The ``MongoClient`` ``tracing`` option as validated from user input.
 
-    ``query_text_max_length`` is None when the user set no value, which is
-    distinct from an explicit 0: None leaves the environment variable free to
-    supply a length, 0 turns ``db.query.text`` off outright. Short-lived, since
-    :func:`_resolve_tracing_options` runs while the client is being built.
+    ``query_text_max_length`` is None when unset, which is distinct from an
+    explicit 0: None lets the environment variable supply a length, 0 turns
+    ``db.query.text`` off.
     """
 
     enabled: bool
@@ -88,9 +87,8 @@ class _UnresolvedTracingOptions(TypedDict):
 class TracingOptions(TypedDict):
     """The ``MongoClient`` ``tracing`` option as a client holds it.
 
-    Both fields have been through :func:`_resolve_tracing_options`, so they
-    account for the two environment variables and need no further
-    interpretation: ``query_text_max_length`` is an int of 0 or more.
+    :func:`_resolve_tracing_options` has folded in the environment variables,
+    so ``query_text_max_length`` is an int of 0 or more.
     """
 
     enabled: bool
@@ -226,22 +224,8 @@ def _build_query_summary(command_name: str, dbname: str, collection: Optional[st
     return f"{command_name} {dbname}"
 
 
-# db.operation.name is the command name for nearly every operation, so `_Op`
-# values (which are the wire names) carry straight through. These are the
-# exceptions the spec's covered operations table names differently.
-#
-# "dropCollection" and "createCollection" are known deviations: their commands
-# are "drop" and "create", and the table departs from its own command-name rule
-# for no stated reason. The spec is expected to correct them, at which point
-# both entries here go away, but the change alters an exported attribute value
-# and so has to land on a major version boundary, in step with the other
-# drivers and the spec's own fixtures.
-#
-# "dropSearchIndexes" is ours, not the spec's: the command really is the
-# singular "dropSearchIndex" and only our `_Op` member is plural.
-#
-# Operations absent from the table (rename, whose command is "renameCollection")
-# have no agreed cross-driver name to match and keep their `_Op` value.
+# Spec operation names that differ from our `_Op` values. "dropCollection" and
+# "createCollection" contradict the spec's own command-name rule, unresolved.
 _OPERATION_NAME_OVERRIDES = {
     "drop": "dropCollection",
     "create": "createCollection",
@@ -302,29 +286,18 @@ def start_command_span(
 ) -> Optional[Span]:
     """Start and return a CLIENT-kind span for a server command, or None.
 
-    Returns None in two cases. With tracing off the call is a no-op. With a
-    sensitive command it is not: the span is suppressed, mirroring the
-    redaction applied to logs, but the current operation span is still
-    backfilled from it, since that span needs the namespace and summary even
-    when the command itself gets no span.
-
-    One span per wire-protocol message, so a retried operation produces one per
-    attempt. The span takes whichever operation span is current as its parent,
-    but is returned rather than made current itself, so nothing else nests
-    inside it. The caller holds it for the command's duration.
+    Returns None when tracing is off, and for a sensitive command, which still
+    backfills the current operation span before being suppressed. One span per
+    wire-protocol message, returned rather than made current, so nothing nests
+    inside it.
     """
     if not _is_tracing_enabled(tracing_options):
         return None
 
     collection = _extract_collection_name(command_name, dbname, cmd)
-    # Backfill the operation span's name/namespace/summary from the command built
-    # inside it. Before the sensitive-command return below, since the operation
-    # span needs those attributes even when the command gets no span. This runs
-    # once per attempt rather than once per operation: an attempt that fails
-    # before building a command never reaches here, so a retry can be where the
-    # operation span first learns its namespace. Repeating it costs a few
-    # attribute writes and is otherwise a no-op, since every attempt of one
-    # operation carries the same namespace.
+    # Runs per attempt, not per operation: an attempt that fails before building
+    # a command never reaches here, so a retry may be where the span learns its
+    # namespace.
     current_operation = _CURRENT_OPERATION_NAME.get()
     if current_operation is not None:
         current_span = trace.get_current_span()

pymongo/asynchronous/cursor_base.py

@@ -175,11 +175,8 @@ async def _die_lock(self) -> None:
             # ___init__ did not run to completion (or at all).
             return
 
-        # Before the cleanup below rather than after: this ends the span of the
-        # operation that created the cursor, which has already completed, while
-        # the cleanup sends killCursors under a separate operation span of its
-        # own. A failure cleaning up belongs to that span, not to a find that
-        # succeeded.
+        # Before the cleanup, which sends killCursors under its own operation
+        # span: a failure there belongs to that span, not to a find that succeeded.
         self._end_operation_telemetry()
         cursor_id, address = self._prepare_to_die(already_killed)
         await self._collection.database.client._cleanup_cursor_lock(

pymongo/asynchronous/mongo_client.py

@@ -1909,9 +1909,8 @@ async def _run_operation(
             that executes the operation on a given connection.
         :param address: Optional address when sending a message
             to a specific server, used for getMore.
-        :param operation_telemetry: The operation span created by the calling
-            cursor (see ``AsyncCursor._refresh``), or None. Passed down so the
-            command spans of this send nest under it.
+        :param operation_telemetry: The calling cursor's operation span, or None,
+            so this send's command spans nest under it.
         """
         if operation.conn_mgr:
             server = await self._select_server(
@@ -2018,13 +2017,8 @@ async def _retry_internal(
         :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: An operation span the caller created and will
-            end itself, defaults to None. Only ``AsyncCursor`` passes one (see
-            ``AsyncCursor._refresh``): its span has to survive send paths that
-            bypass this method, so the cursor ends it rather than this call.
-            Given a span, this method neither creates nor ends one and only
-            makes the caller's current for the duration of the call; given
-            None, it creates a span and ends it before returning.
+        :param operation_telemetry: A cursor's operation span, which this call
+            makes current but neither creates nor ends, defaults to None.
 
         :return: Output of the calling func()
         """
@@ -2074,9 +2068,7 @@ async def _retryable_read(
         :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: An operation span the caller created and will
-            end itself, defaults to None. See ``_retry_internal``, which this
-            forwards to.
+        :param operation_telemetry: As for ``_retry_internal``, defaults to None.
         """
 
         # Ensure that the client supports retrying on reads and there is no session in
@@ -3008,10 +3000,8 @@ def __init__(
         if operation_id is None:
             operation_id = _generate_op_id_or_none(self._client._event_listeners)
         self._operation_id = operation_id
-        # One span covers every attempt. With nothing passed in, create the
-        # span here and end it in run(). With a span passed in (a cursor's,
-        # which has to outlive this object), the caller keeps ownership and
-        # run() only makes it current.
+        # With nothing passed in, create the span here and end it in run(); with
+        # a span passed in, the caller owns it and run() only makes it current.
         self._owns_telemetry = operation_telemetry is None
         if self._owns_telemetry:
             operation_telemetry = _operation_telemetry_or_none(

pymongo/synchronous/cursor_base.py

@@ -175,11 +175,8 @@ def _die_lock(self) -> None:
             # ___init__ did not run to completion (or at all).
             return
 
-        # Before the cleanup below rather than after: this ends the span of the
-        # operation that created the cursor, which has already completed, while
-        # the cleanup sends killCursors under a separate operation span of its
-        # own. A failure cleaning up belongs to that span, not to a find that
-        # succeeded.
+        # Before the cleanup, which sends killCursors under its own operation
+        # span: a failure there belongs to that span, not to a find that succeeded.
         self._end_operation_telemetry()
         cursor_id, address = self._prepare_to_die(already_killed)
         self._collection.database.client._cleanup_cursor_lock(

pymongo/synchronous/mongo_client.py

@@ -1904,9 +1904,8 @@ def _run_operation(
             that executes the operation on a given connection.
         :param address: Optional address when sending a message
             to a specific server, used for getMore.
-        :param operation_telemetry: The operation span created by the calling
-            cursor (see ``Cursor._refresh``), or None. Passed down so the
-            command spans of this send nest under it.
+        :param operation_telemetry: The calling cursor's operation span, or None,
+            so this send's command spans nest under it.
         """
         if operation.conn_mgr:
             server = self._select_server(
@@ -2013,13 +2012,8 @@ def _retry_internal(
         :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: An operation span the caller created and will
-            end itself, defaults to None. Only ``Cursor`` passes one (see
-            ``Cursor._refresh``): its span has to survive send paths that
-            bypass this method, so the cursor ends it rather than this call.
-            Given a span, this method neither creates nor ends one and only
-            makes the caller's current for the duration of the call; given
-            None, it creates a span and ends it before returning.
+        :param operation_telemetry: A cursor's operation span, which this call
+            makes current but neither creates nor ends, defaults to None.
 
         :return: Output of the calling func()
         """
@@ -2069,9 +2063,7 @@ def _retryable_read(
         :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: An operation span the caller created and will
-            end itself, defaults to None. See ``_retry_internal``, which this
-            forwards to.
+        :param operation_telemetry: As for ``_retry_internal``, defaults to None.
         """
 
         # Ensure that the client supports retrying on reads and there is no session in
@@ -2997,10 +2989,8 @@ def __init__(
         if operation_id is None:
             operation_id = _generate_op_id_or_none(self._client._event_listeners)
         self._operation_id = operation_id
-        # One span covers every attempt. With nothing passed in, create the
-        # span here and end it in run(). With a span passed in (a cursor's,
-        # which has to outlive this object), the caller keeps ownership and
-        # run() only makes it current.
+        # With nothing passed in, create the span here and end it in run(); with
+        # a span passed in, the caller owns it and run() only makes it current.
         self._owns_telemetry = operation_telemetry is None
         if self._owns_telemetry:
             operation_telemetry = _operation_telemetry_or_none(