Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

feat(agent): Drupal hook attribute instrumentation #1030

Open
wants to merge 11 commits into
base: dev
Choose a base branch
from
86 changes: 86 additions & 0 deletions agent/fw_drupal8.c
Original file line number Diff line number Diff line change
Expand Up @@ -605,6 +605,89 @@ NR_PHP_WRAPPER(nr_drupal94_invoke_all_with_clean) {
NR_PHP_WRAPPER_END
#endif // OAPI

/*
* Purpose: Instrument Drupal Attribute Hooks for Drupal 11.1+
*
* Params: 1. A zval pointer to the moduleHandler instance in use by Drupal.
*
* Return: bool
*
*/
static bool nr_drupal_hook_attribute_instrument(zval* module_handler) {
zval* hook_implementation_map = NULL;

nr_php_string_hash_key_t* hook_key = NULL;
zval* hook_val = NULL;
nr_php_string_hash_key_t* class_key = NULL;
zval* class_val = NULL;
nr_php_string_hash_key_t* method_key = NULL;
zval* module_val = NULL;

char* hookpath = NULL;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could check module_handler for NULL and exit early.

hook_implementation_map = nr_php_get_zval_object_property(
module_handler, "hookImplementationsMap");

if (NULL == hook_implementation_map) {
nrl_warning(NRL_FRAMEWORK, "NULL hookImplementationsMap object property");
return false;
}

if (!nr_php_is_zval_valid_array(hook_implementation_map)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nr_php_is_zval_valid_array will already check hook_implementation_map for NULL ; it doesn't need the extra check right above.

nrl_warning(NRL_FRAMEWORK,
"hookImplementationsMap property not a valid array");
return false;
}

ZEND_HASH_FOREACH_STR_KEY_VAL(Z_ARRVAL_P(hook_implementation_map), hook_key,
hook_val) {
if ((NULL == hook_key) || (0 == nr_php_is_zval_valid_array(hook_val))) {
nrl_warning(NRL_FRAMEWORK,
"hookImplementationsMap[hook]: invalid key or value");
return false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to return here or just skip this round of the loop?
What if the first hook_key or hook_val is invalid, but the next one is good?

}

ZEND_HASH_FOREACH_STR_KEY_VAL(Z_ARRVAL_P(hook_val), class_key, class_val) {
if ((NULL == class_key) || (0 == nr_php_is_zval_valid_array(class_val))) {
nrl_warning(NRL_FRAMEWORK,
"hookImplementationsMap[class]: invalid key or value");
return false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to https://github.com/newrelic/newrelic-php-agent/pull/1030/files#r1974532294, do we want to exit the whole function or just skip this round of the loop.

}

ZEND_HASH_FOREACH_STR_KEY_VAL(Z_ARRVAL_P(class_val), method_key,
module_val) {
if ((NULL == method_key)
|| (0 == nr_php_is_zval_valid_string(module_val))) {
nrl_warning(NRL_FRAMEWORK,
"hookImplementationsMap[method]: invalid key or value");
return false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/newrelic/newrelic-php-agent/pull/1030/files#r1974532294
skip this round of the loop or completely exit?

}

if (0
== nr_stricmp(ZEND_STRING_VALUE(class_key),
"Drupal\\Core\\Extension\\ProceduralCall")) {
hookpath = nr_formatf("%s", ZEND_STRING_VALUE(method_key));
} else {
hookpath = nr_formatf("%s::%s", ZEND_STRING_VALUE(class_key),
ZEND_STRING_VALUE(method_key));
}

nr_php_wrap_user_function_drupal(
hookpath, nr_strlen(hookpath), Z_STRVAL_P(module_val),
Z_STRLEN_P(module_val), ZEND_STRING_VALUE(hook_key),
ZEND_STRING_LEN(hook_key));

nr_free(hookpath);
}
ZEND_HASH_FOREACH_END();
}
ZEND_HASH_FOREACH_END();
}
ZEND_HASH_FOREACH_END();

return true;
}

/*
* Purpose : Wrap the invoke() method of the module handler instance in use.
*/
Expand Down Expand Up @@ -632,6 +715,9 @@ NR_PHP_WRAPPER(nr_drupal8_module_handler) {

ce = Z_OBJCE_P(*retval_ptr);

if (nr_drupal_hook_attribute_instrument(*retval_ptr)) {
NR_PHP_WRAPPER_LEAVE;
}
nr_drupal8_add_method_callback(ce, NR_PSTR("getimplementations"),
nr_drupal8_post_get_implementations TSRMLS_CC);
nr_drupal8_add_method_callback(ce, NR_PSTR("implementshook"),
Expand Down