From 3411be566ea25511e0427509a501c6a995fd7139 Mon Sep 17 00:00:00 2001 From: bduranleau-nr <106178551+bduranleau-nr@users.noreply.github.com> Date: Wed, 20 Dec 2023 14:35:31 -0600 Subject: [PATCH 1/9] chore: bump agent version to 10.16 (#807) --- VERSION | 2 +- axiom/nr_version.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/VERSION b/VERSION index f9fb144f9..5007551bf 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -10.15.0 +10.16.0 diff --git a/axiom/nr_version.c b/axiom/nr_version.c index 703447ba3..a1ee685ee 100644 --- a/axiom/nr_version.c +++ b/axiom/nr_version.c @@ -42,8 +42,9 @@ * orchid 20Sep2023 (10.12) * poinsettia 03Oct2023 (10.13) * quince 13Nov2023 (10.14) + * rose 20Dec2023 (10.15) */ -#define NR_CODENAME "rose" +#define NR_CODENAME "snapdragon" const char* nr_version(void) { return NR_STR2(NR_VERSION); From 938f0172f1dc1066326541c8954109075dbad863 Mon Sep 17 00:00:00 2001 From: bduranleau-nr <106178551+bduranleau-nr@users.noreply.github.com> Date: Thu, 11 Jan 2024 11:17:42 -0600 Subject: [PATCH 2/9] chore(docs): use https for docs links (#809) Change user-facing documentation links to use `https` instead of `http` --- CONTRIBUTING.md | 2 +- README.md | 2 +- STYLEGUIDE.md | 2 +- agent/README.txt | 2 +- agent/newrelic-install.sh | 16 ++++++++-------- agent/scripts/newrelic.ini.template | 2 +- .../cross_agent_tests/sql_obfuscation/README.md | 12 ++++++------ 7 files changed, 19 insertions(+), 19 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b74c301e4..3746fe0f3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -16,7 +16,7 @@ Before submitting an Issue, please search for similar ones in the 1. Please open the pull request against the `dev` branch. This is the development branch where all changes are tested before merging to main. 2. Ensure any install or build dependencies are removed before the end of the layer when doing a build. -3. Increase the version numbers in any examples files and the README.md to the new version that this Pull Request would represent. The versioning scheme we use is [SemVer](http://semver.org/). +3. Increase the version numbers in any examples files and the README.md to the new version that this Pull Request would represent. The versioning scheme we use is [SemVer](https://semver.org/). 4. You may merge the Pull Request in once you have the sign-off of two other developers, or if you do not have permission to do that, you may request the second reviewer to merge it for you. ## Contributor License Agreement diff --git a/README.md b/README.md index e0bef27e9..8e9aed226 100644 --- a/README.md +++ b/README.md @@ -63,4 +63,4 @@ If you would like to [contribute](https://github.com/newrelic/newrelic-php-agent To all [contributors](https://github.com/newrelic/newrelic-php-agent/graphs/contributors), we thank you! Without your contribution, this project would not be what it is today. We also host a community project page dedicated to the [New Relic PHP agent](https://opensource.newrelic.com/projects/newrelic/newrelic-php-agent). ## License -The PHP agent is licensed under the [Apache 2.0](http://apache.org/licenses/LICENSE-2.0.txt) License and also uses source code from third-party libraries. You can find full details on which libraries are used and the terms under which they are licensed in the third-party notices document. +The PHP agent is licensed under the [Apache 2.0](https://apache.org/licenses/LICENSE-2.0.txt) License and also uses source code from third-party libraries. You can find full details on which libraries are used and the terms under which they are licensed in the third-party notices document. diff --git a/STYLEGUIDE.md b/STYLEGUIDE.md index 9f7b7bd61..60821ab02 100644 --- a/STYLEGUIDE.md +++ b/STYLEGUIDE.md @@ -132,7 +132,7 @@ _"This does not contain everything, but what it contains is true"._ ``` 0. Format code with - [clang-format 3.8](http://releases.llvm.org/3.8.0/tools/clang/docs/ClangFormat.html) + [clang-format 3.8](https://releases.llvm.org/3.8.0/tools/clang/docs/ClangFormat.html) or later using our [style configuration file](.clang-format). 0. Disregard directives from this list when other approaches are more diff --git a/agent/README.txt b/agent/README.txt index eac645387..9da736979 100644 --- a/agent/README.txt +++ b/agent/README.txt @@ -6,4 +6,4 @@ and API descriptions please see the online FAQ at: For license and copyright, see LICENSE. -For support, contact us at http://support.newrelic.com for assistance. +For support, contact us at https://support.newrelic.com for assistance. diff --git a/agent/newrelic-install.sh b/agent/newrelic-install.sh index d538bb260..68c2d1c45 100755 --- a/agent/newrelic-install.sh +++ b/agent/newrelic-install.sh @@ -355,7 +355,7 @@ The New Relic installation directory is incomplete. The files or directories listed above could not be found. This usually means that you have a corrupt installation archive. Please re-download your New Relic software and try again. If the problem persists, please contact -${bold}http://support.newrelic.com${rmso} and report the issue. Be sure +${bold}https://support.newrelic.com${rmso} and report the issue. Be sure to include all the above output in your bug report. We apologize for the inconvenience. @@ -494,7 +494,7 @@ fatal() { echo "FATAL: $@" >&2 cat >&2 < Date: Wed, 17 Jan 2024 08:24:22 -0700 Subject: [PATCH 3/9] feat(daemon): improve logging obfuscation (#795) Follows the ideas set out in https://github.com/newrelic/go-agent/pull/828/files --------- Co-authored-by: Hitesh Ahuja <108540135+hahuja2@users.noreply.github.com> --- src/daemon/main.go | 12 ++++++- src/daemon/worker.go | 3 +- src/newrelic/collector/client.go | 44 +++++++++++++++++++----- src/newrelic/collector/collector_test.go | 9 +++++ src/newrelic/processor.go | 6 ++-- 5 files changed, 60 insertions(+), 14 deletions(-) diff --git a/src/daemon/main.go b/src/daemon/main.go index d91058021..7d45c517e 100644 --- a/src/daemon/main.go +++ b/src/daemon/main.go @@ -363,8 +363,18 @@ func main() { } log.Infof("%s", banner(cfg)) + // Used to not log out the proxy, which has potentially secret credentials + var proxy bool = false for i := range os.Args { - log.Debugf("ARGV[%d]: %s", i, os.Args[i]) + if proxy { + log.Debugf("ARGV[%d]: **REDACTED**", i) + proxy = false + } else { + log.Debugf("ARGV[%d]: %s", i, os.Args[i]) + if ("--proxy" == os.Args[i]) { + proxy = true + } + } } log.Debugf("process role is %v", cfg.Role) diff --git a/src/daemon/worker.go b/src/daemon/worker.go index 98947a90c..2f55b2748 100644 --- a/src/daemon/worker.go +++ b/src/daemon/worker.go @@ -91,10 +91,11 @@ func runWorker(cfg *Config) { clientCfg := &newrelic.ClientConfig{ CAFile: cfg.CAFile, CAPath: cfg.CAPath, - Proxy: cfg.Proxy, + Proxy: "**REDACTED**", } log.Infof("collector configuration is %+v", clientCfg) + clientCfg.Proxy = cfg.Proxy client, err := newrelic.NewClient(clientCfg) if nil != err { diff --git a/src/newrelic/collector/client.go b/src/newrelic/collector/client.go index 5298a6fb7..ce8abc7ab 100644 --- a/src/newrelic/collector/client.go +++ b/src/newrelic/collector/client.go @@ -15,6 +15,7 @@ import ( "net/url" "strings" "time" + "errors" "golang.org/x/net/proxy" @@ -75,6 +76,31 @@ type RPMResponse struct { disconnectSecurityPolicy bool } +// All RPMResponse's should be created through newRPMResponse() methods. +// You must either pass a status code, or an err. +// If no error is passed, one is created when needed based on the status code. +// Otherwise, the error is scrubbed of sensitive information +func NewRPMResponseError(err error) RPMResponse { + return RPMResponse{ + Err: removeURLFromError(err), + } +} + +func removeURLFromError(err error) error { + if nil == err { + return nil + } + + // remove url from errors to avoid sensitive data leaks + var ue *url.Error + if errors.As(err, &ue) { + ue.URL = "**REDACTED-URL**" + } + + + return err +} + func newRPMResponse(StatusCode int) RPMResponse { var err error if StatusCode != 200 && StatusCode != 202 { @@ -212,7 +238,7 @@ func (l *limitClient) Execute(cmd *RpmCmd, cs RpmControls) RPMResponse { resp := l.orig.Execute(cmd, cs) return resp case <-timer: - return RPMResponse{Err: fmt.Errorf("timeout after %v", l.timeout)} + return NewRPMResponseError(fmt.Errorf("timeout after %v", l.timeout)) } } @@ -270,7 +296,7 @@ func NewClient(cfg *ClientConfig) (Client, error) { if "" != cfg.Proxy { url, err := parseProxy(cfg.Proxy) if err != nil { - return nil, err + return nil, removeURLFromError(err) } switch url.Scheme { @@ -323,17 +349,17 @@ type clientImpl struct { func (c *clientImpl) perform(url string, cmd RpmCmd, cs RpmControls) RPMResponse { deflated, err := Compress(cmd.Data) if nil != err { - return RPMResponse{Err: err} + return NewRPMResponseError(err) } if l := deflated.Len(); l > cmd.MaxPayloadSize { - return RPMResponse{Err: fmt.Errorf("payload size too large: %d greater than %d", l, cmd.MaxPayloadSize)} + return NewRPMResponseError(fmt.Errorf("payload size too large: %d greater than %d", l, cmd.MaxPayloadSize)) } req, err := http.NewRequest("POST", url, deflated) if nil != err { - return RPMResponse{Err: err} + return NewRPMResponseError(err) } req.Header.Add("Accept-Encoding", "identity, deflate") @@ -347,7 +373,7 @@ func (c *clientImpl) perform(url string, cmd RpmCmd, cs RpmControls) RPMResponse resp, err := c.httpClient.Do(req) if err != nil { - return RPMResponse{Err: err} + return NewRPMResponseError(err) } defer resp.Body.Close() @@ -402,7 +428,7 @@ func (c *clientImpl) Execute(cmd *RpmCmd, cs RpmControls) RPMResponse { // Create the JSON payload data, err := cs.Collectible.CollectorJSON(false) if nil != err { - return RPMResponse{Err: err} + return NewRPMResponseError(err) } cmd.Data = data @@ -421,7 +447,7 @@ func (c *clientImpl) Execute(cmd *RpmCmd, cs RpmControls) RPMResponse { url := cmd.url(false) cleanURL := cmd.url(true) - log.Audit("command='%s' url='%s' payload={%s}", cmd.Name, url, audit) + log.Audit("command='%s' url='%s' payload={%s}", cmd.Name, cleanURL, audit) log.Debugf("command='%s' url='%s' max_payload_size_in_bytes='%d' payload={%s}", cmd.Name, cleanURL, cmd.MaxPayloadSize, cmd.Data) resp := c.perform(url, *cmd, cs) @@ -430,7 +456,7 @@ func (c *clientImpl) Execute(cmd *RpmCmd, cs RpmControls) RPMResponse { cmd.Name, resp.Err.Error(), cleanURL) } - log.Audit("command='%s' url='%s', status=%d, response={%s}", cmd.Name, url, resp.StatusCode, string(resp.Body)) + log.Audit("command='%s' url='%s', status=%d, response={%s}", cmd.Name, cleanURL, resp.StatusCode, string(resp.Body)) log.Debugf("command='%s' url='%s', status=%d, response={%s}", cmd.Name, cleanURL, resp.StatusCode, string(resp.Body)) return resp diff --git a/src/newrelic/collector/collector_test.go b/src/newrelic/collector/collector_test.go index 9f5364391..c3de73b27 100644 --- a/src/newrelic/collector/collector_test.go +++ b/src/newrelic/collector/collector_test.go @@ -14,6 +14,15 @@ import ( "testing" ) +func TestURLErrorRedaction(t *testing.T) { + _, err := http.Get("http://notexist.example/sensitive?sensitive=very") + rpm := NewRPMResponseError(err) + + if strings.Contains(rpm.Err.Error(), "http://notexist.example/sensitive?sensitive=very") { + t.Error("Sensitive URL should have been removed from the error struct, but were not") + } +} + var ( actualData = "my_data" ) diff --git a/src/newrelic/processor.go b/src/newrelic/processor.go index 08d2af55c..cbe6c5ad2 100644 --- a/src/newrelic/processor.go +++ b/src/newrelic/processor.go @@ -422,17 +422,17 @@ func (p *Processor) processConnectAttempt(rep ConnectAttempt) { app.RawConnectReply = rep.RawReply.Body if rep.RawReply.IsDisconnect() { app.state = AppStateDisconnected - log.Warnf("app '%s' connect attempt returned %s; disconnecting", app, rep.RawReply.Err) + log.Warnf("app '%s' connect attempt returned %s; disconnecting", app, collector.NewRPMResponseError(rep.RawReply.Err).Err) return } else if rep.RawReply.IsRestartException() { // in accord with the spec, invalid license is a restart exception. Except we want // to shutdown instead of restart. if rep.RawReply.IsInvalidLicense() { app.state = AppStateInvalidLicense - log.Warnf("app '%s' connect attempt returned %s; shutting down", app, rep.RawReply.Err) + log.Warnf("app '%s' connect attempt returned %s; shutting down", app, collector.NewRPMResponseError(rep.RawReply.Err).Err) } else { app.state = AppStateRestart - log.Warnf("app '%s' connect attempt returned %s; restarting", app, rep.RawReply.Err) + log.Warnf("app '%s' connect attempt returned %s; restarting", app, collector.NewRPMResponseError(rep.RawReply.Err).Err) } return } else if nil != rep.Err { From 0fa3083c32c65abf8d48b28f76e96ee17679440a Mon Sep 17 00:00:00 2001 From: Amber Sistla Date: Wed, 10 Jan 2024 10:56:02 -0700 Subject: [PATCH 4/9] refactor: wrap wp hook callbacks in add_filter (#768) In PHPs 7.4+ use add_filter wrapper to wrap wordpress hooks callbacks. Older PHPs still use call_user_function_array callback. This approach limits the number of times the agent is called by Zend Engine during WordPress transaction execution because add_filter (and add_action, which is just a wrapper around add_filter) is called less frequently (called when filter is added to a hook) than call_user_function_array (called each time hook executes). The number of times the agent is called by Zend Engine translates to the number of times a wraprec for user function is looked up, and since the agent changed the method how it stores wraprecs in PHPs 7.4+ it is an important optimization. --------- Co-authored-by: Michal Nowacki Co-authored-by: bduranleau-nr <106178551+bduranleau-nr@users.noreply.github.com> --- agent/fw_wordpress.c | 68 +++++++++++++- .../frameworks/wordpress/mock_hooks.php | 27 ++++++ .../test_wordpress_apply_filters.php | 84 ++++++++++++++++++ .../wordpress/test_wordpress_do_action.php | 88 +++++++++++++++++++ 4 files changed, 266 insertions(+), 1 deletion(-) create mode 100644 tests/integration/frameworks/wordpress/mock_hooks.php create mode 100644 tests/integration/frameworks/wordpress/test_wordpress_apply_filters.php create mode 100644 tests/integration/frameworks/wordpress/test_wordpress_do_action.php diff --git a/agent/fw_wordpress.c b/agent/fw_wordpress.c index 3a84450cf..232bb2841 100644 --- a/agent/fw_wordpress.c +++ b/agent/fw_wordpress.c @@ -324,8 +324,9 @@ NR_PHP_WRAPPER(nr_wordpress_wrap_hook) { NR_PHP_WRAPPER_END /* - * A call_user_func_array() callback to ensure that we wrap each hook function. + * PHP 7.3 and below uses old-style wraprec, and will use old style cufa calls. */ +#if ZEND_MODULE_API_NO < ZEND_7_4_X_API_NO static void nr_wordpress_call_user_func_array(zend_function* func, const zend_function* caller NRUNUSED TSRMLS_DC) { @@ -357,6 +358,7 @@ static void nr_wordpress_call_user_func_array(zend_function* func, */ nr_php_wrap_callable(func, nr_wordpress_wrap_hook TSRMLS_CC); } +#endif /* PHP < 7.4 */ static void free_tag(void* tag) { nr_free(tag); @@ -551,6 +553,65 @@ NR_PHP_WRAPPER(nr_wordpress_apply_filters) { } NR_PHP_WRAPPER_END +#if ZEND_MODULE_API_NO >= ZEND_7_4_X_API_NO +/* + * Wrap the wordpress function add_filter + * + * function add_filter( $hook_name, $callback, $priority = 10, $accepted_args = 1 ) + * + * @param string $hook_name The name of the filter to add the callback to. + * @param callable $callback The callback to be run when the filter is applied. + * @param int $priority Optional. Used to specify the order in which the functions + * associated with a particular filter are executed. + * Lower numbers correspond with earlier execution, + * and functions with the same priority are executed + * in the order in which they were added to the filter. Default 10. + * @param int $accepted_args Optional. The number of arguments the function accepts. Default 1. + * @return true Always returns true. + */ + +NR_PHP_WRAPPER(nr_wordpress_add_filter) { + /* Wordpress's add_action() is just a wrapper around add_filter(), + * so we only need to instrument this function */ + NR_UNUSED_SPECIALFN; + (void)wraprec; + const char* filename = NULL; + bool wrap_hook = true; + + NR_PHP_WRAPPER_REQUIRE_FRAMEWORK(NR_FW_WORDPRESS); + + /* + * We only want to hook the function being called if this is a WordPress + * function, we're instrumenting hooks, and WordPress is currently executing + * hooks (denoted by the wordpress_tag being set). + */ + if ((NR_FW_WORDPRESS != NRPRG(current_framework)) + || (0 == NRINI(wordpress_hooks))) { + wrap_hook = false; + } + + if (NRINI(wordpress_hooks_skip_filename) + && 0 != nr_strlen(NRINI(wordpress_hooks_skip_filename))) { + filename = nr_php_op_array_file_name(NR_OP_ARRAY); + + if (nr_strstr(filename, NRINI(wordpress_hooks_skip_filename))) { + nrl_verbosedebug(NRL_FRAMEWORK, "skipping hooks for function from %s", + filename); + wrap_hook = false; + } + } + + if (true == wrap_hook) { + zval* callback = nr_php_arg_get(2, NR_EXECUTE_ORIG_ARGS); + /* the callback here can be any PHP callable. nr_php_wrap_generic_callable + * checks that a valid callable is passed */ + nr_php_wrap_generic_callable(callback, nr_wordpress_wrap_hook); + nr_php_arg_release(&callback); + } +} +NR_PHP_WRAPPER_END +#endif /* PHP 7.4+ */ + void nr_wordpress_enable(TSRMLS_D) { nr_php_wrap_user_function(NR_PSTR("apply_filters"), nr_wordpress_apply_filters TSRMLS_CC); @@ -565,8 +626,13 @@ void nr_wordpress_enable(TSRMLS_D) { nr_php_wrap_user_function(NR_PSTR("do_action_ref_array"), nr_wordpress_exec_handle_tag TSRMLS_CC); +#if ZEND_MODULE_API_NO < ZEND_7_4_X_API_NO nr_php_add_call_user_func_array_pre_callback( nr_wordpress_call_user_func_array TSRMLS_CC); +#else + nr_php_wrap_user_function(NR_PSTR("add_filter"), + nr_wordpress_add_filter); +#endif } } diff --git a/tests/integration/frameworks/wordpress/mock_hooks.php b/tests/integration/frameworks/wordpress/mock_hooks.php new file mode 100644 index 000000000..7419bbc65 --- /dev/null +++ b/tests/integration/frameworks/wordpress/mock_hooks.php @@ -0,0 +1,27 @@ +user_code + * to ensure that cufa instrumentation properly handles skipping + * opline lookups of internal functions + * + * OAPI: Initiates a call to an added action outside + * the context of do_action, to ensure we only instrument + * with an active hook + */ +$function = new ReflectionFunction('g'); +$function->invoke(); + +do_action("f"); From 43590ae2dfe0a86ca12d699e4406ad9d831d04bb Mon Sep 17 00:00:00 2001 From: Michal Nowacki Date: Wed, 10 Jan 2024 14:07:57 -0500 Subject: [PATCH 5/9] refactor: improve performance of wrap_generic_callable (#798) `nr_php_wrap_generic_callable` is used to create transient wraprecs for callables which **_don't require to be named_**. This assumption makes it possible to stay DRY and reuse `nr_php_zval_to_function` in lieu of complex pattern matching for the type of callable. This reduces agent's performance overhead as it now uses `nr_php_wrap_callable` always instead of using `nr_php_wrap_user_function_with_transience(IS_TRANSIENT)` sometimes. TL:DR; The agent's performance overhead when `nr_php_wrap_user_function_with_transience(IS_TRANSIENT)` is used comes from the need to walk the linked list of named wraprecs in `nr_php_add_custom_tracer_named`. --- agent/php_execute.c | 10 +-- agent/php_user_instrument.c | 36 +++----- agent/php_user_instrument.h | 9 +- agent/php_wrapper.c | 90 +++---------------- agent/php_wrapper.h | 5 -- agent/php_zval.h | 10 +++ agent/tests/test_user_instrument.c | 2 +- .../integration/frameworks/drupal/skipif.inc | 9 ++ .../drupal/test_invoke_all_with.php | 20 ++++- .../drupal/test_module_invoke_all.php | 87 ++++++++++++++++++ 10 files changed, 156 insertions(+), 122 deletions(-) create mode 100644 tests/integration/frameworks/drupal/skipif.inc create mode 100644 tests/integration/frameworks/drupal/test_module_invoke_all.php diff --git a/agent/php_execute.c b/agent/php_execute.c index 9cd08ee6e..7afca8af0 100644 --- a/agent/php_execute.c +++ b/agent/php_execute.c @@ -124,11 +124,11 @@ typedef void (*nr_library_enable_fn_t)(TSRMLS_D); avail = avail - 3; \ } -static int nr_format_zval_for_debug(zval* arg, - char* pbuf, - size_t pos, - size_t avail, - size_t depth TSRMLS_DC) { +int nr_format_zval_for_debug(zval* arg, + char* pbuf, + size_t pos, + size_t avail, + size_t depth TSRMLS_DC) { nr_string_len_t len; nr_string_len_t i; char* str; diff --git a/agent/php_user_instrument.c b/agent/php_user_instrument.c index 037bfb8f2..7fd97db69 100644 --- a/agent/php_user_instrument.c +++ b/agent/php_user_instrument.c @@ -131,7 +131,7 @@ static void nr_php_user_wraprec_destroy(nruserfn_t** wraprec_ptr); static void reset_wraprec(nruserfn_t* wraprec) { nruserfn_t* p = wraprec; nr_php_wraprec_hashmap_key_release(&p->key); - if (p->transience == NR_WRAPREC_IS_TRANSIENT) { + if (p->is_transient) { nr_php_user_wraprec_destroy((nruserfn_t**)&wraprec); } else { p->is_wrapped = 0; @@ -176,7 +176,7 @@ static void reset_wraprec(nruserfn_t* wraprec) { * with framework specific instrumentation. Optionally specifies transience. 5) from function `nr_php_wrap_callable` (in `php_wrapper.c`) used only by * Wordpress and predis for custom instrumentation that sets - * `NR_WRAPREC_IS_TRANSIENT`. + * `is_transient`. * * Transient wrappers get disposed of at the end of each request at RSHUTDOWN lifecycle. * @@ -383,7 +383,7 @@ nruserfn_t* nr_php_add_custom_tracer_callable(zend_function* func TSRMLS_DC) { } wraprec = nr_php_user_wraprec_create(); - wraprec->transience = NR_WRAPREC_IS_TRANSIENT; + wraprec->is_transient = true; nrl_verbosedebug(NRL_INSTRUMENT, "adding custom for callable '%s'", name); nr_free(name); @@ -397,8 +397,7 @@ nruserfn_t* nr_php_add_custom_tracer_callable(zend_function* func TSRMLS_DC) { } nruserfn_t* nr_php_add_custom_tracer_named(const char* namestr, - size_t namestrlen, - nr_transience_t transience TSRMLS_DC) { + size_t namestrlen) { nruserfn_t* wraprec; nruserfn_t* p; @@ -431,14 +430,11 @@ nruserfn_t* nr_php_add_custom_tracer_named(const char* namestr, (0 == wraprec->classname) ? "" : "::", NRP_PHP(wraprec->funcname)); nr_php_wrap_user_function_internal(wraprec TSRMLS_CC); - if (transience == NR_WRAPREC_IS_TRANSIENT) { - wraprec->transience = NR_WRAPREC_IS_TRANSIENT; - } else { - /* non-transient wraprecs are added to both the hashmap and linked list. - * At request shutdown, the hashmap will free transients, but leave - * non-transients to be freed when the linked list is disposed of which is at module shutdown */ - nr_php_add_custom_tracer_common(wraprec); - } + /* non-transient wraprecs are added to both the hashmap and linked list. + * At request shutdown, the hashmap will free transients, but leave + * non-transients to be freed when the linked list is disposed of which is at + * module shutdown */ + nr_php_add_custom_tracer_common(wraprec); return wraprec; /* return the new wraprec */ } @@ -484,7 +480,7 @@ void nr_php_remove_transient_user_instrumentation(void) { nruserfn_t* prev = NULL; while (NULL != p) { - if (p->transience == NR_WRAPREC_IS_TRANSIENT) { + if (p->is_transient) { nruserfn_t* trans = p; if (prev) { @@ -519,9 +515,7 @@ void nr_php_add_user_instrumentation(TSRMLS_D) { void nr_php_add_transaction_naming_function(const char* namestr, int namestrlen TSRMLS_DC) { - nruserfn_t* wraprec - = nr_php_add_custom_tracer_named(namestr, namestrlen, - NR_WRAPREC_NOT_TRANSIENT TSRMLS_CC); + nruserfn_t* wraprec = nr_php_add_custom_tracer_named(namestr, namestrlen); if (NULL != wraprec) { wraprec->is_names_wt_simple = 1; @@ -529,9 +523,7 @@ void nr_php_add_transaction_naming_function(const char* namestr, } void nr_php_add_custom_tracer(const char* namestr, int namestrlen TSRMLS_DC) { - nruserfn_t* wraprec - = nr_php_add_custom_tracer_named(namestr, namestrlen, - NR_WRAPREC_NOT_TRANSIENT TSRMLS_CC); + nruserfn_t* wraprec = nr_php_add_custom_tracer_named(namestr, namestrlen); if (NULL != wraprec) { wraprec->create_metric = 1; @@ -588,9 +580,7 @@ void nr_php_user_function_add_declared_callback(const char* namestr, int namestrlen, nruserfn_declared_t callback TSRMLS_DC) { - nruserfn_t* wraprec - = nr_php_add_custom_tracer_named(namestr, namestrlen, - NR_WRAPREC_NOT_TRANSIENT TSRMLS_CC); + nruserfn_t* wraprec = nr_php_add_custom_tracer_named(namestr, namestrlen); if (0 != wraprec) { wraprec->declared_callback = callback; diff --git a/agent/php_user_instrument.h b/agent/php_user_instrument.h index a945fd416..4e71a9f82 100644 --- a/agent/php_user_instrument.h +++ b/agent/php_user_instrument.h @@ -27,10 +27,6 @@ typedef struct nrspecialfn_return_t (*nrspecialfn_t)( typedef void (*nruserfn_declared_t)(TSRMLS_D); -typedef enum { - NR_WRAPREC_NOT_TRANSIENT = 0, - NR_WRAPREC_IS_TRANSIENT = 1 -} nr_transience_t; /* * An equivalent data structure for user functions. * @@ -79,7 +75,7 @@ typedef struct _nruserfn_t { */ int is_names_wt_simple; /* True if this function "names" its enclosing WT; the first such function does the naming */ - nr_transience_t transience; /* Wraprecs that are transient are destroyed + bool is_transient; /* Wraprecs that are transient are destroyed after each request. Wraprecs that are non-transient are kept until module shutdown. Currently, while all wraprecs are stored @@ -161,8 +157,7 @@ extern void nr_php_add_custom_tracer(const char* namestr, extern nruserfn_t* nr_php_add_custom_tracer_callable( zend_function* func TSRMLS_DC); extern nruserfn_t* nr_php_add_custom_tracer_named(const char* namestr, - size_t namestrlen, - nr_transience_t transience TSRMLS_DC); + size_t namestrlen); extern void nr_php_reset_user_instrumentation(void); extern void nr_php_remove_transient_user_instrumentation(void); extern void nr_php_add_user_instrumentation(TSRMLS_D); diff --git a/agent/php_wrapper.c b/agent/php_wrapper.c index 22416a570..a7c4d9bf7 100644 --- a/agent/php_wrapper.c +++ b/agent/php_wrapper.c @@ -11,16 +11,7 @@ nruserfn_t* nr_php_wrap_user_function(const char* name, size_t namelen, nrspecialfn_t callback TSRMLS_DC) { - return nr_php_wrap_user_function_with_transience(name, namelen, callback, - NR_WRAPREC_NOT_TRANSIENT TSRMLS_CC); -} - -nruserfn_t* nr_php_wrap_user_function_with_transience(const char* name, - size_t namelen, - nrspecialfn_t callback, - nr_transience_t transience TSRMLS_DC) { - nruserfn_t* wraprec = nr_php_add_custom_tracer_named(name, namelen, - transience TSRMLS_CC); + nruserfn_t* wraprec = nr_php_add_custom_tracer_named(name, namelen); if (wraprec && callback) { if ((NULL != wraprec->special_instrumentation) @@ -68,79 +59,22 @@ nruserfn_t* nr_php_wrap_callable(zend_function* callable, return wraprec; } +#define NR_WRAPPER_DEBUG_STRBUFSZ (1024) + nruserfn_t* nr_php_wrap_generic_callable(zval* callable, nrspecialfn_t callback TSRMLS_DC) { -#if ZEND_MODULE_API_NO < ZEND_7_0_X_API_NO - char* name = NULL; -#else - zend_string* name = NULL; -#endif - zend_fcall_info_cache fcc; - zend_fcall_info fci; - - /* not calling nr_zend_is_callable because we want to additionally populate name */ - if (zend_is_callable(callable, 0, &name TSRMLS_CC)) { - /* see php source code's zend_is_callable_at_frame function to see from - * where these switch cases are derived */ -#if ZEND_MODULE_API_NO >= ZEND_7_0_X_API_NO -again: -#endif - switch (Z_TYPE_P(callable)) { - /* wrapping a string name of a callable */ - case IS_STRING: -#if ZEND_MODULE_API_NO < ZEND_7_0_X_API_NO - return nr_php_wrap_user_function_with_transience(name, nr_strlen(name), callback, - NR_WRAPREC_IS_TRANSIENT TSRMLS_CC); -#else - return nr_php_wrap_user_function_with_transience(ZEND_STRING_VALUE(name), - ZEND_STRING_LEN(name), callback, - NR_WRAPREC_IS_TRANSIENT TSRMLS_CC); -#endif + zend_function* zf = nr_php_zval_to_function(callable); - /* wrapping an array where [0] is an object and [1] is the method to invoke - * the previous zend_is_callable has created the commbined object::method - * name for us to wrap */ - case IS_ARRAY: -#if ZEND_MODULE_API_NO < ZEND_7_0_X_API_NO - return nr_php_wrap_user_function_with_transience(name, nr_strlen(name), callback, - NR_WRAPREC_IS_TRANSIENT TSRMLS_CC); -#else - return nr_php_wrap_user_function_with_transience(ZEND_STRING_VALUE(name), - ZEND_STRING_LEN(name), callback, - NR_WRAPREC_IS_TRANSIENT TSRMLS_CC); -#endif - - /* wrapping a closure. Need to initialize fcall info in order to wrap the - * underlying zend_function object */ - case IS_OBJECT: - if (SUCCESS == zend_fcall_info_init(callable, 0, &fci, &fcc, - NULL, NULL TSRMLS_CC)) { - /* nr_php_wrap_callable already sets the transience field for us */ - return nr_php_wrap_callable(fcc.function_handler, callback TSRMLS_CC); - } - nrl_verbosedebug(NRL_INSTRUMENT, "Failed to initialize fcall info when wrapping"); - break; - - /* unwrap references */ - /* PHP 5.x handles references in a different manner that do not need to be unwrapped */ -#if ZEND_MODULE_API_NO >= ZEND_7_0_X_API_NO - case IS_REFERENCE: - callable = Z_REFVAL_P(callable); - goto again; -#endif - } + if (NULL != zf) { + return nr_php_wrap_callable(zf, callback); } -#if ZEND_MODULE_API_NO < ZEND_7_0_X_API_NO - nrl_verbosedebug(NRL_INSTRUMENT, - "Failed to wrap callable: %s", name); -#else - if (NULL != name) { - nrl_verbosedebug(NRL_INSTRUMENT, - "Failed to wrap callable: %s", ZEND_STRING_VALUE(name)); - } else { - nrl_verbosedebug(NRL_INSTRUMENT, "Failed to wrap callable with unknown name"); + if (nrl_should_print(NRL_VERBOSEDEBUG, NRL_INSTRUMENT)) { + char strbuf[NR_WRAPPER_DEBUG_STRBUFSZ]; + nr_format_zval_for_debug(callable, strbuf, 0, NR_WRAPPER_DEBUG_STRBUFSZ - 1, + 0); + nrl_verbosedebug(NRL_INSTRUMENT, "Failed to cast to callable zval=%s", + strbuf); } -#endif return NULL; } diff --git a/agent/php_wrapper.h b/agent/php_wrapper.h index 6e3a47d77..ed419bdcc 100644 --- a/agent/php_wrapper.h +++ b/agent/php_wrapper.h @@ -89,11 +89,6 @@ extern nruserfn_t* nr_php_wrap_user_function(const char* name, size_t namelen, nrspecialfn_t callback TSRMLS_DC); -extern nruserfn_t* nr_php_wrap_user_function_with_transience(const char* name, - size_t namelen, - nrspecialfn_t callback, - nr_transience_t TSRMLS_DC); - extern nruserfn_t* nr_php_wrap_user_function_extra(const char* name, size_t namelen, nrspecialfn_t callback, diff --git a/agent/php_zval.h b/agent/php_zval.h index 7993a3345..ccfb97a0f 100644 --- a/agent/php_zval.h +++ b/agent/php_zval.h @@ -417,4 +417,14 @@ static inline zval* nr_php_zval_real_value(zval* zv) { /* }}} */ +/* {{{ Debugging functions */ + +extern int nr_format_zval_for_debug(zval* arg, + char* pbuf, + size_t pos, + size_t avail, + size_t depth TSRMLS_DC); + +/* }}} */ + #endif /* PHP_ZVAL_HDR */ diff --git a/agent/tests/test_user_instrument.c b/agent/tests/test_user_instrument.c index 3f2484c0e..6901044db 100644 --- a/agent/tests/test_user_instrument.c +++ b/agent/tests/test_user_instrument.c @@ -97,7 +97,7 @@ static void test_hashmap_wraprec() { /* instrument user function */ user_func1_wraprec = nr_php_add_custom_tracer_named( - user_func1_name, nr_strlen(user_func1_name), NR_WRAPREC_NOT_TRANSIENT ); + user_func1_name, nr_strlen(user_func1_name)); wraprec_found = nr_php_get_wraprec(user_func1_zf); tlib_pass_if_ptr_equal("lookup instrumented user function succeeds", wraprec_found, user_func1_wraprec); diff --git a/tests/integration/frameworks/drupal/skipif.inc b/tests/integration/frameworks/drupal/skipif.inc new file mode 100644 index 000000000..502ee78c9 --- /dev/null +++ b/tests/integration/frameworks/drupal/skipif.inc @@ -0,0 +1,9 @@ +=")) { + die("skip: PHP >= 8.2 not supported\n"); +} diff --git a/tests/integration/frameworks/drupal/test_invoke_all_with.php b/tests/integration/frameworks/drupal/test_invoke_all_with.php index 656b4fd61..289d4e69a 100644 --- a/tests/integration/frameworks/drupal/test_invoke_all_with.php +++ b/tests/integration/frameworks/drupal/test_invoke_all_with.php @@ -26,6 +26,7 @@ a3 b4 b4 +b2 */ /*EXPECT_METRICS @@ -37,11 +38,11 @@ [{"name":"DurationByCaller/Unknown/Unknown/Unknown/Unknown/all"}, [1, "??", "??", "??", "??", "??"]], [{"name":"DurationByCaller/Unknown/Unknown/Unknown/Unknown/allOther"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Framework/Drupal/Hook/hook_1"}, [2, "??", "??", "??", "??", "??"]], - [{"name":"Framework/Drupal/Hook/hook_2"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"Framework/Drupal/Hook/hook_2"}, [2, "??", "??", "??", "??", "??"]], [{"name":"Framework/Drupal/Hook/hook_3"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Framework/Drupal/Hook/hook_4"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Framework/Drupal/Module/module_a"}, [2, "??", "??", "??", "??", "??"]], - [{"name":"Framework/Drupal/Module/module_b"}, [3, "??", "??", "??", "??", "??"]], + [{"name":"Framework/Drupal/Module/module_b"}, [4, "??", "??", "??", "??", "??"]], [{"name":"OtherTransaction/all"}, [1, "??", "??", "??", "??", "??"]], [{"name":"OtherTransaction/php__FILE__"}, [1, "??", "??", "??", "??", "??"]], [{"name":"OtherTransactionTotalTime"}, [1, "??", "??", "??", "??", "??"]], @@ -49,10 +50,13 @@ [{"name":"Supportability/framework/Drupal8/forced"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Supportability/Logging/Forwarding/PHP/enabled"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Supportability/Logging/Metrics/PHP/enabled"}, [1, "??", "??", "??", "??", "??"]], - [{"name":"Supportability/api/add_custom_tracer"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"Supportability/api/add_custom_tracer"}, [2, "??", "??", "??", "??", "??"]], [{"name":"Custom/invoke_callback_instrumented"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"Custom/invoke_callback"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Custom/invoke_callback_instrumented", "scope":"OtherTransaction/php__FILE__"}, [1, "??", "??", "??", "??", "??"]], + [{"name":"Custom/invoke_callback", + "scope":"OtherTransaction/php__FILE__"}, [1, "??", "??", "??", "??", "??"]], [{"name":"Supportability/Logging/LocalDecorating/PHP/disabled"}, [1, "??", "??", "??", "??", "??"]] ] ] @@ -127,5 +131,15 @@ public function invoke(callable $hook, string $module) { /* At this point, module_b_hook_4 should NOT be instrumented */ // test string callback; function already instrumented +// This will reuse the existing wraprec and successfully +// add instrumentation because the "before" callback is unset $func_name = "invoke_callback_instrumented"; $handler->invokeallwith("hook_4", $func_name); + +// test non-transiently wrapping an already transiently instrumented function +// This will overwrite the existing transient wrapper +$func_name = "invoke_callback"; +newrelic_add_custom_tracer($func_name); +// Now this test will function the same as above: adding special instrumentation +// to an already existing wrapper +$handler->invokeallwith("hook_2", $func_name); diff --git a/tests/integration/frameworks/drupal/test_module_invoke_all.php b/tests/integration/frameworks/drupal/test_module_invoke_all.php new file mode 100644 index 000000000..46400cb81 --- /dev/null +++ b/tests/integration/frameworks/drupal/test_module_invoke_all.php @@ -0,0 +1,87 @@ + Date: Wed, 10 Jan 2024 14:46:46 -0500 Subject: [PATCH 6/9] refactor: reduce # of wp plugin/theme name calculation (#784) Improve agent's performance by figuring out the plugin/theme name at the point an action or filter is added and store it in the wraprec rather than doing it at runtime (which is slow, even with memoization in `wordpress_file_metadata` hashmap). --- agent/fw_wordpress.c | 21 ++++++++++++++++++++- agent/php_user_instrument.h | 3 +++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/agent/fw_wordpress.c b/agent/fw_wordpress.c index 232bb2841..b47384dbd 100644 --- a/agent/fw_wordpress.c +++ b/agent/fw_wordpress.c @@ -296,7 +296,9 @@ static char* nr_wordpress_plugin_from_function(zend_function* func TSRMLS_DC) { } NR_PHP_WRAPPER(nr_wordpress_wrap_hook) { +#if ZEND_MODULE_API_NO < ZEND_7_4_X_API_NO zend_function* func = NULL; +#endif char* plugin = NULL; NR_UNUSED_SPECIALFN; @@ -312,8 +314,12 @@ NR_PHP_WRAPPER(nr_wordpress_wrap_hook) { if ((0 == NRINI(wordpress_hooks)) || (NULL == NRPRG(wordpress_tag))) { NR_PHP_WRAPPER_LEAVE; } +#if ZEND_MODULE_API_NO < ZEND_7_4_X_API_NO func = nr_php_execute_function(NR_EXECUTE_ORIG_ARGS TSRMLS_CC); plugin = nr_wordpress_plugin_from_function(func TSRMLS_CC); +#else + plugin = wraprec->wordpress_plugin_theme; +#endif NR_PHP_WRAPPER_CALL; @@ -602,10 +608,23 @@ NR_PHP_WRAPPER(nr_wordpress_add_filter) { } if (true == wrap_hook) { + nruserfn_t* callback_wraprec; zval* callback = nr_php_arg_get(2, NR_EXECUTE_ORIG_ARGS); /* the callback here can be any PHP callable. nr_php_wrap_generic_callable * checks that a valid callable is passed */ - nr_php_wrap_generic_callable(callback, nr_wordpress_wrap_hook); + callback_wraprec = nr_php_wrap_generic_callable(callback, nr_wordpress_wrap_hook); + + // We can cheat here: wraprecs on callables are always transient, so if + // there's a wordpress_plugin_theme set we know it's from this transaction, + // and we don't have any issues around a possible multi-tenant setup. + if (callback_wraprec && NULL == callback_wraprec->wordpress_plugin_theme) { + // Unlike Drupal, we don't free the wordpress_plugin_theme, since we know + // it's transient anyway, and we only set the field if it was previously + // NULL. + zend_function* fn = nr_php_zval_to_function(callback); + callback_wraprec->wordpress_plugin_theme + = nr_wordpress_plugin_from_function(fn); + } nr_php_arg_release(&callback); } } diff --git a/agent/php_user_instrument.h b/agent/php_user_instrument.h index 4e71a9f82..50e2a54d3 100644 --- a/agent/php_user_instrument.h +++ b/agent/php_user_instrument.h @@ -91,6 +91,9 @@ typedef struct _nruserfn_t { nr_string_len_t drupal_module_len; char* drupal_hook; nr_string_len_t drupal_hook_len; +#if ZEND_MODULE_API_NO >= ZEND_7_4_X_API_NO + char* wordpress_plugin_theme; +#endif } nruserfn_t; extern nruserfn_t* nr_wrapped_user_functions; /* a singly linked list */ From ac78511ede0e96d59610b798b6a41b53a202364b Mon Sep 17 00:00:00 2001 From: Michal Nowacki Date: Wed, 10 Jan 2024 15:12:46 -0500 Subject: [PATCH 7/9] feat: instrument wordpress plugins' callbacks on demand (#799) Improve agent's performance by adding two new toggles around WordPress instrumentation: - `newrelic.framework.wordpress.plugins` - indicates if WordPress hooks callback functions, implemented in WordPress core, plugins and themes, are to be instrumented. - `newrelic.framework.wordpress.hooks_threshold` - sets the WordPress hook's execution duration threshold above which the hook execution will be captured; used when WordPress hooks callback functions are not instrumented. By default, WordPress hooks callback functions are not instrumented and `newrelic.framework.wordpress.hooks_threshold` is set at 1ms. This allows to identify which hooks take up most time during request processing. If more details are needed, i.e. which plugins are the slowest, `newrelic.framework.wordpress.plugins` need to be set to `true`. --- agent/fw_wordpress.c | 32 ++++++++++++++++--- agent/php_newrelic.h | 2 ++ agent/php_nrini.c | 16 ++++++++++ agent/scripts/newrelic.ini.template | 17 ++++++++++ .../test_wordpress_apply_filters.php | 1 + .../wordpress/test_wordpress_do_action.php | 1 + ...st_wordpress_transaction_name_hooks_on.php | 1 + 7 files changed, 65 insertions(+), 5 deletions(-) diff --git a/agent/fw_wordpress.c b/agent/fw_wordpress.c index b47384dbd..5299d1a48 100644 --- a/agent/fw_wordpress.c +++ b/agent/fw_wordpress.c @@ -215,6 +215,21 @@ static void free_wordpress_metadata(void* metadata) { nr_free(metadata); } +static inline void nr_wordpress_hooks_create_metric(nr_segment_t* segment, + const char* hook_name) { + if (NULL == segment) { + return; + } + // need to capture 'now' because segment has not finished yet and therefore + // can't use segment->stop_time + nrtime_t now = nr_txn_now_rel(segment->txn); + nrtime_t duration = nr_time_duration(segment->start_time, now); + if (duration >= NRINI(wordpress_hooks_threshold)) { + // only create metrics for hooks above threshold + nr_wordpress_create_metric(segment, NR_WORDPRESS_HOOK_PREFIX, hook_name); + } +} + static char* nr_wordpress_plugin_from_function(zend_function* func TSRMLS_DC) { const char* filename = NULL; size_t filename_len; @@ -452,6 +467,9 @@ NR_PHP_WRAPPER(nr_wordpress_exec_handle_tag) { NRPRG(wordpress_tag) = nr_wordpress_clean_tag(tag); NR_PHP_WRAPPER_CALL; + if (0 == NRINI(wordpress_plugins)) { + nr_wordpress_hooks_create_metric(auto_segment, NRPRG(wordpress_tag)); + } NRPRG(wordpress_tag) = old_tag; if (NULL == NRPRG(wordpress_tag)) { NRPRG(check_cufa) = false; @@ -541,6 +559,9 @@ NR_PHP_WRAPPER(nr_wordpress_apply_filters) { NRPRG(wordpress_tag) = nr_wordpress_clean_tag(tag); NR_PHP_WRAPPER_CALL; + if (0 == NRINI(wordpress_plugins)) { + nr_wordpress_hooks_create_metric(auto_segment, NRPRG(wordpress_tag)); + } NRPRG(wordpress_tag) = old_tag; if (NULL == NRPRG(wordpress_tag)) { NRPRG(check_cufa) = false; @@ -644,14 +665,15 @@ void nr_wordpress_enable(TSRMLS_D) { nr_php_wrap_user_function(NR_PSTR("do_action_ref_array"), nr_wordpress_exec_handle_tag TSRMLS_CC); - + if (0 != NRINI(wordpress_plugins)) { #if ZEND_MODULE_API_NO < ZEND_7_4_X_API_NO - nr_php_add_call_user_func_array_pre_callback( - nr_wordpress_call_user_func_array TSRMLS_CC); + nr_php_add_call_user_func_array_pre_callback( + nr_wordpress_call_user_func_array TSRMLS_CC); #else - nr_php_wrap_user_function(NR_PSTR("add_filter"), - nr_wordpress_add_filter); + nr_php_wrap_user_function(NR_PSTR("add_filter"), + nr_wordpress_add_filter); #endif + } } } diff --git a/agent/php_newrelic.h b/agent/php_newrelic.h index 82d746b78..f869351c0 100644 --- a/agent/php_newrelic.h +++ b/agent/php_newrelic.h @@ -268,6 +268,8 @@ nrinistr_t browser_monitoring_loader; /* newrelic.browser_monitoring.loader */ nrinibool_t drupal_modules; /* newrelic.framework.drupal.modules */ nrinibool_t wordpress_hooks; /* newrelic.framework.wordpress.hooks */ +nrinitime_t wordpress_hooks_threshold; /* newrelic.framework.wordpress.hooks_threshold */ +nrinibool_t wordpress_plugins; /* newrelic.framework.wordpress.plugins */ nrinistr_t wordpress_hooks_skip_filename; /* newrelic.framework.wordpress.hooks_skip_filename */ diff --git a/agent/php_nrini.c b/agent/php_nrini.c index cea0f216f..13471ba60 100644 --- a/agent/php_nrini.c +++ b/agent/php_nrini.c @@ -2206,6 +2206,22 @@ STD_PHP_INI_ENTRY_EX("newrelic.framework.wordpress.hooks", zend_newrelic_globals, newrelic_globals, nr_on_off_dh) +STD_PHP_INI_ENTRY_EX("newrelic.framework.wordpress.hooks_threshold", + "1ms", + NR_PHP_REQUEST, + nr_time_mh, + wordpress_hooks_threshold, + zend_newrelic_globals, + newrelic_globals, + 0) +STD_PHP_INI_ENTRY_EX("newrelic.framework.wordpress.plugins", + "0", + NR_PHP_REQUEST, + nr_boolean_mh, + wordpress_plugins, + zend_newrelic_globals, + newrelic_globals, + nr_on_off_dh) STD_PHP_INI_ENTRY_EX("newrelic.framework.wordpress.hooks_skip_filename", "", NR_PHP_REQUEST, diff --git a/agent/scripts/newrelic.ini.template b/agent/scripts/newrelic.ini.template index 9e36d4bdc..07d6fcfa5 100644 --- a/agent/scripts/newrelic.ini.template +++ b/agent/scripts/newrelic.ini.template @@ -1166,6 +1166,23 @@ newrelic.daemon.logfile = "/var/log/newrelic/newrelic-daemon.log" ; ;newrelic.framework.wordpress.hooks = true +; Setting: newrelic.framework.wordpress.hooks_threshold +; Type : time specification string ("500ms", "1s750ms" etc) +; Scope : per-directory +; Default: 1ms +; Info : Sets the threshold above which the New Relic agent will record a +; wordpress hooks. +; +;newrelic.framework.wordpress.hooks_threshold = 1ms + +; Setting: newrelic.framework.wordpress.plugins +; Type : boolean +; Scope : per-directory +; Default: false +; Info : Indicates if WordPress plugins are to be instrumented. +; +;newrelic.framework.wordpress.plugins = false + ; Setting: newrelic.application_logging.enabled ; Type : boolean ; Scope : per-directory diff --git a/tests/integration/frameworks/wordpress/test_wordpress_apply_filters.php b/tests/integration/frameworks/wordpress/test_wordpress_apply_filters.php index e0f072592..223127ef8 100644 --- a/tests/integration/frameworks/wordpress/test_wordpress_apply_filters.php +++ b/tests/integration/frameworks/wordpress/test_wordpress_apply_filters.php @@ -13,6 +13,7 @@ /*INI newrelic.framework = wordpress +newrelic.framework.wordpress.hooks_threshold = 0 */ /*EXPECT diff --git a/tests/integration/frameworks/wordpress/test_wordpress_do_action.php b/tests/integration/frameworks/wordpress/test_wordpress_do_action.php index ab70d7d53..901cdbdfc 100644 --- a/tests/integration/frameworks/wordpress/test_wordpress_do_action.php +++ b/tests/integration/frameworks/wordpress/test_wordpress_do_action.php @@ -13,6 +13,7 @@ /*INI newrelic.framework = wordpress +newrelic.framework.wordpress.hooks_threshold = 0 */ /*EXPECT diff --git a/tests/integration/frameworks/wordpress/test_wordpress_transaction_name_hooks_on.php b/tests/integration/frameworks/wordpress/test_wordpress_transaction_name_hooks_on.php index 2b77a7487..66488513f 100644 --- a/tests/integration/frameworks/wordpress/test_wordpress_transaction_name_hooks_on.php +++ b/tests/integration/frameworks/wordpress/test_wordpress_transaction_name_hooks_on.php @@ -16,6 +16,7 @@ /*INI newrelic.framework = wordpress newrelic.framework.wordpress.hooks = true +newrelic.framework.wordpress.hooks_threshold = 0 */ /*ENVIRONMENT From 5b90fb104aa605f0d6e0007ce58d70b3e93babc2 Mon Sep 17 00:00:00 2001 From: Michal Nowacki Date: Wed, 17 Jan 2024 11:47:25 -0500 Subject: [PATCH 8/9] feat: instrument wordpress core callbacks on demand (#802) Improve agent's performance by adding a new toggle around WordPress instrumentation: * `newrelic.framework.wordpress.core` - indicates if WordPress hooks callback functions, implemented in WordPress core, are to be instrumented. By default WordPress hook callbacks from WordPress core are not instrumented because it increases agent's overhead. It is however possible to enable this instrumentation with INI setting. --- agent/fw_wordpress.c | 41 +++++----- agent/php_newrelic.h | 1 + agent/php_nrini.c | 10 ++- agent/scripts/newrelic.ini.template | 12 ++- .../wordpress/mock-wordpress-app.php | 38 ++++++++++ .../wordpress/test_wordpress_00.php | 54 ++++++++++++++ .../wordpress/test_wordpress_00.php7.php | 46 ++++++++++++ .../wordpress/test_wordpress_01.php | 47 ++++++++++++ .../wordpress/test_wordpress_02.php | 53 +++++++++++++ .../wordpress/test_wordpress_03.php | 60 +++++++++++++++ .../wordpress/test_wordpress_03.php7.php | 52 +++++++++++++ .../wordpress/test_wordpress_04.php | 61 +++++++++++++++ .../wordpress/test_wordpress_04.php7.php | 53 +++++++++++++ .../test_wordpress_apply_filters.php | 7 +- .../wordpress/test_wordpress_do_action.php | 7 +- .../test_wordpress_slow_hooks_only.php | 74 +++++++++++++++++++ ...st_wordpress_transaction_name_hooks_on.php | 5 ++ .../frameworks/wordpress/wp-config.php | 6 ++ .../wp-content/plugins/mock-plugin1.php | 6 ++ .../wp-content/plugins/mock-plugin2.php | 6 ++ .../wp-content/themes/mock-theme1.php | 7 ++ .../wp-content/themes/mock-theme2.php | 7 ++ .../wordpress/wp-includes/functions.php | 29 ++++++++ .../wordpress/wp-includes/plugin.php | 32 ++++++++ .../frameworks/wordpress/wp-settings.php | 4 + 25 files changed, 695 insertions(+), 23 deletions(-) create mode 100644 tests/integration/frameworks/wordpress/mock-wordpress-app.php create mode 100644 tests/integration/frameworks/wordpress/test_wordpress_00.php create mode 100644 tests/integration/frameworks/wordpress/test_wordpress_00.php7.php create mode 100644 tests/integration/frameworks/wordpress/test_wordpress_01.php create mode 100644 tests/integration/frameworks/wordpress/test_wordpress_02.php create mode 100644 tests/integration/frameworks/wordpress/test_wordpress_03.php create mode 100644 tests/integration/frameworks/wordpress/test_wordpress_03.php7.php create mode 100644 tests/integration/frameworks/wordpress/test_wordpress_04.php create mode 100644 tests/integration/frameworks/wordpress/test_wordpress_04.php7.php create mode 100644 tests/integration/frameworks/wordpress/test_wordpress_slow_hooks_only.php create mode 100644 tests/integration/frameworks/wordpress/wp-config.php create mode 100644 tests/integration/frameworks/wordpress/wp-content/plugins/mock-plugin1.php create mode 100644 tests/integration/frameworks/wordpress/wp-content/plugins/mock-plugin2.php create mode 100644 tests/integration/frameworks/wordpress/wp-content/themes/mock-theme1.php create mode 100644 tests/integration/frameworks/wordpress/wp-content/themes/mock-theme2.php create mode 100644 tests/integration/frameworks/wordpress/wp-includes/functions.php create mode 100644 tests/integration/frameworks/wordpress/wp-includes/plugin.php create mode 100644 tests/integration/frameworks/wordpress/wp-settings.php diff --git a/agent/fw_wordpress.c b/agent/fw_wordpress.c index 5299d1a48..77c72164f 100644 --- a/agent/fw_wordpress.c +++ b/agent/fw_wordpress.c @@ -337,10 +337,12 @@ NR_PHP_WRAPPER(nr_wordpress_wrap_hook) { #endif NR_PHP_WRAPPER_CALL; - - nr_wordpress_create_metric(auto_segment, NR_WORDPRESS_HOOK_PREFIX, - NRPRG(wordpress_tag)); - nr_wordpress_create_metric(auto_segment, NR_WORDPRESS_PLUGIN_PREFIX, plugin); + if (NULL != plugin || NRINI(wordpress_core)) { + nr_wordpress_create_metric(auto_segment, NR_WORDPRESS_HOOK_PREFIX, + NRPRG(wordpress_tag)); + nr_wordpress_create_metric(auto_segment, NR_WORDPRESS_PLUGIN_PREFIX, + plugin); + } } NR_PHP_WRAPPER_END @@ -631,20 +633,23 @@ NR_PHP_WRAPPER(nr_wordpress_add_filter) { if (true == wrap_hook) { nruserfn_t* callback_wraprec; zval* callback = nr_php_arg_get(2, NR_EXECUTE_ORIG_ARGS); - /* the callback here can be any PHP callable. nr_php_wrap_generic_callable - * checks that a valid callable is passed */ - callback_wraprec = nr_php_wrap_generic_callable(callback, nr_wordpress_wrap_hook); - - // We can cheat here: wraprecs on callables are always transient, so if - // there's a wordpress_plugin_theme set we know it's from this transaction, - // and we don't have any issues around a possible multi-tenant setup. - if (callback_wraprec && NULL == callback_wraprec->wordpress_plugin_theme) { - // Unlike Drupal, we don't free the wordpress_plugin_theme, since we know - // it's transient anyway, and we only set the field if it was previously - // NULL. - zend_function* fn = nr_php_zval_to_function(callback); - callback_wraprec->wordpress_plugin_theme - = nr_wordpress_plugin_from_function(fn); + zend_function* zf = nr_php_zval_to_function(callback); + if (NULL != zf) { + char* wordpress_plugin_theme = nr_wordpress_plugin_from_function(zf); + if (NULL != wordpress_plugin_theme || NRINI(wordpress_core)) { + callback_wraprec = nr_php_wrap_callable(zf, nr_wordpress_wrap_hook); + // We can cheat here: wraprecs on callables are always transient, so if + // there's a wordpress_plugin_theme set we know it's from this + // transaction, and we don't have any issues around a possible + // multi-tenant setup. + if (NULL != callback_wraprec + && NULL == callback_wraprec->wordpress_plugin_theme) { + // Unlike Drupal, we don't free the wordpress_plugin_theme, since we + // know it's transient anyway, and we only set the field if it was + // previously NULL. + callback_wraprec->wordpress_plugin_theme = wordpress_plugin_theme; + } + } } nr_php_arg_release(&callback); } diff --git a/agent/php_newrelic.h b/agent/php_newrelic.h index f869351c0..ad1522ff3 100644 --- a/agent/php_newrelic.h +++ b/agent/php_newrelic.h @@ -270,6 +270,7 @@ nrinibool_t drupal_modules; /* newrelic.framework.drupal.modules */ nrinibool_t wordpress_hooks; /* newrelic.framework.wordpress.hooks */ nrinitime_t wordpress_hooks_threshold; /* newrelic.framework.wordpress.hooks_threshold */ nrinibool_t wordpress_plugins; /* newrelic.framework.wordpress.plugins */ +nrinibool_t wordpress_core; /* newrelic.framework.wordpress.core */ nrinistr_t wordpress_hooks_skip_filename; /* newrelic.framework.wordpress.hooks_skip_filename */ diff --git a/agent/php_nrini.c b/agent/php_nrini.c index 13471ba60..4513699a1 100644 --- a/agent/php_nrini.c +++ b/agent/php_nrini.c @@ -2215,13 +2215,21 @@ STD_PHP_INI_ENTRY_EX("newrelic.framework.wordpress.hooks_threshold", newrelic_globals, 0) STD_PHP_INI_ENTRY_EX("newrelic.framework.wordpress.plugins", - "0", + "1", NR_PHP_REQUEST, nr_boolean_mh, wordpress_plugins, zend_newrelic_globals, newrelic_globals, nr_on_off_dh) +STD_PHP_INI_ENTRY_EX("newrelic.framework.wordpress.core", + "0", + NR_PHP_REQUEST, + nr_boolean_mh, + wordpress_core, + zend_newrelic_globals, + newrelic_globals, + nr_on_off_dh) STD_PHP_INI_ENTRY_EX("newrelic.framework.wordpress.hooks_skip_filename", "", NR_PHP_REQUEST, diff --git a/agent/scripts/newrelic.ini.template b/agent/scripts/newrelic.ini.template index 07d6fcfa5..37dd1ad9a 100644 --- a/agent/scripts/newrelic.ini.template +++ b/agent/scripts/newrelic.ini.template @@ -1178,10 +1178,18 @@ newrelic.daemon.logfile = "/var/log/newrelic/newrelic-daemon.log" ; Setting: newrelic.framework.wordpress.plugins ; Type : boolean ; Scope : per-directory -; Default: false +; Default: true ; Info : Indicates if WordPress plugins are to be instrumented. ; -;newrelic.framework.wordpress.plugins = false +;newrelic.framework.wordpress.plugins = true + +; Setting: newrelic.framework.wordpress.core +; Type : boolean +; Scope : per-directory +; Default: false +; Info : Indicates if WordPress core hook callbacks are to be instrumented. +; +;newrelic.framework.wordpress.core = false ; Setting: newrelic.application_logging.enabled ; Type : boolean diff --git a/tests/integration/frameworks/wordpress/mock-wordpress-app.php b/tests/integration/frameworks/wordpress/mock-wordpress-app.php new file mode 100644 index 000000000..a15387e08 --- /dev/null +++ b/tests/integration/frameworks/wordpress/mock-wordpress-app.php @@ -0,0 +1,38 @@ += 7.4 required\n"); +} +*/ + +/*ENVIRONMENT +REQUEST_METHOD=GET +*/ + +/*EXPECT_METRICS_EXIST +Supportability/framework/WordPress/detected +WebTransaction/Action/page-template +Supportability/InstrumentedFunction/apply_filters +Supportability/InstrumentedFunction/do_action +Supportability/InstrumentedFunction/add_filter +Framework/WordPress/Hook/wp_loaded +Framework/WordPress/Hook/template_include +Framework/WordPress/Plugin/mock-plugin1 +Framework/WordPress/Plugin/mock-plugin2 +Framework/WordPress/Plugin/mock-theme1 +Framework/WordPress/Plugin/mock-theme2 +*/ + +/*EXPECT_METRICS_DONT_EXIST +Framework/WordPress/Hook/wp_init +Framework/WordPress/Hook/the_content +*/ + +/*EXPECT_ERROR_EVENTS null */ + +/* WordPress mock app */ +require_once __DIR__.'/mock-wordpress-app.php'; diff --git a/tests/integration/frameworks/wordpress/test_wordpress_00.php7.php b/tests/integration/frameworks/wordpress/test_wordpress_00.php7.php new file mode 100644 index 000000000..2cf1bcaf1 --- /dev/null +++ b/tests/integration/frameworks/wordpress/test_wordpress_00.php7.php @@ -0,0 +1,46 @@ += 7.4 required\n"); +} +*/ + +/*INI +newrelic.framework.wordpress.hooks = true +newrelic.framework.wordpress.plugins = true +*/ + +/*ENVIRONMENT +REQUEST_METHOD=GET +*/ + +/*EXPECT_METRICS_EXIST +Supportability/framework/WordPress/detected +WebTransaction/Action/page-template +Supportability/InstrumentedFunction/apply_filters +Supportability/InstrumentedFunction/do_action +Supportability/InstrumentedFunction/add_filter +Framework/WordPress/Hook/wp_loaded +Framework/WordPress/Hook/template_include +Framework/WordPress/Plugin/mock-plugin1 +Framework/WordPress/Plugin/mock-plugin2 +Framework/WordPress/Plugin/mock-theme1 +Framework/WordPress/Plugin/mock-theme2 +*/ + +/*EXPECT_METRICS_DONT_EXIST +Framework/WordPress/Hook/wp_init +Framework/WordPress/Hook/the_content +*/ + +/*EXPECT_ERROR_EVENTS null */ + +/* WordPress mock app */ +require_once __DIR__.'/mock-wordpress-app.php'; diff --git a/tests/integration/frameworks/wordpress/test_wordpress_03.php7.php b/tests/integration/frameworks/wordpress/test_wordpress_03.php7.php new file mode 100644 index 000000000..e713db0e5 --- /dev/null +++ b/tests/integration/frameworks/wordpress/test_wordpress_03.php7.php @@ -0,0 +1,52 @@ += 7.4 required\n"); +} +*/ + +/*INI +newrelic.framework.wordpress.hooks = true +newrelic.framework.wordpress.plugins = true +newrelic.framework.wordpress.core = true +*/ + +/*ENVIRONMENT +REQUEST_METHOD=GET +*/ + +/*EXPECT_METRICS_EXIST +Supportability/framework/WordPress/detected +WebTransaction/Action/page-template +Supportability/InstrumentedFunction/apply_filters +Supportability/InstrumentedFunction/do_action +Supportability/InstrumentedFunction/add_filter +Framework/WordPress/Hook/wp_loaded +Framework/WordPress/Hook/template_include +Framework/WordPress/Plugin/mock-plugin1 +Framework/WordPress/Plugin/mock-plugin2 +Framework/WordPress/Plugin/mock-theme1 +Framework/WordPress/Plugin/mock-theme2 +Framework/WordPress/Hook/wp_init +Framework/WordPress/Hook/the_content +*/ + +/*EXPECT_METRICS_DONT_EXIST +*/ + +/*EXPECT_ERROR_EVENTS null */ + +/* WordPress mock app */ +require_once __DIR__.'/mock-wordpress-app.php'; diff --git a/tests/integration/frameworks/wordpress/test_wordpress_04.php7.php b/tests/integration/frameworks/wordpress/test_wordpress_04.php7.php new file mode 100644 index 000000000..1178a3a2b --- /dev/null +++ b/tests/integration/frameworks/wordpress/test_wordpress_04.php7.php @@ -0,0 +1,53 @@ + Date: Wed, 17 Jan 2024 16:26:18 -0500 Subject: [PATCH 9/9] refactor: improve wordpress hooks ini (#814) Hide boolean flags controling wordpress hooks instrumentation behind a `newrelic.framework.wordpress.hooks.options` facade. The facade makes invalid combinations of boolean flags impossible to set. Additionally do not surprise users with a change to agent behavior. Set the new toggles to values that maintain behavior. This allows for transition period when users can understand the new feature and be prepared for the change. --- agent/fw_wordpress.c | 10 ++-- agent/php_newrelic.h | 11 +++- agent/php_nrini.c | 60 +++++++++++++------ agent/scripts/newrelic.ini.template | 36 +++++------ .../wordpress/test_wordpress_00.php | 9 ++- .../wordpress/test_wordpress_00.php7.php | 9 ++- .../wordpress/test_wordpress_00_1.php | 47 +++++++++++++++ .../wordpress/test_wordpress_00_2.php | 47 +++++++++++++++ .../wordpress/test_wordpress_02.php | 4 +- .../wordpress/test_wordpress_03.php | 2 +- .../wordpress/test_wordpress_03.php7.php | 2 +- .../wordpress/test_wordpress_04.php | 3 +- .../wordpress/test_wordpress_04.php7.php | 3 +- .../test_wordpress_slow_hooks_only.php | 4 +- 14 files changed, 184 insertions(+), 63 deletions(-) create mode 100644 tests/integration/frameworks/wordpress/test_wordpress_00_1.php create mode 100644 tests/integration/frameworks/wordpress/test_wordpress_00_2.php diff --git a/agent/fw_wordpress.c b/agent/fw_wordpress.c index 77c72164f..ac59f29b3 100644 --- a/agent/fw_wordpress.c +++ b/agent/fw_wordpress.c @@ -337,7 +337,7 @@ NR_PHP_WRAPPER(nr_wordpress_wrap_hook) { #endif NR_PHP_WRAPPER_CALL; - if (NULL != plugin || NRINI(wordpress_core)) { + if (NULL != plugin || NRPRG(wordpress_core)) { nr_wordpress_create_metric(auto_segment, NR_WORDPRESS_HOOK_PREFIX, NRPRG(wordpress_tag)); nr_wordpress_create_metric(auto_segment, NR_WORDPRESS_PLUGIN_PREFIX, @@ -469,7 +469,7 @@ NR_PHP_WRAPPER(nr_wordpress_exec_handle_tag) { NRPRG(wordpress_tag) = nr_wordpress_clean_tag(tag); NR_PHP_WRAPPER_CALL; - if (0 == NRINI(wordpress_plugins)) { + if (0 == NRPRG(wordpress_plugins)) { nr_wordpress_hooks_create_metric(auto_segment, NRPRG(wordpress_tag)); } NRPRG(wordpress_tag) = old_tag; @@ -561,7 +561,7 @@ NR_PHP_WRAPPER(nr_wordpress_apply_filters) { NRPRG(wordpress_tag) = nr_wordpress_clean_tag(tag); NR_PHP_WRAPPER_CALL; - if (0 == NRINI(wordpress_plugins)) { + if (0 == NRPRG(wordpress_plugins)) { nr_wordpress_hooks_create_metric(auto_segment, NRPRG(wordpress_tag)); } NRPRG(wordpress_tag) = old_tag; @@ -636,7 +636,7 @@ NR_PHP_WRAPPER(nr_wordpress_add_filter) { zend_function* zf = nr_php_zval_to_function(callback); if (NULL != zf) { char* wordpress_plugin_theme = nr_wordpress_plugin_from_function(zf); - if (NULL != wordpress_plugin_theme || NRINI(wordpress_core)) { + if (NULL != wordpress_plugin_theme || NRPRG(wordpress_core)) { callback_wraprec = nr_php_wrap_callable(zf, nr_wordpress_wrap_hook); // We can cheat here: wraprecs on callables are always transient, so if // there's a wordpress_plugin_theme set we know it's from this @@ -670,7 +670,7 @@ void nr_wordpress_enable(TSRMLS_D) { nr_php_wrap_user_function(NR_PSTR("do_action_ref_array"), nr_wordpress_exec_handle_tag TSRMLS_CC); - if (0 != NRINI(wordpress_plugins)) { + if (0 != NRPRG(wordpress_plugins)) { #if ZEND_MODULE_API_NO < ZEND_7_4_X_API_NO nr_php_add_call_user_func_array_pre_callback( nr_wordpress_call_user_func_array TSRMLS_CC); diff --git a/agent/php_newrelic.h b/agent/php_newrelic.h index ad1522ff3..5fbb8f1bd 100644 --- a/agent/php_newrelic.h +++ b/agent/php_newrelic.h @@ -268,9 +268,14 @@ nrinistr_t browser_monitoring_loader; /* newrelic.browser_monitoring.loader */ nrinibool_t drupal_modules; /* newrelic.framework.drupal.modules */ nrinibool_t wordpress_hooks; /* newrelic.framework.wordpress.hooks */ -nrinitime_t wordpress_hooks_threshold; /* newrelic.framework.wordpress.hooks_threshold */ -nrinibool_t wordpress_plugins; /* newrelic.framework.wordpress.plugins */ -nrinibool_t wordpress_core; /* newrelic.framework.wordpress.core */ +nrinistr_t + wordpress_hooks_options; /* newrelic.framework.wordpress.hooks.options */ +nrinitime_t wordpress_hooks_threshold; /* newrelic.framework.wordpress.hooks.threshold + */ +bool wordpress_plugins; /* set based on + newrelic.framework.wordpress.hooks.options */ +bool wordpress_core; /* set based on + newrelic.framework.wordpress.hooks.options */ nrinistr_t wordpress_hooks_skip_filename; /* newrelic.framework.wordpress.hooks_skip_filename */ diff --git a/agent/php_nrini.c b/agent/php_nrini.c index 4513699a1..c97f88d34 100644 --- a/agent/php_nrini.c +++ b/agent/php_nrini.c @@ -1891,6 +1891,40 @@ static PHP_INI_MH(nr_custom_events_max_samples_stored_mh) { return SUCCESS; } +#define DEFAULT_WORDPRESS_HOOKS_OPTIONS "all_callbacks" +static PHP_INI_MH(nr_wordpress_hooks_options_mh) { + nrinistr_t* p; + + char* base = (char*)mh_arg2; + + p = (nrinistr_t*)(base + (size_t)mh_arg1); + + (void)mh_arg3; + NR_UNUSED_TSRMLS; + + if (0 == nr_strcmp(NEW_VALUE, "all_callbacks")) { + NRPRG(wordpress_plugins) = true; + NRPRG(wordpress_core) = true; + } else if (0 == nr_strcmp(NEW_VALUE, "plugin_callbacks")) { + NRPRG(wordpress_plugins) = true; + NRPRG(wordpress_core) = false; + } else if (0 == nr_strcmp(NEW_VALUE, "threshold")) { + NRPRG(wordpress_plugins) = false; + NRPRG(wordpress_core) = false; + } else { + nrl_warning(NRL_INIT, "Invalid %s value \"%s\"; using \"%s\" instead.", + ZEND_STRING_VALUE(entry->name), NEW_VALUE, + DEFAULT_WORDPRESS_HOOKS_OPTIONS); + /* This will cause PHP to call the handler again with default value */ + return FAILURE; + } + + p->value = NEW_VALUE; + p->where = stage; + + return SUCCESS; +} + /* * Now for the actual INI entry table. Please note there are two types of INI * entry specification used. @@ -2206,30 +2240,22 @@ STD_PHP_INI_ENTRY_EX("newrelic.framework.wordpress.hooks", zend_newrelic_globals, newrelic_globals, nr_on_off_dh) -STD_PHP_INI_ENTRY_EX("newrelic.framework.wordpress.hooks_threshold", - "1ms", +STD_PHP_INI_ENTRY_EX("newrelic.framework.wordpress.hooks.options", + DEFAULT_WORDPRESS_HOOKS_OPTIONS, NR_PHP_REQUEST, - nr_time_mh, - wordpress_hooks_threshold, + nr_wordpress_hooks_options_mh, + wordpress_hooks_options, zend_newrelic_globals, newrelic_globals, 0) -STD_PHP_INI_ENTRY_EX("newrelic.framework.wordpress.plugins", - "1", - NR_PHP_REQUEST, - nr_boolean_mh, - wordpress_plugins, - zend_newrelic_globals, - newrelic_globals, - nr_on_off_dh) -STD_PHP_INI_ENTRY_EX("newrelic.framework.wordpress.core", - "0", +STD_PHP_INI_ENTRY_EX("newrelic.framework.wordpress.hooks.threshold", + "1ms", NR_PHP_REQUEST, - nr_boolean_mh, - wordpress_core, + nr_time_mh, + wordpress_hooks_threshold, zend_newrelic_globals, newrelic_globals, - nr_on_off_dh) + 0) STD_PHP_INI_ENTRY_EX("newrelic.framework.wordpress.hooks_skip_filename", "", NR_PHP_REQUEST, diff --git a/agent/scripts/newrelic.ini.template b/agent/scripts/newrelic.ini.template index 37dd1ad9a..40cb464e7 100644 --- a/agent/scripts/newrelic.ini.template +++ b/agent/scripts/newrelic.ini.template @@ -1166,30 +1166,30 @@ newrelic.daemon.logfile = "/var/log/newrelic/newrelic-daemon.log" ; ;newrelic.framework.wordpress.hooks = true -; Setting: newrelic.framework.wordpress.hooks_threshold -; Type : time specification string ("500ms", "1s750ms" etc) +; Setting: newrelic.framework.wordpress.hooks.options +; Type : string (all_callbacks, plugin_callbacks, threshold) ; Scope : per-directory -; Default: 1ms -; Info : Sets the threshold above which the New Relic agent will record a -; wordpress hooks. +; Default: all_callbacks +; Info : Sets the options how WordPress hooks are instrumented. ; -;newrelic.framework.wordpress.hooks_threshold = 1ms - -; Setting: newrelic.framework.wordpress.plugins -; Type : boolean -; Scope : per-directory -; Default: true -; Info : Indicates if WordPress plugins are to be instrumented. +; New Relic agent can provide different levels of insights into WordPress hooks. +; By default, all hook callbacks functions are instrumented ("all_callbacks"). +; To reduce agent's overhead it is possible to limit the instrumentation to only +; plugin/theme callbacks ("plugin_callbacks"). Third option is to monitor hooks +; without instrumenting callbacks ("threshold"). This option does not give insights +; about plugins/themes. ; -;newrelic.framework.wordpress.plugins = true +;newrelic.framework.wordpress.hooks.options = "all_callbacks" -; Setting: newrelic.framework.wordpress.core -; Type : boolean +; Setting: newrelic.framework.wordpress.hooks.threshold +; Type : time specification string ("500ms", "1s750ms" etc) ; Scope : per-directory -; Default: false -; Info : Indicates if WordPress core hook callbacks are to be instrumented. +; Default: 1ms +; Info : Sets the threshold above which the New Relic agent will record a +; wordpress hooks. Used when newrelic.framework.wordpress.hooks.options +; is set to "threshold". ; -;newrelic.framework.wordpress.core = false +;newrelic.framework.wordpress.hooks.threshold = 1ms ; Setting: newrelic.application_logging.enabled ; Type : boolean diff --git a/tests/integration/frameworks/wordpress/test_wordpress_00.php b/tests/integration/frameworks/wordpress/test_wordpress_00.php index 99726e2e3..946e497ba 100644 --- a/tests/integration/frameworks/wordpress/test_wordpress_00.php +++ b/tests/integration/frameworks/wordpress/test_wordpress_00.php @@ -9,9 +9,8 @@ - detect WordPress framework - name the web transaction as an 'Action' named after the template used to generate the page - detect custom plugins and themes - - generate hooks metrics but only when hook executes functions from custom - plugin or theme; the agent must not generate hooks metrics when hook executes - functions from wordpress core + - generate hooks metrics for all callback functions executions; the execution time of callback + functions from wordpress core, custom plugins and themes is captured. No errors should be generated. */ @@ -41,11 +40,11 @@ functions from wordpress core Framework/WordPress/Plugin/mock-plugin2 Framework/WordPress/Plugin/mock-theme1 Framework/WordPress/Plugin/mock-theme2 +Framework/WordPress/Hook/wp_init +Framework/WordPress/Hook/the_content */ /*EXPECT_METRICS_DONT_EXIST -Framework/WordPress/Hook/wp_init -Framework/WordPress/Hook/the_content */ /*EXPECT_ERROR_EVENTS null */ diff --git a/tests/integration/frameworks/wordpress/test_wordpress_00.php7.php b/tests/integration/frameworks/wordpress/test_wordpress_00.php7.php index 2cf1bcaf1..fa2cfa826 100644 --- a/tests/integration/frameworks/wordpress/test_wordpress_00.php7.php +++ b/tests/integration/frameworks/wordpress/test_wordpress_00.php7.php @@ -9,9 +9,8 @@ - detect WordPress framework - name the web transaction as an 'Action' named after the template used to generate the page - detect custom plugins and themes - - generate hooks metrics but only when hook executes functions from custom - plugin or theme; the agent must not generate hooks metrics when hook executes - functions from wordpress core + - generate hooks metrics for all callback functions executions; the execution time of callback + functions from wordpress core, custom plugins and themes is captured. No errors should be generated. */ @@ -33,11 +32,11 @@ functions from wordpress core Framework/WordPress/Plugin/mock-plugin2 Framework/WordPress/Plugin/mock-theme1 Framework/WordPress/Plugin/mock-theme2 +Framework/WordPress/Hook/wp_init +Framework/WordPress/Hook/the_content */ /*EXPECT_METRICS_DONT_EXIST -Framework/WordPress/Hook/wp_init -Framework/WordPress/Hook/the_content */ /*EXPECT_ERROR_EVENTS null */ diff --git a/tests/integration/frameworks/wordpress/test_wordpress_00_1.php b/tests/integration/frameworks/wordpress/test_wordpress_00_1.php new file mode 100644 index 000000000..c12a731de --- /dev/null +++ b/tests/integration/frameworks/wordpress/test_wordpress_00_1.php @@ -0,0 +1,47 @@ +