Skip to content

Commit

Permalink
fix(agent): instrument drupal_http_request (#552)
Browse files Browse the repository at this point in the history
Fix `drupal_http_request` instrumentation for PHP 8.0+ by using Observer
APIs `before` callback to add New Relic headers and `after` callback
to finalize external segment with metrics. Observer API instrumentation
requires the external request segment to be created in `before` callback
but available in `after` callback. Therefore it is made a NRPRG global.
  • Loading branch information
lavarou committed Nov 1, 2022
1 parent 363983d commit 5896966
Show file tree
Hide file tree
Showing 5 changed files with 200 additions and 72 deletions.
250 changes: 188 additions & 62 deletions agent/fw_drupal.c
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,90 @@ NR_PHP_WRAPPER(nr_drupal_qdrupal_name_the_wt) {
}
NR_PHP_WRAPPER_END

#if ZEND_MODULE_API_NO >= ZEND_7_3_X_API_NO
static void nr_drupal_http_request_ensure_second_arg(NR_EXECUTE_PROTO) {
zval* arg = NULL;

/*
* If only one argument is given, an empty list is inserted as second
* argument. NR headers are added during a later step.
*/
if (ZEND_NUM_ARGS() == 1) {
arg = nr_php_zval_alloc();
array_init(arg);

nr_php_arg_add(NR_EXECUTE_ORIG_ARGS, arg);

nr_php_zval_free(&arg);
}
}

static zval* nr_drupal_http_request_add_headers(NR_EXECUTE_PROTO TSRMLS_DC) {
bool is_drupal_7;
zval* second_arg = NULL;
zend_execute_data* ex = nr_get_zend_execute_data(NR_EXECUTE_ORIG_ARGS);

if (nrunlikely(NULL == ex)) {
return NULL;
}

/* Ensure second argument exists in the call frame */
nr_drupal_http_request_ensure_second_arg(NR_EXECUTE_ORIG_ARGS);

is_drupal_7 = (ex->func->common.num_args == 2);

/*
* nr_php_get_user_func_arg is used, as nr_php_arg_get calls ZVAL_DUP
* on the argument zval and thus doesn't allow us to change the
* original argument.
*/
second_arg = nr_php_get_user_func_arg(2, NR_EXECUTE_ORIG_ARGS);

/*
* Add NR headers.
*/
nr_drupal_headers_add(second_arg, is_drupal_7 TSRMLS_CC);

return second_arg;
}
#endif

static char* nr_drupal_http_request_get_method(NR_EXECUTE_PROTO TSRMLS_DC) {
zval* arg2 = NULL;
zval* arg3 = NULL;
zval* method = NULL;
char* http_request_method = NULL;

/*
* Drupal 6 will have a third argument with the method, Drupal 7 will not
* have a third argument it must be parsed from the second.
*/
arg3 = nr_php_arg_get(3, NR_EXECUTE_ORIG_ARGS TSRMLS_CC);
// There is no third arg, this is drupal 7
if (NULL == arg3) {
arg2 = nr_php_arg_get(2, NR_EXECUTE_ORIG_ARGS TSRMLS_CC);
if (NULL != arg2) {
method = nr_php_zend_hash_find(Z_ARRVAL_P(arg2), "method");
if (nr_php_is_zval_valid_string(method)) {
http_request_method
= nr_strndup(Z_STRVAL_P(method), Z_STRLEN_P(method));
}
}
} else if (nr_php_is_zval_valid_string(arg3)) {
// This is drupal 6, the method is the third arg.
http_request_method = nr_strndup(Z_STRVAL_P(arg3), Z_STRLEN_P(arg3));
}
// If the method is not set, Drupal will default to GET
if (NULL == http_request_method) {
http_request_method = nr_strdup("GET");
}

nr_php_arg_release(&arg2);
nr_php_arg_release(&arg3);

return http_request_method;
}

static uint64_t nr_drupal_http_request_get_response_code(
zval** return_value TSRMLS_DC) {
zval* code = NULL;
Expand Down Expand Up @@ -129,23 +213,11 @@ static char* nr_drupal_http_request_get_response_header(
return NULL;
}

/*
* Drupal 6:
* drupal_http_request ($url, $headers = array(), $method = 'GET',
* $data = NULL, $retry = 3, $timeout = 30.0)
*
* Drupal 7:
* drupal_http_request ($url, array $options = array())
*
*/
NR_PHP_WRAPPER(nr_drupal_http_request_exec) {
zval* arg1 = NULL;
zval* arg2 = NULL;
zval* arg3 = NULL;
zval* method = NULL;
zval** return_value = NULL;
#if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \
&& !defined OVERWRITE_ZEND_EXECUTE_DATA

#if ZEND_MODULE_API_NO >= ZEND_7_3_X_API_NO
NR_PHP_WRAPPER(nr_drupal_http_request_before) {
(void)wraprec;
/*
* For PHP 7.3 and newer, New Relic headers are added here.
* For older versions, New Relic headers are added via the proxy function
Expand All @@ -155,40 +227,109 @@ NR_PHP_WRAPPER(nr_drupal_http_request_exec) {
* (nr_php_swap_user_functions), which breaks as since PHP 7.3 user
* functions are stored in shared memory.
*/
bool is_drupal_7;
zval* arg = NULL;
zend_execute_data* ex = nr_get_zend_execute_data(NR_EXECUTE_ORIG_ARGS);
nr_drupal_http_request_add_headers(NR_EXECUTE_ORIG_ARGS);

if (NULL == ex) {
NR_PHP_WRAPPER_LEAVE
}
NR_PHP_WRAPPER_REQUIRE_FRAMEWORK(NR_FW_DRUPAL);

is_drupal_7 = (ex->func->common.num_args == 2);
NRPRG(drupal_http_request_depth) += 1;

/*
* If only one argument is given, an empty list is inserted as second
* argument. NR headers are added during a later step.
* We only want to create a metric here if this isn't a recursive call to
* drupal_http_request() caused by the original call returning a redirect.
* We can check how many drupal_http_request() calls are on the stack by
* checking a counter.
*/
if (ZEND_NUM_ARGS() == 1) {
arg = nr_php_zval_alloc();
array_init(arg);
if (1 == NRPRG(drupal_http_request_depth)) {
NRPRG(drupal_http_request_segment)
= nr_segment_start(NRPRG(txn), NULL, NULL);
}
}
NR_PHP_WRAPPER_END

nr_php_arg_add(NR_EXECUTE_ORIG_ARGS, arg);
NR_PHP_WRAPPER(nr_drupal_http_request_after) {
zval* arg1 = NULL;

nr_php_zval_free(&arg);
(void)wraprec;

NR_PHP_WRAPPER_REQUIRE_FRAMEWORK(NR_FW_DRUPAL);

/*
* Grab the URL for the external metric, which is the first parameter in all
* versions of Drupal.
*/
arg1 = nr_php_arg_get(1, NR_EXECUTE_ORIG_ARGS TSRMLS_CC);
if (0 == nr_php_is_zval_non_empty_string(arg1)) {
goto end;
}

/*
* nr_php_get_user_func_arg is used, as nr_php_arg_get calls ZVAL_DUP
* on the argument zval and thus doesn't allow us to change the
* original argument.
* We only want to create a metric here if this isn't a recursive call to
* drupal_http_request() caused by the original call returning a redirect.
* We can check how many drupal_http_request() calls are on the stack by
* checking a counter.
*/
arg = nr_php_get_user_func_arg(2, NR_EXECUTE_ORIG_ARGS);
if (1 == NRPRG(drupal_http_request_depth)) {
nr_segment_external_params_t external_params
= {.library = "Drupal",
.uri = nr_strndup(Z_STRVAL_P(arg1), Z_STRLEN_P(arg1))};

external_params.procedure
= nr_drupal_http_request_get_method(NR_EXECUTE_ORIG_ARGS);

external_params.encoded_response_header
= nr_drupal_http_request_get_response_header(&func_return_value);

external_params.status
= nr_drupal_http_request_get_response_code(&func_return_value);
if (NRPRG(txn) && NRTXN(special_flags.debug_cat)) {
nrl_verbosedebug(
NRL_CAT, "CAT: outbound response: transport='Drupal 6-7' %s=" NRP_FMT,
X_NEWRELIC_APP_DATA,
NRP_CAT(external_params.encoded_response_header));
}

nr_segment_external_end(&NRPRG(drupal_http_request_segment),
&external_params);
NRPRG(drupal_http_request_segment) = NULL;

nr_free(external_params.encoded_response_header);
nr_free(external_params.procedure);
nr_free(external_params.uri);
}

end:
nr_php_arg_release(&arg1);
NRPRG(drupal_http_request_depth) -= 1;
}
NR_PHP_WRAPPER_END

#else

/*
* Drupal 6:
* drupal_http_request ($url, $headers = array(), $method = 'GET',
* $data = NULL, $retry = 3, $timeout = 30.0)
*
* Drupal 7:
* drupal_http_request ($url, array $options = array())
*
*/
NR_PHP_WRAPPER(nr_drupal_http_request_exec) {
zval* arg1 = NULL;
zval** return_value = NULL;

#if ZEND_MODULE_API_NO >= ZEND_7_3_X_API_NO
/*
* Add NR headers.
* For PHP 7.3 and newer, New Relic headers are added here.
* For older versions, New Relic headers are added via the proxy function
* nr_drupal_replace_http_request.
*
* Reason: using the proxy function involves swizzling
* (nr_php_swap_user_functions), which breaks as since PHP 7.3 user
* functions are stored in shared memory.
*/
nr_drupal_headers_add(arg, is_drupal_7 TSRMLS_CC);
zval* arg
= nr_drupal_http_request_add_headers(NR_EXECUTE_ORIG_ARGS TSRMLS_CC);

/*
* If an invalid argument was given for the second argument ($headers
Expand Down Expand Up @@ -230,30 +371,8 @@ NR_PHP_WRAPPER(nr_drupal_http_request_exec) {
= {.library = "Drupal",
.uri = nr_strndup(Z_STRVAL_P(arg1), Z_STRLEN_P(arg1))};

/*
* Drupal 6 will have a third argument with the method, Drupal 7 will not
* have a third argument it must be parsed from the second.
*/
arg3 = nr_php_arg_get(3, NR_EXECUTE_ORIG_ARGS TSRMLS_CC);
// There is no third arg, this is drupal 7
if (0 == arg3) {
arg2 = nr_php_arg_get(2, NR_EXECUTE_ORIG_ARGS TSRMLS_CC);
if (NULL != arg2) {
method = nr_php_zend_hash_find(Z_ARRVAL_P(arg2), "method");
if (nr_php_is_zval_valid_string(method)) {
external_params.procedure
= nr_strndup(Z_STRVAL_P(method), Z_STRLEN_P(method));
}
}
} else if (nr_php_is_zval_valid_string(arg3)) {
// This is drupal 6, the method is the third arg.
external_params.procedure
= nr_strndup(Z_STRVAL_P(arg3), Z_STRLEN_P(arg3));
}
// If the method is not set, Drupal will default to GET
if (NULL == external_params.procedure) {
external_params.procedure = nr_strdup("GET");
}
external_params.procedure
= nr_drupal_http_request_get_method(NR_EXECUTE_ORIG_ARGS TSRMLS_CC);

segment = nr_segment_start(NRPRG(txn), NULL, NULL);

Expand Down Expand Up @@ -287,12 +406,12 @@ NR_PHP_WRAPPER(nr_drupal_http_request_exec) {

end:
nr_php_arg_release(&arg1);
nr_php_arg_release(&arg2);
nr_php_arg_release(&arg3);
NRPRG(drupal_http_request_depth) -= 1;
}
NR_PHP_WRAPPER_END

#endif

static void nr_drupal_name_the_wt(const zend_function* func TSRMLS_DC) {
char* action = NULL;

Expand Down Expand Up @@ -632,8 +751,15 @@ void nr_drupal_enable(TSRMLS_D) {
nr_drupal_name_wt_as_cached_page TSRMLS_CC);
nr_php_wrap_user_function(NR_PSTR("drupal_cron_run"),
nr_drupal_cron_run TSRMLS_CC);
#if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \
&& !defined OVERWRITE_ZEND_EXECUTE_DATA
nr_php_wrap_user_function_before_after(
NR_PSTR("drupal_http_request"), nr_drupal_http_request_before,
nr_drupal_http_request_after TSRMLS_CC);
#else
nr_php_wrap_user_function(NR_PSTR("drupal_http_request"),
nr_drupal_http_request_exec TSRMLS_CC);
#endif

/*
* The drupal_modules config setting controls instrumentation of modules,
Expand Down
5 changes: 4 additions & 1 deletion agent/php_newrelic.h
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,10 @@ size_t drupal_module_invoke_all_hook_len; /* The length of the current Drupal
hook */
size_t drupal_http_request_depth; /* The current depth of drupal_http_request()
calls */

#if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \
&& !defined OVERWRITE_ZEND_EXECUTE_DATA
nr_segment_t* drupal_http_request_segment;
#endif
int symfony1_in_dispatch; /* Whether we are currently within a
sfFrontWebController::dispatch() frame */
int symfony1_in_error404; /* Whether we are currently within a
Expand Down
4 changes: 4 additions & 0 deletions agent/php_rinit.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ PHP_RINIT_FUNCTION(newrelic) {
NRPRG(sapi_headers) = NULL;
NRPRG(pid) = getpid();
NRPRG(user_function_wrappers) = nr_vector_create(64, NULL, NULL);
#if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \
&& !defined OVERWRITE_ZEND_EXECUTE_DATA
NRPRG(drupal_http_request_segment) = NULL;
#endif

if ((0 == NR_PHP_PROCESS_GLOBALS(enabled)) || (0 == NRINI(enabled))) {
return SUCCESS;
Expand Down
4 changes: 4 additions & 0 deletions agent/php_rshutdown.c
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,10 @@ int nr_php_post_deactivate(void) {

NRPRG(current_framework) = NR_FW_UNSET;
NRPRG(framework_version) = 0;
#if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \
&& !defined OVERWRITE_ZEND_EXECUTE_DATA
NRPRG(drupal_http_request_segment) = NULL;
#endif

nrl_verbosedebug(NRL_INIT, "post-deactivate processing done");
return SUCCESS;
Expand Down
9 changes: 0 additions & 9 deletions agent/tests/test_fw_drupal.c
Original file line number Diff line number Diff line change
Expand Up @@ -370,15 +370,6 @@ static void test_drupal_http_request_drupal_6(TSRMLS_D) {
}

void test_main(void* p NRUNUSED) {
/*
DO NOT LEAVE THIS.
When drupal_http_request header functionality is refactored,
please put the tests back in!
*/
#if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \
&& !defined OVERWRITE_ZEND_EXECUTE_DATA /* PHP 8.0+ and OAPI */
return;
#endif

#if defined(ZTS) && !defined(PHP7)
void*** tsrm_ls = NULL;
Expand Down

0 comments on commit 5896966

Please sign in to comment.