laracraft-tech/laravel-xhprof

How about encapsulating the frontend part?

a-bashtannik opened this issue · 6 comments

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:

  • Encapsulate the https://github.com/preinheimer/xhprof into a resource package to avoid repository clone operation.
  • Make everything configurable in a standard laravel way - today we have to keep 2 copies of the configuration, one for the application and one for the cloned ignored directory.

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

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?

You need some workarounds to make preinheimer/xhprof working, e.g:

setcookie('_profile', 1);

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.

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

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 👍

@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 , 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!