From 6390eddf0d66cd4e9a8904a900c08f61d62fa6c2 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Tue, 30 Apr 2024 11:48:38 -0600 Subject: [PATCH 01/23] initial commit --- .gitignore | 3 +- agent/fw_drupal.c | 4 + agent/fw_drupal_common.c | 1 + agent/fw_wordpress.c | 68 ++++++- agent/lib_mongodb.c | 4 + agent/lib_predis.c | 4 + agent/php_execute.c | 179 +++++++++++++++++- agent/php_observer.c | 39 ++++ agent/php_observer.h | 14 ++ agent/php_user_instrument.c | 69 ++++++- agent/php_user_instrument.h | 5 + agent/php_wrapper.c | 29 +++ agent/php_wrapper.h | 64 ++++++- axiom/nr_segment.h | 5 +- daemon/cmd/integration_runner/main.go | 2 +- .../test_span_events_max_samples_stored1.php | 2 +- .../span_events/test_span_events_on_dt_on.php | 3 + 17 files changed, 475 insertions(+), 20 deletions(-) diff --git a/.gitignore b/.gitignore index d850b2fd0..aec099e8a 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,6 @@ # Build artifacts -agent/configure.ac agent/newrelic.map +agent/configure.ac agent/*.dep axiom/tests/*.tmp bin/ @@ -13,3 +13,4 @@ php_agent.log # Dev artifacts .vscode +*.log diff --git a/agent/fw_drupal.c b/agent/fw_drupal.c index dbc67a7ec..c2628b636 100644 --- a/agent/fw_drupal.c +++ b/agent/fw_drupal.c @@ -268,7 +268,11 @@ NR_PHP_WRAPPER(nr_drupal_http_request_before) { * this new segment is now at the top of the segment stack. */ if (NULL != NRPRG(drupal_http_request_segment)) { +#if ZEND_MODULE_API_NO < ZEND_8_2_X_API_NO NRPRG(drupal_http_request_segment)->wraprec = auto_segment->wraprec; +#else + NRPRG(drupal_http_request_segment)->execute_data = auto_segment->execute_data; +#endif } } } diff --git a/agent/fw_drupal_common.c b/agent/fw_drupal_common.c index 894ff2270..5fe24f5e3 100644 --- a/agent/fw_drupal_common.c +++ b/agent/fw_drupal_common.c @@ -60,6 +60,7 @@ NR_PHP_WRAPPER(nr_drupal_wrap_module_hook) { * function such as a_b_c is ambiguous (is the module a or a_b?). Instead, * we'll see if they're defined in the wraprec. */ + wraprec = nr_php_get_wraprec(execute_data->func); if ((NULL != wraprec->drupal_hook) && (NULL != wraprec->drupal_module)) { nr_drupal_create_metric(auto_segment, NR_PSTR(NR_DRUPAL_MODULE_PREFIX), wraprec->drupal_module, wraprec->drupal_module_len); diff --git a/agent/fw_wordpress.c b/agent/fw_wordpress.c index 8503c5286..cd905fc69 100644 --- a/agent/fw_wordpress.c +++ b/agent/fw_wordpress.c @@ -329,6 +329,60 @@ static char* nr_wordpress_plugin_from_function(zend_function* func TSRMLS_DC) { return plugin; } +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO +NR_PHP_WRAPPER(nr_wordpress_wrap_hook_core) { + NR_UNUSED_SPECIALFN; + (void)wraprec; + + /* + * We only want to hook the function being called if this is a WordPress + * function, we're instrumenting hooks, and WordPress is currently executing + * hooks (denoted by the wordpress_tag being set). + */ + NR_PHP_WRAPPER_REQUIRE_FRAMEWORK(NR_FW_WORDPRESS); + + char* tag = nr_stack_get_top(&NRPRG(wordpress_tags)); + if ((0 == NRINI(wordpress_hooks)) || (NULL == tag)) { + NR_PHP_WRAPPER_LEAVE; + } + + NR_PHP_WRAPPER_CALL; + if (NRPRG(wordpress_core)) { + nr_wordpress_create_metric(auto_segment, NR_WORDPRESS_HOOK_PREFIX, tag); + } +} +NR_PHP_WRAPPER_END + +NR_PHP_WRAPPER(nr_wordpress_wrap_hook_plugin) { + char* plugin = NULL; + + NR_UNUSED_SPECIALFN; + (void)wraprec; + + /* + * We only want to hook the function being called if this is a WordPress + * function, we're instrumenting hooks, and WordPress is currently executing + * hooks (denoted by the wordpress_tag being set). + */ + NR_PHP_WRAPPER_REQUIRE_FRAMEWORK(NR_FW_WORDPRESS); + + char* tag = nr_stack_get_top(&NRPRG(wordpress_tags)); + if ((0 == NRINI(wordpress_hooks)) || (NULL == tag)) { + NR_PHP_WRAPPER_LEAVE; + } + plugin = nr_wordpress_plugin_from_function(execute_data->func); + + NR_PHP_WRAPPER_CALL; + if (NULL != plugin) { + nr_wordpress_create_metric(auto_segment, NR_WORDPRESS_HOOK_PREFIX, tag); + nr_wordpress_create_metric(auto_segment, NR_WORDPRESS_PLUGIN_PREFIX, + plugin); + } +} +NR_PHP_WRAPPER_END +#endif + +#if ZEND_MODULE_API_NO < ZEND_8_2_X_API_NO NR_PHP_WRAPPER(nr_wordpress_wrap_hook) { #if ZEND_MODULE_API_NO < ZEND_7_4_X_API_NO zend_function* func = NULL; @@ -375,6 +429,7 @@ NR_PHP_WRAPPER(nr_wordpress_wrap_hook) { } } NR_PHP_WRAPPER_END +#endif // = ZEND_8_0_X_API_NO \ +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO + if (NULL != wordpress_plugin_theme) { + callback_wraprec = nr_php_wrap_callable_before_after( + zf, NULL, nr_wordpress_wrap_hook_plugin); + } else { + callback_wraprec = nr_php_wrap_callable_before_after( + zf, NULL, nr_wordpress_wrap_hook_core); + } +#elif ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \ && !defined OVERWRITE_ZEND_EXECUTE_DATA callback_wraprec = nr_php_wrap_callable_before_after( zf, NULL, nr_wordpress_wrap_hook); diff --git a/agent/lib_mongodb.c b/agent/lib_mongodb.c index aa96d1b60..4672187ee 100644 --- a/agent/lib_mongodb.c +++ b/agent/lib_mongodb.c @@ -184,7 +184,11 @@ NR_PHP_WRAPPER(nr_mongodb_operation_before) { nr_segment_t* segment = NULL; segment = nr_segment_start(NRPRG(txn), NULL, NULL); if (NULL != segment) { +#if ZEND_MODULE_API_NO < ZEND_8_2_X_API_NO segment->wraprec = auto_segment->wraprec; +#else + segment->execute_data = auto_segment->execute_data; +#endif } } NR_PHP_WRAPPER_END diff --git a/agent/lib_predis.c b/agent/lib_predis.c index f25b3c505..5d009a9df 100644 --- a/agent/lib_predis.c +++ b/agent/lib_predis.c @@ -761,7 +761,11 @@ NR_PHP_WRAPPER(nr_predis_webdisconnection_executeCommand_before) { nr_segment_t* segment = NULL; segment = nr_segment_start(NRPRG(txn), NULL, NULL); if (NULL != segment) { +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO + segment->execute_data = auto_segment->execute_data; +#else segment->wraprec = auto_segment->wraprec; +#endif } } NR_PHP_WRAPPER_END diff --git a/agent/php_execute.c b/agent/php_execute.c index ac5121412..66601635a 100644 --- a/agent/php_execute.c +++ b/agent/php_execute.c @@ -1884,9 +1884,11 @@ static void nr_php_observer_attempt_call_cufa_handler(NR_EXECUTE_PROTO) { static void nr_php_instrument_func_begin(NR_EXECUTE_PROTO) { nr_segment_t* segment = NULL; - nruserfn_t* wraprec = NULL; +#if ZEND_MODULE_API_NO < ZEND_8_2_X_API_NO nrtime_t txn_start_time = 0; + nruserfn_t* wraprec = NULL; int zcaught = 0; +#endif NR_UNUSED_FUNC_RETURN_VALUE; if (NULL == NRPRG(txn)) { @@ -1894,7 +1896,9 @@ static void nr_php_instrument_func_begin(NR_EXECUTE_PROTO) { } NRTXNGLOBAL(execute_count) += 1; +#if ZEND_MODULE_API_NO < ZEND_8_2_X_API_NO txn_start_time = nr_txn_start_time(NRPRG(txn)); +#endif /* * Handle here, but be aware the classes might not be loaded yet. */ @@ -1918,15 +1922,21 @@ static void nr_php_instrument_func_begin(NR_EXECUTE_PROTO) { */ nr_php_observer_attempt_call_cufa_handler(NR_EXECUTE_ORIG_ARGS); } +#if ZEND_MODULE_API_NO < ZEND_8_2_X_API_NO wraprec = nr_php_get_wraprec(execute_data->func); +#endif segment = nr_segment_start(NRPRG(txn), NULL, NULL); +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO + segment->execute_data = execute_data; +#endif if (nrunlikely(NULL == segment)) { nrl_verbosedebug(NRL_AGENT, "Error starting segment."); return; } +#if ZEND_MODULE_API_NO < ZEND_8_2_X_API_NO if (NULL == wraprec) { return; } @@ -1972,16 +1982,64 @@ static void nr_php_instrument_func_begin(NR_EXECUTE_PROTO) { * Check for, and handle, frameworks. */ if (wraprec->is_names_wt_simple) { - nr_txn_name_from_function(NRPRG(txn), wraprec->funcname, - wraprec->classname); + + nr_txn_name_from_function(NRPRG(txn), + nr_php_op_array_function_name(NR_OP_ARRAY); + nr_php_class_entry_name(NR_OP_ARRAY->scope)); + } +#endif +} + +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO +void nr_php_observer_fcall_begin_late(zend_execute_data* execute_data, nrtime_t txn_start_time) { + /* + * During nr_zend_call_oapi_special_before, the transaction may have been + * ended and/or a new transaction may have started. To detect this, we + * compare the currently active transaction's start time with the transaction + * start time we saved before. + * + * Just comparing the transaction pointer is not enough, as a newly + * started transaction might actually obtain the same address as a + * transaction freed before. + */ + if (nrunlikely(nr_txn_start_time(NRPRG(txn)) != txn_start_time)) { + nrl_verbosedebug(NRL_AGENT, + "%s txn ended and/or started while in a wrapped function", + __func__); + + return; + } + + if (NR_OP_ARRAY->scope) { + nr_txn_force_single_count(NRPRG(txn), nr_txn_create_fn_supportability_metric( + nr_php_op_array_function_name(NR_OP_ARRAY), + nr_php_class_entry_name(NR_OP_ARRAY->scope))); + } else { + nr_txn_force_single_count(NRPRG(txn), nr_txn_create_fn_supportability_metric( + nr_php_op_array_function_name(NR_OP_ARRAY), + NULL)); } + /* + * Check for, and handle, frameworks. + */ + //if (wraprec->is_names_wt_simple) { + + // nr_txn_name_from_function(NRPRG(txn), + // nr_php_op_array_function_name(NR_OP_ARRAY), + // nr_php_class_entry_name(NR_OP_ARRAY->scope)); + //} } +#endif +#if ZEND_MODULE_API_NO < ZEND_8_2_X_API_NO static void nr_php_instrument_func_end(NR_EXECUTE_PROTO) { int zcaught = 0; - nr_segment_t* segment = NULL; nruserfn_t* wraprec = NULL; bool create_metric = false; +#else +static void nr_php_instrument_func_end(NR_EXECUTE_PROTO, bool create_metric) { +#endif + nr_segment_t* segment = NULL; nr_php_execute_metadata_t metadata = {0}; nrtime_t txn_start_time = 0; @@ -2017,6 +2075,7 @@ static void nr_php_instrument_func_end(NR_EXECUTE_PROTO) { return; } +#if ZEND_MODULE_API_NO < ZEND_8_2_X_API_NO wraprec = segment->wraprec; if (segment->is_exception_handler) { @@ -2034,6 +2093,9 @@ static void nr_php_instrument_func_end(NR_EXECUTE_PROTO) { NRPRG(txn), exception, nr_php_error_get_priority(E_ERROR), false, "Uncaught exception ", &NRPRG(exception_filters) TSRMLS_CC); } else if (NULL == nr_php_get_return_value(NR_EXECUTE_ORIG_ARGS)) { +#else + if (NULL == nr_php_get_return_value(NR_EXECUTE_ORIG_ARGS)) { +#endif /* * Having no return value (and not being an exception handler) indicates * that this segment had an uncaught exception. We want to add that @@ -2050,7 +2112,6 @@ static void nr_php_instrument_func_end(NR_EXECUTE_PROTO) { __func__); } } - /* * Stop the segment time now so we don't add our additional processing on to * the segment's time. @@ -2062,6 +2123,7 @@ static void nr_php_instrument_func_end(NR_EXECUTE_PROTO) { * has specifically requested it. */ +#if ZEND_MODULE_API_NO < ZEND_8_2_X_API_NO if (NULL != wraprec) { /* * This is the case for specifically requested custom instrumentation. @@ -2081,6 +2143,7 @@ static void nr_php_instrument_func_end(NR_EXECUTE_PROTO) { nr_segment_discard(&segment); return; } +#endif /* * During nr_zend_call_orig_execute_special, the transaction may have been * ended and/or a new transaction may have started. To detect this, we @@ -2120,6 +2183,9 @@ void nr_php_observer_fcall_begin(zend_execute_data* execute_data) { * nr_php_execute_show */ zval* func_return_value = NULL; + //if (execute_data->func && execute_data->func->common.function_name) { + // printf("BEGIN %s\n", ZSTR_VAL(execute_data->func->common.function_name)); + //} if (nrunlikely(NULL == execute_data)) { return; } @@ -2138,13 +2204,60 @@ 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)) { + nrl_verbosedebug(NRL_AGENT, + "Stack depth: %d after OAPI function beginning via %s", + NRPRG(php_cur_stack_depth), __func__); nr_php_show_exec(NR_EXECUTE_ORIG_ARGS); } + if (NULL == NRPRG(txn)) { + return; + } nr_php_instrument_func_begin(NR_EXECUTE_ORIG_ARGS); return; } +void nr_php_observer_fcall_begin_instrumented(zend_execute_data* execute_data) { + /* + * Instrument the function. + * This and any other needed helper functions will replace: + * nr_php_execute_enabled + * nr_php_execute + * nr_php_execute_show + */ + zval* func_return_value = NULL; + //if (execute_data->func && execute_data->func->common.function_name) { + // printf("BEGIN %s\n", ZSTR_VAL(execute_data->func->common.function_name)); + //} + if (nrunlikely(NULL == execute_data)) { + return; + } + + NRPRG(php_cur_stack_depth) += 1; + + if ((0 < ((int)NRINI(max_nesting_level))) + && (NRPRG(php_cur_stack_depth) >= (int)NRINI(max_nesting_level))) { + nr_php_max_nesting_level_reached(); + } + + if (nrunlikely(0 == nr_php_recording())) { + return; + } + + int show_executes = NR_PHP_PROCESS_GLOBALS(special_flags).show_executes; + + if (nrunlikely(show_executes)) { + nr_php_show_exec(NR_EXECUTE_ORIG_ARGS); + } + if (NULL == NRPRG(txn)) { + return; + } + nr_php_instrument_func_begin(NR_EXECUTE_ORIG_ARGS); + nr_php_observer_fcall_begin_late(execute_data, nr_txn_start_time(NRPRG(txn))); + + return; +} + void nr_php_observer_fcall_end(zend_execute_data* execute_data, zval* func_return_value) { /* @@ -2157,16 +2270,22 @@ void nr_php_observer_fcall_end(zend_execute_data* execute_data, if (nrunlikely(NULL == execute_data)) { return; } + //if (execute_data->func && execute_data->func->common.function_name) { + // printf("END %s\n", ZSTR_VAL(execute_data->func->common.function_name)); + //} if (nrlikely(1 == nr_php_recording())) { int show_executes_return = NR_PHP_PROCESS_GLOBALS(special_flags).show_execute_returns; if (nrunlikely(show_executes_return)) { + nrl_verbosedebug(NRL_AGENT, + "Stack depth: %d before OAPI function exiting via %s", + NRPRG(php_cur_stack_depth), __func__); nr_php_show_exec_return(NR_EXECUTE_ORIG_ARGS TSRMLS_CC); } - nr_php_instrument_func_end(NR_EXECUTE_ORIG_ARGS); + nr_php_instrument_func_end(NR_EXECUTE_ORIG_ARGS, false); } NRPRG(php_cur_stack_depth) -= 1; @@ -2174,4 +2293,52 @@ void nr_php_observer_fcall_end(zend_execute_data* execute_data, return; } +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO +// These empty functions (rather than NULL) are used to know if instrumentation +// has been added This is needed because the process for adding instrumentation +// with a transient wrapper differs depending on if the function has been +// previously called. These will only be used when tt_detail is 0. +void nr_php_observer_empty_fcall_begin(zend_execute_data* execute_data) { + (void)execute_data; +} + +void nr_php_observer_empty_fcall_end(zend_execute_data* execute_data, + zval* func_return_value) { + (void)execute_data; + (void)func_return_value; +} + +void nr_php_observer_fcall_end_create_metric(zend_execute_data* execute_data, + zval* func_return_value) { + /* + * Instrument the function. + * This and any other needed helper functions will replace: + * nr_php_execute_enabled + * nr_php_execute + * nr_php_execute_show + */ + if (nrunlikely(NULL == execute_data)) { + return; + } + //if (execute_data->func && execute_data->func->common.function_name) { + // printf("END %s\n", ZSTR_VAL(execute_data->func->common.function_name)); + //} + + if (nrlikely(1 == nr_php_recording())) { + int show_executes_return + = NR_PHP_PROCESS_GLOBALS(special_flags).show_execute_returns; + + if (nrunlikely(show_executes_return)) { + nr_php_show_exec_return(NR_EXECUTE_ORIG_ARGS TSRMLS_CC); + } + + nr_php_instrument_func_end(NR_EXECUTE_ORIG_ARGS, true); + } + + NRPRG(php_cur_stack_depth) -= 1; + + return; +} +#endif + #endif diff --git a/agent/php_observer.c b/agent/php_observer.c index 5719be38e..50ebbad26 100644 --- a/agent/php_observer.c +++ b/agent/php_observer.c @@ -72,6 +72,7 @@ /* * Register the begin and end function handlers with the Observer API. */ +#if ZEND_MODULE_API_NO < ZEND_8_2_X_API_NO /* PHP8+ */ static zend_observer_fcall_handlers nr_php_fcall_register_handlers( zend_execute_data* execute_data) { zend_observer_fcall_handlers handlers = {NULL, NULL}; @@ -86,6 +87,44 @@ static zend_observer_fcall_handlers nr_php_fcall_register_handlers( handlers.end = nr_php_observer_fcall_end; return handlers; } +#else +static zend_observer_fcall_handlers nr_php_fcall_register_handlers( + zend_execute_data* execute_data) { + zend_observer_fcall_handlers handlers = {NULL, NULL}; + nruserfn_t* wraprec = NULL; + if (NULL == execute_data) { + return handlers; + } + if ((NULL == execute_data->func) + || (ZEND_INTERNAL_FUNCTION == execute_data->func->type)) { + return handlers; + } + //if (execute_data->func && execute_data->func->common.function_name) { + // printf("REGISTER %s\n", ZSTR_VAL(execute_data->func->common.function_name)); + //} + wraprec = nr_php_get_wraprec(execute_data->func); + if (wraprec == NULL) { + if (0 == NRINI(tt_detail)) { + handlers.begin = nr_php_observer_empty_fcall_begin; + handlers.end = nr_php_observer_empty_fcall_end; + return handlers; + } else { + handlers.begin = nr_php_observer_fcall_begin; + handlers.end = nr_php_observer_fcall_end; + return handlers; + } + } + handlers.begin = wraprec->special_instrumentation_before ? + (zend_observer_fcall_begin_handler)wraprec->special_instrumentation_before : + nr_php_observer_fcall_begin_instrumented; + handlers.end = wraprec->special_instrumentation ? + (zend_observer_fcall_end_handler)wraprec->special_instrumentation : + wraprec->create_metric ? nr_php_observer_fcall_end_create_metric: + nr_php_observer_fcall_end; + return handlers; +} +#endif + void nr_php_observer_no_op(zend_execute_data* execute_data NRUNUSED){}; diff --git a/agent/php_observer.h b/agent/php_observer.h index 6a80873cb..69dbeb315 100644 --- a/agent/php_observer.h +++ b/agent/php_observer.h @@ -76,6 +76,20 @@ void nr_php_observer_fcall_end(zend_execute_data* execute_data, zval* func_return_value); +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO +// These empty functions (rather than NULL) are used to know if instrumentation +// has been added This is needed because the process for adding instrumentation +// with a transient wrapper differs depending on if the function has been +// previously called. These will only be used when tt_detail is 0. +void nr_php_observer_empty_fcall_begin(zend_execute_data* execute_data); +void nr_php_observer_fcall_begin_instrumented(zend_execute_data* execute_data); + +void nr_php_observer_empty_fcall_end(zend_execute_data* execute_data, + zval* func_return_value); +void nr_php_observer_fcall_begin_late(zend_execute_data* execute_data, nrtime_t txn_start_time); +void nr_php_observer_fcall_end_create_metric(zend_execute_data* execute_data, + zval* func_return_value); +#endif /* PHP 8.2+ */ #endif /* PHP8+ */ #endif // NEWRELIC_PHP_AGENT_PHP_OBSERVER_H diff --git a/agent/php_user_instrument.c b/agent/php_user_instrument.c index c7fab76d0..8ee97004c 100644 --- a/agent/php_user_instrument.c +++ b/agent/php_user_instrument.c @@ -62,6 +62,24 @@ int nr_zend_call_orig_execute(NR_EXECUTE_PROTO TSRMLS_DC) { return zcaught; } #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO /* PHP8+ */ +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO /* PHP8+ */ +int nr_zend_call_oapi_special_before(nruserfn_t* wraprec, + nr_segment_t* segment, + NR_EXECUTE_PROTO) { + volatile int zcaught = 0; + (void)segment; + + if (wraprec && wraprec->special_instrumentation_before) { + zend_try { + wraprec->special_instrumentation_before(NR_EXECUTE_ORIG_ARGS); + } + zend_catch { zcaught = 1; } + zend_end_try(); + } + + return zcaught; +} +#else int nr_zend_call_oapi_special_before(nruserfn_t* wraprec, nr_segment_t* segment, NR_EXECUTE_PROTO) { @@ -79,15 +97,23 @@ int nr_zend_call_oapi_special_before(nruserfn_t* wraprec, return zcaught; } #endif +#endif int nr_zend_call_orig_execute_special(nruserfn_t* wraprec, nr_segment_t* segment, NR_EXECUTE_PROTO TSRMLS_DC) { +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO /* PHP8+ */ + (void)segment; +#endif volatile int zcaught = 0; NR_UNUSED_FUNC_RETURN_VALUE; zend_try { if (wraprec && wraprec->special_instrumentation) { +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO /* PHP8+ */ + wraprec->special_instrumentation(NR_EXECUTE_ORIG_ARGS); +#else wraprec->special_instrumentation(wraprec, segment, NR_EXECUTE_ORIG_ARGS TSRMLS_CC); +#endif } else { NR_PHP_PROCESS_GLOBALS(orig_execute) (NR_EXECUTE_ORIG_ARGS_OVERWRITE TSRMLS_CC); @@ -228,21 +254,21 @@ static void nr_php_wrap_zend_function(zend_function* func, } } -static void nr_php_wrap_user_function_internal(nruserfn_t* wraprec TSRMLS_DC) { +static zend_function* nr_php_wrap_user_function_internal(nruserfn_t* wraprec TSRMLS_DC) { zend_function* orig_func = 0; if (0 == NR_PHP_PROCESS_GLOBALS(done_instrumentation)) { - return; + return NULL; } if (wraprec->is_wrapped) { - return; + return NULL; } #if ZEND_MODULE_API_NO < ZEND_8_0_X_API_NO \ && defined OVERWRITE_ZEND_EXECUTE_DATA /* PHP8+ */ if (nrunlikely(-1 == NR_PHP_PROCESS_GLOBALS(zend_offset))) { - return; + return NULL; } #endif if (0 == wraprec->classname) { @@ -256,7 +282,7 @@ static void nr_php_wrap_user_function_internal(nruserfn_t* wraprec TSRMLS_DC) { if (NULL == orig_func) { /* It could be in a file not yet loaded, no reason to log anything. */ - return; + return NULL; } if (ZEND_USER_FUNCTION != orig_func->type) { @@ -269,9 +295,10 @@ static void nr_php_wrap_user_function_internal(nruserfn_t* wraprec TSRMLS_DC) { * logs with this message. */ wraprec->is_disabled = 1; - return; + return NULL; } nr_php_wrap_zend_function(orig_func, wraprec TSRMLS_CC); + return orig_func; } static nruserfn_t* nr_php_user_wraprec_create(void) { @@ -423,6 +450,10 @@ nruserfn_t* nr_php_add_custom_tracer_named(const char* namestr, size_t namestrlen) { nruserfn_t* wraprec; nruserfn_t* p; + zend_function* orig_func; +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO + zend_observer_fcall_begin_handler *begin_handler; +#endif wraprec = nr_php_user_wraprec_create_named(namestr, namestrlen); if (0 == wraprec) { @@ -452,12 +483,36 @@ nruserfn_t* nr_php_add_custom_tracer_named(const char* namestr, NRP_PHP(wraprec->classname), (0 == wraprec->classname) ? "" : "::", NRP_PHP(wraprec->funcname)); - nr_php_wrap_user_function_internal(wraprec TSRMLS_CC); + orig_func = 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); +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO + if (orig_func) { + // Before messing with our handlers, we must ensure that the observer fields of the function are initialized + begin_handler = (zend_observer_fcall_begin_handler *)&ZEND_OP_ARRAY_EXTENSION((&(orig_func)->common), zend_observer_fcall_op_array_extension); + // begin_handler will be NULL if the observer hasn't been installed yet. + // *begin_Handler will be NULL if the function has not yet been called. + if (begin_handler && *begin_handler) { + if (zend_observer_remove_begin_handler(orig_func, NRINI(tt_detail) ? + nr_php_observer_fcall_begin : + nr_php_observer_empty_fcall_begin)) { + zend_observer_add_begin_handler(orig_func, wraprec->special_instrumentation_before ? + (zend_observer_fcall_begin_handler)wraprec->special_instrumentation_before : + nr_php_observer_fcall_begin_instrumented); + } + if (zend_observer_remove_end_handler(orig_func, NRINI(tt_detail) ? + nr_php_observer_fcall_end : + nr_php_observer_empty_fcall_end)) { + zend_observer_add_end_handler(orig_func, wraprec->special_instrumentation ? + (zend_observer_fcall_end_handler)wraprec->special_instrumentation : + nr_php_observer_fcall_end); + } + } + } +#endif return wraprec; /* return the new wraprec */ } diff --git a/agent/php_user_instrument.h b/agent/php_user_instrument.h index 15fba7bdf..acae87dbb 100644 --- a/agent/php_user_instrument.h +++ b/agent/php_user_instrument.h @@ -19,12 +19,17 @@ struct _nruserfn_t; * This is an unused structure that is used to ensure that a bare return won't * compile in a user wrapper. */ +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO +typedef void (*nrspecialfn_t)(zend_execute_data*, ...); + +#else struct nrspecialfn_return_t { int zcaught; }; typedef struct nrspecialfn_return_t (*nrspecialfn_t)( NR_SPECIALFNPTR_PROTO TSRMLS_DC); +#endif typedef void (*nruserfn_declared_t)(TSRMLS_D); /* diff --git a/agent/php_wrapper.c b/agent/php_wrapper.c index 21fa27f15..16a2d7090 100644 --- a/agent/php_wrapper.c +++ b/agent/php_wrapper.c @@ -140,6 +140,9 @@ nruserfn_t* nr_php_wrap_callable(zend_function* callable, nrspecialfn_t callback TSRMLS_DC) { /* creates a transient wraprec */ nruserfn_t* wraprec = nr_php_add_custom_tracer_callable(callable TSRMLS_CC); +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO + zend_observer_fcall_begin_handler *begin_handler; +#endif if (wraprec && callback) { if ((NULL != wraprec->special_instrumentation) @@ -150,6 +153,32 @@ nruserfn_t* nr_php_wrap_callable(zend_function* callable, __func__); } else { wraprec->special_instrumentation = callback; +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO + if (callable) { + // Before messing with our handlers, we must ensure that the observer fields of the function are initialized + begin_handler = (zend_observer_fcall_begin_handler *)&ZEND_OP_ARRAY_EXTENSION((&(callable)->common), zend_observer_fcall_op_array_extension); + // begin_handler will be NULL if the observer hasn't been installed yet. + // *begin_Handler will be NULL if the function has not yet been called. + if (begin_handler && *begin_handler) { + // It is okay to attempt to remove a handler that doesn't exist + // TODO this could remove nr_php_observer_fcall_begin/end and then re-add it :) + if (zend_observer_remove_begin_handler(callable, NRINI(tt_detail) ? + nr_php_observer_fcall_begin : + nr_php_observer_empty_fcall_begin)) { + zend_observer_add_begin_handler(callable, wraprec->special_instrumentation_before ? + (zend_observer_fcall_begin_handler)wraprec->special_instrumentation_before : + nr_php_observer_fcall_begin); + } + if (zend_observer_remove_end_handler(callable, NRINI(tt_detail) ? + nr_php_observer_fcall_end : + nr_php_observer_empty_fcall_end)) { + zend_observer_add_end_handler(callable, wraprec->special_instrumentation ? + (zend_observer_fcall_end_handler)wraprec->special_instrumentation : + nr_php_observer_fcall_end); + } + } + } +#endif } } diff --git a/agent/php_wrapper.h b/agent/php_wrapper.h index b87a5d9c0..486ab0e5c 100644 --- a/agent/php_wrapper.h +++ b/agent/php_wrapper.h @@ -252,9 +252,17 @@ extern void nr_php_scope_release(zval** ppzv); */ extern zval** nr_php_get_return_value_ptr(TSRMLS_D); +#if ZEND_MODULE_API_NO < ZEND_8_2_X_API_NO #define NR_PHP_WRAPPER_PROTOTYPE(name) \ struct nrspecialfn_return_t name(NR_SPECIALFNPTR_PROTO TSRMLS_DC) +#else + +#define NR_PHP_WRAPPER_PROTOTYPE(name) \ + void name(zend_execute_data* execute_data, ...) +#endif + +#if ZEND_MODULE_API_NO < ZEND_8_2_X_API_NO #define NR_PHP_WRAPPER_START(name) \ NR_PHP_WRAPPER_PROTOTYPE(name) { \ int was_executed = 0; \ @@ -264,14 +272,40 @@ extern zval** nr_php_get_return_value_ptr(TSRMLS_D); \ (void)auto_segment; // auto_segment isn't generally used in wrapper // functions +#else +#define NR_PHP_WRAPPER_START(name) \ + NR_PHP_WRAPPER_PROTOTYPE(name) { \ + int was_executed = 0; \ + int zcaught = 0; \ + bool is_begin = false; \ + nruserfn_t* wraprec = NULL; \ + zval* func_return_value = NULL; \ + zval** func_return_value_ptr = NULL; \ + const nrtxn_t* txn = NRPRG(txn); \ + const nrtime_t txn_start_time = nr_txn_start_time(txn); \ + \ + nr_segment_t* auto_segment = nr_txn_get_current_segment(NRPRG(txn), NULL); \ + if (!auto_segment || auto_segment->execute_data != execute_data) { \ + nr_php_observer_fcall_begin(execute_data); \ + auto_segment = nr_txn_get_current_segment(NRPRG(txn), NULL); \ + is_begin = true; \ + } else { \ + func_return_value_ptr = nr_php_get_return_value_ptr(); \ + func_return_value = func_return_value_ptr ? *func_return_value_ptr : NULL;\ + } +#endif #define NR_PHP_WRAPPER(name) static NR_PHP_WRAPPER_START(name) +#if ZEND_MODULE_API_NO < ZEND_8_2_X_API_NO #define NR_PHP_WRAPPER_END \ callback_end: \ __attribute__((unused)); \ if (!was_executed) { \ NR_PHP_WRAPPER_CALL \ + } \ + if (!is_begin) { \ + nr_php_observer_fcall_end(execute_data, func_return_value); \ } \ \ if (zcaught) { \ @@ -281,7 +315,27 @@ extern zval** nr_php_get_return_value_ptr(TSRMLS_D); struct nrspecialfn_return_t _retval = {zcaught}; \ return _retval; \ } \ - } +} +#else +#define NR_PHP_WRAPPER_END \ + callback_end: \ + __attribute__((unused)); \ + if (!was_executed) { \ + NR_PHP_WRAPPER_CALL \ + } \ + if (!is_begin) { \ + func_return_value_ptr = nr_php_get_return_value_ptr(); \ + nr_php_observer_fcall_end(execute_data, \ + func_return_value_ptr ? *func_return_value_ptr : NULL); \ + } else { \ + nr_php_observer_fcall_begin_late(execute_data, txn_start_time);\ + } \ + if (zcaught) { \ + zend_bailout(); \ + } \ +} +#endif + #define NR_PHP_WRAPPER_CALL \ if (!was_executed) { \ @@ -326,11 +380,19 @@ extern zval** nr_php_get_return_value_ptr(TSRMLS_D); } \ } while (0) +#if ZEND_MODULE_API_NO < ZEND_8_2_X_API_NO #define NR_PHP_WRAPPER_DELEGATE(name) \ if (!was_executed) { \ zcaught = ((name)(NR_SPECIALFNPTR_ORIG_ARGS TSRMLS_CC)).zcaught; \ was_executed = 1; \ } +#else +#define NR_PHP_WRAPPER_DELEGATE(name) \ + if (!was_executed) { \ + ((name)(execute_data)); \ + was_executed = 1; \ + } +#endif static inline bool is_instrumentation_set_and_not_equal( nrspecialfn_t instrumentation, diff --git a/axiom/nr_segment.h b/axiom/nr_segment.h index 56d972579..70fd2d3c3 100644 --- a/axiom/nr_segment.h +++ b/axiom/nr_segment.h @@ -182,7 +182,10 @@ typedef struct _nr_segment_t { external or datastore segments. */ nr_segment_error_t* error; /* segment error attributes */ -#if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \ +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO + void* execute_data; + +#elif ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \ && !defined OVERWRITE_ZEND_EXECUTE_DATA /* PHP 8.0+ and OAPI */ /* diff --git a/daemon/cmd/integration_runner/main.go b/daemon/cmd/integration_runner/main.go index f1a65a402..6edea1394 100644 --- a/daemon/cmd/integration_runner/main.go +++ b/daemon/cmd/integration_runner/main.go @@ -532,7 +532,7 @@ func runTest(t *integration.Test) { if err != nil { t.Output = body - t.Fatal(fmt.Errorf("error executing skipif: %v", err)) + t.Fatal(fmt.Errorf("error executing skipif: %v %v", err, skipIf)) return } diff --git a/tests/integration/span_events/test_span_events_max_samples_stored1.php b/tests/integration/span_events/test_span_events_max_samples_stored1.php index 93807d478..e6937bab5 100644 --- a/tests/integration/span_events/test_span_events_max_samples_stored1.php +++ b/tests/integration/span_events/test_span_events_max_samples_stored1.php @@ -29,7 +29,7 @@ newrelic_add_custom_tracer('main'); function main() { - usleep(10); + usleep(1); } $sample_size = 10000; diff --git a/tests/integration/span_events/test_span_events_on_dt_on.php b/tests/integration/span_events/test_span_events_on_dt_on.php index e94a4570a..d89c0e65c 100644 --- a/tests/integration/span_events/test_span_events_on_dt_on.php +++ b/tests/integration/span_events/test_span_events_on_dt_on.php @@ -76,6 +76,9 @@ Hello */ +if (version_compare(PHP_VERSION, "7.0", "<")) { + die("skip: CLM for PHP 5 not supported\n"); +} newrelic_add_custom_tracer('main'); function main() { From 1a90428dc9b7cb11542b1c41adb1f0eb5a1c2649 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Wed, 1 May 2024 13:09:54 -0600 Subject: [PATCH 02/23] consistent timings --- agent/php_wrapper.h | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/agent/php_wrapper.h b/agent/php_wrapper.h index 486ab0e5c..1b64e3360 100644 --- a/agent/php_wrapper.h +++ b/agent/php_wrapper.h @@ -292,6 +292,8 @@ extern zval** nr_php_get_return_value_ptr(TSRMLS_D); } else { \ func_return_value_ptr = nr_php_get_return_value_ptr(); \ func_return_value = func_return_value_ptr ? *func_return_value_ptr : NULL;\ + nr_php_observer_fcall_end(execute_data, \ + func_return_value_ptr ? *func_return_value_ptr : NULL); \ } #endif @@ -303,9 +305,6 @@ extern zval** nr_php_get_return_value_ptr(TSRMLS_D); __attribute__((unused)); \ if (!was_executed) { \ NR_PHP_WRAPPER_CALL \ - } \ - if (!is_begin) { \ - nr_php_observer_fcall_end(execute_data, func_return_value); \ } \ \ if (zcaught) { \ @@ -323,11 +322,7 @@ extern zval** nr_php_get_return_value_ptr(TSRMLS_D); if (!was_executed) { \ NR_PHP_WRAPPER_CALL \ } \ - if (!is_begin) { \ - func_return_value_ptr = nr_php_get_return_value_ptr(); \ - nr_php_observer_fcall_end(execute_data, \ - func_return_value_ptr ? *func_return_value_ptr : NULL); \ - } else { \ + if (is_begin) { \ nr_php_observer_fcall_begin_late(execute_data, txn_start_time);\ } \ if (zcaught) { \ From 942f0e2e2ebd6fd1f5ac84c634bb7d0ccddace00 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Thu, 2 May 2024 08:31:50 -0600 Subject: [PATCH 03/23] fixups --- agent/fw_wordpress.c | 4 ++++ agent/php_execute.c | 8 +++++++- agent/php_user_instrument.c | 6 +++++- 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/agent/fw_wordpress.c b/agent/fw_wordpress.c index cd905fc69..55662f3d1 100644 --- a/agent/fw_wordpress.c +++ b/agent/fw_wordpress.c @@ -747,7 +747,11 @@ NR_PHP_WRAPPER(nr_wordpress_apply_filters_after) { nr_wordpress_name_the_wt(tag, retval_ptr TSRMLS_CC); } +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO nr_wordpress_handle_tag_stack_after(execute_data); +#else + nr_wordpress_handle_tag_stack_after(NR_SPECIALFNPTR_ORIG_ARGS); +#endif } NR_PHP_WRAPPER_END #endif /* OAPI */ diff --git a/agent/php_execute.c b/agent/php_execute.c index 66601635a..6ef2a4c56 100644 --- a/agent/php_execute.c +++ b/agent/php_execute.c @@ -1984,7 +1984,7 @@ static void nr_php_instrument_func_begin(NR_EXECUTE_PROTO) { if (wraprec->is_names_wt_simple) { nr_txn_name_from_function(NRPRG(txn), - nr_php_op_array_function_name(NR_OP_ARRAY); + nr_php_op_array_function_name(NR_OP_ARRAY), nr_php_class_entry_name(NR_OP_ARRAY->scope)); } #endif @@ -2217,6 +2217,7 @@ void nr_php_observer_fcall_begin(zend_execute_data* execute_data) { return; } +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO void nr_php_observer_fcall_begin_instrumented(zend_execute_data* execute_data) { /* * Instrument the function. @@ -2257,6 +2258,7 @@ void nr_php_observer_fcall_begin_instrumented(zend_execute_data* execute_data) { return; } +#endif void nr_php_observer_fcall_end(zend_execute_data* execute_data, zval* func_return_value) { @@ -2285,7 +2287,11 @@ void nr_php_observer_fcall_end(zend_execute_data* execute_data, nr_php_show_exec_return(NR_EXECUTE_ORIG_ARGS TSRMLS_CC); } +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO nr_php_instrument_func_end(NR_EXECUTE_ORIG_ARGS, false); +#else + nr_php_instrument_func_end(NR_EXECUTE_ORIG_ARGS); +#endif } NRPRG(php_cur_stack_depth) -= 1; diff --git a/agent/php_user_instrument.c b/agent/php_user_instrument.c index 8ee97004c..62b9fb942 100644 --- a/agent/php_user_instrument.c +++ b/agent/php_user_instrument.c @@ -450,8 +450,8 @@ nruserfn_t* nr_php_add_custom_tracer_named(const char* namestr, size_t namestrlen) { nruserfn_t* wraprec; nruserfn_t* p; - zend_function* orig_func; #if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO + zend_function* orig_func; zend_observer_fcall_begin_handler *begin_handler; #endif @@ -483,7 +483,11 @@ nruserfn_t* nr_php_add_custom_tracer_named(const char* namestr, NRP_PHP(wraprec->classname), (0 == wraprec->classname) ? "" : "::", NRP_PHP(wraprec->funcname)); +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO orig_func = nr_php_wrap_user_function_internal(wraprec TSRMLS_CC); +#else + nr_php_wrap_user_function_internal(wraprec TSRMLS_CC); +#endif /* 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 From 8c97ad448a0bfcb12381d4b6ba3d40e590ba739f Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Thu, 2 May 2024 08:42:37 -0600 Subject: [PATCH 04/23] fixups --- agent/fw_drupal_common.c | 2 ++ agent/fw_wordpress.c | 6 ++++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/agent/fw_drupal_common.c b/agent/fw_drupal_common.c index 5fe24f5e3..c09aac3d6 100644 --- a/agent/fw_drupal_common.c +++ b/agent/fw_drupal_common.c @@ -60,7 +60,9 @@ NR_PHP_WRAPPER(nr_drupal_wrap_module_hook) { * function such as a_b_c is ambiguous (is the module a or a_b?). Instead, * we'll see if they're defined in the wraprec. */ +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO wraprec = nr_php_get_wraprec(execute_data->func); +#endif if ((NULL != wraprec->drupal_hook) && (NULL != wraprec->drupal_module)) { nr_drupal_create_metric(auto_segment, NR_PSTR(NR_DRUPAL_MODULE_PREFIX), wraprec->drupal_module, wraprec->drupal_module_len); diff --git a/agent/fw_wordpress.c b/agent/fw_wordpress.c index 55662f3d1..479f0e657 100644 --- a/agent/fw_wordpress.c +++ b/agent/fw_wordpress.c @@ -357,7 +357,6 @@ NR_PHP_WRAPPER(nr_wordpress_wrap_hook_plugin) { char* plugin = NULL; NR_UNUSED_SPECIALFN; - (void)wraprec; /* * We only want to hook the function being called if this is a WordPress @@ -370,7 +369,10 @@ NR_PHP_WRAPPER(nr_wordpress_wrap_hook_plugin) { if ((0 == NRINI(wordpress_hooks)) || (NULL == tag)) { NR_PHP_WRAPPER_LEAVE; } - plugin = nr_wordpress_plugin_from_function(execute_data->func); + // Use optimized wraprec hashmap over plugin hashmap + wraprec = nr_php_get_wraprec(execute_data->func); + plugin = wraprec->wordpress_plugin_theme; + //plugin = nr_wordpress_plugin_from_function(execute_data->func); NR_PHP_WRAPPER_CALL; if (NULL != plugin) { From d596581ce190722c814b4c82d89606eae1ed045a Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Fri, 3 May 2024 11:57:44 -0600 Subject: [PATCH 05/23] fix instrumented function metric --- agent/php_observer.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/agent/php_observer.c b/agent/php_observer.c index 50ebbad26..cb23d566f 100644 --- a/agent/php_observer.c +++ b/agent/php_observer.c @@ -115,12 +115,15 @@ static zend_observer_fcall_handlers nr_php_fcall_register_handlers( } } handlers.begin = wraprec->special_instrumentation_before ? - (zend_observer_fcall_begin_handler)wraprec->special_instrumentation_before : - nr_php_observer_fcall_begin_instrumented; + (zend_observer_fcall_begin_handler)wraprec->special_instrumentation_before : + wraprec->is_transient ? + nr_php_observer_fcall_begin : + nr_php_observer_fcall_begin_instrumented; handlers.end = wraprec->special_instrumentation ? - (zend_observer_fcall_end_handler)wraprec->special_instrumentation : - wraprec->create_metric ? nr_php_observer_fcall_end_create_metric: - nr_php_observer_fcall_end; + (zend_observer_fcall_end_handler)wraprec->special_instrumentation : + wraprec->create_metric ? + nr_php_observer_fcall_end_create_metric : + nr_php_observer_fcall_end; return handlers; } #endif From b983eb7c6b1a1e15794aeb96b53f82ed81b5a73f Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Tue, 7 May 2024 09:41:53 -0600 Subject: [PATCH 06/23] fix transient wrapper creation --- agent/php_wrapper.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/agent/php_wrapper.c b/agent/php_wrapper.c index 16a2d7090..f77e9e808 100644 --- a/agent/php_wrapper.c +++ b/agent/php_wrapper.c @@ -84,6 +84,9 @@ nruserfn_t* nr_php_wrap_callable_before_after( nrspecialfn_t before_callback, nrspecialfn_t after_callback) { char* name = NULL; +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO + zend_observer_fcall_begin_handler *begin_handler; +#endif /* creates a transient wraprec */ nruserfn_t* wraprec = nr_php_add_custom_tracer_callable(callable TSRMLS_CC); @@ -100,6 +103,32 @@ nruserfn_t* nr_php_wrap_callable_before_after( if (nrl_should_print(NRL_VERBOSEDEBUG, NRL_INSTRUMENT) && NULL != name) { nr_free(name); } +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO + if (callable) { + // Before messing with our handlers, we must ensure that the observer fields of the function are initialized + begin_handler = (zend_observer_fcall_begin_handler *)&ZEND_OP_ARRAY_EXTENSION((&(callable)->common), zend_observer_fcall_op_array_extension); + // begin_handler will be NULL if the observer hasn't been installed yet. + // *begin_Handler will be NULL if the function has not yet been called. + if (begin_handler && *begin_handler) { + // It is okay to attempt to remove a handler that doesn't exist + // TODO this could remove nr_php_observer_fcall_begin/end and then re-add it :) + if (zend_observer_remove_begin_handler(callable, NRINI(tt_detail) ? + nr_php_observer_fcall_begin : + nr_php_observer_empty_fcall_begin)) { + zend_observer_add_begin_handler(callable, wraprec->special_instrumentation_before ? + (zend_observer_fcall_begin_handler)wraprec->special_instrumentation_before : + nr_php_observer_fcall_begin); + } + if (zend_observer_remove_end_handler(callable, NRINI(tt_detail) ? + nr_php_observer_fcall_end : + nr_php_observer_empty_fcall_end)) { + zend_observer_add_end_handler(callable, wraprec->special_instrumentation ? + (zend_observer_fcall_end_handler)wraprec->special_instrumentation : + nr_php_observer_fcall_end); + } + } + } +#endif return wraprec; } From beede69ae7967147b316bd4df68b83a94cd55942 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Wed, 8 May 2024 11:44:23 -0600 Subject: [PATCH 07/23] fix double segments --- agent/fw_wordpress.c | 8 ++--- agent/php_execute.c | 73 +++++++++++++++++++++++++++++++++++++++++--- agent/php_newrelic.h | 5 +++ agent/php_observer.h | 3 ++ agent/php_rinit.c | 4 +++ agent/php_wrapper.h | 14 +++++++-- 6 files changed, 94 insertions(+), 13 deletions(-) diff --git a/agent/fw_wordpress.c b/agent/fw_wordpress.c index 479f0e657..af38e0be7 100644 --- a/agent/fw_wordpress.c +++ b/agent/fw_wordpress.c @@ -749,11 +749,9 @@ NR_PHP_WRAPPER(nr_wordpress_apply_filters_after) { nr_wordpress_name_the_wt(tag, retval_ptr TSRMLS_CC); } -#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO - nr_wordpress_handle_tag_stack_after(execute_data); -#else - nr_wordpress_handle_tag_stack_after(NR_SPECIALFNPTR_ORIG_ARGS); -#endif + if (0 != NRINI(wordpress_hooks)) { + clean_wordpress_tag_stack(auto_segment); + } } NR_PHP_WRAPPER_END #endif /* OAPI */ diff --git a/agent/php_execute.c b/agent/php_execute.c index 6ef2a4c56..a08f6f454 100644 --- a/agent/php_execute.c +++ b/agent/php_execute.c @@ -2031,16 +2031,40 @@ void nr_php_observer_fcall_begin_late(zend_execute_data* execute_data, nrtime_t } #endif +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO +void nr_php_observer_fcall_end_late(zend_execute_data* execute_data, bool create_metric, nrtime_t txn_start_time) { + nr_segment_t* segment; + nr_php_execute_metadata_t metadata = {0}; + if (nrunlikely(nr_txn_start_time(NRPRG(txn)) != txn_start_time)) { + nrl_verbosedebug(NRL_AGENT, + "%s txn ended and/or started while in a wrapped function", + __func__); + + return; + } + + /* + * Reassign segment to the current segment, as some before/after wraprecs + * start and then stop a segment. If that happened, we want to ensure we + * get the now-current segment + */ + segment = nr_txn_get_current_segment(NRPRG(txn), NULL); + nr_php_execute_metadata_init(&metadata, NR_OP_ARRAY); + nr_php_execute_segment_end(segment, &metadata, create_metric); + nr_php_execute_metadata_release(&metadata); +} +#endif + #if ZEND_MODULE_API_NO < ZEND_8_2_X_API_NO static void nr_php_instrument_func_end(NR_EXECUTE_PROTO) { int zcaught = 0; nruserfn_t* wraprec = NULL; bool create_metric = false; + nr_php_execute_metadata_t metadata = {0}; #else -static void nr_php_instrument_func_end(NR_EXECUTE_PROTO, bool create_metric) { +static void nr_php_instrument_func_end(NR_EXECUTE_PROTO, bool create_metric, bool end_segment) { #endif nr_segment_t* segment = NULL; - nr_php_execute_metadata_t metadata = {0}; nrtime_t txn_start_time = 0; if (NULL == NRPRG(txn)) { @@ -2143,7 +2167,6 @@ static void nr_php_instrument_func_end(NR_EXECUTE_PROTO, bool create_metric) { nr_segment_discard(&segment); return; } -#endif /* * During nr_zend_call_orig_execute_special, the transaction may have been * ended and/or a new transaction may have started. To detect this, we @@ -2171,6 +2194,11 @@ static void nr_php_instrument_func_end(NR_EXECUTE_PROTO, bool create_metric) { nr_php_execute_metadata_init(&metadata, NR_OP_ARRAY); nr_php_execute_segment_end(segment, &metadata, create_metric); nr_php_execute_metadata_release(&metadata); +#else + if (end_segment) { + nr_php_observer_fcall_end_late(execute_data, create_metric, txn_start_time); + } +#endif return; } @@ -2288,7 +2316,7 @@ void nr_php_observer_fcall_end(zend_execute_data* execute_data, } #if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO - nr_php_instrument_func_end(NR_EXECUTE_ORIG_ARGS, false); + nr_php_instrument_func_end(NR_EXECUTE_ORIG_ARGS, false, true); #else nr_php_instrument_func_end(NR_EXECUTE_ORIG_ARGS); #endif @@ -2304,6 +2332,41 @@ void nr_php_observer_fcall_end(zend_execute_data* execute_data, // has been added This is needed because the process for adding instrumentation // with a transient wrapper differs depending on if the function has been // previously called. These will only be used when tt_detail is 0. +void nr_php_observer_fcall_end_keep_segment(zend_execute_data* execute_data, + zval* func_return_value) { + /* + * Instrument the function. + * This and any other needed helper functions will replace: + * nr_php_execute_enabled + * nr_php_execute + * nr_php_execute_show + */ + if (nrunlikely(NULL == execute_data)) { + return; + } + //if (execute_data->func && execute_data->func->common.function_name) { + // printf("END %s\n", ZSTR_VAL(execute_data->func->common.function_name)); + //} + + if (nrlikely(1 == nr_php_recording())) { + int show_executes_return + = NR_PHP_PROCESS_GLOBALS(special_flags).show_execute_returns; + + if (nrunlikely(show_executes_return)) { + nrl_verbosedebug(NRL_AGENT, + "Stack depth: %d before OAPI function exiting via %s", + NRPRG(php_cur_stack_depth), __func__); + nr_php_show_exec_return(NR_EXECUTE_ORIG_ARGS TSRMLS_CC); + } + + nr_php_instrument_func_end(NR_EXECUTE_ORIG_ARGS, false, false); + } + + NRPRG(php_cur_stack_depth) -= 1; + + return; +} + void nr_php_observer_empty_fcall_begin(zend_execute_data* execute_data) { (void)execute_data; } @@ -2338,7 +2401,7 @@ void nr_php_observer_fcall_end_create_metric(zend_execute_data* execute_data, nr_php_show_exec_return(NR_EXECUTE_ORIG_ARGS TSRMLS_CC); } - nr_php_instrument_func_end(NR_EXECUTE_ORIG_ARGS, true); + nr_php_instrument_func_end(NR_EXECUTE_ORIG_ARGS, true, true); } NRPRG(php_cur_stack_depth) -= 1; diff --git a/agent/php_newrelic.h b/agent/php_newrelic.h index 1e9b5453d..b507e18b4 100644 --- a/agent/php_newrelic.h +++ b/agent/php_newrelic.h @@ -450,6 +450,11 @@ int symfony1_in_dispatch; /* Whether we are currently within a int symfony1_in_error404; /* Whether we are currently within a sfError404Exception::printStackTrace() frame */ +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO \ + && !defined OVERWRITE_ZEND_EXECUTE_DATA +bool in_wrapper; +#endif + #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \ && !defined OVERWRITE_ZEND_EXECUTE_DATA bool check_cufa; diff --git a/agent/php_observer.h b/agent/php_observer.h index 69dbeb315..566a8c8fb 100644 --- a/agent/php_observer.h +++ b/agent/php_observer.h @@ -87,6 +87,9 @@ void nr_php_observer_fcall_begin_instrumented(zend_execute_data* execute_data); void nr_php_observer_empty_fcall_end(zend_execute_data* execute_data, zval* func_return_value); void nr_php_observer_fcall_begin_late(zend_execute_data* execute_data, nrtime_t txn_start_time); +void nr_php_observer_fcall_end_keep_segment(zend_execute_data* execute_data, + zval* func_return_value); +void nr_php_observer_fcall_end_late(zend_execute_data* execute_data, bool create_metric, nrtime_t txn_start_time); void nr_php_observer_fcall_end_create_metric(zend_execute_data* execute_data, zval* func_return_value); #endif /* PHP 8.2+ */ diff --git a/agent/php_rinit.c b/agent/php_rinit.c index d704f935e..5d8634292 100644 --- a/agent/php_rinit.c +++ b/agent/php_rinit.c @@ -126,6 +126,10 @@ PHP_RINIT_FUNCTION(newrelic) { NRPRG(predis_ctxs).dtor = str_stack_dtor; NRPRG(drupal_invoke_all_hooks).dtor = zval_stack_dtor; #endif +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO \ + && !defined OVERWRITE_ZEND_EXECUTE_DATA + NRPRG(in_wrapper) = false; +#endif NRPRG(mysql_last_conn) = NULL; NRPRG(pgsql_last_conn) = NULL; diff --git a/agent/php_wrapper.h b/agent/php_wrapper.h index 1b64e3360..a8dda89ba 100644 --- a/agent/php_wrapper.h +++ b/agent/php_wrapper.h @@ -283,16 +283,21 @@ extern zval** nr_php_get_return_value_ptr(TSRMLS_D); zval** func_return_value_ptr = NULL; \ const nrtxn_t* txn = NRPRG(txn); \ const nrtime_t txn_start_time = nr_txn_start_time(txn); \ + if (NRPRG(in_wrapper)) { \ + printf("AAHHHHHHHHHHH\n"); \ + } \ + NRPRG(in_wrapper) = true; \ \ nr_segment_t* auto_segment = nr_txn_get_current_segment(NRPRG(txn), NULL); \ - if (!auto_segment || auto_segment->execute_data != execute_data) { \ + if (!auto_segment || auto_segment->execute_data != execute_data || \ + auto_segment == NRPRG(txn)->segment_root) { \ nr_php_observer_fcall_begin(execute_data); \ auto_segment = nr_txn_get_current_segment(NRPRG(txn), NULL); \ is_begin = true; \ } else { \ func_return_value_ptr = nr_php_get_return_value_ptr(); \ func_return_value = func_return_value_ptr ? *func_return_value_ptr : NULL;\ - nr_php_observer_fcall_end(execute_data, \ + nr_php_observer_fcall_end_keep_segment(execute_data, \ func_return_value_ptr ? *func_return_value_ptr : NULL); \ } #endif @@ -324,7 +329,10 @@ extern zval** nr_php_get_return_value_ptr(TSRMLS_D); } \ if (is_begin) { \ nr_php_observer_fcall_begin_late(execute_data, txn_start_time);\ - } \ + } else { \ + nr_php_observer_fcall_end_late(execute_data, false, txn_start_time); \ + } \ + NRPRG(in_wrapper) = false; \ if (zcaught) { \ zend_bailout(); \ } \ From 9810417209742424a6fe591921b7df81784f578d Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Thu, 9 May 2024 07:56:35 -0600 Subject: [PATCH 08/23] fix guzzle --- agent/lib_guzzle4.c | 13 +++++++++++++ agent/lib_guzzle4.h | 4 ++++ agent/lib_guzzle6.c | 12 ++++++++++++ agent/lib_guzzle6.h | 4 ++++ agent/php_wrapper.h | 7 ++----- 5 files changed, 35 insertions(+), 5 deletions(-) diff --git a/agent/lib_guzzle4.c b/agent/lib_guzzle4.c index c52ddffff..457054eb4 100644 --- a/agent/lib_guzzle4.c +++ b/agent/lib_guzzle4.c @@ -438,22 +438,33 @@ const zend_function_entry nr_guzzle4_subscriber_functions[] * Purpose : Registers an event subscriber for a newly instantiated * GuzzleHttp\Client object. */ + +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO +void nr_guzzle4_client_construct(NR_EXECUTE_PROTO) { +#else NR_PHP_WRAPPER_START(nr_guzzle4_client_construct) { +#endif zval* emitter = NULL; zval* retval = NULL; zval* subscriber = NULL; zval* this_var = nr_php_scope_get(NR_EXECUTE_ORIG_ARGS TSRMLS_CC); +#if ZEND_MODULE_API_NO < ZEND_8_2_X_API_NO (void)wraprec; +#endif NR_UNUSED_SPECIALFN; /* This is how we distinguish Guzzle 4/5 from other versions. */ if (0 == nr_guzzle_does_zval_implement_has_emitter(this_var TSRMLS_CC)) { +#if ZEND_MODULE_API_NO < ZEND_8_2_X_API_NO NR_PHP_WRAPPER_CALL; +#endif goto end; } +#if ZEND_MODULE_API_NO < ZEND_8_2_X_API_NO NR_PHP_WRAPPER_CALL; +#endif /* * We can't have newrelic\Guzzle4\Subscriber implement @@ -505,7 +516,9 @@ NR_PHP_WRAPPER_START(nr_guzzle4_client_construct) { nr_php_zval_free(&emitter); nr_php_zval_free(&subscriber); } +#if ZEND_MODULE_API_NO < ZEND_8_2_X_API_NO NR_PHP_WRAPPER_END +#endif void nr_guzzle4_enable(TSRMLS_D) { if (0 == NRINI(guzzle_enabled)) { diff --git a/agent/lib_guzzle4.h b/agent/lib_guzzle4.h index b1f58de99..c9eef3d4c 100644 --- a/agent/lib_guzzle4.h +++ b/agent/lib_guzzle4.h @@ -26,6 +26,10 @@ extern void nr_guzzle4_rshutdown(TSRMLS_D); /* * Purpose : Client::__construct() wrapper for Guzzle 4. */ +#if ZEND_MODULE_API_NO < ZEND_8_2_X_API_NO extern NR_PHP_WRAPPER_PROTOTYPE(nr_guzzle4_client_construct); +#else +extern void nr_guzzle4_client_construct(NR_EXECUTE_PROTO); +#endif #endif /* LIB_GUZZLE4_HDR */ diff --git a/agent/lib_guzzle6.c b/agent/lib_guzzle6.c index e65a684b7..54fe17950 100644 --- a/agent/lib_guzzle6.c +++ b/agent/lib_guzzle6.c @@ -343,7 +343,11 @@ const zend_function_entry nr_guzzle6_requesthandler_functions[] /* }}} */ +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO +void nr_guzzle6_client_construct(NR_EXECUTE_PROTO) { +#else NR_PHP_WRAPPER_START(nr_guzzle6_client_construct) { +#endif zval* config; zend_class_entry* guzzle_client_ce; zval* handler_stack; @@ -380,16 +384,22 @@ NR_PHP_WRAPPER_START(nr_guzzle6_client_construct) { version); nr_free(version); +#if ZEND_MODULE_API_NO < ZEND_8_2_X_API_NO (void)wraprec; +#endif NR_UNUSED_SPECIALFN; /* This is how we distinguish Guzzle 4/5. */ if (nr_guzzle_does_zval_implement_has_emitter(this_var TSRMLS_CC)) { +#if ZEND_MODULE_API_NO < ZEND_8_2_X_API_NO NR_PHP_WRAPPER_CALL; +#endif goto end; } +#if ZEND_MODULE_API_NO < ZEND_8_2_X_API_NO NR_PHP_WRAPPER_CALL; +#endif /* * Get our middleware callable (which is just a string), and make sure it's @@ -436,7 +446,9 @@ NR_PHP_WRAPPER_START(nr_guzzle6_client_construct) { nr_php_zval_free(&middleware); nr_php_scope_release(&this_var); } +#if ZEND_MODULE_API_NO < ZEND_8_2_X_API_NO NR_PHP_WRAPPER_END +#endif void nr_guzzle6_enable(TSRMLS_D) { int retval; diff --git a/agent/lib_guzzle6.h b/agent/lib_guzzle6.h index 67bff7ecf..e66c97915 100644 --- a/agent/lib_guzzle6.h +++ b/agent/lib_guzzle6.h @@ -20,6 +20,10 @@ extern void nr_guzzle6_minit(TSRMLS_D); /* * Purpose : Client::__construct() wrapper for Guzzle 6. */ +#if ZEND_MODULE_API_NO < ZEND_8_2_X_API_NO extern NR_PHP_WRAPPER_PROTOTYPE(nr_guzzle6_client_construct); +#else +extern void nr_guzzle6_client_construct(NR_EXECUTE_PROTO); +#endif #endif /* LIB_GUZZLE4_HDR */ diff --git a/agent/php_wrapper.h b/agent/php_wrapper.h index a8dda89ba..d374046f9 100644 --- a/agent/php_wrapper.h +++ b/agent/php_wrapper.h @@ -284,7 +284,7 @@ extern zval** nr_php_get_return_value_ptr(TSRMLS_D); const nrtxn_t* txn = NRPRG(txn); \ const nrtime_t txn_start_time = nr_txn_start_time(txn); \ if (NRPRG(in_wrapper)) { \ - printf("AAHHHHHHHHHHH\n"); \ + printf("AAHHHHHHHHHHH %s\n", #name); \ } \ NRPRG(in_wrapper) = true; \ \ @@ -391,10 +391,7 @@ extern zval** nr_php_get_return_value_ptr(TSRMLS_D); } #else #define NR_PHP_WRAPPER_DELEGATE(name) \ - if (!was_executed) { \ - ((name)(execute_data)); \ - was_executed = 1; \ - } + ((name)(execute_data, func_return_value)); #endif static inline bool is_instrumentation_set_and_not_equal( From 5c58d760766db139caacefb5e85398bffe82e035 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Fri, 10 May 2024 10:54:01 -0600 Subject: [PATCH 09/23] fix return value stuff --- agent/fw_wordpress.c | 2 -- agent/php_execute.c | 10 ++++++++-- agent/php_wrapper.h | 8 ++++---- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/agent/fw_wordpress.c b/agent/fw_wordpress.c index af38e0be7..1f3855e94 100644 --- a/agent/fw_wordpress.c +++ b/agent/fw_wordpress.c @@ -346,7 +346,6 @@ NR_PHP_WRAPPER(nr_wordpress_wrap_hook_core) { NR_PHP_WRAPPER_LEAVE; } - NR_PHP_WRAPPER_CALL; if (NRPRG(wordpress_core)) { nr_wordpress_create_metric(auto_segment, NR_WORDPRESS_HOOK_PREFIX, tag); } @@ -374,7 +373,6 @@ NR_PHP_WRAPPER(nr_wordpress_wrap_hook_plugin) { plugin = wraprec->wordpress_plugin_theme; //plugin = nr_wordpress_plugin_from_function(execute_data->func); - NR_PHP_WRAPPER_CALL; if (NULL != plugin) { nr_wordpress_create_metric(auto_segment, NR_WORDPRESS_HOOK_PREFIX, tag); nr_wordpress_create_metric(auto_segment, NR_WORDPRESS_PLUGIN_PREFIX, diff --git a/agent/php_execute.c b/agent/php_execute.c index a08f6f454..b20909707 100644 --- a/agent/php_execute.c +++ b/agent/php_execute.c @@ -2118,7 +2118,7 @@ static void nr_php_instrument_func_end(NR_EXECUTE_PROTO, bool create_metric, boo "Uncaught exception ", &NRPRG(exception_filters) TSRMLS_CC); } else if (NULL == nr_php_get_return_value(NR_EXECUTE_ORIG_ARGS)) { #else - if (NULL == nr_php_get_return_value(NR_EXECUTE_ORIG_ARGS)) { + if (NULL == func_return_value) { #endif /* * Having no return value (and not being an exception handler) indicates @@ -2130,6 +2130,9 @@ static void nr_php_instrument_func_end(NR_EXECUTE_PROTO, bool create_metric, boo nr_status_t status = nr_php_error_record_exception_segment( NRPRG(txn), &exception, &NRPRG(exception_filters)); + if (execute_data->func && execute_data->func->common.function_name) { + nrl_verbosedebug(NRL_AGENT, "END %s", ZSTR_VAL(execute_data->func->common.function_name)); + } if (NR_FAILURE == status) { nrl_verbosedebug(NRL_AGENT, "%s: unable to record exception on segment", @@ -2214,6 +2217,9 @@ void nr_php_observer_fcall_begin(zend_execute_data* execute_data) { //if (execute_data->func && execute_data->func->common.function_name) { // printf("BEGIN %s\n", ZSTR_VAL(execute_data->func->common.function_name)); //} + if (execute_data->func && execute_data->func->common.function_name) { + nrl_verbosedebug(NRL_AGENT, "BEGIN %s", ZSTR_VAL(execute_data->func->common.function_name)); + } if (nrunlikely(NULL == execute_data)) { return; } @@ -2256,7 +2262,7 @@ void nr_php_observer_fcall_begin_instrumented(zend_execute_data* execute_data) { */ zval* func_return_value = NULL; //if (execute_data->func && execute_data->func->common.function_name) { - // printf("BEGIN %s\n", ZSTR_VAL(execute_data->func->common.function_name)); + // nrl_verbosedebug(NRL_AGENT, "BEGIN %s", ZSTR_VAL(execute_data->func->common.function_name)); //} if (nrunlikely(NULL == execute_data)) { return; diff --git a/agent/php_wrapper.h b/agent/php_wrapper.h index d374046f9..107df82de 100644 --- a/agent/php_wrapper.h +++ b/agent/php_wrapper.h @@ -280,7 +280,6 @@ extern zval** nr_php_get_return_value_ptr(TSRMLS_D); bool is_begin = false; \ nruserfn_t* wraprec = NULL; \ zval* func_return_value = NULL; \ - zval** func_return_value_ptr = NULL; \ const nrtxn_t* txn = NRPRG(txn); \ const nrtime_t txn_start_time = nr_txn_start_time(txn); \ if (NRPRG(in_wrapper)) { \ @@ -295,10 +294,11 @@ extern zval** nr_php_get_return_value_ptr(TSRMLS_D); auto_segment = nr_txn_get_current_segment(NRPRG(txn), NULL); \ is_begin = true; \ } else { \ - func_return_value_ptr = nr_php_get_return_value_ptr(); \ - func_return_value = func_return_value_ptr ? *func_return_value_ptr : NULL;\ + va_list ptr; \ + va_start(ptr, execute_data); \ + func_return_value = va_arg(ptr, zval*); \ nr_php_observer_fcall_end_keep_segment(execute_data, \ - func_return_value_ptr ? *func_return_value_ptr : NULL); \ + func_return_value); \ } #endif From 7ad1ab234727bdd5f531a7cd639822c5dd002f6d Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Tue, 14 May 2024 14:30:21 -0600 Subject: [PATCH 10/23] fix wrapper tracking --- agent/php_execute.c | 8 +------- agent/php_newrelic.h | 5 ----- agent/php_rinit.c | 5 +---- agent/php_wrapper.c | 3 +++ agent/php_wrapper.h | 22 ++++++++-------------- 5 files changed, 13 insertions(+), 30 deletions(-) diff --git a/agent/php_execute.c b/agent/php_execute.c index b20909707..1fc6afb85 100644 --- a/agent/php_execute.c +++ b/agent/php_execute.c @@ -2130,9 +2130,6 @@ static void nr_php_instrument_func_end(NR_EXECUTE_PROTO, bool create_metric, boo nr_status_t status = nr_php_error_record_exception_segment( NRPRG(txn), &exception, &NRPRG(exception_filters)); - if (execute_data->func && execute_data->func->common.function_name) { - nrl_verbosedebug(NRL_AGENT, "END %s", ZSTR_VAL(execute_data->func->common.function_name)); - } if (NR_FAILURE == status) { nrl_verbosedebug(NRL_AGENT, "%s: unable to record exception on segment", @@ -2217,9 +2214,6 @@ void nr_php_observer_fcall_begin(zend_execute_data* execute_data) { //if (execute_data->func && execute_data->func->common.function_name) { // printf("BEGIN %s\n", ZSTR_VAL(execute_data->func->common.function_name)); //} - if (execute_data->func && execute_data->func->common.function_name) { - nrl_verbosedebug(NRL_AGENT, "BEGIN %s", ZSTR_VAL(execute_data->func->common.function_name)); - } if (nrunlikely(NULL == execute_data)) { return; } @@ -2262,7 +2256,7 @@ void nr_php_observer_fcall_begin_instrumented(zend_execute_data* execute_data) { */ zval* func_return_value = NULL; //if (execute_data->func && execute_data->func->common.function_name) { - // nrl_verbosedebug(NRL_AGENT, "BEGIN %s", ZSTR_VAL(execute_data->func->common.function_name)); + // printf("BEGIN %s", ZSTR_VAL(execute_data->func->common.function_name)); //} if (nrunlikely(NULL == execute_data)) { return; diff --git a/agent/php_newrelic.h b/agent/php_newrelic.h index b507e18b4..1e9b5453d 100644 --- a/agent/php_newrelic.h +++ b/agent/php_newrelic.h @@ -450,11 +450,6 @@ int symfony1_in_dispatch; /* Whether we are currently within a int symfony1_in_error404; /* Whether we are currently within a sfError404Exception::printStackTrace() frame */ -#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO \ - && !defined OVERWRITE_ZEND_EXECUTE_DATA -bool in_wrapper; -#endif - #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \ && !defined OVERWRITE_ZEND_EXECUTE_DATA bool check_cufa; diff --git a/agent/php_rinit.c b/agent/php_rinit.c index 5d8634292..922e4a884 100644 --- a/agent/php_rinit.c +++ b/agent/php_rinit.c @@ -124,12 +124,9 @@ PHP_RINIT_FUNCTION(newrelic) { nr_stack_init(&NRPRG(drupal_invoke_all_hooks), NR_STACK_DEFAULT_CAPACITY); nr_stack_init(&NRPRG(drupal_invoke_all_states), NR_STACK_DEFAULT_CAPACITY); NRPRG(predis_ctxs).dtor = str_stack_dtor; + NRPRG(wordpress_tags).dtor = str_stack_dtor; NRPRG(drupal_invoke_all_hooks).dtor = zval_stack_dtor; #endif -#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO \ - && !defined OVERWRITE_ZEND_EXECUTE_DATA - NRPRG(in_wrapper) = false; -#endif NRPRG(mysql_last_conn) = NULL; NRPRG(pgsql_last_conn) = NULL; diff --git a/agent/php_wrapper.c b/agent/php_wrapper.c index f77e9e808..29bc98250 100644 --- a/agent/php_wrapper.c +++ b/agent/php_wrapper.c @@ -110,6 +110,9 @@ nruserfn_t* nr_php_wrap_callable_before_after( // begin_handler will be NULL if the observer hasn't been installed yet. // *begin_Handler will be NULL if the function has not yet been called. if (begin_handler && *begin_handler) { + name = nr_php_function_debug_name(callable); + php_printf("AHHHHHHH %s\n", name); + nr_free(name); // It is okay to attempt to remove a handler that doesn't exist // TODO this could remove nr_php_observer_fcall_begin/end and then re-add it :) if (zend_observer_remove_begin_handler(callable, NRINI(tt_detail) ? diff --git a/agent/php_wrapper.h b/agent/php_wrapper.h index 107df82de..a5ec68f62 100644 --- a/agent/php_wrapper.h +++ b/agent/php_wrapper.h @@ -276,27 +276,22 @@ extern zval** nr_php_get_return_value_ptr(TSRMLS_D); #define NR_PHP_WRAPPER_START(name) \ NR_PHP_WRAPPER_PROTOTYPE(name) { \ int was_executed = 0; \ + bool in_begin = true;\ int zcaught = 0; \ - bool is_begin = false; \ nruserfn_t* wraprec = NULL; \ zval* func_return_value = NULL; \ const nrtxn_t* txn = NRPRG(txn); \ const nrtime_t txn_start_time = nr_txn_start_time(txn); \ - if (NRPRG(in_wrapper)) { \ - printf("AAHHHHHHHHHHH %s\n", #name); \ - } \ - NRPRG(in_wrapper) = true; \ \ nr_segment_t* auto_segment = nr_txn_get_current_segment(NRPRG(txn), NULL); \ - if (!auto_segment || auto_segment->execute_data != execute_data || \ - auto_segment == NRPRG(txn)->segment_root) { \ + if (!auto_segment || auto_segment->execute_data != execute_data) { \ nr_php_observer_fcall_begin(execute_data); \ - auto_segment = nr_txn_get_current_segment(NRPRG(txn), NULL); \ - is_begin = true; \ } else { \ - va_list ptr; \ - va_start(ptr, execute_data); \ - func_return_value = va_arg(ptr, zval*); \ + va_list args; \ + va_start(args, execute_data); \ + func_return_value = va_arg(args, zval*); \ + va_end(args); \ + in_begin = false; \ nr_php_observer_fcall_end_keep_segment(execute_data, \ func_return_value); \ } @@ -327,12 +322,11 @@ extern zval** nr_php_get_return_value_ptr(TSRMLS_D); if (!was_executed) { \ NR_PHP_WRAPPER_CALL \ } \ - if (is_begin) { \ + if (in_begin) { \ nr_php_observer_fcall_begin_late(execute_data, txn_start_time);\ } else { \ nr_php_observer_fcall_end_late(execute_data, false, txn_start_time); \ } \ - NRPRG(in_wrapper) = false; \ if (zcaught) { \ zend_bailout(); \ } \ From bd0df11abe9dbfcc989ffe5305d9e869341dd466 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Wed, 15 May 2024 09:03:53 -0600 Subject: [PATCH 11/23] try not using zend lookup --- agent/php_user_instrument.c | 2 +- agent/php_wrapper.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/agent/php_user_instrument.c b/agent/php_user_instrument.c index 62b9fb942..b607e24eb 100644 --- a/agent/php_user_instrument.c +++ b/agent/php_user_instrument.c @@ -493,7 +493,7 @@ nruserfn_t* nr_php_add_custom_tracer_named(const char* namestr, * non-transients to be freed when the linked list is disposed of which is at * module shutdown */ nr_php_add_custom_tracer_common(wraprec); -#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO +#if ZEND_MODULE_API_NO >= ZEND_8_3_X_API_NO if (orig_func) { // Before messing with our handlers, we must ensure that the observer fields of the function are initialized begin_handler = (zend_observer_fcall_begin_handler *)&ZEND_OP_ARRAY_EXTENSION((&(orig_func)->common), zend_observer_fcall_op_array_extension); diff --git a/agent/php_wrapper.c b/agent/php_wrapper.c index 29bc98250..6fd6f3c51 100644 --- a/agent/php_wrapper.c +++ b/agent/php_wrapper.c @@ -103,7 +103,7 @@ nruserfn_t* nr_php_wrap_callable_before_after( if (nrl_should_print(NRL_VERBOSEDEBUG, NRL_INSTRUMENT) && NULL != name) { nr_free(name); } -#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO +#if ZEND_MODULE_API_NO >= ZEND_8_3_X_API_NO if (callable) { // Before messing with our handlers, we must ensure that the observer fields of the function are initialized begin_handler = (zend_observer_fcall_begin_handler *)&ZEND_OP_ARRAY_EXTENSION((&(callable)->common), zend_observer_fcall_op_array_extension); @@ -185,7 +185,7 @@ nruserfn_t* nr_php_wrap_callable(zend_function* callable, __func__); } else { wraprec->special_instrumentation = callback; -#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO +#if ZEND_MODULE_API_NO >= ZEND_8_3_X_API_NO if (callable) { // Before messing with our handlers, we must ensure that the observer fields of the function are initialized begin_handler = (zend_observer_fcall_begin_handler *)&ZEND_OP_ARRAY_EXTENSION((&(callable)->common), zend_observer_fcall_op_array_extension); From 1cb4b168a42f718a1333c95c8285e52fa6a9c686 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Wed, 15 May 2024 09:07:48 -0600 Subject: [PATCH 12/23] fix --- agent/php_user_instrument.c | 4 ++-- agent/php_wrapper.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/agent/php_user_instrument.c b/agent/php_user_instrument.c index b607e24eb..3e91d13f2 100644 --- a/agent/php_user_instrument.c +++ b/agent/php_user_instrument.c @@ -450,7 +450,7 @@ nruserfn_t* nr_php_add_custom_tracer_named(const char* namestr, size_t namestrlen) { nruserfn_t* wraprec; nruserfn_t* p; -#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO +#if ZEND_MODULE_API_NO >= ZEND_8_3_X_API_NO zend_function* orig_func; zend_observer_fcall_begin_handler *begin_handler; #endif @@ -483,7 +483,7 @@ nruserfn_t* nr_php_add_custom_tracer_named(const char* namestr, NRP_PHP(wraprec->classname), (0 == wraprec->classname) ? "" : "::", NRP_PHP(wraprec->funcname)); -#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO +#if ZEND_MODULE_API_NO >= ZEND_8_3_X_API_NO orig_func = nr_php_wrap_user_function_internal(wraprec TSRMLS_CC); #else nr_php_wrap_user_function_internal(wraprec TSRMLS_CC); diff --git a/agent/php_wrapper.c b/agent/php_wrapper.c index 6fd6f3c51..62a230c47 100644 --- a/agent/php_wrapper.c +++ b/agent/php_wrapper.c @@ -84,7 +84,7 @@ nruserfn_t* nr_php_wrap_callable_before_after( nrspecialfn_t before_callback, nrspecialfn_t after_callback) { char* name = NULL; -#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO +#if ZEND_MODULE_API_NO >= ZEND_8_3_X_API_NO zend_observer_fcall_begin_handler *begin_handler; #endif @@ -172,7 +172,7 @@ nruserfn_t* nr_php_wrap_callable(zend_function* callable, nrspecialfn_t callback TSRMLS_DC) { /* creates a transient wraprec */ nruserfn_t* wraprec = nr_php_add_custom_tracer_callable(callable TSRMLS_CC); -#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO +#if ZEND_MODULE_API_NO >= ZEND_8_3_X_API_NO zend_observer_fcall_begin_handler *begin_handler; #endif From cd274ada0e142c6d60688737b61f4b5ca7600770 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Wed, 15 May 2024 11:27:29 -0600 Subject: [PATCH 13/23] revert version change --- agent/php_user_instrument.c | 6 +++--- agent/php_wrapper.c | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/agent/php_user_instrument.c b/agent/php_user_instrument.c index 3e91d13f2..62b9fb942 100644 --- a/agent/php_user_instrument.c +++ b/agent/php_user_instrument.c @@ -450,7 +450,7 @@ nruserfn_t* nr_php_add_custom_tracer_named(const char* namestr, size_t namestrlen) { nruserfn_t* wraprec; nruserfn_t* p; -#if ZEND_MODULE_API_NO >= ZEND_8_3_X_API_NO +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO zend_function* orig_func; zend_observer_fcall_begin_handler *begin_handler; #endif @@ -483,7 +483,7 @@ nruserfn_t* nr_php_add_custom_tracer_named(const char* namestr, NRP_PHP(wraprec->classname), (0 == wraprec->classname) ? "" : "::", NRP_PHP(wraprec->funcname)); -#if ZEND_MODULE_API_NO >= ZEND_8_3_X_API_NO +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO orig_func = nr_php_wrap_user_function_internal(wraprec TSRMLS_CC); #else nr_php_wrap_user_function_internal(wraprec TSRMLS_CC); @@ -493,7 +493,7 @@ nruserfn_t* nr_php_add_custom_tracer_named(const char* namestr, * non-transients to be freed when the linked list is disposed of which is at * module shutdown */ nr_php_add_custom_tracer_common(wraprec); -#if ZEND_MODULE_API_NO >= ZEND_8_3_X_API_NO +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO if (orig_func) { // Before messing with our handlers, we must ensure that the observer fields of the function are initialized begin_handler = (zend_observer_fcall_begin_handler *)&ZEND_OP_ARRAY_EXTENSION((&(orig_func)->common), zend_observer_fcall_op_array_extension); diff --git a/agent/php_wrapper.c b/agent/php_wrapper.c index 62a230c47..29bc98250 100644 --- a/agent/php_wrapper.c +++ b/agent/php_wrapper.c @@ -84,7 +84,7 @@ nruserfn_t* nr_php_wrap_callable_before_after( nrspecialfn_t before_callback, nrspecialfn_t after_callback) { char* name = NULL; -#if ZEND_MODULE_API_NO >= ZEND_8_3_X_API_NO +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO zend_observer_fcall_begin_handler *begin_handler; #endif @@ -103,7 +103,7 @@ nruserfn_t* nr_php_wrap_callable_before_after( if (nrl_should_print(NRL_VERBOSEDEBUG, NRL_INSTRUMENT) && NULL != name) { nr_free(name); } -#if ZEND_MODULE_API_NO >= ZEND_8_3_X_API_NO +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO if (callable) { // Before messing with our handlers, we must ensure that the observer fields of the function are initialized begin_handler = (zend_observer_fcall_begin_handler *)&ZEND_OP_ARRAY_EXTENSION((&(callable)->common), zend_observer_fcall_op_array_extension); @@ -172,7 +172,7 @@ nruserfn_t* nr_php_wrap_callable(zend_function* callable, nrspecialfn_t callback TSRMLS_DC) { /* creates a transient wraprec */ nruserfn_t* wraprec = nr_php_add_custom_tracer_callable(callable TSRMLS_CC); -#if ZEND_MODULE_API_NO >= ZEND_8_3_X_API_NO +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO zend_observer_fcall_begin_handler *begin_handler; #endif @@ -185,7 +185,7 @@ nruserfn_t* nr_php_wrap_callable(zend_function* callable, __func__); } else { wraprec->special_instrumentation = callback; -#if ZEND_MODULE_API_NO >= ZEND_8_3_X_API_NO +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO if (callable) { // Before messing with our handlers, we must ensure that the observer fields of the function are initialized begin_handler = (zend_observer_fcall_begin_handler *)&ZEND_OP_ARRAY_EXTENSION((&(callable)->common), zend_observer_fcall_op_array_extension); From a9656a8170fdb7bbd97823694d69ac08bf7f98a2 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Wed, 15 May 2024 14:42:59 -0600 Subject: [PATCH 14/23] TEMP remove metric --- agent/php_execute.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/agent/php_execute.c b/agent/php_execute.c index 1fc6afb85..345c45c17 100644 --- a/agent/php_execute.c +++ b/agent/php_execute.c @@ -2010,6 +2010,7 @@ void nr_php_observer_fcall_begin_late(zend_execute_data* execute_data, nrtime_t return; } + if (false) { if (NR_OP_ARRAY->scope) { nr_txn_force_single_count(NRPRG(txn), nr_txn_create_fn_supportability_metric( nr_php_op_array_function_name(NR_OP_ARRAY), @@ -2019,6 +2020,7 @@ void nr_php_observer_fcall_begin_late(zend_execute_data* execute_data, nrtime_t nr_php_op_array_function_name(NR_OP_ARRAY), NULL)); } + } /* * Check for, and handle, frameworks. */ From 6515a43e10f155d8a52caed7948f17fb0ed9af2a Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Tue, 21 May 2024 13:48:49 -0600 Subject: [PATCH 15/23] cleanup dispatch functions --- agent/php_execute.c | 154 ++++++++++--------------------------------- agent/php_observer.h | 4 -- 2 files changed, 35 insertions(+), 123 deletions(-) diff --git a/agent/php_execute.c b/agent/php_execute.c index 345c45c17..564e7fc29 100644 --- a/agent/php_execute.c +++ b/agent/php_execute.c @@ -2002,6 +2002,7 @@ void nr_php_observer_fcall_begin_late(zend_execute_data* execute_data, nrtime_t * started transaction might actually obtain the same address as a * transaction freed before. */ + (void)execute_data; if (nrunlikely(nr_txn_start_time(NRPRG(txn)) != txn_start_time)) { nrl_verbosedebug(NRL_AGENT, "%s txn ended and/or started while in a wrapped function", @@ -2010,17 +2011,6 @@ void nr_php_observer_fcall_begin_late(zend_execute_data* execute_data, nrtime_t return; } - if (false) { - if (NR_OP_ARRAY->scope) { - nr_txn_force_single_count(NRPRG(txn), nr_txn_create_fn_supportability_metric( - nr_php_op_array_function_name(NR_OP_ARRAY), - nr_php_class_entry_name(NR_OP_ARRAY->scope))); - } else { - nr_txn_force_single_count(NRPRG(txn), nr_txn_create_fn_supportability_metric( - nr_php_op_array_function_name(NR_OP_ARRAY), - NULL)); - } - } /* * Check for, and handle, frameworks. */ @@ -2204,7 +2194,11 @@ static void nr_php_instrument_func_end(NR_EXECUTE_PROTO, bool create_metric, boo return; } +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO +static void nr_php_observer_fcall_begin_common(zend_execute_data* execute_data, bool call_late) { +#else void nr_php_observer_fcall_begin(zend_execute_data* execute_data) { +#endif /* * Instrument the function. * This and any other needed helper functions will replace: @@ -2243,55 +2237,24 @@ void nr_php_observer_fcall_begin(zend_execute_data* execute_data) { return; } nr_php_instrument_func_begin(NR_EXECUTE_ORIG_ARGS); - - return; -} - #if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO -void nr_php_observer_fcall_begin_instrumented(zend_execute_data* execute_data) { - /* - * Instrument the function. - * This and any other needed helper functions will replace: - * nr_php_execute_enabled - * nr_php_execute - * nr_php_execute_show - */ - zval* func_return_value = NULL; - //if (execute_data->func && execute_data->func->common.function_name) { - // printf("BEGIN %s", ZSTR_VAL(execute_data->func->common.function_name)); - //} - if (nrunlikely(NULL == execute_data)) { - return; - } - - NRPRG(php_cur_stack_depth) += 1; - - if ((0 < ((int)NRINI(max_nesting_level))) - && (NRPRG(php_cur_stack_depth) >= (int)NRINI(max_nesting_level))) { - nr_php_max_nesting_level_reached(); + if (call_late) { + nr_php_observer_fcall_begin_late(execute_data, nr_txn_start_time(NRPRG(txn))); } - - if (nrunlikely(0 == nr_php_recording())) { - return; - } - - int show_executes = NR_PHP_PROCESS_GLOBALS(special_flags).show_executes; - - if (nrunlikely(show_executes)) { - nr_php_show_exec(NR_EXECUTE_ORIG_ARGS); - } - if (NULL == NRPRG(txn)) { - return; - } - nr_php_instrument_func_begin(NR_EXECUTE_ORIG_ARGS); - nr_php_observer_fcall_begin_late(execute_data, nr_txn_start_time(NRPRG(txn))); +#endif return; } -#endif +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO +static void nr_php_observer_fcall_end_common(zend_execute_data* execute_data, + zval* func_return_value, + bool create_metric, + bool end_segment) { +#else void nr_php_observer_fcall_end(zend_execute_data* execute_data, zval* func_return_value) { +#endif /* * Instrument the function. * This and any other needed helper functions will replace: @@ -2318,7 +2281,7 @@ void nr_php_observer_fcall_end(zend_execute_data* execute_data, } #if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO - nr_php_instrument_func_end(NR_EXECUTE_ORIG_ARGS, false, true); + nr_php_instrument_func_end(NR_EXECUTE_ORIG_ARGS, create_metric, end_segment); #else nr_php_instrument_func_end(NR_EXECUTE_ORIG_ARGS); #endif @@ -2330,45 +2293,29 @@ void nr_php_observer_fcall_end(zend_execute_data* execute_data, } #if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO +void nr_php_observer_fcall_begin(zend_execute_data* execute_data) { + nr_php_observer_fcall_begin_common(execute_data, false); +} +void nr_php_observer_fcall_begin_instrumented(zend_execute_data* execute_data) { + nr_php_observer_fcall_begin_common(execute_data, true); +} +void nr_php_observer_fcall_end(zend_execute_data* execute_data, + zval* func_return_value) { + nr_php_observer_fcall_end_common(execute_data, func_return_value, false, true); +} +void nr_php_observer_fcall_end_create_metric(zend_execute_data* execute_data, + zval* func_return_value) { + nr_php_observer_fcall_end_common(execute_data, func_return_value, true, true); +} +void nr_php_observer_fcall_end_keep_segment(zend_execute_data* execute_data, + zval* func_return_value) { + nr_php_observer_fcall_end_common(execute_data, func_return_value, false, false); +} + // These empty functions (rather than NULL) are used to know if instrumentation // has been added This is needed because the process for adding instrumentation // with a transient wrapper differs depending on if the function has been // previously called. These will only be used when tt_detail is 0. -void nr_php_observer_fcall_end_keep_segment(zend_execute_data* execute_data, - zval* func_return_value) { - /* - * Instrument the function. - * This and any other needed helper functions will replace: - * nr_php_execute_enabled - * nr_php_execute - * nr_php_execute_show - */ - if (nrunlikely(NULL == execute_data)) { - return; - } - //if (execute_data->func && execute_data->func->common.function_name) { - // printf("END %s\n", ZSTR_VAL(execute_data->func->common.function_name)); - //} - - if (nrlikely(1 == nr_php_recording())) { - int show_executes_return - = NR_PHP_PROCESS_GLOBALS(special_flags).show_execute_returns; - - if (nrunlikely(show_executes_return)) { - nrl_verbosedebug(NRL_AGENT, - "Stack depth: %d before OAPI function exiting via %s", - NRPRG(php_cur_stack_depth), __func__); - nr_php_show_exec_return(NR_EXECUTE_ORIG_ARGS TSRMLS_CC); - } - - nr_php_instrument_func_end(NR_EXECUTE_ORIG_ARGS, false, false); - } - - NRPRG(php_cur_stack_depth) -= 1; - - return; -} - void nr_php_observer_empty_fcall_begin(zend_execute_data* execute_data) { (void)execute_data; } @@ -2379,37 +2326,6 @@ void nr_php_observer_empty_fcall_end(zend_execute_data* execute_data, (void)func_return_value; } -void nr_php_observer_fcall_end_create_metric(zend_execute_data* execute_data, - zval* func_return_value) { - /* - * Instrument the function. - * This and any other needed helper functions will replace: - * nr_php_execute_enabled - * nr_php_execute - * nr_php_execute_show - */ - if (nrunlikely(NULL == execute_data)) { - return; - } - //if (execute_data->func && execute_data->func->common.function_name) { - // printf("END %s\n", ZSTR_VAL(execute_data->func->common.function_name)); - //} - - if (nrlikely(1 == nr_php_recording())) { - int show_executes_return - = NR_PHP_PROCESS_GLOBALS(special_flags).show_execute_returns; - - if (nrunlikely(show_executes_return)) { - nr_php_show_exec_return(NR_EXECUTE_ORIG_ARGS TSRMLS_CC); - } - - nr_php_instrument_func_end(NR_EXECUTE_ORIG_ARGS, true, true); - } - - NRPRG(php_cur_stack_depth) -= 1; - - return; -} #endif #endif diff --git a/agent/php_observer.h b/agent/php_observer.h index 566a8c8fb..2e45ff0d1 100644 --- a/agent/php_observer.h +++ b/agent/php_observer.h @@ -77,10 +77,6 @@ void nr_php_observer_fcall_end(zend_execute_data* execute_data, #if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO -// These empty functions (rather than NULL) are used to know if instrumentation -// has been added This is needed because the process for adding instrumentation -// with a transient wrapper differs depending on if the function has been -// previously called. These will only be used when tt_detail is 0. void nr_php_observer_empty_fcall_begin(zend_execute_data* execute_data); void nr_php_observer_fcall_begin_instrumented(zend_execute_data* execute_data); From 643009bc6b246062064d2c9185056dd33bcef156 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Wed, 22 May 2024 09:11:26 -0600 Subject: [PATCH 16/23] unify handler overwrite into function --- agent/php_observer.c | 30 ++++++++++++++++++ agent/php_observer.h | 4 +++ agent/php_user_instrument.c | 24 +-------------- agent/php_wrapper.c | 61 ++----------------------------------- 4 files changed, 38 insertions(+), 81 deletions(-) diff --git a/agent/php_observer.c b/agent/php_observer.c index cb23d566f..ee1f0e790 100644 --- a/agent/php_observer.c +++ b/agent/php_observer.c @@ -126,6 +126,36 @@ static zend_observer_fcall_handlers nr_php_fcall_register_handlers( nr_php_observer_fcall_end; return handlers; } + +bool nr_php_observer_is_registered(zend_function* func) { + zend_observer_fcall_begin_handler *begin_handler; + if (func == NULL) { + return false; + } + begin_handler = (zend_observer_fcall_begin_handler *)&ZEND_OP_ARRAY_EXTENSION((&(func)->common), zend_observer_fcall_op_array_extension); + // begin_handler will be NULL if the observer hasn't been installed yet. + // *begin_Handler will be NULL if the function has not yet been called. + return (begin_handler && *begin_handler); +} + +void nr_php_observer_overwrite_handlers(zend_function* func, nruserfn_t* wraprec) { + if (nr_php_observer_is_registered(func)) { + if (zend_observer_remove_begin_handler(func, NRINI(tt_detail) ? + nr_php_observer_fcall_begin : + nr_php_observer_empty_fcall_begin)) { + zend_observer_add_begin_handler(func, wraprec->special_instrumentation_before ? + (zend_observer_fcall_begin_handler)wraprec->special_instrumentation_before : + nr_php_observer_fcall_begin); + } + if (zend_observer_remove_end_handler(func, NRINI(tt_detail) ? + nr_php_observer_fcall_end : + nr_php_observer_empty_fcall_end)) { + zend_observer_add_end_handler(func, wraprec->special_instrumentation ? + (zend_observer_fcall_end_handler)wraprec->special_instrumentation : + nr_php_observer_fcall_end); + } + } +} #endif diff --git a/agent/php_observer.h b/agent/php_observer.h index 2e45ff0d1..0da7749a1 100644 --- a/agent/php_observer.h +++ b/agent/php_observer.h @@ -17,6 +17,7 @@ #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO /* PHP8+ */ #include "Zend/zend_observer.h" +#include "php_user_instrument.h" /* * Purpose: There are a few various places, aside from the php_execute_* family @@ -77,6 +78,9 @@ void nr_php_observer_fcall_end(zend_execute_data* execute_data, #if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO +bool nr_php_observer_is_registered(zend_function* func); +void nr_php_observer_overwrite_handlers(zend_function* func, nruserfn_t* wraprec); + void nr_php_observer_empty_fcall_begin(zend_execute_data* execute_data); void nr_php_observer_fcall_begin_instrumented(zend_execute_data* execute_data); diff --git a/agent/php_user_instrument.c b/agent/php_user_instrument.c index 62b9fb942..5e0e64069 100644 --- a/agent/php_user_instrument.c +++ b/agent/php_user_instrument.c @@ -452,7 +452,6 @@ nruserfn_t* nr_php_add_custom_tracer_named(const char* namestr, nruserfn_t* p; #if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO zend_function* orig_func; - zend_observer_fcall_begin_handler *begin_handler; #endif wraprec = nr_php_user_wraprec_create_named(namestr, namestrlen); @@ -494,28 +493,7 @@ nruserfn_t* nr_php_add_custom_tracer_named(const char* namestr, * module shutdown */ nr_php_add_custom_tracer_common(wraprec); #if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO - if (orig_func) { - // Before messing with our handlers, we must ensure that the observer fields of the function are initialized - begin_handler = (zend_observer_fcall_begin_handler *)&ZEND_OP_ARRAY_EXTENSION((&(orig_func)->common), zend_observer_fcall_op_array_extension); - // begin_handler will be NULL if the observer hasn't been installed yet. - // *begin_Handler will be NULL if the function has not yet been called. - if (begin_handler && *begin_handler) { - if (zend_observer_remove_begin_handler(orig_func, NRINI(tt_detail) ? - nr_php_observer_fcall_begin : - nr_php_observer_empty_fcall_begin)) { - zend_observer_add_begin_handler(orig_func, wraprec->special_instrumentation_before ? - (zend_observer_fcall_begin_handler)wraprec->special_instrumentation_before : - nr_php_observer_fcall_begin_instrumented); - } - if (zend_observer_remove_end_handler(orig_func, NRINI(tt_detail) ? - nr_php_observer_fcall_end : - nr_php_observer_empty_fcall_end)) { - zend_observer_add_end_handler(orig_func, wraprec->special_instrumentation ? - (zend_observer_fcall_end_handler)wraprec->special_instrumentation : - nr_php_observer_fcall_end); - } - } - } + nr_php_observer_overwrite_handlers(orig_func, wraprec); #endif return wraprec; /* return the new wraprec */ diff --git a/agent/php_wrapper.c b/agent/php_wrapper.c index 29bc98250..ffdfdd74a 100644 --- a/agent/php_wrapper.c +++ b/agent/php_wrapper.c @@ -83,10 +83,7 @@ nruserfn_t* nr_php_wrap_callable_before_after( zend_function* callable, nrspecialfn_t before_callback, nrspecialfn_t after_callback) { - char* name = NULL; -#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO - zend_observer_fcall_begin_handler *begin_handler; -#endif + char* name = NULL; /* creates a transient wraprec */ nruserfn_t* wraprec = nr_php_add_custom_tracer_callable(callable TSRMLS_CC); @@ -104,33 +101,7 @@ nruserfn_t* nr_php_wrap_callable_before_after( nr_free(name); } #if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO - if (callable) { - // Before messing with our handlers, we must ensure that the observer fields of the function are initialized - begin_handler = (zend_observer_fcall_begin_handler *)&ZEND_OP_ARRAY_EXTENSION((&(callable)->common), zend_observer_fcall_op_array_extension); - // begin_handler will be NULL if the observer hasn't been installed yet. - // *begin_Handler will be NULL if the function has not yet been called. - if (begin_handler && *begin_handler) { - name = nr_php_function_debug_name(callable); - php_printf("AHHHHHHH %s\n", name); - nr_free(name); - // It is okay to attempt to remove a handler that doesn't exist - // TODO this could remove nr_php_observer_fcall_begin/end and then re-add it :) - if (zend_observer_remove_begin_handler(callable, NRINI(tt_detail) ? - nr_php_observer_fcall_begin : - nr_php_observer_empty_fcall_begin)) { - zend_observer_add_begin_handler(callable, wraprec->special_instrumentation_before ? - (zend_observer_fcall_begin_handler)wraprec->special_instrumentation_before : - nr_php_observer_fcall_begin); - } - if (zend_observer_remove_end_handler(callable, NRINI(tt_detail) ? - nr_php_observer_fcall_end : - nr_php_observer_empty_fcall_end)) { - zend_observer_add_end_handler(callable, wraprec->special_instrumentation ? - (zend_observer_fcall_end_handler)wraprec->special_instrumentation : - nr_php_observer_fcall_end); - } - } - } + nr_php_observer_overwrite_handlers(callable, wraprec); #endif return wraprec; @@ -172,9 +143,6 @@ nruserfn_t* nr_php_wrap_callable(zend_function* callable, nrspecialfn_t callback TSRMLS_DC) { /* creates a transient wraprec */ nruserfn_t* wraprec = nr_php_add_custom_tracer_callable(callable TSRMLS_CC); -#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO - zend_observer_fcall_begin_handler *begin_handler; -#endif if (wraprec && callback) { if ((NULL != wraprec->special_instrumentation) @@ -186,30 +154,7 @@ nruserfn_t* nr_php_wrap_callable(zend_function* callable, } else { wraprec->special_instrumentation = callback; #if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO - if (callable) { - // Before messing with our handlers, we must ensure that the observer fields of the function are initialized - begin_handler = (zend_observer_fcall_begin_handler *)&ZEND_OP_ARRAY_EXTENSION((&(callable)->common), zend_observer_fcall_op_array_extension); - // begin_handler will be NULL if the observer hasn't been installed yet. - // *begin_Handler will be NULL if the function has not yet been called. - if (begin_handler && *begin_handler) { - // It is okay to attempt to remove a handler that doesn't exist - // TODO this could remove nr_php_observer_fcall_begin/end and then re-add it :) - if (zend_observer_remove_begin_handler(callable, NRINI(tt_detail) ? - nr_php_observer_fcall_begin : - nr_php_observer_empty_fcall_begin)) { - zend_observer_add_begin_handler(callable, wraprec->special_instrumentation_before ? - (zend_observer_fcall_begin_handler)wraprec->special_instrumentation_before : - nr_php_observer_fcall_begin); - } - if (zend_observer_remove_end_handler(callable, NRINI(tt_detail) ? - nr_php_observer_fcall_end : - nr_php_observer_empty_fcall_end)) { - zend_observer_add_end_handler(callable, wraprec->special_instrumentation ? - (zend_observer_fcall_end_handler)wraprec->special_instrumentation : - nr_php_observer_fcall_end); - } - } - } + nr_php_observer_overwrite_handlers(callable, wraprec); #endif } } From 61fd2254ebb9baf716d559dc45350192ba2f07b9 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Wed, 22 May 2024 09:51:48 -0600 Subject: [PATCH 17/23] add exception handler fcall_end --- agent/php_execute.c | 24 ++++++++++++++++-------- agent/php_observer.c | 12 ++++++++---- agent/php_observer.h | 6 ++++-- 3 files changed, 28 insertions(+), 14 deletions(-) diff --git a/agent/php_execute.c b/agent/php_execute.c index 564e7fc29..5b57a81fc 100644 --- a/agent/php_execute.c +++ b/agent/php_execute.c @@ -2054,7 +2054,7 @@ static void nr_php_instrument_func_end(NR_EXECUTE_PROTO) { bool create_metric = false; nr_php_execute_metadata_t metadata = {0}; #else -static void nr_php_instrument_func_end(NR_EXECUTE_PROTO, bool create_metric, bool end_segment) { +static void nr_php_instrument_func_end(NR_EXECUTE_PROTO, bool create_metric, bool end_segment, bool is_exception_handler) { #endif nr_segment_t* segment = NULL; nrtime_t txn_start_time = 0; @@ -2093,8 +2093,10 @@ static void nr_php_instrument_func_end(NR_EXECUTE_PROTO, bool create_metric, boo #if ZEND_MODULE_API_NO < ZEND_8_2_X_API_NO wraprec = segment->wraprec; - if (segment->is_exception_handler) { +#else + if (is_exception_handler) { +#endif /* * After running the exception handler segment, create an error from * the exception it handled, and save the error in the transaction. @@ -2108,9 +2110,10 @@ static void nr_php_instrument_func_end(NR_EXECUTE_PROTO, bool create_metric, boo nr_php_error_record_exception( NRPRG(txn), exception, nr_php_error_get_priority(E_ERROR), false, "Uncaught exception ", &NRPRG(exception_filters) TSRMLS_CC); +#if ZEND_MODULE_API_NO < ZEND_8_2_X_API_NO } else if (NULL == nr_php_get_return_value(NR_EXECUTE_ORIG_ARGS)) { #else - if (NULL == func_return_value) { + } else if (NULL == func_return_value) { #endif /* * Having no return value (and not being an exception handler) indicates @@ -2250,7 +2253,8 @@ void nr_php_observer_fcall_begin(zend_execute_data* execute_data) { static void nr_php_observer_fcall_end_common(zend_execute_data* execute_data, zval* func_return_value, bool create_metric, - bool end_segment) { + bool end_segment, + bool is_exception_handler) { #else void nr_php_observer_fcall_end(zend_execute_data* execute_data, zval* func_return_value) { @@ -2281,7 +2285,7 @@ void nr_php_observer_fcall_end(zend_execute_data* execute_data, } #if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO - nr_php_instrument_func_end(NR_EXECUTE_ORIG_ARGS, create_metric, end_segment); + nr_php_instrument_func_end(NR_EXECUTE_ORIG_ARGS, create_metric, end_segment, is_exception_handler); #else nr_php_instrument_func_end(NR_EXECUTE_ORIG_ARGS); #endif @@ -2301,15 +2305,19 @@ void nr_php_observer_fcall_begin_instrumented(zend_execute_data* execute_data) { } void nr_php_observer_fcall_end(zend_execute_data* execute_data, zval* func_return_value) { - nr_php_observer_fcall_end_common(execute_data, func_return_value, false, true); + nr_php_observer_fcall_end_common(execute_data, func_return_value, false, true, false); } void nr_php_observer_fcall_end_create_metric(zend_execute_data* execute_data, zval* func_return_value) { - nr_php_observer_fcall_end_common(execute_data, func_return_value, true, true); + nr_php_observer_fcall_end_common(execute_data, func_return_value, true, true, false); } void nr_php_observer_fcall_end_keep_segment(zend_execute_data* execute_data, zval* func_return_value) { - nr_php_observer_fcall_end_common(execute_data, func_return_value, false, false); + nr_php_observer_fcall_end_common(execute_data, func_return_value, false, false, false); +} +void nr_php_observer_fcall_end_exception_handler(zend_execute_data* execute_data, + zval* func_return_value) { + nr_php_observer_fcall_end_common(execute_data, func_return_value, false, true, true); } // These empty functions (rather than NULL) are used to know if instrumentation diff --git a/agent/php_observer.c b/agent/php_observer.c index ee1f0e790..ffea6aaca 100644 --- a/agent/php_observer.c +++ b/agent/php_observer.c @@ -121,9 +121,11 @@ static zend_observer_fcall_handlers nr_php_fcall_register_handlers( nr_php_observer_fcall_begin_instrumented; handlers.end = wraprec->special_instrumentation ? (zend_observer_fcall_end_handler)wraprec->special_instrumentation : - wraprec->create_metric ? - nr_php_observer_fcall_end_create_metric : - nr_php_observer_fcall_end; + wraprec->is_exception_handler ? + nr_php_observer_fcall_end_exception_handler : + wraprec->create_metric ? + nr_php_observer_fcall_end_create_metric : + nr_php_observer_fcall_end; return handlers; } @@ -152,7 +154,9 @@ void nr_php_observer_overwrite_handlers(zend_function* func, nruserfn_t* wraprec nr_php_observer_empty_fcall_end)) { zend_observer_add_end_handler(func, wraprec->special_instrumentation ? (zend_observer_fcall_end_handler)wraprec->special_instrumentation : - nr_php_observer_fcall_end); + wraprec->is_exception_handler ? + nr_php_observer_fcall_end_exception_handler : + nr_php_observer_fcall_end); } } } diff --git a/agent/php_observer.h b/agent/php_observer.h index 0da7749a1..352d198ad 100644 --- a/agent/php_observer.h +++ b/agent/php_observer.h @@ -88,10 +88,12 @@ void nr_php_observer_empty_fcall_end(zend_execute_data* execute_data, zval* func_return_value); void nr_php_observer_fcall_begin_late(zend_execute_data* execute_data, nrtime_t txn_start_time); void nr_php_observer_fcall_end_keep_segment(zend_execute_data* execute_data, - zval* func_return_value); + zval* func_return_value); void nr_php_observer_fcall_end_late(zend_execute_data* execute_data, bool create_metric, nrtime_t txn_start_time); void nr_php_observer_fcall_end_create_metric(zend_execute_data* execute_data, - zval* func_return_value); + zval* func_return_value); +void nr_php_observer_fcall_end_exception_handler(zend_execute_data* execute_data, + zval* func_return_value); #endif /* PHP 8.2+ */ #endif /* PHP8+ */ From 1a9b95381265d2b5c39e3df290dd4c2211cd10c0 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Wed, 22 May 2024 12:15:46 -0600 Subject: [PATCH 18/23] InstrumentedFunction supp metric on first call only --- agent/php_observer.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/agent/php_observer.c b/agent/php_observer.c index ffea6aaca..a6d5f1195 100644 --- a/agent/php_observer.c +++ b/agent/php_observer.c @@ -115,17 +115,18 @@ static zend_observer_fcall_handlers nr_php_fcall_register_handlers( } } handlers.begin = wraprec->special_instrumentation_before ? - (zend_observer_fcall_begin_handler)wraprec->special_instrumentation_before : + (zend_observer_fcall_begin_handler) wraprec->special_instrumentation_before : wraprec->is_transient ? nr_php_observer_fcall_begin : nr_php_observer_fcall_begin_instrumented; handlers.end = wraprec->special_instrumentation ? - (zend_observer_fcall_end_handler)wraprec->special_instrumentation : + (zend_observer_fcall_end_handler) wraprec->special_instrumentation : wraprec->is_exception_handler ? nr_php_observer_fcall_end_exception_handler : wraprec->create_metric ? nr_php_observer_fcall_end_create_metric : nr_php_observer_fcall_end; + nr_txn_force_single_count(NRPRG(txn), wraprec->supportability_metric); return handlers; } From f01406bbf9de5239154dc2f77c2e688166a7105e Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Thu, 23 May 2024 10:12:44 -0600 Subject: [PATCH 19/23] add metric for ini instrumented --- agent/php_execute.c | 30 ++++++++++++++++++++---------- agent/php_observer.c | 4 +++- agent/php_observer.h | 3 ++- agent/php_wrapper.h | 2 +- 4 files changed, 26 insertions(+), 13 deletions(-) diff --git a/agent/php_execute.c b/agent/php_execute.c index 5b57a81fc..cfb9be43f 100644 --- a/agent/php_execute.c +++ b/agent/php_execute.c @@ -1991,7 +1991,7 @@ static void nr_php_instrument_func_begin(NR_EXECUTE_PROTO) { } #if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO -void nr_php_observer_fcall_begin_late(zend_execute_data* execute_data, nrtime_t txn_start_time) { +void nr_php_observer_fcall_begin_late(zend_execute_data* execute_data, nrtime_t txn_start_time, bool name_transaction) { /* * During nr_zend_call_oapi_special_before, the transaction may have been * ended and/or a new transaction may have started. To detect this, we @@ -2014,12 +2014,14 @@ void nr_php_observer_fcall_begin_late(zend_execute_data* execute_data, nrtime_t /* * Check for, and handle, frameworks. */ - //if (wraprec->is_names_wt_simple) { + if (name_transaction) { - // nr_txn_name_from_function(NRPRG(txn), - // nr_php_op_array_function_name(NR_OP_ARRAY), - // nr_php_class_entry_name(NR_OP_ARRAY->scope)); - //} + nr_txn_name_from_function(NRPRG(txn), + nr_php_op_array_function_name(NR_OP_ARRAY), + NR_OP_ARRAY->scope ? + nr_php_class_entry_name(NR_OP_ARRAY->scope) : + NULL); + } } #endif @@ -2198,7 +2200,7 @@ static void nr_php_instrument_func_end(NR_EXECUTE_PROTO, bool create_metric, boo } #if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO -static void nr_php_observer_fcall_begin_common(zend_execute_data* execute_data, bool call_late) { +static void nr_php_observer_fcall_begin_common(zend_execute_data* execute_data, bool call_late, bool name_transaction) { #else void nr_php_observer_fcall_begin(zend_execute_data* execute_data) { #endif @@ -2242,7 +2244,7 @@ void nr_php_observer_fcall_begin(zend_execute_data* execute_data) { nr_php_instrument_func_begin(NR_EXECUTE_ORIG_ARGS); #if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO if (call_late) { - nr_php_observer_fcall_begin_late(execute_data, nr_txn_start_time(NRPRG(txn))); + nr_php_observer_fcall_begin_late(execute_data, nr_txn_start_time(NRPRG(txn)), name_transaction); } #endif @@ -2298,10 +2300,13 @@ void nr_php_observer_fcall_end(zend_execute_data* execute_data, #if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO void nr_php_observer_fcall_begin(zend_execute_data* execute_data) { - nr_php_observer_fcall_begin_common(execute_data, false); + nr_php_observer_fcall_begin_common(execute_data, false, false); } void nr_php_observer_fcall_begin_instrumented(zend_execute_data* execute_data) { - nr_php_observer_fcall_begin_common(execute_data, true); + nr_php_observer_fcall_begin_common(execute_data, true, false); +} +void nr_php_observer_fcall_begin_name_transaction(zend_execute_data* execute_data) { + nr_php_observer_fcall_begin_common(execute_data, true, true); } void nr_php_observer_fcall_end(zend_execute_data* execute_data, zval* func_return_value) { @@ -2332,6 +2337,11 @@ void nr_php_observer_empty_fcall_end(zend_execute_data* execute_data, zval* func_return_value) { (void)execute_data; (void)func_return_value; + + /* need a way to register framework info while tt_detail is 0 */ + if (nrunlikely(OP_ARRAY_IS_A_FILE(NR_OP_ARRAY))) { + nr_php_execute_file(NR_OP_ARRAY, NR_EXECUTE_ORIG_ARGS); + } } #endif diff --git a/agent/php_observer.c b/agent/php_observer.c index a6d5f1195..093078321 100644 --- a/agent/php_observer.c +++ b/agent/php_observer.c @@ -118,7 +118,9 @@ static zend_observer_fcall_handlers nr_php_fcall_register_handlers( (zend_observer_fcall_begin_handler) wraprec->special_instrumentation_before : wraprec->is_transient ? nr_php_observer_fcall_begin : - nr_php_observer_fcall_begin_instrumented; + wraprec->is_names_wt_simple ? + nr_php_observer_fcall_begin_name_transaction : + nr_php_observer_fcall_begin_instrumented; handlers.end = wraprec->special_instrumentation ? (zend_observer_fcall_end_handler) wraprec->special_instrumentation : wraprec->is_exception_handler ? diff --git a/agent/php_observer.h b/agent/php_observer.h index 352d198ad..c789f1518 100644 --- a/agent/php_observer.h +++ b/agent/php_observer.h @@ -83,10 +83,11 @@ void nr_php_observer_overwrite_handlers(zend_function* func, nruserfn_t* wraprec void nr_php_observer_empty_fcall_begin(zend_execute_data* execute_data); void nr_php_observer_fcall_begin_instrumented(zend_execute_data* execute_data); +void nr_php_observer_fcall_begin_name_transaction(zend_execute_data* execute_data); void nr_php_observer_empty_fcall_end(zend_execute_data* execute_data, zval* func_return_value); -void nr_php_observer_fcall_begin_late(zend_execute_data* execute_data, nrtime_t txn_start_time); +void nr_php_observer_fcall_begin_late(zend_execute_data* execute_data, nrtime_t txn_start_time, bool name_transaction); void nr_php_observer_fcall_end_keep_segment(zend_execute_data* execute_data, zval* func_return_value); void nr_php_observer_fcall_end_late(zend_execute_data* execute_data, bool create_metric, nrtime_t txn_start_time); diff --git a/agent/php_wrapper.h b/agent/php_wrapper.h index a5ec68f62..23fa7539c 100644 --- a/agent/php_wrapper.h +++ b/agent/php_wrapper.h @@ -323,7 +323,7 @@ extern zval** nr_php_get_return_value_ptr(TSRMLS_D); NR_PHP_WRAPPER_CALL \ } \ if (in_begin) { \ - nr_php_observer_fcall_begin_late(execute_data, txn_start_time);\ + nr_php_observer_fcall_begin_late(execute_data, txn_start_time, false);\ } else { \ nr_php_observer_fcall_end_late(execute_data, false, txn_start_time); \ } \ From 5c966b29afd7b9bcf4bbb7ba1b92838132027b1f Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Wed, 29 May 2024 11:13:26 -0600 Subject: [PATCH 20/23] fixup --- agent/php_observer.c | 124 +++++++++++++++++++++++------------- agent/php_observer.h | 9 ++- agent/php_user_instrument.c | 52 ++++++++++----- agent/php_user_instrument.h | 3 + agent/php_wrapper.c | 60 ++++++++++++----- 5 files changed, 173 insertions(+), 75 deletions(-) diff --git a/agent/php_observer.c b/agent/php_observer.c index 093078321..9a60eea86 100644 --- a/agent/php_observer.c +++ b/agent/php_observer.c @@ -87,7 +87,55 @@ static zend_observer_fcall_handlers nr_php_fcall_register_handlers( handlers.end = nr_php_observer_fcall_end; return handlers; } + #else + +static zend_observer_fcall_begin_handler nr_php_observer_determine_begin_handler(nruserfn_t* wraprec) { + zend_observer_fcall_begin_handler begin = NULL; + + if (NULL == wraprec) { + if (NRINI(tt_detail)) { + begin = nr_php_observer_fcall_begin; + } else { + begin = nr_php_observer_empty_fcall_begin; + } + } else { + if (wraprec->special_instrumentation_before) { + begin = (zend_observer_fcall_begin_handler)wraprec->special_instrumentation_before; + } else if (wraprec->is_names_wt_simple) { + begin = nr_php_observer_fcall_begin_name_transaction; + } else if (wraprec->is_transient) { + begin = nr_php_observer_fcall_begin; + } else { + begin = nr_php_observer_fcall_begin_instrumented; + } + } + return begin; +} + +static zend_observer_fcall_end_handler nr_php_observer_determine_end_handler(nruserfn_t* wraprec) { + zend_observer_fcall_end_handler end = NULL; + + if (NULL == wraprec) { + if (NRINI(tt_detail)) { + end = nr_php_observer_fcall_end; + } else { + end = nr_php_observer_empty_fcall_end; + } + } else { + if (wraprec->special_instrumentation) { + end = (zend_observer_fcall_end_handler)wraprec->special_instrumentation; + } else if (wraprec->is_exception_handler) { + end = nr_php_observer_fcall_end_exception_handler; + } else if (wraprec->create_metric) { + end = nr_php_observer_fcall_end_create_metric; + } else { + end = nr_php_observer_fcall_end; + } + } + return end; +} + static zend_observer_fcall_handlers nr_php_fcall_register_handlers( zend_execute_data* execute_data) { zend_observer_fcall_handlers handlers = {NULL, NULL}; @@ -103,32 +151,11 @@ static zend_observer_fcall_handlers nr_php_fcall_register_handlers( // printf("REGISTER %s\n", ZSTR_VAL(execute_data->func->common.function_name)); //} wraprec = nr_php_get_wraprec(execute_data->func); - if (wraprec == NULL) { - if (0 == NRINI(tt_detail)) { - handlers.begin = nr_php_observer_empty_fcall_begin; - handlers.end = nr_php_observer_empty_fcall_end; - return handlers; - } else { - handlers.begin = nr_php_observer_fcall_begin; - handlers.end = nr_php_observer_fcall_end; - return handlers; - } + handlers.begin = nr_php_observer_determine_begin_handler(wraprec); + handlers.end = nr_php_observer_determine_end_handler(wraprec); + if (wraprec) { + nr_txn_force_single_count(NRPRG(txn), wraprec->supportability_metric); } - handlers.begin = wraprec->special_instrumentation_before ? - (zend_observer_fcall_begin_handler) wraprec->special_instrumentation_before : - wraprec->is_transient ? - nr_php_observer_fcall_begin : - wraprec->is_names_wt_simple ? - nr_php_observer_fcall_begin_name_transaction : - nr_php_observer_fcall_begin_instrumented; - handlers.end = wraprec->special_instrumentation ? - (zend_observer_fcall_end_handler) wraprec->special_instrumentation : - wraprec->is_exception_handler ? - nr_php_observer_fcall_end_exception_handler : - wraprec->create_metric ? - nr_php_observer_fcall_end_create_metric : - nr_php_observer_fcall_end; - nr_txn_force_single_count(NRPRG(txn), wraprec->supportability_metric); return handlers; } @@ -143,25 +170,36 @@ bool nr_php_observer_is_registered(zend_function* func) { return (begin_handler && *begin_handler); } -void nr_php_observer_overwrite_handlers(zend_function* func, nruserfn_t* wraprec) { - if (nr_php_observer_is_registered(func)) { - if (zend_observer_remove_begin_handler(func, NRINI(tt_detail) ? - nr_php_observer_fcall_begin : - nr_php_observer_empty_fcall_begin)) { - zend_observer_add_begin_handler(func, wraprec->special_instrumentation_before ? - (zend_observer_fcall_begin_handler)wraprec->special_instrumentation_before : - nr_php_observer_fcall_begin); - } - if (zend_observer_remove_end_handler(func, NRINI(tt_detail) ? - nr_php_observer_fcall_end : - nr_php_observer_empty_fcall_end)) { - zend_observer_add_end_handler(func, wraprec->special_instrumentation ? - (zend_observer_fcall_end_handler)wraprec->special_instrumentation : - wraprec->is_exception_handler ? - nr_php_observer_fcall_end_exception_handler : - nr_php_observer_fcall_end); - } +bool nr_php_observer_remove_begin_handler(zend_function* func, nruserfn_t* wraprec) { + if (!nr_php_observer_is_registered(func)) { + return false; + } + zend_observer_fcall_begin_handler begin = nr_php_observer_determine_begin_handler(wraprec);; + return zend_observer_remove_begin_handler(func, begin); +} + +bool nr_php_observer_remove_end_handler(zend_function* func, nruserfn_t* wraprec) { + if (!nr_php_observer_is_registered(func)) { + return false; + } + zend_observer_fcall_end_handler end = nr_php_observer_determine_end_handler(wraprec); + return zend_observer_remove_end_handler(func, end); +} + +void nr_php_observer_add_begin_handler(zend_function* func, nruserfn_t* wraprec) { + if (!nr_php_observer_is_registered(func)) { + return; + } + zend_observer_fcall_begin_handler begin = nr_php_observer_determine_begin_handler(wraprec);; + zend_observer_add_begin_handler(func, begin); +} + +void nr_php_observer_add_end_handler(zend_function* func, nruserfn_t* wraprec) { + if (!nr_php_observer_is_registered(func)) { + return; } + zend_observer_fcall_end_handler end = nr_php_observer_determine_end_handler(wraprec); + zend_observer_add_end_handler(func, end); } #endif diff --git a/agent/php_observer.h b/agent/php_observer.h index c789f1518..f6069f7bd 100644 --- a/agent/php_observer.h +++ b/agent/php_observer.h @@ -79,8 +79,15 @@ void nr_php_observer_fcall_end(zend_execute_data* execute_data, #if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO bool nr_php_observer_is_registered(zend_function* func); -void nr_php_observer_overwrite_handlers(zend_function* func, nruserfn_t* wraprec); +bool nr_php_observer_remove_begin_handler(zend_function* func, nruserfn_t* wraprec); +bool nr_php_observer_remove_end_handler(zend_function* func, nruserfn_t* wraprec); +void nr_php_observer_add_begin_handler(zend_function* func, nruserfn_t* wraprec); +void nr_php_observer_add_end_handler(zend_function* func, nruserfn_t* wraprec); +/* + * These different forms of fcall_begin and fcall_end are needed to properly utilize + * the fields in a wraprec without looking it up every call. +*/ void nr_php_observer_empty_fcall_begin(zend_execute_data* execute_data); void nr_php_observer_fcall_begin_instrumented(zend_execute_data* execute_data); void nr_php_observer_fcall_begin_name_transaction(zend_execute_data* execute_data); diff --git a/agent/php_user_instrument.c b/agent/php_user_instrument.c index 5e0e64069..6d2eb9a7c 100644 --- a/agent/php_user_instrument.c +++ b/agent/php_user_instrument.c @@ -254,21 +254,21 @@ static void nr_php_wrap_zend_function(zend_function* func, } } -static zend_function* nr_php_wrap_user_function_internal(nruserfn_t* wraprec TSRMLS_DC) { +static void nr_php_wrap_user_function_internal(nruserfn_t* wraprec TSRMLS_DC) { zend_function* orig_func = 0; if (0 == NR_PHP_PROCESS_GLOBALS(done_instrumentation)) { - return NULL; + return; } if (wraprec->is_wrapped) { - return NULL; + return; } #if ZEND_MODULE_API_NO < ZEND_8_0_X_API_NO \ && defined OVERWRITE_ZEND_EXECUTE_DATA /* PHP8+ */ if (nrunlikely(-1 == NR_PHP_PROCESS_GLOBALS(zend_offset))) { - return NULL; + return; } #endif if (0 == wraprec->classname) { @@ -282,8 +282,9 @@ static zend_function* nr_php_wrap_user_function_internal(nruserfn_t* wraprec TSR if (NULL == orig_func) { /* It could be in a file not yet loaded, no reason to log anything. */ - return NULL; + return; } + wraprec->func = orig_func; if (ZEND_USER_FUNCTION != orig_func->type) { nrl_verbosedebug(NRL_INSTRUMENT, "%s%s%s is not a user function", @@ -295,10 +296,9 @@ static zend_function* nr_php_wrap_user_function_internal(nruserfn_t* wraprec TSR * logs with this message. */ wraprec->is_disabled = 1; - return NULL; + return; } nr_php_wrap_zend_function(orig_func, wraprec TSRMLS_CC); - return orig_func; } static nruserfn_t* nr_php_user_wraprec_create(void) { @@ -429,6 +429,10 @@ nruserfn_t* nr_php_add_custom_tracer_callable(zend_function* func TSRMLS_DC) { nrl_verbosedebug(NRL_INSTRUMENT, "reusing custom wrapper for callable '%s'", name); nr_free(name); +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO + nr_php_observer_remove_begin_handler(func, wraprec); + nr_php_observer_remove_end_handler(func, wraprec); +#endif return wraprec; } @@ -442,6 +446,10 @@ nruserfn_t* nr_php_add_custom_tracer_callable(zend_function* func TSRMLS_DC) { #if ZEND_MODULE_API_NO < ZEND_7_4_X_API_NO nr_php_add_custom_tracer_common(wraprec); #endif +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO + nr_php_observer_remove_begin_handler(func, NULL); + nr_php_observer_remove_end_handler(func, NULL); +#endif return wraprec; } @@ -450,9 +458,6 @@ nruserfn_t* nr_php_add_custom_tracer_named(const char* namestr, size_t namestrlen) { nruserfn_t* wraprec; nruserfn_t* p; -#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO - zend_function* orig_func; -#endif wraprec = nr_php_user_wraprec_create_named(namestr, namestrlen); if (0 == wraprec) { @@ -472,6 +477,10 @@ nruserfn_t* nr_php_add_custom_tracer_named(const char* namestr, nr_php_user_wraprec_destroy(&wraprec); nr_php_wrap_user_function_internal(p TSRMLS_CC); +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO + nr_php_observer_remove_begin_handler(p->func, wraprec); + nr_php_observer_remove_end_handler(p->func, wraprec); +#endif return p; /* return the wraprec we are duplicating */ } p = p->next; @@ -482,18 +491,15 @@ nruserfn_t* nr_php_add_custom_tracer_named(const char* namestr, NRP_PHP(wraprec->classname), (0 == wraprec->classname) ? "" : "::", NRP_PHP(wraprec->funcname)); -#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO - orig_func = nr_php_wrap_user_function_internal(wraprec TSRMLS_CC); -#else nr_php_wrap_user_function_internal(wraprec TSRMLS_CC); -#endif /* 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); #if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO - nr_php_observer_overwrite_handlers(orig_func, wraprec); + nr_php_observer_remove_begin_handler(wraprec->func, NULL); + nr_php_observer_remove_end_handler(wraprec->func, NULL); #endif return wraprec; /* return the new wraprec */ @@ -579,6 +585,10 @@ void nr_php_add_transaction_naming_function(const char* namestr, if (NULL != wraprec) { wraprec->is_names_wt_simple = 1; +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO + nr_php_observer_add_begin_handler(wraprec->func, wraprec); + nr_php_observer_add_end_handler(wraprec->func, wraprec); +#endif } } @@ -588,6 +598,10 @@ void nr_php_add_custom_tracer(const char* namestr, int namestrlen TSRMLS_DC) { if (NULL != wraprec) { wraprec->create_metric = 1; wraprec->is_user_added = 1; +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO + nr_php_observer_add_begin_handler(wraprec->func, wraprec); + nr_php_observer_add_end_handler(wraprec->func, wraprec); +#endif } } @@ -596,6 +610,10 @@ void nr_php_add_exception_function(zend_function* func TSRMLS_DC) { if (wraprec) { wraprec->is_exception_handler = 1; +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO + nr_php_observer_add_begin_handler(func, wraprec); + nr_php_observer_add_end_handler(func, wraprec); +#endif } } @@ -651,6 +669,10 @@ void nr_php_user_function_add_declared_callback(const char* namestr, if (wraprec->is_wrapped && callback) { (callback)(TSRMLS_C); } +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO + nr_php_observer_add_begin_handler(wraprec->func, wraprec); + nr_php_observer_add_end_handler(wraprec->func, wraprec); +#endif } } diff --git a/agent/php_user_instrument.h b/agent/php_user_instrument.h index acae87dbb..f42e50cd4 100644 --- a/agent/php_user_instrument.h +++ b/agent/php_user_instrument.h @@ -110,6 +110,9 @@ typedef struct _nruserfn_t { #if ZEND_MODULE_API_NO >= ZEND_7_4_X_API_NO char* wordpress_plugin_theme; #endif +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO + zend_function* func; /* the underlying function that this wraprec wraps */ +#endif } nruserfn_t; extern nruserfn_t* nr_wrapped_user_functions; /* a singly linked list */ diff --git a/agent/php_wrapper.c b/agent/php_wrapper.c index ffdfdd74a..c34482321 100644 --- a/agent/php_wrapper.c +++ b/agent/php_wrapper.c @@ -9,13 +9,13 @@ #include "util_logging.h" #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO -static void nr_php_wraprec_add_before_after_callbacks( +static bool nr_php_wraprec_can_add_before_after_callbacks( const char* name, size_t namelen, nruserfn_t* wraprec, nrspecialfn_t before_callback, nrspecialfn_t after_callback) { if (NULL == wraprec) { - return; + return false; } /* If any of the callbacks we are attempting to set are already set to @@ -27,7 +27,7 @@ static void nr_php_wraprec_add_before_after_callbacks( "%s: attempting to set special_instrumentation for %.*s, but " "it is already set", __func__, NRSAFELEN(namelen), NRBLANKSTR(name)); - return; + return false; } if (is_instrumentation_set_and_not_equal(wraprec->special_instrumentation_before, @@ -37,8 +37,19 @@ static void nr_php_wraprec_add_before_after_callbacks( "for %.*s, but " "it is already set", __func__, NRSAFELEN(namelen), NRBLANKSTR(name)); - return; + return false; } + if (wraprec->special_instrumentation_before == before_callback && + wraprec->special_instrumentation == after_callback) { + return false; + } + return true; +} + +static void nr_php_wraprec_add_before_after_callbacks( + nruserfn_t* wraprec, + nrspecialfn_t before_callback, + nrspecialfn_t after_callback) { wraprec->special_instrumentation = after_callback; wraprec->special_instrumentation_before = before_callback; @@ -51,10 +62,17 @@ nruserfn_t* nr_php_wrap_user_function_before_after( nrspecialfn_t after_callback) { nruserfn_t* wraprec = nr_php_add_custom_tracer_named(name, namelen); - - nr_php_wraprec_add_before_after_callbacks(name, namelen, wraprec, - before_callback, - after_callback); + if (nr_php_wraprec_can_add_before_after_callbacks(name, namelen, wraprec, + before_callback, + after_callback)) { + nr_php_wraprec_add_before_after_callbacks(wraprec, + before_callback, + after_callback); + } +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO + nr_php_observer_add_begin_handler(wraprec->func, wraprec); + nr_php_observer_add_end_handler(wraprec->func, wraprec); +#endif return wraprec; } @@ -83,7 +101,7 @@ nruserfn_t* nr_php_wrap_callable_before_after( zend_function* callable, nrspecialfn_t before_callback, nrspecialfn_t after_callback) { - char* name = NULL; + char* name = NULL; /* creates a transient wraprec */ nruserfn_t* wraprec = nr_php_add_custom_tracer_callable(callable TSRMLS_CC); @@ -94,15 +112,20 @@ nruserfn_t* nr_php_wrap_callable_before_after( if (nrl_should_print(NRL_VERBOSEDEBUG, NRL_INSTRUMENT)) { name = nr_php_function_debug_name(callable); } - nr_php_wraprec_add_before_after_callbacks(name, nr_strlen(name), wraprec, - before_callback, - after_callback); - if (nrl_should_print(NRL_VERBOSEDEBUG, NRL_INSTRUMENT) && NULL != name) { - nr_free(name); + if (nr_php_wraprec_can_add_before_after_callbacks(name, nr_strlen(name), wraprec, + before_callback, + after_callback)) { + nr_php_wraprec_add_before_after_callbacks(wraprec, + before_callback, + after_callback); } #if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO - nr_php_observer_overwrite_handlers(callable, wraprec); + nr_php_observer_add_begin_handler(callable, wraprec); + nr_php_observer_add_end_handler(callable, wraprec); #endif + if (nrl_should_print(NRL_VERBOSEDEBUG, NRL_INSTRUMENT) && NULL != name) { + nr_free(name); + } return wraprec; } @@ -123,6 +146,10 @@ nruserfn_t* nr_php_wrap_user_function(const char* name, } else { wraprec->special_instrumentation = callback; } +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO + nr_php_observer_add_begin_handler(wraprec->func, wraprec); + nr_php_observer_add_end_handler(wraprec->func, wraprec); +#endif } return wraprec; @@ -154,7 +181,8 @@ nruserfn_t* nr_php_wrap_callable(zend_function* callable, } else { wraprec->special_instrumentation = callback; #if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO - nr_php_observer_overwrite_handlers(callable, wraprec); + nr_php_observer_add_begin_handler(callable, wraprec); + nr_php_observer_add_end_handler(callable, wraprec); #endif } } From 6845e9b169ba1b74819417a9012640b6ddc6aeae Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Wed, 29 May 2024 11:18:58 -0600 Subject: [PATCH 21/23] fix --- agent/php_user_instrument.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/agent/php_user_instrument.c b/agent/php_user_instrument.c index 6d2eb9a7c..73047f7c5 100644 --- a/agent/php_user_instrument.c +++ b/agent/php_user_instrument.c @@ -284,7 +284,9 @@ static void nr_php_wrap_user_function_internal(nruserfn_t* wraprec TSRMLS_DC) { /* It could be in a file not yet loaded, no reason to log anything. */ return; } +#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO wraprec->func = orig_func; +#endif if (ZEND_USER_FUNCTION != orig_func->type) { nrl_verbosedebug(NRL_INSTRUMENT, "%s%s%s is not a user function", From 0b74c04c775ea4bd506515a235a14d0e5d84b6e2 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Wed, 29 May 2024 13:59:42 -0600 Subject: [PATCH 22/23] cleanup --- agent/php_wrapper.c | 33 +++++++++------------------------ 1 file changed, 9 insertions(+), 24 deletions(-) diff --git a/agent/php_wrapper.c b/agent/php_wrapper.c index c34482321..3306a7723 100644 --- a/agent/php_wrapper.c +++ b/agent/php_wrapper.c @@ -9,13 +9,13 @@ #include "util_logging.h" #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO -static bool nr_php_wraprec_can_add_before_after_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) { if (NULL == wraprec) { - return false; + return; } /* If any of the callbacks we are attempting to set are already set to @@ -27,7 +27,7 @@ static bool nr_php_wraprec_can_add_before_after_callbacks( "%s: attempting to set special_instrumentation for %.*s, but " "it is already set", __func__, NRSAFELEN(namelen), NRBLANKSTR(name)); - return false; + return; } if (is_instrumentation_set_and_not_equal(wraprec->special_instrumentation_before, @@ -37,19 +37,12 @@ static bool nr_php_wraprec_can_add_before_after_callbacks( "for %.*s, but " "it is already set", __func__, NRSAFELEN(namelen), NRBLANKSTR(name)); - return false; + return; } if (wraprec->special_instrumentation_before == before_callback && wraprec->special_instrumentation == after_callback) { - return false; + return; } - return true; -} - -static void nr_php_wraprec_add_before_after_callbacks( - nruserfn_t* wraprec, - nrspecialfn_t before_callback, - nrspecialfn_t after_callback) { wraprec->special_instrumentation = after_callback; wraprec->special_instrumentation_before = before_callback; @@ -62,13 +55,9 @@ nruserfn_t* nr_php_wrap_user_function_before_after( nrspecialfn_t after_callback) { nruserfn_t* wraprec = nr_php_add_custom_tracer_named(name, namelen); - if (nr_php_wraprec_can_add_before_after_callbacks(name, namelen, wraprec, + nr_php_wraprec_add_before_after_callbacks(name, namelen, wraprec, before_callback, - after_callback)) { - nr_php_wraprec_add_before_after_callbacks(wraprec, - before_callback, - after_callback); - } + after_callback); #if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO nr_php_observer_add_begin_handler(wraprec->func, wraprec); nr_php_observer_add_end_handler(wraprec->func, wraprec); @@ -112,13 +101,9 @@ nruserfn_t* nr_php_wrap_callable_before_after( if (nrl_should_print(NRL_VERBOSEDEBUG, NRL_INSTRUMENT)) { name = nr_php_function_debug_name(callable); } - if (nr_php_wraprec_can_add_before_after_callbacks(name, nr_strlen(name), wraprec, + nr_php_wraprec_add_before_after_callbacks(name, nr_strlen(name), wraprec, before_callback, - after_callback)) { - nr_php_wraprec_add_before_after_callbacks(wraprec, - before_callback, - after_callback); - } + after_callback); #if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO nr_php_observer_add_begin_handler(callable, wraprec); nr_php_observer_add_end_handler(callable, wraprec); From 6de619972fe2f18a4fb22f7ed3e4773a492f5d12 Mon Sep 17 00:00:00 2001 From: Zach Neumann Date: Thu, 8 Aug 2024 11:30:10 -0600 Subject: [PATCH 23/23] fix ZEND_OBSERVER_NOT_OBSERVED is 2 instead of NULL --- agent/php_observer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/agent/php_observer.c b/agent/php_observer.c index 9a60eea86..019063cb7 100644 --- a/agent/php_observer.c +++ b/agent/php_observer.c @@ -166,8 +166,8 @@ bool nr_php_observer_is_registered(zend_function* func) { } begin_handler = (zend_observer_fcall_begin_handler *)&ZEND_OP_ARRAY_EXTENSION((&(func)->common), zend_observer_fcall_op_array_extension); // begin_handler will be NULL if the observer hasn't been installed yet. - // *begin_Handler will be NULL if the function has not yet been called. - return (begin_handler && *begin_handler); + // *begin_Handler will be (void*)2 if the function has not yet been called. + return (begin_handler && *begin_handler != (void*)2); } bool nr_php_observer_remove_begin_handler(zend_function* func, nruserfn_t* wraprec) {