GODRIVER-3698 Support command logging for transactions#2288
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements command logging support for MongoDB transaction operations (abortTransaction and commitTransaction), addressing the requirement that only handshake and heartbeat commands should be excluded from logging.
Changes:
- Added logger field and Logger() setter method to CommitTransaction and AbortTransaction operation structs
- Updated session methods to pass the client's logger to transaction operations
- Updated specifications submodule reference
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| x/mongo/driver/operation/commit_transaction.go | Added logger field and Logger() method to enable command logging for commit operations |
| x/mongo/driver/operation/abort_transaction.go | Added logger field and Logger() method to enable command logging for abort operations |
| mongo/session.go | Updated AbortTransaction and CommitTransaction to pass client logger to operations |
| testdata/specifications | Updated submodule reference to newer specifications version |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if ct == nil { | ||
| ct = new(CommitTransaction) | ||
| } |
There was a problem hiding this comment.
The nil check pattern is incorrect for a method receiver. If ct is nil when Logger() is called, this will panic before reaching the nil check. This pattern should be removed since calling a method on a nil receiver will panic regardless of this check.
| if at == nil { | ||
| at = new(AbortTransaction) | ||
| } |
There was a problem hiding this comment.
The nil check pattern is incorrect for a method receiver. If at is nil when Logger() is called, this will panic before reaching the nil check. This pattern should be removed since calling a method on a nil receiver will panic regardless of this check.
|
@qingyang-hu Note that the specification updates intentionally point at a forked commit to test: prestonvasquez/specifications@e3259f0 Context: mongodb/specifications#1881 |
🧪 Performance ResultsCommit SHA: 9ca15a4There were no significant changes to the performance to report for version 697259a064b388000722f6d8. For a comprehensive view of all microbenchmark results for this PR's commit, please check out the Evergreen perf task for this patch. |
API Change Report./v2/x/mongo/driver/operationcompatible changes(*AbortTransaction).Logger: added |
prestonvasquez
changed the title
prestonvasquez
force-pushed
the
bug/godriver3698-txn-cmds-not-logging
branch
from
031f942 to
8809a27
Compare
prestonvasquez
added
the
review-priority-normal
GODRIVER-3698
Summary
Add pipelines to support command logging for abortTransaction and commitTransaction.
Background & Motivation
Only handshake and heartbeat commands are excluded from logging. Transaction commands are not excluded.