-
-
Notifications
You must be signed in to change notification settings - Fork 631
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
feat: Integrate XHGui into DDEV, fixes #6894 #7069
base: main
Are you sure you want to change the base?
Conversation
Download the artifacts for this pull request:
See Testing a PR. |
a242807
to
22f4430
Compare
64d750d
to
4fd2b3f
Compare
Moved to "Ready for Review" even though certainly not perfect. Folks who have used xhgui will be great reviewers. Hope to get @tyler36 's expertise in here. |
b6a57c1
to
a00248c
Compare
Did some basic testing with DDEV
|
Thanks @tyler36 - I didn't implement
|
Not sure what I expected. Just felt a little strange that it opened my browser. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow!
This change also needs an update to the global schema.json
(I will test edge cases later today.)
containers/ddev-webserver/ddev-webserver-base-files/usr/local/bin/enable_xhprof
Outdated
Show resolved
Hide resolved
@tyler36 I think I'll change |
I actually like that it opens the browser. No need to remember anything. (See my suggestion for |
f13688d
to
2e557e9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Old approach doesn't work for me.
(I don't see any records when I refresh the page.)
ddev config global --xhprof-mode-reset && ddev restart && ddev xhprof && ddev launch /xhprof
(Not important) I noticed some inconsistency in --xhprof-mode-reset
(it may also exist in another -reset
flag):
ddev config global --xhprof-mode=xhgui
ddev config --xhprof-mode=prepend
# doesn't reset .ddev/config.yaml
ddev config --xhprof-mode-reset
# but this works
ddev config --xhprof-mode=""
containers/ddev-webserver/ddev-webserver-base-files/usr/local/bin/enable_xhprof
Show resolved
Hide resolved
@@ -34,6 +34,8 @@ var SSHAuthTag = "v1.24.3" | |||
// BusyboxImage is used a couple of places for a quick-pull | |||
var BusyboxImage = "busybox:stable" | |||
|
|||
var XhguiImage = "ddev/ddev-xhgui:20250307_rfay_xhgui" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use the same logic as for other images, i.e. two variables Image and Tag.
It's needed for deleteDdevImages
- which also needs an update to clean old xhgui images.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really have wanted to leave the two-variable technique behind. Hate it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's easier for me to see which tags I need to update (looking at the whole variable) when working on a new release.
9854613
to
49bda54
Compare
Co-authored-by: Stanislav Zhuk <[email protected]>
Co-authored-by: Stanislav Zhuk <[email protected]>
Co-authored-by: Stanislav Zhuk <[email protected]>
49bda54
to
594f140
Compare
I pushed several new commits:
|
I added a sleep to the TestCmdXHGui Improved the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested on ddev version v1.24.3-95-g915cd248b
.
-
✅
ddev xhgui
asks to add global configuration- Manually remove
xhprof-mode
from global config
$ ddev xhgui XHProf Mode is set to 'prepend', can't use 'ddev xhgui'. Use the command below to enable it: ddev config global --xhprof-mode=xhgui && ddev restart
- Manually remove
-
✅
ddev config global --xhprof-mode=xhgui
updates config$ ddev config global --xhprof-mode=xhgui && ddev restart ... Your project can be reached at https://lara11-base-demo.ddev.site
-
✅
ddev xhgui
on starts debugging, confirmed withddev xhgui launch
$ ddev xhgui Container ddev-lara11-base-demo-xhgui Created Container ddev-lara11-base-demo-xhgui Started Started optional compose profiles 'xhgui' Enabled XHProf for XHGui.
-
✅
ddev xhgui status
correctly reports on$ ddev xhgui status XHProf is enabled and capturing performance information. The XHGui service is running and you can access it at https://lara11-base-demo.ddev.site:8142
-
✅
ddev xhgui off
and am no longer able to access website via ddev xhgui lanuch$ ddev xhgui off $ ddev xhgui status XHProf is disabled. XHGui is disabled.
-
✅ delete project database, changed to Postgres 16 and correctly ran ddev xhgui launch
$ ddev delete -Oy $ ddev config --project-type=php --database=postgres:16 $ ddev xhgui
-
✅
ddev xhgui -h
andddev xhgui --help
both correctly display help messgae$ ddev xhgui ddev xhgui -h Starts or checks status of XHGui/XHProf performance monitoring and UI. You can use 'ddev xhgui on' to enable, 'ddev xhgui' or 'ddev xhgui launch' to view the UI. 'ddev xhgui status' shows the status. ...
-
✅
ddev xhgui -j
does nothing (previously opened browser)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Install ddev/ddev-xhgui and then ddev start to see removal
- Validate that xhprof still works with xhprof-mode=prepend
- tested it with
mysql:8.4
- tested it without
db
container, seeing an expected error onddev xhgui
My nitpicks:
-
We should have a named volume for xhgui Dockerfile has an unnamed volume for
/run/nginx
perftools/xhgui#505 -
The xhgui image is pulled up during the first
ddev start
, which may slow down some CI tests, but on the other hand, people may see it and wonder "What is this new image? What does it do?" and start using it, which can give us more feedback onddev xhgui
-
The port configuration can only be done with manual editing for
.ddev/config.yaml
, I don't expect people will use this often, so not asking for new flags inddev config
-
Testing without router worked only when I added
host_xhgui_port: 8142
, without it opens the main website (the host port is not assigned automatically, I don't see it in theddev describe
), -
Not related to this PR, but without router
ddev mailpit
openshttp://127.0.0.1:8025
, and not the actual port fromddev describe
The Issue
Honors to @tyler36, who pioneered and then matured the xhgui add-on until it could be brought into DDEV here.
How This PR Solves The Issue
Add integration for xhgui
TODO
ddev xhgui -j
, perhaps add some supportddev xhgui
should launch browser. Adjust as neededManual Testing Instructions
Docs suggestions:
ddev config global --xhprof-mode=xhgui
ddev xhgui on
ddev xhgui status
ddev xhgui launch
ddev xhprof on
ddev start
to see removalAutomated Testing Overview
Release/Deployment Notes