PHPC-2255: Add database name to CommandFailedEvent and CommandSucceededEvent (#1543)

jmikola · web-flow · commit ef9abb18eabf · 2024-05-08T09:40:07.000-04:00
diff --git a/src/MongoDB/Monitoring/CommandFailedEvent.c b/src/MongoDB/Monitoring/CommandFailedEvent.c
@@ -43,6 +43,18 @@ static PHP_METHOD(MongoDB_Driver_Monitoring_CommandFailedEvent, getCommandName)
 	RETVAL_STRING(intern->command_name);
 }
 
+/* Returns the database name for this event */
+static PHP_METHOD(MongoDB_Driver_Monitoring_CommandFailedEvent, getDatabaseName)
+{
+	php_phongo_commandfailedevent_t* intern;
+
+	intern = Z_COMMANDFAILEDEVENT_OBJ_P(getThis());
+
+	PHONGO_PARSE_PARAMETERS_NONE();
+
+	RETVAL_STRING(intern->database_name);
+}
+
 /* Returns the event's duration in microseconds */
 static PHP_METHOD(MongoDB_Driver_Monitoring_CommandFailedEvent, getDurationMicros)
 {
@@ -192,6 +204,10 @@ static void php_phongo_commandfailedevent_free_object(zend_object* object)
 	if (intern->command_name) {
 		efree(intern->command_name);
 	}
+
+	if (intern->database_name) {
+		efree(intern->database_name);
+	}
 }
 
 static zend_object* php_phongo_commandfailedevent_create_object(zend_class_entry* class_type)
diff --git a/src/MongoDB/Monitoring/CommandFailedEvent.stub.php b/src/MongoDB/Monitoring/CommandFailedEvent.stub.php
@@ -13,6 +13,8 @@ final private function __construct() {}
 
     final public function getCommandName(): string {}
 
+    final public function getDatabaseName(): string {}
+
     final public function getDurationMicros(): int {}
 
     final public function getError(): \Exception {}
diff --git a/src/MongoDB/Monitoring/CommandFailedEvent_arginfo.h b/src/MongoDB/Monitoring/CommandFailedEvent_arginfo.h
diff --git a/src/MongoDB/Monitoring/CommandSucceededEvent.c b/src/MongoDB/Monitoring/CommandSucceededEvent.c
@@ -43,6 +43,18 @@ static PHP_METHOD(MongoDB_Driver_Monitoring_CommandSucceededEvent, getCommandNam
 	RETVAL_STRING(intern->command_name);
 }
 
+/* Returns the database name for this event */
+static PHP_METHOD(MongoDB_Driver_Monitoring_CommandSucceededEvent, getDatabaseName)
+{
+	php_phongo_commandsucceededevent_t* intern;
+
+	intern = Z_COMMANDSUCCEEDEDEVENT_OBJ_P(getThis());
+
+	PHONGO_PARSE_PARAMETERS_NONE();
+
+	RETVAL_STRING(intern->database_name);
+}
+
 /* Returns the event's duration in microseconds */
 static PHP_METHOD(MongoDB_Driver_Monitoring_CommandSucceededEvent, getDurationMicros)
 {
@@ -176,6 +188,10 @@ static void php_phongo_commandsucceededevent_free_object(zend_object* object)
 	if (intern->command_name) {
 		efree(intern->command_name);
 	}
+
+	if (intern->database_name) {
+		efree(intern->database_name);
+	}
 }
 
 static zend_object* php_phongo_commandsucceededevent_create_object(zend_class_entry* class_type)
diff --git a/src/MongoDB/Monitoring/CommandSucceededEvent.stub.php b/src/MongoDB/Monitoring/CommandSucceededEvent.stub.php
@@ -13,6 +13,8 @@ final private function __construct() {}
 
     final public function getCommandName(): string {}
 
+    final public function getDatabaseName(): string {}
+
     final public function getDurationMicros(): int {}
 
     final public function getOperationId(): string {}
diff --git a/src/MongoDB/Monitoring/CommandSucceededEvent_arginfo.h b/src/MongoDB/Monitoring/CommandSucceededEvent_arginfo.h
diff --git a/src/phongo_apm.c b/src/phongo_apm.c
@@ -144,11 +144,11 @@ static void phongo_apm_command_started(const mongoc_apm_command_started_t* event
 	p_event = Z_COMMANDSTARTEDEVENT_OBJ_P(&z_event);
 
 	p_event->command_name         = estrdup(mongoc_apm_command_started_get_command_name(event));
+	p_event->database_name        = estrdup(mongoc_apm_command_started_get_database_name(event));
 	p_event->server_id            = mongoc_apm_command_started_get_server_id(event);
 	p_event->operation_id         = mongoc_apm_command_started_get_operation_id(event);
 	p_event->request_id           = mongoc_apm_command_started_get_request_id(event);
 	p_event->command              = bson_copy(mongoc_apm_command_started_get_command(event));
-	p_event->database_name        = estrdup(mongoc_apm_command_started_get_database_name(event));
 	p_event->server_connection_id = mongoc_apm_command_started_get_server_connection_id_int64(event);
 	p_event->has_service_id       = mongoc_apm_command_started_get_service_id(event) != NULL;
 
@@ -190,6 +190,7 @@ static void phongo_apm_command_succeeded(const mongoc_apm_command_succeeded_t* e
 	p_event = Z_COMMANDSUCCEEDEDEVENT_OBJ_P(&z_event);
 
 	p_event->command_name         = estrdup(mongoc_apm_command_succeeded_get_command_name(event));
+	p_event->database_name        = estrdup(mongoc_apm_command_succeeded_get_database_name(event));
 	p_event->server_id            = mongoc_apm_command_succeeded_get_server_id(event);
 	p_event->operation_id         = mongoc_apm_command_succeeded_get_operation_id(event);
 	p_event->request_id           = mongoc_apm_command_succeeded_get_request_id(event);
@@ -237,6 +238,7 @@ static void phongo_apm_command_failed(const mongoc_apm_command_failed_t* event)
 	p_event = Z_COMMANDFAILEDEVENT_OBJ_P(&z_event);
 
 	p_event->command_name         = estrdup(mongoc_apm_command_failed_get_command_name(event));
+	p_event->database_name        = estrdup(mongoc_apm_command_failed_get_database_name(event));
 	p_event->server_id            = mongoc_apm_command_failed_get_server_id(event);
 	p_event->operation_id         = mongoc_apm_command_failed_get_operation_id(event);
 	p_event->request_id           = mongoc_apm_command_failed_get_request_id(event);
diff --git a/src/phongo_structs.h b/src/phongo_structs.h
@@ -283,6 +283,7 @@ typedef struct {
 typedef struct {
 	zval        manager;
 	char*       command_name;
+	char*       database_name;
 	uint32_t    server_id;
 	int64_t     operation_id;
 	int64_t     request_id;
@@ -298,11 +299,11 @@ typedef struct {
 typedef struct {
 	zval        manager;
 	char*       command_name;
+	char*       database_name;
 	uint32_t    server_id;
 	int64_t     operation_id;
 	int64_t     request_id;
 	bson_t*     command;
-	char*       database_name;
 	bool        has_service_id;
 	bson_oid_t  service_id;
 	int64_t     server_connection_id;
@@ -312,6 +313,7 @@ typedef struct {
 typedef struct {
 	zval        manager;
 	char*       command_name;
+	char*       database_name;
 	uint32_t    server_id;
 	int64_t     operation_id;
 	int64_t     request_id;
diff --git a/tests/apm/commandFailedEvent-001.phpt b/tests/apm/commandFailedEvent-001.phpt
@@ -8,14 +8,10 @@ MongoDB\Driver\Monitoring\CommandFailedEvent
 <?php
 require_once __DIR__ . "/../utils/basic.inc";
 
-$m = create_test_manager();
-
 class MySubscriber implements MongoDB\Driver\Monitoring\CommandSubscriber
 {
-    public function commandStarted( \MongoDB\Driver\Monitoring\CommandStartedEvent $event ): void
+    public function commandStarted(MongoDB\Driver\Monitoring\CommandStartedEvent $event): void
     {
-        echo "started: ", $event->getCommandName(), "\n";
-
         /* bson_get_monotonic_time() may only have 10-16 millisecond precision
          * on Windows. Sleep to ensure that a non-zero value is reported for
          * CommandFailedEvent's duration. */
@@ -24,51 +20,51 @@ class MySubscriber implements MongoDB\Driver\Monitoring\CommandSubscriber
         }
     }
 
-    public function commandSucceeded( \MongoDB\Driver\Monitoring\CommandSucceededEvent $event ): void
+    public function commandSucceeded(MongoDB\Driver\Monitoring\CommandSucceededEvent $event): void
     {
     }
 
-    public function commandFailed( \MongoDB\Driver\Monitoring\CommandFailedEvent $event ): void
+    public function commandFailed(MongoDB\Driver\Monitoring\CommandFailedEvent $event): void
     {
-        echo "failed: ", $event->getCommandName(), "\n";
-        echo "- getError() returns an object: ", is_object( $event->getError() ) ? 'yes' : 'no', "\n";
-        echo "- getError() returns an MongoDB\Driver\Exception\Exception object: ", $event->getError() instanceof MongoDB\Driver\Exception\Exception ? 'yes' : 'no', "\n";
-        echo "- getDurationMicros() returns an integer: ", is_integer( $event->getDurationMicros() ) ? 'yes' : 'no', "\n";
-        echo "- getDurationMicros() returns > 0: ", $event->getDurationMicros() > 0 ? 'yes' : 'no', "\n";
-        echo "- getCommandName() returns a string: ", is_string( $event->getCommandName() ) ? 'yes' : 'no', "\n";
-        echo "- getCommandName() returns '", $event->getCommandName(), "'\n";
-        echo "- getServer() returns an object: ", is_object( $event->getServer() ) ? 'yes' : 'no', "\n";
-        echo "- getServer() returns a Server object: ", $event->getServer() instanceof MongoDB\Driver\Server ? 'yes' : 'no', "\n";
-        echo "- getOperationId() returns a string: ", is_string( $event->getOperationId() ) ? 'yes' : 'no', "\n";
-        echo "- getRequestId() returns a string: ", is_string( $event->getRequestId() ) ? 'yes' : 'no', "\n";
+        var_dump($event->getCommandName());
+        var_dump($event->getDatabaseName());
+        var_dump($event->getDurationMicros());
+        echo "getDurationMicros() returns > 0: ", $event->getDurationMicros() > 0 ? 'yes' : 'no', "\n";
+        var_dump($event->getError() instanceof MongoDB\Driver\Exception\Exception);
+        var_dump($event->getOperationId());
+        var_dump($event->getReply());
+        var_dump($event->getRequestId());
+        var_dump($event->getServer());
+
+        /* Note: getServerConnectionId() and getServiceId() have more stringent
+         * requirements and are tested separately. */
     }
 }
 
-$subscriber = new MySubscriber;
+$manager = create_test_manager();
+
+$subscriber = new MySubscriber();
+MongoDB\Driver\Monitoring\addSubscriber($subscriber);
 
-MongoDB\Driver\Monitoring\addSubscriber( $subscriber );
+$command = new MongoDB\Driver\Command(['unsupportedCommand' => 1]);
 
-$primary = get_primary_server(URI);
-$command = new \MongoDB\Driver\Command([
-    'aggregate' => COLLECTION_NAME,
-    'pipeline' => [['$unsupported' => 1]]
-]);
 try {
-    $primary->executeCommand(DATABASE_NAME, $command);
+    $manager->executeCommand('admin', $command);
 } catch (Exception $e) {
-    /* Swallow */
 }
+
 ?>
---EXPECT--
-started: aggregate
-failed: aggregate
-- getError() returns an object: yes
-- getError() returns an MongoDB\Driver\Exception\Exception object: yes
-- getDurationMicros() returns an integer: yes
-- getDurationMicros() returns > 0: yes
-- getCommandName() returns a string: yes
-- getCommandName() returns 'aggregate'
-- getServer() returns an object: yes
-- getServer() returns a Server object: yes
-- getOperationId() returns a string: yes
-- getRequestId() returns a string: yes
+--EXPECTF--
+string(18) "unsupportedCommand"
+string(5) "admin"
+int(%d)
+getDurationMicros() returns > 0: yes
+bool(true)
+string(%d) "%d"
+object(stdClass)#%d (%d) {
+  %A
+}
+string(%d) "%d"
+object(MongoDB\Driver\Server)#%d (%d) {
+  %A
+}
diff --git a/tests/apm/commandStartedEvent-001.phpt b/tests/apm/commandStartedEvent-001.phpt
@@ -8,50 +8,47 @@ MongoDB\Driver\Monitoring\CommandStartedEvent
 <?php
 require_once __DIR__ . "/../utils/basic.inc";
 
-$m = create_test_manager();
-
 class MySubscriber implements MongoDB\Driver\Monitoring\CommandSubscriber
 {
-    public function commandStarted( \MongoDB\Driver\Monitoring\CommandStartedEvent $event ): void
+    public function commandStarted(MongoDB\Driver\Monitoring\CommandStartedEvent $event): void
     {
-        echo "started: ", $event->getCommandName(), "\n";
-        echo "- getCommand() returns an object: ", is_object( $event->getCommand() ) ? 'yes' : 'no', "\n";
-        echo "- getCommand() returns a stdClass object: ", $event->getCommand() instanceof stdClass ? 'yes' : 'no', "\n";
-        echo "- getDatabaseName() returns a string: ", is_string( $event->getDatabaseName() ) ? 'yes' : 'no', "\n";
-        echo "- getDatabaseName() returns '", $event->getDatabaseName(), "'\n";
-        echo "- getCommandName() returns a string: ", is_string( $event->getCommandName() ) ? 'yes' : 'no', "\n";
-        echo "- getCommandName() returns '", $event->getCommandName(), "'\n";
-        echo "- getServer() returns an object: ", is_object( $event->getServer() ) ? 'yes' : 'no', "\n";
-        echo "- getServer() returns a Server object: ", $event->getServer() instanceof MongoDB\Driver\Server ? 'yes' : 'no', "\n";
-        echo "- getOperationId() returns a string: ", is_string( $event->getOperationId() ) ? 'yes' : 'no', "\n";
-        echo "- getRequestId() returns a string: ", is_string( $event->getRequestId() ) ? 'yes' : 'no', "\n";
+        var_dump($event->getCommand());
+        var_dump($event->getCommandName());
+        var_dump($event->getDatabaseName());
+        var_dump($event->getOperationId());
+        var_dump($event->getRequestId());
+        var_dump($event->getServer());
+
+        /* Note: getServerConnectionId() and getServiceId() have more stringent
+         * requirements and are tested separately. */
     }
 
-    public function commandSucceeded( \MongoDB\Driver\Monitoring\CommandSucceededEvent $event ): void
+    public function commandSucceeded(MongoDB\Driver\Monitoring\CommandSucceededEvent $event): void
     {
     }
 
-    public function commandFailed( \MongoDB\Driver\Monitoring\CommandFailedEvent $event ): void
+    public function commandFailed(MongoDB\Driver\Monitoring\CommandFailedEvent $event): void
     {
     }
 }
 
-$query = new MongoDB\Driver\Query( [] );
-$subscriber = new MySubscriber;
+$manager = create_test_manager();
+
+$subscriber = new MySubscriber();
+MongoDB\Driver\Monitoring\addSubscriber($subscriber);
 
-MongoDB\Driver\Monitoring\addSubscriber( $subscriber );
+$command = new MongoDB\Driver\Command(['ping' => 1]);
+$manager->executeCommand('admin', $command);
 
-$cursor = $m->executeQuery( "demo.test", $query );
 ?>
---EXPECT--
-started: find
-- getCommand() returns an object: yes
-- getCommand() returns a stdClass object: yes
-- getDatabaseName() returns a string: yes
-- getDatabaseName() returns 'demo'
-- getCommandName() returns a string: yes
-- getCommandName() returns 'find'
-- getServer() returns an object: yes
-- getServer() returns a Server object: yes
-- getOperationId() returns a string: yes
-- getRequestId() returns a string: yes
+--EXPECTF--
+object(stdClass)#%d (%d) {
+  %A
+}
+string(4) "ping"
+string(5) "admin"
+string(%d) "%d"
+string(%d) "%d"
+object(MongoDB\Driver\Server)#%d (%d) {
+  %A
+}
diff --git a/tests/apm/commandSucceededEvent-001.phpt b/tests/apm/commandSucceededEvent-001.phpt
