search

humanmade / aws-xray

HM Platform AWS X-Ray Integration
23 stars 4 forks source link

Use register_shutdown_function to catch Fatal errors #22

Closed nathanielks closed 5 years ago

nathanielks commented 5 years ago

This PR changes the way the shutdown function is registered so that it is executed by PHP itself instead of WordPress via the shutdown action. register_shutdown_function is executed on each page request, regardless of whether a Fatal occured.

The functions themselves have been extracted into functions.php so that plugin.php can be used as an actual plugin file. This means we can load functions.php early in the process and run the new initialize() function which populates initial values and starts sampling. This does require a coordinated change downstream, so I recommend a major or minor version update with this release.

nathanielks commented 5 years ago

You can see how the plugin is instantiated now, as in my experimental branch on hm-platform: 

require_once __DIR__ . '/plugins/aws-xray/inc/namespace.php';
XRay\initialize();

Then hm-platform will load plugin.php and register the hooks.

nathanielks commented 5 years ago

Checks are failing but there aren't any linting errors mentioned 🤔

nathanielks commented 5 years ago

I'm testing this on AWS and apparently the functions aren't namespaced correctly, or they aren't included properly. I'm investigating further.

nathanielks commented 5 years ago

I'm getting a number of these errors:

Warning: call_user_func_array() expects parameter 1 to be a valid callback, function 'HM\Platform\XRay\on_cloudwatch_error_handler_error' not found or invalid function name in /usr/src/app/wordpress/wp-includes/class-wp-hook.php on line 286
Warning: call_user_func_array() expects parameter 1 to be a valid callback, function 'HM\Platform\XRay\filter_mysql_query' not found or invalid function name in /usr/src/app/wordpress/wp-includes/class-wp-hook.php on line 286
nathanielks commented 5 years ago

Ah, this has to do with us excluding WP CLI from being profiled in hm-platform. This PR still adds the plugin hooks, regardless of whether it has been bootstrapped at all. Working on a fix.

nathanielks commented 5 years ago

Okie doke, this is resolved, @joehoyle 👍

joehoyle commented 5 years ago

Looks good, can you get a PR set up for HM Platform too, just so I can see all the things that are being removed from load.php in the diff.

nathanielks commented 5 years ago

@joehoyle those changes have been made! I retained a function_exists check in plugin.php because otherwise Cavalcade will complain functions don't exist (because the functions aren't required in load.php for Cavalcade invocations).