fix(agent): properly name drupal cached pages #1010
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -66,8 +66,7 @@ | |
|
|
||
| if (NR_SUCCESS | ||
| != nr_php_error_record_exception(NRPRG(txn), exception, priority, true, | ||
| 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) { | ||
|
Comment on lines
-137
to
+141
|
||
| 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); | ||
|
|
||
| nr_free(class_method); | ||
| } | ||
| } | ||
| #endif // OAPI | ||
|
Comment on lines
-167
to
+172
|
||
|
|
||
| /* | ||
| * Purpose : Check if the given function or method is in the current call | ||
|
|
@@ -322,12 +321,54 @@ | |
| * function call of this type gets to name the txn. | ||
| */ | ||
| NR_PHP_WRAPPER(nr_drupal8_name_the_wt_cached) { | ||
| char* name = NULL; | ||
| zval** retval_ptr = NR_GET_RETURN_VALUE_PTR; | ||
| zval* request = NULL; | ||
|
|
||
| zend_function* nrfn = 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)) { | ||
| 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; | ||
|
There was a problem hiding this comment. This could probably set There was a problem hiding this comment. Maybe a better solution to multiple There was a problem hiding this comment. My concern was that it appeared we had a case where we missed naming - if your solution handles these it should work fine. There was a problem hiding this comment. I think 04cbaf6 is a slight improvement on the fallback naming There was a problem hiding this comment. On testing, that seems to have broken something... I'll have to look into this a little more |
||
| } | ||
|
|
||
| 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 +376,17 @@ | |
| * 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 |
||
| if (NULL == name) { | ||
| 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 +549,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)); | ||
|
Comment on lines
-504
to
+552
|
||
| 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 +562,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 | ||
|
Comment on lines
-518
to
+565
|
||
|
|
||
| leave: | ||
| NR_PHP_WRAPPER_CALL; | ||
|
|
@@ -536,7 +583,7 @@ | |
| || defined OVERWRITE_ZEND_EXECUTE_DATA | ||
| char* prev_hook = NULL; | ||
| int prev_hook_len; | ||
| #endif // not OAPI | ||
|
Comment on lines
-539
to
+586
|
||
|
|
||
| (void)wraprec; | ||
|
|
||
|
|
@@ -547,7 +594,7 @@ | |
| #if ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO \ | ||
| && !defined OVERWRITE_ZEND_EXECUTE_DATA | ||
| nr_php_arg_release(&hook); | ||
| #endif // OAPI | ||
|
Comment on lines
-550
to
+597
|
||
| goto leave; | ||
| } | ||
|
|
||
|
|
@@ -561,7 +608,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 | ||
|
Comment on lines
-564
to
+611
|
||
| callback = nr_php_arg_get(2, NR_EXECUTE_ORIG_ARGS TSRMLS_CC); | ||
|
|
||
| /* This instrumentation will fail if callback has already been wrapped | ||
|
|
@@ -581,14 +628,14 @@ | |
| if (NULL == NRPRG(drupal_invoke_all_hook)) { | ||
| NRPRG(check_cufa) = false; | ||
| } | ||
| #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 | ||
|
Comment on lines
-584
to
+638
|
||
| } | ||
| NR_PHP_WRAPPER_END | ||
|
|
||
|
|
@@ -605,7 +652,7 @@ | |
| nr_drupal_invoke_all_hook_stacks_pop(); | ||
| } | ||
| NR_PHP_WRAPPER_END | ||
| #endif // OAPI | ||
|
Comment on lines
-608
to
+655
|
||
|
|
||
| /* | ||
| * Purpose : Wrap the invoke() method of the module handler instance in use. | ||
|
|
@@ -642,10 +689,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); | ||
|
Comment on lines
-645
to
+693
|
||
| #else | ||
| nr_drupal8_add_method_callback(ce, NR_PSTR("invokeallwith"), | ||
| nr_drupal94_invoke_all_with TSRMLS_CC); | ||
|
|
@@ -752,6 +797,64 @@ | |
| } | ||
|
|
||
| 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"); | ||
| } | ||
| } | ||
|
|
||
| /* | ||
| * Obtain a transation name if a page was cached. | ||
| */ | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Formatting