Skip to content

Commit fe696df

Browse files
committed
PYTHON-5993 Trim the getMore span comments and docstrings
Cut each to the invariant a reader needs, dropping spec references and restatements of the code.
1 parent a016200 commit fe696df

14 files changed

Lines changed: 128 additions & 268 deletions

pymongo/_otel.py

Lines changed: 12 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -65,11 +65,8 @@
6565
"_CURRENT_OPERATION_NAME", default=None
6666
)
6767

68-
# True while the driver is iterating a cursor of its own to build the return
69-
# value of one public API call (list_collection_names, index_information, ...).
70-
# Such a call gets a single operation span covering every getMore it sends,
71-
# whereas a cursor handed back to the caller gets a fresh operation span per
72-
# caller-driven getMore. See internal_cursor_iteration.
68+
# True while the driver is draining a cursor of its own to build one public API
69+
# call's return value. See internal_cursor_iteration.
7370
_INTERNAL_CURSOR_ITERATION: ContextVar[bool] = ContextVar(
7471
"_INTERNAL_CURSOR_ITERATION", default=False
7572
)
@@ -140,11 +137,9 @@ def _env_truthy(name: str) -> bool:
140137
def internal_cursor_iteration() -> Iterator[None]:
141138
"""Mark the enclosing block as driver-internal cursor iteration.
142139
143-
Wrap the block in which a public API method creates a cursor and drains it
144-
itself to build its return value. Everything the block sends, including
145-
every getMore, then belongs to that method's one operation span, as the
146-
OTel spec requires. Outside such a block the cursor is assumed to reach the
147-
caller, whose iteration is a separate operation per getMore.
140+
Everything the block sends, every getMore included, belongs to the enclosing
141+
method's one operation span. Outside such a block a getMore is assumed to be
142+
caller-driven and gets an operation span of its own.
148143
"""
149144
token = _INTERNAL_CURSOR_ITERATION.set(True)
150145
try:
@@ -326,11 +321,8 @@ def start_command_span(
326321
return None
327322

328323
collection = _extract_collection_name(command_name, dbname, cmd)
329-
# A getMore's own command value is the id of the cursor being read, which is
330-
# the value db.mongodb.cursor_id takes for a command operating on an
331-
# existing cursor: the id sent, not whatever the reply comes back with. It
332-
# has to be read here rather than from the reply because the reply is 0 once
333-
# the cursor is exhausted, and the attribute is required even then.
324+
# The id sent, not the reply's, which is 0 once the cursor is exhausted
325+
# while the attribute is still required.
334326
sent_cursor_id = cmd.get(_GET_MORE) if command_name == _GET_MORE else None
335327
if not isinstance(sent_cursor_id, int):
336328
sent_cursor_id = None
@@ -391,9 +383,8 @@ def start_command_span(
391383
def _set_operation_cursor_id(cursor_id: int) -> None:
392384
"""Set db.mongodb.cursor_id on the ambient operation span, if there is one.
393385
394-
Guarded on the operation-name contextvar for the same reason
395-
``start_command_span``'s backfill is: without it the "current span" could be
396-
an unrelated span belonging to the host application.
386+
Guarded on the operation-name contextvar, since the current span could
387+
otherwise be an unrelated one belonging to the host application.
397388
"""
398389
if _CURRENT_OPERATION_NAME.get() is None:
399390
return
@@ -408,13 +399,9 @@ def end_command_span_success(span: Optional[Span], reply: _DocumentOut) -> None:
408399
return
409400
cursor = reply.get("cursor")
410401
if isinstance(cursor, Mapping) and cursor.get("id"):
411-
# Per the spec the attribute is omitted rather than set to 0, so a
412-
# cursor-creating command that leaves no cursor open reports nothing. A
413-
# getMore keeps the id it sent, which this does not overwrite with a 0.
402+
# Omitted rather than set to 0, so a getMore keeps the id it sent.
414403
cursor_id = cursor["id"]
415404
span.set_attribute("db.mongodb.cursor_id", cursor_id)
416-
# The operation span carries the same attribute: this reply's id for a
417-
# cursor-creating command, or the already-set sent id for a getMore.
418405
_set_operation_cursor_id(cursor_id)
419406
span.end()
420407

@@ -496,9 +483,8 @@ def start_operation_span(
496483
``parent_span`` becomes an *explicit* parent rather than being read from
497484
ambient context, so a concurrent unrelated session cannot be captured.
498485
499-
``cursor_id`` sets ``db.mongodb.cursor_id`` up front, for an operation
500-
reading an existing cursor: the id is known before the command is built and
501-
is needed even if the operation fails before any command span exists.
486+
``cursor_id`` sets ``db.mongodb.cursor_id`` up front, since a getMore knows
487+
it before the command is built and needs it even if the operation fails.
502488
503489
``set_current=False`` leaves the span and the operation-name contextvar
504490
alone, for a caller that makes it current with ``use_operation_span``.

pymongo/asynchronous/change_stream.py

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -250,14 +250,9 @@ async def _run_aggregation_cmd(
250250
result_processor=self._process_result,
251251
comment=self._comment,
252252
)
253-
# Deliberately no operation_telemetry is attached to the resulting
254-
# cursor here: a change stream can tail indefinitely, so an operation
255-
# span covering its whole lifetime (initial query + every getMore,
256-
# like other command cursors) would never end while it's watching.
257-
# Leaving it unattached means each getMore instead gets its own
258-
# short-lived sibling "getMore" operation span, less ideal nesting,
259-
# but not a leaked/never-exported span. Do not "fix" this without
260-
# addressing that tradeoff.
253+
# No operation span is attached to the resulting cursor: a change stream
254+
# can tail indefinitely, so a span covering its whole lifetime would
255+
# never end. Each getMore gets its own sibling span instead.
261256
return await self._client._retryable_read(
262257
cmd.get_cursor,
263258
self._target._read_preference_for(session),

pymongo/asynchronous/client_bulk.py

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -336,11 +336,8 @@ async def _process_results_cursor(
336336
session=session,
337337
comment=self.comment,
338338
)
339-
# This cursor's getMores run inside the enclosing bulkWrite
340-
# operation span, so their command spans belong under it directly;
341-
# a getMore operation span of their own would be spurious. The
342-
# cursor is also per-batch and never surfaces to the caller, so
343-
# there is no cursor-lifetime span to own here.
339+
# These getMores run inside the enclosing bulkWrite operation span,
340+
# so a getMore operation span of their own would be spurious.
344341
cmd_cursor._reuse_current_span_for_getmore = True
345342
await cmd_cursor._maybe_pin_connection(conn)
346343

pymongo/asynchronous/command_cursor.py

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -246,10 +246,8 @@ async def _refresh(self) -> int:
246246
if not own_span:
247247
await self._send_message(getmore)
248248
else:
249-
# _send_message ends the span itself on every failure path, and
250-
# an exhausted cursor's close() ends it on the way out; both are
251-
# idempotent, so only a successful send leaving the cursor open
252-
# is left to handle here.
249+
# _send_message ends the span on every failure path and close()
250+
# ends it once exhausted, leaving only this case.
253251
try:
254252
await self._send_message(getmore)
255253
except BaseException as exc:

pymongo/asynchronous/cursor.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1089,9 +1089,8 @@ async def _refresh(self) -> int:
10891089
collection=self._collection.name,
10901090
set_current=False,
10911091
)
1092-
# The query's span covers the query alone unless this cursor is
1093-
# being drained by the public API call that created it, in which
1094-
# case the span stays open to cover that call's getMores too.
1092+
# The query's span stays open only when the call that created this
1093+
# cursor drains it itself, to cover that call's getMores too.
10951094
own_span = not is_internal_cursor_iteration()
10961095
await self._send_message_in_operation_span(q, own_span)
10971096
elif self._id: # Get More

pymongo/asynchronous/mongo_client.py

Lines changed: 15 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1912,12 +1912,9 @@ async def _run_operation(
19121912
:param address: Optional address when sending a message
19131913
to a specific server, used for getMore.
19141914
:param operation_telemetry: The calling cursor's operation span, or None,
1915-
so this send's command spans nest under it. Covers one caller-driven
1916-
getMore, or a whole API call that drains the cursor itself.
1917-
:param reuse_current_span: Create no operation span at all and leave the
1918-
ambient span in place as the parent for this operation's command
1919-
spans. Mutually exclusive with ``operation_telemetry``. Defaults to
1920-
False.
1915+
so this send's command spans nest under it.
1916+
:param reuse_current_span: Leave the ambient span as the parent for this
1917+
operation's command spans, creating none, defaults to False.
19211918
"""
19221919
if operation.conn_mgr:
19231920
server = await self._select_server(
@@ -2028,13 +2025,9 @@ async def _retry_internal(
20282025
:param operation_id: Stable operation id shared across retries, defaults to None
20292026
:param operation_telemetry: A cursor's operation span, which this call
20302027
makes current but neither creates nor ends, defaults to None.
2031-
:param reuse_current_span: Create no operation span at all and leave the
2032-
ambient span in place as the parent for this operation's command
2033-
spans. For callers that know a suitable operation span is already
2034-
current, where a second one would be spurious (the client
2035-
bulk-write results cursor's getMores, which belong under the
2036-
enclosing bulkWrite span). Mutually exclusive with
2037-
``operation_telemetry``. Defaults to False.
2028+
:param reuse_current_span: Leave the ambient span as the parent for this
2029+
operation's command spans, creating none. Mutually exclusive with
2030+
``operation_telemetry``, defaults to False.
20382031
20392032
:return: Output of the calling func()
20402033
"""
@@ -2087,10 +2080,7 @@ async def _retryable_read(
20872080
:param is_aggregate_write: If this is a aggregate operation with a write, defaults to False.
20882081
:param operation_id: Stable operation id shared across retries, defaults to None
20892082
:param operation_telemetry: As for ``_retry_internal``, defaults to None.
2090-
:param reuse_current_span: Create no operation span at all and leave the
2091-
ambient span in place as the parent for this operation's command
2092-
spans. Mutually exclusive with ``operation_telemetry``. Defaults to
2093-
False.
2083+
:param reuse_current_span: As for ``_retry_internal``, defaults to False.
20942084
"""
20952085

20962086
# Ensure that the client supports retrying on reads and there is no session in
@@ -2133,15 +2123,12 @@ async def _retryable_read_cursor(
21332123
"""Run a command cursor read within its own operation span.
21342124
21352125
Takes the same arguments as :meth:`_retryable_read`, plus the namespace
2136-
for the span. A command cursor's first batch is fetched inside that
2137-
call, before the cursor exists, so the span cannot be owned by the
2138-
cursor the way a find cursor's is; create it here instead.
2139-
2140-
The span ends with the command that created the cursor. Later getMores
2141-
belong to whoever drives iteration: each one the caller drives gets an
2142-
operation span of its own, so only a public API call that drains the
2143-
cursor itself (see ``_otel.internal_cursor_iteration``) keeps this one
2144-
open, by handing it to the cursor.
2126+
for the span. A command cursor's first batch is fetched before the cursor
2127+
exists, so the span cannot be owned by the cursor and is created here.
2128+
2129+
The span ends with the creating command, unless a public API call
2130+
draining the cursor itself (see ``_otel.internal_cursor_iteration``)
2131+
keeps it open by handing it to the cursor.
21452132
"""
21462133
operation_telemetry = _operation_telemetry_or_none(
21472134
self.options.tracing,
@@ -3041,9 +3028,8 @@ def __init__(
30413028
self._operation_id = operation_id
30423029
if reuse_current_span and operation_telemetry is not None:
30433030
raise ValueError("reuse_current_span and operation_telemetry are mutually exclusive")
3044-
# With nothing passed in, create the span here and end it in run(); with a
3045-
# span passed in, the caller owns it. reuse_current_span means an enclosing
3046-
# span is already current, so this object creates none.
3031+
# With nothing passed in, create the span here and end it in run(); with
3032+
# a span passed in, or reuse_current_span, an outer caller owns it.
30473033
self._owns_telemetry = operation_telemetry is None and not reuse_current_span
30483034
if self._owns_telemetry:
30493035
operation_telemetry = _operation_telemetry_or_none(

pymongo/cursor_shared.py

Lines changed: 14 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,8 @@ class _AgnosticCursorBase(Generic[_DocumentType], ABC):
5858
_session: Optional[Any]
5959
_killed: bool
6060
_operation_telemetry: Optional[Any] = None
61-
# Set by callers whose getMores belong under an operation span that is
62-
# already current (the client bulk-write results cursor), rather than under
63-
# a getMore operation span of their own.
61+
# Set by callers whose getMores belong under an already-current operation
62+
# span (the client bulk-write results cursor) instead of one of their own.
6463
_reuse_current_span_for_getmore: bool = False
6564

6665
@abstractmethod
@@ -125,13 +124,9 @@ def _prepare_to_die(self, already_killed: bool) -> tuple[int, Optional[_CursorAd
125124
def _end_operation_telemetry(self, exc: Optional[BaseException] = None) -> None:
126125
"""End the operation span currently attached to this cursor, exactly once.
127126
128-
No span is ever scoped to the cursor's lifetime: a caller-driven getMore
129-
attaches a span of its own and ends it as soon as that getMore
130-
completes. What can outlive a single command is the span of a public API
131-
call that drains the cursor itself (see
132-
``_otel.internal_cursor_iteration``), which ends when the cursor is
133-
exhausted or, for a cursor abandoned part-way, at close()/__del__.
134-
Idempotent, so every one of those paths can call it unconditionally.
127+
Only the span of a public API call draining the cursor itself outlives a
128+
single command (see ``_otel.internal_cursor_iteration``). Idempotent, so
129+
every path that might end a cursor can call it unconditionally.
135130
"""
136131
telemetry = self._operation_telemetry
137132
if telemetry is None:
@@ -145,22 +140,18 @@ def _end_operation_telemetry(self, exc: Optional[BaseException] = None) -> None:
145140
def _start_getmore_operation_telemetry(self, dbname: str, collname: Optional[str]) -> bool:
146141
"""Give the getMore about to be sent an operation span of its own.
147142
148-
The spec requires an operation span per caller-driven getMore, and
149-
forbids nesting it under the operation that created the cursor, since
150-
the application may do unrelated work between batches.
143+
Never nested under the operation that created the cursor, since the
144+
application may do unrelated work between batches.
151145
152-
Returns True when the caller now owns a span and must end it once the
153-
getMore completes. Returns False when this getMore already belongs to
154-
another operation, or when tracing is off.
146+
:return: True when the caller now owns the span and must end it.
155147
"""
156148
if self._operation_telemetry is not None or self._reuse_current_span_for_getmore:
157149
return False
158150
tracing_options = self._collection.database.client.options.tracing
159151
if not _otel._is_tracing_enabled(tracing_options):
160152
return False
161-
# A cursor opened by a command (listCollections, listIndexes, a
162-
# database-level aggregate) reports a namespace like
163-
# "$cmd.listCollections", which names no user collection.
153+
# A command cursor's namespace looks like "$cmd.listCollections", which
154+
# names no user collection.
164155
if _otel.is_command_namespace(collname):
165156
collname = None
166157
self._operation_telemetry = _operation_telemetry_or_none(
@@ -177,15 +168,10 @@ def _start_getmore_operation_telemetry(self, dbname: str, collname: Optional[str
177168
def _attach_operation_telemetry(self, telemetry: Any) -> None:
178169
"""Adopt the still-open operation span of the call that created this cursor.
179170
180-
For command cursors only, and only when that call goes on to drain the
181-
cursor itself, so its getMores belong to the same operation (see
182-
``_otel.internal_cursor_iteration``). A cursor returned to the caller
183-
has its creating span ended right away and never gets here.
184-
185-
A command cursor exhausted by its first batch is marked ``_killed`` in
186-
``__init__`` without calling ``close()``, so no getMore is sent and
187-
neither ``_refresh()`` nor ``_die_lock()`` runs. Ending the span here
188-
keeps that case prompt instead of leaving it to ``__del__``.
171+
Only for a call that drains the cursor itself, so its getMores belong to
172+
the same operation (see ``_otel.internal_cursor_iteration``). A cursor
173+
exhausted by its first batch never calls ``close()``, so end its span
174+
here rather than leaving it to ``__del__``.
189175
"""
190176
self._operation_telemetry = telemetry
191177
if self._killed:

pymongo/synchronous/change_stream.py

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -248,14 +248,9 @@ def _run_aggregation_cmd(self, session: Optional[ClientSession]) -> CommandCurso
248248
result_processor=self._process_result,
249249
comment=self._comment,
250250
)
251-
# Deliberately no operation_telemetry is attached to the resulting
252-
# cursor here: a change stream can tail indefinitely, so an operation
253-
# span covering its whole lifetime (initial query + every getMore,
254-
# like other command cursors) would never end while it's watching.
255-
# Leaving it unattached means each getMore instead gets its own
256-
# short-lived sibling "getMore" operation span, less ideal nesting,
257-
# but not a leaked/never-exported span. Do not "fix" this without
258-
# addressing that tradeoff.
251+
# No operation span is attached to the resulting cursor: a change stream
252+
# can tail indefinitely, so a span covering its whole lifetime would
253+
# never end. Each getMore gets its own sibling span instead.
259254
return self._client._retryable_read(
260255
cmd.get_cursor,
261256
self._target._read_preference_for(session),

pymongo/synchronous/client_bulk.py

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -334,11 +334,8 @@ def _process_results_cursor(
334334
session=session,
335335
comment=self.comment,
336336
)
337-
# This cursor's getMores run inside the enclosing bulkWrite
338-
# operation span, so their command spans belong under it directly;
339-
# a getMore operation span of their own would be spurious. The
340-
# cursor is also per-batch and never surfaces to the caller, so
341-
# there is no cursor-lifetime span to own here.
337+
# These getMores run inside the enclosing bulkWrite operation span,
338+
# so a getMore operation span of their own would be spurious.
342339
cmd_cursor._reuse_current_span_for_getmore = True
343340
cmd_cursor._maybe_pin_connection(conn)
344341

pymongo/synchronous/command_cursor.py

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -246,10 +246,8 @@ def _refresh(self) -> int:
246246
if not own_span:
247247
self._send_message(getmore)
248248
else:
249-
# _send_message ends the span itself on every failure path, and
250-
# an exhausted cursor's close() ends it on the way out; both are
251-
# idempotent, so only a successful send leaving the cursor open
252-
# is left to handle here.
249+
# _send_message ends the span on every failure path and close()
250+
# ends it once exhausted, leaving only this case.
253251
try:
254252
self._send_message(getmore)
255253
except BaseException as exc:

0 commit comments

Comments
 (0)