From 2197a45266f476553ada0dcce241ecce3ae79c24 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Wed, 20 Mar 2024 14:53:01 -0600 Subject: [PATCH 1/5] refactor(agent): remove oapi 'clean' callback --- agent/fw_cakephp.c | 4 +- agent/fw_drupal.c | 39 +++----- agent/fw_drupal8.c | 24 ++--- agent/fw_laravel.c | 12 +-- agent/fw_laravel_queue.c | 16 ++-- agent/fw_lumen.c | 4 +- agent/fw_magento2.c | 16 ++-- agent/fw_slim.c | 12 +-- agent/fw_symfony4.c | 4 +- agent/fw_wordpress.c | 30 +++---- agent/fw_yii.c | 17 ++-- agent/lib_doctrine2.c | 10 +-- agent/lib_mongodb.c | 64 +++++++------- agent/lib_predis.c | 29 ++---- agent/php_execute.c | 13 +-- agent/php_newrelic.h | 2 +- agent/php_user_instrument.c | 16 ---- agent/php_user_instrument.h | 10 +-- agent/php_wrapper.c | 55 +++++------- agent/php_wrapper.h | 20 ++--- agent/tests/test_php_wrapper.c | 36 ++++---- .../frameworks/drupal/mock_module_handler.php | 5 ++ .../drupal/test_invoke_all_with.php | 5 +- tests/integration/test_simple.php | 88 +++++++++++++++++++ 24 files changed, 262 insertions(+), 269 deletions(-) create mode 100644 tests/integration/test_simple.php diff --git a/agent/fw_cakephp.c b/agent/fw_cakephp.c index 1dfe88231..c956b4de0 100644 --- a/agent/fw_cakephp.c +++ b/agent/fw_cakephp.c @@ -331,8 +331,8 @@ void nr_cakephp_enable_2(TSRMLS_D) { nr_cakephp_name_the_wt_2 TSRMLS_CC); #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \ && !defined OVERWRITE_ZEND_EXECUTE_DATA - nr_php_wrap_user_function_before_after_clean( - NR_PSTR("CakeException::__construct"), nr_cakephp_problem_2, NULL, NULL); + nr_php_wrap_user_function_before_after( + NR_PSTR("CakeException::__construct"), nr_cakephp_problem_2, NULL); #else nr_php_wrap_user_function(NR_PSTR("CakeException::__construct"), nr_cakephp_problem_2 TSRMLS_CC); diff --git a/agent/fw_drupal.c b/agent/fw_drupal.c index 3537f8b42..5a9b2ce4e 100644 --- a/agent/fw_drupal.c +++ b/agent/fw_drupal.c @@ -305,10 +305,10 @@ NR_PHP_WRAPPER(nr_drupal_http_request_after) { = nr_drupal_http_request_get_method(NR_EXECUTE_ORIG_ARGS); external_params.encoded_response_header - = nr_drupal_http_request_get_response_header(&func_return_value); + = nr_drupal_http_request_get_response_header(NR_GET_RETURN_VALUE_PTR); external_params.status - = nr_drupal_http_request_get_response_code(&func_return_value); + = nr_drupal_http_request_get_response_code(NR_GET_RETURN_VALUE_PTR); if (NRPRG(txn) && NRTXN(special_flags.debug_cat)) { nrl_verbosedebug( NRL_CAT, "CAT: outbound response: transport='Drupal 6-7' %s=" NRP_FMT, @@ -331,16 +331,6 @@ NR_PHP_WRAPPER(nr_drupal_http_request_after) { } NR_PHP_WRAPPER_END -NR_PHP_WRAPPER(nr_drupal_http_request_clean) { - NR_UNUSED_SPECIALFN; - NR_UNUSED_FUNC_RETURN_VALUE; - (void)wraprec; - - NR_PHP_WRAPPER_REQUIRE_FRAMEWORK(NR_FW_DRUPAL); - - NRPRG(drupal_http_request_depth) -= 1; -} -NR_PHP_WRAPPER_END #else /* @@ -778,14 +768,6 @@ NR_PHP_WRAPPER(nr_drupal_wrap_module_invoke_all_after) { } NR_PHP_WRAPPER_END -NR_PHP_WRAPPER(nr_drupal_wrap_module_invoke_all_clean) { - NR_UNUSED_SPECIALFN; - NR_UNUSED_FUNC_RETURN_VALUE; - (void)wraprec; - nr_drupal_invoke_all_hook_stacks_pop(); -} -NR_PHP_WRAPPER_END - #else NR_PHP_WRAPPER(nr_drupal_wrap_module_invoke_all) { zval* hook = NULL; @@ -834,14 +816,14 @@ void nr_drupal_enable(TSRMLS_D) { nr_drupal_cron_run TSRMLS_CC); #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \ && !defined OVERWRITE_ZEND_EXECUTE_DATA - nr_php_wrap_user_function_before_after_clean( - NR_PSTR("QFormBase::Run"), nr_drupal_qdrupal_name_the_wt, NULL, NULL); - nr_php_wrap_user_function_before_after_clean( + nr_php_wrap_user_function_before_after( + NR_PSTR("QFormBase::Run"), nr_drupal_qdrupal_name_the_wt, NULL); + nr_php_wrap_user_function_before_after( NR_PSTR("drupal_page_cache_header"), nr_drupal_name_wt_as_cached_page, - NULL, NULL); - nr_php_wrap_user_function_before_after_clean( + NULL); + nr_php_wrap_user_function_before_after( NR_PSTR("drupal_http_request"), nr_drupal_http_request_before, - nr_drupal_http_request_after, nr_drupal_http_request_clean); + nr_drupal_http_request_after); #else nr_php_wrap_user_function(NR_PSTR("QFormBase::Run"), nr_drupal_qdrupal_name_the_wt TSRMLS_CC); @@ -860,10 +842,9 @@ void nr_drupal_enable(TSRMLS_D) { nr_drupal_wrap_module_invoke TSRMLS_CC); #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \ && !defined OVERWRITE_ZEND_EXECUTE_DATA - nr_php_wrap_user_function_before_after_clean( + nr_php_wrap_user_function_before_after( NR_PSTR("module_invoke_all"), nr_drupal_wrap_module_invoke_all_before, - nr_drupal_wrap_module_invoke_all_after, - nr_drupal_wrap_module_invoke_all_clean); + nr_drupal_wrap_module_invoke_all_after); #else nr_php_wrap_user_function(NR_PSTR("module_invoke_all"), nr_drupal_wrap_module_invoke_all TSRMLS_CC); diff --git a/agent/fw_drupal8.c b/agent/fw_drupal8.c index 541cb8b87..41c53a2f9 100644 --- a/agent/fw_drupal8.c +++ b/agent/fw_drupal8.c @@ -76,13 +76,12 @@ static void nr_drupal8_add_method_callback(const zend_class_entry* ce, #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \ && !defined OVERWRITE_ZEND_EXECUTE_DATA -static void nr_drupal8_add_method_callback_before_after_clean( +static void nr_drupal8_add_method_callback_before_after( const zend_class_entry* ce, const char* method, size_t method_len, nrspecialfn_t before_callback, - nrspecialfn_t after_callback, - nrspecialfn_t clean_callback) { + nrspecialfn_t after_callback) { zend_function* function = NULL; if (NULL == ce) { @@ -106,9 +105,9 @@ static void nr_drupal8_add_method_callback_before_after_clean( "%.*s::%.*s", NRSAFELEN(nr_php_class_entry_name_length(ce)), nr_php_class_entry_name(ce), NRSAFELEN(method_len), method); - nr_php_wrap_user_function_before_after_clean( + nr_php_wrap_user_function_before_after( class_method, nr_strlen(class_method), - before_callback, after_callback, clean_callback); + before_callback, after_callback); nr_free(class_method); } @@ -542,12 +541,6 @@ NR_PHP_WRAPPER(nr_drupal94_invoke_all_with_after) { nr_drupal_invoke_all_hook_stacks_pop(); } NR_PHP_WRAPPER_END - -NR_PHP_WRAPPER(nr_drupal94_invoke_all_with_clean) { - (void)wraprec; - nr_drupal_invoke_all_hook_stacks_pop(); -} -NR_PHP_WRAPPER_END #endif // OAPI /* @@ -584,11 +577,10 @@ NR_PHP_WRAPPER(nr_drupal8_module_handler) { /* Drupal 9.4 introduced a replacement method for getImplentations */ #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \ && !defined OVERWRITE_ZEND_EXECUTE_DATA - nr_drupal8_add_method_callback_before_after_clean( + nr_drupal8_add_method_callback_before_after( ce, NR_PSTR("invokeallwith"), nr_drupal94_invoke_all_with, - nr_drupal94_invoke_all_with_after, - nr_drupal94_invoke_all_with_clean); + nr_drupal94_invoke_all_with_after); #else nr_drupal8_add_method_callback(ce, NR_PSTR("invokeallwith"), nr_drupal94_invoke_all_with TSRMLS_CC); @@ -709,10 +701,10 @@ void nr_drupal8_enable(TSRMLS_D) { */ #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \ && !defined OVERWRITE_ZEND_EXECUTE_DATA - nr_php_wrap_user_function_before_after_clean( + nr_php_wrap_user_function_before_after( NR_PSTR("Symfony\\Component\\HttpKernel\\EventListe" "ner\\RouterListener::onKernelRequest"), - nr_drupal8_name_the_wt_via_symfony, NULL, NULL); + nr_drupal8_name_the_wt_via_symfony, NULL); #else nr_php_wrap_user_function(NR_PSTR("Symfony\\Component\\HttpKernel\\EventListe" "ner\\RouterListener::onKernelRequest"), diff --git a/agent/fw_laravel.c b/agent/fw_laravel.c index bbac65b20..0c5496cdc 100644 --- a/agent/fw_laravel.c +++ b/agent/fw_laravel.c @@ -772,8 +772,8 @@ static void nr_laravel5_wrap_middleware(zval* app TSRMLS_DC) { Z_STRVAL_P(classname)); #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \ && !defined OVERWRITE_ZEND_EXECUTE_DATA - nr_php_wrap_user_function_before_after_clean( - name, nr_strlen(name), nr_laravel5_middleware_handle, NULL, NULL); + nr_php_wrap_user_function_before_after( + name, nr_strlen(name), nr_laravel5_middleware_handle, NULL); #else nr_php_wrap_user_function(name, nr_strlen(name), nr_laravel5_middleware_handle TSRMLS_CC); @@ -833,8 +833,8 @@ static void nr_laravel_add_callback_method(const zend_class_entry* ce, #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \ && !defined OVERWRITE_ZEND_EXECUTE_DATA - nr_php_wrap_user_function_before_after_clean( - class_method, nr_strlen(class_method), callback, NULL, NULL); + nr_php_wrap_user_function_before_after( + class_method, nr_strlen(class_method), callback, NULL); #else nr_php_wrap_user_function(class_method, nr_strlen(class_method), callback TSRMLS_CC); @@ -1231,9 +1231,9 @@ void nr_laravel_enable(TSRMLS_D) { */ #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \ && !defined OVERWRITE_ZEND_EXECUTE_DATA - nr_php_wrap_user_function_before_after_clean( + nr_php_wrap_user_function_before_after( NR_PSTR("Illuminate\\Console\\Application::doRun"), - nr_laravel_console_application_dorun, NULL, NULL); + nr_laravel_console_application_dorun, NULL); #else nr_php_wrap_user_function(NR_PSTR("Illuminate\\Console\\Application::doRun"), nr_laravel_console_application_dorun TSRMLS_CC); diff --git a/agent/fw_laravel_queue.c b/agent/fw_laravel_queue.c index b4d5576e7..a972236f6 100644 --- a/agent/fw_laravel_queue.c +++ b/agent/fw_laravel_queue.c @@ -864,18 +864,18 @@ void nr_laravel_queue_enable(TSRMLS_D) { * took. */ - nr_php_wrap_user_function_before_after_clean( + nr_php_wrap_user_function_before_after( NR_PSTR("Illuminate\\Queue\\Worker::raiseBeforeJobEvent"), NULL, - nr_laravel_queue_worker_raiseBeforeJobEvent_after, NULL); - nr_php_wrap_user_function_before_after_clean( + nr_laravel_queue_worker_raiseBeforeJobEvent_after); + nr_php_wrap_user_function_before_after( NR_PSTR("Illuminate\\Queue\\Worker::raiseAfterJobEvent"), - nr_laravel_queue_worker_raiseAfterJobEvent_before, NULL, NULL); - nr_php_wrap_user_function_before_after_clean( + nr_laravel_queue_worker_raiseAfterJobEvent_before, NULL); + nr_php_wrap_user_function_before_after( NR_PSTR("Illuminate\\Queue\\SyncQueue::raiseBeforeJobEvent"), - nr_laravel_queue_syncqueue_raiseBeforeJobEvent_before, NULL, NULL); - nr_php_wrap_user_function_before_after_clean( + nr_laravel_queue_syncqueue_raiseBeforeJobEvent_before, NULL); + nr_php_wrap_user_function_before_after( NR_PSTR("Illuminate\\Queue\\SyncQueue::raiseAfterJobEvent"), - nr_laravel_queue_worker_raiseAfterJobEvent_before, NULL, NULL); + nr_laravel_queue_worker_raiseAfterJobEvent_before, NULL); #else diff --git a/agent/fw_lumen.c b/agent/fw_lumen.c index 2d34551f0..028c2c71c 100644 --- a/agent/fw_lumen.c +++ b/agent/fw_lumen.c @@ -225,9 +225,9 @@ void nr_lumen_enable(TSRMLS_D) { nr_lumen_handle_found_route TSRMLS_CC); #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \ && !defined OVERWRITE_ZEND_EXECUTE_DATA - nr_php_wrap_user_function_before_after_clean( + nr_php_wrap_user_function_before_after( NR_PSTR("Laravel\\Lumen\\Application::sendExceptionToHandler"), - nr_lumen_exception, NULL, NULL); + nr_lumen_exception, NULL); #else nr_php_wrap_user_function( NR_PSTR("Laravel\\Lumen\\Application::sendExceptionToHandler"), diff --git a/agent/fw_magento2.c b/agent/fw_magento2.c index f96994f25..e67f2858b 100644 --- a/agent/fw_magento2.c +++ b/agent/fw_magento2.c @@ -440,9 +440,9 @@ void nr_magento2_enable(TSRMLS_D) { */ #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \ && !defined OVERWRITE_ZEND_EXECUTE_DATA - nr_php_wrap_user_function_before_after_clean( + nr_php_wrap_user_function_before_after( NR_PSTR("Magento\\Framework\\App\\Action\\Action::dispatch"), - nr_magento2_action_dispatch, NULL, NULL); + nr_magento2_action_dispatch, NULL); #else nr_php_wrap_user_function( NR_PSTR("Magento\\Framework\\App\\Action\\Action::dispatch"), @@ -472,10 +472,10 @@ void nr_magento2_enable(TSRMLS_D) { */ #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \ && !defined OVERWRITE_ZEND_EXECUTE_DATA - nr_php_wrap_user_function_before_after_clean( + nr_php_wrap_user_function_before_after( NR_PSTR( "Magento\\Webapi\\Controller\\Rest\\InputParamsResolver::resolve"), - nr_magento2_inputparamsresolver_resolve, NULL, NULL); + nr_magento2_inputparamsresolver_resolve, NULL); #else nr_php_wrap_user_function( NR_PSTR( @@ -497,14 +497,14 @@ void nr_magento2_enable(TSRMLS_D) { nr_magento2_soap_iswsdllistrequest TSRMLS_CC); #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \ && !defined OVERWRITE_ZEND_EXECUTE_DATA - nr_php_wrap_user_function_before_after_clean( + nr_php_wrap_user_function_before_after( NR_PSTR("Magento\\Webapi\\Controller\\Soap\\Request\\Handler::_" "prepareRequestData"), - nr_magento2_soap_handler_preparerequestdata, NULL, NULL); - nr_php_wrap_user_function_before_after_clean( + nr_magento2_soap_handler_preparerequestdata, NULL); + nr_php_wrap_user_function_before_after( NR_PSTR("Magento\\Webapi\\Controller\\Soap\\Request\\Handler::" "prepareOperationInput"), - nr_magento2_soap_handler_prepareoperationinput, NULL, NULL); + nr_magento2_soap_handler_prepareoperationinput, NULL); #else nr_php_wrap_user_function( diff --git a/agent/fw_slim.c b/agent/fw_slim.c index facaeecc1..b352debfe 100644 --- a/agent/fw_slim.c +++ b/agent/fw_slim.c @@ -181,17 +181,17 @@ void nr_slim_enable(TSRMLS_D) { && !defined OVERWRITE_ZEND_EXECUTE_DATA /* Slim 3 */ - nr_php_wrap_user_function_before_after_clean( - NR_PSTR("Slim\\Route::run"), nr_slim3_4_route_run, NULL, NULL); + nr_php_wrap_user_function_before_after( + NR_PSTR("Slim\\Route::run"), nr_slim3_4_route_run, NULL); /* Slim 4 */ - nr_php_wrap_user_function_before_after_clean( - NR_PSTR("Slim\\Routing\\Route::run"), nr_slim3_4_route_run, NULL, NULL); + nr_php_wrap_user_function_before_after( + NR_PSTR("Slim\\Routing\\Route::run"), nr_slim3_4_route_run, NULL); /* Slim 4 */ - nr_php_wrap_user_function_before_after_clean( + nr_php_wrap_user_function_before_after( NR_PSTR("Slim\\Routing\\Dispatcher::dispatch"), nr_slim4_route_dispatch, - NULL, NULL); + NULL); #else /* Slim 4*/ nr_php_wrap_user_function(NR_PSTR("Slim\\Routing\\Route::run"), diff --git a/agent/fw_symfony4.c b/agent/fw_symfony4.c index 8592186ac..de5d00485 100644 --- a/agent/fw_symfony4.c +++ b/agent/fw_symfony4.c @@ -269,9 +269,9 @@ void nr_symfony4_enable(TSRMLS_D) { */ #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \ && !defined OVERWRITE_ZEND_EXECUTE_DATA - nr_php_wrap_user_function_before_after_clean( + nr_php_wrap_user_function_before_after( NR_PSTR("Symfony\\Component\\Console\\Command\\Command::run"), - nr_symfony4_console_application_run, NULL, NULL); + nr_symfony4_console_application_run, NULL); #else nr_php_wrap_user_function( NR_PSTR("Symfony\\Component\\Console\\Command\\Command::run"), diff --git a/agent/fw_wordpress.c b/agent/fw_wordpress.c index 050750f6a..8503c5286 100644 --- a/agent/fw_wordpress.c +++ b/agent/fw_wordpress.c @@ -682,16 +682,6 @@ NR_PHP_WRAPPER(nr_wordpress_handle_tag_stack_after) { } NR_PHP_WRAPPER_END -NR_PHP_WRAPPER(nr_wordpress_handle_tag_stack_clean) { - NR_UNUSED_SPECIALFN; - NR_UNUSED_FUNC_RETURN_VALUE; - (void)wraprec; - if (0 != NRINI(wordpress_hooks)) { - clean_wordpress_tag_stack(auto_segment); - } -} -NR_PHP_WRAPPER_END - NR_PHP_WRAPPER(nr_wordpress_apply_filters_after) { /* using nr_php_get_user_func_arg() so that we don't perform another copy * when all we want to do is check the string length */ @@ -763,8 +753,8 @@ NR_PHP_WRAPPER(nr_wordpress_add_filter) { if (NULL != wordpress_plugin_theme || NRPRG(wordpress_core)) { #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \ && !defined OVERWRITE_ZEND_EXECUTE_DATA - callback_wraprec = nr_php_wrap_callable_before_after_clean( - zf, NULL, nr_wordpress_wrap_hook, nr_wordpress_wrap_hook); + callback_wraprec = nr_php_wrap_callable_before_after( + zf, NULL, nr_wordpress_wrap_hook); #else callback_wraprec = nr_php_wrap_callable(zf, nr_wordpress_wrap_hook); #endif @@ -823,22 +813,22 @@ void nr_wordpress_version() { void nr_wordpress_enable(TSRMLS_D) { #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \ && !defined OVERWRITE_ZEND_EXECUTE_DATA - nr_php_wrap_user_function_before_after_clean( + nr_php_wrap_user_function_before_after( NR_PSTR("apply_filters"), nr_wordpress_apply_filters, - nr_wordpress_apply_filters_after, nr_wordpress_handle_tag_stack_clean); + nr_wordpress_apply_filters_after); if (0 != NRINI(wordpress_hooks)) { - nr_php_wrap_user_function_before_after_clean( + nr_php_wrap_user_function_before_after( NR_PSTR("apply_filters_ref_array"), nr_wordpress_exec_handle_tag, - nr_wordpress_handle_tag_stack_after, nr_wordpress_handle_tag_stack_clean); + nr_wordpress_handle_tag_stack_after); - nr_php_wrap_user_function_before_after_clean( + nr_php_wrap_user_function_before_after( NR_PSTR("do_action"), nr_wordpress_exec_handle_tag, - nr_wordpress_handle_tag_stack_after, nr_wordpress_handle_tag_stack_clean); + nr_wordpress_handle_tag_stack_after); - nr_php_wrap_user_function_before_after_clean( + nr_php_wrap_user_function_before_after( NR_PSTR("do_action_ref_array"), nr_wordpress_exec_handle_tag, - nr_wordpress_handle_tag_stack_after, nr_wordpress_handle_tag_stack_clean); + nr_wordpress_handle_tag_stack_after); if (0 != NRPRG(wordpress_plugins)) { nr_php_wrap_user_function(NR_PSTR("add_filter"), nr_wordpress_add_filter); } diff --git a/agent/fw_yii.c b/agent/fw_yii.c index 0b1af7c96..6d7d08803 100644 --- a/agent/fw_yii.c +++ b/agent/fw_yii.c @@ -93,12 +93,11 @@ NR_PHP_WRAPPER_END void nr_yii1_enable(TSRMLS_D) { #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \ && !defined OVERWRITE_ZEND_EXECUTE_DATA - nr_php_wrap_user_function_before_after_clean( - NR_PSTR("CAction::runWithParams"), nr_yii1_runWithParams_wrapper, NULL, - NULL); - nr_php_wrap_user_function_before_after_clean( + nr_php_wrap_user_function_before_after( + NR_PSTR("CAction::runWithParams"), nr_yii1_runWithParams_wrapper, NULL); + nr_php_wrap_user_function_before_after( NR_PSTR("CInlineAction::runWithParams"), nr_yii1_runWithParams_wrapper, - NULL, NULL); + NULL); #else nr_php_wrap_user_function(NR_PSTR("CAction::runWithParams"), nr_yii1_runWithParams_wrapper TSRMLS_CC); @@ -196,12 +195,12 @@ NR_PHP_WRAPPER_END void nr_yii2_enable(TSRMLS_D) { #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \ && !defined OVERWRITE_ZEND_EXECUTE_DATA - nr_php_wrap_user_function_before_after_clean( + nr_php_wrap_user_function_before_after( NR_PSTR("yii\\base\\Action::runWithParams"), - nr_yii2_runWithParams_wrapper, NULL, NULL); - nr_php_wrap_user_function_before_after_clean( + nr_yii2_runWithParams_wrapper, NULL); + nr_php_wrap_user_function_before_after( NR_PSTR("yii\\base\\InlineAction::runWithParams"), - nr_yii2_runWithParams_wrapper, NULL, NULL); + nr_yii2_runWithParams_wrapper, NULL); #else nr_php_wrap_user_function(NR_PSTR("yii\\base\\Action::runWithParams"), nr_yii2_runWithParams_wrapper TSRMLS_CC); diff --git a/agent/lib_doctrine2.c b/agent/lib_doctrine2.c index 8f8b8dfe3..b71e3b5ca 100644 --- a/agent/lib_doctrine2.c +++ b/agent/lib_doctrine2.c @@ -63,12 +63,6 @@ NR_PHP_WRAPPER_END #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \ && !defined OVERWRITE_ZEND_EXECUTE_DATA -NR_PHP_WRAPPER(nr_doctrine2_cache_dql_clean) { - (void)wraprec; - nr_free(NRPRG(doctrine_dql)); -} -NR_PHP_WRAPPER_END - NR_PHP_WRAPPER(nr_doctrine2_cache_dql_after) { (void)wraprec; nr_free(NRPRG(doctrine_dql)); @@ -99,9 +93,9 @@ nr_slowsqls_labelled_query_t* nr_doctrine2_lookup_input_query(TSRMLS_D) { void nr_doctrine2_enable(TSRMLS_D) { #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \ && !defined OVERWRITE_ZEND_EXECUTE_DATA - nr_php_wrap_user_function_before_after_clean( + nr_php_wrap_user_function_before_after( NR_PSTR("Doctrine\\ORM\\Query::_doExecute"), nr_doctrine2_cache_dql, - nr_doctrine2_cache_dql_after, nr_doctrine2_cache_dql_clean); + nr_doctrine2_cache_dql_after); #else nr_php_wrap_user_function(NR_PSTR("Doctrine\\ORM\\Query::_doExecute"), nr_doctrine2_cache_dql TSRMLS_CC); diff --git a/agent/lib_mongodb.c b/agent/lib_mongodb.c index 8d129ae9d..aa96d1b60 100644 --- a/agent/lib_mongodb.c +++ b/agent/lib_mongodb.c @@ -264,84 +264,84 @@ void nr_mongodb_enable() { #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \ && !defined OVERWRITE_ZEND_EXECUTE_DATA - nr_php_wrap_user_function_before_after_clean_extra( + nr_php_wrap_user_function_before_after_extra( NR_PSTR("MongoDB\\Operation\\Aggregate::execute"), nr_mongodb_operation_before, nr_mongodb_operation_after, - nr_mongodb_operation_after, "aggregate"); + "aggregate"); - nr_php_wrap_user_function_before_after_clean_extra( + nr_php_wrap_user_function_before_after_extra( NR_PSTR("MongoDB\\Operation\\BulkWrite::execute"), nr_mongodb_operation_before, nr_mongodb_operation_after, - nr_mongodb_operation_after, "bulkWrite"); + "bulkWrite"); - nr_php_wrap_user_function_before_after_clean_extra( + nr_php_wrap_user_function_before_after_extra( NR_PSTR("MongoDB\\Operation\\Count::execute"), nr_mongodb_operation_before, nr_mongodb_operation_after, - nr_mongodb_operation_after, "count"); + "count"); - nr_php_wrap_user_function_before_after_clean_extra( + nr_php_wrap_user_function_before_after_extra( NR_PSTR("MongoDB\\Operation\\CountDocuments::execute"), nr_mongodb_operation_before, nr_mongodb_operation_after, - nr_mongodb_operation_after, "countDocuments"); + "countDocuments"); - nr_php_wrap_user_function_before_after_clean_extra( + nr_php_wrap_user_function_before_after_extra( NR_PSTR("MongoDB\\Operation\\CreateIndexes::execute"), nr_mongodb_operation_before, nr_mongodb_operation_after, - nr_mongodb_operation_after, "createIndexes"); + "createIndexes"); - nr_php_wrap_user_function_before_after_clean_extra( + nr_php_wrap_user_function_before_after_extra( NR_PSTR("MongoDB\\Operation\\Delete::execute"), nr_mongodb_operation_before, nr_mongodb_operation_after, - nr_mongodb_operation_after, "delete"); + "delete"); - nr_php_wrap_user_function_before_after_clean_extra( + nr_php_wrap_user_function_before_after_extra( NR_PSTR("MongoDB\\Operation\\Distinct::execute"), nr_mongodb_operation_before, nr_mongodb_operation_after, - nr_mongodb_operation_after, "distinct"); + "distinct"); - nr_php_wrap_user_function_before_after_clean_extra( + nr_php_wrap_user_function_before_after_extra( NR_PSTR("MongoDB\\Operation\\DropCollection::execute"), nr_mongodb_operation_before, nr_mongodb_operation_after, - nr_mongodb_operation_after, "dropCollection"); + "dropCollection"); - nr_php_wrap_user_function_before_after_clean_extra( + nr_php_wrap_user_function_before_after_extra( NR_PSTR("MongoDB\\Operation\\DropIndexes::execute"), nr_mongodb_operation_before, nr_mongodb_operation_after, - nr_mongodb_operation_after, "dropIndexes"); + "dropIndexes"); - nr_php_wrap_user_function_before_after_clean_extra( + nr_php_wrap_user_function_before_after_extra( NR_PSTR("MongoDB\\Operation\\Find::execute"), nr_mongodb_operation_before, - nr_mongodb_operation_after, nr_mongodb_operation_after, "find"); + nr_mongodb_operation_after, "find"); - nr_php_wrap_user_function_before_after_clean_extra( + nr_php_wrap_user_function_before_after_extra( NR_PSTR("MongoDB\\Operation\\FindAndModify::execute"), nr_mongodb_operation_before, nr_mongodb_operation_after, - nr_mongodb_operation_after, "findAndModify"); + "findAndModify"); - nr_php_wrap_user_function_before_after_clean_extra( + nr_php_wrap_user_function_before_after_extra( NR_PSTR("MongoDB\\Operation\\InsertMany::execute"), nr_mongodb_operation_before, nr_mongodb_operation_after, - nr_mongodb_operation_after, "insertMany"); + "insertMany"); - nr_php_wrap_user_function_before_after_clean_extra( + nr_php_wrap_user_function_before_after_extra( NR_PSTR("MongoDB\\Operation\\InsertOne::execute"), nr_mongodb_operation_before, nr_mongodb_operation_after, - nr_mongodb_operation_after, "insertOne"); + "insertOne"); - nr_php_wrap_user_function_before_after_clean_extra( + nr_php_wrap_user_function_before_after_extra( NR_PSTR("MongoDB\\Operation\\ListIndexes::execute"), nr_mongodb_operation_before, nr_mongodb_operation_after, - nr_mongodb_operation_after, "listIndexes"); + "listIndexes"); - nr_php_wrap_user_function_before_after_clean_extra( + nr_php_wrap_user_function_before_after_extra( NR_PSTR("MongoDB\\Operation\\Update::execute"), nr_mongodb_operation_before, nr_mongodb_operation_after, - nr_mongodb_operation_after, "update"); + "update"); - nr_php_wrap_user_function_before_after_clean_extra( + nr_php_wrap_user_function_before_after_extra( NR_PSTR("MongoDB\\Operation\\DatabaseCommand::execute"), nr_mongodb_operation_before, nr_mongodb_operation_after, - nr_mongodb_operation_after, "databaseCommand"); + "databaseCommand"); #else /* Non-OAPI */ diff --git a/agent/lib_predis.c b/agent/lib_predis.c index 607e11ed2..f25b3c505 100644 --- a/agent/lib_predis.c +++ b/agent/lib_predis.c @@ -755,12 +755,6 @@ NR_PHP_WRAPPER(nr_predis_pipeline_executePipeline_after) { } NR_PHP_WRAPPER_END -NR_PHP_WRAPPER(nr_predis_pipeline_executePipeline_clean) { - (void)wraprec; - predis_executePipeline_handle_stack(); -} -NR_PHP_WRAPPER_END - NR_PHP_WRAPPER(nr_predis_webdisconnection_executeCommand_before) { (void)wraprec; @@ -848,34 +842,29 @@ void nr_predis_enable(TSRMLS_D) { */ #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \ && !defined OVERWRITE_ZEND_EXECUTE_DATA - nr_php_wrap_user_function_before_after_clean( + nr_php_wrap_user_function_before_after( NR_PSTR("Predis\\Pipeline\\Pipeline::executePipeline"), nr_predis_pipeline_executePipeline, - nr_predis_pipeline_executePipeline_after, - nr_predis_pipeline_executePipeline_clean); - nr_php_wrap_user_function_before_after_clean( + nr_predis_pipeline_executePipeline_after); + nr_php_wrap_user_function_before_after( NR_PSTR("Predis\\Pipeline\\Atomic::executePipeline"), nr_predis_pipeline_executePipeline, - nr_predis_pipeline_executePipeline_after, - nr_predis_pipeline_executePipeline_clean); - nr_php_wrap_user_function_before_after_clean( + nr_predis_pipeline_executePipeline_after); + nr_php_wrap_user_function_before_after( NR_PSTR("Predis\\Pipeline\\ConnectionErrorProof::executePipeline"), nr_predis_pipeline_executePipeline, - nr_predis_pipeline_executePipeline_after, - nr_predis_pipeline_executePipeline_clean); - nr_php_wrap_user_function_before_after_clean( + nr_predis_pipeline_executePipeline_after); + nr_php_wrap_user_function_before_after( NR_PSTR("Predis\\Pipeline\\FireAndForget::executePipeline"), nr_predis_pipeline_executePipeline, - nr_predis_pipeline_executePipeline_after, - nr_predis_pipeline_executePipeline_clean); + nr_predis_pipeline_executePipeline_after); /* * Instrument Webdis connections, since they don't use the same * writeRequest()/readResponse() pair as the other connection types. */ - nr_php_wrap_user_function_before_after_clean( + nr_php_wrap_user_function_before_after( NR_PSTR("Predis\\Connection\\WebdisConnection::executeCommand"), nr_predis_webdisconnection_executeCommand_before, - nr_predis_webdisconnection_executeCommand_after, nr_predis_webdisconnection_executeCommand_after); #else nr_php_wrap_user_function( diff --git a/agent/php_execute.c b/agent/php_execute.c index e8e0db9ca..ac5121412 100644 --- a/agent/php_execute.c +++ b/agent/php_execute.c @@ -2068,17 +2068,8 @@ static void nr_php_instrument_func_end(NR_EXECUTE_PROTO) { */ create_metric = wraprec->create_metric; - /* - * A NULL return value ptr means that there was an uncaught exception - * and therefore we want to call the 'clean' function type - */ - if (NULL != nr_php_get_return_value(NR_EXECUTE_ORIG_ARGS)) { - zcaught = nr_zend_call_orig_execute_special(wraprec, segment, - NR_EXECUTE_ORIG_ARGS); - } else { - zcaught = nr_zend_call_oapi_special_clean(wraprec, segment, - NR_EXECUTE_ORIG_ARGS); - } + zcaught = nr_zend_call_orig_execute_special(wraprec, segment, + NR_EXECUTE_ORIG_ARGS); if (nrunlikely(zcaught)) { zend_bailout(); } diff --git a/agent/php_newrelic.h b/agent/php_newrelic.h index 1860b5167..1e9b5453d 100644 --- a/agent/php_newrelic.h +++ b/agent/php_newrelic.h @@ -117,7 +117,7 @@ extern zend_module_entry newrelic_module_entry; #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \ && !defined OVERWRITE_ZEND_EXECUTE_DATA -#define NR_GET_RETURN_VALUE_PTR &func_return_value +#define NR_GET_RETURN_VALUE_PTR func_return_value ? &func_return_value : NULL #else #define NR_GET_RETURN_VALUE_PTR nr_php_get_return_value_ptr(TSRMLS_C) #endif diff --git a/agent/php_user_instrument.c b/agent/php_user_instrument.c index 65b078b71..c7fab76d0 100644 --- a/agent/php_user_instrument.c +++ b/agent/php_user_instrument.c @@ -78,22 +78,6 @@ int nr_zend_call_oapi_special_before(nruserfn_t* wraprec, return zcaught; } - -int nr_zend_call_oapi_special_clean(nruserfn_t* wraprec, - nr_segment_t* segment, - NR_EXECUTE_PROTO) { - volatile int zcaught = 0; - - if (wraprec && wraprec->special_instrumentation_clean) { - zend_try { - wraprec->special_instrumentation_clean(wraprec, segment, - NR_EXECUTE_ORIG_ARGS); - } - zend_catch { zcaught = 1; } - zend_end_try(); - } - return zcaught; -} #endif int nr_zend_call_orig_execute_special(nruserfn_t* wraprec, nr_segment_t* segment, diff --git a/agent/php_user_instrument.h b/agent/php_user_instrument.h index f71889751..15fba7bdf 100644 --- a/agent/php_user_instrument.h +++ b/agent/php_user_instrument.h @@ -72,13 +72,10 @@ typedef struct _nruserfn_t { nrspecialfn_t special_instrumentation; /* * Only used by OAPI, PHP 8+. Used to do any special instrumentation actions - * before a function is executed. special_instrumentation_clean will clean up - * any variables that were set in the before calledback but didn't get cleaned - * up when an exception circumvents the end callback. All callbacks can be - * set. Use the `nr_php_wrap_user_function_after_before_clean` to set. + * before a function is executed. All callbacks can be set using + * `nr_php_wrap_user_function_before_after`. */ nrspecialfn_t special_instrumentation_before; - nrspecialfn_t special_instrumentation_clean; nruserfn_declared_t declared_callback; @@ -213,9 +210,6 @@ extern int nr_zend_call_orig_execute_special(nruserfn_t* wraprec, extern int nr_zend_call_oapi_special_before(nruserfn_t* wraprec, nr_segment_t* segment, NR_EXECUTE_PROTO); -extern int nr_zend_call_oapi_special_clean(nruserfn_t* wraprec, - nr_segment_t* segment, - NR_EXECUTE_PROTO); #endif /* * Purpose : Destroy all user instrumentation records, freeing diff --git a/agent/php_wrapper.c b/agent/php_wrapper.c index 6631c06d2..21fa27f15 100644 --- a/agent/php_wrapper.c +++ b/agent/php_wrapper.c @@ -9,12 +9,11 @@ #include "util_logging.h" #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO -static void nr_php_wraprec_add_before_after_clean_callbacks( +static void nr_php_wraprec_add_before_after_callbacks( const char* name, size_t namelen, nruserfn_t* wraprec, nrspecialfn_t before_callback, - nrspecialfn_t after_callback, - nrspecialfn_t clean_callback) { + nrspecialfn_t after_callback) { if (NULL == wraprec) { return; } @@ -41,47 +40,33 @@ static void nr_php_wraprec_add_before_after_clean_callbacks( return; } - if (is_instrumentation_set_and_not_equal(wraprec->special_instrumentation_clean, - clean_callback)) { - nrl_verbosedebug(NRL_INSTRUMENT, - "%s: attempting to set special_instrumentation_clean " - "for %.*s, but " - "it is already set", - __func__, NRSAFELEN(namelen), NRBLANKSTR(name)); - return; - } - wraprec->special_instrumentation = after_callback; wraprec->special_instrumentation_before = before_callback; - wraprec->special_instrumentation_clean = clean_callback; } -nruserfn_t* nr_php_wrap_user_function_before_after_clean( +nruserfn_t* nr_php_wrap_user_function_before_after( const char* name, size_t namelen, nrspecialfn_t before_callback, - nrspecialfn_t after_callback, - nrspecialfn_t clean_callback) { + nrspecialfn_t after_callback) { nruserfn_t* wraprec = nr_php_add_custom_tracer_named(name, namelen); - nr_php_wraprec_add_before_after_clean_callbacks(name, namelen, wraprec, + nr_php_wraprec_add_before_after_callbacks(name, namelen, wraprec, before_callback, - after_callback, - clean_callback); + after_callback); return wraprec; } -nruserfn_t* nr_php_wrap_user_function_before_after_clean_extra( +nruserfn_t* nr_php_wrap_user_function_before_after_extra( const char* name, size_t namelen, nrspecialfn_t before_callback, nrspecialfn_t after_callback, - nrspecialfn_t clean_callback, const char* extra) { - nruserfn_t* wraprec = nr_php_wrap_user_function_before_after_clean( - name, namelen, before_callback, after_callback, clean_callback); + nruserfn_t* wraprec = nr_php_wrap_user_function_before_after( + name, namelen, before_callback, after_callback); if (nrunlikely(NULL == wraprec)) { nrl_warning(NRL_INSTRUMENT, "%s: unable to wrap '%s'", __func__, @@ -94,11 +79,10 @@ nruserfn_t* nr_php_wrap_user_function_before_after_clean_extra( return wraprec; } -nruserfn_t* nr_php_wrap_callable_before_after_clean( +nruserfn_t* nr_php_wrap_callable_before_after( zend_function* callable, nrspecialfn_t before_callback, - nrspecialfn_t after_callback, - nrspecialfn_t clean_callback) { + nrspecialfn_t after_callback) { char* name = NULL; /* creates a transient wraprec */ @@ -110,10 +94,9 @@ nruserfn_t* nr_php_wrap_callable_before_after_clean( if (nrl_should_print(NRL_VERBOSEDEBUG, NRL_INSTRUMENT)) { name = nr_php_function_debug_name(callable); } - nr_php_wraprec_add_before_after_clean_callbacks(name, nr_strlen(name), wraprec, + nr_php_wraprec_add_before_after_callbacks(name, nr_strlen(name), wraprec, before_callback, - after_callback, - clean_callback); + after_callback); if (nrl_should_print(NRL_VERBOSEDEBUG, NRL_INSTRUMENT) && NULL != name) { nr_free(name); } @@ -178,8 +161,8 @@ nruserfn_t* nr_php_wrap_callable(zend_function* callable, * wraprec's internals be evaluated BEFORE for the callable's. As such, * for OAPI, this creates "before" wrappers, where normally the default * is to create "after" wrappers (see nr_php_wrap_user_function). Should - * "after"/"clean" wrappers ever be desired, it is suggested to create a - * separate nr_php_wrap_generic_callable_before_after_clean() function. + * "after" wrappers ever be desired, it is suggested to create a + * separate nr_php_wrap_generic_callable_before_after() function. * * This creates a transient wraprec that does NOT produce an * "InstrumentedFunction" metric. @@ -193,7 +176,7 @@ nruserfn_t* nr_php_wrap_generic_callable(zval* callable, if (NULL != zf) { #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \ && !defined OVERWRITE_ZEND_EXECUTE_DATA - return nr_php_wrap_callable_before_after_clean(zf, callback, NULL, NULL); + return nr_php_wrap_callable_before_after(zf, callback, NULL); #else return nr_php_wrap_callable(zf, callback); #endif @@ -383,7 +366,11 @@ zval** nr_php_get_return_value_ptr(TSRMLS_D) { return NULL; } - return &EG(current_execute_data)->return_value; + if (NULL != EG(current_execute_data)->return_value) { + return &EG(current_execute_data)->return_value; + } else { + return NULL; + } #else return EG(return_value_ptr_ptr); #endif /* PHP7 */ diff --git a/agent/php_wrapper.h b/agent/php_wrapper.h index 058e06e08..b87a5d9c0 100644 --- a/agent/php_wrapper.h +++ b/agent/php_wrapper.h @@ -87,12 +87,11 @@ /* * OAPI updates: - * There are now before, after, and clean callbacks. + * There are now before and after callbacks. * 1) before_callback gets called when OAPI triggers the begin function hook. * 2) after_callback gets called when OAPI triggers the end function hook. - * 3) clean_callback gets called in the case of an exception, because the - * return value will be null, so the after_callback might not function - * correctly. Use clean_callback to reset any variables or states. + * if an exception occurs, return value will be null, so the after_callback + * must check for NULL correctly. * 4) unless explicitly setting any of the above callbacks, the default * callback is set to after_callback. * @@ -138,25 +137,22 @@ * see how it works with frameworks. */ #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO -extern nruserfn_t* nr_php_wrap_user_function_before_after_clean( +extern nruserfn_t* nr_php_wrap_user_function_before_after( const char* name, size_t namelen, nrspecialfn_t before_callback, - nrspecialfn_t after_callback, - nrspecialfn_t clean_callback); + nrspecialfn_t after_callback); -extern nruserfn_t* nr_php_wrap_callable_before_after_clean( +extern nruserfn_t* nr_php_wrap_callable_before_after( zend_function* callable, nrspecialfn_t before_callback, - nrspecialfn_t after_callback, - nrspecialfn_t clean_callback); + nrspecialfn_t after_callback); -extern nruserfn_t* nr_php_wrap_user_function_before_after_clean_extra( +extern nruserfn_t* nr_php_wrap_user_function_before_after_extra( const char* name, size_t namelen, nrspecialfn_t before_callback, nrspecialfn_t after_callback, - nrspecialfn_t clean_callback, const char* extra); #endif extern nruserfn_t* nr_php_wrap_user_function(const char* name, diff --git a/agent/tests/test_php_wrapper.c b/agent/tests/test_php_wrapper.c index 975a46904..9b09cdedd 100644 --- a/agent/tests/test_php_wrapper.c +++ b/agent/tests/test_php_wrapper.c @@ -110,12 +110,12 @@ static void execute_nested_framework_calls(nrspecialfn_t one_before, #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \ && !defined OVERWRITE_ZEND_EXECUTE_DATA - nr_php_wrap_user_function_before_after_clean( - NR_PSTR("one"), one_before, one_after, NULL); - nr_php_wrap_user_function_before_after_clean( - NR_PSTR("two"), two_before, two_after, NULL); - nr_php_wrap_user_function_before_after_clean( - NR_PSTR("three"), three_before, three_after, NULL); + nr_php_wrap_user_function_before_after( + NR_PSTR("one"), one_before, one_after); + nr_php_wrap_user_function_before_after( + NR_PSTR("two"), two_before, two_after); + nr_php_wrap_user_function_before_after( + NR_PSTR("three"), three_before, three_after); #else /* * This will pick up whichever one isn't null. @@ -574,31 +574,31 @@ static void test_add_arg(TSRMLS_D) { #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \ && !defined OVERWRITE_ZEND_EXECUTE_DATA /* PHP 8.0+ and OAPI */ tlib_php_request_eval("function arg0_def0() { return 4; }" TSRMLS_CC); - nr_php_wrap_user_function_before_after_clean( - NR_PSTR("arg0_def0"), test_add_array, NULL, NULL TSRMLS_CC); + nr_php_wrap_user_function_before_after( + NR_PSTR("arg0_def0"), test_add_array, NULL TSRMLS_CC); tlib_php_request_eval("function arg1_def0($a) { return $a; }" TSRMLS_CC); - nr_php_wrap_user_function_before_after_clean( - NR_PSTR("arg1_def0"), test_add_array, NULL, NULL TSRMLS_CC); + nr_php_wrap_user_function_before_after( + NR_PSTR("arg1_def0"), test_add_array, NULL TSRMLS_CC); tlib_php_request_eval( "function arg0_def1($a = null) { return $a; }" TSRMLS_CC); - nr_php_wrap_user_function_before_after_clean( - NR_PSTR("arg0_def1"), test_add_array, NULL, NULL TSRMLS_CC); + nr_php_wrap_user_function_before_after( + NR_PSTR("arg0_def1"), test_add_array, NULL TSRMLS_CC); tlib_php_request_eval( "function arg1_def1($a, $b = null) { return $b; }" TSRMLS_CC); - nr_php_wrap_user_function_before_after_clean( - NR_PSTR("arg1_def1"), test_add_array, NULL, NULL TSRMLS_CC); + nr_php_wrap_user_function_before_after( + NR_PSTR("arg1_def1"), test_add_array, NULL TSRMLS_CC); tlib_php_request_eval( "function arg1_def1_2($a, $b = null) { return $b; }" TSRMLS_CC); - nr_php_wrap_user_function_before_after_clean( - NR_PSTR("arg1_def1_2"), test_add_2_arrays, NULL, NULL TSRMLS_CC); + nr_php_wrap_user_function_before_after( + NR_PSTR("arg1_def1_2"), test_add_2_arrays, NULL TSRMLS_CC); tlib_php_request_eval("function splat(...$a) { return $a[0]; }" TSRMLS_CC); - nr_php_wrap_user_function_before_after_clean( - NR_PSTR("splat"), test_add_array, NULL, NULL TSRMLS_CC); + nr_php_wrap_user_function_before_after( + NR_PSTR("splat"), test_add_array, NULL TSRMLS_CC); #else tlib_php_request_eval("function arg0_def0() { return 4; }" TSRMLS_CC); nr_php_wrap_user_function(NR_PSTR("arg0_def0"), test_add_array TSRMLS_CC); diff --git a/tests/integration/frameworks/drupal/mock_module_handler.php b/tests/integration/frameworks/drupal/mock_module_handler.php index 84d385f10..da1f2b1a6 100644 --- a/tests/integration/frameworks/drupal/mock_module_handler.php +++ b/tests/integration/frameworks/drupal/mock_module_handler.php @@ -10,6 +10,11 @@ interface ModuleHandlerInterface { public function invokeAllWith($hook_str, $callback); } class ModuleHandler implements ModuleHandlerInterface { + public function __construct(bool $except=false) { + if ($except) { + throw new Exception("Constructor told to except"); + } + } public function invokeAllWith($hook_str, $callback) { if ($hook_str == "hook_1") { $module = "module_a"; diff --git a/tests/integration/frameworks/drupal/test_invoke_all_with.php b/tests/integration/frameworks/drupal/test_invoke_all_with.php index 289d4e69a..74e448111 100644 --- a/tests/integration/frameworks/drupal/test_invoke_all_with.php +++ b/tests/integration/frameworks/drupal/test_invoke_all_with.php @@ -108,7 +108,10 @@ public function invoke(callable $hook, string $module) { // Create module handler $drupal = new Drupal(); -$handler = $drupal->moduleHandler(); +// Throw an exception during creation to test agent recovery +$handler = $drupal->moduleHandler(true); +// Create the actual handler +$handler = $drupal->moduleHandler(false); // Test lambda calback $handler->invokeAllWith("hook_1", function (callable $hook, string $module) { diff --git a/tests/integration/test_simple.php b/tests/integration/test_simple.php new file mode 100644 index 000000000..8fa700b5b --- /dev/null +++ b/tests/integration/test_simple.php @@ -0,0 +1,88 @@ + Date: Wed, 20 Mar 2024 15:20:57 -0600 Subject: [PATCH 2/5] fix: accidentally included file --- tests/integration/test_simple.php | 88 ------------------------------- 1 file changed, 88 deletions(-) delete mode 100644 tests/integration/test_simple.php diff --git a/tests/integration/test_simple.php b/tests/integration/test_simple.php deleted file mode 100644 index 8fa700b5b..000000000 --- a/tests/integration/test_simple.php +++ /dev/null @@ -1,88 +0,0 @@ - Date: Wed, 17 Apr 2024 10:45:25 -0600 Subject: [PATCH 3/5] fix drupal dangling segment --- agent/fw_drupal.c | 27 +++++++--- .../test_bad_params_integer_headers.php | 8 --- .../test_bad_params_integer_headers.php8.php | 49 ------------------- .../drupal7/test_bad_params_null_headers.php | 8 --- .../test_bad_params_null_headers.php8.php | 49 ------------------- 5 files changed, 19 insertions(+), 122 deletions(-) delete mode 100644 tests/integration/external/drupal7/test_bad_params_integer_headers.php8.php delete mode 100644 tests/integration/external/drupal7/test_bad_params_null_headers.php8.php diff --git a/agent/fw_drupal.c b/agent/fw_drupal.c index 5a9b2ce4e..dbc67a7ec 100644 --- a/agent/fw_drupal.c +++ b/agent/fw_drupal.c @@ -281,6 +281,10 @@ NR_PHP_WRAPPER(nr_drupal_http_request_after) { NR_PHP_WRAPPER_REQUIRE_FRAMEWORK(NR_FW_DRUPAL); + nr_segment_external_params_t external_params + = {.library = "Drupal", + .uri = NULL}; + /* * Grab the URL for the external metric, which is the first parameter in all * versions of Drupal. @@ -290,6 +294,12 @@ NR_PHP_WRAPPER(nr_drupal_http_request_after) { goto end; } + if (NULL == NR_GET_RETURN_VALUE_PTR) { + goto end; + } + + external_params.uri = nr_strndup(Z_STRVAL_P(arg1), Z_STRLEN_P(arg1)); + /* * We only want to create a metric here if this isn't a recursive call to * drupal_http_request() caused by the original call returning a redirect. @@ -297,10 +307,6 @@ NR_PHP_WRAPPER(nr_drupal_http_request_after) { * checking a counter. */ if (1 == NRPRG(drupal_http_request_depth)) { - nr_segment_external_params_t external_params - = {.library = "Drupal", - .uri = nr_strndup(Z_STRVAL_P(arg1), Z_STRLEN_P(arg1))}; - external_params.procedure = nr_drupal_http_request_get_method(NR_EXECUTE_ORIG_ARGS); @@ -315,17 +321,22 @@ NR_PHP_WRAPPER(nr_drupal_http_request_after) { X_NEWRELIC_APP_DATA, NRP_CAT(external_params.encoded_response_header)); } + } - nr_segment_external_end(&NRPRG(drupal_http_request_segment), - &external_params); +end: + if (1 == NRPRG(drupal_http_request_depth)) { + if (external_params.uri == NULL) { + nr_segment_discard(&NRPRG(drupal_http_request_segment)); + } else { + nr_segment_external_end(&NRPRG(drupal_http_request_segment), + &external_params); + } NRPRG(drupal_http_request_segment) = NULL; nr_free(external_params.encoded_response_header); nr_free(external_params.procedure); nr_free(external_params.uri); } - -end: nr_php_arg_release(&arg1); NRPRG(drupal_http_request_depth) -= 1; } diff --git a/tests/integration/external/drupal7/test_bad_params_integer_headers.php b/tests/integration/external/drupal7/test_bad_params_integer_headers.php index 1f0aec4c8..27beebb0c 100644 --- a/tests/integration/external/drupal7/test_bad_params_integer_headers.php +++ b/tests/integration/external/drupal7/test_bad_params_integer_headers.php @@ -14,9 +14,6 @@ /*SKIPIF =")) { - die("skip: PHP >= 8.0 not supported\n"); -} */ /*EXPECT_METRICS @@ -35,11 +32,6 @@ [{"name":"OtherTransactionTotalTime"}, [1, "??", "??", "??", "??", "??"]], [{"name":"OtherTransactionTotalTime/php__FILE__"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Supportability/framework/Drupal/forced"}, [1, 0, 0, 0, 0, 0]], - [{"name":"External/127.0.0.1/all"}, [1, "??", "??", "??", "??", "??"]], - [{"name":"External/all"}, [1, "??", "??", "??", "??", "??"]], - [{"name":"External/allOther"}, [1, "??", "??", "??", "??", "??"]], - [{"name":"External/127.0.0.1/all", - "scope":"OtherTransaction/php__FILE__"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Supportability/Logging/LocalDecorating/PHP/disabled"}, [1, "??", "??", "??", "??", "??"]] ] ] diff --git a/tests/integration/external/drupal7/test_bad_params_integer_headers.php8.php b/tests/integration/external/drupal7/test_bad_params_integer_headers.php8.php deleted file mode 100644 index 36cf6b0f1..000000000 --- a/tests/integration/external/drupal7/test_bad_params_integer_headers.php8.php +++ /dev/null @@ -1,49 +0,0 @@ - 22)); diff --git a/tests/integration/external/drupal7/test_bad_params_null_headers.php b/tests/integration/external/drupal7/test_bad_params_null_headers.php index fe9463431..bfb431e9b 100644 --- a/tests/integration/external/drupal7/test_bad_params_null_headers.php +++ b/tests/integration/external/drupal7/test_bad_params_null_headers.php @@ -14,9 +14,6 @@ /*SKIPIF =")) { - die("skip: PHP >= 8.0 not supported\n"); -} */ /*EXPECT_METRICS @@ -35,11 +32,6 @@ [{"name":"OtherTransactionTotalTime"}, [1, "??", "??", "??", "??", "??"]], [{"name":"OtherTransactionTotalTime/php__FILE__"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Supportability/framework/Drupal/forced"}, [1, 0, 0, 0, 0, 0]], - [{"name":"External/127.0.0.1/all"}, [1, "??", "??", "??", "??", "??"]], - [{"name":"External/all"}, [1, "??", "??", "??", "??", "??"]], - [{"name":"External/allOther"}, [1, "??", "??", "??", "??", "??"]], - [{"name":"External/127.0.0.1/all", - "scope":"OtherTransaction/php__FILE__"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Supportability/Logging/LocalDecorating/PHP/disabled"}, [1, "??", "??", "??", "??", "??"]] ] ] diff --git a/tests/integration/external/drupal7/test_bad_params_null_headers.php8.php b/tests/integration/external/drupal7/test_bad_params_null_headers.php8.php deleted file mode 100644 index 1189821b1..000000000 --- a/tests/integration/external/drupal7/test_bad_params_null_headers.php8.php +++ /dev/null @@ -1,49 +0,0 @@ - NULL)); From d03e5b2cfa1f7ea24e0a8a23baccffcaa714b6c5 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Wed, 17 Apr 2024 10:51:06 -0600 Subject: [PATCH 4/5] fix tests --- .../test_bad_params_integer_headers.php | 8 +++ .../test_bad_params_integer_headers.php8.php | 50 +++++++++++++++++++ .../drupal7/test_bad_params_null_headers.php | 8 +++ .../test_bad_params_null_headers.php8.php | 50 +++++++++++++++++++ 4 files changed, 116 insertions(+) create mode 100644 tests/integration/external/drupal7/test_bad_params_integer_headers.php8.php create mode 100644 tests/integration/external/drupal7/test_bad_params_null_headers.php8.php diff --git a/tests/integration/external/drupal7/test_bad_params_integer_headers.php b/tests/integration/external/drupal7/test_bad_params_integer_headers.php index 27beebb0c..1f0aec4c8 100644 --- a/tests/integration/external/drupal7/test_bad_params_integer_headers.php +++ b/tests/integration/external/drupal7/test_bad_params_integer_headers.php @@ -14,6 +14,9 @@ /*SKIPIF =")) { + die("skip: PHP >= 8.0 not supported\n"); +} */ /*EXPECT_METRICS @@ -32,6 +35,11 @@ [{"name":"OtherTransactionTotalTime"}, [1, "??", "??", "??", "??", "??"]], [{"name":"OtherTransactionTotalTime/php__FILE__"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Supportability/framework/Drupal/forced"}, [1, 0, 0, 0, 0, 0]], + [{"name":"External/127.0.0.1/all"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"External/all"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"External/allOther"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"External/127.0.0.1/all", + "scope":"OtherTransaction/php__FILE__"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Supportability/Logging/LocalDecorating/PHP/disabled"}, [1, "??", "??", "??", "??", "??"]] ] ] diff --git a/tests/integration/external/drupal7/test_bad_params_integer_headers.php8.php b/tests/integration/external/drupal7/test_bad_params_integer_headers.php8.php new file mode 100644 index 000000000..f1fba9355 --- /dev/null +++ b/tests/integration/external/drupal7/test_bad_params_integer_headers.php8.php @@ -0,0 +1,50 @@ + 22)); diff --git a/tests/integration/external/drupal7/test_bad_params_null_headers.php b/tests/integration/external/drupal7/test_bad_params_null_headers.php index bfb431e9b..fe9463431 100644 --- a/tests/integration/external/drupal7/test_bad_params_null_headers.php +++ b/tests/integration/external/drupal7/test_bad_params_null_headers.php @@ -14,6 +14,9 @@ /*SKIPIF =")) { + die("skip: PHP >= 8.0 not supported\n"); +} */ /*EXPECT_METRICS @@ -32,6 +35,11 @@ [{"name":"OtherTransactionTotalTime"}, [1, "??", "??", "??", "??", "??"]], [{"name":"OtherTransactionTotalTime/php__FILE__"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Supportability/framework/Drupal/forced"}, [1, 0, 0, 0, 0, 0]], + [{"name":"External/127.0.0.1/all"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"External/all"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"External/allOther"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"External/127.0.0.1/all", + "scope":"OtherTransaction/php__FILE__"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Supportability/Logging/LocalDecorating/PHP/disabled"}, [1, "??", "??", "??", "??", "??"]] ] ] diff --git a/tests/integration/external/drupal7/test_bad_params_null_headers.php8.php b/tests/integration/external/drupal7/test_bad_params_null_headers.php8.php new file mode 100644 index 000000000..9cde3d021 --- /dev/null +++ b/tests/integration/external/drupal7/test_bad_params_null_headers.php8.php @@ -0,0 +1,50 @@ + NULL)); From 8b2ec4b36696f78536b148a97f2ba2572255808a Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Wed, 17 Apr 2024 11:25:09 -0600 Subject: [PATCH 5/5] fix tests --- .../drupal/test_module_invoke_all.php | 3 - .../drupal/test_module_invoke_all.php8.php | 91 ------------------- 2 files changed, 94 deletions(-) delete mode 100644 tests/integration/frameworks/drupal/test_module_invoke_all.php8.php diff --git a/tests/integration/frameworks/drupal/test_module_invoke_all.php b/tests/integration/frameworks/drupal/test_module_invoke_all.php index c9b70e1cf..45ca084c8 100644 --- a/tests/integration/frameworks/drupal/test_module_invoke_all.php +++ b/tests/integration/frameworks/drupal/test_module_invoke_all.php @@ -15,9 +15,6 @@ /*SKIPIF =")) { - die("skip: PHP >= 8.0 uses other test\n"); -} */ /*EXPECT diff --git a/tests/integration/frameworks/drupal/test_module_invoke_all.php8.php b/tests/integration/frameworks/drupal/test_module_invoke_all.php8.php deleted file mode 100644 index fc694e039..000000000 --- a/tests/integration/frameworks/drupal/test_module_invoke_all.php8.php +++ /dev/null @@ -1,91 +0,0 @@ -