diff --git a/agent/fw_laravel_queue.c b/agent/fw_laravel_queue.c index a76b9cfbf..165b1bc8b 100644 --- a/agent/fw_laravel_queue.c +++ b/agent/fw_laravel_queue.c @@ -303,8 +303,6 @@ NR_PHP_WRAPPER_END /* * Handles: * Illuminate\\Queue\\Worker::process - * Illuminate\\Queue\\SyncQueue::executeJob (laravel 11+) - * Illuminate\\Queue\\SyncQueue::push (before laravel 11) * * Ends/discards the unneeded worker txn * Starts the job txn @@ -340,8 +338,6 @@ NR_PHP_WRAPPER_END /* * Handles: * Illuminate\\Queue\\Worker::process - * Illuminate\\Queue\\SyncQueue::executeJob (laravel 11+) - * Illuminate\\Queue\\SyncQueue::push (before laravel 11) * * Closes the job txn * Records any exceptions as needed @@ -894,7 +890,6 @@ void nr_laravel_queue_enable(TSRMLS_D) { * So instead, what we'll do is to keep recording, but ensure that we ignore * the transaction before and after * Illuminate\\Queue\\Worker::process - * Illuminate\\Queue\\SyncQueue::executeJob/push * and we'll use raiseBeforeJobEvent to ensure we have the most up to date Job * info. This ensures that we instrument the entirety of the job (including * any handle/failed functions and also exceptions). @@ -915,8 +910,6 @@ void nr_laravel_queue_enable(TSRMLS_D) { * you'd have to modify one of the checks in end_func to account for the fact * that a root segment is okay to encounter if it has a wraprec. * - * 2. Sync doesn't have all the resolved queue info at the beginning (or end) - * or executeJob/push. It creates a temporary job that it uses internally. * * Why not just use the raiseAfterEventJob listener to end the txn? * It's not called all the time. For instance, it is not called in the case @@ -933,15 +926,6 @@ void nr_laravel_queue_enable(TSRMLS_D) { * 3) restarting the txn instrumentation going */ /*Laravel 11+*/ - nr_php_wrap_user_function_before_after_clean( - NR_PSTR("Illuminate\\Queue\\SyncQueue::executeJob"), - nr_laravel_queue_worker_before, nr_laravel_queue_worker_after, - nr_laravel_queue_worker_after); - /* Laravel below 11*/ - nr_php_wrap_user_function_before_after_clean( - NR_PSTR("Illuminate\\Queue\\SyncQueue::push"), - nr_laravel_queue_worker_before, nr_laravel_queue_worker_after, - nr_laravel_queue_worker_after); nr_php_wrap_user_function_before_after_clean( NR_PSTR("Illuminate\\Queue\\Worker::process"), nr_laravel_queue_worker_before, nr_laravel_queue_worker_after, @@ -951,9 +935,6 @@ void nr_laravel_queue_enable(TSRMLS_D) { nr_php_wrap_user_function_before_after_clean( NR_PSTR("Illuminate\\Queue\\Worker::raiseBeforeJobEvent"), nr_laravel_queue_worker_raiseBeforeJobEvent_before, NULL, NULL); - nr_php_wrap_user_function_before_after_clean( - NR_PSTR("Illuminate\\Queue\\SyncQueue::raiseBeforeJobEvent"), - nr_laravel_queue_worker_raiseBeforeJobEvent_before, NULL, NULL); #else diff --git a/agent/tests/test_fw_laravel_queue.c b/agent/tests/test_fw_laravel_queue.c index 2b33f3a14..922dfe7db 100644 --- a/agent/tests/test_fw_laravel_queue.c +++ b/agent/tests/test_fw_laravel_queue.c @@ -36,12 +36,17 @@ static void setup_classes() { const char* queue_classes = "namespace Illuminate\\Queue;" "class SyncQueue{" - "function trycatchExecuteJob() { try { $this->executeJob(); } catch (\\Exception $e) { } }" - "function executeJob() { throw new \\Exception('oops'); }" "function raiseBeforeJobEvent($job) { return; }" "}" "class Worker{" - "function process() { return; }" + "function executeJob() { throw new \\Exception('oops'); }" + "function process($flag = false) {" + "if ($flag) {" + " try { $this->executeJob(); } catch (\\Exception $e) { } " + "} else {" + "return;" + "}" + "}" "function raiseBeforeJobEvent(string $connectionName, $job) { return; }" "}"; // clang-format on @@ -51,10 +56,9 @@ static void setup_classes() { static void test_job_txn_naming_wrappers(TSRMLS_D) { /* - * Test the wrappers that name the job txn: + * Test the wrapper that names the job txn: * Illuminate\Queue\Worker::raiseBeforeJobEvent(connectionName, job) - * Illuminate\Queue\SyncQueue::raiseBeforeJobEvent(job) - * These wrappers should correctly name the transaction with the format: + * This wrapper should correctly name the transaction with the format: * " (:)" */ @@ -185,55 +189,6 @@ static void test_job_txn_naming_wrappers(TSRMLS_D) { "JobName (ConnectionName:QueueName)", NRTXN(path)); nr_php_zval_free(&expr); nr_php_zval_free(&job_obj); - - /* - * Create the mocked Illuminate\Queue\SyncQueue queue worker obj to trigger - * the wrappers that we tested with Worker::raiseBeforeJobEvent, we only to do - * basic tests of all null, all emptystring, and all properly set. - */ - nr_php_zval_free(&worker_obj); - worker_obj = tlib_php_request_eval_expr("new Illuminate\\Queue\\SyncQueue"); - tlib_pass_if_not_null("Mocked worker object shouldn't be NULL", worker_obj); - - /* Get class with all values set to NULL.*/ - job_obj = tlib_php_request_eval_expr("new my_job"); - tlib_pass_if_not_null("Mocked job object shouldn't be NULL", job_obj); - /* trigger raiseBeforeJobEvent to name the txn*/ - expr = nr_php_call(worker_obj, "raiseBeforeJobEvent", job_obj); - tlib_pass_if_not_null("Expression should evaluate.", expr); - tlib_pass_if_not_null("Txn name should not be null", NRTXN(path)); - tlib_pass_if_str_equal("Txn name should be changed", - "unknown (unknown:default)", NRTXN(path)); - nr_php_zval_free(&expr); - nr_php_zval_free(&job_obj); - - /* Get class with all set to empty string. */ - job_obj = tlib_php_request_eval_expr( - "new my_job(job_name:'', connection_name:'', queue_name:'')"); - tlib_pass_if_not_null("Mocked job object shouldn't be NULL", job_obj); - /* trigger raiseBeforeJobEvent to name the txn*/ - expr = nr_php_call(worker_obj, "raiseBeforeJobEvent", job_obj); - tlib_pass_if_not_null("Expression should evaluate.", expr); - tlib_pass_if_not_null("Txn name should not be null", NRTXN(path)); - tlib_pass_if_str_equal("Txn name should be changed", - "unknown (unknown:default)", NRTXN(path)); - nr_php_zval_free(&expr); - nr_php_zval_free(&job_obj); - - /* Get class all set.*/ - job_obj = tlib_php_request_eval_expr( - "new my_job(job_name:'JobName', connection_name:'ConnectionName', " - "queue_name:'QueueName')"); - tlib_pass_if_not_null("Mocked job object shouldn't be NULL", job_obj); - /* trigger raiseBeforeJobEvent to name the txn*/ - expr = nr_php_call(worker_obj, "raiseBeforeJobEvent", job_obj); - tlib_pass_if_not_null("Expression should evaluate.", expr); - tlib_pass_if_not_null("Txn name should not be null", NRTXN(path)); - tlib_pass_if_str_equal("Txn name should be changed", - "JobName (ConnectionName:QueueName)", NRTXN(path)); - nr_php_zval_free(&expr); - nr_php_zval_free(&job_obj); - nr_php_zval_free(&worker_obj); nr_php_zval_free(&arg_unused); tlib_php_request_end(); @@ -241,15 +196,15 @@ static void test_job_txn_naming_wrappers(TSRMLS_D) { static void test_job_txn_startstop_wrappers(TSRMLS_D) { /* - * Test the wrappers that start and end the job txn: + * Test the wrapper that starts and ends the job txn: * Illuminate\Queue\Worker::process - * Illuminate\Queue\SyncQueue::executeJob - * These wrappers should correctly end the current txn and start a new one + * This wrapper should correctly end the current txn and start a new one * in the before wrapper and end/start again in the after/clean */ zval* expr = NULL; zval* obj = NULL; + zval* arg = NULL; nrtime_t txn_time = 0; nrtime_t new_txn_time = 0; tlib_php_engine_create(""); @@ -266,9 +221,7 @@ static void test_job_txn_startstop_wrappers(TSRMLS_D) { * segment in func_begin. Since nr_laravel_queue_worker_before is destroying * the old txn and discarding all segments, ensure the wraprec is preserved on * a segment for "after" wrappers that could be called in func_end. - * Illuminate\\Queue\\SyncQueue::executeJob and - * Illuminate\\Queue\\Worker::process both resolve to the same wrapper - * callback. We'll use the mocked process to show the happy path, and we'll + * We'll use the mocked process to show the happy path, and we'll * use execute job to show the exception path. */ @@ -320,12 +273,13 @@ static void test_job_txn_startstop_wrappers(TSRMLS_D) { NR_OK_TO_OVERWRITE); tlib_pass_if_str_equal("Path should exist", "Farewell", NRTXN(path)); - /* Create the mocked Worker and call trycatchExecuteJob, which calls executeJob*/ - obj = tlib_php_request_eval_expr("new Illuminate\\Queue\\SyncQueue"); + /* Create the mocked Worker and call process, which calls executeJob*/ + obj = tlib_php_request_eval_expr("new Illuminate\\Queue\\Worker"); tlib_pass_if_not_null("object shouldn't be NULL", obj); /* We're doing the trycatch because otherwise our fragile testing system can't * handle an uncaught exception.*/ - expr = nr_php_call(obj, "trycatchExecuteJob"); + arg = tlib_php_request_eval_expr("true"); + expr = nr_php_call(obj, "process", arg); tlib_pass_if_not_null("Expression should evaluate.", expr); new_txn_time = nr_txn_start_time(NRPRG(txn)); tlib_pass_if_not_null( @@ -341,6 +295,7 @@ static void test_job_txn_startstop_wrappers(TSRMLS_D) { tlib_pass_if_null("Txn name should be NULL", NRTXN(path)); nr_php_zval_free(&expr); nr_php_zval_free(&obj); + nr_php_zval_free(&arg); tlib_php_request_end(); tlib_php_engine_destroy(); }