feat: Add toggle option for xhprof command, rework xdebug, fixes #5782

[GuySartorelli]

The Issue

The xdebug command has a toggle option, but xhprof doesn't. They're both really similar in their operation so it makes sense to add a ddev xhprof toggle option.

How This PR Solves The Issue

Adds the option.

Also some unrelated-but-adjacent refactoring of xdebug, which was pretty convoluted.

Manual Testing Instructions

• Try ddev xhprof toggle and validate the status with ddev xhprof status
• Check that hitting tab after typing ddev xhprof gives you toggle amongst the other options.
• Try ddev xdebug toggle and ddev xdebug status as a sanity check for the refactor

Automated Testing Overview

Existing tests should cover the xdebug refactoring.

There's no tests at all for xhprof that I could find, so adding them seems out of scope. Happy to add if maintainers disagree with that assessment though.

Related Issue Link(s)

• xhprof command should have a toggle argument #5782

Release/Deployment Notes

N/A

[github-actions]

Download the artifacts for this pull request:

• all-ddev-executables.zip
• ddev-linux-amd64.zip
• ddev-linux-arm64.zip
• ddev-macos-amd64.zip
• ddev-macos-arm64.zip
• ddev-windows-amd64-installer.zip

See Testing a PR.

• Test on Gitpod.
• Test on GitHub Codespaces.

[rfay]

See

ddev/pkg/ddevapp/ddevapp_test.go

Lines 1097 to 1209 in b0fb8ce

	// TestDdevXhprofEnabled tests running with xhprof_enabled = true, etc.
	func TestDdevXhprofEnabled(t *testing.T) {
	if runtime.GOOS == "darwin" {
	// TODO: Return to this when working on xhprof xhgui etc.
	t.Skip("Skipping on darwin to ignore problems with 'connection reset by peer'")
	}
	assert := asrt.New(t)
	origDir, _ := os.Getwd()
	testcommon.ClearDockerEnv()
	projDir := testcommon.CreateTmpDir(t.Name())
	app, err := ddevapp.NewApp(projDir, false)
	require.NoError(t, err)
	app.Type = nodeps.AppTypePHP
	err = app.WriteConfig()
	require.NoError(t, err)
	// Create the simplest possible php file
	err = fileutil.TemplateStringToFile("<?php\nphpinfo();\n", nil, filepath.Join(app.AppRoot, "index.php"))
	require.NoError(t, err)
	runTime := util.TimeTrackC(fmt.Sprintf("%s %s", app.Name, t.Name()))
	// Does not work with php5.6 anyway (SEGV), for resource conservation
	// skip older unsupported versions
	phpKeys := []string{}
	exclusions := []string{"5.6", "7.0", "7.1", "7.2", "7.3", "7.4", "8.0"}
	for k := range nodeps.ValidPHPVersions {
	if !nodeps.ArrayContainsString(exclusions, k) {
	phpKeys = append(phpKeys, k)
	}
	}
	sort.Strings(phpKeys)
	// Most of the time we can just test with the default PHP version
	if os.Getenv("GOTEST_SHORT") != "" {
	phpKeys = []string{nodeps.PHPDefault}
	}
	err = app.Init(app.AppRoot)
	require.NoError(t, err)
	t.Cleanup(func() {
	err = app.Stop(true, false)
	assert.NoError(err)
	err = os.Chdir(origDir)
	assert.NoError(err)
	_ = os.RemoveAll(projDir)
	})
	webserverKeys := make([]string, 0, len(nodeps.ValidWebserverTypes))
	for k := range nodeps.ValidWebserverTypes {
	if k == nodeps.WebserverNginxGunicorn {
	continue
	}
	webserverKeys = append(webserverKeys, k)
	}
	// Most of the time we can just test with the default webserver_type
	if os.Getenv("GOTEST_SHORT") != "" {
	webserverKeys = []string{nodeps.WebserverDefault}
	}
	for _, webserverKey := range webserverKeys {
	app.WebserverType = webserverKey
	for _, v := range phpKeys {
	t.Logf("Beginning XHProf checks with XHProf webserver_type=%s php%s\n", webserverKey, v)
	fmt.Printf("Attempting XHProf checks with XHProf PHP%s\n", v)
	app.PHPVersion = v
	err = app.Restart()
	require.NoError(t, err)
	stdout, _, err := app.Exec(&ddevapp.ExecOpts{
	Service: "web",
	Cmd: "php --ri xhprof",
	})
	require.Error(t, err)
	assert.Contains(stdout, "Extension 'xhprof' not present")
	// Run with Xhprof enabled
	_, _, err = app.Exec(&ddevapp.ExecOpts{
	Cmd: "enable_xhprof",
	})
	assert.NoError(err)
	stdout, _, err = app.Exec(&ddevapp.ExecOpts{
	Service: "web",
	Cmd: "php --ri xhprof",
	})
	require.NoError(t, err)
	assert.Contains(stdout, "xhprof.output_dir", "xhprof is not enabled for %s", v)
	out, _, err := testcommon.GetLocalHTTPResponse(t, app.GetPrimaryURL(), 2)
	require.NoError(t, err, "Failed to get base URL webserver_type=%s, php_version=%s", webserverKey, v)
	require.Contains(t, out, "module_xhprof")
	out, _, err = testcommon.GetLocalHTTPResponse(t, app.GetPrimaryURL()+"/xhprof/", 2)
	require.NoError(t, err)
	// Output should contain at least one run
	assert.Contains(out, ".ddev.xhprof</a><small>")
	// Disable all to avoid confusion
	_, _, err = app.Exec(&ddevapp.ExecOpts{
	Cmd: "disable_xhprof && rm -rf /tmp/xhprof",
	})
	require.NoError(t, err)
	}
	}
	runTime()
	}

for TestDdevXhprofEnabled.

Your IDE should have been able to find this! :)

[rfay]

Does this mean that you often use xhprof?

[GuySartorelli]

Does this mean that you often use xhprof?

I almost never use it, but if there's anyone using DDEV who does a lot of profiling, I could see this being helpful - while at the same time it adds next to no additional maintenance.

[GuySartorelli]

Your IDE should have been able to find this! :)

If I had done a blanket search for xhprof it would have - I was just expecting a separate file like xdebug has since the commands are basically identical.

Looking at that test, it's testing the underlying enable_xhprof and disable_xhprof functions, not the ddev xhprof command itself. Unless I missed something.

[rfay]

The xhprof command just runs enable_xhprof, etc.

I found the test by using symbol search and typing TestXhprof and it brought it right up (Goland)

[GuySartorelli]

The xhprof command just runs enable_xhprof, etc.

By that argument there's no need for me to update any tests, because that's still what it's doing. That doesn't test whether the existing ddev xhprof status command outputs correctly or whether this new toggle option works as expected - but as I say, that's a completely different test than the one that currently exists.

[GuySartorelli]

I'm happy to add testing coverage for the actual ddev xhprof command itself if you want - just noting that that's not what the current test covers, which is why I didn't add tests for the toggle option there.

[rfay]

It's all good, no need to go further IMO. Maybe when we integrate xhprof more thoroughly later this year with integrating the ddev-xhgui behavior.

[stasadev]

Tested, looks good to me 👍

P.S. Nice refactoring for the xdebug command, thanks!

[rfay]

In testing this PR I did ddev xdebug toggle a number of times. When xdebug was not enabled, it did the right thing. But often when xdebug was enabled, it would hang. The hang is in php -r 'echo ini_get("xdebug.mode");'.

I was testing on macOS, and where this happened I was on php 8.2

• It has nothing to do with this PR. I tested with HEAD and with v1.23.0 and saw the same.
• It doesn't matter whether mutagen is enabled or not
• It happens on xdebug 3.2.2 or other
• ddev xdebug status hangs for the same reason when xdebug is enabled

[GuySartorelli]

Ohhh, interesting. I guess that's 'cause in order to get the xdebug status, we're executing php code while xdebug is enabled - and xdebug is trying to connect to an IDE or anything that'll listen, and not finding anything to connect to?

Probably we should use php -i, or check the INI file(s) directly, or something else that doesn't require executing code via PHP to get the status.

[GuySartorelli]

php --ri xdebug will exit with a 0 status if xdebug is enabled, and a non-0 status if it's disabled, so that's another option.
That should work regardless of xdebug version too, so would simplify the command slightly.

[rfay]

You're right, congratulations! This doesn't happen if PhpStorm IDE isn't listening!

[rfay]

I think that php --version, which is widely used in tests and such is probably an extremely robust technique. php --version | grep xdebug for example.

[rfay]

Followups in

• ddev xdebug toggle and ddev xdebug status hang if xdebug is enabled and an IDE is listening for xdebug #6191

[rfay]

This is fine, thanks so much, and sorry for the long wait.

I've asked our local xhprof expert, @tyler36, to take a quick look, but this can be pulled in the morning if he either has no objections or can't get to it.

[rfay]

Wow, I forgot all about Xdebug v2. But I guess we still support several PHP versions that can only have that.

[rfay]

This could be simpler with

if php -m | grep xhprof >/dev/null; then

but it's fine the way it is.

[tyler36]

XHprof works as expected. No conflicts with tyler36/ddev-xhgui.
xdebug also continues to function after "rework".

Seems good to me.

Test

1. Confirm DDEV version

$ ddev -v                                                          
ddev version v1.23.0-beta1-23-gbb51e6fc8

2. Start a new Drupal 10 project following quick start guide.
3. Toggle xhprof

$ ddev xhprof toggle
Enabled xhprof.
After each web request or CLI process you can see all runs, most recent first, at
https://xhprof-test.ddev.site/xhprof

4. Visit https://xhprof-test.ddev.site/xhprof to confirm working.
5. Refresh page to confirm XHprof is generating content

Test with tyler36/ddev-xhgui

1. Install tyler36/ddev-xhgui addon

ddev get tyler36/ddev-xhgui
ddev restart

2. Following Drupal instructions, add perftools/php-profiler

ddev composer require perftools/php-profiler --dev

3. Following Drupal instructions, update web/sites/default/settings.php

if (file_exists("/mnt/ddev_config/xhgui/collector/xhgui.collector.php")) {
 require_once "/mnt/ddev_config/xhgui/collector/xhgui.collector.php";
}

4. Toggle xhprof

ddev xhprof toggle

5. Launch XHGui page

ddev xhgui

6. Visit main Drupal page and confirm reports are generated in XHGui.

Test Xdebug still functions

1. Add breakpoint to web/index.php on lin 16: $kernel = new DrupalKernel('prod', $autoloader);
2. Start xdebug listening in Vscode
3. Toggle xdebug on

ddev xdebug on

4. Attempt to homepage; page does NOT finish loading as breakpoint is hit (expected behavior).