From 5a897c8ae58f37aeb32bd277bc1299f4744857fb Mon Sep 17 00:00:00 2001 From: Michael Fulbright <89205663+mfulb@users.noreply.github.com> Date: Mon, 21 Apr 2025 14:39:33 -0400 Subject: [PATCH 1/4] chore: Bump to 11.9.0 (#1054) --- VERSION | 2 +- axiom/nr_version.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/VERSION b/VERSION index 897063bb3..ba9aff72c 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -11.8.0 +11.9.0 diff --git a/axiom/nr_version.c b/axiom/nr_version.c index e81165a44..902f25ba4 100644 --- a/axiom/nr_version.c +++ b/axiom/nr_version.c @@ -49,8 +49,9 @@ * emerald 13Jan2025 (11.5) * fluorite 18Feb2025 (11.6) * gaspeite 19Mar2025 (11.7) + * hiddenite 21Apr2025 (11.8) */ -#define NR_CODENAME "hiddenite" +#define NR_CODENAME "indicolite" const char* nr_version(void) { return NR_STR2(NR_VERSION); From f6b2914634fea8c9354fc59016847bf56dbbb31a Mon Sep 17 00:00:00 2001 From: Michal Nowacki Date: Thu, 8 May 2025 12:43:51 -0400 Subject: [PATCH 2/4] refactor(agent): optimize user function instrumentation lookup for PHPs 8.0+ (#1042) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reduce agent’s CPU overhead when observer API is used to hook into Zend Engine. This is achieved by: * Simplifying the checks the agent needs to perform to determine if special instrumentation for user function is provided. Before this change, on every function execution the agent used a hashmap to check if special instrumentation for user function is provided. However, when the observer API is used to hook into Zend Engine, this can be done only once per user function execution at the time when the user function is executed for the first time. * Reducing the number of times the name of the file is compared with the list of ‘magic files’ to detect package and/or framework used. Before this change on every file execution the agent scanned the list of magic files twice. However, when the observer API is used to hook into Zend Engine, this can be done only once per file execution. --------- Co-authored-by: Amber Sistla --- agent/Makefile.frag | 1 + agent/config.m4 | 1 + agent/fw_drupal8.c | 7 +- agent/fw_laminas3.c | 2 +- agent/fw_laravel.c | 4 +- agent/lib_php_amqplib.c | 24 - agent/php_execute.c | 47 +- agent/php_execute.h | 15 + agent/php_globals.h | 6 +- agent/php_minit.c | 21 +- agent/php_observer.c | 33 ++ agent/php_rinit.c | 5 +- agent/php_user_instrument.c | 186 +++++-- agent/php_user_instrument.h | 13 +- agent/php_user_instrument_hashmap.c | 4 +- agent/php_user_instrument_hashmap.h | 4 +- agent/php_user_instrument_hashmap_key.h | 4 +- agent/php_user_instrument_wraprec_hashmap.c | 488 ++++++++++++++++++ agent/php_user_instrument_wraprec_hashmap.h | 20 + agent/tests/test_txn.c | 7 + agent/tests/test_user_instrument.c | 41 +- agent/tests/test_user_instrument_hashmap.c | 4 +- .../test_user_instrument_wraprec_hashmap.c | 84 +++ .../add_custom_tracer/test_invalid_input.php | 98 ++++ .../frameworks/drupal/mock_page_cache_get.php | 1 - ...ith.php => test_invoke_all_with.php74.php} | 4 +- .../drupal/test_invoke_all_with.php8.php | 147 ++++++ .../frameworks/laravel/mock_http_options.php | 3 - tests/integration/lang/test_attributes.php | 122 +++++ .../lang/trampoline/test_trampoline_00.php | 39 ++ .../lang/trampoline/test_trampoline_01.php | 47 ++ .../lang/trampoline/test_trampoline_02.php | 115 +++++ ..._tt_detail_off_with_custom_wrapper_api.php | 99 ++++ ..._detail_off_with_custom_wrapper_config.php | 98 ++++ .../test_tt_detail_off_with_error.php | 115 +++++ .../test_tt_detail_off_with_exception.php | 115 +++++ .../test_tt_detail_off_with_long_function.php | 62 +++ .../test_tt_detail_on_with_custom_wrapper.php | 110 ++++ .../test_tt_detail_on_with_long_function.php | 104 ++++ .../tt_detail/tt_detail_helper.inc | 9 + 40 files changed, 2194 insertions(+), 115 deletions(-) create mode 100644 agent/php_user_instrument_wraprec_hashmap.c create mode 100644 agent/php_user_instrument_wraprec_hashmap.h create mode 100644 agent/tests/test_user_instrument_wraprec_hashmap.c create mode 100644 tests/integration/api/add_custom_tracer/test_invalid_input.php rename tests/integration/frameworks/drupal/{test_invoke_all_with.php => test_invoke_all_with.php74.php} (98%) create mode 100644 tests/integration/frameworks/drupal/test_invoke_all_with.php8.php create mode 100644 tests/integration/lang/test_attributes.php create mode 100644 tests/integration/lang/trampoline/test_trampoline_00.php create mode 100644 tests/integration/lang/trampoline/test_trampoline_01.php create mode 100644 tests/integration/lang/trampoline/test_trampoline_02.php create mode 100644 tests/integration/span_events/tt_detail/test_tt_detail_off_with_custom_wrapper_api.php create mode 100644 tests/integration/span_events/tt_detail/test_tt_detail_off_with_custom_wrapper_config.php create mode 100644 tests/integration/span_events/tt_detail/test_tt_detail_off_with_error.php create mode 100644 tests/integration/span_events/tt_detail/test_tt_detail_off_with_exception.php create mode 100644 tests/integration/span_events/tt_detail/test_tt_detail_off_with_long_function.php create mode 100644 tests/integration/span_events/tt_detail/test_tt_detail_on_with_custom_wrapper.php create mode 100644 tests/integration/span_events/tt_detail/test_tt_detail_on_with_long_function.php create mode 100644 tests/integration/span_events/tt_detail/tt_detail_helper.inc diff --git a/agent/Makefile.frag b/agent/Makefile.frag index 2bfc562a8..7d6a4f0d1 100644 --- a/agent/Makefile.frag +++ b/agent/Makefile.frag @@ -117,6 +117,7 @@ TEST_BINARIES = \ tests/test_txn_private \ tests/test_user_instrument \ tests/test_user_instrument_hashmap \ + tests/test_user_instrument_wraprec_hashmap \ tests/test_zval .PHONY: unit-tests diff --git a/agent/config.m4 b/agent/config.m4 index 19785b2a3..03a386992 100644 --- a/agent/config.m4 +++ b/agent/config.m4 @@ -220,6 +220,7 @@ if test "$PHP_NEWRELIC" = "yes"; then php_pdo_mysql.c php_pdo_pgsql.c php_pgsql.c php_psr7.c php_redis.c \ php_rinit.c php_rshutdown.c php_samplers.c php_stack.c \ php_stacked_segment.c php_txn.c php_user_instrument.c \ + php_user_instrument_wraprec_hashmap.c \ php_user_instrument_hashmap.c php_vm.c php_wrapper.c" FRAMEWORKS="fw_cakephp.c fw_codeigniter.c fw_drupal8.c \ fw_drupal.c fw_drupal_common.c fw_joomla.c fw_kohana.c \ diff --git a/agent/fw_drupal8.c b/agent/fw_drupal8.c index 1ed5d2fcb..27eafe6bc 100644 --- a/agent/fw_drupal8.c +++ b/agent/fw_drupal8.c @@ -140,6 +140,7 @@ static void nr_drupal8_add_method_callback_before_after_clean( nrspecialfn_t after_callback, nrspecialfn_t clean_callback) { zend_function* function = NULL; + char* methodLC = NULL; if (NULL == ce) { nrl_verbosedebug(NRL_FRAMEWORK, "Drupal 8: got NULL class entry in %s", @@ -147,7 +148,9 @@ static void nr_drupal8_add_method_callback_before_after_clean( return; } - function = nr_php_find_class_method(ce, method); + methodLC = nr_string_to_lowercase(method); + function = nr_php_find_class_method(ce, methodLC); + nr_free(methodLC); if (NULL == function) { nrl_verbosedebug(NRL_FRAMEWORK, "Drupal 8+: cannot get zend_function entry for %.*s::%.*s", @@ -640,7 +643,7 @@ NR_PHP_WRAPPER(nr_drupal8_module_handler) { #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \ && !defined OVERWRITE_ZEND_EXECUTE_DATA nr_drupal8_add_method_callback_before_after_clean( - ce, NR_PSTR("invokeallwith"), nr_drupal94_invoke_all_with, + ce, NR_PSTR("invokeAllWith"), nr_drupal94_invoke_all_with, nr_drupal94_invoke_all_with_after, nr_drupal94_invoke_all_with_clean); #else nr_drupal8_add_method_callback(ce, NR_PSTR("invokeallwith"), diff --git a/agent/fw_laminas3.c b/agent/fw_laminas3.c index a5d27de94..a57ce7286 100644 --- a/agent/fw_laminas3.c +++ b/agent/fw_laminas3.c @@ -155,7 +155,7 @@ void nr_laminas3_enable(TSRMLS_D) { */ nr_php_wrap_user_function( - NR_PSTR("Laminas\\Router\\HTTP\\RouteMatch::setMatchedRouteName"), + NR_PSTR("Laminas\\Router\\Http\\RouteMatch::setMatchedRouteName"), nr_laminas3_name_the_wt TSRMLS_CC); nr_php_wrap_user_function( NR_PSTR("Laminas\\Router\\RouteMatch::setMatchedRouteName"), diff --git a/agent/fw_laravel.c b/agent/fw_laravel.c index bbac65b20..29621e37d 100644 --- a/agent/fw_laravel.c +++ b/agent/fw_laravel.c @@ -1232,10 +1232,10 @@ 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_PSTR("Illuminate\\Console\\Application::doRun"), + NR_PSTR("Symfony\\Component\\Console\\Application::doRun"), nr_laravel_console_application_dorun, NULL, NULL); #else - nr_php_wrap_user_function(NR_PSTR("Illuminate\\Console\\Application::doRun"), + nr_php_wrap_user_function(NR_PSTR("Symfony\\Component\\Console\\Application::doRun"), nr_laravel_console_application_dorun TSRMLS_CC); #endif /* diff --git a/agent/lib_php_amqplib.c b/agent/lib_php_amqplib.c index 5bc4bbfa2..4750c94bd 100644 --- a/agent/lib_php_amqplib.c +++ b/agent/lib_php_amqplib.c @@ -81,29 +81,6 @@ * needed to work with MQ_BROKER. */ -/* - * Purpose : Ensures the php-amqplib instrumentation gets wrapped. - * - * Params : None - * - * Returns : None - */ -static void nr_php_amqplib_ensure_class() { - int result = FAILURE; - zend_class_entry* class_entry = NULL; - - class_entry = nr_php_find_class("phpamqplib\\channel\\amqpchannel"); - if (NULL == class_entry) { - result = zend_eval_stringl( - NR_PSTR("class_exists('PhpAmqpLib\\Channel\\AMQPChannel');"), NULL, - "nr_php_amqplib_class_exists_channel_amqpchannel"); - } - /* - * We don't need to check anything else at this point. If this fails, there's - * nothing else we can do anyway. - */ -} - /* * Version information will be pulled from PhpAmqpLib\\Package::VERSION * nr_php_amqplib_handle_version will automatically load the class if it isn't @@ -810,7 +787,6 @@ void nr_php_amqplib_enable() { /* Extract the version */ nr_php_amqplib_handle_version(); - nr_php_amqplib_ensure_class(); #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO /* less than PHP8.0 */ nr_php_wrap_user_function_before_after_clean( diff --git a/agent/php_execute.c b/agent/php_execute.c index 41c430d11..df3d86c71 100644 --- a/agent/php_execute.c +++ b/agent/php_execute.c @@ -95,7 +95,6 @@ static void nr_php_show_exec_return(NR_EXECUTE_PROTO TSRMLS_DC); static int nr_php_show_exec_indentation(TSRMLS_D); -static void nr_php_show_exec(NR_EXECUTE_PROTO TSRMLS_DC); /* * Purpose: Enable monitoring on specific functions in the framework. @@ -594,11 +593,11 @@ static int nr_php_show_exec_indentation(TSRMLS_D) { * Note that this function doesn't handle internal functions, and will crash if * you give it one. */ -static void nr_php_show_exec(NR_EXECUTE_PROTO TSRMLS_DC) { +void nr_php_show_exec(const char* context, NR_EXECUTE_PROTO TSRMLS_DC) { char argstr[NR_EXECUTE_DEBUG_STRBUFSZ]; const char* filename = nr_php_op_array_file_name(NR_OP_ARRAY); const char* function_name = nr_php_op_array_function_name(NR_OP_ARRAY); - + const char* ctx = context ? context : "execute"; argstr[0] = '\0'; if (NR_OP_ARRAY->scope) { @@ -608,12 +607,13 @@ static void nr_php_show_exec(NR_EXECUTE_PROTO TSRMLS_DC) { nr_show_execute_params(NR_EXECUTE_ORIG_ARGS, argstr TSRMLS_CC); nrl_verbosedebug( NRL_AGENT, - "execute: %.*s scope={%.*s} function={" NRP_FMT_UQ + NRP_FMT_UQ ": %.*s scope={%.*s} function={" NRP_FMT_UQ "}" " params={" NRP_FMT_UQ "}" " %.5s" "@ " NRP_FMT_UQ ":%d", + NRP_SHOW_EXEC_CONTEXT(ctx), nr_php_show_exec_indentation(TSRMLS_C), nr_php_indentation_spaces, NRSAFELEN(nr_php_class_entry_name_length(NR_OP_ARRAY->scope)), nr_php_class_entry_name(NR_OP_ARRAY->scope), @@ -631,12 +631,13 @@ static void nr_php_show_exec(NR_EXECUTE_PROTO TSRMLS_DC) { nr_show_execute_params(NR_EXECUTE_ORIG_ARGS, argstr TSRMLS_CC); nrl_verbosedebug( NRL_AGENT, - "execute: %.*s function={" NRP_FMT_UQ + NRP_FMT_UQ ": %.*s function={" NRP_FMT_UQ "}" " params={" NRP_FMT_UQ "}" " %.5s" "@ " NRP_FMT_UQ ":%d", + NRP_SHOW_EXEC_CONTEXT(ctx), nr_php_show_exec_indentation(TSRMLS_C), nr_php_indentation_spaces, NRP_PHP(function_name), NRP_ARGSTR(argstr), #if ZEND_MODULE_API_NO < ZEND_7_4_X_API_NO @@ -649,16 +650,17 @@ static void nr_php_show_exec(NR_EXECUTE_PROTO TSRMLS_DC) { /* * file */ - nrl_verbosedebug(NRL_AGENT, "execute: %.*s file={" NRP_FMT "}", + nrl_verbosedebug(NRL_AGENT, NRP_FMT_UQ ": %.*s file={" NRP_FMT "}", + NRP_SHOW_EXEC_CONTEXT(ctx), nr_php_show_exec_indentation(TSRMLS_C), nr_php_indentation_spaces, NRP_FILENAME(filename)); } else { /* * unknown */ - nrl_verbosedebug(NRL_AGENT, "execute: %.*s ?", - nr_php_show_exec_indentation(TSRMLS_C), - nr_php_indentation_spaces); + nrl_verbosedebug(NRL_AGENT, NRP_FMT_UQ ": %.*s ?", + NRP_SHOW_EXEC_CONTEXT(ctx), + nr_php_show_exec_indentation(TSRMLS_C), nr_php_indentation_spaces); } } @@ -982,7 +984,7 @@ static void nr_php_user_instrumentation_from_file(const char* filename, */ #define METRIC_NAME_MAX_LEN 512 -static void nr_php_execute_file(const zend_op_array* op_array, +void nr_php_execute_file(const zend_op_array* op_array, NR_EXECUTE_PROTO TSRMLS_DC) { const char* filename = nr_php_op_array_file_name(op_array); size_t filename_len = nr_php_op_array_file_name_len(op_array); @@ -1007,7 +1009,9 @@ static void nr_php_execute_file(const zend_op_array* op_array, return; } +#if ZEND_MODULE_API_NO < ZEND_8_0_X_API_NO nr_php_add_user_instrumentation(TSRMLS_C); +#endif } /* @@ -1519,7 +1523,7 @@ static void nr_php_execute_enabled(NR_EXECUTE_PROTO TSRMLS_DC) { static void nr_php_execute_show(NR_EXECUTE_PROTO TSRMLS_DC) { if (nrunlikely(NR_PHP_PROCESS_GLOBALS(special_flags).show_executes)) { - nr_php_show_exec(NR_EXECUTE_ORIG_ARGS TSRMLS_CC); + nr_php_show_exec("execute", NR_EXECUTE_ORIG_ARGS TSRMLS_CC); } nr_php_execute_enabled(NR_EXECUTE_ORIG_ARGS TSRMLS_CC); @@ -1905,16 +1909,7 @@ static void nr_php_instrument_func_begin(NR_EXECUTE_PROTO) { NRTXNGLOBAL(execute_count) += 1; txn_start_time = nr_txn_start_time(NRPRG(txn)); - /* - * Handle here, but be aware the classes might not be loaded yet. - */ - if (nrunlikely(OP_ARRAY_IS_A_FILE(NR_OP_ARRAY))) { - const char* filename = nr_php_op_array_file_name(NR_OP_ARRAY); - size_t filename_len = nr_php_op_array_file_name_len(NR_OP_ARRAY); - nr_execute_handle_framework(all_frameworks, num_all_frameworks, - filename, filename_len TSRMLS_CC); - return; - } + if (NULL != NRPRG(cufa_callback) && NRPRG(check_cufa)) { /* * For PHP 7+, call_user_func_array() is flattened into an inline by @@ -2000,14 +1995,6 @@ static void nr_php_instrument_func_end(NR_EXECUTE_PROTO) { } txn_start_time = nr_txn_start_time(NRPRG(txn)); - /* - * Let's get the framework info. - */ - if (nrunlikely(OP_ARRAY_IS_A_FILE(NR_OP_ARRAY))) { - nr_php_execute_file(NR_OP_ARRAY, NR_EXECUTE_ORIG_ARGS TSRMLS_CC); - return; - } - /* * Get the current segment and return if null. */ @@ -2157,7 +2144,7 @@ void nr_php_observer_fcall_begin(zend_execute_data* execute_data) { int show_executes = NR_PHP_PROCESS_GLOBALS(special_flags).show_executes; if (nrunlikely(show_executes)) { - nr_php_show_exec(NR_EXECUTE_ORIG_ARGS); + nr_php_show_exec("execute", NR_EXECUTE_ORIG_ARGS); } nr_php_instrument_func_begin(NR_EXECUTE_ORIG_ARGS); diff --git a/agent/php_execute.h b/agent/php_execute.h index dd446a594..c92ec6e3d 100644 --- a/agent/php_execute.h +++ b/agent/php_execute.h @@ -27,6 +27,21 @@ #define OP_ARRAY_IS_METHOD(OP, FNAME) \ (0 == nr_strcmp(nr_php_op_array_function_name(OP), (FNAME))) +extern void nr_php_execute_file(const zend_op_array* op_array, + NR_EXECUTE_PROTO TSRMLS_DC); + +/* + * Purpose: Log information about the execute data in a given execution + * context - either 'execute' (zend_execute) or 'observe' (fcall_init). + * Only first 8 characters of the context are printed. + * + * Caveat: This function doesn't handle internal functions, and will crash if + * you give it one. + */ +extern void nr_php_show_exec(const char*, NR_EXECUTE_PROTO TSRMLS_DC); +/* Limit length of execution context printed in the log file to 8 characters */ +#define NRP_SHOW_EXEC_CONTEXT(C) 8, NRSAFESTR(C) + /* * Purpose: Look through the PHP symbol table for special names or symbols * that provide additional hints that a specific framework has been loaded. diff --git a/agent/php_globals.h b/agent/php_globals.h index c73b93e43..ff9a9824f 100644 --- a/agent/php_globals.h +++ b/agent/php_globals.h @@ -49,11 +49,11 @@ typedef struct _nrphpglobals_t { * variable with the key `NEW_RELIC_LABELS` */ #if ZEND_MODULE_API_NO >= ZEND_8_1_X_API_NO /* PHP 8.1+ */ zend_long zend_offset; /* Zend extension offset */ - zend_long - zend_op_array_offset; /* Zend extension op_array to modify reserved */ #else int zend_offset; /* Zend extension offset */ - int zend_op_array_offset; /* Zend extension op_array to modify reserved */ +#endif +#if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO /* PHP 8.0+ */ + int op_array_extension_handle; /* Zend op_array extension handle to attach agent's data to function */ #endif int done_instrumentation; /* Set to true if we have installed instrumentation handlers */ diff --git a/agent/php_minit.c b/agent/php_minit.c index 7cf4f3e8e..08f5a02c5 100644 --- a/agent/php_minit.c +++ b/agent/php_minit.c @@ -19,6 +19,7 @@ #include "php_internal_instrument.h" #include "php_samplers.h" #include "php_user_instrument.h" +#include "php_user_instrument_wraprec_hashmap.h" #include "php_vm.h" #include "php_wrapper.h" #include "fw_laravel.h" @@ -491,6 +492,16 @@ PHP_MINIT_FUNCTION(newrelic) { */ nr_php_generate_internal_wrap_records(); +#if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO + /* + * The user function wraprec hashmap must be initialized before INI processing + * because INI processing adds wraprecs: + * - newrelic.webtransaction.name.functions + * - newrelic.transaction_tracer.custom + */ + nr_php_user_instrument_wraprec_hashmap_init(); +#endif + nr_php_register_ini_entries(module_number TSRMLS_CC); if (0 == NR_PHP_PROCESS_GLOBALS(enabled)) { @@ -720,8 +731,16 @@ PHP_MINIT_FUNCTION(newrelic) { nr_php_set_opcode_handlers(); nrl_debug(NRL_INIT, "MINIT processing done"); -#if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO /* PHP 7.4+ */ +#if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO /* PHP 8.0+ */ NR_PHP_PROCESS_GLOBALS(zend_offset) = zend_get_resource_handle(dummy); + NR_PHP_PROCESS_GLOBALS(op_array_extension_handle) = zend_get_op_array_extension_handle("newrelic"); +#if ZEND_MODULE_API_NO >= ZEND_8_4_X_API_NO /* PHP 8.4+ */ + /* When observer API is used by an extension, both handles (for user + * and internal functions) must be initialized, even when one of them + * is not used (as in our case). Observer API was changed in PHP 8.4. + * For more details see: https://github.com/php/php-src/pull/14252 */ + (void) zend_get_internal_function_extension_handle("newrelic"); +#endif #else NR_PHP_PROCESS_GLOBALS(zend_offset) = zend_get_resource_handle(&dummy); #endif diff --git a/agent/php_observer.c b/agent/php_observer.c index 5719be38e..01f84b08f 100644 --- a/agent/php_observer.c +++ b/agent/php_observer.c @@ -23,6 +23,7 @@ #include "php_observer.h" #include "php_samplers.h" #include "php_user_instrument.h" +#include "php_user_instrument_wraprec_hashmap.h" #include "php_vm.h" #include "php_wrapper.h" #include "fw_laravel.h" @@ -82,6 +83,38 @@ static zend_observer_fcall_handlers nr_php_fcall_register_handlers( || (ZEND_INTERNAL_FUNCTION == execute_data->func->type)) { return handlers; } + + if (nrunlikely(NR_PHP_PROCESS_GLOBALS(special_flags).show_executes)) { + nr_php_show_exec("observe", execute_data, NULL); + } + + if (OP_ARRAY_IS_A_FILE(NR_OP_ARRAY)) { + /* + * Let's get the framework info. + */ + nr_php_execute_file(NR_OP_ARRAY, execute_data, NULL TSRMLS_CC); + return handlers; + } + + // The function cache slots are not available if the function is a trampoline + if (execute_data->func->op_array.fn_flags & ZEND_ACC_CALL_VIA_TRAMPOLINE) { + if (nrl_should_print(NRL_VERBOSEDEBUG, NRL_INSTRUMENT)) { + char* name = nr_php_function_debug_name(execute_data->func); + nrl_verbosedebug(NRL_INSTRUMENT, "%s - %s is a trampoline function", + __func__, NRSAFESTR(name)); + nr_free(name); + } + return handlers; + } + + if (!ZEND_OP_ARRAY_EXTENSION(NR_OP_ARRAY, NR_PHP_PROCESS_GLOBALS(op_array_extension_handle))) { + zend_string* func_name = NR_OP_ARRAY->function_name; + zend_string* scope_name = OP_ARRAY_IS_A_METHOD(NR_OP_ARRAY)? NR_OP_ARRAY->scope->name : NULL; + nruserfn_t* wr = nr_php_user_instrument_wraprec_hashmap_get(func_name, scope_name); + // store the wraprec in the op_array extension for the duration of the request for later lookup + ZEND_OP_ARRAY_EXTENSION(NR_OP_ARRAY, NR_PHP_PROCESS_GLOBALS(op_array_extension_handle)) = wr; + } + handlers.begin = nr_php_observer_fcall_begin; handlers.end = nr_php_observer_fcall_end; return handlers; diff --git a/agent/php_rinit.c b/agent/php_rinit.c index d704f935e..899f91f74 100644 --- a/agent/php_rinit.c +++ b/agent/php_rinit.c @@ -18,6 +18,7 @@ #include "nr_slowsqls.h" #include "util_logging.h" #include "util_strings.h" +#include "util_syscalls.h" static void nr_php_datastore_instance_destroy( nr_datastore_instance_t* instance) { @@ -53,14 +54,16 @@ PHP_RINIT_FUNCTION(newrelic) { NRPRG(sapi_headers) = NULL; NRPRG(error_group_user_callback).is_set = false; #if ZEND_MODULE_API_NO >= ZEND_7_4_X_API_NO +#if ZEND_MODULE_API_NO == ZEND_7_4_X_API_NO nr_php_init_user_instrumentation(); +#endif #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \ && !defined OVERWRITE_ZEND_EXECUTE_DATA NRPRG(drupal_http_request_segment) = NULL; NRPRG(drupal_http_request_depth) = 0; #endif #else - NRPRG(pid) = getpid(); + NRPRG(pid) = nr_getpid(); NRPRG(user_function_wrappers) = nr_vector_create(64, NULL, NULL); #endif diff --git a/agent/php_user_instrument.c b/agent/php_user_instrument.c index 65b078b71..1673949a2 100644 --- a/agent/php_user_instrument.c +++ b/agent/php_user_instrument.c @@ -7,11 +7,13 @@ #include "php_globals.h" #include "php_user_instrument.h" #include "php_user_instrument_hashmap.h" +#include "php_user_instrument_wraprec_hashmap.h" #include "php_wrapper.h" #include "lib_guzzle_common.h" #include "util_logging.h" #include "util_memory.h" #include "util_strings.h" +#include "util_syscalls.h" /* * The mechanism of zend_try .. zend_catch .. zend_end_try @@ -114,7 +116,66 @@ int nr_zend_call_orig_execute_special(nruserfn_t* wraprec, return zcaught; } -#if ZEND_MODULE_API_NO >= ZEND_7_4_X_API_NO +#if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO +static inline void nr_php_wraprec_lookup_set(nruserfn_t* wr, + zend_function* zf) { + // The function cache slots are not available if the function is a trampoline + if (zf->op_array.fn_flags & ZEND_ACC_CALL_VIA_TRAMPOLINE) { + if (nrl_should_print(NRL_VERBOSEDEBUG, NRL_INSTRUMENT)) { + char* name = nr_php_function_debug_name(zf); + nrl_verbosedebug(NRL_INSTRUMENT, "%s - %s is a trampoline function", + __func__, NRSAFESTR(name)); + nr_free(name); + } + + /* + * Prevent future wrap attempts for performance and to prevent spamming the + * logs with this message. + */ + wr->is_disabled = 1; + return; + } + // for situation when wraprec is added after first execution of the function + // store the wraprec in the op_array extension for the duration of the request for later lookup + // The op_array extension slot for function may not be initialized yet because it is + // initialized only on the first call made to the function in that request. This would + // mean that run_time_cache is NULL and wraprec cannot be stored yet! It will be stored + // on the first call to the function when observer is registered for that function. + zend_init_func_run_time_cache(&zf->op_array); + if (NULL != RUN_TIME_CACHE(&zf->op_array)) { + ZEND_OP_ARRAY_EXTENSION(&zf->op_array, NR_PHP_PROCESS_GLOBALS(op_array_extension_handle)) = wr; + } +} + +static inline nruserfn_t* nr_php_wraprec_lookup_get(zend_function* zf) { + nruserfn_t *wraprec = NULL; + + // The function cache slots are not available if the function is a trampoline + if (zf->op_array.fn_flags & ZEND_ACC_CALL_VIA_TRAMPOLINE) { + if (nrl_should_print(NRL_VERBOSEDEBUG, NRL_INSTRUMENT)) { + char* name = nr_php_function_debug_name(zf); + nrl_verbosedebug(NRL_INSTRUMENT, "%s - %s is a trampoline function", + __func__, NRSAFESTR(name)); + nr_free(name); + } + return NULL; + } + + if (NULL != RUN_TIME_CACHE(&zf->op_array)) { + wraprec = ZEND_OP_ARRAY_EXTENSION(&zf->op_array, NR_PHP_PROCESS_GLOBALS(op_array_extension_handle)); + } + if (NULL != wraprec && wraprec->magic != NR_USERFN_T_MAGIC) { + if (nrl_should_print(NRL_VERBOSEDEBUG, NRL_INSTRUMENT)) { + char* name = nr_php_function_debug_name(zf); + nrl_verbosedebug(NRL_INSTRUMENT, "%s - wraprec for {%s} is invalid", + __func__, NRSAFESTR(name)); + nr_free(name); + } + wraprec = NULL; + } + return wraprec; +} +#elif ZEND_MODULE_API_NO == ZEND_7_4_X_API_NO /* Hashmap with pointers to wraprecs. Some, that are re-usable between requests, * are stored in linked list. These wraprecs are created once per interesting * function detection, and destroyed at module shutdown. Some, that are @@ -164,7 +225,6 @@ void nr_php_init_user_instrumentation(void) { * wrapped). This happens because with new request/transaction php is loading * all new user code. */ -static void nr_php_user_wraprec_destroy(nruserfn_t** wraprec_ptr); static void reset_wraprec(nruserfn_t* wraprec) { nruserfn_t* p = wraprec; nr_php_wraprec_hashmap_key_release(&p->key); @@ -290,10 +350,20 @@ static void nr_php_wrap_user_function_internal(nruserfn_t* wraprec TSRMLS_DC) { nr_php_wrap_zend_function(orig_func, wraprec TSRMLS_CC); } -static nruserfn_t* nr_php_user_wraprec_create(void) { - return (nruserfn_t*)nr_zalloc(sizeof(nruserfn_t)); +nruserfn_t* nr_php_user_wraprec_create(void) { + nruserfn_t* wr = (nruserfn_t*)nr_zalloc(sizeof(nruserfn_t)); + wr->magic = NR_USERFN_T_MAGIC; + return wr; } +#if ZEND_MODULE_API_NO < ZEND_8_0_X_API_NO + +/* This function expects full_name and full_name_len to be validated with + * nr_php_user_instrument_is_name_valid() before being passed in. + * Specifically, it requires that: + * - full_name_len be greater than 0 + * - full_name must not be NULL and must not end with `:` (colon) . */ + static nruserfn_t* nr_php_user_wraprec_create_named(const char* full_name, int full_name_len) { int i; @@ -303,13 +373,6 @@ static nruserfn_t* nr_php_user_wraprec_create_named(const char* full_name, int klass_len; nruserfn_t* wraprec; - if (0 == full_name) { - return 0; - } - if (full_name_len <= 0) { - return 0; - } - name = full_name; name_len = full_name_len; klass = 0; @@ -343,8 +406,9 @@ static nruserfn_t* nr_php_user_wraprec_create_named(const char* full_name, return wraprec; } +#endif -static void nr_php_user_wraprec_destroy(nruserfn_t** wraprec_ptr) { +void nr_php_user_wraprec_destroy(nruserfn_t** wraprec_ptr) { nruserfn_t* wraprec; if (0 == wraprec_ptr) { @@ -365,6 +429,7 @@ static void nr_php_user_wraprec_destroy(nruserfn_t** wraprec_ptr) { nr_realfree((void**)wraprec_ptr); } +#if ZEND_MODULE_API_NO < ZEND_8_0_X_API_NO static int nr_php_user_wraprec_is_match(const nruserfn_t* w1, const nruserfn_t* w2) { if ((0 == w1) && (0 == w2)) { @@ -381,11 +446,42 @@ static int nr_php_user_wraprec_is_match(const nruserfn_t* w1, } return 1; } +#endif + +#if ZEND_MODULE_API_NO > ZEND_7_4_X_API_NO +static nruserfn_t* nr_transient_wraprecs = NULL; /* a singly linked list */ +#else +static nruserfn_t* nr_wrapped_user_functions = NULL; /* a singly linked list */ +#endif static void nr_php_add_custom_tracer_common(nruserfn_t* wraprec) { /* Add the wraprecord to the list. */ +#if ZEND_MODULE_API_NO > ZEND_7_4_X_API_NO + if (wraprec->is_transient) { + /* Transient (unnamed) wraprecs are not added to wraprec hashmap which only stores named + * wraprecs. Keep track of all transient wraprecs so that they can be destroyed at the + * end of the request. */ + wraprec->next = nr_transient_wraprecs; + nr_transient_wraprecs = wraprec; + return; + } +#endif +#if ZEND_MODULE_API_NO == ZEND_7_4_X_API_NO + if (!wraprec->is_transient) { + /* Non-transient wraprecs are added to both the hashmap and linked list. + * At request shutdown, the hashmap will free transients, but leave + * non-transients to be freed when the linked list is disposed of which is at + * module shutdown */ + wraprec->next = nr_wrapped_user_functions; + nr_wrapped_user_functions = wraprec; + return; + } +#endif +#if ZEND_MODULE_API_NO < ZEND_7_4_X_API_NO wraprec->next = nr_wrapped_user_functions; nr_wrapped_user_functions = wraprec; + return; +#endif } #define NR_PHP_UNKNOWN_FUNCTION_NAME "{unknown}" @@ -428,27 +524,40 @@ nruserfn_t* nr_php_add_custom_tracer_callable(zend_function* func TSRMLS_DC) { nr_free(name); nr_php_wrap_zend_function(func, wraprec TSRMLS_CC); -#if ZEND_MODULE_API_NO < ZEND_7_4_X_API_NO nr_php_add_custom_tracer_common(wraprec); -#endif return wraprec; } +static inline bool nr_php_user_instrument_is_name_valid(const char* namestr, + ssize_t namestrlen) { + if (NULL == namestr || namestrlen <= 0) { + return false; + } + + if (':' == namestr[namestrlen - 1]) { + return false; + } + + return true; +} + nruserfn_t* nr_php_add_custom_tracer_named(const char* namestr, size_t namestrlen) { nruserfn_t* wraprec; - nruserfn_t* p; + if (!nr_php_user_instrument_is_name_valid(namestr, namestrlen)) { + return NULL; + } + +#if ZEND_MODULE_API_NO < ZEND_8_0_X_API_NO wraprec = nr_php_user_wraprec_create_named(namestr, namestrlen); if (0 == wraprec) { return 0; } /* Make sure that we are not duplicating an existing wraprecord */ - p = nr_wrapped_user_functions; - - while (0 != p) { + for (nruserfn_t* p = nr_wrapped_user_functions; 0 != p; p = p->next) { if (nr_php_user_wraprec_is_match(p, wraprec)) { nrl_verbosedebug( NRL_INSTRUMENT, @@ -460,19 +569,16 @@ nruserfn_t* nr_php_add_custom_tracer_named(const char* namestr, nr_php_wrap_user_function_internal(p TSRMLS_CC); return p; /* return the wraprec we are duplicating */ } - p = p->next; } - +#else + wraprec = nr_php_user_instrument_wraprec_hashmap_add(namestr, namestrlen); +#endif nrl_verbosedebug( NRL_INSTRUMENT, "adding custom for '" NRP_FMT_UQ "%.5s" NRP_FMT_UQ "'", NRP_PHP(wraprec->classname), (0 == wraprec->classname) ? "" : "::", NRP_PHP(wraprec->funcname)); nr_php_wrap_user_function_internal(wraprec TSRMLS_CC); - /* non-transient wraprecs are added to both the hashmap and linked list. - * At request shutdown, the hashmap will free transients, but leave - * non-transients to be freed when the linked list is disposed of which is at - * module shutdown */ nr_php_add_custom_tracer_common(wraprec); return wraprec; /* return the new wraprec */ @@ -489,7 +595,13 @@ nruserfn_t* nr_php_add_custom_tracer_named(const char* namestr, * */ void nr_php_reset_user_instrumentation(void) { -#if ZEND_MODULE_API_NO >= ZEND_7_4_X_API_NO +#if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO + /* No need to do anything at rshutdown: + * - Observer API takes care of resetting user instrumentation for each request + * - All named wraprecs ever created persist in wraprec hashmap until mshutdown + */ + return; +#elif ZEND_MODULE_API_NO >= ZEND_7_4_X_API_NO // send a metric with the number of transient wrappers if (NULL != user_function_wrappers) { nr_php_wraprec_hashmap_stats_t stats @@ -514,6 +626,15 @@ void nr_php_reset_user_instrumentation(void) { * Remove any transient wraprecs. This must only be called on request shutdown! */ void nr_php_remove_transient_user_instrumentation(void) { +#if ZEND_MODULE_API_NO > ZEND_7_4_X_API_NO + nruserfn_t* p = nr_transient_wraprecs; + while (p) { + nruserfn_t* wraprec = p; + p = wraprec->next; + nr_php_user_wraprec_destroy(&wraprec); + } + nr_transient_wraprecs = NULL; +#endif #if ZEND_MODULE_API_NO < ZEND_7_4_X_API_NO nruserfn_t* p = nr_wrapped_user_functions; nruserfn_t* prev = NULL; @@ -542,6 +663,7 @@ void nr_php_remove_transient_user_instrumentation(void) { * Wrap all the interesting user functions with instrumentation. */ void nr_php_add_user_instrumentation(TSRMLS_D) { +#if ZEND_MODULE_API_NO < ZEND_8_0_X_API_NO nruserfn_t* p = nr_wrapped_user_functions; while (0 != p) { @@ -550,6 +672,7 @@ void nr_php_add_user_instrumentation(TSRMLS_D) { } p = p->next; } +#endif } void nr_php_add_transaction_naming_function(const char* namestr, @@ -596,6 +719,7 @@ void nr_php_remove_exception_function(zend_function* func TSRMLS_DC) { } void nr_php_destroy_user_wrap_records(void) { +#if ZEND_MODULE_API_NO < ZEND_8_0_X_API_NO nruserfn_t* next_user_wraprec; next_user_wraprec = nr_wrapped_user_functions; @@ -607,14 +731,11 @@ void nr_php_destroy_user_wrap_records(void) { } nr_wrapped_user_functions = NULL; +#else + nr_php_user_instrument_wraprec_hashmap_destroy(); +#endif } -/* - * This is a similar list, but for the dynamically added user-defined functions - * rather than the statically defined internal/binary functions above. - */ -nruserfn_t* nr_wrapped_user_functions = 0; - void nr_php_user_function_add_declared_callback(const char* namestr, int namestrlen, nruserfn_declared_t callback @@ -635,6 +756,9 @@ void nr_php_user_function_add_declared_callback(const char* namestr, #if ZEND_MODULE_API_NO >= ZEND_7_4_X_API_NO nruserfn_t* nr_php_get_wraprec(zend_function* zf) { + if (nrunlikely(NULL == zf)) { + return NULL; + } return nr_php_wraprec_lookup_get(zf); } #else diff --git a/agent/php_user_instrument.h b/agent/php_user_instrument.h index f71889751..1f6fb52ce 100644 --- a/agent/php_user_instrument.h +++ b/agent/php_user_instrument.h @@ -14,6 +14,7 @@ #include "php_user_instrument_hashmap_key.h" struct _nruserfn_t; +#define NR_USERFN_T_MAGIC 0x6e72757372666e74ULL /* "nrusrfnt" */ /* * This is an unused structure that is used to ensure that a bare return won't @@ -35,9 +36,10 @@ typedef void (*nruserfn_declared_t)(TSRMLS_D); * and so the strings must be discarded in nr_php_user_wraprec_destroy; */ typedef struct _nruserfn_t { + uint64_t magic; /* memory marker */ struct _nruserfn_t* next; /* singly linked list next pointer */ -#if ZEND_MODULE_API_NO >= ZEND_7_4_X_API_NO +#if ZEND_MODULE_API_NO == ZEND_7_4_X_API_NO /* wraprec hashmap key */ nr_php_wraprec_hashmap_key_t key; #endif @@ -110,9 +112,10 @@ typedef struct _nruserfn_t { #endif } nruserfn_t; -extern nruserfn_t* nr_wrapped_user_functions; /* a singly linked list */ - -#if ZEND_MODULE_API_NO >= ZEND_7_4_X_API_NO +#if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO +/* PHPs 8.0+ use ZEND_OP_ARRAY_EXTENSION to store wraprecs and Observer API to install them */ +extern nruserfn_t* nr_php_get_wraprec(zend_function* zf); +#elif ZEND_MODULE_API_NO == ZEND_7_4_X_API_NO /* * Purpose : Init user instrumentation. This must only be called on request @@ -217,6 +220,8 @@ extern int nr_zend_call_oapi_special_clean(nruserfn_t* wraprec, nr_segment_t* segment, NR_EXECUTE_PROTO); #endif +extern nruserfn_t* nr_php_user_wraprec_create(void); +extern void nr_php_user_wraprec_destroy(nruserfn_t** wraprec_ptr); /* * Purpose : Destroy all user instrumentation records, freeing * associated memory. diff --git a/agent/php_user_instrument_hashmap.c b/agent/php_user_instrument_hashmap.c index 3ceec61e1..dde758133 100644 --- a/agent/php_user_instrument_hashmap.c +++ b/agent/php_user_instrument_hashmap.c @@ -6,7 +6,7 @@ #include "php_agent.h" #include "php_user_instrument_hashmap.h" -#if ZEND_MODULE_API_NO >= ZEND_7_4_X_API_NO +#if ZEND_MODULE_API_NO == ZEND_7_4_X_API_NO typedef struct _nr_wraprecs_bucket { struct _nr_wraprecs_bucket* prev; @@ -342,4 +342,4 @@ int nr_php_wraprec_hashmap_get_into(nr_php_wraprec_hashmap_t* hashmap, return 0; } -#endif \ No newline at end of file +#endif diff --git a/agent/php_user_instrument_hashmap.h b/agent/php_user_instrument_hashmap.h index 7158d153e..383c88248 100644 --- a/agent/php_user_instrument_hashmap.h +++ b/agent/php_user_instrument_hashmap.h @@ -13,7 +13,7 @@ #include "php_includes.h" #include "php_user_instrument.h" -#if ZEND_MODULE_API_NO >= ZEND_7_4_X_API_NO +#if ZEND_MODULE_API_NO == ZEND_7_4_X_API_NO /* * The opaque hashmap type. */ @@ -96,4 +96,4 @@ extern int nr_php_wraprec_hashmap_get_into(nr_php_wraprec_hashmap_t*, nruserfn_t**); #endif -#endif \ No newline at end of file +#endif diff --git a/agent/php_user_instrument_hashmap_key.h b/agent/php_user_instrument_hashmap_key.h index 5d8168c08..8fec78bcd 100644 --- a/agent/php_user_instrument_hashmap_key.h +++ b/agent/php_user_instrument_hashmap_key.h @@ -12,7 +12,7 @@ #include "php_includes.h" -#if ZEND_MODULE_API_NO >= ZEND_7_4_X_API_NO +#if ZEND_MODULE_API_NO == ZEND_7_4_X_API_NO /* * The hashmap key constructed from zend_function metadata */ @@ -46,4 +46,4 @@ extern void nr_php_wraprec_hashmap_key_set(nr_php_wraprec_hashmap_key_t*, extern void nr_php_wraprec_hashmap_key_release(nr_php_wraprec_hashmap_key_t*); #endif -#endif \ No newline at end of file +#endif diff --git a/agent/php_user_instrument_wraprec_hashmap.c b/agent/php_user_instrument_wraprec_hashmap.c new file mode 100644 index 000000000..6aab143c2 --- /dev/null +++ b/agent/php_user_instrument_wraprec_hashmap.c @@ -0,0 +1,488 @@ +/* + * Copyright 2025 New Relic Corporation. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +#include "php_agent.h" +#include "php_user_instrument_wraprec_hashmap.h" +#include "util_logging.h" +#include "util_memory.h" +#include "util_strings.h" + +// ----------------------------------------------------------------------------- +// func hash map + +typedef struct { + bool is_method; + const char* name; + int name_len; + zend_ulong name_hash; +} nr_func_hashmap_key_t; + +typedef struct _nr_func_bucket { + struct _nr_func_bucket* prev; + struct _nr_func_bucket* next; + nr_func_hashmap_key_t* key; + nruserfn_t* wraprec; +} nr_func_bucket_t; + +typedef struct _nr_func_hashmap { + size_t log2_num_buckets; + nr_func_bucket_t** buckets; + size_t elements; +} nr_func_hashmap_t; + +static nr_func_hashmap_t* nr_func_hashmap_create_internal(size_t log2_num_buckets) { + nr_func_hashmap_t* hashmap = NULL; + + if (0 == log2_num_buckets) { + /* + * Encode the default value in one place: namely, here. + */ + log2_num_buckets = 8; + } else if (log2_num_buckets > 24) { + /* + * Basic sanity check: it's extremely unlikely that we'll ever need a + * hashmap for the user function wraprecs that has more than 2^24 buckets. + */ + log2_num_buckets = 24; + } + + hashmap + = (nr_func_hashmap_t*)nr_malloc(sizeof(nr_func_hashmap_t)); + hashmap->log2_num_buckets = log2_num_buckets; + hashmap->buckets = (nr_func_bucket_t**)nr_calloc( + (1 << log2_num_buckets), sizeof(nr_func_bucket_t*)); + hashmap->elements = 0; + + return hashmap; +} + +static size_t nr_func_hashmap_hash_key(size_t log2_num_buckets, nr_func_hashmap_key_t* key) { + return (key->name_hash & ((1 << log2_num_buckets) - 1)); +} + +static bool nr_func_hashmap_key_equals(nr_func_hashmap_key_t* a, nr_func_hashmap_key_t* b) { + return (a->name_hash == b->name_hash) && (a->name_len == b->name_len) + && (0 == nr_strncmp(a->name, b->name, a->name_len)); +} + +static bool nr_func_hashmap_fetch_internal(nr_func_hashmap_t* hashmap, size_t hash, nr_func_hashmap_key_t* key, nr_func_bucket_t** bucket_ptr) { + for (nr_func_bucket_t* bucket = hashmap->buckets[hash]; bucket; bucket = bucket->next) { + if (nr_func_hashmap_key_equals(bucket->key, key)) { + if (bucket_ptr) { + *bucket_ptr = bucket; + } + return true; + } + } + return false; +} + +static nruserfn_t* nr_func_hashmap_add_internal(nr_func_hashmap_t* hashmap, + size_t hash_key, + nr_func_hashmap_key_t* key) { + nr_func_bucket_t* bucket = (nr_func_bucket_t*)nr_malloc(sizeof(nr_func_bucket_t)); + + bucket->prev = NULL; + bucket->next = hashmap->buckets[hash_key]; + bucket->key = (nr_func_hashmap_key_t*)nr_malloc(sizeof(nr_func_hashmap_key_t)); + bucket->key->is_method = key->is_method; + bucket->key->name = nr_strndup(key->name, key->name_len); + bucket->key->name_len = key->name_len; + bucket->key->name_hash = key->name_hash; + bucket->wraprec = nr_php_user_wraprec_create(); + + if (hashmap->buckets[hash_key]) { + hashmap->buckets[hash_key]->prev = bucket; + } + + hashmap->buckets[hash_key] = bucket; + ++hashmap->elements; + + return bucket->wraprec; +} + +static nruserfn_t* nr_func_hashmap_lookup_internal(nr_func_hashmap_t* hashmap, nr_func_hashmap_key_t* key) { + size_t hash; + nr_func_bucket_t* bucket = NULL; + + if (nrunlikely((NULL == hashmap) || (NULL == key))) { + return NULL; + } + + hash = nr_func_hashmap_hash_key(hashmap->log2_num_buckets, key); + if (nr_func_hashmap_fetch_internal(hashmap, hash, key, &bucket)) { + return bucket->wraprec; + } + + return NULL; +} + +static nruserfn_t* nr_func_hashmap_update_internal(nr_func_hashmap_t* hashmap, nr_func_hashmap_key_t* key, bool* created) { + size_t hash; + nr_func_bucket_t* bucket = NULL; + nruserfn_t* wraprec = NULL; + + if (nrunlikely((NULL == hashmap) || (NULL == key))) { + return NULL; + } + + hash = nr_func_hashmap_hash_key(hashmap->log2_num_buckets, key); + if (nr_func_hashmap_fetch_internal(hashmap, hash, key, &bucket)) { + if (created) { + *created = false; + } + return bucket->wraprec; + } + + if (created) { + *created = true; + } + return nr_func_hashmap_add_internal(hashmap, hash, key); +} + +static void nr_func_hashmap_destroy_bucket_internal(nr_func_bucket_t** bucket_ptr) { + nr_func_bucket_t* bucket = *bucket_ptr; + if (NULL != bucket->key) { + nr_free(bucket->key->name); + } + nr_free(bucket->key); + nr_php_user_wraprec_destroy(&bucket->wraprec); + nr_realfree((void**)bucket_ptr); +} + +static void nr_func_hashmap_destroy_internal(nr_func_hashmap_t** hashmap_ptr) { + size_t count; + nr_func_hashmap_t* hashmap = NULL; + size_t i; + + if ((NULL == hashmap_ptr) || (NULL == *hashmap_ptr)) { + return; + } + hashmap = *hashmap_ptr; + + count = (size_t)(1 << hashmap->log2_num_buckets); + for (i = 0; i < count; i++) { + nr_func_bucket_t* bucket = hashmap->buckets[i]; + + while (bucket) { + nr_func_bucket_t* next = bucket->next; + + nr_func_hashmap_destroy_bucket_internal(&bucket); + + bucket = next; + } + } + + nr_free(hashmap->buckets); + nr_realfree((void**)hashmap_ptr); +} + +// ----------------------------------------------------------------------------- +// scope hash map + +typedef struct { + const char* name; + int name_len; + zend_ulong name_hash; +} nr_scope_hashmap_key_t; + +typedef struct _nr_scope_bucket { + struct _nr_scope_bucket* prev; + struct _nr_scope_bucket* next; + nr_scope_hashmap_key_t* key; + nr_func_hashmap_t* scoped_funcs_ht; +} nr_scope_bucket_t; + +typedef struct _nr_scope_hashmap { + size_t log2_num_buckets; + nr_scope_bucket_t** buckets; + size_t elements; +} nr_scope_hashmap_t; + +static nr_scope_hashmap_t* nr_scope_hashmap_create_internal(size_t log2_num_buckets) { + nr_scope_hashmap_t* hashmap = NULL; + + if (0 == log2_num_buckets) { + /* + * Encode the default value in one place: namely, here. + */ + log2_num_buckets = 8; + } else if (log2_num_buckets > 24) { + /* + * Basic sanity check: it's extremely unlikely that we'll ever need a + * hashmap for the user function wraprecs that has more than 2^24 buckets. + */ + log2_num_buckets = 24; + } + + hashmap + = (nr_scope_hashmap_t*)nr_malloc(sizeof(nr_scope_hashmap_t)); + hashmap->log2_num_buckets = log2_num_buckets; + hashmap->buckets = (nr_scope_bucket_t**)nr_calloc( + (1 << log2_num_buckets), sizeof(nr_scope_bucket_t*)); + hashmap->elements = 0; + + return hashmap; +} + +static size_t nr_scope_hashmap_hash_key(size_t log2_num_buckets, nr_scope_hashmap_key_t* key) { + return (key->name_hash & ((1 << log2_num_buckets) - 1)); +} + +static bool nr_scope_hashmap_key_equals(nr_scope_hashmap_key_t* a, nr_scope_hashmap_key_t* b) { + return (a->name_hash == b->name_hash) && (a->name_len == b->name_len) + && (0 == nr_strncmp(a->name, b->name, a->name_len)); +} + +static bool nr_scope_hashmap_fetch_internal(nr_scope_hashmap_t* hashmap, size_t hash, nr_scope_hashmap_key_t* key, nr_scope_bucket_t** bucket_ptr) { + + for (nr_scope_bucket_t* bucket = hashmap->buckets[hash]; bucket; bucket = bucket->next) { + if (nr_scope_hashmap_key_equals(bucket->key, key)) { + if (bucket_ptr) { + *bucket_ptr = bucket; + } + return true; + } + } + return false; +} + +static nr_func_hashmap_t* nr_scope_hashmap_add_internal(nr_scope_hashmap_t* hashmap, + size_t hash_key, + nr_scope_hashmap_key_t* key) { + nr_scope_bucket_t* bucket = (nr_scope_bucket_t*)nr_malloc(sizeof(nr_scope_bucket_t)); + bucket->prev = NULL; + bucket->next = hashmap->buckets[hash_key]; + bucket->key = (nr_scope_hashmap_key_t*)nr_malloc(sizeof(nr_scope_hashmap_key_t)); + bucket->key->name = nr_strndup(key->name, key->name_len); + bucket->key->name_len = key->name_len; + bucket->key->name_hash = key->name_hash; + bucket->scoped_funcs_ht = nr_func_hashmap_create_internal(0); + + if (hashmap->buckets[hash_key]) { + hashmap->buckets[hash_key]->prev = bucket; + } + + hashmap->buckets[hash_key] = bucket; + ++hashmap->elements; + + return bucket->scoped_funcs_ht; +} + +static nr_func_hashmap_t* nr_scope_hashmap_lookup_internal(nr_scope_hashmap_t* hashmap, nr_scope_hashmap_key_t* key) { + size_t hash; + nr_scope_bucket_t* bucket = NULL; + + if (nrunlikely((NULL == hashmap) || (NULL == key))) { + return NULL; + } + + hash = nr_scope_hashmap_hash_key(hashmap->log2_num_buckets, key); + if (nr_scope_hashmap_fetch_internal(hashmap, hash, key, &bucket)) { + return bucket->scoped_funcs_ht; + } + + return NULL; +} + +static nr_func_hashmap_t* nr_scope_hashmap_update_internal(nr_scope_hashmap_t* hashmap, nr_scope_hashmap_key_t* key) { + size_t hash; + nr_scope_bucket_t* bucket = NULL; + + if (nrunlikely((NULL == hashmap) || (NULL == key))) { + return NULL; + } + + hash = nr_scope_hashmap_hash_key(hashmap->log2_num_buckets, key); + if (nr_scope_hashmap_fetch_internal(hashmap, hash, key, &bucket)) { + return bucket->scoped_funcs_ht; + } + + return nr_scope_hashmap_add_internal(hashmap, hash, key); +} + +static void nr_scope_hashmap_destroy_bucket_internal(nr_scope_bucket_t** bucket_ptr) { + nr_scope_bucket_t* bucket = *bucket_ptr; + if (NULL != bucket->key) { + nr_free(bucket->key->name); + } + nr_free(bucket->key); + nr_func_hashmap_destroy_internal(&bucket->scoped_funcs_ht); + nr_realfree((void**)bucket_ptr); +} + +static void nr_scope_hashmap_destroy_internal(nr_scope_hashmap_t** hashmap_ptr) { + size_t count; + nr_scope_hashmap_t* hashmap = NULL; + size_t i; + + if ((NULL == hashmap_ptr) || (NULL == *hashmap_ptr)) { + return; + } + hashmap = *hashmap_ptr; + + count = (size_t)(1 << hashmap->log2_num_buckets); + for (i = 0; i < count; i++) { + nr_scope_bucket_t* bucket = hashmap->buckets[i]; + + while (bucket) { + nr_scope_bucket_t* next = bucket->next; + + nr_scope_hashmap_destroy_bucket_internal(&bucket); + + bucket = next; + } + } + + nr_free(hashmap->buckets); + nr_realfree((void**)hashmap_ptr); +} + +nr_func_hashmap_t* global_funcs_ht = NULL; +nr_scope_hashmap_t* scope_ht = NULL; + +/* This function expects full_name and full_name_len to be validated with + * nr_php_user_instrument_is_name_valid() before being passed in. + * Specifically, it requires that: + * - full_name_len be greater than 0 + * - full_name must not be NULL and must not end with `:` (colon) . */ + +static void nr_php_user_instrument_wraprec_hashmap_name2keys( + nr_func_hashmap_key_t* func, + nr_scope_hashmap_key_t* scope, + const char* full_name, + int full_name_len) { + + func->name = full_name; + func->name_len = full_name_len; + func->name_hash = 0; + scope->name = NULL; + scope->name_len = 0; + scope->name_hash = 0; + + /* If scope::method, then break into two strings */ + for (int i = 0; i < full_name_len; i++) { + if ((':' == full_name[i]) && (':' == full_name[i + 1])) { + func->is_method = true; + scope->name = full_name; + scope->name_len = i; + func->name = full_name + i + 2; + func->name_len = full_name_len - i - 2; + } + } + + if (func->is_method) { + scope->name_hash = zend_hash_func(scope->name, scope->name_len); + } + func->name_hash = zend_hash_func(func->name, func->name_len); +} + +void nr_php_user_instrument_wraprec_hashmap_init(void) { + if (NULL == scope_ht) { + scope_ht = nr_scope_hashmap_create_internal(0); + } + if (NULL == global_funcs_ht) { + global_funcs_ht = nr_func_hashmap_create_internal(0); + } +} + +/* This function expects namestr and namestrlen to be validated with + * nr_php_user_instrument_is_name_valid() before being passed in. + * Specifically, it requires that: + * - namestrlen be greater than 0 + * - namestr must not be NULL and must not end with `:` (colon) . */ + +nruserfn_t* nr_php_user_instrument_wraprec_hashmap_add(const char* namestr, size_t namestrlen) { + nr_scope_hashmap_key_t scope_key = {0}; + nr_func_hashmap_key_t func_key = {0}; + nr_func_hashmap_t* funcs_ht = NULL; + bool is_new_wraprec = false; + nruserfn_t* wraprec = NULL; + + + if (NULL == scope_ht || NULL == global_funcs_ht) { + return NULL; + } + + nr_php_user_instrument_wraprec_hashmap_name2keys(&func_key, &scope_key, namestr, namestrlen); + + if (func_key.is_method) { + funcs_ht = nr_scope_hashmap_update_internal(scope_ht, &scope_key); + } else { + funcs_ht = global_funcs_ht; + } + + if (NULL == funcs_ht) { + return NULL; + } + + wraprec = nr_func_hashmap_update_internal(funcs_ht, &func_key, &is_new_wraprec); + + if (NULL == wraprec) { + return NULL; + } + + if (is_new_wraprec) { + wraprec->funcname = nr_strndup(func_key.name, func_key.name_len); + wraprec->funcnamelen = func_key.name_len; + wraprec->funcnameLC = nr_string_to_lowercase(wraprec->funcname); + if (func_key.is_method) { + wraprec->classname = nr_strndup(scope_key.name, scope_key.name_len); + wraprec->classnamelen = scope_key.name_len; + wraprec->classnameLC = nr_string_to_lowercase(wraprec->classname); + wraprec->is_method = 1; + } + + wraprec->supportability_metric = nr_txn_create_fn_supportability_metric( + wraprec->funcname, wraprec->classname); + } else { + nrl_verbosedebug(NRL_INSTRUMENT, "reusing custom wrapper for '%s'", namestr); + } + + return wraprec; +} + +nruserfn_t* nr_php_user_instrument_wraprec_hashmap_get(zend_string *func_name, zend_string *scope_name) { + nr_scope_hashmap_key_t scope_key = {0}; + nr_func_hashmap_key_t func_key = {0}; + nr_func_hashmap_t* funcs_ht = NULL; + + if (NULL == scope_ht || NULL == global_funcs_ht) { + return NULL; + } + if (NULL == func_name) { + return NULL; + } + + if (NULL != scope_name) { + func_key.is_method = true; + scope_key.name = ZSTR_VAL(scope_name); + scope_key.name_len = ZSTR_LEN(scope_name); + scope_key.name_hash = ZSTR_HASH(scope_name); + funcs_ht = nr_scope_hashmap_lookup_internal(scope_ht, &scope_key); + } else { + funcs_ht = global_funcs_ht; + } + + if (NULL == funcs_ht) { + return NULL; + } + + func_key.name = ZSTR_VAL(func_name); + func_key.name_len = ZSTR_LEN(func_name); + func_key.name_hash = ZSTR_HASH(func_name); + + return nr_func_hashmap_lookup_internal(funcs_ht, &func_key); +} + +void nr_php_user_instrument_wraprec_hashmap_destroy(void) { + if (NULL != scope_ht) { + nr_scope_hashmap_destroy_internal(&scope_ht); + } + if (NULL != global_funcs_ht) { + nr_func_hashmap_destroy_internal(&global_funcs_ht); + } + return; +} diff --git a/agent/php_user_instrument_wraprec_hashmap.h b/agent/php_user_instrument_wraprec_hashmap.h new file mode 100644 index 000000000..bf14cca8e --- /dev/null +++ b/agent/php_user_instrument_wraprec_hashmap.h @@ -0,0 +1,20 @@ +/* + * Copyright 2025 New Relic Corporation. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +#ifndef PHP_USER_INSTRUMENT_WRAPREC_HASHMAP_HDR +#define PHP_USER_INSTRUMENT_WRAPREC_HASHMAP_HDR + +#include "php_user_instrument.h" + +// clang-format off + +extern void nr_php_user_instrument_wraprec_hashmap_init(void); +extern nruserfn_t* nr_php_user_instrument_wraprec_hashmap_add(const char* namestr, size_t namestrlen); +extern nruserfn_t* nr_php_user_instrument_wraprec_hashmap_get(zend_string *func_name, zend_string *scope_name); +extern void nr_php_user_instrument_wraprec_hashmap_destroy(void); + +// clang-format on + +#endif diff --git a/agent/tests/test_txn.c b/agent/tests/test_txn.c index c641fa8e7..4edfc56bd 100644 --- a/agent/tests/test_txn.c +++ b/agent/tests/test_txn.c @@ -50,7 +50,14 @@ static void test_handle_fpm_error(TSRMLS_D) { nr_txn_set_path(NULL, NRPRG(txn), "foo", NR_PATH_TYPE_URI, NR_NOT_OK_TO_OVERWRITE); +#if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO /* PHP8+ */ + // file execution no longer increments execute_count on PHPs 8.0+ + // only user function calls do increment execute_count: + tlib_php_request_eval("function f() {$a = 1 + 1;}\n" + "f(); // create a PHP call frame" TSRMLS_CC); +#else tlib_php_request_eval("$a = 1 + 1; // create a PHP call frame" TSRMLS_CC); +#endif nr_php_txn_handle_fpm_error(NRPRG(txn) TSRMLS_CC); tlib_pass_if_str_equal("transaction path should be unchanged", "foo", NRTXN(path)); diff --git a/agent/tests/test_user_instrument.c b/agent/tests/test_user_instrument.c index 6901044db..74b5638f6 100644 --- a/agent/tests/test_user_instrument.c +++ b/agent/tests/test_user_instrument.c @@ -53,7 +53,7 @@ static void test_op_array_wraprec(TSRMLS_D) { } #endif /* PHP < 7.4 */ -#if ZEND_MODULE_API_NO >= ZEND_7_4_X_API_NO +#if ZEND_MODULE_API_NO == ZEND_7_4_X_API_NO static void test_hashmap_wraprec() { const char* user_func1_name = "user_function_to_be_instrumented"; zend_function* user_func1_zf; @@ -145,6 +145,41 @@ static void test_hashmap_wraprec() { } #endif /* PHP >= 7.4 */ +static void test_add_custom_tracer_named() { + nruserfn_t* wr = NULL; + tlib_php_request_start(); + + wr = nr_php_add_custom_tracer_named(NULL, 10); + tlib_pass_if_null("add_custom_tracer_named with NULL name", wr); + + wr = nr_php_add_custom_tracer_named("function_name", 0); + tlib_pass_if_null("add_custom_tracer_named with length == 0", wr); + + // nr_php_add_custom_tracer_named's second argument is of size_t type, + // which is **unsigned**!!! However, under the hood, it calls a function + // that converts it to an int! So this is testing that no unwanted + // side effects will happen when this silent type conversion takes place. + wr = nr_php_add_custom_tracer_named("function_name", -1); + tlib_pass_if_null("add_custom_tracer_named with length < 0", wr); + + wr = nr_php_add_custom_tracer_named(NR_PSTR("scope_name::")); + tlib_pass_if_null("add_custom_tracer_named with name ending with :", wr); + + wr = nr_php_add_custom_tracer_named(NR_PSTR("function_name")); + tlib_pass_if_not_null( + "add_custom_tracer_named with valid name " + "(unscoped global function)", + wr); + + wr = nr_php_add_custom_tracer_named(NR_PSTR("scope_name::function_name")); + tlib_pass_if_not_null( + "add_custom_tracer_named with valid name " + "(scoped method)", + wr); + + tlib_php_request_end(); +} + void test_main(void* p NRUNUSED) { #if defined(ZTS) && !defined(PHP7) void*** tsrm_ls = NULL; @@ -154,9 +189,11 @@ void test_main(void* p NRUNUSED) { #if ZEND_MODULE_API_NO < ZEND_7_4_X_API_NO test_op_array_wraprec(TSRMLS_C); -#else +#elif ZEND_MODULE_API_NO == ZEND_7_4_X_API_NO test_hashmap_wraprec(); #endif /* PHP >= 7.4 */ + test_add_custom_tracer_named(); + tlib_php_engine_destroy(TSRMLS_C); } diff --git a/agent/tests/test_user_instrument_hashmap.c b/agent/tests/test_user_instrument_hashmap.c index 1df2c3690..d5ee22ddb 100644 --- a/agent/tests/test_user_instrument_hashmap.c +++ b/agent/tests/test_user_instrument_hashmap.c @@ -12,7 +12,7 @@ tlib_parallel_info_t parallel_info = {.suggested_nthreads = -1, .state_size = 0}; -#if ZEND_MODULE_API_NO >= ZEND_7_4_X_API_NO +#if ZEND_MODULE_API_NO == ZEND_7_4_X_API_NO #define STRING_SZ(s) (s), nr_strlen(s) @@ -579,7 +579,7 @@ void test_main(void* p NRUNUSED) { tlib_php_engine_create("" PTSRMLS_CC); -#if ZEND_MODULE_API_NO >= ZEND_7_4_X_API_NO +#if ZEND_MODULE_API_NO == ZEND_7_4_X_API_NO test_wraprecs_hashmap(); test_zend_string_hash_before_set(); test_zend_string_hash_after_set_before_get(); diff --git a/agent/tests/test_user_instrument_wraprec_hashmap.c b/agent/tests/test_user_instrument_wraprec_hashmap.c new file mode 100644 index 000000000..c23bc4f8e --- /dev/null +++ b/agent/tests/test_user_instrument_wraprec_hashmap.c @@ -0,0 +1,84 @@ +/* + * Copyright 2025 New Relic Corporation. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +#include "tlib_php.h" + +#include "php_agent.h" +#include "php_globals.h" +#include "php_user_instrument_wraprec_hashmap.h" + +tlib_parallel_info_t parallel_info + = {.suggested_nthreads = -1, .state_size = 0}; + +// clang-format off + +#define SCOPE_NAME "Vendor\\Namespace\\ClassName" +#define METHOD_NAME "getSomething" +#define SCOPED_METHOD_NAME SCOPE_NAME "::" METHOD_NAME +#define FUNCTION_NAME "global_function" + +static void test_wraprecs_hashmap() { + nruserfn_t *wraprec, *found_wraprec; + zend_string *func_name, *scope_name, *method_name; + + func_name = zend_string_init(NR_PSTR(FUNCTION_NAME), 0); + scope_name = zend_string_init(NR_PSTR(SCOPE_NAME), 0); + method_name = zend_string_init(NR_PSTR(METHOD_NAME), 0); + + // user_instrument_wraprec_hashmap is initialized at minit + // destroy it to test agent's behavior when it is not initialized + nr_php_user_instrument_wraprec_hashmap_destroy(); + + // Test valid operations before initializing the hashmap + wraprec = nr_php_user_instrument_wraprec_hashmap_add(NR_PSTR(FUNCTION_NAME)); + tlib_pass_if_null("adding valid function before init", wraprec); + wraprec = nr_php_user_instrument_wraprec_hashmap_add(NR_PSTR(SCOPED_METHOD_NAME)); + tlib_pass_if_null("adding valid method before init", wraprec); + + // Initialize the hashmap + nr_php_user_instrument_wraprec_hashmap_init(); + + // Test valid operations after initializing the hashmap + wraprec = nr_php_user_instrument_wraprec_hashmap_add(NR_PSTR(FUNCTION_NAME)); + tlib_pass_if_not_null("adding valid global function", wraprec); + + found_wraprec = nr_php_user_instrument_wraprec_hashmap_get(func_name, NULL); + tlib_pass_if_ptr_equal("getting valid global function", wraprec, found_wraprec); + + found_wraprec = nr_php_user_instrument_wraprec_hashmap_get(func_name, scope_name); + tlib_pass_if_null("getting global function with scope", found_wraprec); + + wraprec = nr_php_user_instrument_wraprec_hashmap_add(NR_PSTR(SCOPED_METHOD_NAME)); + tlib_pass_if_not_null("adding valid scoped method", wraprec); + + found_wraprec = nr_php_user_instrument_wraprec_hashmap_get(method_name, scope_name); + tlib_pass_if_ptr_equal("getting scoped method", wraprec, found_wraprec); + + found_wraprec = nr_php_user_instrument_wraprec_hashmap_get(NULL, scope_name); + tlib_pass_if_null("getting scoped method without method name", found_wraprec); + + found_wraprec = nr_php_user_instrument_wraprec_hashmap_get(method_name, NULL); + tlib_pass_if_null("getting scoped method without scope", found_wraprec); + + nr_php_user_instrument_wraprec_hashmap_destroy(); + + zend_string_free(func_name); + zend_string_free(scope_name); + zend_string_free(method_name); +} + +// clang-format on + +void test_main(void* p NRUNUSED) { +#if defined(ZTS) && !defined(PHP7) + void*** tsrm_ls = NULL; +#endif /* ZTS && !PHP7 */ + + tlib_php_engine_create("" PTSRMLS_CC); + + test_wraprecs_hashmap(); + + tlib_php_engine_destroy(TSRMLS_C); +} diff --git a/tests/integration/api/add_custom_tracer/test_invalid_input.php b/tests/integration/api/add_custom_tracer/test_invalid_input.php new file mode 100644 index 000000000..278c7b4ec --- /dev/null +++ b/tests/integration/api/add_custom_tracer/test_invalid_input.php @@ -0,0 +1,98 @@ += 7.4 required\n"); +if (PHP_MAJOR_VERSION != 7 || PHP_MINOR_VERSION != 4) { + die("skip: PHP = 7.4 required\n"); } */ diff --git a/tests/integration/frameworks/drupal/test_invoke_all_with.php8.php b/tests/integration/frameworks/drupal/test_invoke_all_with.php8.php new file mode 100644 index 000000000..f592f770d --- /dev/null +++ b/tests/integration/frameworks/drupal/test_invoke_all_with.php8.php @@ -0,0 +1,147 @@ +moduleHandler(); + +// Test lambda calback +$handler->invokeAllWith("hook_1", function (callable $hook, string $module) { + $hook(); +}); + +// Test string and reference callback +$func_name = "invoke_callback"; +$func_name_ref =& $func_name; +$handler->invokeAllWith("hook_2", $func_name_ref); + +//Test callable array callback +$invoker = new Invoker(); +$handler->invokeAllWith("hook_3", [$invoker, "invoke"]); + +// test callable array callback; function already special instrumented +// but wraprec not installed yet by observer API because the function +// has not been called yet - drupal's hook instrumentation will be +// installed +$page_cache = new Drupal\page_cache\StackMiddleware\PageCache; +$handler->invokeallwith("hook_4", [$page_cache, "get"]); + +// test string callback; function already instrumented +// This will reuse the existing wraprec and successfully +// add instrumentation because the "before" callback is unset +$func_name = "invoke_callback_instrumented"; +$handler->invokeallwith("hook_4", $func_name); + +// test non-transiently wrapping an already transiently instrumented function +// This will overwrite the existing transient wrapper +$func_name = "invoke_callback"; +newrelic_add_custom_tracer($func_name); +// Now this test will function the same as above: adding special instrumentation +// to an already existing wrapper +$handler->invokeallwith("hook_2", $func_name); diff --git a/tests/integration/frameworks/laravel/mock_http_options.php b/tests/integration/frameworks/laravel/mock_http_options.php index 85df12f2f..5eb02823e 100644 --- a/tests/integration/frameworks/laravel/mock_http_options.php +++ b/tests/integration/frameworks/laravel/mock_http_options.php @@ -58,6 +58,3 @@ public function setMockedRoute($route) { } } -namespace Illuminate\Routing { - echo ""; -} diff --git a/tests/integration/lang/test_attributes.php b/tests/integration/lang/test_attributes.php new file mode 100644 index 000000000..7cccdf0ec --- /dev/null +++ b/tests/integration/lang/test_attributes.php @@ -0,0 +1,122 @@ +info; + } +} + +#[ExampleAttribute("This is a sample attribute")] +class SampleClass +{ + #[ExampleAttribute("This is a method attribute")] + public function sampleMethod() {} +} + +newrelic_add_custom_tracer("ExampleAttribute::getInfo"); + +// Reflect the class +$reflectionClass = new ReflectionClass(SampleClass::class); + +// Get class attributes +$classAttributes = $reflectionClass->getAttributes(ExampleAttribute::class); + +foreach ($classAttributes as $attribute) { + $instance = $attribute->newInstance(); + echo "Class Attribute Info: " . $instance->getInfo() . "\n"; +} + +// Reflect the method +$reflectionMethod = $reflectionClass->getMethod('sampleMethod'); + +// Get method attributes +$methodAttributes = $reflectionMethod->getAttributes(ExampleAttribute::class); + +foreach ($methodAttributes as $attribute) { + $instance = $attribute->newInstance(); + echo "Method Attribute Info: " . $instance->getInfo() . "\n"; +} diff --git a/tests/integration/lang/trampoline/test_trampoline_00.php b/tests/integration/lang/trampoline/test_trampoline_00.php new file mode 100644 index 000000000..c1b1f837e --- /dev/null +++ b/tests/integration/lang/trampoline/test_trampoline_00.php @@ -0,0 +1,39 @@ +bar('foo')); diff --git a/tests/integration/lang/trampoline/test_trampoline_01.php b/tests/integration/lang/trampoline/test_trampoline_01.php new file mode 100644 index 000000000..326ac8bd5 --- /dev/null +++ b/tests/integration/lang/trampoline/test_trampoline_01.php @@ -0,0 +1,47 @@ +getMethod('execute'); +$klass_name = $klass->getName(); +$method_name = $method->getName(); + +newrelic_add_custom_tracer("$klass_name::$method_name"); + +(new Wrapper($anon))->execute( + (new Wrapper($anon))->execute() +); + +class Wrapper +{ + protected $wrapped; + + function __construct($wrapped) + { + $this->wrapped = $wrapped; + } + + public function __call($method, $arguments) + { + return call_user_func([$this->wrapped, $method]); + } +} diff --git a/tests/integration/span_events/tt_detail/test_tt_detail_off_with_custom_wrapper_api.php b/tests/integration/span_events/tt_detail/test_tt_detail_off_with_custom_wrapper_api.php new file mode 100644 index 000000000..7b50f9302 --- /dev/null +++ b/tests/integration/span_events/tt_detail/test_tt_detail_off_with_custom_wrapper_api.php @@ -0,0 +1,99 @@ + Date: Mon, 12 May 2025 10:52:20 -0500 Subject: [PATCH 3/4] feat(agent): Drupal hook attribute instrumentation (#1030) Adds support for Drupal Attribute Hooks added in Drupal 11.1. --- agent/fw_drupal8.c | 92 +++++++++++++++++++ .../mock_module_handler_empty_array.php | 34 +++++++ .../drupal/mock_module_handler_empty_key.php | 34 +++++++ .../mock_module_handler_invalid_key.php | 34 +++++++ .../mock_module_handler_invalid_map.php | 30 ++++++ .../mock_module_handler_invalid_module.php | 34 +++++++ .../mock_module_handler_invalid_value.php | 34 +++++++ .../drupal/mock_module_handler_valid.php | 34 +++++++ ...t_hook_implementations_map_empty_array.php | 42 +++++++++ ...est_hook_implementations_map_empty_key.php | 42 +++++++++ ...t_hook_implementations_map_invalid_key.php | 42 +++++++++ ...t_hook_implementations_map_invalid_map.php | 42 +++++++++ ...ook_implementations_map_invalid_module.php | 42 +++++++++ ...hook_implementations_map_invalid_value.php | 42 +++++++++ .../test_hook_implementations_map_valid.php | 42 +++++++++ 15 files changed, 620 insertions(+) create mode 100644 tests/integration/frameworks/drupal/mock_module_handler_empty_array.php create mode 100644 tests/integration/frameworks/drupal/mock_module_handler_empty_key.php create mode 100644 tests/integration/frameworks/drupal/mock_module_handler_invalid_key.php create mode 100644 tests/integration/frameworks/drupal/mock_module_handler_invalid_map.php create mode 100644 tests/integration/frameworks/drupal/mock_module_handler_invalid_module.php create mode 100644 tests/integration/frameworks/drupal/mock_module_handler_invalid_value.php create mode 100644 tests/integration/frameworks/drupal/mock_module_handler_valid.php create mode 100644 tests/integration/frameworks/drupal/test_hook_implementations_map_empty_array.php create mode 100644 tests/integration/frameworks/drupal/test_hook_implementations_map_empty_key.php create mode 100644 tests/integration/frameworks/drupal/test_hook_implementations_map_invalid_key.php create mode 100644 tests/integration/frameworks/drupal/test_hook_implementations_map_invalid_map.php create mode 100644 tests/integration/frameworks/drupal/test_hook_implementations_map_invalid_module.php create mode 100644 tests/integration/frameworks/drupal/test_hook_implementations_map_invalid_value.php create mode 100644 tests/integration/frameworks/drupal/test_hook_implementations_map_valid.php diff --git a/agent/fw_drupal8.c b/agent/fw_drupal8.c index 27eafe6bc..2178c9aee 100644 --- a/agent/fw_drupal8.c +++ b/agent/fw_drupal8.c @@ -608,6 +608,95 @@ NR_PHP_WRAPPER(nr_drupal94_invoke_all_with_clean) { NR_PHP_WRAPPER_END #endif // OAPI +static bool nr_is_invalid_key_val_arr(nr_php_string_hash_key_t* key, + zval* val) { + if (NULL == key || 0 == ZEND_STRING_LEN(key) + || 0 == nr_php_is_zval_valid_array(val) + || 0 == zend_hash_num_elements(Z_ARRVAL_P(val))) { + return true; + } else { + return false; + } +} + +/* + * Purpose: Instrument Drupal Attribute Hooks for Drupal 11.1+ + * + * Params: 1. A zval pointer to the moduleHandler instance in use by Drupal. + * + * Return: bool + * + */ +static bool nr_drupal_hook_attribute_instrument(zval* module_handler) { + zval* hook_implementation_map = NULL; + + nr_php_string_hash_key_t* hook_key = NULL; + zval* hook_val = NULL; + nr_php_string_hash_key_t* class_key = NULL; + zval* class_val = NULL; + nr_php_string_hash_key_t* method_key = NULL; + zval* module_val = NULL; + + char* hookpath = NULL; + + hook_implementation_map = nr_php_get_zval_object_property( + module_handler, "hookImplementationsMap"); + + if (!nr_php_is_zval_valid_array(hook_implementation_map)) { + nrl_verbosedebug(NRL_FRAMEWORK, + "hookImplementationsMap property not a valid array"); + return false; + } + + ZEND_HASH_FOREACH_STR_KEY_VAL(Z_ARRVAL_P(hook_implementation_map), hook_key, + hook_val) { + if (nr_is_invalid_key_val_arr(hook_key, hook_val)) { + nrl_warning(NRL_FRAMEWORK, + "hookImplementationsMap[hook]: invalid key or value"); + return false; + } + + ZEND_HASH_FOREACH_STR_KEY_VAL(Z_ARRVAL_P(hook_val), class_key, class_val) { + if (nr_is_invalid_key_val_arr(class_key, class_val)) { + nrl_warning(NRL_FRAMEWORK, + "hookImplementationsMap[class]: invalid key or value"); + return false; + } + + ZEND_HASH_FOREACH_STR_KEY_VAL(Z_ARRVAL_P(class_val), method_key, + module_val) { + if (NULL == method_key + || 0 == nr_php_is_zval_valid_string(module_val)) { + nrl_warning(NRL_FRAMEWORK, + "hookImplementationsMap[method]: invalid key or value"); + return false; + } + + if (nr_striendswith( + ZEND_STRING_VALUE(class_key), ZEND_STRING_LEN(class_key), + NR_PSTR("Drupal\\Core\\Extension\\ProceduralCall"))) { + hookpath = nr_formatf("%s", ZEND_STRING_VALUE(method_key)); + } else { + hookpath = nr_formatf("%s::%s", ZEND_STRING_VALUE(class_key), + ZEND_STRING_VALUE(method_key)); + } + + nr_php_wrap_user_function_drupal( + hookpath, nr_strlen(hookpath), Z_STRVAL_P(module_val), + Z_STRLEN_P(module_val), ZEND_STRING_VALUE(hook_key), + ZEND_STRING_LEN(hook_key)); + + nr_free(hookpath); + } + ZEND_HASH_FOREACH_END(); + } + ZEND_HASH_FOREACH_END(); + } + ZEND_HASH_FOREACH_END(); + + return true; +} + /* * Purpose : Wrap the invoke() method of the module handler instance in use. */ @@ -635,6 +724,9 @@ NR_PHP_WRAPPER(nr_drupal8_module_handler) { ce = Z_OBJCE_P(*retval_ptr); + if (nr_drupal_hook_attribute_instrument(*retval_ptr)) { + NR_PHP_WRAPPER_LEAVE; + } nr_drupal8_add_method_callback(ce, NR_PSTR("getimplementations"), nr_drupal8_post_get_implementations TSRMLS_CC); nr_drupal8_add_method_callback(ce, NR_PSTR("implementshook"), diff --git a/tests/integration/frameworks/drupal/mock_module_handler_empty_array.php b/tests/integration/frameworks/drupal/mock_module_handler_empty_array.php new file mode 100644 index 000000000..0cf5db0a8 --- /dev/null +++ b/tests/integration/frameworks/drupal/mock_module_handler_empty_array.php @@ -0,0 +1,34 @@ + array('classname' => array('methodname' => 'modulename')), + 'hookname_b' => array(), + 'hookname_c' => array('classname_c' => array('methodname_c' => 'modulename_c')), + ); + + // to avoid editor warnings + public function invokeAllWith($hook_str, $callback) + { + return null; + } + + // for debugging purposes + public function dump() + { + var_dump($this->hookImplementationsMap); + } + } +} diff --git a/tests/integration/frameworks/drupal/mock_module_handler_empty_key.php b/tests/integration/frameworks/drupal/mock_module_handler_empty_key.php new file mode 100644 index 000000000..3f057711e --- /dev/null +++ b/tests/integration/frameworks/drupal/mock_module_handler_empty_key.php @@ -0,0 +1,34 @@ + array('classname' => array('methodname' => 'modulename')), + 'hookname_b' => array('' => array('methodname_b' => 'modulename_b')), + 'hookname_c' => array('classname_c' => array('methodname_c' => 'modulename_c')), + ); + + // to avoid editor warnings + public function invokeAllWith($hook_str, $callback) + { + return null; + } + + // for debugging purposes + public function dump() + { + var_dump($this->hookImplementationsMap); + } + } +} diff --git a/tests/integration/frameworks/drupal/mock_module_handler_invalid_key.php b/tests/integration/frameworks/drupal/mock_module_handler_invalid_key.php new file mode 100644 index 000000000..faffb7ded --- /dev/null +++ b/tests/integration/frameworks/drupal/mock_module_handler_invalid_key.php @@ -0,0 +1,34 @@ + array('classname' => array('methodname' => 'modulename')), + 1 => array('classname_b' => array('methodname_b' => 'modulename_b')), + 'hookname_c' => array('classname_c', array('methodname_c', 'modulename_c')), + ); + + // to avoid editor warnings + public function invokeAllWith($hook_str, $callback) + { + return null; + } + + // for debugging purposes + public function dump() + { + var_dump($this->hookImplementationsMap); + } + } +} diff --git a/tests/integration/frameworks/drupal/mock_module_handler_invalid_map.php b/tests/integration/frameworks/drupal/mock_module_handler_invalid_map.php new file mode 100644 index 000000000..ac61d62bf --- /dev/null +++ b/tests/integration/frameworks/drupal/mock_module_handler_invalid_map.php @@ -0,0 +1,30 @@ +hookImplementationsMap); + } + } +} diff --git a/tests/integration/frameworks/drupal/mock_module_handler_invalid_module.php b/tests/integration/frameworks/drupal/mock_module_handler_invalid_module.php new file mode 100644 index 000000000..d6fa753aa --- /dev/null +++ b/tests/integration/frameworks/drupal/mock_module_handler_invalid_module.php @@ -0,0 +1,34 @@ + array('classname' => array('methodname' => 'modulename')), + 'hookname_b' => array('classname_b' => array('methodname_b' => array(1, 2, 3))), + 'hookname_c' => array('classname_c' => array('methodname_c' => 'modulename_c')), + ); + + // to avoid editor warnings + public function invokeAllWith($hook_str, $callback) + { + return null; + } + + // for debugging purposes + public function dump() + { + var_dump($this->hookImplementationsMap); + } + } +} diff --git a/tests/integration/frameworks/drupal/mock_module_handler_invalid_value.php b/tests/integration/frameworks/drupal/mock_module_handler_invalid_value.php new file mode 100644 index 000000000..cdec24a98 --- /dev/null +++ b/tests/integration/frameworks/drupal/mock_module_handler_invalid_value.php @@ -0,0 +1,34 @@ + array('classname' => array('methodname' => 'modulename')), + 'hookname_b' => array('classname_b' => 'just a string'), + 'hookname_c' => array('classname_c' => array('methodname_c' => 'modulename_c')), + ); + + // to avoid editor warnings + public function invokeAllWith($hook_str, $callback) + { + return null; + } + + // for debugging purposes + public function dump() + { + var_dump($this->hookImplementationsMap); + } + } +} diff --git a/tests/integration/frameworks/drupal/mock_module_handler_valid.php b/tests/integration/frameworks/drupal/mock_module_handler_valid.php new file mode 100644 index 000000000..a17bb4d2b --- /dev/null +++ b/tests/integration/frameworks/drupal/mock_module_handler_valid.php @@ -0,0 +1,34 @@ + array('classname' => array('methodname' => 'modulename')), + 'hookname_b' => array('classname_b' => array('methodname_b' => 'modulename_b')), + 'hookname_c' => array('classname_c' => array('methodname_c' => 'modulename_c')), + ); + + // to avoid editor warnings + public function invokeAllWith($hook_str, $callback) + { + return null; + } + + // for debugging purposes + public function dump() + { + var_dump($this->hookImplementationsMap); + } + } +} diff --git a/tests/integration/frameworks/drupal/test_hook_implementations_map_empty_array.php b/tests/integration/frameworks/drupal/test_hook_implementations_map_empty_array.php new file mode 100644 index 000000000..997dd7b0c --- /dev/null +++ b/tests/integration/frameworks/drupal/test_hook_implementations_map_empty_array.php @@ -0,0 +1,42 @@ += 7.4 required\n"); +} +*/ + +/*INI +newrelic.framework = drupal8 +*/ + +/*EXPECT_TRACED_ERRORS null */ + +/*EXPECT_ERROR_EVENTS null */ + +/*EXPECT +*/ + +require_once __DIR__ . '/mock_module_handler_empty_array.php'; + +// This specific API is needed for us to instrument the ModuleHandler +class Drupal +{ + public function moduleHandler() + { + return new Drupal\Core\Extension\ModuleHandler(); + } +} + +// Create module handler +$drupal = new Drupal(); +$handler = $drupal->moduleHandler(); diff --git a/tests/integration/frameworks/drupal/test_hook_implementations_map_empty_key.php b/tests/integration/frameworks/drupal/test_hook_implementations_map_empty_key.php new file mode 100644 index 000000000..1ee7a2400 --- /dev/null +++ b/tests/integration/frameworks/drupal/test_hook_implementations_map_empty_key.php @@ -0,0 +1,42 @@ += 7.4 required\n"); +} +*/ + +/*INI +newrelic.framework = drupal8 +*/ + +/*EXPECT_TRACED_ERRORS null */ + +/*EXPECT_ERROR_EVENTS null */ + +/*EXPECT +*/ + +require_once __DIR__ . '/mock_module_handler_empty_key.php'; + +// This specific API is needed for us to instrument the ModuleHandler +class Drupal +{ + public function moduleHandler() + { + return new Drupal\Core\Extension\ModuleHandler(); + } +} + +// Create module handler +$drupal = new Drupal(); +$handler = $drupal->moduleHandler(); diff --git a/tests/integration/frameworks/drupal/test_hook_implementations_map_invalid_key.php b/tests/integration/frameworks/drupal/test_hook_implementations_map_invalid_key.php new file mode 100644 index 000000000..ac60f928f --- /dev/null +++ b/tests/integration/frameworks/drupal/test_hook_implementations_map_invalid_key.php @@ -0,0 +1,42 @@ += 7.4 required\n"); +} +*/ + +/*INI +newrelic.framework = drupal8 +*/ + +/*EXPECT_TRACED_ERRORS null */ + +/*EXPECT_ERROR_EVENTS null */ + +/*EXPECT +*/ + +require_once __DIR__ . '/mock_module_handler_invalid_key.php'; + +// This specific API is needed for us to instrument the ModuleHandler +class Drupal +{ + public function moduleHandler() + { + return new Drupal\Core\Extension\ModuleHandler(); + } +} + +// Create module handler +$drupal = new Drupal(); +$handler = $drupal->moduleHandler(); diff --git a/tests/integration/frameworks/drupal/test_hook_implementations_map_invalid_map.php b/tests/integration/frameworks/drupal/test_hook_implementations_map_invalid_map.php new file mode 100644 index 000000000..a55a2306b --- /dev/null +++ b/tests/integration/frameworks/drupal/test_hook_implementations_map_invalid_map.php @@ -0,0 +1,42 @@ += 7.4 required\n"); +} +*/ + +/*INI +newrelic.framework = drupal8 +*/ + +/*EXPECT_TRACED_ERRORS null */ + +/*EXPECT_ERROR_EVENTS null */ + +/*EXPECT +*/ + +require_once __DIR__ . '/mock_module_handler_invalid_map.php'; + +// This specific API is needed for us to instrument the ModuleHandler +class Drupal +{ + public function moduleHandler() + { + return new Drupal\Core\Extension\ModuleHandler(); + } +} + +// Create module handler +$drupal = new Drupal(); +$handler = $drupal->moduleHandler(); diff --git a/tests/integration/frameworks/drupal/test_hook_implementations_map_invalid_module.php b/tests/integration/frameworks/drupal/test_hook_implementations_map_invalid_module.php new file mode 100644 index 000000000..3a9b9c529 --- /dev/null +++ b/tests/integration/frameworks/drupal/test_hook_implementations_map_invalid_module.php @@ -0,0 +1,42 @@ += 7.4 required\n"); +} +*/ + +/*INI +newrelic.framework = drupal8 +*/ + +/*EXPECT_TRACED_ERRORS null */ + +/*EXPECT_ERROR_EVENTS null */ + +/*EXPECT +*/ + +require_once __DIR__ . '/mock_module_handler_invalid_module.php'; + +// This specific API is needed for us to instrument the ModuleHandler +class Drupal +{ + public function moduleHandler() + { + return new Drupal\Core\Extension\ModuleHandler(); + } +} + +// Create module handler +$drupal = new Drupal(); +$handler = $drupal->moduleHandler(); diff --git a/tests/integration/frameworks/drupal/test_hook_implementations_map_invalid_value.php b/tests/integration/frameworks/drupal/test_hook_implementations_map_invalid_value.php new file mode 100644 index 000000000..892bc533a --- /dev/null +++ b/tests/integration/frameworks/drupal/test_hook_implementations_map_invalid_value.php @@ -0,0 +1,42 @@ += 7.4 required\n"); +} +*/ + +/*INI +newrelic.framework = drupal8 +*/ + +/*EXPECT_TRACED_ERRORS null */ + +/*EXPECT_ERROR_EVENTS null */ + +/*EXPECT +*/ + +require_once __DIR__ . '/mock_module_handler_invalid_value.php'; + +// This specific API is needed for us to instrument the ModuleHandler +class Drupal +{ + public function moduleHandler() + { + return new Drupal\Core\Extension\ModuleHandler(); + } +} + +// Create module handler +$drupal = new Drupal(); +$handler = $drupal->moduleHandler(); diff --git a/tests/integration/frameworks/drupal/test_hook_implementations_map_valid.php b/tests/integration/frameworks/drupal/test_hook_implementations_map_valid.php new file mode 100644 index 000000000..afe54c69c --- /dev/null +++ b/tests/integration/frameworks/drupal/test_hook_implementations_map_valid.php @@ -0,0 +1,42 @@ += 7.4 required\n"); +} +*/ + +/*INI +newrelic.framework = drupal8 +*/ + +/*EXPECT_TRACED_ERRORS null */ + +/*EXPECT_ERROR_EVENTS null */ + +/*EXPECT +*/ + +require_once __DIR__ . '/mock_module_handler_valid.php'; + +// This specific API is needed for us to instrument the ModuleHandler +class Drupal +{ + public function moduleHandler() + { + return new Drupal\Core\Extension\ModuleHandler(); + } +} + +// Create module handler +$drupal = new Drupal(); +$handler = $drupal->moduleHandler(); From 5b84fc81ab1c63af558485f27ee59e90ae1f0fd8 Mon Sep 17 00:00:00 2001 From: Michal Nowacki Date: Mon, 12 May 2025 15:15:42 -0400 Subject: [PATCH 4/4] refactor(agent): improve logging when adding named tracers (#1061) Make log messages easier to understand when adding a named custom tracer by logging only a single message. Instead of logging one message when the wraprec is added, and two messages when wraprec is re-used, log a single message that indicates if the wraprec was added or re-used. --- agent/php_user_instrument.c | 6 +++--- agent/php_user_instrument_wraprec_hashmap.c | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/agent/php_user_instrument.c b/agent/php_user_instrument.c index 1673949a2..1fe6a114b 100644 --- a/agent/php_user_instrument.c +++ b/agent/php_user_instrument.c @@ -570,13 +570,13 @@ nruserfn_t* nr_php_add_custom_tracer_named(const char* namestr, return p; /* return the wraprec we are duplicating */ } } -#else - wraprec = nr_php_user_instrument_wraprec_hashmap_add(namestr, namestrlen); -#endif nrl_verbosedebug( NRL_INSTRUMENT, "adding custom for '" NRP_FMT_UQ "%.5s" NRP_FMT_UQ "'", NRP_PHP(wraprec->classname), (0 == wraprec->classname) ? "" : "::", NRP_PHP(wraprec->funcname)); +#else + wraprec = nr_php_user_instrument_wraprec_hashmap_add(namestr, namestrlen); +#endif nr_php_wrap_user_function_internal(wraprec TSRMLS_CC); nr_php_add_custom_tracer_common(wraprec); diff --git a/agent/php_user_instrument_wraprec_hashmap.c b/agent/php_user_instrument_wraprec_hashmap.c index 6aab143c2..dbada0077 100644 --- a/agent/php_user_instrument_wraprec_hashmap.c +++ b/agent/php_user_instrument_wraprec_hashmap.c @@ -437,6 +437,7 @@ nruserfn_t* nr_php_user_instrument_wraprec_hashmap_add(const char* namestr, size wraprec->supportability_metric = nr_txn_create_fn_supportability_metric( wraprec->funcname, wraprec->classname); + nrl_verbosedebug(NRL_INSTRUMENT, "adding custom wrapper for '%s'", namestr); } else { nrl_verbosedebug(NRL_INSTRUMENT, "reusing custom wrapper for '%s'", namestr); }