fix(agent): properly name drupal cached pages

agent/fw_drupal8.c

@@ -66,8 +66,7 @@
 
   if (NR_SUCCESS
       != nr_php_error_record_exception(NRPRG(txn), exception, priority, true,
-                                       NULL,
-                                       &NRPRG(exception_filters))) {
+                                       NULL, &NRPRG(exception_filters))) {
     nrl_verbosedebug(NRL_TXN, "Drupal: unable to record exception");
   }
 
@@ -134,12 +133,12 @@
 #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) {
+    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) {
@@ -164,13 +163,13 @@
         nr_php_class_entry_name(ce), NRSAFELEN(method_len), method);
 
     nr_php_wrap_user_function_before_after_clean(
-                              class_method, nr_strlen(class_method),
-                              before_callback, after_callback, clean_callback);
+        class_method, nr_strlen(class_method), before_callback, after_callback,
+        clean_callback);
 
     nr_free(class_method);
   }
 }
-#endif // OAPI
+#endif  // OAPI
 
 /*
  * Purpose : Check if the given function or method is in the current call
@@ -313,6 +312,58 @@
 }
 NR_PHP_WRAPPER_END
 
+static nr_status_t nr_inject_drupal_cache_naming() {
+  int retval = FAILURE;
+  zend_class_entry* drupal_ce = NULL;
+  zend_class_entry* symfony_ce = NULL;
+
+  drupal_ce = nr_php_find_class("Drupal\\Core\\Routing\\RouteMatch");
+  if (NULL == drupal_ce) {
+    nrl_warning(NRL_FRAMEWORK, "Missing Drupal RouteMatch Class");
+    return NR_FAILURE;
+  }
+
+  symfony_ce = nr_php_find_class("Symfony\\Component\\HttpFoundation\\Request");
+  if (NULL == symfony_ce) {
+    nrl_warning(NRL_FRAMEWORK, "Missing Symfony Request Class");
+    return NR_FAILURE;
+  }
+
+  // clang-format off
+  retval = zend_eval_string(
+    "namespace newrelic\\Drupal8;"
+
+    "use Symfony\\Component\\HttpFoundation\\Request;"
+    "use Drupal\\Core\\Routing\\RouteMatch;"
+
+    "if (!function_exists('newrelic\\Drupal8\\newrelic_name_cached')) {"
+    " function newrelic_name_cached(Request $request) {"
+    "   try {"
+    "     $routeCollection = \\Drupal::service('router.route_provider')->getRouteCollectionForRequest($request);"
+    "     $routeMatch = RouteMatch::createFromRequest($request);"
+    "     $route = $routeCollection->get($routeMatch->getRouteName());"
+    "     $defaults = $route->getDefaults();"
+    "     if (isset($defaults['_controller'])) {"
+    "       $controller = str_replace('::', '->', $defaults['_controller']);"
+    "       $controller = ltrim($controller, '\\\\');"
+    "       return $controller;"
+    "     }"
+    "   } catch (Throwable $e) {}"
+    " }"
+    "}",
+    NULL, "newrelic/Drupal8");
+  // clang-format on
+
+  if (SUCCESS != retval) {
+    nrl_warning(NRL_FRAMEWORK,
+                "%s: error injecting newrelic page cache naming code",
+                __func__);
+    return NR_FAILURE;
+  }
+
+  return NR_SUCCESS;
+}
+
 /*
  * txn naming scheme:
  * In this case, `nr_txn_set_path` is called after `NR_PHP_WRAPPER_CALL` with
@@ -322,12 +373,67 @@
  * function call of this type gets to name the txn.
  */
 NR_PHP_WRAPPER(nr_drupal8_name_the_wt_cached) {
-  const char* name = "page_cache";
+  char* name = NULL;
   zval** retval_ptr = NR_GET_RETURN_VALUE_PTR;
+  zval* request = NULL;
+  zval* controller = NULL;
 
   (void)wraprec;
 
   NR_PHP_WRAPPER_REQUIRE_FRAMEWORK(NR_FW_DRUPAL8);
+
+  request = nr_php_arg_get(1, NR_EXECUTE_ORIG_ARGS);
+
+  if (0 == nr_php_is_zval_valid_object(request)) {
+    nrl_verbosedebug(
+        NRL_TXN, "Drupal 8 PageCache::get does not have a request parameter");
+
+    goto end;
+  }
+
+  if (0
+      == nr_php_object_instanceof_class(
+          request, "Symfony\\Component\\HttpFoundation\\Request")) {
+    nrl_verbosedebug(
+        NRL_TXN, "Drupal 8 PageCache::get parameter is a non-Request object");
+    goto end;
+  }
+
+  if (!NRINI(drupal_page_cache_naming)) {
+    nrl_verbosedebug(NRL_FRAMEWORK,
+                     "Skipping PageCache naming due to INI setting");
+    goto end;
+  }
+
+  if (NULL == nr_php_find_function("newrelic_name_cached")) {
+    nrl_debug(NRL_FRAMEWORK,
+              "Failed to retrieve NR cached page naming function, attempting "
+              "to re-inject");
+
+    // Re-attempt injection of the naming function in case we were too early in
+    // enable
+    if (NR_FAILURE == nr_inject_drupal_cache_naming()) {
+      nrl_warning(NRL_FRAMEWORK,
+                  "%s: Failed to re-inject drupal cache naming function",
+                  __FUNCTION__);
+      goto end;
+    }
+
+    if (NULL == nr_php_find_function("newrelic_name_cached")) {
+      nrl_warning(NRL_FRAMEWORK,
+                  "Failed to retrieve NR cached page naming function");
+      goto end;
+    }
+  }
+
+  controller
+      = nr_php_call(NULL, "\\newrelic\\Drupal8\\newrelic_name_cached", request);
+
+  if (nr_php_is_zval_non_empty_string(controller)) {
+    name = nr_strndup(Z_STRVAL_P(controller), Z_STRLEN_P(controller));
+  }
+
+end:
   NR_PHP_WRAPPER_CALL;
 
   /*
@@ -335,10 +441,19 @@
    * Symfony\Component\HttpFoundation\Response if there is a cache hit and false
    * otherwise.
    */
   if (retval_ptr && nr_php_is_zval_valid_object(*retval_ptr)) {
+    if (NULL == name) {
+      nrl_debug(NRL_FRAMEWORK,
+                "Unable to find cached name, defaulting to 'page_cache'");
+      name = nr_strdup("page_cache");
+    }
     nr_txn_set_path("Drupal8", NRPRG(txn), name, NR_PATH_TYPE_ACTION,
                     NR_OK_TO_OVERWRITE);
   }
+
+  nr_free(name);
+  nr_php_zval_free(&request);
+  nr_php_zval_free(&controller);
 }
 NR_PHP_WRAPPER_END
 
@@ -501,8 +616,7 @@
 
 #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));
+  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",
@@ -515,7 +629,7 @@
   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
+#endif  // OAPI
 
 leave:
   NR_PHP_WRAPPER_CALL;
@@ -536,7 +650,7 @@
     || defined OVERWRITE_ZEND_EXECUTE_DATA
   char* prev_hook = NULL;
   int prev_hook_len;
-#endif // not OAPI
+#endif  // not OAPI
 
   (void)wraprec;
 
@@ -547,7 +661,7 @@
 #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \
     && !defined OVERWRITE_ZEND_EXECUTE_DATA
     nr_php_arg_release(&hook);
-#endif // OAPI
+#endif  // OAPI
     goto leave;
   }
 
@@ -561,7 +675,7 @@
       = nr_strndup(Z_STRVAL_P(hook), Z_STRLEN_P(hook));
   NRPRG(drupal_invoke_all_hook_len) = Z_STRLEN_P(hook);
   NRPRG(check_cufa) = true;
-#endif // OAPI
+#endif  // OAPI
   callback = nr_php_arg_get(2, NR_EXECUTE_ORIG_ARGS TSRMLS_CC);
 
   /* This instrumentation will fail if callback has already been wrapped
@@ -581,14 +695,14 @@
   if (NULL == NRPRG(drupal_invoke_all_hook)) {
     NRPRG(check_cufa) = false;
   }
-#endif // not OAPI
+#endif  // not OAPI
 
-leave: ;
+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
+#endif  // not OAPI
 }
 NR_PHP_WRAPPER_END
 
@@ -605,7 +719,7 @@
   nr_drupal_invoke_all_hook_stacks_pop();
 }
 NR_PHP_WRAPPER_END
-#endif // OAPI
+#endif  // OAPI
 
 /*
  * Purpose : Wrap the invoke() method of the module handler instance in use.
@@ -642,10 +756,8 @@
 #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);
+      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);
@@ -752,6 +864,10 @@
 }
 
 void nr_drupal8_enable(TSRMLS_D) {
+  if (NRINI(drupal_page_cache_naming)) {
+    nr_inject_drupal_cache_naming();
+  }
+
   /*
    * Obtain a transation name if a page was cached.
    */

agent/php_newrelic.h

@@ -316,6 +316,9 @@ nrinibool_t browser_monitoring_debug; /* newrelic.browser_monitoring.debug */
 nrinistr_t browser_monitoring_loader; /* newrelic.browser_monitoring.loader */
 
 nrinibool_t drupal_modules;  /* newrelic.framework.drupal.modules */
+nrinibool_t
+    drupal_page_cache_naming; /* newrelic.framework.drupal.page_cache_naming.enabled
+                               */
 nrinibool_t wordpress_hooks; /* newrelic.framework.wordpress.hooks */
 nrinistr_t
     wordpress_hooks_options; /* newrelic.framework.wordpress.hooks.options */

agent/php_nrini.c

@@ -2237,6 +2237,14 @@ STD_PHP_INI_ENTRY_EX("newrelic.framework.drupal.modules",
                      zend_newrelic_globals,
                      newrelic_globals,
                      nr_on_off_dh)
+STD_PHP_INI_ENTRY_EX("newrelic.framework.drupal.page_cache_naming.enabled",
+                     "1",
+                     NR_PHP_REQUEST,
+                     nr_boolean_mh,
+                     drupal_page_cache_naming,
+                     zend_newrelic_globals,
+                     newrelic_globals,
+                     nr_on_off_dh)
 STD_PHP_INI_ENTRY_EX("newrelic.framework.wordpress.hooks",
                      "1",
                      NR_PHP_REQUEST,

agent/scripts/newrelic.ini.template

@@ -1158,6 +1158,15 @@ newrelic.daemon.logfile = "/var/log/newrelic/newrelic-daemon.log"
 ;
 ;newrelic.framework.drupal.modules = true
 
+; Setting: newrelic.framework.drupal.page_cache_naming.enabled
+; Type   : boolean
+; Scope  : per-directory
+; Default: true
+; Info   : Indicates if Drupal page cache transactions should be named based
+;          on their _controller value or assigned a default name "page_cache".
+;
+;newrelic.framework.drupal.page_cache_naming.enabled = true
+
 ; Setting: newrelic.framework.wordpress.hooks
 ; Type   : boolean
 ; Scope  : per-directory