Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve xhprof availability and revert to just using the classic run list page #3087

Merged
merged 12 commits into from Jul 6, 2021

Conversation

rfay
Copy link
Member

@rfay rfay commented Jul 4, 2021

The Problem/Issue/Bug:

  • The xhprof footer wasn't available in symfony-based CMSs like Drupal 9
  • Sometimes the xhprof page got profiled itself, making a mess of things
  • There was an undefined variable complaint on the xhprof page itself

How this PR Solves The Problem:

  • Abandoned the attempt to add a footer to the profiled page. This completely failed in TYPO3 because it ruined json responses, etc. It would break any API response. You just can't naively add text to a modern webapp's html output.
  • Make sure /xhprof* didn't get profiled
  • Made it work with command-line execution and API and TYPO3

Manual Testing Instructions:

  • Use the ddev download from this build; you can brew unlink ddev and just put the downloaded ddev in your PATH. Alternately, use gitpod by visiting link and you'll have the built item all set up.
  • If your project has a custom .ddev/apache/apache-site.conf please remove it temporarily (the URL for the xhprof app is added in the default apache config)
  • ddev xhprof on
  • Visit a page and profile it. (This gives instructions about the URL with run details)

Automated Testing Overview:

Nothing new

Related Issue Link(s):

Release/Deployment notes:

@gitpod-io
Copy link

gitpod-io bot commented Jul 4, 2021

@rfay
Copy link
Member Author

rfay commented Jul 4, 2021

@LionsAd @penyaskito I'd love if you could take a look at this, especially the rewritten prepend script (and append is gone).

I'm adding manual testing instructions to the OP.

@rfay rfay marked this pull request as ready for review July 4, 2021 05:56
@rfay rfay force-pushed the 20210703_improve_xhprof branch from 841b043 to f00a97b Compare July 5, 2021 21:26
@rfay rfay changed the title Improve xhprof formatting and availability Improve xhprof availability and revert to just using the classic run list page Jul 6, 2021
@rfay rfay merged commit 0ebd0fa into ddev:master Jul 6, 2021
@rfay rfay deleted the 20210703_improve_xhprof branch July 6, 2021 03:05
include_once '/var/www/xhprof/xhprof_lib/utils/xhprof_runs.php';

$xhprof_runs = new XHProfRuns_Default();
$run_id = $xhprof_runs->save_run($xhprof_data, $appNamespace);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we want the ability for a dev user to supply their own script here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have been thinking about that, and definitely agree. Can go so very far into the weeds. I've also thought about having the script optionally add the link at the bottom. But mucking with the output breaks all APIs and all command-line scripts and lots of web pages (like TYPO3).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be cool to have the shutdown function include a file which was easily editable by the user, perhaps empty by default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants