From 05190d5c418d070e26e63b1b3c1bd1473b57a9d2 Mon Sep 17 00:00:00 2001 From: Amber Sistla Date: Sun, 14 Sep 2025 16:37:28 -0700 Subject: [PATCH 01/14] chore: remove leftover in progress comment --- agent/fw_laravel_queue.c | 75 +++++++++++++++++++++++----------------- agent/lib_php_amqplib.c | 2 -- 2 files changed, 43 insertions(+), 34 deletions(-) diff --git a/agent/fw_laravel_queue.c b/agent/fw_laravel_queue.c index e79451ead..ec9bfd133 100644 --- a/agent/fw_laravel_queue.c +++ b/agent/fw_laravel_queue.c @@ -233,10 +233,19 @@ static char* nr_laravel_queue_job_txn_name(zval* job TSRMLS_DC) { } /* - * Handle: - * Illuminate\\Queue\\SyncQueue::raiseBeforeJobEvent(Job $job):void + * Handle: Illuminate\Queue\SyncQueue::executeJob + * /** + * Execute a given job synchronously. + * + * @param string $job + * @param mixed $data + * @param string|null $queue + * @return int + * + * @throws \Throwable + protected function executeJob($job, $data = '', $queue = null) */ -NR_PHP_WRAPPER(nr_laravel_queue_syncqueue_raiseBeforeJobEvent_before) { +NR_PHP_WRAPPER(nr_laravel_queue_syncqueue_executeJob_before) { zval* job = NULL; NR_UNUSED_SPECIALFN; @@ -283,10 +292,15 @@ NR_PHP_WRAPPER_END /* * Handle: - * Illuminate\\Queue\\Worker::raiseBeforeJobEvent(string $connectionName, Job - * $job):void + * Illuminate\\Queue\\Worker::process + * @param string $connectionName + * @param \Illuminate\Contracts\Queue\Job $job + * @param \Illuminate\Queue\WorkerOptions $options + * @return void + * + * @throws \Throwable */ -NR_PHP_WRAPPER(nr_laravel_queue_worker_raiseBeforeJobEvent_after) { +NR_PHP_WRAPPER(nr_laravel_queue_worker_process_before) { zval* job = NULL; NR_UNUSED_SPECIALFN; @@ -300,7 +314,7 @@ NR_PHP_WRAPPER(nr_laravel_queue_worker_raiseBeforeJobEvent_after) { nr_php_txn_end(1, 0 TSRMLS_CC); /* - * Laravel 7 and later passes Job as the second parameter. + * Job is the second parameter. */ char* txn_name = NULL; @@ -330,27 +344,22 @@ NR_PHP_WRAPPER(nr_laravel_queue_worker_raiseBeforeJobEvent_after) { NR_PHP_WRAPPER_END /* - * Handle: - * Illuminate\\Queue\\Worker::raiseAfterJobEvent(string $connectionName, Job - * $job):void Illuminate\\Queue\\SyncQueue::raiseAfterJobEvent(Job $job):void + * Handles: + * Illuminate\\Queue\\Worker::process + * Illuminate\\Queue\\SyncQueue::executeJob */ -NR_PHP_WRAPPER(nr_laravel_queue_worker_raiseAfterJobEvent_before) { + +NR_PHP_WRAPPER(nr_laravel_queue_worker_after) { NR_UNUSED_SPECIALFN; (void)wraprec; NR_PHP_WRAPPER_REQUIRE_FRAMEWORK(NR_FW_LARAVEL); - /* - * If we made it here, we are assured there are no uncaught exceptions (as it - * would be noticed with the oapi exception handling before calling this - * callback so no need to check before ending the txn. - */ - /* * End the real transaction and then start a new transaction so our * instrumentation continues to fire, knowing that we'll ignore that - * transaction either when Worker::process() is called again or when - * WorkCommand::handle() exits. + * transaction either when Illuminate\\Queue\\Worker::process or + * Illuminate\\Queue\\SyncQueue::executeJob is called again */ nr_php_txn_end(0, 0 TSRMLS_CC); nr_php_txn_begin(NULL, NULL TSRMLS_CC); @@ -854,23 +863,24 @@ void nr_laravel_queue_enable(TSRMLS_D) { * that is executed, but don't want to record a transaction for the actual * queue:work command, since it spends most of its time sleeping. * - * We use the raiseBeforeJobEvent and raiseAfterJobEvent listeners which we + * We use the process and executeJob functions which we * can use to name the Laravel Job and capture the true time that the job * took. */ + /* + * Wrap: + * Illuminate\\Queue\\Worker::process + * Illuminate\\Queue\\SyncQueue::executeJob + */ nr_php_wrap_user_function_before_after_clean( - NR_PSTR("Illuminate\\Queue\\Worker::raiseBeforeJobEvent"), NULL, - nr_laravel_queue_worker_raiseBeforeJobEvent_after, NULL); - nr_php_wrap_user_function_before_after_clean( - NR_PSTR("Illuminate\\Queue\\Worker::raiseAfterJobEvent"), - nr_laravel_queue_worker_raiseAfterJobEvent_before, NULL, NULL); - nr_php_wrap_user_function_before_after_clean( - NR_PSTR("Illuminate\\Queue\\SyncQueue::raiseBeforeJobEvent"), - nr_laravel_queue_syncqueue_raiseBeforeJobEvent_before, NULL, NULL); + NR_PSTR("Illuminate\\Queue\\SyncQueue::executeJob"), + nr_laravel_queue_syncqueue_executeJob_before, + nr_laravel_queue_worker_after, nr_laravel_queue_worker_after); nr_php_wrap_user_function_before_after_clean( - NR_PSTR("Illuminate\\Queue\\SyncQueue::raiseAfterJobEvent"), - nr_laravel_queue_worker_raiseAfterJobEvent_before, NULL, NULL); + NR_PSTR("Illuminate\\Queue\\Worker::process"), + nr_laravel_queue_worker_process_before, nr_laravel_queue_worker_after, + nr_laravel_queue_worker_after); #else @@ -883,8 +893,9 @@ void nr_laravel_queue_enable(TSRMLS_D) { * aren't executed if we're not actually in a transaction. * * So instead, what we'll do is to keep recording, but ensure that we ignore - * the transaction after WorkCommand::handle() has finished executing, at - * which point no more jobs can be run. + * the transaction before and after + * Illuminate\\Queue\\Worker::process + * Illuminate\\Queue\\SyncQueue::executeJob */ nr_php_wrap_user_function( diff --git a/agent/lib_php_amqplib.c b/agent/lib_php_amqplib.c index 4750c94bd..121445e6e 100644 --- a/agent/lib_php_amqplib.c +++ b/agent/lib_php_amqplib.c @@ -768,8 +768,6 @@ NR_PHP_WRAPPER(nr_rabbitmq_basic_get) { * want to strdup everything if we don't have to. RabbitMQ basic_get PHP 7.x * will only strdup server_address and destination_name. */ - // amber make these peristent for all since retval of null clears the values - // from the cxn UNDO_PERSISTENCE(message_params.server_address); UNDO_PERSISTENCE(message_params.destination_name); } From 92263282acc00cbd7ec1249365c7d737966114e8 Mon Sep 17 00:00:00 2001 From: Amber Sistla Date: Sun, 14 Sep 2025 16:56:10 -0700 Subject: [PATCH 02/14] fix(agent): allow wraprecs to be propagated when manually ending/starting txn within agent --- agent/fw_laravel_queue.c | 46 +++++++++++++++++++++++++++++----------- agent/php_execute.c | 15 ++++++++++--- 2 files changed, 46 insertions(+), 15 deletions(-) diff --git a/agent/fw_laravel_queue.c b/agent/fw_laravel_queue.c index ec9bfd133..4111df7f7 100644 --- a/agent/fw_laravel_queue.c +++ b/agent/fw_laravel_queue.c @@ -247,9 +247,9 @@ static char* nr_laravel_queue_job_txn_name(zval* job TSRMLS_DC) { */ NR_PHP_WRAPPER(nr_laravel_queue_syncqueue_executeJob_before) { zval* job = NULL; + nr_segment_t* segment = NULL; NR_UNUSED_SPECIALFN; - (void)wraprec; NR_PHP_WRAPPER_REQUIRE_FRAMEWORK(NR_FW_LARAVEL); @@ -275,6 +275,16 @@ NR_PHP_WRAPPER(nr_laravel_queue_syncqueue_executeJob_before) { if (NR_SUCCESS == nr_php_txn_begin(NULL, NULL)) { nr_txn_set_as_background_job(NRPRG(txn), "Laravel job"); + segment = nr_txn_get_current_segment(NRPRG(txn), NULL); + /* + * Transfer the old wraprec to the newly created segment inside the brand + * new txn so that the agent will be able to call any _after or _clean + * callbacks. + */ + if (NULL != segment) { + segment->wraprec = wraprec; + } + if (NULL == txn_name) { txn_name = nr_strdup("unknown"); } @@ -302,9 +312,9 @@ NR_PHP_WRAPPER_END */ NR_PHP_WRAPPER(nr_laravel_queue_worker_process_before) { zval* job = NULL; + nr_segment_t* segment = NULL; NR_UNUSED_SPECIALFN; - (void)wraprec; NR_PHP_WRAPPER_REQUIRE_FRAMEWORK(NR_FW_LARAVEL); @@ -312,7 +322,6 @@ NR_PHP_WRAPPER(nr_laravel_queue_worker_process_before) { * End the current txn to prepare for the Job txn. */ nr_php_txn_end(1, 0 TSRMLS_CC); - /* * Job is the second parameter. */ @@ -327,7 +336,15 @@ NR_PHP_WRAPPER(nr_laravel_queue_worker_process_before) { if (NR_SUCCESS == nr_php_txn_begin(NULL, NULL)) { nr_txn_set_as_background_job(NRPRG(txn), "Laravel job"); - + segment = nr_txn_get_current_segment(NRPRG(txn), NULL); + /* + * Transfer the old wraprec to the newly created segment inside the brand + * new txn so that the agent will be able to call any _after or _clean + * callbacks. + */ + if (NULL != segment) { + segment->wraprec = wraprec; + } if (NULL == txn_name) { txn_name = nr_strdup("unknown"); } @@ -337,6 +354,7 @@ NR_PHP_WRAPPER(nr_laravel_queue_worker_process_before) { nr_txn_set_path("Laravel", NRPRG(txn), txn_name, NR_PATH_TYPE_CUSTOM, NR_OK_TO_OVERWRITE); } + nr_free(txn_name); nr_php_arg_release(&job); NR_PHP_WRAPPER_CALL; @@ -352,7 +370,6 @@ NR_PHP_WRAPPER_END NR_PHP_WRAPPER(nr_laravel_queue_worker_after) { NR_UNUSED_SPECIALFN; (void)wraprec; - NR_PHP_WRAPPER_REQUIRE_FRAMEWORK(NR_FW_LARAVEL); /* @@ -861,11 +878,17 @@ void nr_laravel_queue_enable(TSRMLS_D) { /* * Here's the problem: we want to record individual transactions for each job * that is executed, but don't want to record a transaction for the actual - * queue:work command, since it spends most of its time sleeping. + * queue:work command, since it spends most of its time sleeping. The naive + * approach would be to end the transaction immediately and instrument + * Worker::process(). The issue with that is that instrumentation hooks + * aren't executed if we're not actually in a transaction. * - * We use the process and executeJob functions which we - * can use to name the Laravel Job and capture the true time that the job - * took. + * 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 + * This ensures that we instrument the entirety of the job (including any + * handle/failed functions) */ /* @@ -893,9 +916,8 @@ void nr_laravel_queue_enable(TSRMLS_D) { * aren't executed if we're not actually in a transaction. * * 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 + * the transaction after WorkCommand::handle() has finished executing, at + * which point no more jobs can be run. */ nr_php_wrap_user_function( diff --git a/agent/php_execute.c b/agent/php_execute.c index c654402d5..a5b0a1065 100644 --- a/agent/php_execute.c +++ b/agent/php_execute.c @@ -1936,11 +1936,20 @@ static void nr_php_instrument_func_end(NR_EXECUTE_PROTO) { return; } if (nrunlikely(NRPRG(txn)->segment_root == segment)) { - /* +/* * There should be no fcall_end associated with the segment root, If we are - * here, it is most likely due to an API call to newrelic_end_transaction + * here, it is most likely due to an API call to newrelic_end_transaction. + * However, there's a special case here, if we call nr_php_txn_end + * and then call nr_php_txn_start from within a wrapped call inside the agent, + * then there will be an fcall associated with the segment root. + * We may or may not need to do anything with this so we check if a wraprec exists. + * If a wraprec exists, we proceed so we can use the _after and _clean callbacks + * if they exist. If it doesn't exist, we exit as nothing needs to be done. */ - return; + + if (NULL == segment->wraprec) { + return; + } } wraprec = segment->wraprec; From a274bbecf4a5bea21b263864c2fd0692d7548d43 Mon Sep 17 00:00:00 2001 From: Amber Sistla Date: Mon, 15 Sep 2025 19:29:15 -0700 Subject: [PATCH 03/14] fix: queue handling --- agent/fw_laravel_queue.c | 285 +++++++++++++++++++++++++-------------- 1 file changed, 185 insertions(+), 100 deletions(-) diff --git a/agent/fw_laravel_queue.c b/agent/fw_laravel_queue.c index 4111df7f7..d84f9878e 100644 --- a/agent/fw_laravel_queue.c +++ b/agent/fw_laravel_queue.c @@ -233,85 +233,84 @@ static char* nr_laravel_queue_job_txn_name(zval* job TSRMLS_DC) { } /* - * Handle: Illuminate\Queue\SyncQueue::executeJob - * /** - * Execute a given job synchronously. - * - * @param string $job - * @param mixed $data - * @param string|null $queue - * @return int - * - * @throws \Throwable - protected function executeJob($job, $data = '', $queue = null) + * Handle: + * Illuminate\\Queue\\Worker::raiseBeforeJobEvent + * Raise the before queue job event. + * + * @param string $connectionName + * @param \Illuminate\Contracts\Queue\Job $job + * @return void + * protected function raiseBeforeJobEvent($connectionName, $job) + * + * Illuminate\\Queue\\SyncQueue::raiseBeforeJobEvent + * Raise the before queue job event. + * + * @param \Illuminate\Contracts\Queue\Job $job + * @return void + * protected function raiseBeforeJobEvent(Job $job) + * + * The reason these functions are used for txn naming: + * 1. It has been a consistent API called directly before the Job across Laravel + * going back to at leaset 6 + * 2. It allows us to have a reusable function callback for both sync/async. + * 3. Sync jobs don't use the job that is passed in, they create a brand new job + * based off of the passed in job that then then attempt to run. */ -NR_PHP_WRAPPER(nr_laravel_queue_syncqueue_executeJob_before) { +NR_PHP_WRAPPER(nr_laravel_queue_worker_raiseBeforeJobEvent_before) { zval* job = NULL; nr_segment_t* segment = NULL; + char* txn_name = NULL; + size_t argc = 0; + int job_param_num = 2; /* Most likely case of async job. */ NR_UNUSED_SPECIALFN; NR_PHP_WRAPPER_REQUIRE_FRAMEWORK(NR_FW_LARAVEL); - /* - * End the current txn in preparation for the Job txn. + /* For raiseBeforeJobEvent: + * For Async jobs, Job is the second parameter. + * For Sync jobs, Job is the first and only parameter. */ - nr_php_txn_end(1, 0); - - /* - * Laravel 7+ passes Job as the first parameter. - */ - char* txn_name = NULL; - - job = nr_php_arg_get(1, NR_EXECUTE_ORIG_ARGS); + argc = nr_php_get_user_func_arg_count(NR_EXECUTE_ORIG_ARGS); + if (1 == argc) { + /* This is a sync job*/ + job_param_num = 1; + } - /* txn_name needs to be freed by the caller. */ + job = nr_php_arg_get(job_param_num, NR_EXECUTE_ORIG_ARGS); txn_name = nr_laravel_queue_job_txn_name(job); - /* - * Begin the transaction we'll actually record. - */ - - if (NR_SUCCESS == nr_php_txn_begin(NULL, NULL)) { - nr_txn_set_as_background_job(NRPRG(txn), "Laravel job"); + if (NULL == txn_name) { + txn_name = nr_strdup("unknown"); + } - segment = nr_txn_get_current_segment(NRPRG(txn), NULL); - /* - * Transfer the old wraprec to the newly created segment inside the brand - * new txn so that the agent will be able to call any _after or _clean - * callbacks. - */ - if (NULL != segment) { - segment->wraprec = wraprec; - } + segment = NRPRG(txn)->segment_root; + if (NULL != segment) { + nr_segment_set_name(segment, txn_name); + } - if (NULL == txn_name) { - txn_name = nr_strdup("unknown"); - } + nr_laravel_queue_set_cat_txn(job TSRMLS_CC); - nr_laravel_queue_set_cat_txn(job TSRMLS_CC); + nr_txn_set_path("Laravel", NRPRG(txn), txn_name, NR_PATH_TYPE_CUSTOM, + NR_OK_TO_OVERWRITE); - nr_txn_set_path("Laravel", NRPRG(txn), txn_name, NR_PATH_TYPE_CUSTOM, - NR_OK_TO_OVERWRITE); - } - nr_php_arg_release(&job); nr_free(txn_name); + nr_php_arg_release(&job); NR_PHP_WRAPPER_CALL; } NR_PHP_WRAPPER_END /* - * Handle: + * Handles: * Illuminate\\Queue\\Worker::process - * @param string $connectionName - * @param \Illuminate\Contracts\Queue\Job $job - * @param \Illuminate\Queue\WorkerOptions $options - * @return void + * Illuminate\\Queue\\SyncQueue::executeJob (laravel 11+) + * Illuminate\\Queue\\SyncQueue::push (before laravel 11) * - * @throws \Throwable + * Ends/discards the unneeded worker txn + * Starts the job txn */ -NR_PHP_WRAPPER(nr_laravel_queue_worker_process_before) { - zval* job = NULL; + +NR_PHP_WRAPPER(nr_laravel_queue_worker_before) { nr_segment_t* segment = NULL; NR_UNUSED_SPECIALFN; @@ -319,16 +318,9 @@ NR_PHP_WRAPPER(nr_laravel_queue_worker_process_before) { NR_PHP_WRAPPER_REQUIRE_FRAMEWORK(NR_FW_LARAVEL); /* - * End the current txn to prepare for the Job txn. + * End and discard the current txn to prepare for the Job txn. */ nr_php_txn_end(1, 0 TSRMLS_CC); - /* - * Job is the second parameter. - */ - char* txn_name = NULL; - - job = nr_php_arg_get(2, NR_EXECUTE_ORIG_ARGS); - txn_name = nr_laravel_queue_job_txn_name(job); /* * Begin the transaction we'll actually record. @@ -336,27 +328,12 @@ NR_PHP_WRAPPER(nr_laravel_queue_worker_process_before) { if (NR_SUCCESS == nr_php_txn_begin(NULL, NULL)) { nr_txn_set_as_background_job(NRPRG(txn), "Laravel job"); + segment = nr_txn_get_current_segment(NRPRG(txn), NULL); - /* - * Transfer the old wraprec to the newly created segment inside the brand - * new txn so that the agent will be able to call any _after or _clean - * callbacks. - */ if (NULL != segment) { segment->wraprec = wraprec; } - if (NULL == txn_name) { - txn_name = nr_strdup("unknown"); - } - - nr_laravel_queue_set_cat_txn(job TSRMLS_CC); - - nr_txn_set_path("Laravel", NRPRG(txn), txn_name, NR_PATH_TYPE_CUSTOM, - NR_OK_TO_OVERWRITE); } - - nr_free(txn_name); - nr_php_arg_release(&job); NR_PHP_WRAPPER_CALL; } NR_PHP_WRAPPER_END @@ -364,12 +341,17 @@ NR_PHP_WRAPPER_END /* * Handles: * Illuminate\\Queue\\Worker::process - * Illuminate\\Queue\\SyncQueue::executeJob + * Illuminate\\Queue\\SyncQueue::executeJob (laravel 11+) + * Illuminate\\Queue\\SyncQueue::push (before laravel 11) + * + * Closes the job txn + * Records any exceptions as needed + * Restarts the txn instrumentation */ NR_PHP_WRAPPER(nr_laravel_queue_worker_after) { NR_UNUSED_SPECIALFN; - (void)wraprec; + nr_php_execute_metadata_t metadata; NR_PHP_WRAPPER_REQUIRE_FRAMEWORK(NR_FW_LARAVEL); /* @@ -378,6 +360,53 @@ NR_PHP_WRAPPER(nr_laravel_queue_worker_after) { * transaction either when Illuminate\\Queue\\Worker::process or * Illuminate\\Queue\\SyncQueue::executeJob is called again */ + + if (NULL != auto_segment->error) { + /* An exception occurred we need to record it on the txn if it isn't already + * there. */ + if (NULL == NRPRG(txn)->error) { + /* Since we are ending this txn within the wrapper, and we know it has an + error, apply it to the txn; otherwise, the + way it is handled in Laravel means the exception is caught by an internal + job exceptions handler and then thrown again AFTER we've already ended the + txn for the job + * + */ + zval exception; + ZVAL_OBJ(&exception, EG(exception)); + nr_status_t st; + st = nr_php_error_record_exception( + NRPRG(txn), &exception, 50, false /* add to segment */, + NULL /* use default prefix */, &NRPRG(exception_filters)); + + if (NR_FAILURE == st) { + nrl_verbosedebug(NRL_FRAMEWORK, "%s: unable to record exception", + __func__); + } + } + } + + nr_php_txn_end(0, 0 TSRMLS_CC); + nr_php_txn_begin(NULL, NULL TSRMLS_CC); +} +NR_PHP_WRAPPER_END + +/* + * Handles: + * Illuminate\\Queue\\Worker::process + * Illuminate\\Queue\\SyncQueue::executeJob + */ + +NR_PHP_WRAPPER(nr_laravel_queue_worker_stop_after) { + NR_UNUSED_SPECIALFN; + (void)wraprec; + NR_PHP_WRAPPER_REQUIRE_FRAMEWORK(NR_FW_LARAVEL); + /* + * Anytime the worker is considering whether to stop or not (or actually + * stopping), we can ignore everything that's happened previously (sleeping + * etc) that is non job processing related and avoid a useless txn. + */ + nr_php_txn_end(0, 0 TSRMLS_CC); nr_php_txn_begin(NULL, NULL TSRMLS_CC); } @@ -676,12 +705,11 @@ NR_PHP_WRAPPER(nr_laravel_queue_worker_process) { nr_php_arg_release(&job); /* - * End the real transaction and then start a new transaction so our + * End the useless transaction and then start a new transaction so our * instrumentation continues to fire, knowing that we'll ignore that - * transaction either when Worker::process() is called again or when - * WorkCommand::handle() exits. + * transaction if the worker continues when Worker::process() is called again. */ - nr_php_txn_end(0, 0 TSRMLS_CC); + nr_php_txn_end(1, 0 TSRMLS_CC); nr_php_txn_begin(NULL, NULL TSRMLS_CC); } NR_PHP_WRAPPER_END @@ -817,9 +845,9 @@ NR_PHP_WRAPPER(nr_laravel_queue_queue_createpayload) { } /* - * The payload should be a JSON string: in essence, we want to decode it, add - * our attributes, and then re-encode it. Unfortunately, the payload will - * include NULL bytes for closures, and this causes nro to choke badly + * The payload should be a JSON string: in essence, we want to decode it, + * add our attributes, and then re-encode it. Unfortunately, the payload + * will include NULL bytes for closures, and this causes nro to choke badly * because it can't handle NULLs in strings, so we'll call back into PHP's * own JSON functions. */ @@ -876,35 +904,92 @@ void nr_laravel_queue_enable(TSRMLS_D) { && !defined OVERWRITE_ZEND_EXECUTE_DATA /* - * Here's the problem: we want to record individual transactions for each job - * that is executed, but don't want to record a transaction for the actual - * queue:work command, since it spends most of its time sleeping. The naive - * approach would be to end the transaction immediately and instrument + * Here's the problem: we want to record individual transactions for each + * job that is executed, but don't want to record a transaction for the + * actual queue:work command, since it spends most of its time sleeping. The + * naive approach would be to end the transaction immediately and instrument * Worker::process(). The issue with that is that instrumentation hooks * aren't executed if we're not actually in a transaction. * * 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 - * This ensures that we instrument the entirety of the job (including any - * handle/failed functions) + * 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). + * + * + * Why so many? + * 1. The main reason is because of the trickiness when starting/stopping txns + * within an OAPI wrapped func. OAPI handling assumes a segment is created + * with w/associated wraprecs in func_begin. However, when we end/discard the + * txn, we discard all those previous segments, so any functions that had + * wrapped callbacks will all go away after a txn end. We have only one + * opportunity to preserve any wraprecs and that is when we stop/start in a + * wrapped func that has both a before and an after/clean. Because we are + * starting/ending txns within the wrapper, it requires much more handling for + * OAPI compatibility. Namely, you would need to transfer the old wraprec + * from the old segment to the newly created segment inside the brand new txn + * so that the agent will be able to call any _after or _clean callbacks AND + * 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 + * of exceptions. */ /* - * Wrap: - * Illuminate\\Queue\\Worker::process - * Illuminate\\Queue\\SyncQueue::executeJob + * The before callbacks will handle: + * 1) ending the unneeded worker txn + * 2) starting + * the after/clean callbacks will handle: + * 1) ending the job txn + * 2) handling any exception + * 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_syncqueue_executeJob_before, - nr_laravel_queue_worker_after, nr_laravel_queue_worker_after); + 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_process_before, nr_laravel_queue_worker_after, + nr_laravel_queue_worker_before, nr_laravel_queue_worker_after, nr_laravel_queue_worker_after); + /* These wrappers will handle naming the job txn*/ + 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); + + /* + * Additional cleanup so we don't help prevent a useless excess txn after the + * worker ends. Anytime the worker is questioning whether it should stop or is + * going to stop, we can end the txn. + */ + nr_php_wrap_user_function_before_after_clean( + NR_PSTR("Illuminate\\Queue\\Worker::stopIfNecessary"), NULL, + nr_laravel_queue_worker_stop_after, NULL); + nr_php_wrap_user_function_before_after_clean( + NR_PSTR("Illuminate\\Queue\\Worker::stopWorkerIfLostConnection"), NULL, + nr_laravel_queue_worker_stop_after, NULL); + nr_php_wrap_user_function_before_after_clean( + NR_PSTR("Illuminate\\Queue\\Worker::stop"), NULL, + nr_laravel_queue_worker_stop_after, NULL); + #else /* From 437141ab234754e616cad44714b8790ecd7c42cf Mon Sep 17 00:00:00 2001 From: Amber Sistla Date: Mon, 15 Sep 2025 20:52:51 -0700 Subject: [PATCH 04/14] try: remove the stop funcs --- agent/fw_laravel_queue.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/agent/fw_laravel_queue.c b/agent/fw_laravel_queue.c index d84f9878e..319e927a0 100644 --- a/agent/fw_laravel_queue.c +++ b/agent/fw_laravel_queue.c @@ -980,15 +980,17 @@ void nr_laravel_queue_enable(TSRMLS_D) { * worker ends. Anytime the worker is questioning whether it should stop or is * going to stop, we can end the txn. */ - nr_php_wrap_user_function_before_after_clean( - NR_PSTR("Illuminate\\Queue\\Worker::stopIfNecessary"), NULL, - nr_laravel_queue_worker_stop_after, NULL); - nr_php_wrap_user_function_before_after_clean( - NR_PSTR("Illuminate\\Queue\\Worker::stopWorkerIfLostConnection"), NULL, - nr_laravel_queue_worker_stop_after, NULL); - nr_php_wrap_user_function_before_after_clean( - NR_PSTR("Illuminate\\Queue\\Worker::stop"), NULL, - nr_laravel_queue_worker_stop_after, NULL); + if (0) { + nr_php_wrap_user_function_before_after_clean( + NR_PSTR("Illuminate\\Queue\\Worker::stopIfNecessary"), NULL, + nr_laravel_queue_worker_stop_after, NULL); + nr_php_wrap_user_function_before_after_clean( + NR_PSTR("Illuminate\\Queue\\Worker::stopWorkerIfLostConnection"), NULL, + nr_laravel_queue_worker_stop_after, NULL); + nr_php_wrap_user_function_before_after_clean( + NR_PSTR("Illuminate\\Queue\\Worker::stop"), NULL, + nr_laravel_queue_worker_stop_after, NULL); + } #else From 2357c1cc339dee67b9fb6477e8d7e74b97a83eb6 Mon Sep 17 00:00:00 2001 From: Amber Sistla Date: Tue, 16 Sep 2025 05:01:26 -0700 Subject: [PATCH 05/14] fix: give basic name to queue process throwaway txn In case the queue process txn that we keep open so we caninstrument actually becomes a real txn, name it. --- agent/fw_laravel_queue.c | 52 ++++++++-------------------------------- 1 file changed, 10 insertions(+), 42 deletions(-) diff --git a/agent/fw_laravel_queue.c b/agent/fw_laravel_queue.c index 319e927a0..7c95c54b1 100644 --- a/agent/fw_laravel_queue.c +++ b/agent/fw_laravel_queue.c @@ -361,9 +361,9 @@ NR_PHP_WRAPPER(nr_laravel_queue_worker_after) { * Illuminate\\Queue\\SyncQueue::executeJob is called again */ - if (NULL != auto_segment->error) { - /* An exception occurred we need to record it on the txn if it isn't already - * there. */ + if (NULL == nr_php_get_return_value(NR_EXECUTE_ORIG_ARGS)) { + /* An exception occurred and we need to record it on the txn if it isn't + * already there. */ if (NULL == NRPRG(txn)->error) { /* Since we are ending this txn within the wrapper, and we know it has an error, apply it to the txn; otherwise, the @@ -387,28 +387,13 @@ NR_PHP_WRAPPER(nr_laravel_queue_worker_after) { } nr_php_txn_end(0, 0 TSRMLS_CC); - nr_php_txn_begin(NULL, NULL TSRMLS_CC); -} -NR_PHP_WRAPPER_END - -/* - * Handles: - * Illuminate\\Queue\\Worker::process - * Illuminate\\Queue\\SyncQueue::executeJob - */ - -NR_PHP_WRAPPER(nr_laravel_queue_worker_stop_after) { - NR_UNUSED_SPECIALFN; - (void)wraprec; - NR_PHP_WRAPPER_REQUIRE_FRAMEWORK(NR_FW_LARAVEL); - /* - * Anytime the worker is considering whether to stop or not (or actually - * stopping), we can ignore everything that's happened previously (sleeping - * etc) that is non job processing related and avoid a useless txn. - */ - - nr_php_txn_end(0, 0 TSRMLS_CC); - nr_php_txn_begin(NULL, NULL TSRMLS_CC); + /* Let's give it some rudimentary name so it won't be unknown in case we exit + * and this becomes a txn.*/ + if (NR_SUCCESS == nr_php_txn_begin(NULL, NULL TSRMLS_CC)) { + nr_txn_set_as_background_job(NRPRG(txn), "Laravel queue worker process"); + nr_txn_set_path("Laravel", NRPRG(txn), "Laravel queue worker process", + NR_PATH_TYPE_CUSTOM, NR_OK_TO_OVERWRITE); + } } NR_PHP_WRAPPER_END @@ -975,23 +960,6 @@ void nr_laravel_queue_enable(TSRMLS_D) { NR_PSTR("Illuminate\\Queue\\SyncQueue::raiseBeforeJobEvent"), nr_laravel_queue_worker_raiseBeforeJobEvent_before, NULL, NULL); - /* - * Additional cleanup so we don't help prevent a useless excess txn after the - * worker ends. Anytime the worker is questioning whether it should stop or is - * going to stop, we can end the txn. - */ - if (0) { - nr_php_wrap_user_function_before_after_clean( - NR_PSTR("Illuminate\\Queue\\Worker::stopIfNecessary"), NULL, - nr_laravel_queue_worker_stop_after, NULL); - nr_php_wrap_user_function_before_after_clean( - NR_PSTR("Illuminate\\Queue\\Worker::stopWorkerIfLostConnection"), NULL, - nr_laravel_queue_worker_stop_after, NULL); - nr_php_wrap_user_function_before_after_clean( - NR_PSTR("Illuminate\\Queue\\Worker::stop"), NULL, - nr_laravel_queue_worker_stop_after, NULL); - } - #else /* From 1269a02cda1f7be5d62827f851aac72946b9aa29 Mon Sep 17 00:00:00 2001 From: Amber Sistla Date: Tue, 16 Sep 2025 08:18:47 -0700 Subject: [PATCH 06/14] try: cleanup wrappers --- agent/fw_laravel_queue.c | 37 ++++++++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/agent/fw_laravel_queue.c b/agent/fw_laravel_queue.c index 7c95c54b1..cf7c5ad56 100644 --- a/agent/fw_laravel_queue.c +++ b/agent/fw_laravel_queue.c @@ -387,16 +387,25 @@ NR_PHP_WRAPPER(nr_laravel_queue_worker_after) { } nr_php_txn_end(0, 0 TSRMLS_CC); - /* Let's give it some rudimentary name so it won't be unknown in case we exit - * and this becomes a txn.*/ - if (NR_SUCCESS == nr_php_txn_begin(NULL, NULL TSRMLS_CC)) { - nr_txn_set_as_background_job(NRPRG(txn), "Laravel queue worker process"); - nr_txn_set_path("Laravel", NRPRG(txn), "Laravel queue worker process", - NR_PATH_TYPE_CUSTOM, NR_OK_TO_OVERWRITE); - } + nr_php_txn_begin(NULL, NULL TSRMLS_CC); } NR_PHP_WRAPPER_END +NR_PHP_WRAPPER(nr_laravel_queue_worker_stop_after) { + NR_UNUSED_SPECIALFN; + (void)wraprec; + NR_PHP_WRAPPER_REQUIRE_FRAMEWORK(NR_FW_LARAVEL); + /* + * Anytime the worker is considering whether to stop or not (or actually stopping), + * we can ignore everything that's happened previously (sleeping etc) that is non job processing related and avoid a useless txn. + */ + + + nr_php_txn_end(0, 0 TSRMLS_CC); + nr_php_txn_begin(NULL, NULL TSRMLS_CC); + } + NR_PHP_WRAPPER_END + #else /* * Purpose : Extract the actual job name from a job that used CallQueuedHandler @@ -960,6 +969,20 @@ void nr_laravel_queue_enable(TSRMLS_D) { NR_PSTR("Illuminate\\Queue\\SyncQueue::raiseBeforeJobEvent"), nr_laravel_queue_worker_raiseBeforeJobEvent_before, NULL, NULL); + /* + * Additional cleanup so we don't help prevent a useless excess txn after the worker ends. + * Anytime the worker is questioning whether it should stop or is going to stop, we can end the txn. + */ + nr_php_wrap_user_function_before_after_clean( + NR_PSTR("Illuminate\\Queue\\Worker::stopIfNecessary"), + NULL, nr_laravel_queue_worker_stop_after, NULL); + nr_php_wrap_user_function_before_after_clean( + NR_PSTR("Illuminate\\Queue\\Worker::stopWorkerIfLostConnection"), + NULL, nr_laravel_queue_worker_stop_after, NULL); + nr_php_wrap_user_function_before_after_clean( + NR_PSTR("Illuminate\\Queue\\Worker::stop"), + NULL, nr_laravel_queue_worker_stop_after, NULL); + #else /* From 04ec6b93b38187fe78abc8af3483b3e272d785d4 Mon Sep 17 00:00:00 2001 From: Amber Sistla Date: Wed, 17 Sep 2025 09:19:52 -0700 Subject: [PATCH 07/14] fix: remove unnecessary wraps --- agent/fw_laravel_queue.c | 32 +------------------------------- 1 file changed, 1 insertion(+), 31 deletions(-) diff --git a/agent/fw_laravel_queue.c b/agent/fw_laravel_queue.c index cf7c5ad56..7c2c72755 100644 --- a/agent/fw_laravel_queue.c +++ b/agent/fw_laravel_queue.c @@ -391,21 +391,6 @@ NR_PHP_WRAPPER(nr_laravel_queue_worker_after) { } NR_PHP_WRAPPER_END -NR_PHP_WRAPPER(nr_laravel_queue_worker_stop_after) { - NR_UNUSED_SPECIALFN; - (void)wraprec; - NR_PHP_WRAPPER_REQUIRE_FRAMEWORK(NR_FW_LARAVEL); - /* - * Anytime the worker is considering whether to stop or not (or actually stopping), - * we can ignore everything that's happened previously (sleeping etc) that is non job processing related and avoid a useless txn. - */ - - - nr_php_txn_end(0, 0 TSRMLS_CC); - nr_php_txn_begin(NULL, NULL TSRMLS_CC); - } - NR_PHP_WRAPPER_END - #else /* * Purpose : Extract the actual job name from a job that used CallQueuedHandler @@ -659,8 +644,6 @@ NR_PHP_WRAPPER(nr_laravel_queue_worker_process) { nr_free(txn_name); } - NR_PHP_WRAPPER_CALL; - /* * We need to report any uncaught exceptions now, so that they're on the * transaction we're about to end. We can see if there's an exception waiting @@ -705,6 +688,7 @@ NR_PHP_WRAPPER(nr_laravel_queue_worker_process) { */ nr_php_txn_end(1, 0 TSRMLS_CC); nr_php_txn_begin(NULL, NULL TSRMLS_CC); + NR_PHP_WRAPPER_CALL; } NR_PHP_WRAPPER_END @@ -969,20 +953,6 @@ void nr_laravel_queue_enable(TSRMLS_D) { NR_PSTR("Illuminate\\Queue\\SyncQueue::raiseBeforeJobEvent"), nr_laravel_queue_worker_raiseBeforeJobEvent_before, NULL, NULL); - /* - * Additional cleanup so we don't help prevent a useless excess txn after the worker ends. - * Anytime the worker is questioning whether it should stop or is going to stop, we can end the txn. - */ - nr_php_wrap_user_function_before_after_clean( - NR_PSTR("Illuminate\\Queue\\Worker::stopIfNecessary"), - NULL, nr_laravel_queue_worker_stop_after, NULL); - nr_php_wrap_user_function_before_after_clean( - NR_PSTR("Illuminate\\Queue\\Worker::stopWorkerIfLostConnection"), - NULL, nr_laravel_queue_worker_stop_after, NULL); - nr_php_wrap_user_function_before_after_clean( - NR_PSTR("Illuminate\\Queue\\Worker::stop"), - NULL, nr_laravel_queue_worker_stop_after, NULL); - #else /* From abd3a285d7597da108d4fd93c94563ee2b6a2df9 Mon Sep 17 00:00:00 2001 From: Amber Sistla Date: Fri, 19 Sep 2025 19:56:10 -0700 Subject: [PATCH 08/14] feat: add unit tests for laravel queue --- agent/Makefile.frag | 1 + agent/tests/test_fw_laravel_queue.c | 361 ++++++++++++++++++++++++++++ 2 files changed, 362 insertions(+) create mode 100644 agent/tests/test_fw_laravel_queue.c diff --git a/agent/Makefile.frag b/agent/Makefile.frag index 7d6a4f0d1..843aa5de9 100644 --- a/agent/Makefile.frag +++ b/agent/Makefile.frag @@ -87,6 +87,7 @@ TEST_BINARIES = \ tests/test_environment \ tests/test_fw_codeigniter \ tests/test_fw_drupal \ + tests/test_fw_laravel_queue \ tests/test_fw_support \ tests/test_fw_wordpress \ tests/test_globals \ diff --git a/agent/tests/test_fw_laravel_queue.c b/agent/tests/test_fw_laravel_queue.c new file mode 100644 index 000000000..5e318a1b2 --- /dev/null +++ b/agent/tests/test_fw_laravel_queue.c @@ -0,0 +1,361 @@ +/* + * Copyright 2024 New Relic Corporation. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +#include "tlib_php.h" + +#include "php_agent.h" +#include "php_call.h" +#include "php_wrapper.h" +#include "fw_support.h" +#include "fw_laravel_queue.h" + +tlib_parallel_info_t parallel_info + = {.suggested_nthreads = -1, .state_size = 0}; + +#if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO + +static void setup_classes() { + // clang-format off + /* Mock up the classes we'll use to test. */ + const char* job_class = + "class my_job {" + "private ?string $job_name;" + "private ?string $connection_name;" + "private ?string $queue_name;" + "function resolveName() {return $this->job_name;}" + "function getConnectionName() {return $this->connection_name;}" + "function getQueue() {return $this->queue_name;}" + "function __construct(?string $job_name = null, ?string $connection_name = null, ?string $queue_name = null) {" + "$this->job_name = $job_name;" + "$this->connection_name = $connection_name;" + "$this->queue_name = $queue_name;" + "}" + "}"; + 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 raiseBeforeJobEvent(string $connectionName, $job) { return; }" + "}"; + // clang-format on + tlib_php_request_eval(job_class); + tlib_php_request_eval(queue_classes); +} + +static void test_job_txn_naming_wrappers(TSRMLS_D) { + /* + * Test the wrappers that name the job txn: + * Illuminate\Queue\Worker::raiseBeforeJobEvent(connectionName, job) + * Illuminate\Queue\SyncQueue::raiseBeforeJobEvent(job) + * These wrappers should correctly name the transaction with the format: + * " (:)" + */ + + zval* expr = NULL; + zval* worker_obj = NULL; + zval* job_obj = NULL; + zval* arg_unused = NULL; + char* arg_unused_string = NULL; + + tlib_php_request_start(); + + setup_classes(); + + arg_unused_string = "'unused'"; + arg_unused = tlib_php_request_eval_expr(arg_unused_string); + + NRINI(force_framework) = NR_FW_LARAVEL; + nr_laravel_queue_enable(); + + tlib_pass_if_not_null("Txn should not be null at the start of the test.", + NRPRG(txn)); + nr_txn_set_path("ToBeChanged", NRPRG(txn), "Farewell", NR_PATH_TYPE_CUSTOM, + NR_OK_TO_OVERWRITE); + tlib_pass_if_str_equal("Path should exist", "Farewell", NRTXN(path)); + + /* + * Create the mocked Illuminate\Queue\Work queue worker obj to trigger the + * wrappers + */ + worker_obj = tlib_php_request_eval_expr("new Illuminate\\Queue\\Worker"); + 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", arg_unused, 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 job_name empty string. */ + job_obj = tlib_php_request_eval_expr("new my_job(job_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", arg_unused, 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 jobname set.*/ + job_obj = tlib_php_request_eval_expr("new my_job(job_name:'JobName')"); + 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", arg_unused, 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 (unknown:default)", NRTXN(path)); + nr_php_zval_free(&expr); + nr_php_zval_free(&job_obj); + + /* Get class with connection_name empty string. */ + job_obj = tlib_php_request_eval_expr("new my_job(connection_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", arg_unused, 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 connection_name set. */ + job_obj = tlib_php_request_eval_expr( + "new my_job(connection_name:'ConnectionName')"); + 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", arg_unused, 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 (ConnectionName:default)", NRTXN(path)); + nr_php_zval_free(&expr); + nr_php_zval_free(&job_obj); + + /* Get class with queue_name empty string. */ + job_obj = tlib_php_request_eval_expr("new my_job(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", arg_unused, 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 queue_name set. */ + job_obj = tlib_php_request_eval_expr("new my_job(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", arg_unused, 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:QueueName)", NRTXN(path)); + nr_php_zval_free(&expr); + nr_php_zval_free(&job_obj); + + /* Get class all vars 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", arg_unused, 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); + + /* + * 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(); +} + +static void test_job_txn_startstop_wrappers(TSRMLS_D) { + /* + * Test the wrappers that start and end the job txn: + * Illuminate\Queue\Worker::process + * Illuminate\Queue\SyncQueue::executeJob + * These wrappers 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; + nrtime_t txn_time = 0; + nrtime_t new_txn_time = 0; + tlib_php_engine_create(""); + tlib_php_request_start(); + + setup_classes(); + + NRINI(force_framework) = NR_FW_LARAVEL; + nr_laravel_queue_enable(); + + /* + * nr_laravel_queue_worker_before will end the txn and discard it and all + * segments before starting a new txn. With OAPI we store wraprecs on the + * 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 + * use execute job to show the exception path. + */ + + tlib_pass_if_not_null("Txn should not be null at the start of the test.", + NRPRG(txn)); + txn_time = nr_txn_start_time(NRPRG(txn)); + + nr_txn_set_path("ToBeDiscarded", NRPRG(txn), "Farewell", NR_PATH_TYPE_CUSTOM, + NR_OK_TO_OVERWRITE); + tlib_pass_if_str_equal("Path should exist", "Farewell", NRTXN(path)); + + /* Create the mocked Worker and call process*/ + obj = tlib_php_request_eval_expr("new Illuminate\\Queue\\Worker"); + tlib_pass_if_not_null("object shouldn't be NULL", obj); + expr = nr_php_call(obj, "process"); + tlib_pass_if_not_null("Expression should evaluate.", expr); + new_txn_time = nr_txn_start_time(NRPRG(txn)); + tlib_pass_if_not_null( + "Txn should not be null after the call to end and start a txn.", + NRPRG(txn)); + tlib_pass_if_true("Txn times should NOT match.", txn_time != new_txn_time, + "Verified times are different, new time is: " NR_TIME_FMT, + new_txn_time); + /* + * The before wrapper will stop/start a txn and name the new one unknown until + * we get naming. The after/clean wrapper stop/start a txn and give no name to + * the new txn that wlil get discarded later. So if both txns have been + * started/stopped, we should end up with a NULL txn name. + */ + tlib_pass_if_null("Txn name should be NULL", NRTXN(path)); + nr_php_zval_free(&expr); + nr_php_zval_free(&obj); + tlib_php_request_end(); + tlib_php_engine_destroy(); + + tlib_php_engine_create(""); + tlib_php_request_start(); + + setup_classes(); + + NRINI(force_framework) = NR_FW_LARAVEL; + nr_laravel_queue_enable(); + + tlib_pass_if_not_null("Txn should not be null at the start of the test.", + NRPRG(txn)); + txn_time = nr_txn_start_time(NRPRG(txn)); + + nr_txn_set_path("ToBeDiscarded", NRPRG(txn), "Farewell", NR_PATH_TYPE_CUSTOM, + NR_OK_TO_OVERWRITE); + tlib_pass_if_str_equal("Path should exist", "Farewell", NRTXN(path)); + + /* Create the mocked Worker and call process*/ + obj = tlib_php_request_eval_expr("new Illuminate\\Queue\\SyncQueue"); + 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"); + tlib_pass_if_not_null("Expression should evaluate.", expr); + new_txn_time = nr_txn_start_time(NRPRG(txn)); + tlib_pass_if_not_null( + "Txn should not be null after the call to end and start a txn.", + NRPRG(txn)); + tlib_pass_if_true("Txn times should NOT match.", txn_time != new_txn_time, + "Verified times are different, new time is: " NR_TIME_FMT, + new_txn_time); + /* + * The Job txn will either be named after the job or named with unknown. + * Any txn started as we wait for another job will have a NULL name. + */ + tlib_pass_if_null("Txn name should be NULL", NRTXN(path)); + nr_php_zval_free(&expr); + nr_php_zval_free(&obj); + tlib_php_request_end(); + tlib_php_engine_destroy(); +} + +#endif + +void test_main(void* p NRUNUSED) { +#if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO + tlib_php_engine_create(""); + + test_job_txn_naming_wrappers(); + + tlib_php_engine_destroy(); + + test_job_txn_startstop_wrappers(); + +#endif /* PHP 8.0+ */ +} From e3db8f0b469a094daee546b8aad27497d9f1af55 Mon Sep 17 00:00:00 2001 From: Amber Sistla Date: Fri, 19 Sep 2025 20:04:53 -0700 Subject: [PATCH 09/14] fix: formatting --- agent/Makefile.frag | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/Makefile.frag b/agent/Makefile.frag index 843aa5de9..f1b07447b 100644 --- a/agent/Makefile.frag +++ b/agent/Makefile.frag @@ -87,7 +87,7 @@ TEST_BINARIES = \ tests/test_environment \ tests/test_fw_codeigniter \ tests/test_fw_drupal \ - tests/test_fw_laravel_queue \ + tests/test_fw_laravel_queue \ tests/test_fw_support \ tests/test_fw_wordpress \ tests/test_globals \ From 1ec2a7e5ed837df1fd64f6f1391d2bdf1a881a81 Mon Sep 17 00:00:00 2001 From: Amber Sistla Date: Mon, 22 Sep 2025 14:11:16 -0700 Subject: [PATCH 10/14] fix: proper priority --- agent/fw_laravel_queue.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/agent/fw_laravel_queue.c b/agent/fw_laravel_queue.c index 7c2c72755..d0b062959 100644 --- a/agent/fw_laravel_queue.c +++ b/agent/fw_laravel_queue.c @@ -372,12 +372,13 @@ NR_PHP_WRAPPER(nr_laravel_queue_worker_after) { txn for the job * */ + zval exception; ZVAL_OBJ(&exception, EG(exception)); nr_status_t st; st = nr_php_error_record_exception( - NRPRG(txn), &exception, 50, false /* add to segment */, - NULL /* use default prefix */, &NRPRG(exception_filters)); + NRPRG(txn), &exception, NR_PHP_ERROR_PRIORITY_UNCAUGHT_EXCEPTION, false /* add to segment */, + "Unhandled exception within Laravel Queue job: ", &NRPRG(exception_filters)); if (NR_FAILURE == st) { nrl_verbosedebug(NRL_FRAMEWORK, "%s: unable to record exception", From 8d29127e1668124becf36fed339d5e3e2fcaff55 Mon Sep 17 00:00:00 2001 From: Amber Sistla Date: Mon, 22 Sep 2025 14:16:29 -0700 Subject: [PATCH 11/14] fix: revert mistaken change --- agent/fw_laravel_queue.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/agent/fw_laravel_queue.c b/agent/fw_laravel_queue.c index d0b062959..182650004 100644 --- a/agent/fw_laravel_queue.c +++ b/agent/fw_laravel_queue.c @@ -645,6 +645,8 @@ NR_PHP_WRAPPER(nr_laravel_queue_worker_process) { nr_free(txn_name); } + NR_WRAPPER_CALL + /* * We need to report any uncaught exceptions now, so that they're on the * transaction we're about to end. We can see if there's an exception waiting @@ -683,13 +685,13 @@ NR_PHP_WRAPPER(nr_laravel_queue_worker_process) { nr_php_arg_release(&job); /* - * End the useless transaction and then start a new transaction so our + * End the real transaction and then start a new transaction so our * instrumentation continues to fire, knowing that we'll ignore that - * transaction if the worker continues when Worker::process() is called again. + * transaction either when Worker::process() is called again or when + * WorkCommand::handle() exits. */ - nr_php_txn_end(1, 0 TSRMLS_CC); + nr_php_txn_end(0, 0 TSRMLS_CC); nr_php_txn_begin(NULL, NULL TSRMLS_CC); - NR_PHP_WRAPPER_CALL; } NR_PHP_WRAPPER_END From ca9bd70646085401403746b792facf5ea43b035c Mon Sep 17 00:00:00 2001 From: Amber Sistla Date: Mon, 22 Sep 2025 14:28:11 -0700 Subject: [PATCH 12/14] fix: semicolon --- agent/fw_laravel_queue.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/fw_laravel_queue.c b/agent/fw_laravel_queue.c index 182650004..cdc03e10b 100644 --- a/agent/fw_laravel_queue.c +++ b/agent/fw_laravel_queue.c @@ -645,7 +645,7 @@ NR_PHP_WRAPPER(nr_laravel_queue_worker_process) { nr_free(txn_name); } - NR_WRAPPER_CALL + NR_WRAPPER_CALL; /* * We need to report any uncaught exceptions now, so that they're on the From c473d6c16c32b1d2aab593895fb745688adf32a8 Mon Sep 17 00:00:00 2001 From: Amber Sistla Date: Mon, 22 Sep 2025 14:39:15 -0700 Subject: [PATCH 13/14] aarg --- agent/fw_laravel_queue.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/fw_laravel_queue.c b/agent/fw_laravel_queue.c index cdc03e10b..5d9013214 100644 --- a/agent/fw_laravel_queue.c +++ b/agent/fw_laravel_queue.c @@ -645,7 +645,7 @@ NR_PHP_WRAPPER(nr_laravel_queue_worker_process) { nr_free(txn_name); } - NR_WRAPPER_CALL; + NR_PHP_WRAPPER_CALL; /* * We need to report any uncaught exceptions now, so that they're on the From 69509f2d4bcf6acdedeef62c52f27cd65bd371ba Mon Sep 17 00:00:00 2001 From: Amber Sistla Date: Mon, 22 Sep 2025 14:49:51 -0700 Subject: [PATCH 14/14] remove call as it's at the end of the func and gets called in NR_PHP_WRAPPER_END anyway --- agent/fw_laravel_queue.c | 1 - 1 file changed, 1 deletion(-) diff --git a/agent/fw_laravel_queue.c b/agent/fw_laravel_queue.c index 5d9013214..fab0ab374 100644 --- a/agent/fw_laravel_queue.c +++ b/agent/fw_laravel_queue.c @@ -334,7 +334,6 @@ NR_PHP_WRAPPER(nr_laravel_queue_worker_before) { segment->wraprec = wraprec; } } - NR_PHP_WRAPPER_CALL; } NR_PHP_WRAPPER_END