Skip to content

DRIVERS-3598 Specify and test getMore span nesting in the OpenTelemetry spec - #1973

Open
blink1073 wants to merge 14 commits into
mongodb:masterfrom
blink1073:DRIVERS-3598
Open

DRIVERS-3598 Specify and test getMore span nesting in the OpenTelemetry spec#1973
blink1073 wants to merge 14 commits into
mongodb:masterfrom
blink1073:DRIVERS-3598

Conversation

@blink1073

@blink1073 blink1073 commented Aug 12, 2026

Copy link
Copy Markdown
Member

Please complete the following before merging:

  • Is the relevant DRIVERS ticket in the PR title?

Context

Command spans must nest under "the corresponding driver operation span", which is unambiguous only while an operation sends a single command. For a cursor, the spec never said which operation span a getMore belongs to, and no fixture exercised getMore — so both readings passed the suite, and db.mongodb.cursor_id was asserted absent in all 25 places it appeared.

That ambiguity was not hypothetical. PyMongo had implemented one reading (a single operation span covering a cursor's whole lifetime), and the new fixture caught it on its first run.

Changes

Nesting. A caller-driven getMore gets its own operation span, sibling to the cursor-creating operation's span. Driver-internal iteration, where one public API call drains the cursor itself, creates no additional operation spans. No span is scoped to a cursor's lifetime, so a cursor that is never exhausted leaves nothing unfinished. A change-stream resume ends the failed getMore operation span rather than extending it. A cursor iterated inside a transaction nests under the withTransaction span or the pseudo operation transaction span, depending on which API started it.

db.mongodb.cursor_id. Raised from SHOULD to MUST and added to operation spans. It holds the id the driver sent for a getMore, even when that reply returns 0. It is omitted rather than emitted as 0 for a cursor-creating command that leaves no cursor open, and omitted for commands that may operate on several cursors at once.

Tests. operation/get_more.yml covers the nesting and both cursor_id outcomes. transaction/get_more.yml covers a cursor iterated inside a core-API transaction. Both use ignoreExtraSpans: false. Prose tests 3 and 4 cover the two things the unified format cannot express: that cursor_id holds the id sent rather than the 0 returned ($$type cannot exclude 0), and the convenient transaction API's callback.

Unified test format. ignoreExtraSpans applies at every level of the span tree. Without this, the negative assertion in operation/get_more.yml — that the getMore is not nested under find — is not enforceable by a conformant runner.

Two decisions worth a second opinion

Neither is stated in the design document, so please confirm rather than assume:

  1. Span name getMore <db>.<collection>, following the existing convention of naming operation spans after commands rather than public-API methods. Naming it after the driver's iteration method would vary per driver and make the test unassertable cross-driver.
  2. Cursor id 0 omitted rather than emitted as a literal 0. This matches the existing find.yml and aggregate.yml fixtures, which omit cursor_id on commands that exhaust in the first batch, and OpenTelemetry's convention of omitting unavailable attributes rather than encoding a sentinel.

Verification

Both fixtures pass against PyMongo, transaction/get_more.yml unchanged from the first run. Prose tests 3 and 4 are implemented there. The generated JSON is byte-identical to a fresh run of this repository's generator, and both fixtures are schema-valid against their declared schema version 1.27.

AI disclosure

Drafted with Claude Code (Claude Opus 5 and Sonnet 5). Reviewed by the human author before opening.

Resolves a changelog conflict in source/open-telemetry/open-telemetry.md:
upstream's DRIVERS-3597 entry and this branch's DRIVERS-3598 entry were
both prepended to the same section. Kept both, newest first.
Comment thread source/open-telemetry/open-telemetry.md Outdated
This operation span MUST NOT be nested under the operation span of the command that created the cursor. A host
application may do unrelated work between batches, and nesting each `getMore` under the cursor-creating operation would
attribute that work to the original operation. Ordinary nesting still applies otherwise: a cursor iterated inside a
`withTransaction` callback nests into the `withTransaction` span.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
`withTransaction` callback nests into the `withTransaction` span.
`withTransaction` callback nests into the `withTransaction` span; a cursor iterated inside a transaction created using core transactions api nests into the pseudo operation `transaction` span.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

Comment thread source/open-telemetry/tests/README.md Outdated
5. Assert that on each of those two spans, the value is the cursor id the driver sent in the `getMore` command, and not
the `0` returned in that command's reply.

*Test 4: `getMore` inside a transaction nests under the transaction span*

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to implement this test as a unified? If we do not use the convenient transaction api, it seems to be possible. Something like this:

operations:
  - { name: startTransaction, object: *session0 }
  - name: createFindCursor
    object: *collection0
    arguments: { filter: {}, batchSize: 2, session: *session0 }
    saveResultAsEntity: &cursor0 cursor0
  - { name: iterateUntilDocumentOrError, object: *cursor0, expectResult: { _id: 1 } }
  - { name: iterateUntilDocumentOrError, object: *cursor0, expectResult: { _id: 2 } }
  - { name: iterateUntilDocumentOrError, object: *cursor0, expectResult: { _id: 3 } }
  - { name: commitTransaction, object: *session0 }

expectTracingMessages:
  - client: *client0
    ignoreExtraSpans: false
    spans:
      - name: transaction
        attributes: { db.system.name: mongodb }
        nested:
          - name: find transaction-get-more.test
          - name: getMore transaction-get-more.test
          - name: commitTransaction admin

If we want to test withTransaction explicitly, then we need a prose test.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a unified test and clarified the intent of the prose test

blink1073 added a commit to mongodb/mongo-python-driver that referenced this pull request Aug 18, 2026
Adds tests/transaction/get_more.yml from DRIVERS-3598, which covers a
cursor iterated inside a transaction started with the core transaction
API: the getMore operation span is a sibling of the find operation span
that created the cursor, and both nest under the transaction span.

Review feedback on mongodb/specifications#1973 asked for this case as a
unified test rather than a prose test. It passes against the driver
unchanged. The fixture comes from a specification change that is not
merged yet.
Review feedback asked for the transaction case as a unified test rather
than a prose test. A cursor iterated inside a transaction started with
the core transaction API is expressible in the unified format, since the
operations are flat rather than inside a callback, so add
tests/transaction/get_more.yml for it.

Prose test 4 now covers the convenient transaction API specifically, and
points at the unified test for the core API case.

Also state in the spec that a cursor iterated inside a transaction
started with the core transaction API nests into the pseudo operation
`transaction` span, alongside the existing `withTransaction` case.

@comandeo-mongo comandeo-mongo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work, thank you!

@blink1073

Copy link
Copy Markdown
Member Author

@paulinevos, can you please review the changes to unified-test-format.md and approve on behalf of dbx-spec-maintainers-unified-test-format?

@nhachicha
nhachicha self-requested a review August 19, 2026 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants