diff --git a/doc/changelog.rst b/doc/changelog.rst index 3571516bfd..06fe5ec688 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -1,6 +1,13 @@ Changelog ========= +Changes in Version 4.6 +---------------------- + +PyMongo 4.6 brings a number of improvements including: +- Added the :attr:`pymongo.monitoring.CommandSucceededEvent.database_name` property. +- Added the :attr:`pymongo.monitoring.CommandFailedEvent.database_name` property. + Changes in Version 4.5 ---------------------- diff --git a/pymongo/message.py b/pymongo/message.py index c370cfa03c..5190ce23e2 100644 --- a/pymongo/message.py +++ b/pymongo/message.py @@ -1097,6 +1097,7 @@ def _succeed(self, request_id: int, reply: _DocumentOut, duration: timedelta) -> self.conn.address, self.op_id, self.conn.service_id, + database_name=self.db_name, ) def _fail(self, request_id: int, failure: _DocumentOut, duration: timedelta) -> None: @@ -1109,6 +1110,7 @@ def _fail(self, request_id: int, failure: _DocumentOut, duration: timedelta) -> self.conn.address, self.op_id, self.conn.service_id, + database_name=self.db_name, ) diff --git a/pymongo/monitoring.py b/pymongo/monitoring.py index a14d9a9110..5bc3fda497 100644 --- a/pymongo/monitoring.py +++ b/pymongo/monitoring.py @@ -561,7 +561,7 @@ def _is_speculative_authenticate(command_name: str, doc: Mapping[str, Any]) -> b class _CommandEvent: """Base class for command events.""" - __slots__ = ("__cmd_name", "__rqst_id", "__conn_id", "__op_id", "__service_id") + __slots__ = ("__cmd_name", "__rqst_id", "__conn_id", "__op_id", "__service_id", "__db") def __init__( self, @@ -570,12 +570,14 @@ def __init__( connection_id: _Address, operation_id: Optional[int], service_id: Optional[ObjectId] = None, + database_name: str = "", ) -> None: self.__cmd_name = command_name self.__rqst_id = request_id self.__conn_id = connection_id self.__op_id = operation_id self.__service_id = service_id + self.__db = database_name @property def command_name(self) -> str: @@ -605,6 +607,14 @@ def operation_id(self) -> Optional[int]: """An id for this series of events or None.""" return self.__op_id + @property + def database_name(self) -> str: + """The database_name this command was sent to, or ``""``. + + .. versionadded:: 4.6 + """ + return self.__db + class CommandStartedEvent(_CommandEvent): """Event published when a command starts. @@ -619,7 +629,7 @@ class CommandStartedEvent(_CommandEvent): - `service_id`: The service_id this command was sent to, or ``None``. """ - __slots__ = ("__cmd", "__db") + __slots__ = ("__cmd",) def __init__( self, @@ -635,14 +645,18 @@ def __init__( # Command name must be first key. command_name = next(iter(command)) super().__init__( - command_name, request_id, connection_id, operation_id, service_id=service_id + command_name, + request_id, + connection_id, + operation_id, + service_id=service_id, + database_name=database_name, ) cmd_name = command_name.lower() if cmd_name in _SENSITIVE_COMMANDS or _is_speculative_authenticate(cmd_name, command): self.__cmd: _DocumentOut = {} else: self.__cmd = command - self.__db = database_name @property def command(self) -> _DocumentOut: @@ -652,7 +666,7 @@ def command(self) -> _DocumentOut: @property def database_name(self) -> str: """The name of the database this command was run against.""" - return self.__db + return super().database_name def __repr__(self) -> str: return ("<{} {} db: {!r}, command: {!r}, operation_id: {}, service_id: {}>").format( @@ -677,6 +691,7 @@ class CommandSucceededEvent(_CommandEvent): was sent to. - `operation_id`: An optional identifier for a series of related events. - `service_id`: The service_id this command was sent to, or ``None``. + - `database_name`: The database this command was sent to, or ``""``. """ __slots__ = ("__duration_micros", "__reply") @@ -690,9 +705,15 @@ def __init__( connection_id: _Address, operation_id: Optional[int], service_id: Optional[ObjectId] = None, + database_name: str = "", ) -> None: super().__init__( - command_name, request_id, connection_id, operation_id, service_id=service_id + command_name, + request_id, + connection_id, + operation_id, + service_id=service_id, + database_name=database_name, ) self.__duration_micros = _to_micros(duration) cmd_name = command_name.lower() @@ -713,10 +734,11 @@ def reply(self) -> _DocumentOut: def __repr__(self) -> str: return ( - "<{} {} command: {!r}, operation_id: {}, duration_micros: {}, service_id: {}>" + "<{} {} db: {!r}, command: {!r}, operation_id: {}, duration_micros: {}, service_id: {}>" ).format( self.__class__.__name__, self.connection_id, + self.database_name, self.command_name, self.operation_id, self.duration_micros, @@ -736,6 +758,7 @@ class CommandFailedEvent(_CommandEvent): was sent to. - `operation_id`: An optional identifier for a series of related events. - `service_id`: The service_id this command was sent to, or ``None``. + - `database_name`: The database this command was sent to, or ``""``. """ __slots__ = ("__duration_micros", "__failure") @@ -749,9 +772,15 @@ def __init__( connection_id: _Address, operation_id: Optional[int], service_id: Optional[ObjectId] = None, + database_name: str = "", ) -> None: super().__init__( - command_name, request_id, connection_id, operation_id, service_id=service_id + command_name, + request_id, + connection_id, + operation_id, + service_id=service_id, + database_name=database_name, ) self.__duration_micros = _to_micros(duration) self.__failure = failure @@ -768,11 +797,12 @@ def failure(self) -> _DocumentOut: def __repr__(self) -> str: return ( - "<{} {} command: {!r}, operation_id: {}, duration_micros: {}, " + "<{} {} db: {!r}, command: {!r}, operation_id: {}, duration_micros: {}, " "failure: {!r}, service_id: {}>" ).format( self.__class__.__name__, self.connection_id, + self.database_name, self.command_name, self.operation_id, self.duration_micros, @@ -1491,6 +1521,7 @@ def publish_command_success( op_id: Optional[int] = None, service_id: Optional[ObjectId] = None, speculative_hello: bool = False, + database_name: str = "", ) -> None: """Publish a CommandSucceededEvent to all command listeners. @@ -1504,6 +1535,7 @@ def publish_command_success( - `op_id`: The (optional) operation id for this operation. - `service_id`: The service_id this command was sent to, or ``None``. - `speculative_hello`: Was the command sent with speculative auth? + - `database_name`: The database this command was sent to, or ``""``. """ if op_id is None: op_id = request_id @@ -1512,7 +1544,14 @@ def publish_command_success( # speculativeAuthenticate. reply = {} event = CommandSucceededEvent( - duration, reply, command_name, request_id, connection_id, op_id, service_id + duration, + reply, + command_name, + request_id, + connection_id, + op_id, + service_id, + database_name=database_name, ) for subscriber in self.__command_listeners: try: @@ -1529,6 +1568,7 @@ def publish_command_failure( connection_id: _Address, op_id: Optional[int] = None, service_id: Optional[ObjectId] = None, + database_name: str = "", ) -> None: """Publish a CommandFailedEvent to all command listeners. @@ -1542,11 +1582,19 @@ def publish_command_failure( command was sent to. - `op_id`: The (optional) operation id for this operation. - `service_id`: The service_id this command was sent to, or ``None``. + - `database_name`: The database this command was sent to, or ``""``. """ if op_id is None: op_id = request_id event = CommandFailedEvent( - duration, failure, command_name, request_id, connection_id, op_id, service_id=service_id + duration, + failure, + command_name, + request_id, + connection_id, + op_id, + service_id=service_id, + database_name=database_name, ) for subscriber in self.__command_listeners: try: diff --git a/pymongo/network.py b/pymongo/network.py index b5de9092ce..14160a516c 100644 --- a/pymongo/network.py +++ b/pymongo/network.py @@ -205,7 +205,13 @@ def command( assert listeners is not None assert address is not None listeners.publish_command_failure( - duration, failure, name, request_id, address, service_id=conn.service_id + duration, + failure, + name, + request_id, + address, + service_id=conn.service_id, + database_name=dbname, ) raise if publish: @@ -220,6 +226,7 @@ def command( address, service_id=conn.service_id, speculative_hello=speculative_hello, + database_name=dbname, ) if client and client._encrypter and reply: diff --git a/pymongo/server.py b/pymongo/server.py index 93bdeadeb4..985d45acb3 100644 --- a/pymongo/server.py +++ b/pymongo/server.py @@ -178,6 +178,7 @@ def run_operation( request_id, conn.address, service_id=conn.service_id, + database_name=dbn, ) raise @@ -203,6 +204,7 @@ def run_operation( request_id, conn.address, service_id=conn.service_id, + database_name=dbn, ) # Decrypt response. diff --git a/test/command_monitoring/find.json b/test/command_monitoring/find.json index 4b5f45ae99..bc9668499b 100644 --- a/test/command_monitoring/find.json +++ b/test/command_monitoring/find.json @@ -1,6 +1,6 @@ { "description": "find", - "schemaVersion": "1.1", + "schemaVersion": "1.15", "createEntities": [ { "client": { @@ -103,7 +103,8 @@ ] } }, - "commandName": "find" + "commandName": "find", + "databaseName": "command-monitoring-tests" } } ] @@ -198,7 +199,8 @@ ] } }, - "commandName": "find" + "commandName": "find", + "databaseName": "command-monitoring-tests" } } ] @@ -262,7 +264,8 @@ ] } }, - "commandName": "find" + "commandName": "find", + "databaseName": "command-monitoring-tests" } } ] @@ -338,7 +341,8 @@ ] } }, - "commandName": "find" + "commandName": "find", + "databaseName": "command-monitoring-tests" } }, { @@ -376,7 +380,8 @@ ] } }, - "commandName": "getMore" + "commandName": "getMore", + "databaseName": "command-monitoring-tests" } } ] @@ -464,7 +469,8 @@ ] } }, - "commandName": "find" + "commandName": "find", + "databaseName": "command-monitoring-tests" } }, { @@ -498,7 +504,8 @@ ] } }, - "commandName": "getMore" + "commandName": "getMore", + "databaseName": "command-monitoring-tests" } } ] @@ -539,7 +546,8 @@ }, { "commandFailedEvent": { - "commandName": "find" + "commandName": "find", + "databaseName": "command-monitoring-tests" } } ] diff --git a/test/command_monitoring/writeConcernError.json b/test/command_monitoring/writeConcernError.json new file mode 100644 index 0000000000..7bc16f2ab7 --- /dev/null +++ b/test/command_monitoring/writeConcernError.json @@ -0,0 +1,155 @@ +{ + "description": "writeConcernError", + "schemaVersion": "1.4", + "runOnRequirements": [ + { + "minServerVersion": "4.1.0", + "topologies": [ + "replicaset" + ], + "serverless": "forbid" + } + ], + "createEntities": [ + { + "client": { + "id": "client", + "observeEvents": [ + "commandStartedEvent", + "commandSucceededEvent", + "commandFailedEvent" + ] + } + }, + { + "database": { + "id": "database", + "client": "client", + "databaseName": "command-monitoring-tests" + } + }, + { + "collection": { + "id": "collection", + "database": "database", + "collectionName": "test" + } + } + ], + "initialData": [ + { + "collectionName": "test", + "databaseName": "command-monitoring-tests", + "documents": [ + { + "_id": 1, + "x": 11 + } + ] + } + ], + "tests": [ + { + "description": "A retryable write with write concern errors publishes success event", + "operations": [ + { + "name": "failPoint", + "object": "testRunner", + "arguments": { + "client": "client", + "failPoint": { + "configureFailPoint": "failCommand", + "mode": { + "times": 1 + }, + "data": { + "failCommands": [ + "insert" + ], + "writeConcernError": { + "code": 91, + "errorLabels": [ + "RetryableWriteError" + ] + } + } + } + } + }, + { + "name": "insertOne", + "object": "collection", + "arguments": { + "document": { + "_id": 2, + "x": 22 + } + } + } + ], + "expectEvents": [ + { + "client": "client", + "events": [ + { + "commandStartedEvent": { + "command": { + "insert": "test", + "documents": [ + { + "_id": 2, + "x": 22 + } + ], + "ordered": true + }, + "commandName": "insert", + "databaseName": "command-monitoring-tests" + } + }, + { + "commandSucceededEvent": { + "reply": { + "ok": 1, + "n": 1, + "writeConcernError": { + "code": 91, + "errorLabels": [ + "RetryableWriteError" + ] + } + }, + "commandName": "insert" + } + }, + { + "commandStartedEvent": { + "command": { + "insert": "test", + "documents": [ + { + "_id": 2, + "x": 22 + } + ], + "ordered": true + }, + "commandName": "insert", + "databaseName": "command-monitoring-tests" + } + }, + { + "commandSucceededEvent": { + "reply": { + "ok": 1, + "n": 1 + }, + "commandName": "insert" + } + } + ] + } + ] + } + ] +} diff --git a/test/test_monitoring.py b/test/test_monitoring.py index 7aa3d67ebb..8ccc844d32 100644 --- a/test/test_monitoring.py +++ b/test/test_monitoring.py @@ -1087,7 +1087,12 @@ def test_sensitive_commands(self): listeners.publish_command_start(cmd, "pymongo_test", 12345, self.client.address) # type: ignore[arg-type] delta = datetime.timedelta(milliseconds=100) listeners.publish_command_success( - delta, {"nonce": "e474f4561c5eb40b", "ok": 1.0}, "getnonce", 12345, self.client.address # type: ignore[arg-type] + delta, + {"nonce": "e474f4561c5eb40b", "ok": 1.0}, + "getnonce", + 12345, + self.client.address, # type: ignore[arg-type] + database_name="pymongo_test", ) started = self.listener.started_events[0] succeeded = self.listener.succeeded_events[0] @@ -1148,9 +1153,9 @@ def test_simple(self): class TestEventClasses(unittest.TestCase): def test_command_event_repr(self): - request_id, connection_id, operation_id = 1, ("localhost", 27017), 2 + request_id, connection_id, operation_id, db_name = 1, ("localhost", 27017), 2, "admin" event = monitoring.CommandStartedEvent( - {"ping": 1}, "admin", request_id, connection_id, operation_id + {"ping": 1}, db_name, request_id, connection_id, operation_id ) self.assertEqual( repr(event), @@ -1159,20 +1164,20 @@ def test_command_event_repr(self): ) delta = datetime.timedelta(milliseconds=100) event = monitoring.CommandSucceededEvent( - delta, {"ok": 1}, "ping", request_id, connection_id, operation_id + delta, {"ok": 1}, "ping", request_id, connection_id, operation_id, database_name=db_name ) self.assertEqual( repr(event), - "", ) event = monitoring.CommandFailedEvent( - delta, {"ok": 0}, "ping", request_id, connection_id, operation_id + delta, {"ok": 0}, "ping", request_id, connection_id, operation_id, database_name=db_name ) self.assertEqual( repr(event), - "", ) diff --git a/test/unified_format.py b/test/unified_format.py index aebee8d5f2..80c6f03342 100644 --- a/test/unified_format.py +++ b/test/unified_format.py @@ -134,14 +134,14 @@ # Build up a placeholder map. PLACEHOLDER_MAP = {} -for (provider_name, provider_data) in [ +for provider_name, provider_data in [ ("local", {"key": LOCAL_MASTER_KEY}), ("aws", AWS_CREDS), ("azure", AZURE_CREDS), ("gcp", GCP_CREDS), ("kmip", KMIP_CREDS), ]: - for (key, value) in provider_data.items(): + for key, value in provider_data.items(): placeholder = f"/clientEncryptionOpts/kmsProviders/{provider_name}/{key}" PLACEHOLDER_MAP[placeholder] = value @@ -156,6 +156,7 @@ def with_metaclass(meta, *bases): Vendored from six: https://github.com/benjaminp/six/blob/master/six.py """ + # This requires a bit of explanation: the basic idea is to make a dummy # metaclass for one level of class instantiation that replaces itself with # the actual metaclass. @@ -746,6 +747,10 @@ def match_result(self, expectation, actual, in_recursive_call=False): self.test.assertEqual(expectation, actual) return None + def assertHasDatabaseName(self, spec, actual): + if "databaseName" in spec: + self.test.assertEqual(spec["databaseName"], actual.database_name) + def assertHasServiceId(self, spec, actual): if "hasServiceId" in spec: if spec.get("hasServiceId"): @@ -778,21 +783,21 @@ def match_event(self, event_type, expectation, actual): if name == "commandStartedEvent": self.test.assertIsInstance(actual, CommandStartedEvent) command = spec.get("command") - database_name = spec.get("databaseName") if command: self.match_result(command, actual.command) - if database_name: - self.test.assertEqual(database_name, actual.database_name) + self.assertHasDatabaseName(spec, actual) self.assertHasServiceId(spec, actual) elif name == "commandSucceededEvent": self.test.assertIsInstance(actual, CommandSucceededEvent) reply = spec.get("reply") if reply: self.match_result(reply, actual.reply) + self.assertHasDatabaseName(spec, actual) self.assertHasServiceId(spec, actual) elif name == "commandFailedEvent": self.test.assertIsInstance(actual, CommandFailedEvent) self.assertHasServiceId(spec, actual) + self.assertHasDatabaseName(spec, actual) elif name == "poolCreatedEvent": self.test.assertIsInstance(actual, PoolCreatedEvent) elif name == "poolReadyEvent": @@ -863,7 +868,7 @@ class UnifiedSpecTestMixinV1(IntegrationTest): a class attribute ``TEST_SPEC``. """ - SCHEMA_VERSION = Version.from_string("1.12") + SCHEMA_VERSION = Version.from_string("1.15") RUN_ON_LOAD_BALANCER = True RUN_ON_SERVERLESS = True TEST_SPEC: Any