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

@@ -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);

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
                                     */

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,

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

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 */

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 */

tests/integration/frameworks/wordpress/test_wordpress_00_1.php

@@ -0,0 +1,47 @@
+<?php
+/*
+ * Copyright 2020 New Relic Corporation. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+/*DESCRIPTION
+Test that agent falls back to WordPress default instrumentation when the settings in INI are invalid.
+The agent should:
+ - 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 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.
+*/
+
+/*INI
+newrelic.framework.wordpress.hooks.options=
+*/
+
+/*ENVIRONMENT
+REQUEST_METHOD=GET
+*/
+
+/*EXPECT_METRICS_EXIST
+Supportability/framework/WordPress/detected
+WebTransaction/Action/page-template
+Supportability/InstrumentedFunction/apply_filters
+Supportability/InstrumentedFunction/do_action
+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';

tests/integration/frameworks/wordpress/test_wordpress_00_2.php

@@ -0,0 +1,47 @@
+<?php
+/*
+ * Copyright 2020 New Relic Corporation. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+/*DESCRIPTION
+Test that agent falls back to WordPress default instrumentation when the settings in INI are invalid.
+The agent should:
+ - 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 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.
+*/
+
+/*INI
+newrelic.framework.wordpress.hooks.options="foobar"
+*/
+
+/*ENVIRONMENT
+REQUEST_METHOD=GET
+*/
+
+/*EXPECT_METRICS_EXIST
+Supportability/framework/WordPress/detected
+WebTransaction/Action/page-template
+Supportability/InstrumentedFunction/apply_filters
+Supportability/InstrumentedFunction/do_action
+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';

tests/integration/frameworks/wordpress/test_wordpress_02.php

@@ -20,8 +20,8 @@
 
 /*INI
 newrelic.framework.wordpress.hooks = true
-newrelic.framework.wordpress.plugins = false
-newrelic.framework.wordpress.hooks_threshold = 0
+newrelic.framework.wordpress.hooks.options = threshold
+newrelic.framework.wordpress.hooks.threshold = 0
 */
 
 /*ENVIRONMENT

tests/integration/frameworks/wordpress/test_wordpress_03.php

@@ -28,7 +28,7 @@
 
 /*INI
 newrelic.framework.wordpress.hooks = true
-newrelic.framework.wordpress.plugins = true
+newrelic.framework.wordpress.hooks.options = plugin_callbacks
 */
 
 /*ENVIRONMENT

tests/integration/frameworks/wordpress/test_wordpress_03.php7.php

@@ -21,7 +21,7 @@
 
 /*INI
 newrelic.framework.wordpress.hooks = true
-newrelic.framework.wordpress.plugins = true
+newrelic.framework.wordpress.hooks.options = plugin_callbacks
 */
 
 /*ENVIRONMENT

tests/integration/frameworks/wordpress/test_wordpress_04.php

@@ -28,8 +28,7 @@ functions from wordpress core, custom plugins and themes is captured.
 
 /*INI
 newrelic.framework.wordpress.hooks = true
-newrelic.framework.wordpress.plugins = true
-newrelic.framework.wordpress.core = true
+newrelic.framework.wordpress.hooks.options = all_callbacks
 */
 
 /*ENVIRONMENT

tests/integration/frameworks/wordpress/test_wordpress_04.php7.php

@@ -21,8 +21,7 @@ functions from wordpress core, custom plugins and themes is captured.
 
 /*INI
 newrelic.framework.wordpress.hooks = true
-newrelic.framework.wordpress.plugins = true
-newrelic.framework.wordpress.core = true
+newrelic.framework.wordpress.hooks.options = all_callbacks
 */
 
 /*ENVIRONMENT