Skip to content
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
fix: add fallback injection logic
This change handles the edge case exposed in our tests where the agent
may fail to inject the cache naming function at enable because it looked
for drupal and symfony classes before composer autoload had time to load
them. In the normal flow, this would only be a temporary problem since
nr_drupal_enable should be re-run every request, and subsequent requests
should have these resources available. Our test setup, however, shows
that if in the process of handling one request, another request is
generated and the agent doesn't re-trigger RINIT processing, the
function name from the first request will bleed through to the second
request and result in an incorrect name.
To fix, extract logic responsible for injecting the cache naming
function to its own function and call both at enable and within the
PageCache::get function instrumentation, provided the injection function
(newrelic_name_cached) is not detected when we look for it. This should
strike the best balance between performance and correctness.
  • Loading branch information
bduranleau-nr committed Jan 28, 2025
commit 17a32d3bba5971b6031fd8ed4518b88c683e1024
139 changes: 76 additions & 63 deletions agent/fw_drupal8.c
Original file line number Diff line number Diff line change
Expand Up @@ -312,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
Expand All @@ -324,9 +376,6 @@
char* name = NULL;
zval** retval_ptr = NR_GET_RETURN_VALUE_PTR;
zval* request = NULL;

zend_function* nrfn = NULL;

zval* controller = NULL;

(void)wraprec;
Expand All @@ -351,14 +400,30 @@
}

if (!NRINI(drupal_page_cache_naming)) {
nrl_verbosedebug(NRL_FRAMEWORK,
"Skipping PageCache naming due to INI setting");
goto end;
}

nrfn = nr_php_find_function("newrelic_name_cached");
if (NULL == nrfn) {
nrl_warning(NRL_FRAMEWORK,
"Failed to retrieve NR cached page naming function");
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
Expand All @@ -376,8 +441,10 @@
* Symfony\Component\HttpFoundation\Response if there is a cache hit and false
* otherwise.
*/
if (retval_ptr && nr_php_is_zval_valid_object(*retval_ptr)) {

Check failure

Code scanning / CodeQL

Redundant null check due to previous dereference High

This null check is redundant because
the value is dereferenced
in any case.
This null check is redundant because
the value is dereferenced
in any case.
This null check is redundant because
the value is dereferenced
in any case.
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,
Expand Down Expand Up @@ -797,62 +864,8 @@
}

void nr_drupal8_enable(TSRMLS_D) {
int retval;
zend_class_entry* drupal_ce = NULL;
zend_class_entry* symfony_ce = NULL;
bool inject = true;

if (NRINI(drupal_page_cache_naming)) {
drupal_ce = nr_php_find_class("Drupal\\Core\\Routing\\RouteMatch");

if (NULL == drupal_ce) {
inject = false;
nrl_warning(NRL_FRAMEWORK, "Missing Drupal RouteMatch Class");
} else {
symfony_ce
= nr_php_find_class("Symfony\\Component\\HttpFoundation\\Request");
if (NULL == symfony_ce) {
inject = false;
nrl_warning(NRL_FRAMEWORK, "Missing Symfony Request Class");
}
}

if (inject) {
// 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__);
}
} else {
nrl_warning(
NRL_FRAMEWORK,
"Skipped injecting page_cache naming function: Missing Classes");
}
nr_inject_drupal_cache_naming();
}

/*
Expand Down