search

laracraft-tech / laravel-xhprof

Easy XHProf setup to profile your Laravel application!
MIT License
224 stars 11 forks source link

How about encapsulating the frontend part? #1

Closed a-bashtannik closed 2 years ago

a-bashtannik commented 2 years ago

Hey

I have some ideas on how to make the package better considering you depend on https://github.com/preinheimer/xhprof - one of the ugliest things I have ever seen lol.

How about making 2 things:

We can start on these changes and move forward step by step making the profiling process great.

Sairahcaz commented 2 years ago

Hi @a-bashtannik,

thanks for reaching out!

Ugliness is not so much a problem for me :D I just want to analyze and profile the application, so beautiful interfaces have not so much priority for me now.

Since preinheimers GUI is recommanded here https://www.php.net/manual/en/xhprof.requirements.php I just used it...

But I like your ideas. Which other GUIs do you know?

Right now the XHProfMiddleware is pretty simple. Just include the header.php, is this also that simple with the other GUIs?

a-bashtannik commented 2 years ago

You need some workarounds to make preinheimer/xhprof working, e.g: https://github.com/laracraft-tech/laravel-xhprof/blob/24df9d34f35594d1c67f3aa34f45c182a4a705a0/src/Middleware/XHProfMiddleware.php#L31 Starting the output here will cause errors in some cases (headers were already sent).

Basically, by importing the preinheimer/xhprof we are forced to bring bad experience touching raw global variables and importing an extremely shitty code that can easily affect your app. It could be even ok if you are in development, but not in production, where xhporf is very useful by design.

Moreover, debugging is affected as well, in some cases register_shutdown_function won't be called and you will never know why because of set_error_handler somewhere inside Symfony components.

At least https://github.com/preinheimer/xhprof/tree/master/external must be re-written in a more modern and safe way.

Regarding GUI, to be honest, I don't know anything else, but at least we can wrap the existing preinheimer/xhprof GUI part and deliver it with the package you created.

a-bashtannik commented 2 years ago

Moreover, if you want to collect a huge amount of data on production servers, you have to keep the GUI open because of https://github.com/preinheimer/xhprof/blob/01b6a1992bbacd7575b78e67e9bdf9c0fc1f2188/xhprof_lib/config.sample.php#L52-L54

Sairahcaz commented 2 years ago

Hi @a-bashtannik,

I see your points and I agree that there is a lot that could be done better.

But I use this package only locally, so I do not have problems with security here.

Maintaining a own GUI and rewirte lots of things of the preinheimer package could be done, but I have not the time for that.

Feel free to create a PR for this :thumbsup:

Sairahcaz commented 2 years ago

@a-bashtannik , just wanted to let you know, that we maybe relay now will work on this issue. I'm excited how it will look at the end. But this definitely will take while... :D

a-bashtannik commented 2 years ago

@a-bashtannik , just wanted to let you know, that we maybe relay now will work on this issue. I'm excited how it will look at the end. But this definitely will take while... :D

For sure. I tried and dropped, it is a fulltime job. Good luck to you!