Skip to content

feat(agent): update how laravel sync jobs are handled #1128

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 6, 2025
Merged

feat(agent): update how laravel sync jobs are handled #1128

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0c0a9b6
feat(agent): remove sync queue wrap functions and update unit tests
hahuja2 Oct 21, 2025
00bae1e
feat(agent): remove sync queue wrap function
hahuja2 Oct 21, 2025
d3933a0
chore(agent): free zval
hahuja2 Oct 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 0 additions & 19 deletions agent/fw_laravel_queue.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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).
Expand All @@ -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
Expand All @@ -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,
Expand All @@ -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

Expand Down
83 changes: 19 additions & 64 deletions agent/tests/test_fw_laravel_queue.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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:
* "<job_name> (<connection_name>:<queue_name>)"
*/

Expand Down Expand Up @@ -185,71 +189,22 @@ 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();
}

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("");
Expand All @@ -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.
*/

Expand Down Expand Up @@ -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(
Expand All @@ -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();
}
Expand Down