feat(agent): instrument Drupal 9.4 hooks with oapi (#701)

In addition to OAPI Drupal 9.4 support, this PR does some refactoring:

1. The hook stacks have been renamed from `module_invoke_all` to
`invoke_all` to be more aligned with the name of Drupal functions that
can now push to those stacks.

2. The generic wrapper function (originally added when Drupal 9.4
   instrumentation was introduced) has been updated to support OAPI.

3. `nr_php_wrap_callable_before_after_clean` has been introduced as a
   parallel for `nr_php_wrap_user_function_before_after_clean`. To
facilitate this, many of the internals have been extracted to a common
function.

4. Speaking of, lots of the maintenance of the Drupal's hook stacks has
   been extracted to common functions to reduce code reuse

---------

Co-authored-by: Michal Nowacki <[email protected]>

Author: ZNeumann

agent/fw_drupal.c

@@ -470,7 +470,7 @@ static void nr_drupal_wrap_hook_within_module_invoke_all(
 #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \
     && !defined OVERWRITE_ZEND_EXECUTE_DATA
   zval* curr_hook
-      = (zval*)nr_stack_get_top(&NRPRG(drupal_module_invoke_all_hooks));
+      = (zval*)nr_stack_get_top(&NRPRG(drupal_invoke_all_hooks));
   if (!nr_php_is_zval_non_empty_string(curr_hook)) {
     nrl_verbosedebug(NRL_FRAMEWORK,
                      "%s: cannot extract hook name from global stack",
@@ -480,8 +480,8 @@ static void nr_drupal_wrap_hook_within_module_invoke_all(
   char* hook_name = Z_STRVAL_P(curr_hook);
   size_t hook_len = Z_STRLEN_P(curr_hook);
 #else
-  char* hook_name = NRPRG(drupal_module_invoke_all_hook);
-  size_t hook_len = NRPRG(drupal_module_invoke_all_hook_len);
+  char* hook_name = NRPRG(drupal_invoke_all_hook);
+  size_t hook_len = NRPRG(drupal_invoke_all_hook_len);
 #endif
   if (NULL == hook_name) {
     nrl_verbosedebug(NRL_FRAMEWORK,
@@ -753,33 +753,21 @@ NR_PHP_WRAPPER(nr_drupal_wrap_module_invoke_all_before) {
   NR_PHP_WRAPPER_REQUIRE_FRAMEWORK(NR_FW_DRUPAL);
 
   hook_copy = nr_php_arg_get(1, NR_EXECUTE_ORIG_ARGS);
-  if (nr_php_is_zval_non_empty_string(hook_copy)) {
-    nr_stack_push(&NRPRG(drupal_module_invoke_all_hooks), hook_copy);
-    nr_stack_push(&NRPRG(drupal_module_invoke_all_states), (void*)!NULL);
-  } else {
-    nr_stack_push(&NRPRG(drupal_module_invoke_all_states), NULL);
-  }
+  nr_drupal_invoke_all_hook_stacks_push(hook_copy);
 }
 NR_PHP_WRAPPER_END
 
-static void module_invoke_all_clean_stacks() {
-  if ((bool)nr_stack_pop(&NRPRG(drupal_module_invoke_all_states))) {
-    zval* hook_copy = nr_stack_pop(&NRPRG(drupal_module_invoke_all_hooks));
-    nr_php_arg_release(&hook_copy);
-  }
-}
-
 NR_PHP_WRAPPER(nr_drupal_wrap_module_invoke_all_after) {
   (void)wraprec;
-  module_invoke_all_clean_stacks();
+  nr_drupal_invoke_all_hook_stacks_pop();
 }
 NR_PHP_WRAPPER_END
 
 NR_PHP_WRAPPER(nr_drupal_wrap_module_invoke_all_clean) {
   NR_UNUSED_SPECIALFN;
   NR_UNUSED_FUNC_RETURN_VALUE;
   (void)wraprec;
-  module_invoke_all_clean_stacks();
+  nr_drupal_invoke_all_hook_stacks_pop();
 }
 NR_PHP_WRAPPER_END
 
@@ -799,17 +787,17 @@ NR_PHP_WRAPPER(nr_drupal_wrap_module_invoke_all) {
     goto leave;
   }
 
-  prev_hook = NRPRG(drupal_module_invoke_all_hook);
-  prev_hook_len = NRPRG(drupal_module_invoke_all_hook_len);
-  NRPRG(drupal_module_invoke_all_hook)
+  prev_hook = NRPRG(drupal_invoke_all_hook);
+  prev_hook_len = NRPRG(drupal_invoke_all_hook_len);
+  NRPRG(drupal_invoke_all_hook)
       = nr_strndup(Z_STRVAL_P(hook), Z_STRLEN_P(hook));
-  NRPRG(drupal_module_invoke_all_hook_len) = Z_STRLEN_P(hook);
+  NRPRG(drupal_invoke_all_hook_len) = Z_STRLEN_P(hook);
 
   NR_PHP_WRAPPER_CALL;
 
-  nr_free(NRPRG(drupal_module_invoke_all_hook));
-  NRPRG(drupal_module_invoke_all_hook) = prev_hook;
-  NRPRG(drupal_module_invoke_all_hook_len) = prev_hook_len;
+  nr_free(NRPRG(drupal_invoke_all_hook));
+  NRPRG(drupal_invoke_all_hook) = prev_hook;
+  NRPRG(drupal_invoke_all_hook_len) = prev_hook_len;
 
 leave:
   nr_php_arg_release(&hook);

agent/fw_drupal8.c

@@ -71,6 +71,48 @@ static void nr_drupal8_add_method_callback(const zend_class_entry* ce,
   }
 }
 
+#if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \
+    && !defined OVERWRITE_ZEND_EXECUTE_DATA
+static void nr_drupal8_add_method_callback_before_after_clean(
+                                           const zend_class_entry* ce,
+                                           const char* method,
+                                           size_t method_len,
+                                           nrspecialfn_t before_callback,
+                                           nrspecialfn_t after_callback,
+                                           nrspecialfn_t clean_callback) {
+  zend_function* function = NULL;
+
+  if (NULL == ce) {
+    nrl_verbosedebug(NRL_FRAMEWORK, "Drupal 8: got NULL class entry in %s",
+                     __func__);
+    return;
+  }
+
+  function = nr_php_find_class_method(ce, method);
+  if (NULL == function) {
+    nrl_verbosedebug(NRL_FRAMEWORK,
+                     "Drupal 8+: cannot get zend_function entry for %.*s::%.*s",
+                     NRSAFELEN(nr_php_class_entry_name_length(ce)),
+                     nr_php_class_entry_name(ce), NRSAFELEN(method_len),
+                     method);
+    return;
+  }
+
+  if (NULL == nr_php_get_wraprec(function)) {
+    char* class_method = nr_formatf(
+        "%.*s::%.*s", NRSAFELEN(nr_php_class_entry_name_length(ce)),
+        nr_php_class_entry_name(ce), NRSAFELEN(method_len), method);
+
+    nr_php_wrap_user_function_before_after_clean_with_transience(
+                              class_method, nr_strlen(class_method),
+                              before_callback, after_callback, clean_callback,
+                              NR_WRAPREC_NOT_TRANSIENT);
+
+    nr_free(class_method);
+  }
+}
+#endif // OAPI
+
 /*
  * Purpose : Check if the given function or method is in the current call
  *           stack.
@@ -383,18 +425,10 @@ NR_PHP_WRAPPER(nr_drupal8_post_implements_hook) {
 }
 NR_PHP_WRAPPER_END
 
-/*
- * Temporary exclusion via #if defined OVERWRITE_ZEND_EXECUTE_DATA
- * This needs to be excluded for now because the hooks handling was changed
- * for oapi so the hook vars this is referencing do not exist.
- *
- */
-#if defined OVERWRITE_ZEND_EXECUTE_DATA
 /*
  * Purpose : Handles ModuleHandlerInterface::invokeAllWith()'s callback
  *           and ensure that the relevant module_hook function is instrumented.
  */
-
 NR_PHP_WRAPPER(nr_drupal94_invoke_all_with_callback) {
   zval* module = NULL;
 
@@ -406,9 +440,24 @@ NR_PHP_WRAPPER(nr_drupal94_invoke_all_with_callback) {
     goto leave;
   }
 
+#if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \
+    && !defined OVERWRITE_ZEND_EXECUTE_DATA
+  zval* curr_hook
+      = (zval*)nr_stack_get_top(&NRPRG(drupal_invoke_all_hooks));
+  if (UNEXPECTED(!nr_php_is_zval_non_empty_string(curr_hook))) {
+    nrl_verbosedebug(NRL_FRAMEWORK,
+                     "%s: cannot extract hook name from global stack",
+                     __func__);
+    goto leave;
+  }
   nr_drupal_hook_instrument(Z_STRVAL_P(module), Z_STRLEN_P(module),
-                            NRPRG(drupal_module_invoke_all_hook),
-                            NRPRG(drupal_module_invoke_all_hook_len) TSRMLS_CC);
+                            Z_STRVAL_P(curr_hook), Z_STRLEN_P(curr_hook));
+#else
+  nr_drupal_hook_instrument(Z_STRVAL_P(module), Z_STRLEN_P(module),
+                            NRPRG(drupal_invoke_all_hook),
+                            NRPRG(drupal_invoke_all_hook_len) TSRMLS_CC);
+#endif // OAPI
+
 leave:
   NR_PHP_WRAPPER_CALL;
   nr_php_arg_release(&module);
@@ -424,52 +473,77 @@ NR_PHP_WRAPPER_END
 NR_PHP_WRAPPER(nr_drupal94_invoke_all_with) {
   zval* callback = NULL;
   zval* hook = NULL;
+#if ZEND_MODULE_API_NO < ZEND_8_0_X_API_NO \
+    || defined OVERWRITE_ZEND_EXECUTE_DATA
   char* prev_hook = NULL;
   int prev_hook_len;
+#endif // not OAPI
 
   (void)wraprec;
 
   NR_PHP_WRAPPER_REQUIRE_FRAMEWORK(NR_FW_DRUPAL8);
 
   hook = nr_php_arg_get(1, NR_EXECUTE_ORIG_ARGS TSRMLS_CC);
   if (!nr_php_is_zval_non_empty_string(hook)) {
+#if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \
+    && !defined OVERWRITE_ZEND_EXECUTE_DATA
+    nr_php_arg_release(&hook);
+#endif // OAPI
     goto leave;
   }
 
-  prev_hook = NRPRG(drupal_module_invoke_all_hook);
-  prev_hook_len = NRPRG(drupal_module_invoke_all_hook_len);
-  NRPRG(drupal_module_invoke_all_hook)
+#if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \
+    && !defined OVERWRITE_ZEND_EXECUTE_DATA
+  nr_drupal_invoke_all_hook_stacks_push(hook);
+#else
+  prev_hook = NRPRG(drupal_invoke_all_hook);
+  prev_hook_len = NRPRG(drupal_invoke_all_hook_len);
+  NRPRG(drupal_invoke_all_hook)
       = nr_strndup(Z_STRVAL_P(hook), Z_STRLEN_P(hook));
-  NRPRG(drupal_module_invoke_all_hook_len) = Z_STRLEN_P(hook);
-
+  NRPRG(drupal_invoke_all_hook_len) = Z_STRLEN_P(hook);
+#endif // OAPI
   callback = nr_php_arg_get(2, NR_EXECUTE_ORIG_ARGS TSRMLS_CC);
+
   /* This instrumentation will fail if callback has already been wrapped
    * with a special instrumentation callback in a different context.
    * In this scenario, we will be unable to instrument hooks and modules for
    * this particular call */
-
-  /*
-   * Temporary exclusion via #if defined OVERWRITE_ZEND_EXECUTE_DATA
-   * This needs to be excluded for now because the hooks handling was changed
-   * for oapi so the hook vars this is referencing do not exist.
-   *
-   */
-#if defined OVERWRITE_ZEND_EXECUTE_DATA
   nr_php_wrap_generic_callable(callback,
                                nr_drupal94_invoke_all_with_callback TSRMLS_CC);
-#endif
   NR_PHP_WRAPPER_CALL;
 
   nr_php_arg_release(&callback);
-  nr_free(NRPRG(drupal_module_invoke_all_hook));
-  NRPRG(drupal_module_invoke_all_hook) = prev_hook;
-  NRPRG(drupal_module_invoke_all_hook_len) = prev_hook_len;
-
-leave:
+#if ZEND_MODULE_API_NO < ZEND_8_0_X_API_NO \
+    || defined OVERWRITE_ZEND_EXECUTE_DATA
+  nr_free(NRPRG(drupal_invoke_all_hook));
+  NRPRG(drupal_invoke_all_hook) = prev_hook;
+  NRPRG(drupal_invoke_all_hook_len) = prev_hook_len;
+#endif // not OAPI
+
+leave: ;
+#if ZEND_MODULE_API_NO < ZEND_8_0_X_API_NO \
+    || defined OVERWRITE_ZEND_EXECUTE_DATA
+  /* for OAPI, the _after callback handles this free */
   nr_php_arg_release(&hook);
+#endif // not OAPI
+}
+NR_PHP_WRAPPER_END
+
+#if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \
+    && !defined OVERWRITE_ZEND_EXECUTE_DATA
+NR_PHP_WRAPPER(nr_drupal94_invoke_all_with_after) {
+  (void)wraprec;
+  nr_drupal_invoke_all_hook_stacks_pop();
+}
+NR_PHP_WRAPPER_END
+
+NR_PHP_WRAPPER(nr_drupal94_invoke_all_with_clean) {
+  (void)wraprec;
+  nr_drupal_invoke_all_hook_stacks_pop();
 }
 NR_PHP_WRAPPER_END
-#endif /* temporary: defined OVERWRITE_ZEND_EXECUTE_DATA */
+#endif // OAPI
+
 /*
  * Purpose : Wrap the invoke() method of the module handler instance in use.
  */
@@ -502,13 +576,14 @@ NR_PHP_WRAPPER(nr_drupal8_module_handler) {
   nr_drupal8_add_method_callback(ce, NR_PSTR("implementshook"),
                                  nr_drupal8_post_implements_hook TSRMLS_CC);
   /* Drupal 9.4 introduced a replacement method for getImplentations */
-  /*
-   * Temporary exclusion via #if defined OVERWRITE_ZEND_EXECUTE_DATA
-   * This needs to be excluded for now because the hooks handling was changed
-   * for oapi so the hook vars this is referencing do not exist.
-   *
-   */
-#if defined OVERWRITE_ZEND_EXECUTE_DATA
+#if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \
+    && !defined OVERWRITE_ZEND_EXECUTE_DATA
+  nr_drupal8_add_method_callback_before_after_clean(
+                                 ce, NR_PSTR("invokeallwith"),
+                                 nr_drupal94_invoke_all_with,
+                                 nr_drupal94_invoke_all_with_after,
+                                 nr_drupal94_invoke_all_with_clean);
+#else
   nr_drupal8_add_method_callback(ce, NR_PSTR("invokeallwith"),
                                  nr_drupal94_invoke_all_with TSRMLS_CC);
 #endif

agent/fw_drupal_common.c

@@ -294,3 +294,22 @@ void nr_drupal_headers_add(zval* arg, bool is_drupal_7 TSRMLS_DC) {
 
   nr_php_zval_free(&newrelic_headers);
 }
+
+#if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \
+    && !defined OVERWRITE_ZEND_EXECUTE_DATA
+void nr_drupal_invoke_all_hook_stacks_push(zval* hook_copy) {
+  if (nr_php_is_zval_non_empty_string(hook_copy)) {
+    nr_stack_push(&NRPRG(drupal_invoke_all_hooks), hook_copy);
+    nr_stack_push(&NRPRG(drupal_invoke_all_states), (void*)!NULL);
+  } else {
+    nr_stack_push(&NRPRG(drupal_invoke_all_states), NULL);
+  }
+}
+
+void nr_drupal_invoke_all_hook_stacks_pop() {
+  if ((bool)nr_stack_pop(&NRPRG(drupal_invoke_all_states))) {
+    zval* hook_copy = nr_stack_pop(&NRPRG(drupal_invoke_all_hooks));
+    nr_php_arg_release(&hook_copy);
+  }
+}
+#endif

agent/fw_drupal_common.h

@@ -147,4 +147,23 @@ nr_status_t module_invoke_all_parse_module_and_hook_from_strings(
  */
 void nr_drupal_headers_add(zval* arg, bool is_drupal_7 TSRMLS_DC);
 
+
+#if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \
+    && !defined OVERWRITE_ZEND_EXECUTE_DATA
+/*
+ * Purpose: Before an invoke_all style call, adds the hook to that hook states stacks
+ *
+ * Params  : 1. A zval holding a copy of the hook invoked, to be managed by the hook
+ *              states stacks and freed by nr_drupal_invoke_all_hook_stacks_pop() after the
+ *              invoke_all call completes
+ */
+void nr_drupal_invoke_all_hook_stacks_push(zval* hook_copy);
+
+/*
+ * Purpose: After an invoke_all style call, pops that states stack and conditionally
+ *          pops the hook stack based on the previously popped state
+ */
+void nr_drupal_invoke_all_hook_stacks_pop();
+#endif // OAPI
+
 #endif /* FW_DRUPAL_COMMON_HDR */

agent/php_newrelic.h

@@ -410,13 +410,13 @@ int framework_version; /* Current framework version */
 /* Without OAPI, we are able to utilize the call stack to keep track
  * of the previous hooks. With OAPI, we can no longer do this so
  * we track the stack manually */
-nr_stack_t drupal_module_invoke_all_hooks; /* stack of Drupal hooks */
-nr_stack_t drupal_module_invoke_all_states; /* stack of bools indicating
+nr_stack_t drupal_invoke_all_hooks; /* stack of Drupal hooks */
+nr_stack_t drupal_invoke_all_states; /* stack of bools indicating
                                                whether the current hook
                                                needs to be released */
 #else
-char* drupal_module_invoke_all_hook;      /* The current Drupal hook */
-size_t drupal_module_invoke_all_hook_len; /* The length of the current Drupal
+char* drupal_invoke_all_hook;      /* The current Drupal hook */
+size_t drupal_invoke_all_hook_len; /* The length of the current Drupal
                                              hook */
 #endif //OAPI
 size_t drupal_http_request_depth; /* The current depth of drupal_http_request()

agent/php_rinit.c

@@ -125,11 +125,11 @@ PHP_RINIT_FUNCTION(newrelic) {
   nr_stack_init(&NRPRG(predis_ctxs), NR_STACK_DEFAULT_CAPACITY);
   nr_stack_init(&NRPRG(wordpress_tags), NR_STACK_DEFAULT_CAPACITY);
   nr_stack_init(&NRPRG(wordpress_tag_states), NR_STACK_DEFAULT_CAPACITY);
-  nr_stack_init(&NRPRG(drupal_module_invoke_all_hooks), NR_STACK_DEFAULT_CAPACITY);
-  nr_stack_init(&NRPRG(drupal_module_invoke_all_states), NR_STACK_DEFAULT_CAPACITY);
+  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_module_invoke_all_hooks).dtor = zval_stack_dtor;
+  NRPRG(drupal_invoke_all_hooks).dtor = zval_stack_dtor;
 #endif
 
   NRPRG(mysql_last_conn) = NULL;

agent/php_rshutdown.c

@@ -120,8 +120,8 @@ int nr_php_post_deactivate(void) {
    */
   nr_stack_destroy_fields(&NRPRG(wordpress_tags));
   nr_stack_destroy_fields(&NRPRG(wordpress_tag_states));
-  nr_stack_destroy_fields(&NRPRG(drupal_module_invoke_all_hooks));
-  nr_stack_destroy_fields(&NRPRG(drupal_module_invoke_all_states));
+  nr_stack_destroy_fields(&NRPRG(drupal_invoke_all_hooks));
+  nr_stack_destroy_fields(&NRPRG(drupal_invoke_all_states));
 #endif
 
 #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \