search

Chassis / Chassis_XHGui

A Chassis extension to install and setup XHGui on your server
1 stars 2 forks source link

This breaks functionality that relies on shutdown functions #19

Closed johnbillion closed 4 years ago

johnbillion commented 5 years ago

This is an interesting bug: https://github.com/perftools/xhgui-collector/issues/19

The most visible problem this causes is that Query Monitor doesn't work when Chassis-XHGui is in use, because it uses the shutdown action in WordPress, which is itself attached to a shutdown function which doesn't fire due to the above bug.

Opening this here for visibility and as a reminder that the fix will need to be pulled downstream.

johnbillion commented 5 years ago

Alright, there's a fix in place that requires a config option to be set that disables the call to fastcgi_finish_request().

https://github.com/perftools/xhgui-collector/pull/23

I think it makes sense to set that option to prevent Query Monitor from disappearing. What do you think @BronsonQuick ?

BronsonQuick commented 5 years ago

@johnbillion Awesome. I was just checking my inbox and saw that fix come through.

Yeah that makes sense to me!

BronsonQuick commented 5 years ago

@johnbillion Sorry for the delay on this. I've updated the submodule and the config so it's working again with Query Monitor.

Chassis_Site__Just_another_WordPress_site_2019-08-19_12-52-58
svandragt commented 4 years ago

Unfortunately this is an issue currently.

I install this plugin by adding it to the extensions list in the config.yaml for Chassis. When I load any WP screen with an admin bar, the Query Monitor menu is empty. When I add - chassis/chassis_xhgui to the disabed_extensinos list and run vagrant provision and reload, the menu appears.

joemcgill commented 4 years ago

Can confirm what @svandragt reported. This still seems to be broken. I've tried deleting and reinstalling all extensions and Query Monitor is still broken unless I disable the Chassis XHGUI extension.

BronsonQuick commented 4 years ago

Yeah I'm gonna have to see if I can figure out some kinda logic so that both can work together. I'll have a bit of a play and see if I can come up with something.

BronsonQuick commented 4 years ago

Turns out I needed to add a sha upstream to made sure this was fixed: https://github.com/Chassis/xhgui/compare/daaae4406c0a5a2c3e08e4e97afd53c636b2b2be...b263243e6e4e23a576cd793b732a96ddf62767db#diff-b5d0ee8c97c7abd7e3fa29b9a27d1780

joemcgill commented 4 years ago

@BronsonQuick how should I test this? I tried git pull && git submodule update --recursive inside the extensions/chassis_xhgui directory, but QM is still not showing any output.

BronsonQuick commented 4 years ago

@joemcgill You'll want to delete the vendor directory so then run vagrant provision so that composer installs the Chassis Xhgui Collector fork at the correct Sha

joemcgill commented 4 years ago

Thanks, B. I tried doing this and it didn't work, but deleting both the chassis_xhgui and xhprof directories from extensions and then running vagrant provision did the job 🎉. So nice to have this working!

BronsonQuick commented 4 years ago

Ahh right. Sounds like you needed to do a git submodule sync then. I'm happy to have it working again as well. I still don't really get why composer needed the sha. I should've probably tagged it with a release instead. It's a shame flamegraphs were removed but hey, I guess that gives me another performance project to work on haha